fix(deepseek): preserve parallel reasoning replay - #1501
Merged
Conversation
Contributor
|
✅ Deterministic PR hygiene checks passed. |
3 tasks
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe DeepSeek Responses adapter now normalizes complete, unambiguous parallel tool-call/result batches. It preserves reasoning context, moves injected messages after batches, and leaves ambiguous or incomplete input unchanged. Tests and provider documentation cover these rules. ChangesDeepSeek Responses normalization
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs-site/src/content/docs/reference/adapters.md`:
- Around line 61-65: Update the localized adapter pages at the specified
reference sections to include the DeepSeek stateless Responses parser exception:
describe provider-scoped history normalization, moving hook-injected context
after unambiguous tool-call/result batches, grouping parallel calls with
matching outputs, and preserving original input order for tolerant providers or
ambiguous duplicate, missing, or out-of-order call IDs. Keep the localized
wording consistent with the existing English adapter documentation.
In `@src/adapters/openai-responses.ts`:
- Around line 579-618: Update the normalization logic around the calls/outputs
collection in src/adapters/openai-responses.ts:579-618 to reject any output key
without exactly one matching call, and reject batches whose output indices do
not follow the collected call order before constructing batch; otherwise return
the original body without reordering. Add focused regression coverage in
tests/deepseek-inbound-wire.test.ts:896-918 for a valid pair plus an unmatched
output, asserting byte/order-equivalent input, and in
tests/deepseek-inbound-wire.test.ts:920-929 for callA, callB, injected, outputB,
outputA, asserting normalization preserves the original order.
In `@src/types.ts`:
- Around line 1230-1232: Update the public contract comment for the response
normalizer to limit “intervening messages” to hook-injected context, using the
exact predicate applied by the normalizer to identify that context. Preserve the
stated contiguous call/result batch and parallel-call behavior, and align the
wording with the narrower provider and adapter documentation.
🪄 Autofix
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: 5a0503e7-cdf9-461e-9c69-4c02287fb492
📒 Files selected for processing (6)
docs-site/src/content/docs/reference/adapters.mdsrc/adapters/openai-responses.tssrc/providers/registry.tssrc/types.tsstructure/04_transports-and-sidecars.mdtests/deepseek-inbound-wire.test.ts
Comment on lines
+61
to
+65
| - DeepSeek's stateless Responses parser receives provider-scoped history normalization: hook-injected | ||
| context moves after an unambiguous tool-call/result batch. Parallel calls remain grouped before | ||
| their matching outputs so every call stays in the reasoning-bearing assistant turn. Tolerant | ||
| providers and ambiguous duplicate, missing, or out-of-order call IDs keep their original input order. | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
mapfile -t files < <(
fd -t f -i 'adapters\.md$' docs-site/src/content/docs \
| rg '/(ja|ko|ru|zh-cn)/' || true
)
((${`#files`[@]})) || exit 0
rg -n -i \
'immediate|adjacent|parallel|reasoning|duplicate|missing|out.of.order|original input order' \
"${files[@]}"Repository: lidge-jun/opencodex
Length of output: 209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' 'Localized adapter pages:'
find docs-site/src/content/docs -type f \( -iname 'adapters.md' -o -iname 'adapters.mdx' \) \
-print | sort
printf '%s\n' 'Potentially contradictory localized claims:'
find docs-site/src/content/docs -type f \( -iname 'adapters.md' -o -iname 'adapters.mdx' \) \
-print0 |
while IFS= read -r -d '' file; do
case "$file" in
*/ja/*|*/ko/*|*/ru/*|*/zh-cn/*)
rg -n -i \
'immediate|adjacent|parallel|reasoning|duplicate|missing|out[ -]?of[ -]?order|original input order|call.?result|tool.?call|pair|順序|並列|重複|欠落|последователь|параллел|дублик|пропущ|поряд|调用|并行|重复|缺失|顺序' \
"$file" || true
;;
esac
doneRepository: lidge-jun/opencodex
Length of output: 4134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
docs-site/src/content/docs/ja/reference/adapters.md
docs-site/src/content/docs/ko/reference/adapters.md
docs-site/src/content/docs/ru/reference/adapters.md
docs-site/src/content/docs/zh-cn/reference/adapters.md
)
for file in "${files[@]}"; do
printf '\n--- %s ---\n' "$file"
rg -n -i -C 4 \
'deepseek|responses|openai.?responses|hook|injected|context|history|normaliz|tool.?call|tool.?result|call.?id|immediate|adjacent|order|pair|parallel|duplicate|missing|out[ -]?of[ -]?order' \
"$file" || true
doneRepository: lidge-jun/opencodex
Length of output: 19297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' 'English reference section:'
sed -n '44,70p' docs-site/src/content/docs/reference/adapters.md
printf '%s\n' 'Implementation and test references:'
rg -n -i \
'deepseek|responsesPath|hook.?inject|tool.?call|tool.?result|call.?id|history normalization|normalize.*history|original input order|reasoning-bearing|passthrough' \
--glob '!docs-site/src/content/docs/**' .Repository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' 'English reference section:'
sed -n '44,70p' docs-site/src/content/docs/reference/adapters.md
printf '%s\n' 'Relevant source files and focused matches:'
rg -l -i \
'deepseek|hook.?inject|original input order|reasoning-bearing|responses normalization|normalize.*(history|input)|passthrough' \
src tests docs-site/src/content/docs/reference/adapters.md \
| sort | head -100Repository: lidge-jun/opencodex
Length of output: 5234
Document the DeepSeek exception in all localized adapter pages. Update ja/reference/adapters.md:46, ko/reference/adapters.md:52-53, ru/reference/adapters.md:56-57, and zh-cn/reference/adapters.md:49-50 to explain history normalization, parallel-call grouping, and input-order preservation for ambiguous IDs.
🤖 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 `@docs-site/src/content/docs/reference/adapters.md` around lines 61 - 65,
Update the localized adapter pages at the specified reference sections to
include the DeepSeek stateless Responses parser exception: describe
provider-scoped history normalization, moving hook-injected context after
unambiguous tool-call/result batches, grouping parallel calls with matching
outputs, and preserving original input order for tolerant providers or ambiguous
duplicate, missing, or out-of-order call IDs. Keep the localized wording
consistent with the existing English adapter documentation.
Sources: Path instructions, Learnings
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/adapters/openai-responses.ts (1)
600-615: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFail closed when a later tool call splits a result batch.
callA, callB, outputA, callC, outputB, outputCpasses the current uniqueness, forward-order, and reversed-output checks. The loop groupscallAandcallB, then rewritesoutputBbeforecallC. This fabricates a new tool chronology instead of preserving ambiguous history.
src/adapters/openai-responses.ts#L600-L615: Returnbodywhen the next call starts before any output in the current group has completed.tests/deepseek-inbound-wire.test.ts#L930-L961: Add this interleaved-call sequence and assert thatbody.inputequals the original input.Proposed guard
+ const nextCallIndex = pairs[next]?.callIndex; + if (nextCallIndex !== undefined && group.some(pair => pair.outputIndex > nextCallIndex)) { + return body; + } + // Within one reasoning turn the outputs must appear in the same order as theirAs per path instructions,
src/**requires adapter contract checks andtests/**requires focused regression coverage.🤖 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 600 - 615, In src/adapters/openai-responses.ts:600-615, update the grouping logic around the pairs-processing loop to return the original body when a later call begins before any output in the current group has completed, preventing rewriting across a split result batch. In tests/deepseek-inbound-wire.test.ts:930-961, add the interleaved callA, callB, outputA, callC, outputB, outputC sequence and assert body.input remains unchanged.Source: Path instructions
🤖 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 `@docs-site/src/content/docs/ru/reference/adapters.md`:
- Around line 65-69: Update the DeepSeek Responses normalization description in
the Russian adapter documentation so parallel calls are described as remaining
within a single assistant turn, while still grouped before their corresponding
results. Remove the wording that says each call retains its own separate
assistant turn, and keep the rest of the ordering and tolerant-provider behavior
consistent with the English source.
---
Outside diff comments:
In `@src/adapters/openai-responses.ts`:
- Around line 600-615: In src/adapters/openai-responses.ts:600-615, update the
grouping logic around the pairs-processing loop to return the original body when
a later call begins before any output in the current group has completed,
preventing rewriting across a split result batch. In
tests/deepseek-inbound-wire.test.ts:930-961, add the interleaved callA, callB,
outputA, callC, outputB, outputC sequence and assert body.input remains
unchanged.
🪄 Autofix
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: c4f306e5-ac35-4454-8a93-c201f637dfc4
📒 Files selected for processing (8)
docs-site/src/content/docs/ja/reference/adapters.mddocs-site/src/content/docs/ko/reference/adapters.mddocs-site/src/content/docs/ru/reference/adapters.mddocs-site/src/content/docs/zh-cn/reference/adapters.mddocs-site/src/content/docs/zh-tw/reference/adapters.mdsrc/adapters/openai-responses.tssrc/types.tstests/deepseek-inbound-wire.test.ts
4 tasks
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.
Summary
Supersedes #1479, rebased onto the latest dev head (e8db4e0), and resolves the blocking CHANGES_REQUESTED review.
Root cause (remaining blocker this PR closes)
The provider-scoped #1292 normalizer repaired every call/result pair independently, so reasoning + call A + call B + output A + output B became two assistant turns and DeepSeek rejected the second call for missing reasoning_text (#1477).
The batch grouping fixed the parallel case but still skipped calls with no matching output. A partially matched history such as [call A, call B, injected, output B] could move output B ahead of injected even though call A was unresolved, violating the documented fail-closed boundary for ambiguous histories.
This change returns the original body unchanged whenever any collected call lacks exactly one later matching result, and documents duplicate, missing, and out-of-order pairs as left untouched.
Verification
Checklist
Summary by CodeRabbit
Bug Fixes
Documentation