fix(metrics): make the last-24h classifier window a real 24h, and a plan-pace test time bomb #114
@@ -1761,7 +1761,7 @@ def classifier_degradation_warning(conn: sqlite3.Connection, cfg: Any) -> list[s
|
|||||||
) AS degraded
|
) AS degraded
|
||||||
FROM route_decisions
|
FROM route_decisions
|
||||||
WHERE classification_source IS NOT NULL
|
WHERE classification_source IS NOT NULL
|
||||||
AND observed_at >= datetime('now', '-24 hours')
|
AND julianday(observed_at) >= julianday('now', '-24 hours')
|
||||||
"""
|
"""
|
||||||
).fetchone()
|
).fetchone()
|
||||||
total = (row["total"] if row else 0) or 0
|
total = (row["total"] if row else 0) or 0
|
||||||
@@ -1801,7 +1801,7 @@ def _declined_for(conn: sqlite3.Connection) -> str:
|
|||||||
AND classification_source IN
|
AND classification_source IN
|
||||||
('fallback', 'session_stale', 'session_history',
|
('fallback', 'session_stale', 'session_history',
|
||||||
'classifier_cloud')
|
'classifier_cloud')
|
||||||
AND observed_at >= datetime('now', '-24 hours')
|
AND julianday(observed_at) >= julianday('now', '-24 hours')
|
||||||
GROUP BY classifier_reject
|
GROUP BY classifier_reject
|
||||||
ORDER BY n DESC, classifier_reject
|
ORDER BY n DESC, classifier_reject
|
||||||
"""
|
"""
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ happened to produce the same answer.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import sqlite3
|
import sqlite3
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, time, timedelta, timezone
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from types import SimpleNamespace
|
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 source, count in pairs:
|
||||||
for _ in range(count):
|
for _ in range(count):
|
||||||
conn.execute(
|
conn.execute(
|
||||||
@@ -233,7 +234,7 @@ def _seed_sources(conn, pairs):
|
|||||||
observed_at)
|
observed_at)
|
||||||
VALUES ('chat','general_chat',2,0,'m','neuralwatt',?,?)
|
VALUES ('chat','general_chat',2,0,'m','neuralwatt',?,?)
|
||||||
""",
|
""",
|
||||||
(source, _now().isoformat()),
|
(source, ts),
|
||||||
)
|
)
|
||||||
conn.commit()
|
conn.commit()
|
||||||
|
|
||||||
@@ -272,7 +273,8 @@ def test_degradation_warning_silent_when_healthy(db):
|
|||||||
assert classifier_degradation_warning(db, cfg) == []
|
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):
|
for _ in range(count):
|
||||||
conn.execute(
|
conn.execute(
|
||||||
"""
|
"""
|
||||||
@@ -282,7 +284,7 @@ def _seed_declined(conn, source, reason, count):
|
|||||||
classifier_reject, observed_at)
|
classifier_reject, observed_at)
|
||||||
VALUES ('chat','general_chat',2,0,'m','neuralwatt',?,?,?)
|
VALUES ('chat','general_chat',2,0,'m','neuralwatt',?,?,?)
|
||||||
""",
|
""",
|
||||||
(source, reason, _now().isoformat()),
|
(source, reason, ts),
|
||||||
)
|
)
|
||||||
conn.commit()
|
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)
|
(warning,) = metrics.classifier_degradation_warning(db, cfg)
|
||||||
|
|
||||||
assert "60%" in warning and "Declined for" not in warning
|
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
|
||||||
|
|
||||||
|
|||||||
@@ -30,6 +30,7 @@ from starlette.testclient import TestClient
|
|||||||
import dispatcher
|
import dispatcher
|
||||||
from config import load_config
|
from config import load_config
|
||||||
from metrics import (
|
from metrics import (
|
||||||
|
_billing_period_start,
|
||||||
_next_reset_date,
|
_next_reset_date,
|
||||||
cache_rate_series,
|
cache_rate_series,
|
||||||
cache_rate_warnings,
|
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 -----------------------------------
|
# --- 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):
|
def test_quota_accounts_alarm_plan_pace_warning(tmp_path):
|
||||||
"""Usage pace > 1.25x triggers plan_pace alarm."""
|
"""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)
|
conn = _make_db(tmp_path)
|
||||||
now = _now()
|
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(
|
conn.execute(
|
||||||
"INSERT INTO energy_observations "
|
"INSERT INTO energy_observations "
|
||||||
"(model_id, provider, energy_kwh, completion_tokens, observed_at) "
|
"(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):
|
def test_quota_accounts_alarm_plan_pace_critical(tmp_path):
|
||||||
"""Usage pace > 2.0 triggers critical severity."""
|
"""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)
|
conn = _make_db(tmp_path)
|
||||||
now = _now()
|
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(
|
conn.execute(
|
||||||
"INSERT INTO energy_observations "
|
"INSERT INTO energy_observations "
|
||||||
"(model_id, provider, energy_kwh, completion_tokens, observed_at) "
|
"(model_id, provider, energy_kwh, completion_tokens, observed_at) "
|
||||||
|
|||||||
136
tests/test_sql_time_windows.py
Normal file
136
tests/test_sql_time_windows.py
Normal file
@@ -0,0 +1,136 @@
|
|||||||
|
"""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)
|
||||||
Reference in New Issue
Block a user