Skip to content

fix(sse): reasoning_content placeholder skips messages already carrying the reasoning alias - #18273

Merged
diegosouzapw merged 2 commits into
release/v3.8.53from
fix/port-pr-4881-reasoning-injector-alias-idempotent
Oct 27, 2026
Merged

diegosouzapw merged 2 commits into
release/v3.8.53from
fix/port-pr-4881-reasoning-injector-alias-idempotent

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Summary

  • The thinking-model reasoning_content placeholder injector (OpenCode thinking models, Kimi/Moonshot and requiresReasoningContentEcho registry entries) only treated a string reasoning_content as present. An assistant message carrying the reasoning alias, or an array-valued carrier, got a second reasoning_content, and serde-based upstreams (DeepSeek, Kimi, GLM) reject that body with 400 duplicate field reasoning_content.
  • The injector now skips any assistant message with a non-empty (trimmed) string or a non-empty array in either reasoning_content or reasoning. Messages with no carrier still get the placeholder (unsupported_country_region_territory (bug?) #1543 contract kept).
  • Narrow delta only: the translator half (folding reasoning into reasoning_content on replay turns) already landed in fix(sse): drop reasoning alias once reasoning_content carries the replay #16646. The injector does not rename or delete the alias.

Attribution

Thanks to @AppleVibe for the original implementation.

Changes

  • open-sse/utils/reasoningContentInjector.ts: hasNonEmptyReasoningContent → hasAnyReasoningCarrier (string or array, reasoning_content or reasoning; strings keep the .trim() semantics).
  • tests/unit/reasoningContentInjector.test.ts: 6 new cases (alias-only, both carriers, array reasoning_content, array reasoning, no carrier, whitespace/empty carriers still injected).
  • reasoning_details (OpenRouter shape) deliberately not counted as a carrier, same as the original fix; no test needs it.

Test plan

  • TDD, before the fix: node --import tsx/esm --test tests/unit/reasoningContentInjector.test.ts → ℹ tests 11 ℹ pass 8 ℹ fail 3 (alias-only, array reasoning_content, array reasoning). After: ℹ tests 11 ℹ pass 11 ℹ fail 0.
  • Every unit test file citing reasoningContentInjector or reasoning_content (150 files, test:unit:ci preamble, concurrency 4): ℹ tests 2207 ℹ pass 2195 ℹ fail 12. All 12 failures were in tests/unit/deepseek-web.test.ts (10 s AbortSignal.timeout on PoW under load average ~17; this executor does not import the injector). Rerun alone at lower load: ℹ tests 35 ℹ pass 35 ℹ fail 0.
  • Integration files citing the symbols (7 files, concurrency 1): ℹ tests 26 ℹ pass 8 ℹ fail 0 ℹ skipped 18 (live/e2e suites skip without credentials).
  • npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files> → exit 0; npx prettier --check <changed files> → clean.
  • npm run typecheck:core → exit 0.
  • GITHUB_BASE_REF=release/v3.8.53 node scripts/check/check-pr-test-policy.mjs → Result: PASS; GITHUB_BASE_REF=release/v3.8.53 npm run check:test-masking → OK — 1 modificado(s) … sem enfraquecimento.

diegosouzapw and others added 2 commits October 26, 2026 18:20
…arry the reasoning alias

The thinking-model placeholder injector only looked at a string
reasoning_content. An assistant message carrying the `reasoning` alias, or an
array-valued carrier, got a second reasoning_content, and serde-based upstreams
(DeepSeek, Kimi, GLM) reject that body with `duplicate field reasoning_content`.
The injector now skips any message with a non-empty string or array in either
reasoning_content or reasoning; messages with no carrier still get the
placeholder.

Co-authored-by: AppleVibe <AppleVibe@users.noreply.github.com>
Inspired-by: decolua/9router#4881
@diegosouzapw
diegosouzapw merged commit 61cc8be into release/v3.8.53 Oct 27, 2026
8 of 9 checks passed
@diegosouzapw
diegosouzapw deleted the fix/port-pr-4881-reasoning-injector-alias-idempotent branch October 28, 2026 09:50
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.

1 participant