Sync ORM index metadata with DB, add partial unique index for dashboard dedup rows

claude-bot commented

2026-07-18 05:31:22 +00:00

Contributor

Six live indexes exist only in migrations, not models (sent_alerts
location_id/nws_alert_id/expires, locations nws_state/enabled, sent_forecasts
sent_date) — autogenerate would propose dropping them. Add index=True to
match (no schema change). The sent_alerts unique constraint includes nullable
channel_id, so dashboard rows have no DB-enforced dedup (Postgres NULL
semantics); the SELECT-then-INSERT check (alert_processor.py:584-591) is
unprotected. Add CREATE UNIQUE INDEX ... ON sent_alerts (nws_alert_id, location_id) WHERE channel_id IS NULL and a composite (location_id, channel_id) index for the dedup hot path. Optionally drop the redundant
standalone indexes duplicating unique-constraint prefixes, and add an env.py
comment noting the ALTER TYPE ADD VALUE single-transaction pitfall (0007) for
future migration authors.

Acceptance criteria:

alembic revision --autogenerate on a clean head produces an empty diff
Partial unique index prevents duplicate dashboard rows (Postgres-tier test)
Dedup lookup uses an index (EXPLAIN spot-check)

Filed from the 2026-07-17 codebase audit (docs/.internal/report-2026-07-17.md), finding F-18.

Six live indexes exist only in migrations, not models (sent_alerts location_id/nws_alert_id/expires, locations nws_state/enabled, sent_forecasts sent_date) — autogenerate would propose dropping them. Add `index=True` to match (no schema change). The sent_alerts unique constraint includes nullable channel_id, so dashboard rows have no DB-enforced dedup (Postgres NULL semantics); the SELECT-then-INSERT check (alert_processor.py:584-591) is unprotected. Add `CREATE UNIQUE INDEX ... ON sent_alerts (nws_alert_id, location_id) WHERE channel_id IS NULL` and a composite `(location_id, channel_id)` index for the dedup hot path. Optionally drop the redundant standalone indexes duplicating unique-constraint prefixes, and add an env.py comment noting the ALTER TYPE ADD VALUE single-transaction pitfall (0007) for future migration authors. **Acceptance criteria:** - [ ] `alembic revision --autogenerate` on a clean head produces an empty diff - [ ] Partial unique index prevents duplicate dashboard rows (Postgres-tier test) - [ ] Dedup lookup uses an index (EXPLAIN spot-check) --- _Filed from the 2026-07-17 codebase audit (`docs/.internal/report-2026-07-17.md`), finding F-18._

claude-bot added this to the v1.4.0 - Codebase health & maintainability milestone

2026-07-18 05:31:22 +00:00

claude-bot added the

type:maintenance

label

2026-07-18 05:31:22 +00:00

rbrooks referenced this issue from a commit

2026-07-18 14:57:55 +00:00

Add Postgres integration test tier and close named coverage gaps

claude-bot referenced this issue

2026-07-18 14:58:13 +00:00

v1.1.0 Phase 1: Postgres test tier + CI hardening #94

claude-bot referenced this issue

2026-07-18 15:49:33 +00:00

Add a Postgres integration test tier and close named coverage gaps #68

rbrooks referenced this issue from a commit

2026-07-19 03:08:14 +00:00

