Repository navigation
fix(kimi-coding): capture and replay reasoning for thinking-mode turns - #7673
Merged
diegosouzapw merged 2 commits intoJul 19, 2026
Merged
diegosouzapw merged 2 commits into
diegosouzapw merged 2 commits into
Conversation
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
xz-dev
added a commit
to xz-dev/OmniRoute
that referenced
this pull request
Jul 18, 2026
…asoning Complete PR diegosouzapw#7673: the createSSEStream passthrough path accumulated delta.thinking into passthroughAccumulatedContent instead of passthroughAccumulatedReasoning, so the reconstructed completion body carried no reasoning_content for the replay cache to capture.
Owner
|
Solid diagnosis and fix — verified both defects independently: (1) the missing kimi-coding provider registration / narrow regex, and (2) confirmed via a red/green test that reverting only the stream.ts accumulator fix reproduces the exact |
Kimi Coding (claude-format upstream) never engaged reasoning replay: requiresReasoningReplay() had no kimi-coding/kimi-coding-apikey provider entry and only matched /kimi-k2/i model ids, so thinking was neither captured nor re-injected on multi-turn requests. Additionally, streamed Claude thinking_delta chunks were accumulated into content instead of accumulatedReasoning in createSSEStream, so the reconstructed completion body carried no reasoning_content for the cache to capture. - reasoningCache: add kimi-coding/kimi-coding-apikey providers; broaden model pattern to /kimi[-/]k\d/i (covers k2.6/k2.7 incl. namespaced ids, excludes kimi-latest and non-thinking aliases) - stream: accumulate Claude delta.thinking into accumulatedReasoning so the completion body exposes reasoning_content for replay capture - tests: provider/model predicate cases + a reconstructed-stream-body regression test separating thinking from visible text - docs: sync REASONING_REPLAY provider/pattern lists
xz-dev
force-pushed
the
fix/kimi-coding-reasoning-replay
branch
from
July 19, 2026 12:27
baa5dcf to
a4c463b
Compare
Owner
|
The fix itself is still small and correct (reasoningCache provider/model-pattern entries for kimi-coding + accumulatedReasoning wiring in stream.ts) — nice regression tests too. But the branch has drifted from the current release/v3.8.49 tip and now shows real merge conflicts. Could you rebase onto the latest release/v3.8.49 and force-push? The one CI red (Quality Gates Extended / zizmor count) looks like a pre-existing baseline issue on files you don't touch, not something from your change — will re-verify after the rebase. |
Owner
|
Validated in local merge-train runs 4+5 on 192.168.0.113 @ a00def8f1/477d9f4d3 (11-PR batch: static gates green ×2; test:unit union green — 24512/24529 + 24504/24521, residual fails proven environmental (/tmp stale) + timing-flake (6/6 isolated); vitest 260/260 on the equivalent local tree). Suite logs: /srv/omniroute-train/.claude/worktrees/merge-train-20260719-{231?,233341}-suite.log + /tmp/mt-full.log |
diegosouzapw
merged commit Jul 19, 2026
44e57a1
into
diegosouzapw:release/v3.8.49
4 of 5 checks passed
Merged
HouMinXi
pushed a commit
to HouMinXi/OmniRoute
that referenced
this pull request
Aug 2, 2026
diegosouzapw#7673) Kimi Coding (claude-format upstream) never engaged reasoning replay: requiresReasoningReplay() had no kimi-coding/kimi-coding-apikey provider entry and only matched /kimi-k2/i model ids, so thinking was neither captured nor re-injected on multi-turn requests. Additionally, streamed Claude thinking_delta chunks were accumulated into content instead of accumulatedReasoning in createSSEStream, so the reconstructed completion body carried no reasoning_content for the cache to capture. - reasoningCache: add kimi-coding/kimi-coding-apikey providers; broaden model pattern to /kimi[-/]k\d/i (covers k2.6/k2.7 incl. namespaced ids, excludes kimi-latest and non-thinking aliases) - stream: accumulate Claude delta.thinking into accumulatedReasoning so the completion body exposes reasoning_content for replay capture - tests: provider/model predicate cases + a reconstructed-stream-body regression test separating thinking from visible text - docs: sync REASONING_REPLAY provider/pattern lists Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
diegosouzapw
added a commit
that referenced
this pull request
Aug 8, 2026
…ther half of the conflict can silently re-break #9610 removed the placeholder globally on the strength of ONE provider's observed behavior (deepseek-v4-flash accepting an absent reasoning_content), which re-opened the MiMo 400 (9router#1321). The previous commit scoped the placeholder to xiaomi-mimo; this pins BOTH directions in one test so the next global edit fails loudly instead of trading the bugs again: - xiaomi-mimo plain replay turn, cache miss -> reasoning_content present (narrowing the scope away from MiMo re-opens 9router#1321) - deepseek plain replay turn, cache miss -> reasoning_content absent (widening it back to DeepSeek re-opens the #9573 echo bug) Guard verified by mutation: forcing requiresReasoningContentPresence() to return true makes the DeepSeek half fail (1 pass / 1 fail), and the file was restored from the pre-probe copy before committing. Also checked kimi-coding/kimi-coding-apikey, the other strict-contract entries in REASONING_REPLAY_PROVIDERS: their originating PR (#7673) fixes capture and replay of REAL reasoning and documents no 400 on an absent field, so they stay out of the placeholder scope — evidence-scoped, not speculatively widened. Reasoning suites together: 87/87. Gates: file-size, test-discovery, mutation-test-coverage, dead-code exit 0; eslint clean. Refs #9737
diegosouzapw
added a commit
that referenced
this pull request
Aug 8, 2026
…hat.ts, Responses payload regression, born-red stdio test, gate drifts (#9757) * fix(ci): drop unused RadarReferrals type export — dead-code ratchet back to 227 baseline The radar referral-links feature (#9697) exported the inferred type RadarReferrals from feedSchema.ts but nothing imports it (the singular RadarReferral is the consumed type). knip counts it as a new dead export, pushing the dead-code ratchet to 228 > 227 and failing Fast Quality Gates on every PR born after the merge. RadarReferralsSchema itself stays — it is used by RadarFeedSchema. Refs #9737 * fix(ci): clear the 08-08 base-red layer — prod crash in chat.ts, Responses API payload regression, born-red stdio test, gate drifts Six independent base-reds from the 08-07 evening merge batch, each verified against the pure release/v3.8.50 tip: - src/sse/handlers/chat.ts: #9467's squash carried a refactor hunk that renamed the all-rate-limited breaker guard to an UNDEFINED variable (isAllRateLimited) — a production ReferenceError on the all-accounts-429 path (chat.ts is outside typecheck:core scope, so only tests caught it). Restore credentials?.allRateLimited. Guard: chat-rate-limit-body-lock (2/2), also un-breaks batch_api and chat-combo-live-test. - open-sse/utils/stream.ts: #9315 switched providerPayload summaries to the accumulated responseBody, but in passthrough paths that body is synthesized in chat-completion shape — Responses API lost its `response` object in the dashboard payload. Keep the events-derived summary for OPENAI_RESPONSES only. Guard: stream-utils + stream-collector-9315 suites (51/51). - tests/unit/mcp-stdio-json-purity.test.ts: born red — the full CLI chain takes ~10s (2x tsx import + DB init) and the test slept a fixed 4s. Poll for the first stdout line with a 60s deadline instead. - tests/unit/plugins-route-error-sanitization.test.ts: register #9445's new marketplace/install route in PLUGIN_ROUTES (route already sanitizes) (33/33). - tests/unit/provider-models-route-codex.test.ts: realign pinned GPT-5.6 input limit to #9432's deliberate 272000→922000 bump (7/7). - lint: fix 11 no-explicit-any errors in repro-9630 + specialty-9293 tests, prune 1 orphaned suppression, allowlist the opencode-ai devDependency (#8869, publisher-verified), and reword a doc line the fabricated-docs gate misread as an env var. Gates re-verified locally: lint:json --max-warnings 0 exit 0, dead-code 227, typecheck:core clean, check:deps OK, check:fabricated-docs OK. Refs #9737 * fix(ci): clear the third 08-08 base-red layer — invalid ru rule pack, stale event pin, orphaned UI repro test, pack/mutation/file-size drifts Follow-up to the previous layer: the serial fast-gates chain unmasked one more stratum after file-size/dead-code went green, all verified against the merged release/v3.8.50 tip: - compression rules ru/ultra.json (#9581): two rules shipped minIntensity "notes", which is not a valid CavemanIntensity (lite|full|ultra) — loading ANY language pack list threw and killed the rtk-loader suite. Mapped both to "ultra" (they are the most aggressive punctuation/case rules, matching the en pack tiers). 2/2. - plugins-welcome-banner-e2e: #9668 added the onStreamComplete builtin event (real emission path via runOnStreamCompleteHooks) and missed this pinned-list sibling. 35/35. - tests/unit/free-pool-frontend-repro (#9046): landed as .tsx with node:test semantics — no runner collects tests/unit/*.tsx, so it NEVER ran (test-discovery NEW-orphan). It contains zero JSX; renamed to .test.ts so the unit runner's existing glob collects it. 5/5 (first real run). - pack-policy: allow + require bin/mcpStdioConsoleGuard.mjs (#9281) — it is preloaded via node --import by bin/mcp-server.mjs, so a published artifact without it crashes 'omniroute --mcp' at startup. - stryker.conf.json: add 5 covering unit tests from the batch (#8779/#9204/ #9330/#9630/openrouter-passthrough) to tap.testFiles (--strict drift). - file-size-baseline: consolidate the base-drift rebaseline for the 12 files grown by the 08-06..08-08 batches (#9616's entries never reached the base; measured on this branch's tree — this PR's own source edits add zero lines to any frozen file). Local battery: file-size/deps/test-discovery/mutation/pack-policy/dead-code/ duplication/docs-all/secrets/vuln/workflows ratchets all exit 0; full lint gate --max-warnings 0 exit 0. Refs #9737 * fix(types): clear the 3 uncovered open-sse-typecheck regressions + realign combo skip-code siblings Fourth base-red layer unmasked by the serial gates. The other 4 typecheck regressions (codex.ts, kiro.ts, tierResolver.test.ts, translator/index.ts) already have dedicated open [TS7] PRs (#9748/#9753/#9742/#9747) — not duplicated here. This commit covers only what no open PR owns: - devin-agentic/serializer.ts TS2367: drop the dead 'role === "system"' branch — the guard above already narrows role to user|assistant (system throws unsupported_role). Devin suites 104/104. - raycast.ts TS2416: the buildHeaders 'override' never matched the base signature (2nd param is the signed payload string, not the stream boolean) — renamed to a private buildRaycastRequestHeaders helper so a polymorphic buildHeaders(credentials, true) call can never bind here. - modelMetadataRegistry.ts TS2352: PricingByProvider → nested-record cast now goes through unknown (shape is runtime-guarded by findInsensitive). - combo-routing-engine.test.ts: realign 2 pre-dispatch-skip expectations to #9630's deliberate ALL_TARGETS_SKIPPED contract (87/87). Refs #9737 * fix(ci): clear the fifth 08-08 base-red layer — reasoning-placeholder contract sweep, GPT-5.6 limits sweep, vi key parity The 08-08 merges (#9610 reasoning replay, #9432 GPT-5.6 limits, #9630 combo skip codes, #9336 provider key links) each changed a contract and left sibling tests pinning the old one. Full grep sweep per contract, not just the shard that happened to go red: - reasoning placeholder (#9573/#9610): the fix DELIBERATELY removed NON_ANTHROPIC_THINKING_PLACEHOLDER injection on cache miss — the model echoed the placeholder as its own reasoning (empty stop) and re-poisoned cache + client history; DeepSeek's 400 is specific to an EMPTY STRING, not an absent field. Realigned reasoning-cache (2 cases, renamed to describe omission) + tool-request-sanitization (1 case + dead import). 60/60. - GPT-5.6 Codex limits (#9432, 272000 -> 1050000 ctx / 922000 input): realigned vscode-token-routes-gpt56 (2) + vscode-token-routes (3). 43/43 together with t23-t24. - combo skip codes (#9630): t23-t24-fallback-resilience T24 now expects ALL_TARGETS_SKIPPED like the combo-routing-engine siblings. - vi.json key parity: #9336 added providers.getApiKey/getApiKeyDescription to en.json without syncing vi (the only locale with a parity gate). Translated both; providers block reordered to match en key order. 5/5. - pack-artifact-policy.test.ts: sibling of this PR's own required-paths change (bin/mcpStdioConsoleGuard.mjs). 10/10. - combo-routing-engine.test.ts: dropped the 6 comment lines added in the previous commit so the frozen test file-size stays at its baseline (the rationale lives in that commit message, not the test body). Gates: file-size, test-discovery, mutation-test-coverage, pack-policy, open-sse-typecheck, dead-code all exit 0. Refs #9737 * fix(translator): keep the reasoning_content placeholder for Xiaomi MiMo — #9610 traded one live 400 for another The xiaomi-mimo replay test (9router#1321) went red on the base after #9610 removed the NON_ANTHROPIC_THINKING_PLACEHOLDER injection globally. That test is NOT stale — it guards a documented upstream 400 ('Param Incorrect: The reasoning_content in the thinking mode must be passed back to the API'), so realigning it would have masked a reintroduced production bug. Two real bugs conflict here: - #9573: forwarding the placeholder makes the model continue its chain of thought FROM that text (echo -> empty stop) and re-poisons cache/history. - 9router#1321/#1337: omitting reasoning_content on a plain replay turn makes Xiaomi MiMo reject the request outright. #9610's evidence for omitting is provider-specific — it verified that deepseek-v4-flash accepts an ABSENT field. It does not extend to MiMo. So the omission stays for every provider #9610 covered, and the placeholder survives the cache miss only for xiaomi-mimo (new requiresReasoningContentPresence predicate next to isReasoningOnlyReplayTarget). The echo that comes back is still stripped on the way in by isInternalReasoningPlaceholder(), so #9573's cache/history poisoning stays fixed for MiMo too. Both contracts now hold simultaneously: xiaomi-mimo replay + reasoning-cache + tool-request-sanitization 61/61; placeholder-strip/responses/translator/combo regression sweep 168/168. Gates: file-size, open-sse-typecheck, dead-code, mutation-test-coverage exit 0; typecheck:core clean. A live check on the VPS (Hard Rule #18 path 2) is the only way to confirm the DeepSeek half of #9610's empirical claim; flagging it in the PR rather than widening this fix on speculation. Refs #9737 * test(translator): pin the reasoning-placeholder provider scope so neither half of the conflict can silently re-break #9610 removed the placeholder globally on the strength of ONE provider's observed behavior (deepseek-v4-flash accepting an absent reasoning_content), which re-opened the MiMo 400 (9router#1321). The previous commit scoped the placeholder to xiaomi-mimo; this pins BOTH directions in one test so the next global edit fails loudly instead of trading the bugs again: - xiaomi-mimo plain replay turn, cache miss -> reasoning_content present (narrowing the scope away from MiMo re-opens 9router#1321) - deepseek plain replay turn, cache miss -> reasoning_content absent (widening it back to DeepSeek re-opens the #9573 echo bug) Guard verified by mutation: forcing requiresReasoningContentPresence() to return true makes the DeepSeek half fail (1 pass / 1 fail), and the file was restored from the pre-probe copy before committing. Also checked kimi-coding/kimi-coding-apikey, the other strict-contract entries in REASONING_REPLAY_PROVIDERS: their originating PR (#7673) fixes capture and replay of REAL reasoning and documents no 400 on an absent field, so they stay out of the placeholder scope — evidence-scoped, not speculatively widened. Reasoning suites together: 87/87. Gates: file-size, test-discovery, mutation-test-coverage, dead-code exit 0; eslint clean. Refs #9737 --------- Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
diegosouzapw#7673) Kimi Coding (claude-format upstream) never engaged reasoning replay: requiresReasoningReplay() had no kimi-coding/kimi-coding-apikey provider entry and only matched /kimi-k2/i model ids, so thinking was neither captured nor re-injected on multi-turn requests. Additionally, streamed Claude thinking_delta chunks were accumulated into content instead of accumulatedReasoning in createSSEStream, so the reconstructed completion body carried no reasoning_content for the cache to capture. - reasoningCache: add kimi-coding/kimi-coding-apikey providers; broaden model pattern to /kimi[-/]k\d/i (covers k2.6/k2.7 incl. namespaced ids, excludes kimi-latest and non-thinking aliases) - stream: accumulate Claude delta.thinking into accumulatedReasoning so the completion body exposes reasoning_content for replay capture - tests: provider/model predicate cases + a reconstructed-stream-body regression test separating thinking from visible text - docs: sync REASONING_REPLAY provider/pattern lists Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…hat.ts, Responses payload regression, born-red stdio test, gate drifts (diegosouzapw#9757) * fix(ci): drop unused RadarReferrals type export — dead-code ratchet back to 227 baseline The radar referral-links feature (diegosouzapw#9697) exported the inferred type RadarReferrals from feedSchema.ts but nothing imports it (the singular RadarReferral is the consumed type). knip counts it as a new dead export, pushing the dead-code ratchet to 228 > 227 and failing Fast Quality Gates on every PR born after the merge. RadarReferralsSchema itself stays — it is used by RadarFeedSchema. Refs diegosouzapw#9737 * fix(ci): clear the 08-08 base-red layer — prod crash in chat.ts, Responses API payload regression, born-red stdio test, gate drifts Six independent base-reds from the 08-07 evening merge batch, each verified against the pure release/v3.8.50 tip: - src/sse/handlers/chat.ts: diegosouzapw#9467's squash carried a refactor hunk that renamed the all-rate-limited breaker guard to an UNDEFINED variable (isAllRateLimited) — a production ReferenceError on the all-accounts-429 path (chat.ts is outside typecheck:core scope, so only tests caught it). Restore credentials?.allRateLimited. Guard: chat-rate-limit-body-lock (2/2), also un-breaks batch_api and chat-combo-live-test. - open-sse/utils/stream.ts: diegosouzapw#9315 switched providerPayload summaries to the accumulated responseBody, but in passthrough paths that body is synthesized in chat-completion shape — Responses API lost its `response` object in the dashboard payload. Keep the events-derived summary for OPENAI_RESPONSES only. Guard: stream-utils + stream-collector-9315 suites (51/51). - tests/unit/mcp-stdio-json-purity.test.ts: born red — the full CLI chain takes ~10s (2x tsx import + DB init) and the test slept a fixed 4s. Poll for the first stdout line with a 60s deadline instead. - tests/unit/plugins-route-error-sanitization.test.ts: register diegosouzapw#9445's new marketplace/install route in PLUGIN_ROUTES (route already sanitizes) (33/33). - tests/unit/provider-models-route-codex.test.ts: realign pinned GPT-5.6 input limit to diegosouzapw#9432's deliberate 272000→922000 bump (7/7). - lint: fix 11 no-explicit-any errors in repro-9630 + specialty-9293 tests, prune 1 orphaned suppression, allowlist the opencode-ai devDependency (diegosouzapw#8869, publisher-verified), and reword a doc line the fabricated-docs gate misread as an env var. Gates re-verified locally: lint:json --max-warnings 0 exit 0, dead-code 227, typecheck:core clean, check:deps OK, check:fabricated-docs OK. Refs diegosouzapw#9737 * fix(ci): clear the third 08-08 base-red layer — invalid ru rule pack, stale event pin, orphaned UI repro test, pack/mutation/file-size drifts Follow-up to the previous layer: the serial fast-gates chain unmasked one more stratum after file-size/dead-code went green, all verified against the merged release/v3.8.50 tip: - compression rules ru/ultra.json (diegosouzapw#9581): two rules shipped minIntensity "notes", which is not a valid CavemanIntensity (lite|full|ultra) — loading ANY language pack list threw and killed the rtk-loader suite. Mapped both to "ultra" (they are the most aggressive punctuation/case rules, matching the en pack tiers). 2/2. - plugins-welcome-banner-e2e: diegosouzapw#9668 added the onStreamComplete builtin event (real emission path via runOnStreamCompleteHooks) and missed this pinned-list sibling. 35/35. - tests/unit/free-pool-frontend-repro (diegosouzapw#9046): landed as .tsx with node:test semantics — no runner collects tests/unit/*.tsx, so it NEVER ran (test-discovery NEW-orphan). It contains zero JSX; renamed to .test.ts so the unit runner's existing glob collects it. 5/5 (first real run). - pack-policy: allow + require bin/mcpStdioConsoleGuard.mjs (diegosouzapw#9281) — it is preloaded via node --import by bin/mcp-server.mjs, so a published artifact without it crashes 'omniroute --mcp' at startup. - stryker.conf.json: add 5 covering unit tests from the batch (diegosouzapw#8779/diegosouzapw#9204/ diegosouzapw#9330/diegosouzapw#9630/openrouter-passthrough) to tap.testFiles (--strict drift). - file-size-baseline: consolidate the base-drift rebaseline for the 12 files grown by the 08-06..08-08 batches (diegosouzapw#9616's entries never reached the base; measured on this branch's tree — this PR's own source edits add zero lines to any frozen file). Local battery: file-size/deps/test-discovery/mutation/pack-policy/dead-code/ duplication/docs-all/secrets/vuln/workflows ratchets all exit 0; full lint gate --max-warnings 0 exit 0. Refs diegosouzapw#9737 * fix(types): clear the 3 uncovered open-sse-typecheck regressions + realign combo skip-code siblings Fourth base-red layer unmasked by the serial gates. The other 4 typecheck regressions (codex.ts, kiro.ts, tierResolver.test.ts, translator/index.ts) already have dedicated open [TS7] PRs (diegosouzapw#9748/diegosouzapw#9753/diegosouzapw#9742/diegosouzapw#9747) — not duplicated here. This commit covers only what no open PR owns: - devin-agentic/serializer.ts TS2367: drop the dead 'role === "system"' branch — the guard above already narrows role to user|assistant (system throws unsupported_role). Devin suites 104/104. - raycast.ts TS2416: the buildHeaders 'override' never matched the base signature (2nd param is the signed payload string, not the stream boolean) — renamed to a private buildRaycastRequestHeaders helper so a polymorphic buildHeaders(credentials, true) call can never bind here. - modelMetadataRegistry.ts TS2352: PricingByProvider → nested-record cast now goes through unknown (shape is runtime-guarded by findInsensitive). - combo-routing-engine.test.ts: realign 2 pre-dispatch-skip expectations to diegosouzapw#9630's deliberate ALL_TARGETS_SKIPPED contract (87/87). Refs diegosouzapw#9737 * fix(ci): clear the fifth 08-08 base-red layer — reasoning-placeholder contract sweep, GPT-5.6 limits sweep, vi key parity The 08-08 merges (diegosouzapw#9610 reasoning replay, diegosouzapw#9432 GPT-5.6 limits, diegosouzapw#9630 combo skip codes, diegosouzapw#9336 provider key links) each changed a contract and left sibling tests pinning the old one. Full grep sweep per contract, not just the shard that happened to go red: - reasoning placeholder (diegosouzapw#9573/diegosouzapw#9610): the fix DELIBERATELY removed NON_ANTHROPIC_THINKING_PLACEHOLDER injection on cache miss — the model echoed the placeholder as its own reasoning (empty stop) and re-poisoned cache + client history; DeepSeek's 400 is specific to an EMPTY STRING, not an absent field. Realigned reasoning-cache (2 cases, renamed to describe omission) + tool-request-sanitization (1 case + dead import). 60/60. - GPT-5.6 Codex limits (diegosouzapw#9432, 272000 -> 1050000 ctx / 922000 input): realigned vscode-token-routes-gpt56 (2) + vscode-token-routes (3). 43/43 together with t23-t24. - combo skip codes (diegosouzapw#9630): t23-t24-fallback-resilience T24 now expects ALL_TARGETS_SKIPPED like the combo-routing-engine siblings. - vi.json key parity: diegosouzapw#9336 added providers.getApiKey/getApiKeyDescription to en.json without syncing vi (the only locale with a parity gate). Translated both; providers block reordered to match en key order. 5/5. - pack-artifact-policy.test.ts: sibling of this PR's own required-paths change (bin/mcpStdioConsoleGuard.mjs). 10/10. - combo-routing-engine.test.ts: dropped the 6 comment lines added in the previous commit so the frozen test file-size stays at its baseline (the rationale lives in that commit message, not the test body). Gates: file-size, test-discovery, mutation-test-coverage, pack-policy, open-sse-typecheck, dead-code all exit 0. Refs diegosouzapw#9737 * fix(translator): keep the reasoning_content placeholder for Xiaomi MiMo — diegosouzapw#9610 traded one live 400 for another The xiaomi-mimo replay test (9router#1321) went red on the base after diegosouzapw#9610 removed the NON_ANTHROPIC_THINKING_PLACEHOLDER injection globally. That test is NOT stale — it guards a documented upstream 400 ('Param Incorrect: The reasoning_content in the thinking mode must be passed back to the API'), so realigning it would have masked a reintroduced production bug. Two real bugs conflict here: - diegosouzapw#9573: forwarding the placeholder makes the model continue its chain of thought FROM that text (echo -> empty stop) and re-poisons cache/history. - 9router#1321/diegosouzapw#1337: omitting reasoning_content on a plain replay turn makes Xiaomi MiMo reject the request outright. diegosouzapw#9610's evidence for omitting is provider-specific — it verified that deepseek-v4-flash accepts an ABSENT field. It does not extend to MiMo. So the omission stays for every provider diegosouzapw#9610 covered, and the placeholder survives the cache miss only for xiaomi-mimo (new requiresReasoningContentPresence predicate next to isReasoningOnlyReplayTarget). The echo that comes back is still stripped on the way in by isInternalReasoningPlaceholder(), so diegosouzapw#9573's cache/history poisoning stays fixed for MiMo too. Both contracts now hold simultaneously: xiaomi-mimo replay + reasoning-cache + tool-request-sanitization 61/61; placeholder-strip/responses/translator/combo regression sweep 168/168. Gates: file-size, open-sse-typecheck, dead-code, mutation-test-coverage exit 0; typecheck:core clean. A live check on the VPS (Hard Rule diegosouzapw#18 path 2) is the only way to confirm the DeepSeek half of diegosouzapw#9610's empirical claim; flagging it in the PR rather than widening this fix on speculation. Refs diegosouzapw#9737 * test(translator): pin the reasoning-placeholder provider scope so neither half of the conflict can silently re-break diegosouzapw#9610 removed the placeholder globally on the strength of ONE provider's observed behavior (deepseek-v4-flash accepting an absent reasoning_content), which re-opened the MiMo 400 (9router#1321). The previous commit scoped the placeholder to xiaomi-mimo; this pins BOTH directions in one test so the next global edit fails loudly instead of trading the bugs again: - xiaomi-mimo plain replay turn, cache miss -> reasoning_content present (narrowing the scope away from MiMo re-opens 9router#1321) - deepseek plain replay turn, cache miss -> reasoning_content absent (widening it back to DeepSeek re-opens the diegosouzapw#9573 echo bug) Guard verified by mutation: forcing requiresReasoningContentPresence() to return true makes the DeepSeek half fail (1 pass / 1 fail), and the file was restored from the pre-probe copy before committing. Also checked kimi-coding/kimi-coding-apikey, the other strict-contract entries in REASONING_REPLAY_PROVIDERS: their originating PR (diegosouzapw#7673) fixes capture and replay of REAL reasoning and documents no 400 on an absent field, so they stay out of the placeholder scope — evidence-scoped, not speculatively widened. Reasoning suites together: 87/87. Gates: file-size, test-discovery, mutation-test-coverage, dead-code exit 0; eslint clean. Refs diegosouzapw#9737 --------- Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
For the Kimi Coding provider (
kimi-coding/kimi-coding-apikey, the claude-format upstream atapi.kimi.com/coding), thinking blocks were not handled correctly in multi-turn conversations.Two compounding defects:
Reasoning replay never engaged.
requiresReasoningReplay()inopen-sse/services/reasoningCache.tshad nokimi-coding/kimi-coding-apikeyprovider entry, and its model pattern was only/kimi-k2/i— which matcheskimi-k2but not the catalog idskimi-k2.6/kimi-k2.6-thinking/kimi-k2.7-code*. Soreasoning_contentwas neither captured from responses nor re-injected on subsequent turns, and the strict thinking-mode upstream rejected tool-call turns that lacked a preceding thinking block.Streamed Claude thinking was never captured. In
createSSEStream(open-sse/utils/stream.ts), raw Claudecontent_block_delta→delta.thinkingwas accumulated intoaccumulatedContent(visible text) instead ofaccumulatedReasoning. The reconstructed completion body (onComplete.responseBody) therefore carried noreasoning_content, socacheReasoningFromAssistantMessagehad nothing to cache — even with replay engaged, multi-turn replay still failed. This affected both the translated path and the claude-passthrough accumulation.Fix
open-sse/services/reasoningCache.tskimi-codingandkimi-coding-apikeytoREASONING_REPLAY_PROVIDERS./kimi-k2/i→/kimi[-/]k\d/i: coverskimi-k2.6/kimi-k2.7-code*and namespaced ids (moonshotai/kimi-k2.7-code) for combo fallback, while still excludingkimi-latestand other non-thinking aliases.open-sse/utils/stream.tsdelta.thinkingintoaccumulatedReasoning(both the translated-stream and claude-passthrough accumulation sites), so the reconstructed completion body exposesreasoning_contentfor replay capture.Tests
tests/unit/service-reasoning-cache.test.ts— provider + model predicate cases (kimi-coding/kimi-coding-apikey; kimi-kN ids on any provider; negatives fork3,kimi-latest,moonshot-v1-8k).tests/unit/claude-stream-reconstructed-body.test.ts(new) — feeds a full Claude SSE sequence (thinking + text + tool_use + stop) throughcreateSSEStreamand asserts the reconstructed body separatesreasoning_content("plan carefully") from visiblecontent("Visible answer"). Red→green verified: fails before the stream fix, passes after.docs/routing/REASONING_REPLAY.md— synced provider/pattern lists.Verification (all green)
claude-stream-reconstructed-body: 1/1service-reasoning-cache: 17/17reasoning-cache: 52/52executor-kimi: 4/4kimi-native-reasoning-injected-1480: 2/2openai-to-claude-redacted-replay-5312: 4/4anthropic-stream-thinking-close-marker: 2/2prettier --checkandgit diff --checkclean.