[Tech debt] Enforce one active Poll response per identified respondent #1

New Issue

zemion · 2026-07-20T17:30:30+02:00

zemion commented

2026-07-20 17:30:30 +02:00

Problem

Identified Poll responses currently use an application-level read-or-create path. The Scheduling integration now serializes that path by locking the parent Poll row, but the database does not enforce the invariant that one active response exists per (poll_id, respondent_id). SQLite ignores FOR UPDATE, future writers may bypass the service, and locking one Poll serializes unrelated respondents.

Acceptance criteria

Add a Poll migration after 2a3b4c5d6e7f that finds duplicate active, non-anonymous responses and keeps the deterministic latest row by (submitted_at DESC, id DESC).
Tombstone older duplicates by setting both deleted_at and updated_at to the migration timestamp; leave anonymous responses and already-deleted rows unchanged.
Add and ORM-mirror uq_poll_responses_active_respondent on (poll_id, respondent_id) with deleted_at IS NULL AND respondent_id IS NOT NULL predicates for PostgreSQL and SQLite.
Keep the existing non-unique lookup index unless measurements justify replacing it.
Make submission race-safe with a nested transaction: on this specific uniqueness conflict, load the winning row and apply normal update/not-allowed semantics; do not mask unrelated integrity errors.
Make _existing_response use the same deterministic submitted_at DESC, id DESC ordering.
Cover migration deduplication, anonymous responses, update-disabled behavior, and the invariant in SQLite; add a real two-session PostgreSQL concurrency check.
Update the migration/release baseline and document that index creation requires a quiesced write path or the platform maintenance lock.

Context

Discovered while implementing govoplan-scheduling#4. The current parent-row lock is a safe interim guard, not a substitute for the database invariant.

## Problem Identified Poll responses currently use an application-level read-or-create path. The Scheduling integration now serializes that path by locking the parent Poll row, but the database does not enforce the invariant that one active response exists per `(poll_id, respondent_id)`. SQLite ignores `FOR UPDATE`, future writers may bypass the service, and locking one Poll serializes unrelated respondents. ## Acceptance criteria - Add a Poll migration after `2a3b4c5d6e7f` that finds duplicate active, non-anonymous responses and keeps the deterministic latest row by `(submitted_at DESC, id DESC)`. - Tombstone older duplicates by setting both `deleted_at` and `updated_at` to the migration timestamp; leave anonymous responses and already-deleted rows unchanged. - Add and ORM-mirror `uq_poll_responses_active_respondent` on `(poll_id, respondent_id)` with `deleted_at IS NULL AND respondent_id IS NOT NULL` predicates for PostgreSQL and SQLite. - Keep the existing non-unique lookup index unless measurements justify replacing it. - Make submission race-safe with a nested transaction: on this specific uniqueness conflict, load the winning row and apply normal update/not-allowed semantics; do not mask unrelated integrity errors. - Make `_existing_response` use the same deterministic `submitted_at DESC, id DESC` ordering. - Cover migration deduplication, anonymous responses, update-disabled behavior, and the invariant in SQLite; add a real two-session PostgreSQL concurrency check. - Update the migration/release baseline and document that index creation requires a quiesced write path or the platform maintenance lock. ## Context Discovered while implementing `govoplan-scheduling#4`. The current parent-row lock is a safe interim guard, not a substitute for the database invariant.

zemion commented

2026-07-20 17:36:40 +02:00

Codex State: ready

Summary

Local commit e7f3b1d adds an interim parent-Poll row lock around response read-or-create and closes the immediate Scheduling race through the normal service path.
The database invariant, migration deduplication, savepoint reconciliation, and concurrent PostgreSQL verification described in this ticket remain intentionally unimplemented.

Next / Blocked

Implement as a separate migration/release slice; consider FOR SHARE for Poll lifecycle coordination so unrelated respondents are not serialized after the unique index exists.

Suggested status label: status/ready

## Codex State: ready ### Summary - Local commit e7f3b1d adds an interim parent-Poll row lock around response read-or-create and closes the immediate Scheduling race through the normal service path. - The database invariant, migration deduplication, savepoint reconciliation, and concurrent PostgreSQL verification described in this ticket remain intentionally unimplemented. ### Next / Blocked - Implement as a separate migration/release slice; consider FOR SHARE for Poll lifecycle coordination so unrelated respondents are not serialized after the unique index exists. Suggested status label: `status/ready`

zemion commented

