fix(deepseek): preserve Responses reasoning replay on continuations (#875) - #906
fix(deepseek): preserve Responses reasoning replay on continuations (#875)#906Yuxin-Qiao wants to merge 1 commit into
Conversation
|
✅ PR quality gates passed This pull request now targets The title was left unchanged. The pull request has been marked ready for review again. |
📝 WalkthroughWalkthroughDeepSeek replay now preserves configured raw reasoning content, removes ChangesReasoning replay sanitization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72313f1a54
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (hasOcxEnvelope) { | ||
| delete next.encrypted_content; | ||
| mutated = true; |
There was a problem hiding this comment.
When reasoning.summary is absent or none, src/bridge.ts:380-397 stores the prior raw reasoning only in a txt-only ocxr1 envelope, with no content array. On the next DeepSeek continuation, this branch deletes that sole copy, while preserveContent cannot help because hasRawContent is false; the stateless upstream therefore still receives no reasoning replay and tool-call loops can stall in the default hidden-summary mode. Decode the envelope's txt into content: [{ type: "reasoning_text", text }] for preserve-listed models before removing encrypted_content, and cover a bridge-output-to-passthrough round trip.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
72313f1 to
2691d00
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/openai-responses.ts`:
- Around line 1042-1045: Refactor the sanitization pipeline in the surrounding
response-body handling code into sequential intermediate assignments, reusing a
mutable value for each step from scrubOcxCompactionItems through
sanitizeReasoningInputContent, stripUnsupportedHostedTools, stripInvalidItemIds,
stripItemIdsWhenUnstored, stripUnsupportedReasoningParams,
stripSparkCompatibility, and normalizeToolSchemas. Preserve the existing call
order and pass the same preserveContentModels and modelId options to
sanitizeReasoningInputContent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4e1e9495-ad1f-4bbf-9f43-e35278bcbcba
📒 Files selected for processing (2)
src/adapters/openai-responses.tstests/deepseek-reasoning-replay.test.ts
| const sanitizedBody = normalizeToolSchemas(stripSparkCompatibility(stripUnsupportedReasoningParams(stripItemIdsWhenUnstored(stripInvalidItemIds(stripUnsupportedHostedTools(sanitizeReasoningInputContent(scrubOcxCompactionItems(outBody), { | ||
| preserveContentModels: provider.preserveReasoningContentModels, | ||
| modelId: parsed.modelId, | ||
| }))))))); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Deeply nested call chain gets harder to read with each new parameter.
This line already chains eight function calls (normalizeToolSchemas → stripSparkCompatibility → ... → sanitizeReasoningInputContent → scrubOcxCompactionItems), and this PR adds a two-key options object inside the innermost call, pushing the single logical statement further out. Reviewers and future editors must track matching parens across the whole line. Consider unwinding this into sequential let-reassignments so each sanitization step is independently readable and diffable.
[optional_refactor_low_effort_high_reward_placeholder]
♻️ Proposed refactor: sequential pipeline instead of nested calls
- const sanitizedBody = normalizeToolSchemas(stripSparkCompatibility(stripUnsupportedReasoningParams(stripItemIdsWhenUnstored(stripInvalidItemIds(stripUnsupportedHostedTools(sanitizeReasoningInputContent(scrubOcxCompactionItems(outBody), {
- preserveContentModels: provider.preserveReasoningContentModels,
- modelId: parsed.modelId,
- })))))));
+ let sanitizedBody = scrubOcxCompactionItems(outBody);
+ sanitizedBody = sanitizeReasoningInputContent(sanitizedBody, {
+ preserveContentModels: provider.preserveReasoningContentModels,
+ modelId: parsed.modelId,
+ });
+ sanitizedBody = stripUnsupportedHostedTools(sanitizedBody);
+ sanitizedBody = stripInvalidItemIds(sanitizedBody);
+ sanitizedBody = stripItemIdsWhenUnstored(sanitizedBody);
+ sanitizedBody = stripUnsupportedReasoningParams(sanitizedBody);
+ sanitizedBody = stripSparkCompatibility(sanitizedBody);
+ sanitizedBody = normalizeToolSchemas(sanitizedBody);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sanitizedBody = normalizeToolSchemas(stripSparkCompatibility(stripUnsupportedReasoningParams(stripItemIdsWhenUnstored(stripInvalidItemIds(stripUnsupportedHostedTools(sanitizeReasoningInputContent(scrubOcxCompactionItems(outBody), { | |
| preserveContentModels: provider.preserveReasoningContentModels, | |
| modelId: parsed.modelId, | |
| }))))))); | |
| let sanitizedBody = scrubOcxCompactionItems(outBody); | |
| sanitizedBody = sanitizeReasoningInputContent(sanitizedBody, { | |
| preserveContentModels: provider.preserveReasoningContentModels, | |
| modelId: parsed.modelId, | |
| }); | |
| sanitizedBody = stripUnsupportedHostedTools(sanitizedBody); | |
| sanitizedBody = stripInvalidItemIds(sanitizedBody); | |
| sanitizedBody = stripItemIdsWhenUnstored(sanitizedBody); | |
| sanitizedBody = stripUnsupportedReasoningParams(sanitizedBody); | |
| sanitizedBody = stripSparkCompatibility(sanitizedBody); | |
| sanitizedBody = normalizeToolSchemas(sanitizedBody); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/openai-responses.ts` around lines 1042 - 1045, Refactor the
sanitization pipeline in the surrounding response-body handling code into
sequential intermediate assignments, reusing a mutable value for each step from
scrubOcxCompactionItems through sanitizeReasoningInputContent,
stripUnsupportedHostedTools, stripInvalidItemIds, stripItemIdsWhenUnstored,
stripUnsupportedReasoningParams, stripSparkCompatibility, and
normalizeToolSchemas. Preserve the existing call order and pass the same
preserveContentModels and modelId options to sanitizeReasoningInputContent.
|
Closing as a functional duplicate: the proxy half of #875 already landed in #892 (wt3, b42d573) — the registry seeds |
2 participants
Summary
Fixes the proxy half of #875: DeepSeek V4 Flash Responses turns stall after tool calls because the passthrough sanitizer blanked
reasoning.contentfor every provider. DeepSeek's Responses API is stateless and requires replaying prior assistant reasoning text on every continuation; an emptied reasoning item breaks the replay, so tool-call loops cannot finish.sanitizeReasoningInputContentnow keeps rawreasoning_textcontent for models inpreserveReasoningContentModels(already seeded for DeepSeek thinking models, includingdeepseek-v4-flash) and keeps the existing blanking for ChatGPT-native passthrough, which accepts reasoning input only with emptycontent.Why it matters for subagents
With this fix,
deepseek/deepseek-v4-flashcan be selected as a sub-agent worker (ocx agent subagents set deepseek/deepseek-v4-flashor"subagentModels": ["deepseek/deepseek-v4-flash"]) and its tool-call loops continue correctly. Added a regression test proving the routed entry is a viable sub-agent candidate.Change
src/adapters/openai-responses.ts:sanitizeReasoningInputContent(body, opts)with a preserve list + model id; content kept for preserve-listed models, blanked otherwise; envelopes always stripped. Call site passesprovider.preserveReasoningContentModelsandparsed.modelId.tests/deepseek-reasoning-replay.test.ts: new file, 5 tests covering preserve/blank/envelope unit behavior, an end-to-end handleResponses continuation carrying reasoning + tool output upstream, and sub-agent routability.Validation
bun test tests/deepseek-reasoning-replay.test.ts tests/deepseek-inbound-wire.test.ts— 19 pass, 0 failbun run typecheck— passgit diff --check— passSource commits
2691d006fix(deepseek): preserve Responses reasoning replay on continuations ([Bug] DeepSeek V4 Flash Responses route stalls after tool calls #875)Summary by CodeRabbit
Bug Fixes
Tests