Skip to content

fix(sse): guard reasoning-cache write by the same predicate its readers use - #10978

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.50from
maxmad64bis:fix/reasoning-cache-write-guard
Aug 21, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.50from
maxmad64bis:fix/reasoning-cache-write-guard

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Aug 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

cacheReasoningFromAssistantMessage() writes to the reasoning-replay cache on every
non-streaming and streaming response that carries reasoning_content, no matter the
provider. Both read paths that ever consume this cache are already narrower than that —
one is gated by requiresReasoningReplay(), the other by a thinkingEnabled request
flag on a specific non-Anthropic Claude-shape branch. So on an install whose traffic
never touches a replay provider, every reasoning-bearing response pays for a cache write,
an index update, and a catch {} for data that will never be read back. TTL keeps it
bounded, but it's still wasted work.

Wrapped both write sites in if (requiresReasoningReplay({ provider, model })). It's not
a perfect match for what the reads actually need — see the caveat below — but it's a
safe superset, and it stops the obviously wasteful case.

Related Issues

Builds on the reasoning-replay cache (#1628). No new issue filed.

Validation

  • Change type: other (write-path guard, no change for providers that already use replay)
  • Focused tests: tests/unit/chatCore-reasoning-cache-guard.test.ts,
    tests/unit/chatcore-reasoning-cache-write-guard.test.ts
  • npm run lint
  • New tests included in this PR

Tests Added Or Updated

  • tests/unit/chatCore-reasoning-cache-guard.test.ts — checks requiresReasoningReplay()
    directly: it says yes for the provider families the two read sites actually serve
    (deepseek, kimi-coding, xiaomi-mimo explicitly; glm/kimi-k2-style models via the
    pattern fallback), and no for a plain openai/anthropic request. Confirms the guard
    isn't a no-op.
  • tests/unit/chatcore-reasoning-cache-write-guard.test.ts — calls the real
    handleChatCore with a mocked upstream and checks the cache through
    lookupReasoning(), for both a replay provider and a non-replay one, streaming and
    non-streaming. This is the part that actually proves the two call sites are wired
    correctly, not just that the predicate itself works.

Coverage Notes

Both write sites (streaming, non-streaming) just get one if around an existing,
already-tested call — no new branches inside the write function itself.

Reviewer Notes

This guard isn't an exact match for what the read side checks — it's close enough in
practice, but worth knowing where it's not: the non-Anthropic Claude-shape read path
(kimi-coding / glm-thinking / zai-style) actually gates on a thinkingEnabled request
flag, and this write guard doesn't look at that flag at all. Every provider/model tested
is covered anyway through requiresReasoningReplay()'s own fallback matching, but that's
not proven exhaustive for every model a given provider might serve — a zai deployment
on a model that doesn't match the kimi-style pattern wouldn't be caught, for example. If
that turns out to matter, the fix is to reconstruct thinkingEnabled at the write site —
skipped here because by the time you're at the write site, chatCore.ts may have already
rewritten the thinking flag
(normalizeClaudeAdaptiveThinking / normalizeClaudeHaikuConstraints), so doing it
properly is its own piece of work, not a one-line fix.

No migration, no feature flag, no new index — this just narrows an existing write.

@maxmad64bis
maxmad64bis force-pushed the fix/reasoning-cache-write-guard branch from cdcd7a8 to 8f7fbd7 Compare August 21, 2026 13:54
@diegosouzapw
diegosouzapw merged commit 1f4bde1 into diegosouzapw:release/v3.8.50 Aug 21, 2026
7 of 16 checks passed
Sa3id23 pushed a commit to Sa3id23/OmniRoute that referenced this pull request Aug 21, 2026
…rs use (diegosouzapw#10978)

⭐5 — Cache de reasoning-replay escrevia em toda resposta com reasoning_content, mesmo quando nenhum read-path jamais consumiria (install sem provider de replay). Guard com requiresReasoningReplay() nos dois write-sites, superset seguro do que os readers checam. Testes cobrindo o predicate isoladamente e o wiring real via handleChatCore.
@maxmad64bis
maxmad64bis deleted the fix/reasoning-cache-write-guard branch September 24, 2026 21:15
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…rs use (diegosouzapw#10978)

⭐5 — Cache de reasoning-replay escrevia em toda resposta com reasoning_content, mesmo quando nenhum read-path jamais consumiria (install sem provider de replay). Guard com requiresReasoningReplay() nos dois write-sites, superset seguro do que os readers checam. Testes cobrindo o predicate isoladamente e o wiring real via handleChatCore.
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.

2 participants