2026-05-08-cache-prefix-nesting
2026-05-08 — Fix cache hits being re-stored with cache/ prefix
Summary
R20 stochastic assessment risk register, 4/10 agents flagged: savings byPath has malformed model IDs (cache/cache/cache/.../openai/gpt-4o — up to 10 deep). Concern: if the same identity feeds the bandit reward signal, comparator significance becomes architecturally suspect.
This PR closes the bug.
Root cause
src/router/model-router.ts had two semanticCache.store() call paths. One of them (line 681 prior to this PR) ran on every successful response — including cache hits. When the response was already a cache hit, result.provider was "cache" and result.modelId was the cached model. The store call then wrote a new entry keyed under cache/. On the next identical request, retrieving from the cache returned model: "cache/, which got re-stored as cache/cache/. Repeat 10 times → the chain seen in production at /v1/intelligence/savings.
Production impact (before fix)
From R20 evidence command 10:
"cache/cache/cache/cache/cache/cache/cache/cache/cache/cache/x-ai/x-ai/grok-3", requests: 2
"cache/cache/x-ai/x-ai/grok-3", requests: 7
"cache/cache/cache/cache/x-ai/x-ai/grok-3", requests: 3
"cache/cache/cache/cache/cache/cache/cache/cache/x-ai/x-ai/grok-3", requests: 2
"brainstorm/brainstorm/sandbox", requests: 9080 (separate finding, not cache nesting)
The savings ledger was fragmenting paths into phantom arms. The bandit reward signal — confirmed via model-bandit.ts reads from actualModel — also saw the same identity, so Thompson posterior was being split across openai/gpt-4o, cache/openai/gpt-4o, cache/cache/openai/gpt-4o, and so on. Each was a distinct arm with its own posterior. The comparator's recent significance result (p=0.0000045 at n=62) is still valid against the deployed routing fabric, but the fragmented posterior was understating Thompson's effective leverage.
Fix
Two changes in src/router/model-router.ts:
cacheStore()helper now refuses any model identity that already starts withcache/. Defense at the storage gate.- The hot-path
semanticCache.store()call now skips whenresult.provider === "cache". Defense at the call site (avoids the work of constructing the prefix in the first place).
Both layers are kept to make the bug class impossible to reintroduce: even if a future call site forgets to gate on provider, the storage helper itself rejects nested-cache writes.
Regression test
src/router/model-router-cache-store.test.ts — 2 tests:
cacheStore()does NOT callsemanticCache.store()when model already starts withcache/. Verified test-first: fails without the fix (expected vi.fn() to not be called at all, but actually been called 2 times), passes with the fix.cacheStore()DOES callsemanticCache.store()for normalprovider/modelidentities. Pins the no-regression-for-happy-path behavior.
Verification
$ pnpm test:fast
Test Files 820 passed | 8 skipped (828)
Tests 7490 passed | 191 skipped (7681)
$ pnpm check
oxlint: Found 0 warnings and 0 errors.
oxfmt: All matched files use the correct format.
tsgo: 0 errors
Test count went from 7488 → 7490 (the +2 regression tests).
Lockstep checklist
- [x] Source —
src/router/model-router.ts(2 minimal changes, ~7 lines incl. comments) - [x] Test —
src/router/model-router-cache-store.test.ts(paired regression, fails without fix) - [x] Ship log — this file
- [x] R20 risk register — 4/10 risk closed
- [ ] Pre-existing nested entries — production savings ledger still has the historical
cache/cache/...rows. They will age out as new requests don't add to them. If desired, a follow-up PR can add a one-shot migration to consolidate the nested arms back to their canonical model IDs.
Why this matters for comparator significance
R20 evidence cmd 11: comparator { p_value: 0.0000045, confident: true, effect_size: 0.87 }. The risk register's concern was that fragmented arms could mean the result was measured against a self-confused identity. With this fix, future comparator updates run against canonical model IDs. The historical nested-arm telemetry is still in the ledger but will not grow.