fix(calibration): per-cycle calibration log lookup can return a neighbouring cycle's log

gabogg commented

2026-09-25 23:37:19 +00:00

Owner

Split out of the #126 review (pass 1, Standards P2). It's inherited behaviour, not introduced by #126, but since #126 the daily series, hourly series and dayparts all read past-cycle calibration through this one lookup.

Problem

OccupancyRepository.get_calibration_log_for_cycle_async(date_str, start_epoch, end_epoch) picks "the" calibration log for a business cycle with:

WHERE (cycle_date = ? OR (timestamp_epoch >= ? AND timestamp_epoch <= ?))
  AND is_current = 1
ORDER BY CASE WHEN calibration_type IN ('MANUAL_OVERRIDE', 'MANUAL_ADMIN', 'RETROACTIVE_GUARD_AUDIT') THEN 0 ELSE 1 END,
         timestamp_epoch DESC LIMIT 1

It can return another cycle's log:

A manual override or retroactive audit for cycle D, written while cycle D+1 is running, has its timestamp_epoch inside D+1's window. It is ranked first, so D+1 picks up D's exit multiplier and guard count.
D's nocturnal audit, if it is stamped after the reset, also falls inside D+1's window.
It does not filter superseded_by, unlike the trusted-history and trust-verdict queries, which use is_current = 1 AND superseded_by IS NULL.

Effect: a past day can be shown (occupancy, peak, dwell, dayparts) with the neighbouring day's k, which is against ADR 0006.

Proposal

Key the lookup on cycle_date = ? and add superseded_by IS NULL.
Use the timestamp window only as a fallback for legacy rows without a cycle_date, e.g. (cycle_date = ? OR ((cycle_date IS NULL OR cycle_date = '') AND timestamp_epoch BETWEEN ? AND ?)).
Keep the verdict priority (retroactive and manual before automatic) within the same cycle.
Tests:
- an override for D written during D+1 does not affect D+1;
- a superseded log is ignored;
- a legacy row with no cycle_date is still found by its window.

Related: #76 (which cycle a calibration window belongs to), #126 (shared _cycle_exit_multiplier_async), ADR 0006.

🤖 Generated with Claude Code

Split out of the #126 review (pass 1, Standards P2). It's inherited behaviour, not introduced by #126, but since #126 the daily series, hourly series and dayparts all read past-cycle calibration through this one lookup. ## Problem `OccupancyRepository.get_calibration_log_for_cycle_async(date_str, start_epoch, end_epoch)` picks "the" calibration log for a business cycle with: ```sql WHERE (cycle_date = ? OR (timestamp_epoch >= ? AND timestamp_epoch <= ?)) AND is_current = 1 ORDER BY CASE WHEN calibration_type IN ('MANUAL_OVERRIDE', 'MANUAL_ADMIN', 'RETROACTIVE_GUARD_AUDIT') THEN 0 ELSE 1 END, timestamp_epoch DESC LIMIT 1 ``` It can return **another cycle's** log: - A manual override or retroactive audit for cycle **D**, written while cycle **D+1** is running, has its `timestamp_epoch` inside D+1's window. It is ranked first, so D+1 picks up D's exit multiplier and guard count. - D's nocturnal audit, if it is stamped after the reset, also falls inside D+1's window. - It does not filter `superseded_by`, unlike the trusted-history and trust-verdict queries, which use `is_current = 1 AND superseded_by IS NULL`. Effect: a past day can be shown (occupancy, peak, dwell, dayparts) with the neighbouring day's k, which is against ADR 0006. ## Proposal - Key the lookup on `cycle_date = ?` and add `superseded_by IS NULL`. - Use the timestamp window **only as a fallback for legacy rows without a `cycle_date`**, e.g. `(cycle_date = ? OR ((cycle_date IS NULL OR cycle_date = '') AND timestamp_epoch BETWEEN ? AND ?))`. - Keep the verdict priority (retroactive and manual before automatic) within the same cycle. - Tests: - an override for D written during D+1 does not affect D+1; - a superseded log is ignored; - a legacy row with no `cycle_date` is still found by its window. Related: #76 (which cycle a calibration window belongs to), #126 (shared `_cycle_exit_multiplier_async`), ADR 0006. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

gabogg added the

ready-for-agent

label

2026-09-25 23:37:19 +00:00

gabogg referenced this issue

2026-09-25 23:43:46 +00:00

fix(statistics): daypart dwell multiplier, usual daypart labels, per-day excluded flag (#106) #126

gabogg referenced this issue

2026-09-26 01:39:09 +00:00

fix(statistics): daypart dwell multiplier, usual daypart labels, per-day excluded flag (#106) #126

gabogg referenced this issue

2026-09-26 01:41:03 +00:00

fix(statistics): daypart dwell multiplier, usual daypart labels, per-day excluded flag (#106) #126

~~gabogg referenced this issue 2026-09-26 01:41:04 +00:00~~

follow-up(statistics): P3 cleanups from #126 review (dwell multiplier, labels, excluded/trusted flags) #128

gabogg referenced this issue

2026-09-26 11:39:33 +00:00

fix(statistics): one ranked cycle verdict; offset resets are not verdicts #137

gabogg commented

2026-10-03 12:55:33 +00:00

Author

Owner

Related evidence from PR #227 review pass 2. The single-row calibration-log lookup uses an inclusive window end (timestamp_epoch <= ?, occupancy_repository.py ~3324), while the batched lookup excludes it (< p.end_epoch). A log stamped exactly at a cycle boundary therefore resolves to different cycles depending on which path reads it, which is this issue's neighbouring-cycle symptom. The statistics audit doc (line ~62) claims the two paths match; it should be corrected when this is fixed.

Related evidence from PR #227 review pass 2. The single-row calibration-log lookup uses an **inclusive** window end (`timestamp_epoch <= ?`, `occupancy_repository.py` ~3324), while the batched lookup **excludes** it (`< p.end_epoch`). A log stamped exactly at a cycle boundary therefore resolves to different cycles depending on which path reads it, which is this issue's neighbouring-cycle symptom. The statistics audit doc (line ~62) claims the two paths match; it should be corrected when this is fixed.

Rows
Columns

fix(calibration): per-cycle calibration log lookup can return a neighbouring cycle's log #127

Problem

Proposal