refactor(occupancy): one query shape for counts-over-time; delete get_hourly_flow_distribution_async

gabogg commented

2026-09-22 16:31:16 +00:00

Owner

Spun out of #35 during the grilling session of 2026-09-22. #35's fix (1) — adding the missing join — lands in the camera-group join cutover (phase 2). This issue is #35's fix (3), the structural half.

Problem

There are two query shapes for "counts over time", and they have already drifted once.

get_bucketed_cycle_flow_async (app/db/occupancy_repository.py:1471) has the camera join, the is_excluded = 0 AND is_active = 1 filter, a direction split and bucket parameterisation.

get_hourly_flow_distribution_async (:1391) has none of them — no join, no filter, a single SUM(count) with no direction split, and it skips empty hours entirely, so len(rows) is "active hours", not 24:

SELECT CAST((timestamp_epoch - ?) / 3600 AS INTEGER) AS hour_idx, SUM(count) AS hourly_vol
FROM people_counting_events
WHERE timestamp_epoch >= ? AND timestamp_epoch < ?
GROUP BY hour_idx

It is the sole input to trust rules R1 (temporal dispersion) and R2 (burst concentration) in evaluate_cycle_integrity_async — so the engine deciding whether a cycle is trustworthy reads a different dataset from the one whose numbers get published.

Why a second issue

Adding the join (phase 2) makes the two queries agree today. It does not stop them drifting again, because there are still two of them. #35 says it directly:

Consider deleting the function and reimplementing R1/R2 on top of get_bucketed_cycle_flow_async(bucket_seconds=3600), so there is exactly one query shape for "counts over time" and it cannot drift again.

It was split out because phase 2 already carries a six-query cutover, a sync rewrite and a deck change; folding in a rewrite of the trust-rule input would widen the blast radius on a PR whose correctness is hard enough to review as is.

Note for the statistics deck

The deck's diurnal series needs 24 dense buckets split by direction, with occupancy and CI bounds per bucket (DiurnalTimeseriesBucket, app/schemas/occupancy_models.py). It should be built on get_bucketed_cycle_flow_async, not by extending the unfiltered query.

Acceptance criteria

R1 and R2 read get_bucketed_cycle_flow_async(bucket_seconds=3600).
get_hourly_flow_distribution_async deleted.
Exactly one query shape for counts-over-time remains.
Test: excluding a camera group changes the KPI totals and the trust-rule input identically.
Trust verdicts unchanged for cycles where the two datasets already agreed.

🤖 Generated with Claude Code

Triage resolution — 2026-09-23

Waits on PR #54 (amended 2026-09-23). PR #54's scope narrowed: it now only gives
get_hourly_flow_distribution_async the same join and filter as the KPI queries, and the
full camera-group join cutover moved to #62 (1:1 camera groups, ADR 0005 on #54). #58 still
waits on #54 because it removes the function #54 edits. Verify #54 has merged before starting.

This is a strict consolidation of the counts-over-time query, preserving trust
verdicts wherever the underlying datasets already agree. Any change to trust
semantics requires separate acceptance criteria and separate scope.

Acceptance:

R1/R2 consume get_bucketed_cycle_flow_async(bucket_seconds=3600) and
get_hourly_flow_distribution_async is removed.
Group sparse directional rows by distinct hour and sum IN and OUT volume
before applying trust rules. Do not count direction rows as separate hours.
Preserve hours containing stored zero-count events as represented hours.
Do not pad to 24 hours before computing R1.
Preserve the existing verdict for cycles with no events.
Dense chart series may fill missing buckets for display, without changing
the sparse-hour meaning used by trust evaluation.
Tests demonstrate unchanged verdicts for matching datasets, plus identical
camera exclusion effects on KPI totals and trust-rule inputs. The camera-group
variant of this test follows #62.

