Skip to content

fix(responses): keep untyped assistant messages inside their tool-call turn - #8064

Merged
lidge-jun merged 3 commits into
lidge-jun:devfrom
fec123:fix/deepseek-shorthand-assistant-turn
Nov 3, 2026
Merged

lidge-jun merged 3 commits into
lidge-jun:devfrom
fec123:fix/deepseek-shorthand-assistant-turn

Conversation

@fec123

@fec123 fec123 commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

ZCode sends assistant text as the Responses message shorthand — {"role":"assistant","content":[...]} with no type. isAssistantTurnItem recognized only a typed message, so the shorthand closed the assistant turn:

before: [user] [assistant text] | [function_call] [output]
        turn split; the bare-tool-call placeholder lands after the text
after:  [user] [assistant text] [function_call] [output]
        one turn; the placeholder lands at the head, before the text

DeepSeek's strict plaintext replay contract then rejects every continuation of that session with The reasoning_text in the thinking mode must be passed back to the API. Switching models away and back does not help, because the client keeps rebuilding the same history shape.

The fix normalizes the item type the same way the Responses parser already does (type ?? (role present ? "message" : undefined)), so a shorthand with role: "assistant" belongs to the surrounding assistant turn. The placeholder then precedes the whole turn — the shape the typed-message case already produces and DeepSeek accepts.

Scope: isAssistantTurnItem is used only by backfillBareToolCallTurnReasoning, which runs only for providers that declare both preserveResponsesReasoningContent and requiresAdjacentResponsesToolResults. Other providers keep byte-identical behavior.

Verification

Added two cases to the existing regression file for this contract (tests/providers/deepseek-bare-tool-call-replay.test.ts): the turn-grouping helper, and the DeepSeek passthrough wire ending up as message, reasoning, undefined (shorthand), function_call, function_call_output.

bun test ./tests/providers/deepseek-bare-tool-call-replay.test.ts
# 8 pass, 0 fail

bun test ./tests/test-layout.test.ts
# 2 pass, 0 fail

bun test ./tests/ci-workflows/file-size-ratchet.test.ts
# 9 pass, 0 fail

bun run typecheck
# No type errors in 2,259 files

Before/after on the wire: replaying a captured failing ZCode request body against a live proxy returned HTTP 400 with the message above; the same body returns HTTP 200 after this change, including with stream: true.

Full-suite exception: bun run test was not run locally. This is a two-file change — one helper in a Responses adapter and its own regression file — and the focused test above exercises the changed behavior. The two gates that can reject a test-case addition (test layout, file-size ratchet) were run explicitly. Remaining coverage is left to the sharded PR CI.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (No user-facing option or documented behavior changed.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (No auth, credential, workflow, or dependency surface touched.)

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes
    • Improved compatibility with Responses messages that omit an explicit type. Assistant messages are now correctly recognized during tool-call replay, and their original format is preserved when sent to DeepSeek.
    • Added regression coverage for reasoning backfill when a shorthand assistant message appears before a tool call.

…l turn

ZCode sends assistant text as the Responses message shorthand
({role, content} with no type). isAssistantTurnItem only recognized a typed
message, so the shorthand closed the assistant turn and
backfillBareToolCallTurnReasoning inserted its placeholder after the text
instead of at the head of the turn. DeepSeek's thinking mode rejects that
ordering with "The reasoning_text in the thinking mode must be passed back to
the API", and every continuation of the session fails the same way.

Normalize the item type the way the Responses parser already does, so a
shorthand with role=assistant belongs to the surrounding assistant turn. The
placeholder then precedes the whole turn, matching the typed-message shape
that already passes.
@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration
Commits

Reviewing files that changed from the base of the PR and between ca443ca and 16149ea.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

isAssistantTurnItem now accepts message objects that have a role but no type. Regression tests cover reasoning backfill and DeepSeek wire output for this shorthand message shape.

Changes: Assistant turn handling

Layer / File(s) Summary
Recognize shorthand assistant messages
src/adapters/openai-responses/reasoning.ts, tests/providers/deepseek-bare-tool-call-replay.test.ts
isAssistantTurnItem now accepts objects with a role and no type. Tests check that placeholder reasoning appears before the shorthand assistant message and tool call, and that the shorthand message's type remains undefined in the wire output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: lidge-jun

Merge Risk: Minimal · previous review ca443

Untyped assistant messages remain grouped with their tool calls. No material merge risk was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the main change: untyped assistant messages (shorthand {role, content} items) are now kept inside their tool-call turn. The raw summary says isAssistantTurnItem now recognizes role-bea…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: untyped assistant messages (shorthand {role, content} items) are now kept inside their tool-call turn. The raw summary says isAssistantTurnItem now recognizes role-bearing items without a type as messages, and the PR objectives describe the same fix for backfillBareToolCallTurnReasoning placement. The title is specific and matches the changeset, so it passes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 98bd8c0 and ca443ca.

Files selected for processing: 2
  • src/adapters/openai-responses/reasoning.ts
  • tests/providers/deepseek-bare-tool-call-replay.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 31, 2026
@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked.

Hygiene

✅ Deterministic PR hygiene checks passed.

@github-actions
github-actions Bot marked this pull request as ready for review October 31, 2026 23:11
@lidge-jun lidge-jun added the ci:full Run the four full Ubuntu test shards on this pull request label Nov 3, 2026
@github-actions
github-actions Bot marked this pull request as draft November 3, 2026 09:21
@lidge-jun
lidge-jun marked this pull request as ready for review November 3, 2026 09:38
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer integration into dev (account lidge-jun) at head 16149ea76bb25d5707e03a71f74399e8967c2171. The PR is moved out of draft by a maintainer for this integration (the checklist gate re-drafted it after a maintainer dev-merge push).

fix(responses): keep untyped assistant messages inside their tool-call turn.

  • The branch merges current dev (dev-merge only); later dev commits share no source file with it. Exact-head required CI passed: ci, enforce-target, hygiene, test 1/4–4/4.
  • Independent review (gpt-6.1-terra, high) on the code merged onto dev: focused tests plus layout and file-size guards 82 pass / 0 fail; narrowly scoped, no auth/credential/logging change.
  • No unresolved threads or changes-requested reviews.

Thanks for the fix.

@lidge-jun
lidge-jun merged commit fde2e4c into lidge-jun:dev Nov 3, 2026
31 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T09:44:02.889248Z 16149ea Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16149ea76b

ℹ️ 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".

// text that way, directly ahead of the same turn's function calls; reading only a typed
// `message` would split the turn and place the placeholder backfill after the text, which
// DeepSeek refuses with the same reasoning_text 400.
const type = typeof item.type === "string" ? item.type : "role" in item ? "message" : undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update the mapped replay contract alongside the adapter change

This changes assistant-turn membership and DeepSeek wire ordering, but the commit updates none of the mapped structure documentation. The scoped src/AGENTS.md explicitly requires that “every doc listed for an area is updated in the same change that changes the area.” Update structure/transports/native-reasoning-retention.md to record that untyped assistant messages remain inside the tool-call turn and that the reasoning placeholder precedes their text, and review the other mapped adapter documents. Otherwise this compatibility-sensitive history shape remains documented only in implementation comments and tests rather than the required architectural contract.

AGENTS.md reference: src/AGENTS.md:L11-L11

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ci:full Run the four full Ubuntu test shards on this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants