fix(metrics): make the last-24h classifier window a real 24h, and a plan-pace test time bomb #114

Merged
alee merged 3 commits from fix/julianday-24h-window into main 2026-10-08 00:11:29 +00:00
Owner

What

The "last 24 hours" window in the classifier-degradation warning is really 24 to 48 hours, depending on the hour of day.

route_decisions.observed_at is written by datetime.now(timezone.utc).isoformat(), so it has a T separator: 2026-10-05T05:31:12.306322+00:00. SQLite's datetime('now', ...) returns a space: 2026-10-06 00:54:20. 'T' sorts after ' ', so once the two dates match the time of day never participates, and the comparison admits every row on the cutoff's calendar date.

Two queries in src/metrics.py did this: classifier_degradation_warning and _declined_for. Both now use julianday(observed_at) >= julianday('now', '-24 hours'), the form poller.mark_stale and the outcome-attribution query in dispatcher.py already use. The project hit this exact trap once before (spurious 409s); this closes the last two sites and adds a tripwire so a third cannot appear quietly.

Measured on synthetic rows at 00:54 UTC: a row aged 24.5h was admitted by the old comparison and rejected by the new one. On the live DB today the shipped window admitted 0 extra rows, only because no traffic fell in that gap.

Commits

  1. 7777002 the fix, three window tests, and the tripwire tests/test_sql_time_windows.py. The tripwire scans src/ for an ISO-T column compared with datetime('now' in either order, and carries positive controls so its regexes cannot silently stop matching.
  2. 2c358cd a separate, pre-existing bug (below).
  3. 6b0c3e3 tidy: SQL indent, a duplicate scan test removed from the tripwire, one em dash.

Separate: a test time bomb this branch ran into

test_quota_accounts_alarm_plan_pace_warning and _critical hardcoded billing_reset_day=6 and claimed to hold "on any day". quota_accounts deliberately sets elapsed_fraction to None below 0.02 (day 0 of a period) and skips the pace rule there, so on the 6th UTC both saw the usage-floor alarm instead of plan_pace and failed. They went red on 2026-10-06 UTC, on main at d97588c as well, so every gate that day was red.

The tests now take a reset day from _pace_reset_day(today), which always puts today at least one day into the period. A sweep test checks every calendar day of a leap and a common year against the same 0.02 guard using metrics' own _billing_period_start and _next_reset_date. Product behavior is unchanged; the guard is deliberate.

Verification

  • scripts/verify_commit.py --full HEAD from the worktree: lint clean, 2678 tests pass. It was run on the 6th UTC, the day the pace tests fail without commit 2.
  • Mutation check: with src/metrics.py reverted to origin/main, test_degradation_window_excludes_rows_older_than_24h_on_the_cutoff_date, test_declined_for_ignores_reasons_older_than_24h and the tripwire fail; the in-window test passes either way. The fix turns all of them green.
  • The window tests build timestamps from one now, using midnight at the start of the cutoff's own date as the "just outside" row, so they are deterministic at every hour of the day.
  • Dry run of the tripwire regexes over src/ before the fix: exactly the two sites, no false positives. The other datetime('now' uses write space-format columns (applied_at, added_at) and are fine.

Not in this PR

  • Only the two sites the tripwire found were changed. Time windows built another way (for example Python-side isoformat() cutoffs) were not audited here.
  • No change to the stored timestamp format.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KkCGRantZsSwmcFpet6FTa

## What The "last 24 hours" window in the classifier-degradation warning is really 24 to 48 hours, depending on the hour of day. `route_decisions.observed_at` is written by `datetime.now(timezone.utc).isoformat()`, so it has a `T` separator: `2026-10-05T05:31:12.306322+00:00`. SQLite's `datetime('now', ...)` returns a space: `2026-10-06 00:54:20`. `'T'` sorts after `' '`, so once the two dates match the time of day never participates, and the comparison admits every row on the cutoff's calendar date. Two queries in `src/metrics.py` did this: `classifier_degradation_warning` and `_declined_for`. Both now use `julianday(observed_at) >= julianday('now', '-24 hours')`, the form `poller.mark_stale` and the outcome-attribution query in `dispatcher.py` already use. The project hit this exact trap once before (spurious 409s); this closes the last two sites and adds a tripwire so a third cannot appear quietly. Measured on synthetic rows at 00:54 UTC: a row aged 24.5h was admitted by the old comparison and rejected by the new one. On the live DB today the shipped window admitted 0 extra rows, only because no traffic fell in that gap. ## Commits 1. `7777002` the fix, three window tests, and the tripwire `tests/test_sql_time_windows.py`. The tripwire scans `src/` for an ISO-`T` column compared with `datetime('now'` in either order, and carries positive controls so its regexes cannot silently stop matching. 2. `2c358cd` a separate, pre-existing bug (below). 3. `6b0c3e3` tidy: SQL indent, a duplicate scan test removed from the tripwire, one em dash. ## Separate: a test time bomb this branch ran into `test_quota_accounts_alarm_plan_pace_warning` and `_critical` hardcoded `billing_reset_day=6` and claimed to hold "on any day". `quota_accounts` deliberately sets `elapsed_fraction` to `None` below 0.02 (day 0 of a period) and skips the pace rule there, so on the 6th UTC both saw the usage-floor alarm instead of `plan_pace` and failed. They went red on 2026-10-06 UTC, on `main` at `d97588c` as well, so every gate that day was red. The tests now take a reset day from `_pace_reset_day(today)`, which always puts today at least one day into the period. A sweep test checks every calendar day of a leap and a common year against the same 0.02 guard using metrics' own `_billing_period_start` and `_next_reset_date`. Product behavior is unchanged; the guard is deliberate. ## Verification - `scripts/verify_commit.py --full HEAD` from the worktree: lint clean, 2678 tests pass. It was run on the 6th UTC, the day the pace tests fail without commit 2. - Mutation check: with `src/metrics.py` reverted to `origin/main`, `test_degradation_window_excludes_rows_older_than_24h_on_the_cutoff_date`, `test_declined_for_ignores_reasons_older_than_24h` and the tripwire fail; the in-window test passes either way. The fix turns all of them green. - The window tests build timestamps from one `now`, using midnight at the start of the cutoff's own date as the "just outside" row, so they are deterministic at every hour of the day. - Dry run of the tripwire regexes over `src/` before the fix: exactly the two sites, no false positives. The other `datetime('now'` uses write space-format columns (`applied_at`, `added_at`) and are fine. ## Not in this PR - Only the two sites the tripwire found were changed. Time windows built another way (for example Python-side `isoformat()` cutoffs) were not audited here. - No change to the stored timestamp format. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KkCGRantZsSwmcFpet6FTa
alee added 3 commits 2026-10-06 01:35:12 +00:00
Two sites in src/metrics.py (classifier_degradation_warning and
_declined_for) compared observed_at against datetime('now', ...) using
string comparison. Since observed_at is stored as ISO-8601 ('T'
separator) and datetime('now') returns a space separator, the 'T' > ' '
sorting made the time-of-day component irrelevant — a 24h window
silently admitted all rows on the cutoff's calendar date.

Replace both with julianday() on both sides, matching the pattern
already used in dispatcher.py and poller.py.

Add three tests in test_classifier_cascade.py using a just_outside
timestamp (midnight of the cutoff's calendar date — the exact shape
the old comparison wrongly admitted) and an inside timestamp, proving
the fix works.

Add tests/test_sql_time_windows.py as a tripwire: scans src/ for
any ISO-T column compared against datetime('now') with an operator,
with positive controls that verify the regexes match the bad pattern
and reject the safe julianday pattern.
test_quota_accounts_alarm_plan_pace_warning and _critical hardcoded
billing_reset_day=6 and claimed to hold "on any day". quota_accounts sets
elapsed_fraction to None below 0.02 (day 0 of a period) and skips the pace
rule there on purpose, so on the 6th UTC both tests saw the usage-floor alarm
instead of plan_pace and failed. They went red on 2026-10-06 UTC, on main at
d97588c as well as on this branch, so every gate that day was red.

The tests now take a reset day from _pace_reset_day(today), which always puts
today at least one day into the period (valid days are 1..28). A sweep test
checks every calendar day of a leap and a common year against the same 0.02
guard, using metrics' own _billing_period_start and _next_reset_date.

Product behavior is unchanged: the guard is deliberate.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KkCGRantZsSwmcFpet6FTa
Match the SQL indent of the two edited lines in metrics.py to their
neighbours (one space too deep, which made the diff noisier than the change),
drop test_src_has_no_unexpected_hits from the tripwire (an identical copy of
test_no_iso_t_datetime_comparison misfiled under the positive controls), and
replace an em dash in a comment with ASCII.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KkCGRantZsSwmcFpet6FTa
alee merged commit bcaf5caaa0 into main 2026-10-08 00:11:29 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: alee/6krrt#114