chore(occupancy): third-pass P3 follow-ups from PR #170 #276

New issue

Open

opened 2026-10-03 20:34:11 +00:00 by gabogg · 0 comments

gabogg commented

2026-10-03 20:34:11 +00:00

Owner

Third-pass P3 findings from PR #170 (spec #163). Not needed to deliver M1; none blocks merge. Earlier P3s are in #273.

CHECK widening doesn't reach dev DBs from earlier branch builds. app/db/database.py:866, :946: CREATE TABLE IF NOT EXISTS keeps the old CHECK, so #171's status='APPROVED' fails on a dev/validation DB that ran an earlier #170 build. Prod never had these tables. The PR also adds ALTER TABLE ADD COLUMN migrations (status_reason, is_exception, exception_name, database.py:908-917) for tables that never shipped (same dead-migration pattern as #273). Pick one: drop the branch-era ALTERs and document dropping flow_history_curation_* on dev DBs, or rebuild when the CHECK differs.
Domain gates duplicated in the repository. prepare_curation_draft_record_async (app/db/flow_history_repository.py:900-932) re-implements the camera-mismatch, schedule-confirmed and unreviewed-days gates with broad except Exception. The in-lock revision check already makes them redundant; keep only the revision/status guard in the repository.
HolidayHint.name: str vs the null convention from #222. Unnamed exceptions now yield "" from occupancy_holidays.name. Use str | None and normalise blank to None.
camera_index_codes: [] means "all cameras". request.camera_index_codes or cameras (app/services/flow_history_service.py:1198). Reject an empty list, or treat only None as all.
Holiday rename leaves exception_name stale. flow_history_service.py:1095 keeps day_row.exception_name while holiday_name changes. Keep them in sync or drop the duplicate.
Orphaned running fetch blocks draft creation. A running row left by a crashed process makes POST draft return 409 (flow_history_service.py:596) until another fetch starts, because only start_fetch_async marks dead rows interrupted. Check for a live task the same way.
Test-only fallback in get_current_revision_async. MAX(revision) from flow_history_fetches (flow_history_repository.py:398-413) exists only for test seeding that skips flow_history_months. Seed the row in tests and drop the fallback.

Third-pass P3 findings from PR #170 (spec #163). Not needed to deliver M1; none blocks merge. Earlier P3s are in #273. - [ ] **CHECK widening doesn't reach dev DBs from earlier branch builds.** `app/db/database.py:866`, `:946`: `CREATE TABLE IF NOT EXISTS` keeps the old CHECK, so #171's `status='APPROVED'` fails on a dev/validation DB that ran an earlier #170 build. Prod never had these tables. The PR also adds `ALTER TABLE ADD COLUMN` migrations (`status_reason`, `is_exception`, `exception_name`, `database.py:908-917`) for tables that never shipped (same dead-migration pattern as #273). Pick one: drop the branch-era ALTERs and document dropping `flow_history_curation_*` on dev DBs, or rebuild when the CHECK differs. - [ ] **Domain gates duplicated in the repository.** `prepare_curation_draft_record_async` (`app/db/flow_history_repository.py:900-932`) re-implements the camera-mismatch, schedule-confirmed and unreviewed-days gates with broad `except Exception`. The in-lock revision check already makes them redundant; keep only the revision/status guard in the repository. - [ ] **`HolidayHint.name: str` vs the null convention from #222.** Unnamed exceptions now yield `""` from `occupancy_holidays.name`. Use `str | None` and normalise blank to `None`. - [ ] **`camera_index_codes: []` means "all cameras".** `request.camera_index_codes or cameras` (`app/services/flow_history_service.py:1198`). Reject an empty list, or treat only `None` as all. - [ ] **Holiday rename leaves `exception_name` stale.** `flow_history_service.py:1095` keeps `day_row.exception_name` while `holiday_name` changes. Keep them in sync or drop the duplicate. - [ ] **Orphaned `running` fetch blocks draft creation.** A `running` row left by a crashed process makes POST draft return 409 (`flow_history_service.py:596`) until another fetch starts, because only `start_fetch_async` marks dead rows interrupted. Check for a live task the same way. - [ ] **Test-only fallback in `get_current_revision_async`.** `MAX(revision)` from `flow_history_fetches` (`flow_history_repository.py:398-413`) exists only for test seeding that skips `flow_history_months`. Seed the row in tests and drop the fallback.

gabogg added the

ready-for-agent

low-priority

labels

2026-10-03 20:34:11 +00:00