From 7777002981d6e132d8913365d9d48ee521125e24 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Mon, 5 Oct 2026 21:18:19 -0400 Subject: [PATCH 1/3] fix: replace raw datetime('now') comparisons with julianday() for ISO-T column 24h windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/metrics.py | 4 +- tests/test_classifier_cascade.py | 68 ++++++++++++- tests/test_sql_time_windows.py | 162 +++++++++++++++++++++++++++++++ 3 files changed, 227 insertions(+), 7 deletions(-) create mode 100644 tests/test_sql_time_windows.py diff --git a/src/metrics.py b/src/metrics.py index f3bd7b9..6ab3290 100644 --- a/src/metrics.py +++ b/src/metrics.py @@ -1761,7 +1761,7 @@ def classifier_degradation_warning(conn: sqlite3.Connection, cfg: Any) -> list[s ) AS degraded FROM route_decisions WHERE classification_source IS NOT NULL - AND observed_at >= datetime('now', '-24 hours') + AND julianday(observed_at) >= julianday('now', '-24 hours') """ ).fetchone() total = (row["total"] if row else 0) or 0 @@ -1801,7 +1801,7 @@ def _declined_for(conn: sqlite3.Connection) -> str: AND classification_source IN ('fallback', 'session_stale', 'session_history', 'classifier_cloud') - AND observed_at >= datetime('now', '-24 hours') + AND julianday(observed_at) >= julianday('now', '-24 hours') GROUP BY classifier_reject ORDER BY n DESC, classifier_reject """ diff --git a/tests/test_classifier_cascade.py b/tests/test_classifier_cascade.py index 6f5abe2..a8ff57f 100644 --- a/tests/test_classifier_cascade.py +++ b/tests/test_classifier_cascade.py @@ -7,7 +7,7 @@ happened to produce the same answer. from __future__ import annotations import sqlite3 -from datetime import datetime, timedelta, timezone +from datetime import datetime, time, timedelta, timezone from pathlib import Path from types import SimpleNamespace @@ -222,7 +222,8 @@ def test_cascade_with_no_session_key_falls_straight_through(db): # --------------------------------------------------------------------------- -def _seed_sources(conn, pairs): +def _seed_sources(conn, pairs, at=None): + ts = at.isoformat() if at is not None else _now().isoformat() for source, count in pairs: for _ in range(count): conn.execute( @@ -233,7 +234,7 @@ def _seed_sources(conn, pairs): observed_at) VALUES ('chat','general_chat',2,0,'m','neuralwatt',?,?) """, - (source, _now().isoformat()), + (source, ts), ) conn.commit() @@ -272,7 +273,8 @@ def test_degradation_warning_silent_when_healthy(db): assert classifier_degradation_warning(db, cfg) == [] -def _seed_declined(conn, source, reason, count): +def _seed_declined(conn, source, reason, count, at=None): + ts = at.isoformat() if at is not None else _now().isoformat() for _ in range(count): conn.execute( """ @@ -282,7 +284,7 @@ def _seed_declined(conn, source, reason, count): classifier_reject, observed_at) VALUES ('chat','general_chat',2,0,'m','neuralwatt',?,?,?) """, - (source, reason, _now().isoformat()), + (source, reason, ts), ) conn.commit() @@ -334,3 +336,59 @@ def test_degradation_warning_survives_a_database_without_the_column(db, monkeypa (warning,) = metrics.classifier_degradation_warning(db, cfg) assert "60%" in warning and "Declined for" not in warning + + +def test_degradation_window_excludes_rows_older_than_24h_on_the_cutoff_date(db): + from metrics import classifier_degradation_warning + + now = _now() + cutoff = now - timedelta(hours=24) + just_outside = datetime.combine(cutoff.date(), time(0, 0, 0), tzinfo=timezone.utc) + inside = now - timedelta(hours=1) + + cfg = SimpleNamespace( + classifier=SimpleNamespace(degraded_warn_min=20, degraded_warn_threshold=0.5) + ) + _seed_sources(db, [("fallback", 25)], at=just_outside) + _seed_sources(db, [("classifier", 25)], at=inside) + + out = classifier_degradation_warning(db, cfg) + assert out == [] + + +def test_degradation_window_counts_rows_inside_24h(db): + from metrics import classifier_degradation_warning + + now = _now() + inside = now - timedelta(hours=1) + + cfg = SimpleNamespace( + classifier=SimpleNamespace(degraded_warn_min=20, degraded_warn_threshold=0.5) + ) + _seed_sources(db, [("fallback", 15), ("classifier", 10)], at=inside) + + out = classifier_degradation_warning(db, cfg) + assert len(out) == 1 + assert "60%" in out[0] + + +def test_declined_for_ignores_reasons_older_than_24h(db): + from metrics import classifier_degradation_warning + + now = _now() + cutoff = now - timedelta(hours=24) + just_outside = datetime.combine(cutoff.date(), time(0, 0, 0), tzinfo=timezone.utc) + inside = now - timedelta(hours=1) + + cfg = SimpleNamespace( + classifier=SimpleNamespace(degraded_warn_min=20, degraded_warn_threshold=0.2) + ) + _seed_sources(db, [("classifier", 60)], at=inside) + _seed_declined(db, "session_history", "below_confidence_min", 20, at=inside) + _seed_declined(db, "fallback", "below_coverage_min", 7, at=just_outside) + + (warning,) = classifier_degradation_warning(db, cfg) + + assert "Declined for: 20 below_confidence_min." in warning + assert "below_coverage_min" not in warning + diff --git a/tests/test_sql_time_windows.py b/tests/test_sql_time_windows.py new file mode 100644 index 0000000..0bfec66 --- /dev/null +++ b/tests/test_sql_time_windows.py @@ -0,0 +1,162 @@ +"""Tripwire: detect ISO-T vs datetime('now', ...) comparisons in src/*.py. + +SQLite ``datetime()`` returns a space-separated string (``2026-08-23 00:18:04``). +Columns in this codebase are written by ``datetime.now(timezone.utc).isoformat()``, +so they carry a ``T`` separator (``2026-08-23T00:20:04.577131+00:00``). + +Comparing them with ``>=`` / ``<=`` / ``>`` / ``<`` is a silent bug: ``T > ' '``, +so once the dates match the time-of-day never participates and the window +becomes "everything today". + +Use ``julianday()`` on both sides of the comparison instead. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +SRC = Path(__file__).resolve().parent.parent / "src" + +_COLUMNS = ( + "observed_at|ticked_at|created_at|opened_at|" + "last_fired_at|resolved_at|updated_at" +) + +# Column on the left: observed_at >= datetime('now', ...) +_RE_COL_OP_NOW = re.compile( + rf"\b(?:{_COLUMNS})\s*(?:>=|<=|>|<)\s*" + rf"datetime\(\s*'now'" +) + +# Column on the right: datetime('now', ...) <= observed_at +_RE_NOW_OP_COL = re.compile( + rf"datetime\(\s*'now'[^)]*\)\s*(?:>=|<=|>|<)\s*" + rf"\b(?:{_COLUMNS})\b" +) + +_FAIL_MSG = ( + "compare with julianday(col) >= julianday('now', ...); " + "the column is ISO with a T, " + "datetime() returns a space, " + "and 'T' > ' ' makes the time of day never participate." +) + + +def _iter_source_files() -> list[Path]: + """Return all .py files under src/, sorted.""" + return sorted(SRC.rglob("*.py")) + + +# --------------------------------------------------------------------------- +# Tripwire +# --------------------------------------------------------------------------- + + +def test_no_iso_t_datetime_comparison(): + """Fail if any src/*.py file compares an ISO-T column against + datetime('now', ...) with an operator.""" + hits: list[tuple[Path, int, str]] = [] + for fpath in _iter_source_files(): + text = fpath.read_text(encoding="utf-8") + for lineno, line in enumerate(text.splitlines(), 1): + if _RE_COL_OP_NOW.search(line) or _RE_NOW_OP_COL.search(line): + hits.append((fpath, lineno, line.strip())) + + if hits: + parts = [_FAIL_MSG, ""] + for fpath, lineno, line in hits: + parts.append(f" {fpath}:{lineno} {line}") + pytest.fail("\n".join(parts)) + + +# --------------------------------------------------------------------------- +# Positive controls — verify the regexes themselves are correct +# --------------------------------------------------------------------------- + + +class TestRegexPositiveControls: + """Regexes must match the bad pattern and reject the safe pattern.""" + + # --- bad pattern (column op datetime) ----------------------------------- + + def test_re_col_op_now_matches_forbidden_pattern(self): + assert _RE_COL_OP_NOW.search( + "AND observed_at >= datetime('now', '-24 hours')" + ) + + def test_re_now_op_col_matches_forbidden_pattern(self): + assert _RE_NOW_OP_COL.search( + "AND datetime('now', '-24 hours') <= observed_at" + ) + + def test_re_col_op_now_matches_gt(self): + assert _RE_COL_OP_NOW.search("WHERE observed_at > datetime('now')") + + def test_re_col_op_now_matches_lt(self): + assert _RE_COL_OP_NOW.search("WHERE ticked_at < datetime('now')") + + def test_re_now_op_col_matches_ge(self): + assert _RE_NOW_OP_COL.search( + "datetime('now') >= opened_at" + ) + + # --- safe pattern (julianday) ------------------------------------------- + + def test_re_col_op_now_rejects_julianday(self): + assert not _RE_COL_OP_NOW.search( + "AND julianday(observed_at) >= julianday('now', '-24 hours')" + ) + + def test_re_now_op_col_rejects_julianday(self): + assert not _RE_NOW_OP_COL.search( + "AND julianday(observed_at) >= julianday('now', '-24 hours')" + ) + + def test_both_regexes_reject_julianday_all_columns(self): + """Spot-check a few column names with julianday wrappers.""" + for col in ("observed_at", "created_at", "updated_at", "last_fired_at", + "resolved_at", "ticked_at", "opened_at"): + sql = ( + f"AND julianday({col}) >= " + f"julianday('now', '-24 hours')" + ) + assert not _RE_COL_OP_NOW.search(sql), f"col side failed for {col}" + assert not _RE_NOW_OP_COL.search(sql), f"now side failed for {col}" + + # --- regression: docstring must not trip the scan ----------------------- + + def test_docstring_does_not_trip(self): + """The docstring near dispatcher.py:3187 mentions both names without + an operator between them.""" + line = "``datetime('now', ...)``. ``observed_at`` is written by" + assert not _RE_COL_OP_NOW.search(line) + assert not _RE_NOW_OP_COL.search(line) + + # --- any extra hits after the fix are a problem ------------------------- + + def test_src_has_no_unexpected_hits(self): + """If the scan reports any site other than the two that were fixed, + this test stops and reports the extra file:line. + + The known site is the docstring at dispatcher.py:3187, which must + NOT match (verified in test_docstring_does_not_trip). + """ + hits: list[tuple[Path, int, str]] = [] + for fpath in _iter_source_files(): + text = fpath.read_text(encoding="utf-8") + for lineno, line in enumerate(text.splitlines(), 1): + if _RE_COL_OP_NOW.search(line) or _RE_NOW_OP_COL.search(line): + hits.append((fpath, lineno, line.strip())) + if hits: + msg_parts = [ + "UNEXPECTED MATCH(ES). Lines that tripped the tripwire:", + "", + ] + for fpath, lineno, line in hits: + msg_parts.append(f" {fpath}:{lineno} {line}") + msg_parts.append("") + msg_parts.append(_FAIL_MSG) + pytest.fail("\n".join(msg_parts)) \ No newline at end of file -- 2.49.1 From 2c358cd40e37310087319e32e9efd7df3d7e01af Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Mon, 5 Oct 2026 21:32:02 -0400 Subject: [PATCH 2/3] test(metrics): stop the plan-pace tests failing on the billing reset day 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 Claude-Session: https://claude.ai/code/session_01KkCGRantZsSwmcFpet6FTa --- tests/test_metrics.py | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/tests/test_metrics.py b/tests/test_metrics.py index 853af9c..6fe0f30 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -30,6 +30,7 @@ from starlette.testclient import TestClient import dispatcher from config import load_config from metrics import ( + _billing_period_start, _next_reset_date, cache_rate_series, cache_rate_warnings, @@ -155,6 +156,31 @@ def _telemetry_provider_cfg(**objective_overrides) -> SimpleNamespace: ) +def _pace_reset_day(today: date) -> int: + """A billing_reset_day that puts ``today`` at least one day into the period. + + quota_accounts sets elapsed_fraction to None below 0.02, which is day 0 of a + period, and then skips the pace rule on purpose: a ratio over a few hours of + elapsed time is noise. A hardcoded reset day therefore turns the pace tests + red on that one calendar day every month. They did on 2026-10-06 UTC with + billing_reset_day=6, while their comments claimed "on any day". Valid days + are 1..28. + """ + return today.day - 1 if 2 <= today.day <= 29 else 28 + + +def test_pace_reset_day_is_never_the_reset_day_itself(): + """Every calendar day of a leap year and a common year lands past the 0.02 guard.""" + d = date(2024, 1, 1) + while d < date(2026, 1, 1): + reset_day = _pace_reset_day(d) + assert 1 <= reset_day <= 28, d + start = date.fromisoformat(_billing_period_start(reset_day, d)) + nxt = date.fromisoformat(_next_reset_date(reset_day, d)) + assert (d - start).days / (nxt - start).days >= 0.02, d + d += timedelta(days=1) + + # --- Import / no-circular-import smoke tests ----------------------------------- @@ -466,10 +492,13 @@ def test_quota_accounts_interleaved_providers_independent(tmp_path): def test_quota_accounts_alarm_plan_pace_warning(tmp_path): """Usage pace > 1.25x triggers plan_pace alarm.""" - cfg = _telemetry_provider_cfg(plan_kwh_per_period=6.25, billing_reset_day=6) + cfg = _telemetry_provider_cfg( + plan_kwh_per_period=6.25, billing_reset_day=_pace_reset_day(_now().date()) + ) conn = _make_db(tmp_path) now = _now() - # Seed 8.0 kWh to guarantee used_fraction > 1.25 * elapsed_fraction on any day + # Seed 8.0 kWh to guarantee used_fraction > 1.25 * elapsed_fraction; the reset + # day keeps today past the 0.02 elapsed guard on every calendar day conn.execute( "INSERT INTO energy_observations " "(model_id, provider, energy_kwh, completion_tokens, observed_at) " @@ -510,10 +539,13 @@ def test_quota_accounts_alarm_stale_reading(tmp_path): def test_quota_accounts_alarm_plan_pace_critical(tmp_path): """Usage pace > 2.0 triggers critical severity.""" - cfg = _telemetry_provider_cfg(plan_kwh_per_period=6.25, billing_reset_day=6) + cfg = _telemetry_provider_cfg( + plan_kwh_per_period=6.25, billing_reset_day=_pace_reset_day(_now().date()) + ) conn = _make_db(tmp_path) now = _now() - # Seed 15 kWh to guarantee pace > 2.0 on any day + # Seed 15 kWh to guarantee pace > 2.0; the reset day keeps today past the + # 0.02 elapsed guard on every calendar day conn.execute( "INSERT INTO energy_observations " "(model_id, provider, energy_kwh, completion_tokens, observed_at) " -- 2.49.1 From 6b0c3e3e8565f9cbe4a99af0337a96424e55910f Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Mon, 5 Oct 2026 21:32:12 -0400 Subject: [PATCH 3/3] test: tidy the julianday window fix 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 Claude-Session: https://claude.ai/code/session_01KkCGRantZsSwmcFpet6FTa --- src/metrics.py | 4 ++-- tests/test_sql_time_windows.py | 28 +--------------------------- 2 files changed, 3 insertions(+), 29 deletions(-) diff --git a/src/metrics.py b/src/metrics.py index 6ab3290..114525e 100644 --- a/src/metrics.py +++ b/src/metrics.py @@ -1761,7 +1761,7 @@ def classifier_degradation_warning(conn: sqlite3.Connection, cfg: Any) -> list[s ) AS degraded FROM route_decisions WHERE classification_source IS NOT NULL - AND julianday(observed_at) >= julianday('now', '-24 hours') + AND julianday(observed_at) >= julianday('now', '-24 hours') """ ).fetchone() total = (row["total"] if row else 0) or 0 @@ -1801,7 +1801,7 @@ def _declined_for(conn: sqlite3.Connection) -> str: AND classification_source IN ('fallback', 'session_stale', 'session_history', 'classifier_cloud') - AND julianday(observed_at) >= julianday('now', '-24 hours') + AND julianday(observed_at) >= julianday('now', '-24 hours') GROUP BY classifier_reject ORDER BY n DESC, classifier_reject """ diff --git a/tests/test_sql_time_windows.py b/tests/test_sql_time_windows.py index 0bfec66..4599c72 100644 --- a/tests/test_sql_time_windows.py +++ b/tests/test_sql_time_windows.py @@ -73,7 +73,7 @@ def test_no_iso_t_datetime_comparison(): # --------------------------------------------------------------------------- -# Positive controls — verify the regexes themselves are correct +# Positive controls: verify the regexes themselves are correct # --------------------------------------------------------------------------- @@ -134,29 +134,3 @@ class TestRegexPositiveControls: line = "``datetime('now', ...)``. ``observed_at`` is written by" assert not _RE_COL_OP_NOW.search(line) assert not _RE_NOW_OP_COL.search(line) - - # --- any extra hits after the fix are a problem ------------------------- - - def test_src_has_no_unexpected_hits(self): - """If the scan reports any site other than the two that were fixed, - this test stops and reports the extra file:line. - - The known site is the docstring at dispatcher.py:3187, which must - NOT match (verified in test_docstring_does_not_trip). - """ - hits: list[tuple[Path, int, str]] = [] - for fpath in _iter_source_files(): - text = fpath.read_text(encoding="utf-8") - for lineno, line in enumerate(text.splitlines(), 1): - if _RE_COL_OP_NOW.search(line) or _RE_NOW_OP_COL.search(line): - hits.append((fpath, lineno, line.strip())) - if hits: - msg_parts = [ - "UNEXPECTED MATCH(ES). Lines that tripped the tripwire:", - "", - ] - for fpath, lineno, line in hits: - msg_parts.append(f" {fpath}:{lineno} {line}") - msg_parts.append("") - msg_parts.append(_FAIL_MSG) - pytest.fail("\n".join(msg_parts)) \ No newline at end of file -- 2.49.1