Spun out of #35 during the grilling session of 2026-09-22. #35's fix (1) — adding the missing join — lands in the camera-group join cutover (phase 2). This issue is #35's **fix (3)**, the structural half. ## Problem There are two query shapes for "counts over time", and they have already drifted once. `get_bucketed_cycle_flow_async` (`app/db/occupancy_repository.py:1471`) has the camera join, the `is_excluded = 0 AND is_active = 1` filter, a direction split and bucket parameterisation. `get_hourly_flow_distribution_async` (`:1391`) has **none of them** — no join, no filter, a single `SUM(count)` with no direction split, and it skips empty hours entirely, so `len(rows)` is "active hours", not 24: ```sql SELECT CAST((timestamp_epoch - ?) / 3600 AS INTEGER) AS hour_idx, SUM(count) AS hourly_vol FROM people_counting_events WHERE timestamp_epoch >= ? AND timestamp_epoch < ? GROUP BY hour_idx ``` It is the sole input to trust rules R1 (temporal dispersion) and R2 (burst concentration) in `evaluate_cycle_integrity_async` — so the engine deciding whether a cycle is trustworthy reads a different dataset from the one whose numbers get published. ## Why a second issue Adding the join (phase 2) makes the two queries agree **today**. It does not stop them drifting again, because there are still two of them. #35 says it directly: > Consider deleting the function and reimplementing R1/R2 on top of `get_bucketed_cycle_flow_async(bucket_seconds=3600)`, so there is exactly one query shape for "counts over time" and it cannot drift again. It was split out because phase 2 already carries a six-query cutover, a sync rewrite and a deck change; folding in a rewrite of the trust-rule input would widen the blast radius on a PR whose correctness is hard enough to review as is. ## Note for the statistics deck The deck's diurnal series needs 24 **dense** buckets split by direction, with occupancy and CI bounds per bucket (`DiurnalTimeseriesBucket`, `app/schemas/occupancy_models.py`). It should be built on `get_bucketed_cycle_flow_async`, not by extending the unfiltered query. ## Acceptance criteria - [ ] R1 and R2 read `get_bucketed_cycle_flow_async(bucket_seconds=3600)`. - [ ] `get_hourly_flow_distribution_async` deleted. - [ ] Exactly one query shape for counts-over-time remains. - [ ] Test: excluding a camera group changes the KPI totals and the trust-rule input identically. - [ ] Trust verdicts unchanged for cycles where the two datasets already agreed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ### Triage resolution — 2026-09-23 **Waits on PR #54** (amended 2026-09-23). PR #54's scope narrowed: it now only gives `get_hourly_flow_distribution_async` the same join and filter as the KPI queries, and the full camera-group join cutover moved to #62 (1:1 camera groups, ADR 0005 on #54). #58 still waits on #54 because it removes the function #54 edits. Verify #54 has merged before starting. This is a strict consolidation of the counts-over-time query, preserving trust verdicts wherever the underlying datasets already agree. Any change to trust semantics requires separate acceptance criteria and separate scope. Acceptance: - [ ] R1/R2 consume `get_bucketed_cycle_flow_async(bucket_seconds=3600)` and `get_hourly_flow_distribution_async` is removed. - [ ] Group sparse directional rows by distinct hour and sum IN and OUT volume before applying trust rules. Do not count direction rows as separate hours. - [ ] Preserve hours containing stored zero-count events as represented hours. Do not pad to 24 hours before computing R1. - [ ] Preserve the existing verdict for cycles with no events. - [ ] Dense chart series may fill missing buckets for display, without changing the sparse-hour meaning used by trust evaluation. - [ ] Tests demonstrate unchanged verdicts for matching datasets, plus identical camera exclusion effects on KPI totals and trust-rule inputs. The camera-group variant of this test follows #62.

gabogg added the

needs-triage

label

2026-09-22 16:32:06 +00:00

gabogg added

ready-for-agent

and removed

needs-triage

labels

2026-09-23 14:31:01 +00:00

gabogg referenced this issue

2026-09-23 16:29:51 +00:00

docs: align calibration glossary and record triage decisions #63

gabogg referenced this issue from a commit

2026-09-23 18:00:28 +00:00

docs: address PR #63 review — policy-free glossary, current #58 dependency

gabogg commented

2026-09-23 18:00:59 +00:00

Author

Owner

Dependency amended (2026-09-23), following the pre-merge review of PR #63.

The body said this was blocked on "the camera-group join cutover in PR #54". That cutover moved to #62. PR #54 now only aligns get_hourly_flow_distribution_async with the KPI queries' join and filter. This issue still waits on #54, because it removes the function #54 edits, but the camera-group exclusion test can only be written after #62. The blocking paragraph and the test criterion above were updated to match, as was the triage record in docs/audit/issue-triage-2026-09-23.md.