2026-07-22 02:54:53 +02:00

Codex State: progress

Summary

Local commits cac7733 and eb00320 implement the active identified-response invariant, deterministic deduplication, shared lifecycle locking, and savepoint race reconciliation.

Changed Files

src/govoplan_poll/backend/db/models.py
src/govoplan_poll/backend/migrations/versions/6e7f8a9b0c1d_v0110_poll_response_uniqueness.py
src/govoplan_poll/backend/service.py
src/govoplan_poll/backend/participation_service.py
tests/test_migrations.py
tests/test_response_uniqueness.py
README.md

Verification

Ruff: all checks passed.
Pytest with optional local PostgreSQL integration enabled: 55 passed.
Release migration audit: graph_errors=[]; Poll head 6e7f8a9b0c1d.

Next / Blocked

Commits are intentionally not pushed yet, pending parent integration.
Record 6e7f8a9b0c1d in the migration baseline only with the reviewed release; the baseline is not an unreleased-head ledger.

Suggested status label: status/in-progress

## Codex State: progress ### Summary - Local commits cac7733 and eb00320 implement the active identified-response invariant, deterministic deduplication, shared lifecycle locking, and savepoint race reconciliation. ### Changed Files - `src/govoplan_poll/backend/db/models.py` - `src/govoplan_poll/backend/migrations/versions/6e7f8a9b0c1d_v0110_poll_response_uniqueness.py` - `src/govoplan_poll/backend/service.py` - `src/govoplan_poll/backend/participation_service.py` - `tests/test_migrations.py` - `tests/test_response_uniqueness.py` - `README.md` ### Verification - `Ruff: all checks passed.` - `Pytest with optional local PostgreSQL integration enabled: 55 passed.` - `Release migration audit: graph_errors=[]; Poll head 6e7f8a9b0c1d.` ### Next / Blocked - Commits are intentionally not pushed yet, pending parent integration. - Record 6e7f8a9b0c1d in the migration baseline only with the reviewed release; the baseline is not an unreleased-head ledger. Suggested status label: `status/in-progress`

zemion commented

2026-07-22 03:28:31 +02:00

Codex State: progress

Summary

Implemented a database-enforced partial uniqueness invariant for one active identified response, deterministic migration cleanup, and savepoint-based concurrent submission reconciliation.

Changed Files

migrations/versions/6e7f8a9b0c1d_enforce_active_respondent_uniqueness.py

Verification

Poll suite: 59 passed, 1 optional test skipped.

Next / Blocked

Publish the reviewable commits and advance the migration release baseline only as part of the reviewed release.

Suggested status label: status/in-progress

## Codex State: progress ### Summary - Implemented a database-enforced partial uniqueness invariant for one active identified response, deterministic migration cleanup, and savepoint-based concurrent submission reconciliation. ### Changed Files - `migrations/versions/6e7f8a9b0c1d_enforce_active_respondent_uniqueness.py` ### Verification - `Poll suite: 59 passed, 1 optional test skipped.` ### Next / Blocked - Publish the reviewable commits and advance the migration release baseline only as part of the reviewed release. Suggested status label: `status/in-progress`

zemion commented

2026-07-22 08:16:40 +02:00

Codex State: progress

Summary

The migration, deterministic deduplication, partial unique index, race-safe nested-transaction reconciliation, ORM mirror, and SQLite coverage are integrated on remote main at v0.1.11.

Verification

Poll focused migration/uniqueness/lifecycle suite: 21 passed, 1 PostgreSQL-only concurrency test skipped

Next / Blocked

Keep open until the real two-session PostgreSQL concurrency check passes. This environment can reach the host Docker CLI through flatpak-spawn, but the current login lacks /var/run/docker.sock permission and non-interactive sudo requires a password.

Suggested status label: status/in-progress

## Codex State: progress ### Summary - The migration, deterministic deduplication, partial unique index, race-safe nested-transaction reconciliation, ORM mirror, and SQLite coverage are integrated on remote main at v0.1.11. ### Verification - `Poll focused migration/uniqueness/lifecycle suite: 21 passed, 1 PostgreSQL-only concurrency test skipped` ### Next / Blocked - Keep open until the real two-session PostgreSQL concurrency check passes. This environment can reach the host Docker CLI through flatpak-spawn, but the current login lacks /var/run/docker.sock permission and non-interactive sudo requires a password. Suggested status label: `status/in-progress`

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: GovOPlaN/govoplan-poll#1