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:

  1. cacheStore() helper now refuses any model identity that already starts with cache/. Defense at the storage gate.
  2. The hot-path semanticCache.store() call now skips when result.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:

  1. cacheStore() does NOT call semanticCache.store() when model already starts with cache/. 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.
  2. cacheStore() DOES call semanticCache.store() for normal provider/model identities. 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] Sourcesrc/router/model-router.ts (2 minimal changes, ~7 lines incl. comments)
  • [x] Testsrc/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.