Sync ORM index metadata with DB; close dashboard-dedup NULL gap (#57)

rbrooks referenced this issue from a commit

2026-07-19 03:08:41 +00:00

Sync ORM index metadata with DB; close dashboard-dedup NULL gap (#57)

claude-bot referenced this issue from a pull request that will close it,

2026-07-19 03:08:58 +00:00

v1.4.0 Phase 1: ORM/DB index sync + dashboard-dedup gap (#57) and UI consistency pass (#69) #107

claude-bot closed this issue

2026-07-19 03:16:01 +00:00

claude-bot referenced this issue from a commit

2026-07-19 03:16:01 +00:00

Merge pull request 'v1.4.0 Phase 1: ORM/DB index sync + dashboard-dedup gap (#57) and UI consistency pass (#69)' (#107) from feat/v1.4.0-phase1 into main

claude-bot commented

2026-07-19 03:16:13 +00:00

Author

Contributor

Done in #107 (merged). Verified against a live-schema Postgres on the dev server: alembic revision --autogenerate now yields a genuinely empty diff, migration 0024 applies on upgrade head, bulk suite 658 passed, postgres tier 4 passed.

Scope turned out broader than the six enumerated indexes — autogenerate flagged mismatches on six further tables, all reconciled (model-metadata-only, no schema change):

Index metadata added to match migration-only indexes: sent_alerts (location_id, nws_alert_id, expires), locations (nws_state, enabled), sent_forecasts (sent_date), notification_deliveries (sent_alert_id + composite ix_notification_deliveries_due).
Unique-constraint-vs-unique-index drift reconciled on ai_summary_policies, product_freshness, public_alert_pages, public_tokens, redirect_tokens (models now declare the named uq_* constraint + non-unique ix_* index the DB actually has).
Migration 0024: partial unique index uq_sent_alerts_dashboard_dedup (nws_alert_id, location_id) WHERE channel_id IS NULL + (location_id, channel_id) hot-path index; new postgres-tier test asserts it rejects a duplicate dashboard row.
Also fixed: alembic/script.py.mako was never committed, so alembic revision failed on any fresh clone — that's what blocked the empty-diff check; added the standard template. env.py now notes the ALTER TYPE ADD VALUE single-transaction pitfall.

Follow-up noted (not blocking): the SELECT-then-insert dashboard dedup is race-safe today under the scheduler's max_instances=1, so the new index is defensive; if WeatherBot ever runs multiple app instances or a concurrent injector, add a savepoint-based IntegrityError handler around the insert so a race degrades gracefully instead of raising.

Done in #107 (merged). Verified against a live-schema Postgres on the dev server: **`alembic revision --autogenerate` now yields a genuinely empty diff**, migration 0024 applies on `upgrade head`, bulk suite 658 passed, postgres tier 4 passed. Scope turned out broader than the six enumerated indexes — autogenerate flagged mismatches on six further tables, all reconciled (model-metadata-only, no schema change): - Index metadata added to match migration-only indexes: sent_alerts (location_id, nws_alert_id, expires), locations (nws_state, enabled), sent_forecasts (sent_date), notification_deliveries (sent_alert_id + composite `ix_notification_deliveries_due`). - Unique-constraint-vs-unique-index drift reconciled on ai_summary_policies, product_freshness, public_alert_pages, public_tokens, redirect_tokens (models now declare the named `uq_*` constraint + non-unique `ix_*` index the DB actually has). - Migration 0024: partial unique index `uq_sent_alerts_dashboard_dedup (nws_alert_id, location_id) WHERE channel_id IS NULL` + `(location_id, channel_id)` hot-path index; new postgres-tier test asserts it rejects a duplicate dashboard row. - **Also fixed:** `alembic/script.py.mako` was never committed, so `alembic revision` failed on any fresh clone — that's what blocked the empty-diff check; added the standard template. env.py now notes the ALTER TYPE ADD VALUE single-transaction pitfall. **Follow-up noted (not blocking):** the SELECT-then-insert dashboard dedup is race-safe today under the scheduler's `max_instances=1`, so the new index is defensive; if WeatherBot ever runs multiple app instances or a concurrent injector, add a savepoint-based `IntegrityError` handler around the insert so a race degrades gracefully instead of raising.

rbrooks referenced this issue from a commit

2026-07-19 03:53:45 +00:00

Fix dashboard-dedup migration + make SPC dashboard inserts conflict-safe

claude-bot referenced this issue

2026-07-19 03:53:58 +00:00

Fix: de-dup before the 0024 unique index + conflict-safe SPC dashboard inserts #109

claude-bot commented

2026-07-19 04:08:49 +00:00

Author

Contributor

⚠️ Deploy follow-up (fixed in #109): the initial #107 deploy to dev failed and rolled back — migration 0024's partial UNIQUE index couldn't be built because the dev DB already held 18 groups of duplicate channel_id=NULL SPC dashboard rows (up to 147 copies each). Root cause: poll_spc and poll_spc_dashboard (two independent 5-min jobs) race the same SELECT-then-INSERT for a dashboard record. The original PR's empty-throwaway-DB test didn't surface this.

#109 fixes it: migration 0024 now de-duplicates (keep newest per group) before creating the index, and both NULL-dashboard insert sites are now savepoint-guarded so a lost race is handled as "already exists" instead of poisoning the poll transaction. Verified end-to-end: deployed to dev (v1.3.0-5-g98fbfd8), DB at 0024, 0 duplicate groups, healthy. This also validates the empty-diff/index work from this issue against real data.

⚠️ Deploy follow-up (fixed in #109): the initial #107 deploy to dev **failed and rolled back** — migration 0024's partial UNIQUE index couldn't be built because the dev DB already held 18 groups of duplicate `channel_id=NULL` SPC dashboard rows (up to 147 copies each). Root cause: `poll_spc` and `poll_spc_dashboard` (two independent 5-min jobs) race the same SELECT-then-INSERT for a dashboard record. The original PR's empty-throwaway-DB test didn't surface this. #109 fixes it: migration 0024 now de-duplicates (keep newest per group) before creating the index, and both NULL-dashboard insert sites are now savepoint-guarded so a lost race is handled as "already exists" instead of poisoning the poll transaction. **Verified end-to-end**: deployed to dev (`v1.3.0-5-g98fbfd8`), DB at 0024, 0 duplicate groups, healthy. This also validates the empty-diff/index work from this issue against real data.

claude-bot referenced this issue

2026-07-19 04:29:43 +00:00

Add v1.4.0 changelog #111

Rows
Columns

Sync ORM index metadata with DB, add partial unique index for dashboard dedup rows #57