**Dependency amended** (2026-09-23), following the pre-merge review of PR #63. The body said this was blocked on "the camera-group join cutover in PR #54". That cutover moved to **#62**. PR #54 now only aligns `get_hourly_flow_distribution_async` with the KPI queries' join and filter. This issue still waits on #54, because it removes the function #54 edits, but the camera-group exclusion test can only be written after #62. The blocking paragraph and the test criterion above were updated to match, as was the triage record in `docs/audit/issue-triage-2026-09-23.md`.

gabogg referenced this issue

2026-09-23 18:01:23 +00:00

docs: align calibration glossary and record triage decisions #63

gabogg referenced this issue

2026-09-23 18:01:26 +00:00

docs: align calibration glossary and record triage decisions #63

gabogg referenced this issue

2026-09-23 18:05:22 +00:00

fix(occupancy): trust/KPI dataset parity and 1:1 camera-group invariant — phase 2 (#28, #35) #54

gabogg referenced this issue

2026-09-23 18:10:45 +00:00

fix(occupancy): trust/KPI dataset parity and 1:1 camera-group invariant — phase 2 (#28, #35) #54

gabogg referenced this issue

2026-09-23 18:32:38 +00:00

fix(occupancy): trust/KPI dataset parity and 1:1 camera-group invariant — phase 2 (#28, #35) #54

gabogg referenced this issue

2026-09-23 18:44:25 +00:00

fix(occupancy): trust/KPI dataset parity and 1:1 camera-group invariant — phase 2 (#28, #35) #54

gabogg referenced this issue

2026-09-23 22:15:43 +00:00

refactor(occupancy): PR #54 review follow-ups — third copy of the counted-camera rule, window-end convention, test edges #66

gabogg referenced this issue

2026-09-23 22:16:09 +00:00

fix(occupancy): trust/KPI dataset parity and 1:1 camera-group invariant — phase 2 (#28, #35) #54

gabogg commented

2026-09-23 22:16:12 +00:00

Author

Owner

Implementation caveat from PR #54 (merged): window-end parity.

PR #54 aligned get_hourly_flow_distribution_async with the KPI queries on the window end: <= cycle_end. get_bucketed_cycle_flow_async, which this issue makes the sole trust-rule input, still ends at < ?. When you consolidate onto it, keep the trust input and the KPI totals on the same bound, otherwise an event stamped exactly at cycle_end is counted by one and not the other.

In practice callers pass cycle_end = reset - 0.001 (or start + 86400 - 0.001), so only the final millisecond differs. But tests/test_trust_dataset_parity.py pins the parity, and it will fail if the bounds diverge. See also #66 item 2.

**Implementation caveat from PR #54** (merged): window-end parity. PR #54 aligned `get_hourly_flow_distribution_async` with the KPI queries on the window end: **`<= cycle_end`**. `get_bucketed_cycle_flow_async`, which this issue makes the sole trust-rule input, still ends at **`< ?`**. When you consolidate onto it, keep the trust input and the KPI totals on the same bound, otherwise an event stamped exactly at `cycle_end` is counted by one and not the other. In practice callers pass `cycle_end = reset - 0.001` (or `start + 86400 - 0.001`), so only the final millisecond differs. But `tests/test_trust_dataset_parity.py` pins the parity, and it will fail if the bounds diverge. See also #66 item 2.

gabogg referenced this issue from a commit

2026-09-23 22:16:33 +00:00

fix(occupancy): trust/KPI dataset parity and 1:1 camera-group invariant — phase 2 (#54)

~~gabogg referenced this issue 2026-09-23 22:41:33 +00:00~~

fix(occupancy): passenger flow ingestion honesty — counter resets, outage gaps, peak ordering (#26, #31, #30) #68

gabogg referenced this issue

2026-09-27 23:33:18 +00:00

feat(occupancy): reviewed monthly historical passenger-flow backfill #160

~~gabogg referenced this issue 2026-09-27 23:39:54 +00:00~~

feat(occupancy): retrieve and stage historical HikCentral flow #166