Skip to content

fix(deepseek): normalize tool result adjacency - #1334

Closed
Ingwannu wants to merge 1 commit into
devfrom
agent/fix-1292-deepseek-tool-order
Closed

fix(deepseek): normalize tool result adjacency#1334
Ingwannu wants to merge 1 commit into
devfrom
agent/fix-1292-deepseek-tool-order

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • Normalize unambiguous DeepSeek Responses tool-call continuations so each result immediately follows its matching call.
  • Preserve hook-injected developer content by moving it after the matched result instead of dropping it.
  • Gate the behavior behind provider capability metadata so tolerant Responses providers keep their original history order.
  • Backfill the DeepSeek capability into stale provider configurations and document the provider-specific decision boundary.

Verification

  • taskset -c 0-1 bun run typecheck — passed on the rebased exact head.
  • taskset -c 0-1 bun test tests/deepseek-inbound-wire.test.ts tests/openai-responses-passthrough.test.ts tests/deepseek-reasoning-replay.test.ts tests/deepseek-reasoning-replay-gaps.test.ts tests/config.test.ts tests/provider-registry-parity.test.ts tests/config-save-boundary.test.ts — 277 passed, 0 failed on the rebased exact head.
  • Full pre-rebase suite — 10,166 passed, 10 skipped, 2 unrelated local-environment failures:
    • the existing Codex shim token-file test resolves the live OpenCodex token path in this running proxy environment and also fails in isolation;
    • the npm cache worker test exceeded its 5-second bound during the 828-second CPU-limited suite, then passed 11/11 in 1.1 seconds when rerun alone.
  • git diff --check origin/dev...HEAD — passed.

Decision Log

  • 목적과 의도: Make valid Codex tool continuations acceptable to DeepSeek when hook context is persisted between a call and its result.
  • 기존 구현 및 제약 조건: Existing orphan repair checks pair presence only; globally changing valid history would affect tolerant providers and moving or dropping the developer message is semantically meaningful.
  • 검토한 주요 대안: Reorder all Responses providers, drop the injected message, or add a provider capability and perform a lossless one-to-one reorder.
  • 선택한 방식: Enable adjacency normalization only for DeepSeek, move only unique matched pairs, and preserve every intervening item after the result.
  • 다른 대안 대신 이 방식을 선택한 이유: This fixes the confirmed strict parser without changing tolerant provider history or guessing across duplicate call ids.
  • 장점, 단점 및 영향: DeepSeek accepts the continuation and no context is lost; the relative position of intervening context moves after the tool result only on the declared strict provider.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. The provider contract, type documentation, and in-code Decision Log are updated; no user command or configuration workflow changes.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. No credential destination or authentication path changes.

Closes #1292

Summary by CodeRabbit

  • Bug Fixes
    • Improved compatibility with providers that require tool results to immediately follow their corresponding tool calls.
    • Automatically preserves message order when tool-call matches are ambiguous or when providers do not require adjacent results.
    • Enabled this behavior for DeepSeek while preserving intervening developer context.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds provider-aware Responses history normalization. Enabled providers move uniquely matched tool results directly after their calls. DeepSeek enables the capability, while explicit provider settings remain authoritative.

Changes

Responses tool-result adjacency

Layer / File(s) Summary
Provider capability and configuration flow
src/types.ts, src/config.ts, src/providers/registry.ts, src/providers/derive.ts, src/router.ts
Adds the optional requiresAdjacentResponsesToolResults capability, enables it for DeepSeek, and propagates it through provider seed, backfill, and routing paths without overriding explicit values.
Responses request normalization
src/adapters/openai-responses.ts
Indexes function, shell, and custom tool calls and outputs. When enabled, request construction moves only uniquely matched non-adjacent outputs directly after their calls.
DeepSeek capability and ordering tests
tests/deepseek-inbound-wire.test.ts
Tests registry propagation, DeepSeek reordering with preserved developer context, and unchanged interleaved history for providers without the capability.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ProviderRegistry
  participant Router
  participant OpenAIResponsesAdapter
  participant DeepSeek
  ProviderRegistry->>Router: provide adjacency capability
  Router->>OpenAIResponsesAdapter: construct provider configuration
  OpenAIResponsesAdapter->>OpenAIResponsesAdapter: normalize unique tool call/result pairs
  OpenAIResponsesAdapter->>DeepSeek: send reordered Responses input
Loading

Possibly related PRs

Suggested labels: review-ready

Suggested reviewers: lidge-jun, wibias

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #1292 by conditionally moving matching tool results next to calls while preserving intervening developer content.
Out of Scope Changes check ✅ Passed The provider capability plumbing, DeepSeek registry configuration, normalization logic, and tests all support the linked issue objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: normalizing DeepSeek tool result adjacency.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/fix-1292-deepseek-tool-order

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

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev as 56cca2f, rebased onto the current head with your authorship preserved.

Verified before merge: src/adapters/openai-responses.ts:507-535 only checked functionCallIds.has(callId) and preserved original ordering, with no adjacency normalization in the forwarding pipeline at :1158-1160 — so developer context between a tool call and its result really did break DeepSeek continuations. Gating the normalization on the provider capability and moving only uniquely matched pairs is the right conservative shape: tolerant providers are untouched and intervening content survives. Full suite green (10243 pass, 0 fail).

Thanks — the stale-config backfill and the tolerant-provider control test are what make the capability plumbing convincing rather than just plausible.

@lidge-jun lidge-jun closed this Aug 9, 2026
@Wibias
Wibias deleted the agent/fix-1292-deepseek-tool-order branch August 9, 2026 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants