fix(xai): seed Responses tool-result adjacency for interrupted Codex threads - #4871
Conversation
…threads Grok 4.6/4.5 OAuth Responses replays Codex tool history. After a mid-stream 502/reset the client can resend a function_call without its output, or with hook-injected developer context between the pair. Google already synthesizes a missing tool_result; xAI did not, so later turns in the same thread 400. Reuse requiresAdjacentResponsesToolResults (Kimi lidge-jun#4726, DeepSeek lidge-jun#1292) and run the existing orphan-call placeholder for non-forward adjacency providers. Do not set statelessResponses: xAI stores responses for 30 days. Closes lidge-jun#4870
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe xAI Responses configuration now supports separate adjacency and paired-result capabilities. Replayed stateful requests can synthesize missing tool outputs without removing ChangesxAI Responses paired tool-result repair
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant ProviderRegistry
participant enrichProviderFromRegistry
participant buildRequest
participant repairOrphanedInputItems
participant xAIResponsesAPI
ProviderRegistry->>enrichProviderFromRegistry: Seed paired and adjacent capabilities
enrichProviderFromRegistry->>buildRequest: Provide resolved xAI configuration
buildRequest->>repairOrphanedInputItems: Repair missing tool outputs
repairOrphanedInputItems->>xAIResponsesAPI: Send repaired stateful Responses input
Merge Risk: ⚪ Minimal · up to The replay repair is covered across the configured xAI, Kimi, and forward-auth behaviors, with no concrete unresolved merge risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 9 files. (2 skipped: 2 unsupported.)
✨ 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 |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. Automatic draft conversion failed (token cannot change draft status). Please convert this pull request to a draft manually. The required |
리뷰 · 우선순위 74 / 80이 PR은 MerryEcho의 Draft 버그 수정이다. 제목대로 지금 증상은 #4870에 숫자로 적혀 있다. 첫 실패는 대개 업스트림 502/connection-reset(자주 ~15초)이다. 그 뒤 같은 스레드의 다음 요청 실패율이 약 20.6%로 치솟고, 한 번도 안 깨진 이웃 스레드는 깨끗하다. Google는 이미 빠진 고치는 방법은 기존 능력을 재사용한다. (1) xAI 레지스트리에 테스트 네 축이 핵심이다. 레지스트리 시드가 있고 types.ts/config.ts 대분할 캠페인으로 바로 무효화되는 PR은 아니다. 레지스트리 capability 한 줄과 passthrough 게이트 확장, 문서·테스트다. 다만 passthrough 게이트를 passthrough.ts / synthesizeMissingCallOutputs 게이트 - forward||stateless만 돌리던 orphan 합성을 adjacency 공급자까지 넓힌다. xAI에는 필요하지만 Kimi/kimi-code(Responses로 붙을 때) 동작도 함께 바뀐다. 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Gating the missing-output synthesis on requiresAdjacentResponsesToolResults enrolled kimi and kimi-code in it too. Both carry that flag because their parser rejects a hook-split pair, but the same report (lidge-jun#4726) shows a call left without any result is accepted, so they would have started receiving placeholder tool turns for a shape they never rejected. Adjacency reorders items the upstream would accept in some order. Pairing synthesizes an item the client never sent, which is a larger claim about what happened in the conversation, so it gets its own capability: requiresPairedResponsesToolResults, seeded on xai only. statelessResponses still implies it, which is how DeepSeek already had the repair. Stateful behavior is untouched: store and previous_response_id survive, and forward auth still suppresses synthesis.
|
Reviewed at The blocker, and what the follow-up does. Gating the missing-output synthesis on That matters beyond the registry bookkeeping. Adjacency reorders items the upstream would accept in some order, so it is safe to apply wherever the parser is strict about ordering. Synthesis inserts an item the client never sent, which is a claim about what happened in the conversation. Those deserve separate capabilities, and So What I verified and agree with in your change.
One thing still open for a maintainer. No local suite was run for the follow-up commit; it was verified by source reading and hosted CI on this head, per the constraints this review is operating under. |
…actually takes The previous case declared no custom tool, so collectRoutedCustomToolNames found nothing and the lowering never ran; it proved only that the repair indexes custom calls. xAI sets supportsResponsesCustomTools: false, so the production shape is a declared custom tool that gets lowered, and that is what this now asserts end to end.
… page providers/xai-grok.md co-owns src/providers/ and says the surface retains its existing behavior, which stopped being true once a capability was seeded for xAI alone. Points at chat-compat.md rather than restating the contract in two places.
…d policy PROVIDER_CONFIG_FIELD_POLICY is satisfies Record<keyof OcxProviderConfig, ...>, so adding a field to OcxProviderConfig without a policy row fails the whole typecheck from a different file. That is what broke gates and the production adapter contract test on Linux and Windows. editor matches the sibling wire capabilities (statelessResponses, requiresAdjacentResponsesToolResults, annotateEmptyToolOutputs) and matches what the policy means: the field is user-authorable through the leaf validator schema and is seeded from the registry only when the user has not set it. It carries no credential and is not a runtime observation.
|
Maintainer integration note (recorded per MAINTAINERS.md). Repository CI evidence for this tree exists and was produced by a maintainer, not by the author. The exact tree at An earlier dispatch of this tree failed on Two further dispatches of this tree failed on Windows for a reason that was not in this change: the evidence branch pre-dated #4876, and Marking this ready for review on that basis. The readiness checklist's local-CI box is an author attestation that a fork contributor cannot satisfy against repository CI; the dispatch above is stronger evidence of the same property, and it is recorded here rather than asserted. Authorship and the |
|
@MerryEcho — this is ready from our side and the remaining step is yours. Repository CI has now run on this exact head ( That gate is what is holding the PR in draft. It needs the four boxes in the description ticked by you: local CI green, branch on the latest Once the boxes are ticked the gate marks the PR ready and we will merge it. Thanks for the fix. |
|
Merging with maintainer admin rights. Three non-green entries at this head, and none of them is a defect in this change.
Everything substantive passed: |
Summary
xai/grok-4.6OAuth Responses often fail once (upstream 502 / connection-reset), then keep failing because the replayed input still has afunction_callwithout a matching output, or a hook-injected developer message between the pair.requiresAdjacentResponsesToolResultson the xAI registry entry (same capability as Kimi [Provider] kimi: no requiresAdjacentResponsesToolResults → 400 when hook-injected developer context splits a tool call from its result #4726 and DeepSeek [Provider compatibility] DeepSeek V4 Flash returns 400 when developer message is interleaved between function_call and function_call_output #1292) so passthrough reorders an unambiguous call/result batch.requiresPairedResponsesToolResults, for the other half of the same interrupt: a call whose output never arrived at all. xAI carries both;statelessResponsesimplies it, which is how DeepSeek already had the repair.kimiandkimi-code, and [Provider] kimi: no requiresAdjacentResponsesToolResults → 400 when hook-injected developer context splits a tool call from its result #4726 shows Kimi accepting a call with no result at all — they would have started receiving placeholders for a shape they never rejected.statelessResponses: its Responses API stores conversations for 30 days and documentsprevious_response_id.storeandprevious_response_idsurvive the repair, and forward auth still suppresses the synthesis because the backend holding the conversation can resolve the pair itself.Closes #4870
Verification
Author's verification of the first commit (7026676):
bun test tests/providers/xai/xai-responses-adjacency.test.ts tests/providers/kimi-responses-adjacency.test.ts tests/responses/responses-forward-dangling-call.test.ts tests/responses/responses-stateless-dangling-call-repair.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts— 44 pass / 0 failbun test tests/providers/deepseek-inbound-wire.test.ts tests/ci-workflows/structure-ssot.test.ts tests/providers/provider-registry-parity.test.ts tests/server/server-combo-reasoning-replay-eligibility.test.ts— 134 pass / 0 failbun run typecheck— passMaintainer follow-up commit (d7e4efc), which separates pairing from adjacency, was verified by source reading and hosted CI on this head only. No local suite was run for it.
requiresAdjacentResponsesToolResults:src/types/provider.ts,src/providers/registry/types.ts, both sites insrc/providers/derive.ts,src/config/schema/leaf-validators.ts, and the stale-row backfill insrc/router.ts. Neither flag is exposed throughsrc/config/provider-validation.tsor the management provider routes, so nothing further is needed there.dev: it is notforward, notstatelessResponses, and no longerrequiresPairedResponsesToolResults, sorepairOrphanedInputItemsdoes not run for it at all.tests/providers/kimi-responses-adjacency.test.tscovers matched pairs only and is unaffected.synthesizeMissingCallOutputsis!forward && ..., so a forward request still callsrepairOrphanedInputItems(outBody, unexpandedMiss, false).kimiandkimi-codecarry adjacency without pairing, and a danglingcustom_tool_callis paired beforerewriteRoutedCustomToolsForUpstreamlowers it.structure/providers/chat-compat.mdrecords both capabilities and why they do not collapse into one.PROVIDER_CONFIG_FIELD_POLICYinsrc/server/auth-cors.tsissatisfies Record<keyof OcxProviderConfig, ProviderConfigFieldPolicy>, so the new capability needs a policy row or the whole typecheck fails from a file the change never touched. It is classifiededitor, matching its sibling wire capabilities (statelessResponses,requiresAdjacentResponsesToolResults,annotateEmptyToolOutputs) and matching what that policy means in the record's own comment: the field is user-authorable through the leaf validator schema and is seeded from the registry only when the user has not set it, it carries no credential, and it is not a runtime observation.Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
Bug Fixes
Documentation