fix(runtime): route DeepSeek reasoning through Open Responses with tool fallbacks - #2972
Conversation
English — Request changesConclusionThis PR addresses a real bug, and routing DeepSeek through The root cause is not that DeepSeek uses a separate “plaintext Responses dialect.” It is a mismatch between:
The PR currently collapses those concerns into Root cause1.
|
|
Follow-up: I opened vercel/ai#18839 for the underlying If the schedule allows us to wait for an upstream fix, I strongly recommend doing so before landing this adapter. A correct upstream implementation that preserves item order, item identity, and opaque reasoning state would let us remove a substantial amount of local replay/dialect compensation and avoid owning a parallel Responses codec in Maka. After that fix, the Maka-side design could be much smaller:
This would also make the abstraction boundary clearer: encrypted versus plaintext reasoning is not a provider “dialect” distinction—the Responses/Open Responses item model permits both. The generic adapter should preserve either representation, while provider-specific code should cover only actual provider extensions. If waiting is not possible, the current workaround should be treated as temporary, isolated behind a DeepSeek-specific boundary, and covered by round-trip tests so that it can be deleted when the upstream issue is fixed. |
me2seeks
changed the title
There was a problem hiding this comment.
Anchored to
f531b0f58; I posted this 39 minutes after9e9d1461clanded, which is the same version slip I attribute to another review below, with less excuse. Re-checked against9e9d1461c: all five inline findings still hold and the design note is now obsolete — corrected in place. The upstream table is unaffected, being probe results against the published 2.0.27 rather than against this branch.
The direction is right and the core claim holds. I drove a full AiSdkBackend.send() against a DeepSeek connection with a real getAIModel and a capturing fetch; the second-round body is what you say it is:
POST https://api.deepseek.com/responses
input: [reasoning{content:[reasoning_text]}, function_call, function_call_output, message]
The OpenAI encrypted path is not regressed. Deleting the self-maintained transport is the right call — upstream reads response.reasoning_text.delta and now writes content[].reasoning_text back, which is exactly the half the old transport could not fix.
Most of what came out of review is upstream's to fix, not yours. Splitting it that way leaves you with a short list.
Yours: one blocker
Stored apply_patch history replays as a malformed call. Sessions created on main stored freeform patch strings — ApplyPatch is the default routing for v4-flash/pro there. With the profile now null, normalizeApplyPatchReplayInput returns the stored input untouched and it reaches the wire as non-JSON arguments naming a tool that is no longer declared. Two reviewers reproduced the same body independently. downgradedApplyPatchCalls is the intended handler for exactly this and is unreachable behind an early return. Detail inline; this is the only finding no upstream change can rescue.
Three P1s, each small, all in seams this PR moved:
- session thinking level now bypasses
resolveThinkingLevel, so undeclared efforts reach the wire (model-adapter.ts:281) - the plaintext reasoning branch has no empty-text guard, unlike both paths it replaces (
ai-sdk-backend.ts:3756) execution-model-authority.ts:499never got the second effort channel, so DeepSeek's title/memory/evaluator calls now send noreasoningat all
One P2 in the same family: upstream's assistant branch only recognises reasoning / text / tool-call, so a provider-executed tool-result is dropped while its tool-call still serialises — I reproduced a function_call with no matching function_call_output. canReplayProviderNative (ai-sdk-backend.ts:3686) does not screen for this. Model switching isn't from this PR, but routing DeepSeek through this codec is what makes "used hosted search on OpenAI, then switched to DeepSeek" reach it.
Upstream's: file these, don't work around them
Each of these is reproducible against the published 2.0.27. Several overlap with what @M4n5ter is already filing, so they are probably one issue rather than seven.
| # | Behaviour | Probe result |
|---|---|---|
| 1 | effort: 'max' removes reasoning from the body entirely |
max → body.reasoning undefined + one unsupported warning; enum is minimal/low/medium/high/xhigh (open-responses-language-model.ts:159-173) |
| 2 | Non-function tools are dropped with zero warning |
1 function + 2 provider-defined in → body.tools has 1, warnings: [] (:120-128) |
| 3 | Converter regroups assistant parts, losing interleaved order | text→reasoning→toolcall→reasoning→text out as [reasoning, reasoning, message(merged), function_call] (convert-to-open-responses-input.ts:105-161) |
| 4 | Serializer hardcodes summary: [] and emits no id / encrypted_content |
part providerOptions discarded (:115-119) |
| 5 | Decoder drops summary, encrypted_content, item ids, annotations |
a reasoning item with only summary decodes to 0 parts; providerMetadata hardcoded undefined (:262-297, :336) |
| 6 | Stream and non-stream disagree on the same response | stream keeps item ids, non-stream has none; stream reads reasoning only from response.reasoning_text.delta, which upstream's own comment calls a non-spec LM Studio extension (:507-519) |
| 7 | Unknown incomplete_details.reason + tool calls → tool-calls |
a truncated response is reported as a normal tool turn; raw keeps the real reason, unified does not (map-open-responses-finish-reason.ts:10-20) |
| 8 | Truncation flush emits reasoning-end with a hardcoded reasoning-0 |
real id was rs_REAL_ID; AI SDK core then injects error: reasoning part reasoning-0 not found (:567-571) |
| 9 | No way to send store, and no previous_response_id support |
body has neither |
#1 is why max maps to xhigh here, and that mapping is correct — do not change it. Passing max through does not clamp, it deletes the whole reasoning field, so the model runs with thinking off. DeepSeek's table is xhigh → high, max → max, so today max runs at high where main ran at max. That is a measurable regression for every DeepSeek run at max, benchmark baseline included, and it should be stated in the description rather than left implicit — the PR already notes no live credential smoke test was run.
Correcting myself on the fix for #1. I first wrote that upstream should add max to the effort enum. That enum is in @ai-sdk/provider (reasoning?: 'provider-default' | 'none' | 'minimal' | 'low' | 'medium' | 'high' | 'xhigh') — the cross-provider abstraction, which should not carry each vendor's private ladder. The real gap is that @ai-sdk/open-responses exposes exactly one provider-specific field, reasoningSummary, and nothing equivalent for effort, so effort can only arrive through the standard enum and a hardcoded effortMap. That is the issue I will file. Separately, we do not have to wait for it: requestBodyOverlay (llm-connections.ts:122, applied at the fetch layer in model-factory.ts:62-66) can put a literal { reasoning: { effort: 'max' } } on the wire per connection, so restoring real max for DeepSeek is available to this PR today.
#2 is why apply_patch and native web search are off, and the zero-warning part is what makes it dangerous: a provider tool that silently never ships looks identical to one the model chose not to call. Both capabilities return on their own once upstream serializes provider-defined tools — worth a comment at the two disable sites recording that recovery condition.
#7 and #8 do not stay upstream — Maka amplifies both. model-adapter.ts:897 turns the SDK error chunk from #8 into a ModelFailure, so hitting max_output_tokens mid-reasoning ends the step with an unintelligible provider error instead of length. And model-adapter.ts:567 only treats other/unknown as truncated, so #7's fake tool-calls is accepted as success and the tool loop continues. Both are worth a local guard even before upstream lands — the raw finish reason is already available on streamedFinishReason.
Two things about the description
The capability regression belongs in the title. Swapping applyPatchProtocol for responsesDialect is one line, but it changes the headless coding tool surface from apply_patch to Write/Edit and rewrites HEADLESS_CODING_V1_TOOLS_HASH. That is material user-visible behaviour under AGENTS.md and refactor: does not carry it. Equivalence on terminal-bench cannot be assumed — freeform V4A patches edit multiple files per call, while Write/Edit requires reading first and re-matching strings, costing turns and input tokens. An A/B on one or two tasks, or an explicit accepted-regression note, would settle it.
Tavily is not actually still available. That only holds when defaultProvider is explicitly tavily; at the default 'model', native-web-search-tool.ts:83 removes the client WebSearch tool entirely, so a DeepSeek session has no search at all and nothing is logged. Benchmarks are unaffected — eval blocks web tools regardless.
On @M4n5ter's review
Thorough, and the upstream reproductions in it are the most valuable thing anyone has produced on this. Both of its blocking findings point at code that no longer exists — it landed 32 minutes after f531b0f58 deleted open-responses-plaintext-model.ts. Its design argument about responsesDialect was the part that still stood, and 2c3f18620 has since answered it: the concern is split into responsesAdapter and responsesReasoningReplay, and model-adapter.ts:283 gates on runtime.responsesAdapter rather than deriving the adapter from the reasoning replay shape. That is the right shape, and it is also what one of my reviewers arrived at independently. Nothing left to answer there.
Remaining P2 notes are inline. Nothing else blocks.
Review assistance: Claude Code (Opus) ran seven independent fresh-eye passes (runtime correctness, capability regression, test coverage, architecture, upstream package behaviour, persistence compatibility, dependency/security), each blind to the others' findings. I verified the DeepSeek effort table against the official docs, confirmed the store: false filter in @ai-sdk/openai at dist/index.js:5328-5338, and dropped two agent conclusions that did not survive that check.
中文
方向是对的，核心主张也成立。我用真实 getAIModel 加捕获 fetch 驱动了完整的 AiSdkBackend.send()，第二轮请求体确实如你所说：
POST https://api.deepseek.com/responses
input: [reasoning{content:[reasoning_text]}, function_call, function_call_output, message]
OpenAI 加密路径没有回归。删掉自维护的 transport 是对的——上游已经读 response.reasoning_text.delta，现在请求侧也会写回 content[].reasoning_text，正是旧 transport 修不了的那一半。
评审出来的问题大部分该由上游修，不是你的事。这样切开之后，留给你的清单很短。
你的：一个阻塞项
存量 apply_patch 历史会重放成非法调用。 在 main 上创建的会话存的是 freeform 补丁字符串——而 ApplyPatch 正是 v4-flash/pro 在那边的默认路由。现在 profile 为 null，normalizeApplyPatchReplayInput 原样返回存储内容，最终以非 JSON 的 arguments 调用一个已不再声明的工具上线。两位评审独立复现出同一个请求体。downgradedApplyPatchCalls 正是为这种情况准备的处理路径，却被一个提前 return 挡在外面够不着。细节在行内；这是唯一一条上游怎么改都救不了的发现。
另有三条 P1，都很小，都落在本 PR 挪动过的 seam 上：
- 会话 thinking level 现在绕过了
resolveThinkingLevel，模型未声明的档位会直接上线（model-adapter.ts:281） - 明文 reasoning 分支没有空文本护栏，而它取代的两条路径都有（
ai-sdk-backend.ts:3756） execution-model-authority.ts:499没跟上新增的第二条 effort 通道，DeepSeek 的 title/memory/evaluator 调用现在完全不带reasoning
同一类的还有一条 P2：上游的 assistant 分支只认 reasoning / text / tool-call，所以 provider-executed 的 tool-result 会被丢弃、而它的 tool-call 仍被序列化——我复现出了一个没有对应 function_call_output 的 function_call。canReplayProviderNative（ai-sdk-backend.ts:3686）没有筛掉这种情况。会话内换模型不是本 PR 引入的，但让 DeepSeek 走上这条 codec，才使得「在 OpenAI 上用过 hosted 搜索、然后切到 DeepSeek」这条路径变得可达。
上游的：提上去，不要在我们这边绕
以下每条都能对已发布的 2.0.27 复现。其中几条和 @M4n5ter 正在提的重叠，估计合成一个 issue 就够，不需要七个。
| # | 行为 | 探针结果 |
|---|---|---|
| 1 | effort: 'max' 会让 reasoning 字段整个从请求体消失 |
max → body.reasoning undefined + 一条 unsupported warning；枚举只有 minimal/low/medium/high/xhigh（open-responses-language-model.ts:159-173） |
| 2 | 非 function 工具被丢弃且 warnings 为空数组 |
传入 1 个 function + 2 个 provider-defined → body.tools 只剩 1 个，warnings: []（:120-128） |
| 3 | converter 重新分组 assistant part，丢失交错顺序 | text→reasoning→toolcall→reasoning→text 发出为 [reasoning, reasoning, message(合并), function_call]（convert-to-open-responses-input.ts:105-161） |
| 4 | 序列化侧写死 summary: []，不发 id 和 encrypted_content |
part 上的 providerOptions 被丢弃（:115-119） |
| 5 | decoder 丢掉 summary、encrypted_content、item id、annotation |
只带 summary 的 reasoning item 解出 0 个 part；providerMetadata 写死 undefined（:262-297、:336） |
| 6 | 流式与非流式对同一份响应解码结果不同 | 流式保留 item id，非流式完全没有；流式只从 response.reasoning_text.delta 读 reasoning，而上游自己的注释说这是非 spec 的 LM Studio 扩展（:507-519） |
| 7 | 未知 incomplete_details.reason + 有 tool call → tool-calls |
被截断的响应被报告成正常工具轮次；raw 里留着真实原因，unified 已经说谎（map-open-responses-finish-reason.ts:10-20） |
| 8 | 截断时 flush 出的 reasoning-end 用硬编码的 reasoning-0 |
真实 id 是 rs_REAL_ID；AI SDK core 随即注入 error: reasoning part reasoning-0 not found（:567-571） |
| 9 | 无法发送 store，也不支持 previous_response_id |
请求体里两者都没有 |
第 1 条就是这里把 max 映射成 xhigh 的原因，而这个映射是对的，不要改。 直传 max 不是钳位，是把整个 reasoning 字段删掉，模型会在关闭思考的状态下运行。DeepSeek 的映射表是 xhigh → high、max → max，所以现在 max 实际跑在 high，而 main 上跑的是 max。这对所有以 max 运行的 DeepSeek 会话（含跑分基线）都是可度量的退化，应当写进描述而不是留白——PR 自己也写了没有做真实凭据的 smoke test。
关于第 1 条的修法，更正我自己。 我最初写的是让上游给 effort 枚举补上 max。那个枚举在 @ai-sdk/provider （reasoning?: 'provider-default' | 'none' | 'minimal' | 'low' | 'medium' | 'high' | 'xhigh'），是跨 provider 的标准化抽象，不应该承载各家私有的档位。真正的缺口是 @ai-sdk/open-responses 的 provider 专属选项里只开了 reasoningSummary 一个字段，effort 没有对应通道，只能走标准枚举加一张写死的 effortMap。我要提的是这个 issue。另外这件事不必等上游：requestBodyOverlay（llm-connections.ts:122，在 model-factory.ts:62-66 的 fetch 层生效）可以按连接把 { reasoning: { effort: 'max' } } 原样打到请求体上，所以在这个 PR 里就能把 DeepSeek 的真 max 恢复回来。
第 2 条就是 apply_patch 和原生 web search 被关掉的原因，而「零 warning」才是真正危险的地方：一个永远发不出去的 provider 工具，和一个模型选择不调用的工具，从外部看完全一样。一旦上游支持序列化 provider-defined 工具，这两项能力会自己回来——建议在两个关闭点各写一句记录恢复条件。
第 7、8 条不会止步于上游——Maka 把两者都放大了。 model-adapter.ts:897 会把第 8 条产生的 SDK error chunk 转成 ModelFailure，于是在 reasoning 未闭合时撞上 max_output_tokens，这一步不是以 length 结束，而是以一条用户看不懂的 provider 错误终结。而 model-adapter.ts:567 只把 other/unknown 判为截断，所以第 7 条那个假的 tool-calls 会被当成功接受，工具循环继续跑下去。即便上游还没修，这两处都值得在本地加个守卫——原始 finish reason 在 streamedFinishReason 上已经拿得到。
关于描述的两点
能力回退应当体现在标题里。 用 responsesDialect 顶掉 applyPatchProtocol 只是一行，但它把 headless coding 的工具面从 apply_patch 换成了 Write/Edit，并重写了 HEADLESS_CODING_V1_TOOLS_HASH。按 AGENTS.md 这属于 material user-visible behaviour，refactor: 承载不了。terminal-bench 上的等价性不能假定：freeform V4A 补丁一次调用可以改多个文件，而 Write/Edit 需要先读再精确匹配字符串，这会多花轮次和输入 token。在一两个 task 上做个 A/B，或者写明这是已接受的退化，都可以。
Tavily 其实并没有保住。 只有在 defaultProvider 显式设为 tavily 时才成立；默认值是 'model'，此时 native-web-search-tool.ts:83 会把客户端 WebSearch 工具整个删掉，DeepSeek 会话完全没有搜索能力，且没有任何日志。跑分不受影响——eval 本来就禁用 web 工具。
关于 @M4n5ter 的评审
很扎实，其中对上游的复现是目前所有人在这件事上产出的最有价值的东西。两条 blocking 发现指向的代码已经不存在了——它比 f531b0f58 删除 open-responses-plaintext-model.ts 晚了 32 分钟。真正仍然成立的是关于 responsesDialect 的设计论点，而 2c3f18620 已经回应了它：关注点被拆成 responsesAdapter 和 responsesReasoningReplay，model-adapter.ts:283 也改为按 runtime.responsesAdapter 判断，不再从 reasoning 回放形状反推 adapter。这个形状是对的，也正是我这边一位评审独立得出的结论。这条没有遗留问题了。
其余 P2 在行内。没有别的阻塞项。
评审协助说明：Claude Code (Opus) 跑了七轮相互隔离的 fresh-eye 审查（运行时正确性、能力退化、测试覆盖、架构边界、上游包实际行为、持久化兼容、依赖与安全），彼此不知道对方的发现。DeepSeek 的 effort 映射表我对照官方文档核实过，@ai-sdk/openai 里 store: false 的过滤逻辑在 dist/index.js:5328-5338 也已确认，另有两条 agent 结论没通过复核，已被我剔除。
|
Upstream update: vercel/ai#18839 has now been fixed by vercel/ai#18844 and closed:
The upstream fix preserves heterogeneous item order, item IDs, reasoning summaries and encrypted content, reasoning-content boundaries, and URL-citation annotations across generated/streamed responses and manual-history replay. These are the generic codec responsibilities that currently account for much of the complexity in this PR.
|
me2seeks
changed the title
|
Thanks for the upstream update — waiting for the release containing vercel/ai#18844 makes sense. I pushed b093de9 with the fixes that are independent of that release:
I also updated the PR description to record the temporary I’ll keep this Draft, upgrade once the release containing #18844 is available, and then remove any safeguards made obsolete by the released codec. Local affected suites are green; GitHub CI is still running. |
|
vercel/ai#18880 All the upstream issues has been fixed and published in the newest version. Most of previous issues could be fixed by introduce the newer version. |
|
Thanks — that matches what I see: What the upgrade lets me remove/simplify:
Tests updated to pin |
|
Warning Review limit reached
Next review available in: 5 minutes Limit details: You’ve used all 3 included reviews currently available under your plan. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
📝 WalkthroughSummaryThis PR fixes DeepSeek V4 reasoning replay during tool loops. DeepSeek plaintext reasoning no longer enters the OpenAI encrypted-reasoning path. The runtime now supports separate encrypted and plaintext Responses replay modes. Genuine encrypted reasoning remains replayable. The PR separates Responses adapter selection from reasoning replay format. OpenAI-compatible providers select The implementation preserves replay chronology and metadata through the updated Open Responses SDK. Historical Reasoning settings resolve once and apply consistently to primary and auxiliary calls. DeepSeek reasoning effort, including Design assessmentThe PR extends the existing provider registry, model runtime, adapter, and replay abstractions. It does not create a parallel runtime path. The Open Responses SDK provides the protocol implementation, while the runtime selects the adapter and replay representation. This is the smallest coherent solution supported by the current evidence. Adapter selection, reasoning continuation, provider-executed tools, and The deleted plaintext transport and removed private request rewriting simplify the implementation. No additional deletion is supported by the current evidence without weakening coverage for ordered replay, item IDs, reasoning metadata, citations, ordinary client tools, hosted-tool fallback, and generic-provider compatibility. Complexity deltaThe PR adds:
The PR removes:
The public type surface and test-maintenance burden increase. Protocol-specific translation and implicit replay behavior decrease. Overall maintenance complexity decreases because the removed private path is replaced by explicit provider configuration and the upstream Open Responses implementation. The remaining complexity is justified by the provider differences. Validation and risksReported validation includes workspace typechecking, runtime, runtime-host, and core suites, request-level checks, formatter checks, and diff checks. The tests cover endpoint normalization, explicit adapter selection, exact No live DeepSeek credential smoke test was run. A known intermittent Review-relevant risks
The person performing the merge reviews the final diff. A maintainer makes the final determination. WalkthroughThe runtime adds Open Responses SDK routing, typed Responses adapter metadata, dialect-aware reasoning replay, normalized endpoints, and DeepSeek fallbacks. Tests cover provider options, tool serialization, reasoning replay, web-search availability, and auxiliary model calls. ChangesResponses adapter contracts and runtime resolution
DeepSeek and validation behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to This change updates reasoning routing and replay behavior across providers. A remaining provider-matrix mismatch could select the wrong replay assumptions for some provider combinations, so the PR is mergeable with explicit owner awareness or follow-up. Sequence Diagram(s)sequenceDiagram
participant ProviderRegistry
participant ModelRuntime
participant ModelFactory
participant AiSdkBackend
participant OpenResponsesSDK
ProviderRegistry->>ModelRuntime: resolve Responses adapter and reasoning contract
ModelRuntime->>ModelFactory: select wire and normalized endpoint
ModelFactory->>OpenResponsesSDK: create Open Responses model
AiSdkBackend->>OpenResponsesSDK: send reasoning and function-tool request
OpenResponsesSDK-->>AiSdkBackend: return reasoning and tool events
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
Caution CodeRabbit couldn't update its existing comment. The review summary may be out of date. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/runtime/src/__tests__/provider-contract-matrix.ts (1)
344-355: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign
wireDimensionCellwith runtime wire resolutionWhen
kind === 'openai-compatible',apiProtocol === 'openai-responses', andsupportsOpenAiResponsesis absent, the runtime selectsopenai-chat, butwireDimensionCellmarksexact-model-idandtool-loopas overrides. RequiresupportsOpenAiResponses === truein this branch. If you extract a shared helper, pass the effective protocol becauseusesOpenAiResponsesWirealso resolves `openAiAdapterApiProtocol(modelId, providerType).Source: Path instructions
🧹 Nitpick comments (5)
packages/core/src/__tests__/model-web-search.test.ts (1)
102-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis assertion now duplicates the earlier test.
Lines 102-105 are identical to lines 8-11. The unique behavior of this test is the protocol-selection path at lines 106-134. Remove the duplicated leading assertion so the test states only what it owns.
As per path instructions: "Flag tests that duplicate existing coverage".
Source: Path instructions
packages/runtime/src/__tests__/ai-sdk-backend.test.ts (1)
3092-3095: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer structured absence checks over whole-prompt substring matching.
JSON.stringify(prompt).includes('tool-call')matches any occurrence anywhere in the serialized prompt, including tool names or text content. The sibling test at Lines 289-300 already filtersmessage.contentbypart.type. Use the same structured form here so the assertion stays stable when unrelated prompt fields change.packages/runtime/src/model-runtime.ts (1)
97-106: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCompute
responsesAdapterContract(adapter)once.The same call runs twice in one return expression. A single local keeps the two consumers in sync by construction.
♻️ Proposed simplification
const wire = resolveModelRuntimeWire(connection.providerType, modelId, adapter, apiProtocol); + const responsesAdapter = wire === 'openai-responses' ? responsesAdapterContract(adapter) : undefined; return { adapter, baseUrl: connection.providerType === 'kimi-coding-plan' && apiProtocol === 'openai-chat' ? kimiOpenAiBaseUrl(resolvedBaseUrl) : resolvedBaseUrl, ...(apiProtocol ? { apiProtocol } : {}), wire, - ...(wire === 'openai-responses' ? { responsesAdapter: responsesAdapterContract(adapter) } : {}), + ...(responsesAdapter ? { responsesAdapter } : {}), reasoningReplay: reasoningReplayContract(adapter, wire), applyPatchProfile: resolveApplyPatchProfile( { wire, - ...(wire === 'openai-responses' - ? { responsesAdapter: responsesAdapterContract(adapter) } - : {}), + ...(responsesAdapter ? { responsesAdapter } : {}), applyPatchProtocol: adapter.applyPatchProtocol, }, modelId, ), };Source: Path instructions
packages/runtime/src/__tests__/model-factory-thinking.test.ts (1)
504-518: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis test cannot distinguish the two namespace helpers.
The chat path uses
openAiCompatibleProviderOptionsKeyand the Responses path usesopenAiCompatibleProviderName. Fordeepseekboth resolve to the same string, so both assertions expect{ deepseek: { reasoningEffort: 'high' } }, which the assertion at Line 210 already pins. The test therefore duplicates coverage instead of protecting the raw-name-versus-camelCase contract the production comment relies on. Either drop the Responses half or use a connection whose slug-derived key differs from its raw name.Source: Path instructions
packages/runtime/src/__tests__/provider-conformance.test.ts (1)
658-671: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe
$schemaassertion pins a dependency detail, not provider behavior.
$schema: 'http://json-schema.org/draft-07/schema#'comes from theaipackage schema conversion, not from the Open Responses wire contract. Anaiorzodupgrade that changes the emitted draft breaks this test without any provider-behavior regression. Assert the fields this test owns (type,name,description, and theparametersshape) and drop$schema.Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e280b3f7-c56d-467d-bb32-a7f69af2ed9b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txtpackages/core/src/__tests__/model-web-search.test.tspackages/core/src/model-web-search.tspackages/core/src/provider-registry.tspackages/runtime-host/src/__tests__/execution-model-composition.test.tspackages/runtime-host/src/server/execution-model-authority.tspackages/runtime/package.jsonpackages/runtime/src/__tests__/ai-sdk-backend.test.tspackages/runtime/src/__tests__/apply-patch-profile.test.tspackages/runtime/src/__tests__/model-adapter.test.tspackages/runtime/src/__tests__/model-factory-thinking.test.tspackages/runtime/src/__tests__/native-web-search-tool.test.tspackages/runtime/src/__tests__/openai-responses-plaintext-reasoning.test.tspackages/runtime/src/__tests__/provider-conformance.test.tspackages/runtime/src/__tests__/provider-contract-matrix.tspackages/runtime/src/__tests__/provider-contract-overrides.tspackages/runtime/src/__tests__/responses-wire-contract.test.tspackages/runtime/src/ai-sdk-backend.tspackages/runtime/src/apply-patch-profile.tspackages/runtime/src/model-adapter.tspackages/runtime/src/model-factory.tspackages/runtime/src/model-runtime.tspackages/runtime/src/openai-responses-plaintext-reasoning-transport.tspackages/runtime/src/provider-urls.tspackages/runtime/src/test-connection.ts
💤 Files with no reviewable changes (1)
- packages/runtime/src/openai-responses-plaintext-reasoning-transport.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
Note on CI: the only failing check is the e2e |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed exact head 5ee70e5d2021df786ca992bc60d762f372d5aad7, including the complete diff, upstream adapter behavior, all nine resolved review threads, and the current CI logs. I found no P0–P3 code issue.
The root fix is at the right boundary. SDK adapter selection and reasoning-continuation representation are separate provider contracts; DeepSeek uses the upstream Open Responses codec for plaintext replay, while OpenAI’s encrypted continuation remains separate. Unsupported ApplyPatch, hosted-tool history, and WebSearch capabilities fail closed at the codec boundary. The local response-rewriting transport is deleted rather than retained as a parallel implementation.
I also independently rechecked the earlier findings. Historical ApplyPatch replay, unsupported thinking levels, empty reasoning, missing auxiliary options, provider-tool orphaning, endpoint normalization, and frozen provider-option handling are fixed on this head. I would not split this cohesive provider-contract change.
The remaining stop is operational rather than a diff finding: CI is still red on streaming-remount.spec.ts waiting for the Stop button. The same locator and line have failed on unrelated PRs, and this diff does not touch that lifecycle, so the evidence points to the known flake; nevertheless, the branch should rerun to green before merge.
The PR also explicitly notes that no live DeepSeek credential smoke was run. Recorded live fixtures and the fake-fetch wire tests are strong, but I would prefer one real V4 tool loop confirming plaintext reasoning replay against /responses, or an explicit maintainer decision to accept that residual validation risk.
Code recommendation: go. Merge recommendation: wait for green CI.
Disclosure: Codex performed the read-only adversarial source, upstream package, test, CI, and feedback-ledger analysis. The human contributor remains responsible for independently verifying the final diff and deciding whether to merge.
中文
代码层面没有 P0–P3，adapter/replay 的职责拆分和 fail-closed fallback 都合理。当前仅因已知 E2E flake 保持合并 STOP，需 rerun green；真实 DeepSeek V4 tool loop 建议补一次，但不是代码 finding。
M4n5ter
left a comment
M4n5ter
left a comment
There was a problem hiding this comment.
English — Request changes
I reviewed exact head 5ee70e5d2021df786ca992bc60d762f372d5aad7, including the full diff, the published SDK behavior, current CI, and all existing review threads.
The adapter/reasoning separation is directionally correct, and the previously reported ApplyPatch, reasoning-option, empty-reasoning, auxiliary-call, and provider-orphan issues are fixed. Two new correctness issues remain:
- P1: the provider-tool fallback is plan-wide and discards unrelated client tool results. I reproduced a mixed history where a completed
Readresult disappeared solely because the same plan contained a historical provider WebSearch. - P2: an endpoint-form custom Responses relay passes connection validation but the native OpenAI adapter sends its model request to
/responses/responses.
The first issue can lose the only record of completed work and should be fixed before merge. The second makes a configuration accepted by the probe unusable at runtime.
The simplification audit also found two non-blocking opportunities: remove the production-unreachable DeepSeek freeform ApplyPatch target path, and replace the duplicated Responses capability fields with a discriminated contract that excludes lossy adapter/replay pairings.
Focused verification on the exact head:
- Responses wire-contract tests: 10/10 passed.
- Existing hosted-tool fallback test: 1/1 passed, but it covers only provider-tool history with grounded text.
- Mixed client/provider history reproduction: client result was dropped.
- Endpoint capture: actual URL was
https://relay.example/v1/responses/responses. - Diff check passed.
- GitHub E2E remains red on the existing streaming-remount Stop-button failure; this diff does not touch that path.
- No live DeepSeek credential smoke was performed.
简体中文 — 请求修改
我审查了精确 head 5ee70e5d2021df786ca992bc60d762f372d5aad7，包括完整 diff、已发布 SDK 的实际行为、当前 CI，以及全部已有 review thread。
adapter 与 reasoning continuation 的职责拆分方向正确；此前报告的 ApplyPatch、reasoning option、空 reasoning、辅助调用和 provider orphan 问题也已修复。但仍有两个新的 correctness 问题：
- P1： provider-tool fallback 是 plan 级的，会连带丢弃无关的客户端工具结果。我已复现：只因同一历史中存在旧的 provider WebSearch，一次已完成的
Read结果就从 prompt 中消失。 - P2： 使用完整 endpoint 的自定义 Responses relay 可以通过连接测试，但原生 OpenAI adapter 会把实际模型请求发送到
/responses/responses。
第一个问题可能丢失已完成工作的唯一记录，应在合并前修复。第二个问题则使 probe 明确认可的配置无法正常运行。
简化审计还发现两个非阻塞机会：删除生产环境不可达的 DeepSeek freeform ApplyPatch 目标路径；将重复的 Responses capability 字段合并为 discriminated contract，从类型层排除有损的 adapter/replay 组合。
针对精确 head 的聚焦验证：
- Responses wire-contract 测试：10/10 通过。
- 现有 hosted-tool fallback 测试：1/1 通过，但只覆盖带 grounded text 的 provider-tool 历史。
- 混合客户端/provider 历史复现：客户端工具结果被丢弃。
- Endpoint 捕获：实际 URL 为
https://relay.example/v1/responses/responses。 - Diff check 通过。
- GitHub E2E 仍因既有 streaming-remount Stop-button 失败而红；本 diff 未修改该路径。
- 未执行带真实 DeepSeek credential 的 smoke test。
|
Both blocking findings are fixed on
Runtime suite green locally (2886 tests, 0 fail). The two simplification opportunities are acknowledged in their threads and tracked as follow-ups so this head stays reviewable. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/runtime/src/__tests__/ai-sdk-backend.test.ts (1)
3202-3208: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRecommended: assert on the prompt structure, not the serialized string.
compactPrompt(model)already returns structured messages. Line 3202 flattens it to JSON, so line 3207 must accept two escape shapes and the checks cannot pin the survivingReadcall and its result to the correct message roles or order. A structural assertion fails for the right reason when replay regresses. Disposition: optional; the current assertions do protect the observable behavior.♻️ Proposed structural assertions
- const wire = JSON.stringify(compactPrompt(model)); - // The unsupported provider-executed pair degrades away… - assert.equal(wire.includes('latest Maka'), false, wire); - // …but the unrelated client Read call and its result survive (`#2972`). - assert.match(wire, /CLIENT_READ_SENTINEL_CONTENT/); - assert.match(wire, /"toolName":"Read"|\\"toolName\\":\\"Read\\"/); - assert.match(wire, /Maka shipped the feature/); + const prompt = compactPrompt(model) as Array<{ + role: string; + content: Array<{ type: string; toolName?: string; text?: string }>; + }>; + const parts = prompt.flatMap((message) => message.content); + // The unsupported provider-executed pair degrades away… + assert.equal( + parts.some((part) => part.toolName === 'WebSearch'), + false, + JSON.stringify(prompt), + ); + // …but the unrelated client Read call and its result survive (`#2972`). + assert.equal( + parts.some((part) => part.type === 'tool-call' && part.toolName === 'Read'), + true, + JSON.stringify(prompt), + ); + assert.equal( + parts.some((part) => part.type === 'tool-result' && part.toolName === 'Read'), + true, + JSON.stringify(prompt), + ); + assert.match(JSON.stringify(prompt), /CLIENT_READ_SENTINEL_CONTENT/); + assert.equal( + parts.some((part) => part.text?.includes('Maka shipped the feature')), + true, + JSON.stringify(prompt), + );Source: Path instructions
packages/runtime/src/provider-urls.ts (1)
53-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate the shared
/responsesnormalization.
openResponsesUrlandopenAiResponsesBaseUrlduplicate the same normalization. Extract it into one private helper and reuse it in both functions. This is an optional maintainability improvement; no correctness issue is present.Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c7c5819-1d19-496a-b1c0-842ac3c6e94d
📒 Files selected for processing (5)
packages/runtime/src/__tests__/ai-sdk-backend.test.tspackages/runtime/src/__tests__/responses-wire-contract.test.tspackages/runtime/src/ai-sdk-backend.tspackages/runtime/src/model-factory.tspackages/runtime/src/provider-urls.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/runtime/src/model-factory.ts
- packages/runtime/src/ai-sdk-backend.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.
@ai-sdk/open-responses@2.0.28 resolves a provider-native reasoningEffort from providerOptions ahead of the cross-provider top-level `reasoning` enum, which cannot express DeepSeek's `max` (its documented mapping sends `xhigh` to high). Key the open-responses providerOptions namespace by the same provider name passed to createOpenResponses so `max` reaches the wire unchanged, and fold the top-level reasoning channel into buildProviderOptions as the single reasoning seam.
…nostic readers - AiSdkBackend stores one resolvedProviderOptions value (caller-supplied or derived from buildProviderOptions) and reads it for the main call, the memory-extraction snapshot, and the request-shape diagnostics, so every reader describes the request actually sent. - runHostAuxiliaryModelCall lets request-carried frozen provider options win over freshly resolved ones, preserving the source turn's reasoning and provider-specific semantics for memory proposal/extraction. - The Open Responses connection probe normalizes through openResponsesUrl so a base URL that already names the endpoint is not probed as /responses/responses, with a provider-conformance regression pinning the single /responses path. - Drop the providerOptions: undefined assertion from the plaintext replay test; assert part count, type, and text instead of coupling to AI SDK normalization.
0695cf3 to
865cd54
Compare
me2seeks
left a comment
me2seeks
left a comment
There was a problem hiding this comment.
Publishing the inline responses for the current implementation and validation.
|
Follow-up on the two findings added to Qodo’s aggregate review for 865cd54:
|
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The latest head now has a coherent provider contract from first principles: each Responses dialect owns its continuation format, provider options are keyed where the selected SDK adapter actually reads them, and unsupported hosted-tool replay is degraded per item instead of discarding valid client-tool history. The endpoint-form URL fix also removes the previous /responses/responses failure without adding a compatibility branch.
I found no reproducible P0-P2 issue on the latest head. The focused checks are green.
Review performed with Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the final claims against the latest head and current main.
中文评论
最新 head 的 provider contract 已形成清晰闭环：每种 Responses 方言管理自己的 continuation 格式，provider options 使用实际 SDK adapter 读取的 namespace，不支持的 hosted-tool replay 按 item 降级，不再误删有效的 client-tool 历史。endpoint-form URL 也修复了此前的 /responses/responses 问题，没有增加兼容分支。
在最新 head 上未发现可复现的 P0-P2 问题，相关检查均已通过。
本次审查使用了 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具；我已依据最新 head 和当前 main 复核最终结论。
|
@M4n5ter The two issues from your changes-requested review were fixed on the current head 95b0f5e: provider-tool replay now degrades per item without dropping unrelated client-tool results, and endpoint-form Responses URLs no longer append a second /responses segment. All review threads are resolved, focused checks are green, and Astro-Han has approved this exact head. GitHub does not allow this fork author to use the reviewer-request API; could you re-review the current head when convenient so the stale aggregate CHANGES_REQUESTED state can be cleared? |
…signature Main's apache#2972 landed a test constructing createHostSessionEffectModel with claudeDeviceId, which this branch removes with the provider that needed it. The merge is otherwise clean; only the field goes. Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
3 participants
Summary
responsesAdapter: 'openai' | 'open-responses'encrypted-contentorplaintext-content@ai-sdk/open-responsesand delete Maka's protocol wrapper and private request rewritingapply_patchwork as a durable fact when the selected codec cannot replay the toolWhy
DeepSeek V4 returns plaintext
reasoning.content[].reasoning_text. The OpenAI-specific Responses codec expects provider-issued IDs or encrypted content for stateless continuation, so DeepSeek reasoning could be visible in the UI but disappear from the next tool-loop request.The runtime now models two independent choices:
model-factorychooses the SDK only from the adapter contract. Durable replay chooses its representation only from the continuation contract.Current capability boundary
@ai-sdk/open-responses@2.0.28includes vercel/ai#18844, so history round trips preserve heterogeneous item order, item IDs, reasoning summaries/encrypted content, reasoning boundaries, and URL citations. The remaining boundaries fail closed:WriteandEdit; the provider's customapply_patchcapability remains declared, but the generic Open Responses codec does not advertise it until it can serialize provider-defined custom toolsapply_patchis converted to a bounded assistant fact instead of being sent as an undeclared tool or silently droppedDeepSeek's declared reasoning effort is passed through verbatim under the provider-native namespace, so
maxis sent literally instead of being clamped by the SDK'sxhighmapping.Verification
maininto the branch and resolved conflicts (provider registry contracts, wire-contract tests, notices, lockfile)npm run typecheckpasses@maka/runtimesuite: 2877 passed@maka/runtime-hostexecution-model composition + protocol: 50 passed@maka/coreprovider-contract matrix / model-web-search / provider-registry: 5 passedAuthorization,high/max, unsupported effort filtering, auxiliary-call reasoning, freeform ApplyPatch downgrade, empty reasoning, and hosted-tool replay fallbackA live DeepSeek credential smoke test has not been run.
Fixes #2513
Follow-up to #2328 and #2518.
中文说明
这个 PR 把两个概念拆开：使用哪个 Responses SDK adapter，以及 reasoning 历史按什么契约继续。DeepSeek V4 使用通用
@ai-sdk/open-responses和 plaintext continuation；OpenAI、xAI、custom relay 继续使用 OpenAI adapter 和 encrypted-content continuation。Maka 自己的 Responses 协议实现和私有请求改写已经删除。
@ai-sdk/open-responses@2.0.28已包含上游 vercel/ai#18844 的无损回放修复。剩余的能力边界显式收缩：DeepSeek 暂时使用Write/Edit，旧apply_patch会保留为事实，模型原生 WebSearch 暂不启用，provider-hosted tool 历史会回退到 grounded text（上游 vercel/ai#18899 跟踪中）。DeepSeek 声明的 reasoning effort 通过 provider-native namespace 原样下发，
max不再被 SDK 的xhigh映射降级。