Skip to content

fix(cache): fold generation params (reasoning.effort/max_tokens) into semantic cache signature - #15156

Open
Laksopan23 wants to merge 3 commits into
diegosouzapw:release/v3.8.52from
Laksopan23:fix/semantic-cache-signature-15149
Open

Laksopan23 wants to merge 3 commits into
diegosouzapw:release/v3.8.52from
Laksopan23:fix/semantic-cache-signature-15149

Conversation

@Laksopan23

Copy link
Copy Markdown
Contributor

What

Fixes #15149 — folds generation params into the semantic-cache signature so two temperature: 0 requests that differ only in reasoning.effort / max_tokens (or top_k, seed, stop, penalties, logit_bias) no longer collide on the same cache entry.

⚠️ base-red inherited: #15100

Root cause

generateSignature() hashed {model, messages, temperature, top_p} + the output contract (tools/tool_choice/response_format — #12307/#12734), and generateDirectHash() folded the same contract, but neither carried generation params: outputContractOf() never extracted them. A request with reasoning: {"effort": "none"} was therefore stored and looked up under the exact signature a later reasoning: {"effort": "max"} request computes — the second request got x-omniroute-cache: HIT with the first request's body.

Fix

outputContractOf() (src/lib/semanticCache.ts) now also extracts, when present:

reasoning, reasoning_effort, max_tokens, max_completion_tokens, top_k, seed, stop, presence_penalty, frequency_penalty, logit_bias

Every read/store call site already passes that contract through — legacy signature (read fallback, non-streaming store, streaming store) and the Layer-1 direct hash (manager lookup/store) — so no call-site changes were needed. Requests carrying none of these fields still get null from outputContractOf(), so plain-chat signatures — and every cache entry already written for them — stay byte-identical.

Also adjusts tests/unit/chat-combo-live-test.test.ts: it hand-seeded its cached entry with a bare legacy signature while the production read path folds the request's max_tokens; the seed now uses the same contract the production paths compute (the test previously only passed because outputContractOf returned null for generation params).

Tests (TDD)

  • New tests/unit/15149-semantic-cache-signature-generation-params.test.ts — written first, red on base (all collision assertions fail: identical digests), 7/7 green with the fix:
    • effort none vs max → different signatures; effort vs absent → different
    • max_tokens 1500 vs 800 vs absent → different signatures
    • every generation param named in the issue splits the signature from plain chat
    • Layer-1 direct hash folds them too (manager lookup/store path)
    • plain chat keeps the byte-identical legacy signature (existing entries survive)
  • Retention: 12 semantic-cache files 113/113, chat-combo-live-test 5/5, plus cache-signature-roundtrip, chatcore-*, chat-route-edge-cases, cache-config-route, db-logs-cache — all green.
  • npm run test:vitest: 51/52 files pass; mcp-server/__tests__/audit.test.ts failed only under parallel run contention and passes 8/8 in isolation.
  • Gates: ESLint (with suppressions, as CI runs) on changed files: 0 errors; tsc -p tsconfig.typecheck-core.json: clean; Prettier applied; pre-commit hooks (lint-staged, docs-sync, t11 any-budget, tracked-artifacts, ai-attribution) all passed.

Base-red failures (inherited, not this PR)

Local runs on pristine origin/release/v3.8.52 code reproduce exactly the same failures as this branch:

  • tests/unit/provider-request-failure-pipeline.test.ts — 6 failures (pipelinePayloads null) — identical count on base code
  • tests/unit/chatcore-translation-paths.test.ts — 3 failures (same family) — identical count on base code

Tracked by #15100.

…gosouzapw#15149)

The signature hashed model/messages/temperature/top_p plus the output contract (tools/tool_choice/response_format - diegosouzapw#12307/diegosouzapw#12734) but not generation params, so two temperature=0 requests differing only in reasoning.effort or max_tokens collided on one cache entry and the second was served the first's response verbatim under x-omniroute-cache: HIT.

outputContractOf() now also extracts reasoning/reasoning_effort, max_tokens/max_completion_tokens, top_k, seed, stop, penalties and logit_bias when present, so both the legacy signature and the Layer-1 direct hash fold them - every read/store call site already passes the contract through. Bodies carrying none keep the byte-identical legacy key, so existing plain-chat cache entries stay valid.

The combo-live test seeds its cached entry with the contract the production read path now folds in (max_tokens is in its request body); a bare legacy signature would miss the seeded entry and assert against leftover state from a previous test.
@Laksopan23

Copy link
Copy Markdown
Contributor Author

CI run 1 — evidence

Passing (8): Change Classification, Merge integrity (changelog + generated skills), API Route Typecheck, Vitest (fast-path), No new ESLint warnings, semgrep, semgrep-cloud-platform/scan, (Build skipped advisory).

Docs Gates (fast-path) — inherited base-red, not this PR:

✗ README.md — stale version: "OmniRoute v3.8.51" — package.json is 3.8.52
✗ llm.txt — stale version: "Current version:** 3.8.51" — package.json is 3.8.52
✗ 2 STRICT drift(s) detected.

Verified inheritance:

  • git diff origin/release/v3.8.52 -- README.md llm.txt → empty (this PR touches neither file)
  • base llm.txt:11 = **Current version:** 3.8.51 while base package.json version = 3.8.52 → the version bump landed without the doc sync on release/v3.8.52

Tracked by the base-red issue for this branch: #15100.

(Additional local base-red reproduced identically on pristine base code, for context: tests/unit/provider-request-failure-pipeline.test.ts 6 failures, tests/unit/chatcore-translation-paths.test.ts 3 failures — same counts with and without this branch.)

@Laksopan23

Copy link
Copy Markdown
Contributor Author

CI run 1 — full failure attribution (all inherited base-red)

Final check results: 8 pass / 4 fail (plus Build skipped advisory).

Failing checks

1. Docs Gates (fast-path) — version drift on release/v3.8.52:

✗ README.md — stale version: "OmniRoute v3.8.51" — package.json is 3.8.52
✗ llm.txt — stale version: "Current version:** 3.8.51" — package.json is 3.8.52
  • This PR touches neither file (git diff origin/release/v3.8.52 -- README.md llm.txt → empty)
  • Documented hard failure of the base itself in the base-red issue: 🔴 Release branch not green: release/v3.8.52 #15100 ("Docs sync + fabricated-docs (strict): ✗ README.md — stale version…", from the chore(release): open v3.8.52 development cycle push)

2. Fast Quality Gates — mutation-test-coverage:

✗ 24 covering unit test(s) across 8 module(s) are missing from stryker.conf.json tap.testFiles

All 24 are pre-existing test files (antigravity-verify-account-403…, circuit-breaker-kind-cooldown-escalation-14960…, authz/peer-stamp…, …) over pre-existing stryker modules (routeGuard.ts, error.ts, circuitBreaker.ts, comboStructure.ts, …). Neither the new test file nor src/lib/semanticCache.ts appears — this PR adds nothing to that list. All other FQG sub-gates passed.

3. Unit Tests fast-path (1/4, 3/4, 4/4) — 9 failing tests, none cache-related:

Shard Failing tests
1/4 glm-executor (expects pinned claude-cli/2.1.258, code ships 2.1.280), opencode-geo-block-rotation ×3 (451≠200…)
3/4 adobe-firefly-test-browser-guard, connection-test-respects-operator-disable
4/4 check-docs-counts-sync (fails with the same "2 STRICT drift(s)" as Docs Gates → #15100), combo-lockout-quota-reset-6863, issue-14629-commandcode-400-recovery-unreachable

Local base-vs-branch proof (same 9 failures both ways)

Ran the failing files locally in two passes:

  1. with this branch's code → EXIT=1, 9 failures
  2. after restoring pristine git show origin/release/v3.8.52:src/lib/semanticCache.ts → EXIT=1, identical 9 failures (same files, same positions)
adobe-firefly-test-browser-guard:1:304 / :1180
check-docs-counts-sync:1:2481
combo-lockout-quota-reset-6863:1:2766
glm-executor:1:3447
issue-14629-commandcode-400-recovery-unreachable:1:427
opencode-geo-block-rotation:1:7631 / :9339 / :10451

(connection-test-respects-operator-disable is CI-only and does not import the semantic cache.) Additionally provider-request-failure-pipeline (6) and chatcore-translation-paths (3) fail identically on pristine base code in the local full sweep.

Passing (8)

Change Classification · Merge integrity (changelog + generated skills) · API Route Typecheck · Vitest (fast-path) · No new ESLint warnings · semgrep · semgrep-cloud-platform/scan · Mergify Merge Protections (skip)

No failing check involves src/lib/semanticCache.ts or the cache test suites (new test, 12 semantic-cache files, chat-combo-live-test all green locally; not among CI failures).

@Laksopan23

Copy link
Copy Markdown
Contributor Author

Retriggering CI: base has moved to \dbe703a000\ since the last run — the inherited reds (Docs Gates via #15113, unit fast-path failures incl. glm pin / geo rotation / 400-recovery / connection-test via the base-red drain rounds) reproduce green on the current base under the canonical harness. FQG mutation-gate drift stays tracked as base-red (#15185 / #15227).

@Laksopan23 Laksopan23 closed this Oct 1, 2026
@Laksopan23 Laksopan23 reopened this Oct 1, 2026
…(refresh merge-base so the new-code gates measure only this PR's changes)
…(sync to base tip 5282351 so PR checks scope only this PR's files)
@Laksopan23

Copy link
Copy Markdown
Contributor Author

CI evidence after merging base tip 5282351753 (comment 3/3 — supersedes the FQG part of comment 2)

After f2ce517bac, GitHub's PR merge ref included base commits that landed after this branch's merge-base, so complexity-ratchets measured 27 files (3 mine + 24 base-side) and attributed open-sse/utils/opencodeHeaders.ts: 0 → 1 and open-sse/translator/deepseekWebTools.ts: 5 → 6 to this PR. Merging origin/release/v3.8.52 (63dcfe6183) removed the gap: git diff --name-only origin/release/v3.8.52 HEAD is exactly the 3 files of this PR, local check:complexity-ratchets --base-ref origin/release/v3.8.52 = 1 file in scope, 0 violations, and Fast Quality Gates now passes (complexity + mutation gate).

Remaining reds are all base-side / infra — none touch this PR's diff:

Check Root cause Proof
Docs Sync (Strict) i18n drift: docs/architecture/QUALITY_GATES.md (source-changed) node scripts/i18n/check-translation-drift.mjs FAILs in this worktree, whose docs/ is byte-identical to base tip (diff vs base = 3 code files only)
Lint failing step is npm run audit:deps — 16 vulns incl. critical next GHSA-vcvr-r3jv-pc5j (fix needs next@16.3.8, outside the pinned range) this PR changes no dependencies
Unit Tests (6/8) tests/unit/usage-pending-sweep.test.ts → assert.ok(getPendingById().size > 5000) file untouched by this PR; passes locally under the canonical harness (exit 0)
Vitest (MCP/UI) 431/431 files, 2841/2841 tests pass; 4 unhandled window is not defined from tests/unit/ui/flowCanvas.test.tsx file untouched by this PR
Build npm run build step cancelled (runner/timeout), no compile error steps show cancelled Run npm run build
Gate / CI + Gate / Quality downstream admission: fail citing fast-gates: required result is failure while required checks above are red base's own runs at 52823517 are red too: CI #36950659779 (Gate / CI), Quality Gates #36950659850

⚠️ base-red inherited: #15246 (release/v3.8.52 not green) + the items above from the 52823517 push wave.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(backend): semantic cache signature omits reasoning.effort and max_tokens — stale responses served across different effort levels

1 participant