# session-cache-and-baseline-comparator review Status: done -- review of shipped work Source: manual review of the batch built from [`session-classification-cache-ttl.md`](session-classification-cache-ttl.md), [`baseline-routing-comparator.md`](baseline-routing-comparator.md), and [`classifier-input-scope-check.md`](classifier-input-scope-check.md). Full suite verified green independently (638 passed). One component is clean and committed; the other has a bug confirmed live against `router.db`, not just read off the diff — and is not actually committed yet, despite the delivery summary saying "all deliverables are complete and merged." --- ## Session classification cache — clean, matches spec Commits `245842d`, `ba5b50d`, `f4557d4`, `0a26f58`, `756341d`. No findings. Specifically verified: - `session_cache.py` is pure (no I/O, no imports of `dispatcher`/`config`), TTL comparison is `>` not `>=` (a hit exactly on the boundary is still fresh, matching the spec's "exactly 60s later... not strictly greater... so fresh" framing), and `get` never extends an entry's life — only `put` writes a fresh timestamp. - `dispatcher.py`'s integration correctly reorders to compute `measured` tokens before the cache check, issues exactly one `route()` call on a cache hit (never a wasted classify-then-reroute), and captures `classified_src` *before* the override reroute can overwrite `decision.classification.source` — so the cache-write gate (`classified_src == "classifier"`) reflects the original classification attempt, not whatever the source ends up as after a measured-context reroute. - Fallback classifications are never written to the cache (tested directly: `test_fallback_classification_is_never_cached`). - Capability flags stay fresh on a cache hit — tested directly (`test_capability_flags_are_still_read_fresh_on_a_cache_hit`) by sending a same-session image turn after a cached text turn and confirming it still routes to a vision-capable model and logs `source="cached"` for the *routing* decision while still applying the freshly-read `has_images` gate. This was the one property that mattered most in the spec, and it's the one with a dedicated test. ## Baseline routing comparator — not committed, and wrong against real data `baseline_report.py` and `tests/test_baseline_report.py` are untracked in git (`git status --short` shows both as `??`), unlike every other file in this batch. Nothing here is merged. ### 1. `load_decisions` filters `kind = 'route'`, which excludes 99.5% of real traffic **File:** `baseline_report.py:85` (`load_decisions`), consumed by `analyze`. **Problem.** The query hardcodes `kind = 'route'`. Live `router.db`: ``` sqlite> SELECT kind, COUNT(*) FROM route_decisions GROUP BY kind; chat|2209 route|10 ``` `"route"` is the low-volume `/route` probe endpoint (no quota spent, mostly used for testing). `"chat"` is `/v1/chat/completions` — the actual dogfooding traffic this whole tool exists to audit, and it's 99.5% of the table. Running the report against the real database does not error, it just silently analyzes almost nothing: ``` $ python baseline_report.py --since 2026-08-01 baseline routing comparator decisions: 10 ... ``` 10 decisions, all of which are leftover manual test calls from this session's own debugging, not the dogfooding traffic the tool is meant to audit. This is exactly the failure mode the README repeatedly warns about in its own history — "scored the rig, not the model" — reproduced in a new tool: the test fixture (`tests/test_baseline_report.py:96`, `INSERT INTO route_decisions (... kind ...) VALUES (?, 'route', ...)`) only ever seeds `kind='route'` rows, so the suite is internally consistent and green without ever exercising what the live table actually contains. 638 passing tests didn't catch this because no test's fixture data resembled production shape here. **Fix.** `persist_route_decision` is called with four kinds that represent an actual scored routing decision — `"route"` (`dispatcher.py:1628`), `"chat"` (`:2255`, `:2278`), and `"dispatch"` (`:2664`) — versus two that don't: `"passthrough"` (`:2297`, a client pin that bypasses scoring entirely) and `"local_vision"` (`:2214`, logged specifically because routing found *no* eligible cloud candidate — there's no "eligible set" to rank a baseline within). The filter should be `kind IN ('route', 'chat', 'dispatch')`, not `kind = 'route'`. ### 2. Rejected decisions (no `selected_model`) would pollute the aggregate once #1 is fixed **File:** `baseline_report.py:230-253` (`analyze`'s accumulation loop). **Problem.** `dispatcher.py:2255` logs a `"chat"` row with `rejected_reason` set and no `selected_model` when routing finds nothing eligible (a 422 to the client — no work was actually dispatched). Once finding #1's filter is widened to include `"chat"`, these rows enter `analyze`'s loop. `agg["actual_cost"] += d["est_cost_usd"] or 0.0` and `agg["actual_proficiency"] += ap if ap is not None else 0.0` both silently add `0.0` for a request that was never served, which pools "the router refused this" together with "the router served this for free" in the `actual_*` aggregates — understating real average cost/proficiency for any category with rejections. `cheapest["model_id"] == d["selected_model"]` degrades gracefully here (`None` never matches, so it just doesn't count as dominance) but the cost/proficiency pollution does not degrade gracefully. Not currently visible in `router.db` — `SELECT COUNT(*) FROM route_decisions WHERE kind IN ('route','chat','dispatch') AND selected_model IS NULL` returns `0` on this deployment right now — but it's a latent bug in the aggregation logic, not something that depends on luck staying the current way, and the fix is one clause. **Fix.** Add `selected_model IS NOT NULL` to `load_decisions`'s `WHERE` clause alongside the `kind` fix, so a rejected/unserved request is excluded from both the aggregate and the dominance count, the same way this project already excludes non-model-attributable failures elsewhere (`model_attributable = 0` in the verification-feedback path). ### Everything else in the file checks out `reconstruct_decision`'s filter-dict construction correctly gates `min_tool_proficiency`/`require_vision`/`require_json_mode` only when the original decision's `tools`/`images`/`json_mode` flags were set (mirroring `capabilities.py`'s "only applies when the request carries it" rule), the cheapest/best-proficiency tie-breaks are deterministic (`model_id` as the final tie-break in both), and `--csv`/human-table output both read from the same `analyze()` result so they can't drift from each other. The "current catalog, not historical snapshot" limitation is stated plainly in the module docstring, matching the plan. ## Recommendation Fix both findings in `baseline_report.py` (one `WHERE` clause each), add a `chat`-kind fixture row to `tests/test_baseline_report.py` so the test suite would have caught #1, then commit. Session cache needs nothing further.