Skip to content

fix(core): keep tool parameters on the MiniMax chat-completions wire - #11842

Open
yiliang114 wants to merge 9 commits into
mainfrom
fix/issue-11834-minimax-tool-parameters
Open

yiliang114 wants to merge 9 commits into
mainfrom
fix/issue-11834-minimax-tool-parameters

Conversation

@yiliang114

@yiliang114 yiliang114 commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Keeps an explicit empty parameters object on zero-argument tools only after the request has been routed to the MiniMax provider. The shared OpenAI-compatible serializer and every other provider keep their existing behavior.

Why it's needed

#11834 reports that MiniMax rejects requests containing a function declaration without a parameters key with 400 invalid params, function parameters is empty (2013). Qwen Code intentionally omits that key on the shared path because llama.cpp and LM Studio require the omission, so the two contracts must be handled at the provider boundary.

Reviewer Test Plan

How to verify

  • With the official MiniMax OpenAI-compatible endpoint selected, pass a request containing a zero-argument tool and confirm the serialized provider request contains "parameters":{"type":"object","properties":{}}.
  • With a local OpenAI-compatible endpoint and a MiniMax-named model, confirm the default provider remains selected and the parameter omission is unchanged.

Evidence (Before & After)

N/A — request serialization only.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Focused provider tests, core build and core typecheck on macOS.

Risk & Scope

  • Main risk or tradeoff: the explicit empty schema is applied only to requests already classified as MiniMax by the existing hostname-based provider selection.
  • Not validated / out of scope: no live MiniMax API call was made. Aggregating gateways whose hostname does not identify MiniMax remain unchanged and should be handled separately if needed.
  • Breaking changes / migration notes: none.

Linked Issues

Fixes #11834

Related: #10080 and #11410 require the shared OpenAI-compatible path to continue omitting empty tool parameters.

中文说明

这个 PR 做了什么

仅在请求已经被路由到 MiniMax provider 之后,为零参数工具保留显式的空 parameters 对象。共享的 OpenAI-compatible 序列化逻辑和其他所有 provider 都保持现有行为。

为什么需要

#11834 报告 MiniMax 会拒绝包含无 parameters 键的函数声明,并返回 400 invalid params, function parameters is empty (2013)。Qwen Code 的共享路径会有意省略该键,因为 llama.cpp 与 LM Studio 需要这种形态,因此两种不兼容要求应在 provider 边界分别处理。

Reviewer Test Plan

如何验证

  • 选择官方 MiniMax OpenAI-compatible endpoint,传入包含零参数工具的请求,确认 provider 最终序列化的请求包含 "parameters":{"type":"object","properties":{}}
  • 使用本地 OpenAI-compatible endpoint 和带 MiniMax 名称的模型,确认仍选择默认 provider,且参数省略行为不变。

前后对比证据

N/A —— 仅修改请求序列化。

测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

在 macOS 上运行了聚焦 provider 测试、core build 与 core typecheck。

风险与范围

  • 主要风险或取舍:显式空 schema 只作用于已经通过现有 hostname 规则判定为 MiniMax 的请求。
  • 未验证 / 不在范围内:没有调用真实 MiniMax API。hostname 无法识别 MiniMax 的聚合网关保持不变,如确有需要应单独处理。
  • Breaking change / 迁移说明:无。

关联 Issue

Fixes #11834

相关:#10080#11410 要求共享的 OpenAI-compatible 路径继续省略空工具参数。

Since #11431, convertLlmToolsToOpenAI() drops the `parameters` key for any tool
that declares an empty argument list, so JSON.stringify sends a function
declaration without it. `list_agents` is registered unconditionally with
`{type:'object',properties:{},additionalProperties:false}`, which means every
default interactive request carries one. MiniMax rejects that with
`400 invalid params, function parameters is empty (2013)`, so a bare greeting
fails before any tool call exists in the conversation (#11834).

The omission is load-bearing for llama.cpp (#10080) and for strict
OpenAI-contract validators such as LM Studio (#11410), so it stays the default.
Add an opt-out that the MiniMax routing turns on, emitting
`{type:'object',properties:{}}` -- the shape the Anthropic wire already
substitutes for a missing `inputSchema`. The --openai-logging reconstruction
follows the same gate so the logged body matches the body actually sent.

Fixes #11834

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-issue-patrol/jmu12tvxm45
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on 0b41973 and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— 0b41973 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR — and for going back to fix your own regression rather than leaving it for someone else to untangle.

Template looks good ✓

Problem. Observed, not theoretical. #11834 is open, the reporter pasted the exact gateway rejection (400 invalid params, function parameters is empty (2013)) on a bare greeting, and then supplied the /about that pins the routing: MiniMax API Key, https://api.minimaxi.com/v1, MiniMax-M3, 0.23.3, win32. I re-derived the mechanism independently at the base commit and it holds end to end: list_agents carries { type: 'object', properties: {}, additionalProperties: false }, is registered eagerly in the always-registered core block (shouldDefer defaults to false and the tool never passes it), so it satisfies declaresEmptyArgumentList and lands in the parameters = undefined branch — which JSON.stringify then erases. Every default request ships a function with no parameters. A greeting fails before any tool call exists, exactly as reported.

Direction. Aligned. Per-routing wire-shape gating is already how this file handles gateway disagreement — isDashScopeProvider, isOpenRouterHostname and isDeepSeekHostname are all imported into pipeline.ts for precisely this kind of branch, and #11590 set the same precedent for a metadata field an aggregating gateway rejected. Two judgement calls in the description deserve credit rather than scrutiny: correcting the earlier automated triage's DashScope guess (the reporter's own /about says MiniMax, so a DashScope-scoped gate would never have fired for them), and refusing to reverse the default globally, which would re-break #11410 and #10080 for backends that are unidentifiable by hostname. CHANGELOG: no direct reference to this class of gateway rejection, but the area is clearly relevant.

Size. Core paths are touched. 36 production lines (converter.ts 15, pipeline.ts 11, loggingContentGenerator.ts 10) against 149 test lines and no generated/schema lines. Well under the 500-line escalation threshold, so no maintainer-awareness flag on size.

Approach. The scope feels right — an optional trailing options argument defaulting to {}, so every existing caller and every existing pin is untouched, plus the same gate mirrored into the logging reconstruction so the logged body stays faithful to the sent one. That second part is the bit I'd have missed, and it matters here because the logged body is the artifact the issue thread asked the reporter to capture. One question worth sitting with, not a blocker: the hostname predicate is now evaluated at two separate call sites. That is the pragmatic choice today — LoggingContentGenerator holds a ContentGeneratorConfig but no provider instance, so a capability method on the provider interface would need new plumbing to reach the logging path. If a third gateway ever needs the same opt-out, that is the moment to fold it into the provider abstraction instead of adding a third gate.

Risk. Elevated. converter.ts and pipeline.ts both match the high-risk path set from this repo's revert-history analysis (openaiContentGenerator — 10 of 31 reverted PRs touched these paths vs 5 of 60 controls). That does not block anything, but it means full review depth below, CI evidence required before approval, and a named sandboxed lane.

Moving on to code review. 🔍

中文说明

感谢贡献 —— 也感谢你回头修自己引入的回归,而不是留给别人收拾。

模板完整 ✓

问题:已观测,不是理论性的。#11834 仍处于 open,报告者贴出了确切的网关拒绝信息(400 invalid params, function parameters is empty (2013)),只发一句问候就失败;随后补充的 /about 钉住了路由:MiniMax API Key、https://api.minimaxi.com/v1MiniMax-M3、0.23.3、win32。我在 base commit 上独立复推了这条机制,端到端成立:list_agents 的 schema 是 { type: 'object', properties: {}, additionalProperties: false },在「always registered」核心工具块中被即时注册(shouldDefer 默认 false,该工具也没传),因此命中 declaresEmptyArgumentList 走进 parameters = undefined 分支,再被 JSON.stringify 整个抹掉。默认的每一次请求都带有一个没有 parameters 的 function,所以会话里还没有任何 tool call 就报错 —— 与报告完全一致。

方向:对齐。按路由区分 wire 形态本来就是本文件处理网关分歧的既有做法 —— pipeline.ts 已经为此引入了 isDashScopeProviderisOpenRouterHostnameisDeepSeekHostname#11590 也为「聚合网关拒绝某个自动插入字段」立过同样的先例。描述里有两个判断值得肯定而不是质疑:一是纠正了此前自动 triage 对 DashScope 的猜测(报告者自己的 /about 显示是 MiniMax,按 DashScope 收口对他根本不会生效);二是拒绝全局反转默认值,那会让 #11410#10080 对应的、无法用 hostname 识别的后端重新回归。CHANGELOG:没有直接对应的条目,但该领域显然相关。

规模:触及核心路径。生产代码 36 行(converter.ts 15、pipeline.ts 11、loggingContentGenerator.ts 10),测试 149 行,无生成/schema 行。远低于 500 行的升级阈值,因此不因规模触发维护者关注。

方案:范围合理 —— 一个默认 {} 的可选尾参,既有调用点与既有断言全部不动;同时把同一个 gate 镜像进日志重建路径,让日志里的请求体与真正发出的保持一致。第二点是我自己会漏掉的,而在这个 issue 里尤其重要:日志请求体正是 triage 请报告者去抓的东西。有一个值得想一想的问题,但不是阻塞项:hostname 判断现在落在两个调用点上。这在当下是务实选择 —— LoggingContentGenerator 只持有 ContentGeneratorConfig,没有 provider 实例,把能力做成 provider 接口上的方法需要额外管线才能覆盖日志路径。如果将来出现第三个有同样需求的网关,那就是把它收进 provider 抽象、而不是加第三处 gate 的时机。

风险:偏高。converter.tspipeline.ts 都命中了本仓库 revert 历史分析出的高风险路径集合(openaiContentGenerator —— 31 个被 revert 的 PR 中有 10 个触及这些路径,对照组 60 个中只有 5 个)。这不构成阻塞,但意味着下面会做完整深度的 review、批准前必须有 CI 证据,并点名沙箱验证通道。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 0b4197362c521c173e1f882cd88c2df8af4de185 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Code review

I wrote down my own approach before reading the diff: put a capability on the OpenAICompatibleProvider interface — the slot getResponseParsingOptions() already occupies — have MiniMaxOpenAICompatibleProvider override it, and read it in pipeline.ts off this.config.provider, which the pipeline already has in hand. Then I hit the wrinkle that kills it: LoggingContentGenerator holds a ContentGeneratorConfig but no provider instance, so a capability method cannot reach the logging reconstruction without new plumbing. Given that constraint your static hostname predicate is what I would have converged on too, and it matches what pipeline.ts already does for DashScope, OpenRouter and DeepSeek. No simpler path that I can see.

What I verified rather than assumed, all read at the base commit:

  • The default path is provably unchanged. The third parameter defaults to {}, so options.keepParameterlessParameters is undefined at every existing call site and the ternary resolves to parameters = undefined exactly as before. The only two production callers are the ones you patched; code-mode.test.ts and the 234 cases in converter.test.ts use the 2-arg form and cannot be reached by the change.
  • No import cycle. provider/minimax.tsprovider/default.ts, and that module's imports of contentGenerator.js and config/config.js are both type-only; nothing in the chain imports loggingContentGenerator. pipeline.ts was already pulling provider/default.js transitively through isDeepSeekHostname.
  • this.generatorConfig is real — a private readonly constructor property, so the logging gate compiles against the same config object the provider selection used.
  • The test premise holds. get schema() returns { name, description, parametersJsonSchema }, so new ListAgentsTool({} as Config).schema reproduces the declaration the registry actually sends rather than a hand-copied approximation — and asserting on JSON.stringify targets the real defect surface, since an object-level assertion could pass while the wire still drops the key. Keeping the "never carried a schema" case un-synthesised is the right call; it scopes the opt-out to undoing fix(core): omit parameterless OpenAI tool schemas #11431 instead of inventing schemas.

One finding worth your attention — not a blocker, but it sharpens the risk you already flagged.

The shape this PR restores is not the shape MiniMax received before #11431. Tracing list_agents through the base code: convertSchema(_, 'auto') is a no-op ("Default ('auto') mode now does nothing"), and because the omission branch only fires when canValidateLocally is true, relaxSchemaForFunctionCalling always runs here with relaxGrammarConstraints = true. With hasEmptyProperties true it drops the properties key, and with hasNoDeclaredProperties true it drops additionalProperties: false. So the pre-regression wire was:

"parameters": {"type": "object"}

— which is precisely the shape #11410 reports LM Studio rejecting, and which is why omitting the key satisfied both backends at once. Your { type: 'object', properties: {} } is a third shape that qwen-code has never sent MiniMax on this wire.

That cuts both ways, and I think net in your favour. If MiniMax accepted {"type":"object"} before v0.23.2, its validator cannot be demanding a non-empty properties map, and the strictly-more-explicit object you ship should clear it. But that inference rests on a premise nobody has actually stated: the reporter never said it worked on an earlier version, and may be a first-time MiniMax user. If (2013) function parameters is empty means "the parameters object declares no properties" rather than "the key is absent", then neither candidate shape works and the only remaining fixes are a synthetic property or dropping zero-argument tools from the MiniMax wire — a follow-up, not a reason to hold this PR, since today MiniMax fails 100% of the time and the new shape can only improve the odds.

This is exactly the discriminator the triage on #11834 asked @wangvhero for — the model.enableOpenAILogging request body — and it never arrived. The reporter's retest against a build containing this change is the only oracle that settles it, so it is worth asking them to confirm on the issue once it ships rather than closing #11834 on merge alone.

Small factual correction to the description. macOS and Windows are not "left to CI": test_macos and test_windows in .github/workflows/ci.yml are gated on merge_group || schedule || workflow_dispatch and never run on pull_request — both report skipped on this commit. Immaterial to the change (a pure in-memory branch touching no platform API, as you say), but the reporter is on win32 and nobody should wait for a Windows signal that is not coming.

Testing evidence

Unattended CI run — I did not build, run, or execute anything from this PR. Evidence below is the PR's own CI, read through the API for commit 0b41973.

Nothing is red. Three substantive jobs are still in flight, so there is no test result to report yet:

Final CI results for 0b41973 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (ubuntu-latest, Node 22.x) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The 579 passing tests quoted in the description are the author's own local run on Linux — attributed, not independently re-run here, and not a substitute for the Ubuntu job above. The Qwen Triage Finalize job rewrites the table in place once CI settles.

Sandboxed verification would settle part of this: @qwen-code /verify — an A/B against the base build would prove the serializer change is load-bearing, i.e. that a MiniMax baseUrl really does put "parameters":{"type":"object","properties":{}} on the wire and that the new test fails without the diff. It cannot prove MiniMax returns 200; nothing short of the reporter's credentials can, and that gap is a retest request, not a CI job.

中文说明

代码审查

读 diff 之前我先写下了自己的方案:在 OpenAICompatibleProvider 接口上加一个能力位(就是 getResponseParsingOptions() 占的那个位置),由 MiniMaxOpenAICompatibleProvider override,然后在 pipeline.ts 里通过已经持有的 this.config.provider 读取。随后我撞上了让它行不通的那个点:LoggingContentGenerator 只有 ContentGeneratorConfig,没有 provider 实例,所以能力方法到不了日志重建路径,除非额外铺管线。在这个约束下,你选的静态 hostname 判断也正是我会收敛到的写法,而且与 pipeline.ts 里 DashScope / OpenRouter / DeepSeek 已有的做法一致。我没有找到更简单的路径。

以下都是我在 base commit 上实际读过、而不是默认成立的:

  • 默认路径可证明未变。 第三个参数默认 {},因此在所有既有调用点上 options.keepParameterlessParameters 都是 undefined,三元表达式落回 parameters = undefined,与改动前完全一致。生产调用点只有你改的那两个;code-mode.test.tsconverter.test.ts 的 234 个用例都用两参形式,碰不到这个改动。
  • 没有循环依赖。 provider/minimax.tsprovider/default.ts,而后者对 contentGenerator.jsconfig/config.js 的导入都是 type-only;整条链上没有任何模块导入 loggingContentGeneratorpipeline.ts 本来也已通过 isDeepSeekHostname 间接引入了 provider/default.js
  • this.generatorConfig 确实存在 —— 一个 private readonly 构造参数属性,所以日志侧的 gate 能编译,且用的是 provider 选择时的同一个 config 对象。
  • 测试前提是成立的。 get schema() 返回 { name, description, parametersJsonSchema },因此 new ListAgentsTool({} as Config).schema 复现的是注册表真正发出的声明,而不是手抄的近似物;断言打在 JSON.stringify 上,正好对准缺陷本身 —— 只断返回对象可能在 wire 仍然缺键的情况下变绿。「从未带 schema 的声明不合成 parameters」这个用例也是对的,它把 opt-out 限定在撤销 fix(core): omit parameterless OpenAI tool schemas #11431,而不是去发明 schema。

一个值得你注意的发现 —— 不是阻塞项,但它让你已经点出的风险更精确。

本 PR 补回的形态,不是 #11431 之前 MiniMax 收到的那个形态。在 base 代码上跟一遍 list_agentsconvertSchema(_, 'auto') 是空操作("Default ('auto') mode now does nothing");而省略分支只有在 canValidateLocally 为真时才会触发,所以这里 relaxSchemaForFunctionCalling 必然以 relaxGrammarConstraints = true 运行。hasEmptyProperties 为真时它丢掉 properties 键,hasNoDeclaredProperties 为真时它丢掉 additionalProperties: false。所以回归之前的 wire 是:

"parameters": {"type": "object"}

—— 这恰好就是 #11410 里 LM Studio 拒绝的那个形态,也正是「省略整个键」能同时满足两类后端的原因。你选的 { type: 'object', properties: {} } 是第三种形态,qwen-code 从来没有在这条 wire 上发给过 MiniMax。

这一点两面都成立,但我认为净效果对你有利。如果 MiniMax 在 v0.23.2 之前能接受 {"type":"object"},那它的校验器就不可能要求 properties 非空,你发出的这个信息量更足的对象应该能过。但这个推论依赖一个其实没人明确说过的前提:报告者从没说过早先版本是好的,他也可能是第一次用 MiniMax。如果 (2013) function parameters is empty 的含义是「parameters 对象没有声明任何 property」而不是「这个键缺失」,那两个候选形态都不行,剩下的办法只有塞一个合成属性、或者在 MiniMax 这条 wire 上干脆不发零参数工具 —— 那是后续 PR 的事,不构成本 PR 的保留理由,因为现状是 MiniMax 100% 失败,新形态只可能提高成功概率。

这正是 #11834 的 triage 向 @wangvhero 索要的判别信息(model.enableOpenAILogging 的请求体),而它一直没有来。唯一能定论的是报告者用包含本改动的构建重测,所以合并后值得请他在 issue 上确认,而不是仅凭合并就关掉 #11834

描述里一个小的事实更正。 macOS 和 Windows 并不是「交给 CI」:.github/workflows/ci.yml 里的 test_macostest_windows 条件是 merge_group || schedule || workflow_dispatch,在 pull_request 上从不运行 —— 本次 commit 上两者都是 skipped。对本改动没有实质影响(如你所说,只是一个纯内存分支,不碰任何平台 API),但报告者是 win32,不该有人去等一个不会出现的 Windows 信号。

测试证据:本次为无人值守 CI 运行,我没有构建、运行或执行本 PR 的任何代码。下表是针对 commit 0b41973 通过 API 读取的 PR 自身 CI 结果。没有红灯,三个实质任务仍在运行,因此目前还没有测试结果可报。描述中 579 个通过的用例是作者本人在 Linux 上的本地运行 —— 明确标注为作者陈述,此处未独立复跑,也不能替代上面的 Ubuntu 任务。CI 结束后 Qwen Triage Finalize 会就地重写该表格。

沙箱验证可以定论其中一部分:@qwen-code /verify —— 与 base 构建做 A/B,可以证明序列化改动是承重的,即 MiniMax 的 baseUrl 确实会让 wire 上出现 "parameters":{"type":"object","properties":{}},且新测试在没有该 diff 时会失败。它无法证明 MiniMax 返回 200;除报告者的凭据之外没有任何手段能证明,那个缺口是一次重测请求,不是一个 CI 任务。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 0b4197362c521c173e1f882cd88c2df8af4de185 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — the mechanism, the blast radius and the default path are all verified; the only thing nobody can verify from here is which of two literals MiniMax's validator actually wants.

Stepping back: my independent proposal was a provider capability method, and it died on the logging path — you cannot reach a provider instance from LoggingContentGenerator without new plumbing. So the approach here matches what I'd have landed on, and it covers a call site I had not thought about until I read the diff. That is the part that makes me comfortable: the author did not just fix the request, they fixed the reconstruction of the request, because a logged body that disagrees with the sent body is worthless as a diagnostic — and the logged body is exactly what the triage on #11834 asked the reporter to produce. Someone thinking about the next debugging session, not just the next green test.

Everything else is proportionate. A ternary and a hostname gate at two sites, 36 production lines, no drive-by refactors, no formatting churn, nothing unrelated in the diff. The 148 test lines are not padding for a wire-shape regression that already shipped once in v0.23.2 — asserting on JSON.stringify rather than the returned object is the difference between a test that pins this defect and one that waves at it. Six months from now the comments in converter.ts naming all three shapes and their issue numbers are what stops the next person from "simplifying" the branch back into a regression. I would thank whoever wrote that, not curse them.

I re-derived the problem rather than accepting the framing: list_agents really is registered eagerly, its schema really does satisfy declaresEmptyArgumentList, and the omission really does reach the wire on every default request. This is not a hypothesis about a value that could theoretically be passed — it is a hard 400 on the first message for every MiniMax user, four days after the change that caused it shipped.

Two reservations, both stated plainly:

  1. The restored literal is inferred, and it is not the pre-regression shape. Before fix(core): omit parameterless OpenAI tool schemas #11431 the relaxation step had already stripped the empty properties and the additionalProperties: false, so MiniMax was receiving {"type":"object"}. That is probably fine — a validator that accepted {"type":"object"} cannot be demanding a non-empty properties map — but it leans on an assumption nobody has confirmed, since the reporter never said an earlier version worked. If (2013) turns out to mean "declares no properties", this needs a follow-up. Please ask @wangvhero to retest against a build carrying this change and confirm on > 你好 ✕ [API Error: 400 invalid params, function parameters is empty (2013)] > /update ●︎ Qwen Code 0.23.3 已是最新! #11834, rather than letting the merge close the issue on its own.
  2. CI has not landed. Test, Lint & Static and Integration Tests (no-AK, No Sandbox) were still running on 0b41973 at review time, and this PR touches two paths this repo's revert history flags as high-risk, so I am not approving against a result that does not exist yet. Nothing is red; there is simply nothing to read yet. The 579 passing tests in the description are the author's local Linux run, attributed as such.

I am not approving this because I ran out of reasons to say no. Holding a guaranteed-broken P1 regression for an external validator none of us can query would be worse than shipping the canonical shape — the same one both sibling wires already use — with a retest request attached. Approval is deferred until CI lands green on the commit below.

中文说明

置信度:4/5 —— 机制、影响范围与默认路径都已验证;唯一无法从这里验证的,是 MiniMax 的校验器究竟要两个字面量中的哪一个。

退一步看:我自己的方案是在 provider 上加能力位,而它死在日志路径上 —— 不额外铺管线,LoggingContentGenerator 里拿不到 provider 实例。所以本 PR 的做法与我最终会收敛到的方案一致,而且它覆盖了一个我读 diff 之前根本没想到的调用点。这正是让我放心的地方:作者修的不只是请求本身,还有请求的重建 —— 因为一份与真实发送体不一致的日志体作为诊断材料毫无价值,而这个日志体恰恰是 #11834 的 triage 请报告者去抓的东西。这是在为下一次排查考虑,而不只是为下一个变绿的测试。

其余部分都很得体。一个三元表达式加两处 hostname gate,36 行生产代码,没有顺手重构,没有格式化噪音,diff 里没有无关改动。148 行测试不是凑数 —— 一个已经在 v0.23.2 里真实发布过一次的 wire 形态回归,断言打在 JSON.stringify 上而不是返回对象上,正是「钉住这个缺陷」与「朝它挥挥手」的区别。六个月后,converter.ts 里点名三种形态及其 issue 编号的注释,就是阻止下一个人把这个分支「简化」回回归的东西。这样的作者我会感谢,不会暗骂。

我是重新推导了问题,而不是接受它的表述:list_agents 确实是即时注册的,它的 schema 确实满足 declaresEmptyArgumentList,这个省略确实会出现在默认路径的每一次请求里。这不是「某个值理论上可能被传进去」的假设,而是每一个 MiniMax 用户第一条消息就吃到的硬 400,就在引入它的那次改动发布四天后。

两点保留意见,直说:

  1. 补回的字面量是推断出来的,而且不是回归之前的形态。 fix(core): omit parameterless OpenAI tool schemas #11431 之前,relaxation 步骤已经剥掉了空的 propertiesadditionalProperties: false,所以 MiniMax 收到的是 {"type":"object"}。这大概没问题 —— 一个能接受 {"type":"object"} 的校验器不可能要求 properties 非空 —— 但它依赖一个没人确认过的假设,因为报告者从没说过早先版本是好的。如果 (2013) 的实际含义是「没有声明任何 property」,那就还需要后续 PR。请让 @wangvhero 用包含本改动的构建重测,并在 > 你好 ✕ [API Error: 400 invalid params, function parameters is empty (2013)] > /update ●︎ Qwen Code 0.23.3 已是最新! #11834 上确认,而不是让合并动作自己把 issue 关掉。
  2. CI 还没落地。 review 时 TestLint & StaticIntegration Tests (no-AK, No Sandbox)0b41973 上仍在运行,而本 PR 触及了本仓库 revert 历史标记为高风险的两条路径,所以我不会对着一个尚不存在的结果批准。没有红灯,只是还没有可读的结果。描述中 579 个通过的用例是作者本地 Linux 运行,已如实标注。

我不是因为想不出反对理由才批准。为一个我们谁都查询不了的外部校验器,扣住一个必然损坏的 P1 回归,比发出这个规范形态(两条同级 wire 已在用的同一个)并附上一份重测请求要更糟。批准推迟到下面这个 commit 的 CI 全绿之后。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 0b4197362c521c173e1f882cd88c2df8af4de185 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship — CI landed green after the review. ✅

…hint

Aggregating gateways proxy MiniMax backends under their own hostname and
forward the `invalid params, function parameters is empty (2013)`
rejection verbatim, so the hostname-only gate left those routings
broken: verified live against such a gateway, where the hostname-only
build still 400s on a bare greeting while the widened gate (hostname OR
model id containing "minimax") gets a normal response. Measured on the
same gateway: MiniMax-direct model ids reject the parameterless shape
and accept `parameters: {type: object, properties: {}}`, while
DashScope-backed routings accept both, so widening is side-effect free
there; llama.cpp / LM Studio model ids never contain the string and
keep the omission they require (#11834).

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@yiliang114

yiliang114 commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator Author

Scope ledger — round 3, scope corrected after explicit maintainer direction.

Original baseline (0b4197362c52): 5 files, +184/−1. Before this round (fdf880a4ec41): 8 files, +431/−1, including model-name routing for generic gateways and a no-capture logging reconstruction path.

Current head 3d6aa013d344: 2 files, +44/−0 — one MiniMax provider implementation file (+20) and its focused test (+24). R2-1 was resolved by removing the host-blind model-name gate instead of adding locality detection; local llama.cpp / LM Studio routes therefore keep the shared omission required by #10080. R2-2 was resolved structurally: the sole behavior test now exercises the official MiniMax hostname through the provider that owns the wire rewrite. The logging fallback, shared converter option, pipeline gate, gateway-routing helper, and their review-driven tests were removed.

Verification: provider test 10/10; focused ESLint and Prettier clean; core build and core typecheck clean. Full repository build was attempted but stopped in untouched packages/acp-bridge/src/process-registry.ts:744 (string | NonSharedBuffer not assignable to string).

Scope verdict: corrected. Generic gateway detection is an explicit non-goal and can be handled separately if evidence warrants it.

The gate read `contentGeneratorConfig.model`, but requests can carry a
model override (`client.ts` `modelOverride`, set by the core tool
scheduler), so a MiniMax-named config answering a non-MiniMax request
flipped the shape while a MiniMax request under a non-MiniMax config kept
the omission that 400s. Gate on the wire model instead — the same source
the `enable_thinking` gate in pipeline.ts already documents for exactly
this reason: the pipeline passes `context.model`, and the logging mirror
passes `request.model` and falls back to the config model inside the
predicate. Covered by new pins for both directions (#11834).

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@yiliang114

Copy link
Copy Markdown
Collaborator Author

Two design corrections on top of the first commit, both from live measurement rather than review:

1. The gate was too narrow (d22e4cb7b6). Hostname-only left every gateway that proxies MiniMax under its own host broken. Measured against such a gateway (curl, 7 model ids × 2 shapes): the MiniMax-direct ids return 400 for a tool entry without parameters and 200 for parameters: {type:'object',properties:{}}, and the 400 body forwards MiniMax verbatim (detailMessage contains "invalid params, function parameters is empty (2013)"); the bailian/-prefixed ids accept both shapes, so widening is side-effect free there. The gate is now isMiniMaxRouting = hostname match OR wire model id contains minimax. Verified end-to-end with a tmux-captured real v0.23.3 against that gateway: the previous head 400s on a bare 你好, the widened head gets a normal reply.

2. The gate read the wrong model (f4d8dd65a4). It read contentGeneratorConfig.model, but requests can carry modelOverride (client.ts:3239, set by coreToolScheduler.ts:1785), so a MiniMax-named config answering a non-MiniMax request flipped the shape and a MiniMax request under a non-MiniMax config kept the omission. This is the desync pipeline.ts already documents a few hundred lines below for the enable_thinking gate, so the fix follows the established rule: the pipeline passes the wire model (context.model), the logging mirror passes request.model and falls back to the config model inside the predicate. Pins added for both directions.

584 tests pass across the five affected files. Blast radius is unchanged from the original patch — convertLlmToolsToOpenAI has exactly two call sites, both here; isMiniMaxProvider (provider selection, tagged-thinking parsing) is untouched; the default path stays byte-identical, so llama.cpp (#10080) and LM Studio (#11410) keep the shape they require.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

Patrol-Run: qwen-pr-conflict/jmu1aot5f4g
wenshao added a commit to wenshao/qwen-code that referenced this pull request Sep 14, 2026
@wenshao

wenshao commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Local verification against a real bundled CLI — merge reference

I built both arms and ran them end to end against a host that really answers on api.minimaxi.com, rather than at the serializer level. The fix does what it claims: the reporter's line reproduces verbatim on main and is gone at the PR head, non-matching routings stay byte-identical, and the suite now pins the behaviour (it did not at the first head). Two things are worth acting on before merge — the Prettier failure the lane is now red on, and a wrong workaround published on #11834.

Verified at head f4d8dd65a4; the current head 415c5c21a0 only merges origin/main, and the PR's own seven files are byte-identical between the two, so everything below applies unchanged.

Rig

Docker --add-host api.minimaxi.com:127.0.0.1 (plus api.minimax.io, proxy.minimaxi.com, api.deepseek.com, api.minimaxi.com.evil.example) + a self-signed CA trusted through NODE_EXTRA_CA_CERTS, so the CLI's base URL is the literal string from the reporter's /about. A stand-in gateway on 443 records every request body and answers 400 {"error":{"message":"invalid params, function parameters is empty (2013)", …}} only when a function declaration carries no parameters key; otherwise it streams a normal completion. Arms are dist/cli.js from a full npm run build && npm run bundle of e04f2ec5d4 (merge base = released v0.23.3 code) and of the PR head. One gateway process per run, isolated HOME per run.

1. The reported failure, reproduced and fixed

TUI before/after

Real TUI in tmux, typed 你好啊: main answers ✕ [API Error: 400 invalid params, function parameters is empty (2013)] — the reporter's line character for character, vendor code included — and the PR head gets a reply.

2. What changes on the wire, and where

wire and routing

  • 22 tool declarations ship before the user's first word; two of them are parameterless — list_agents and cron_list, which is named nowhere in > 你好 ✕ [API Error: 400 invalid params, function parameters is empty (2013)] > /update ●︎ Qwen Code 0.23.3 已是最新! #11834 or in this PR.
  • End to end with the strict gateway: main fails on api.minimaxi.com; the PR head succeeds. The gateway-proxy case the second commit targets also works: api.deepseek.com + model MiniMax-M3 fails on main, succeeds at head.
  • The widened gate fires on /minimax/i in the wire model for any host, including a local llama.cpp / LM Studio endpoint serving a MiniMax GGUF — the two backends fix(core): omit parameterless OpenAI tool schemas #11431 introduced the omission for. I measured that case against a real llama-server b10621 (--jinja --alias MiniMax-M2-GGUF): both arms answer 200, so that engine tolerates the restored shape (cost: +18 prompt tokens by llama.cpp's own count, 20 371 → 20 389). Worth a line in the risk section, not a blocker.
  • The three unmatched rows are byte-identical between arms after normalising session id / timestamps / cwd; the whole 22-tool array hashes to 6e239f1f1bc5 on both, and only matched routings move to 209b979d306f. The look-alike host api.minimaxi.com.evil.example is correctly not treated as MiniMax.

3. Does the suite hold the fix down?

mutation matrix and checks

Six of seven mutants die. The one that matters: on the first head (0b4197362c) hard-wiring the pipeline gate to false — i.e. restoring the shipped defect with an untouched signature — left the whole core suite green in the changed area — all 26 461 cases re-run, the only reds being this machine's timing-flaky files that fail on the merge base too — because the only pin was an argument-shape assertion holding that same literal false. The two follow-up commits added behavioural pipeline cases and closed that hole. Only the logging mirror stays unpinned (N5).

Also in that figure, and the reason to touch the branch once more:

prettier --check fails on packages/core/src/core/openaiContentGenerator/provider/minimax.test.ts — the createRoutedConfig('llama-3.1-8b-instruct', 'http://127.0.0.1:8080/v1') call added by d22e4cb7b6 is over 80 columns. I found this locally; CI has since confirmed it at the current head. At f4d8dd65a4 the lane died early at Check lint gate freshness (.github/workflows/ci.yml moved on main via #11787 after the branch point) and never reached the linters; your 415c5c21a0 merge cleared that gate, and the lane now fails at step 26 Run Prettier:

Running Prettier...
[warn] packages/core/src/core/openaiContentGenerator/provider/minimax.test.ts
[warn] Code style issues found in 1 file. Run Prettier with --write to fix.

One prettier --write on that file fixes it — it is the only thing standing between this branch and a green lane.

4. Two corrections to claims made around the fix

two corrections

  1. The published workaround does not work. Both the triage comment and your reply on > 你好 ✕ [API Error: 400 invalid params, function parameters is empty (2013)] > /update ●︎ Qwen Code 0.23.3 已是最新! #11834 tell the reporter to exclude list_agents. Measured on main: with "tools": {"exclude": ["list_agents"]} the request still carries cron_list without parameters and still fails with the same 400. Only ["list_agents", "cron_list"] gets through. Worth a follow-up on the issue, since the reporter may be trying it right now.
  2. Hunk 2 cannot change what it is said to change. "Without it, the logged body would diverge from the body actually sent" does not hold for this path: startCaptureSession() resolves captured ?? buildOpenAIRequestForLogging(req), and pipeline.ts:1510 publishes the real request on every attempt, so the patched reconstruction is only a fallback for generators that do not capture. Measured with enableOpenAILogging: logged tools == wire tools on both arms, and still equal on a bundle rebuilt without hunk 2. The hunk is harmless and keeps that fallback consistent — the reason given for it is just not the operative one.

Checks

check result
5 test files named in the PR body, at the PR head 584 / 584
same 5 files with the PR merged onto current main 584 / 584
the new test file against unmodified main 2 failed / 6 — the claimed red, exactly
whole packages/core suite at the PR head 26 424 passed / 26 466; 5 failures in 4 timing-sensitive files. client.test.ts and exit-worktree.test.ts fail identically on the merge base (the latter 2/2 when run alone there); code-mode and recall-scan-latency are CPU-budget/latency cases that vary run to run. None is reachable from a diff that only touches OpenAI chat-completions tool serialisation
tsc --noEmit (core), PR head and merged tree clean
eslint over both changed directories clean
prettier --check fails — see above

Residuals (all consistent with your own risk section)

  • The real MiniMax endpoint was never contacted. My gateway is a stand-in built from the reporter's error string; whether the live API accepts {"type":"object","properties":{}} is still inferred, not measured. I attempted a credential-free probe of the vendor endpoint to settle it and my sandbox blocked the request, so I cannot independently confirm your curl measurements against a proxying gateway.
  • A gateway that proxies MiniMax under a model alias not containing "minimax" stays broken — measured: PR head, api.deepseek.com, model deepseek-chat → still 400 from the strict gateway. Unfixable by hostname or model id; correctly out of scope.
  • Tiny asymmetry: the pipeline resolves the wire model with request.model || config.model while the predicate uses wireModel ?? config.model, so an empty-string request.model would gate differently in the mirror than on the wire. Cosmetic.

Machine: macOS 26.6.2 (arm64), Node v24.18.1, containers on node:22. Every figure is a screenshot of data produced by the runs described above.

中文说明

本地真实环境验证 —— 供合并参考

我没有停在序列化层,而是把两条臂都打成 dist/cli.js,在一个真的会在 api.minimaxi.com 上应答的主机上端到端跑通。结论:这个修复确实做到了它声称的事 —— 报告者那行错误在 main 上逐字复现、在 PR 头上消失;未命中的路由逐字节不变;测试也终于把行为钉住了(第一个 head 上并没有)。合并前有两件事值得处理:lane 现在正红着的那个 Prettier 失败,以及 #11834 上发布的一个不成立的绕过方案。

验证基于 head f4d8dd65a4;当前 head 415c5c21a0 只是合入了 origin/main,PR 自身那 7 个文件在两者之间逐字节相同,所以下面全部结论照旧成立。

装置

Docker --add-host api.minimaxi.com:127.0.0.1(另加 api.minimax.ioproxy.minimaxi.comapi.deepseek.comapi.minimaxi.com.evil.example)+ 自签 CA 经 NODE_EXTRA_CA_CERTS 信任,于是 CLI 的 base URL 就是报告者 /about 里的那个字面量。443 上的替身网关记录全部请求体,只有当某个 function 声明缺 parameters 键时才回 400 …invalid params, function parameters is empty (2013),否则正常流式应答。两条臂分别是 e04f2ec5d4(merge base,等同已发布的 v0.23.3 代码)与 PR head 的完整 npm run build && npm run bundle 产物;每次运行一个独立网关进程、独立 HOME

1. 报告的故障:复现并修复

tmux 里的真 TUI,输入 你好啊main 打出 ✕ [API Error: 400 invalid params, function parameters is empty (2013)],与报告者贴的那行逐字一致(含厂商码),PR head 正常拿到回复。见图 1。

2. wire 上变了什么、在哪里变

  • 用户还没说第一个字,就已经发出 22 个工具声明;其中两个是零参数的 —— list_agents cron_list,后者在 > 你好 ✕ [API Error: 400 invalid params, function parameters is empty (2013)] > /update ●︎ Qwen Code 0.23.3 已是最新! #11834 和本 PR 里从未被提及。
  • 严格网关下端到端:mainapi.minimaxi.com 上失败,PR head 成功;第二个提交针对的"网关代理"场景也成立:api.deepseek.com + 模型 MiniMax-M3main 上失败、在 head 上成功。
  • 放宽后的 gate 会对任意主机/minimax/i 命中的 wire 模型触发,包括本地 llama.cpp / LM Studio 跑 MiniMax GGUF —— 正是 fix(core): omit parameterless OpenAI tool schemas #11431 当初为之引入省略的那两类后端。我对着真的 llama-server b10621(--jinja --alias MiniMax-M2-GGUF)实测:两条臂都是 200,该引擎能接受补回来的形态(代价:按 llama.cpp 自己的计数多 18 个 prompt token,20 371 → 20 389)。建议在风险一节补一句,不构成阻塞。
  • 三个未命中的行在归一化 session id / 时间戳 / cwd 后逐字节相同:22 个工具的数组两臂都哈希到 6e239f1f1bc5,只有命中路由变成 209b979d306f。仿冒域名 api.minimaxi.com.evil.example 正确地没有被当成 MiniMax。

3. 测试能不能把修复钉住

7 个变异体杀 6。关键的一个:在第一个 head(0b4197362c)上,把 pipeline 的 gate 硬写成 false —— 等于原样把线上缺陷放回去、签名一个字不动 —— 整套 core 跑完、改动范围内一个用例都没红(26 461 个用例全部重跑,唯一的红是本机那几个时间敏感文件,它们在 merge base 上同样红),因为唯一的钉子是一条断言调用参数、且期望值恰好就是那个字面量 false。两个后续提交补上了行为级 pipeline 用例,这个洞已经补上。只剩日志镜像没有被钉住(N5)。

同一张图里还有必须再动一次分支的原因:

prettier --check 失败,文件是 packages/core/src/core/openaiContentGenerator/provider/minimax.test.ts —— d22e4cb7b6 新增的 createRoutedConfig('llama-3.1-8b-instruct', 'http://127.0.0.1:8080/v1') 超过 80 列。我先在本地发现,CI 随后已在当前 head 上证实:在 f4d8dd65a4 上 lane 更早就死在 Check lint gate freshness(分支点之后 .github/workflows/ci.yml#11787 在 main 上动过),根本没跑到各个 linter;你推的 415c5c21a0 合并解掉了那道门,于是 lane 现在失败在第 26 步 Run Prettier

Running Prettier...
[warn] packages/core/src/core/openaiContentGenerator/provider/minimax.test.ts
[warn] Code style issues found in 1 file. Run Prettier with --write to fix.

对该文件跑一次 prettier --write 即可 —— 这是这条分支变绿前唯一挡着的东西。

4. 两处需要更正的说法

  1. 发布出去的绕过方案不成立。 triage 评论和你在 > 你好 ✕ [API Error: 400 invalid params, function parameters is empty (2013)] > /update ●︎ Qwen Code 0.23.3 已是最新! #11834 的回复都让报告者排除 list_agents。在 main 上实测:"tools": {"exclude": ["list_agents"]} 时请求里仍带着没有 parameterscron_list,照样同样的 400;必须 ["list_agents", "cron_list"] 才能过。建议到 issue 下补一条 —— 报告者此刻很可能正在试。
  2. Hunk 2 改不了它被说成要改的东西。 "没有它日志里的请求体就会和真正发出的不一致"在这条路径上不成立:startCaptureSession() 解析的是 captured ?? buildOpenAIRequestForLogging(req),而 pipeline.ts:1510 在每次尝试时都会把真实请求发布出去,所以被改的那段重建只是"没人捕获时"的兜底。实测(开 enableOpenAILogging):两条臂的日志工具数组与 wire 逐字节相同,去掉 hunk 2 重新打包后依然相同。这段代码无害、也让兜底保持一致,只是它被给出的理由不是真正起作用的那条。

检查项

检查 结果
PR 正文点名的 5 个测试文件(PR head) 584 / 584
同样 5 个文件,PR 合到当前 main 之后 584 / 584
新测试文件对未改动的 main 2 failed / 6,与正文所述完全一致
packages/core 全量套件(PR head) 26 424 通过 / 26 466;5 个失败分布在 4 个时间敏感文件。client.test.tsexit-worktree.test.ts 在 merge base 上同样失败(后者单跑 2/2 全红);code-moderecall-scan-latency 是 CPU 预算 / 时延类用例,逐次运行结果不稳定。本 diff 只改 OpenAI chat-completions 的工具序列化,这四个文件都不可达
tsc --noEmit(core),PR head 与合并后的树 干净
eslint 覆盖两个改动目录 干净
prettier --check 失败,见上

残留(与你自己的风险一节一致)

  • 我始终没有连过真正的 MiniMax 端点。 替身网关是按报告者的错误串造的;线上 API 是否接受 {"type":"object","properties":{}} 仍然是推断而非实测。我试过用一次不带凭据的探测去坐实它,被本机沙箱策略拦下,因此也无法独立复核你对代理网关做的 curl 测量。
  • 不含 "minimax" 的模型别名代理 MiniMax 的网关仍然坏着 —— 实测:PR head + api.deepseek.com + 模型 deepseek-chat 依旧被严格网关 400。靠主机名或模型 id 都救不了,划出范围是对的。
  • 一处极小的不对称:pipeline 用 request.model || config.model 取 wire 模型,而断言函数用的是 wireModel ?? config.model,于是 request.model 为空字符串时,镜像与 wire 的判定会不一致。属于外观问题。

机器:macOS 26.6.2(arm64)、Node v24.18.1,容器用 node:22。所有图都是上述运行产出数据的截图。

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@yiliang114

Copy link
Copy Markdown
Collaborator Author

All three points acted on:

  1. Prettier — fixed at head c8893183 (prettier --write on minimax.test.ts; --check now clean across all files in the diff). CI is re-running on the new head.
  2. Workaround correction — posted on > 你好 ✕ [API Error: 400 invalid params, function parameters is empty (2013)] > /update ●︎ Qwen Code 0.23.3 已是最新! #11834: excluding only list_agents is not sufficient, cron_list trips the same 400; effective exclude is ["list_agents", "cron_list"] until this ships. > 你好 ✕ [API Error: 400 invalid params, function parameters is empty (2013)] > /update ●︎ Qwen Code 0.23.3 已是最新! #11834 (comment)
  3. Risk section — added a line covering local llama.cpp / LM Studio serving a MiniMax GGUF coming back under the widened gate, with your llama-server b10621 measurement (both arms 200, +18 prompt tokens).

On hunk 2: agreed — the operative reason is keeping the no-capture fallback consistent, not the log/wire divergence I claimed. Keeping the hunk as-is.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.

Test Plan (not a blocker): src/core/openaiContentGenerator/converter-parameterless-parameters.test.tsno such file or directory; src/core/openaiContentGenerator/converter.test.tsno such file or directory; src/core/openaiContentGenerator/pipeline.test.tsno such file or directory; src/core/openaiContentGenerator/provider/minimax.test.tsno such file or directory; src/core/loggingContentGenerator/loggingContentGenerator.test.tsno such file or directory; and 1 more.

中文说明

已审查——无阻断问题。 建议见行内评论。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。

Test Plan(非阻断):src/core/openaiContentGenerator/converter-parameterless-parameters.test.tsno such file or directory; src/core/openaiContentGenerator/converter.test.tsno such file or directory; src/core/openaiContentGenerator/pipeline.test.tsno such file or directory; src/core/openaiContentGenerator/provider/minimax.test.tsno such file or directory; src/core/loggingContentGenerator/loggingContentGenerator.test.tsno such file or directory; and 1 more。

— qwen3.8-max via Qwen Code /review (v0.23.3)

Comment thread packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts Outdated
wenshao added a commit to wenshao/qwen-code that referenced this pull request Sep 14, 2026
@wenshao

wenshao commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Re-verified at c889318356 — everything re-run, still good

Re-ran the whole rig on the new head. Nothing regressed, and the one CI blocker I reported is gone. One item from my report is still open, and it is the same one the review bot raised as R1-1.

What actually moved

what moved

The branch gained exactly one commit since the head I reported on — style(core): format minimax provider test, the Prettier fix. The four production files are byte-identical to what I verified, so every behavioural result carries over; I re-measured them anyway.

One thing worth recording because it looks alarming and is not: rebuilding dist/ at the two heads produces a different name for every chunk. That is not the test file leaking into the bundle — grep -rl "llama-3.1-8b-instruct" dist/ finds nothing. The build embeds the commit sha (c889318356 is in dist/chunks/chunk-TZK5FCHL.js, f4d8dd65a4 is nowhere), and that shared string cascades a new content hash into every chunk. Two rebuilds at the same commit reproduce byte for byte.

Re-run on the new head

check result
end-to-end, strict gateway, api.minimaxi.com + MiniMax-M3 main exit 1 with the reporter's 400 … (2013); PR head exit 0, model answered
gateway-proxy case (api.deepseek.com + MiniMax-M3) main exit 1, PR head exit 0
host × model matrix, 14 runs of the real bundle unchanged — 4 routings keep {}, the other 3 stay byte-identical (6e239f1f1bc5209b979d306f only where the gate fires)
5 test files named in the PR body 584 / 584
whole packages/core suite 26 429 passed / 26 467 — one failure, client.test.ts microcompaction, which fails identically on the merge base
PR merged with today's main (10 commits ahead, no conflicts) tsc --noEmit clean · 584 / 584 · prettier --check clean
eslint --max-warnings 0, both changed directories clean
7-mutant matrix same verdicts — 6 killed, only N5 survives
CI every lane green, including Lint & Static (15m50s)

TUI at the new head

Closed since my last report

  • The Prettier failure is fixed. node scripts/lint.js --prettier is clean repo-wide, and CI's lane went from red to pass.
  • The exclude workaround on > 你好 ✕ [API Error: 400 invalid params, function parameters is empty (2013)] > /update ●︎ Qwen Code 0.23.3 已是最新! #11834 is corrected — the advice there is now ["list_agents", "cron_list"]. Thanks for turning that around quickly; that was the one thing actively misleading a user in the field.
  • The risk section now covers the local-engine case (line 96 of the body), with the llama-server b10621 numbers. Accurate as written.
  • Cosmetic leftover: the body still names only list_agents as the always-registered parameterless tool — two ship on every request.

Still open — and it is R1-1

open item

Mutant N5 (gate the logging mirror on generatorConfig.model instead of request.model) still passes all 584 cases at this head, which is exactly what the bot's unresolved R1-1 thread says: the logging hunk is the one production change nothing pins.

One clarification I can add from measurement, since R1-1's rationale rests on the fallback being reachable: for a MiniMax chat-completions session it is not. The only generators that emit chat completions are OpenAIContentGenerator and its Qwen subclass, both run through ContentGenerationPipeline, and both logging entry points (generateContent :423 and generateContentStream :593) wrap the call in the capture context — so captured is always set and the reconstruction never runs. Measured: logged tools == wire tools on both arms, and still equal on a bundle rebuilt with the hunk deleted. The reconstruction is reached only if buildRequest() throws before pipeline.ts:1510, or for a generator that is not chat completions (Anthropic / Gemini / /responses). That does not make R1-1 wrong — it is still the one unpinned production change, and the suggested pair of argument assertions is cheap — it just narrows what a reader should expect the hunk to protect.

Verdict from this side: the fix is measured, the wire change is confined to the routings it is supposed to touch, non-matching routings are byte-identical, and CI is green. The only judgement call left is whether R1-1's test is required before merge.

中文说明

c889318356 上重新验证 —— 全部重跑,结论不变

整套装置在新 head 上重跑了一遍。没有任何回归,我上次报告里那个卡 CI 的问题也已经消失。 报告里还剩一项没关,而它正是评审机器人提的 R1-1。

真正变了什么

自我上次报告的那个 head 以来,分支只多了一个提交 —— style(core): format minimax provider test,就是那个 Prettier 修复。四个生产文件与我验证过的版本逐字节相同,所以行为结论原样成立;我仍然把它们全部重测了一遍。

有一点看着吓人但其实无害,值得记一笔:在两个 head 上重新构建 dist/每个 chunk 的文件名都不一样。这不是测试文件混进了包 —— grep -rl "llama-3.1-8b-instruct" dist/ 什么也搜不到。原因是构建把 commit sha 嵌进了产物(c889318356 出现在 dist/chunks/chunk-TZK5FCHL.jsf4d8dd65a4 哪里都没有),这个共享字符串会让每个 chunk 的内容哈希连锁改变。同一个 commit 上连续构建两次,产物逐字节一致。

新 head 上的重跑结果

检查 结果
端到端,严格网关,api.minimaxi.com + MiniMax-M3 main exit 1,报告者的 400 … (2013);PR head exit 0,模型正常应答
网关代理场景(api.deepseek.com + MiniMax-M3 main exit 1,PR head exit 0
主机 × 模型矩阵,真包跑 14 次 与上次一致 —— 4 条路由保留 {},另外 3 条逐字节不变(只有命中 gate 时 6e239f1f1bc5209b979d306f
PR 正文点名的 5 个测试文件 584 / 584
packages/core 全量套件 26 429 通过 / 26 467 —— 仅 1 个失败 client.test.ts microcompaction,它在 merge base 上同样失败
PR 合到今天的 main(领先 10 个提交,无冲突) tsc --noEmit 干净 · 584 / 584 · prettier --check 干净
eslint --max-warnings 0,两个改动目录 干净
7 个变异体矩阵 判定不变 —— 杀 6,只有 N5 存活
CI 全部 lane 绿,包括 Lint & Static(15m50s)

上次报告里已关掉的

仍未关闭 —— 就是 R1-1

变异体 N5(把日志镜像的判定改成 generatorConfig.model 而不是 request.model)在这个 head 上依旧跑过全部 584 个用例,这正是机器人那条未解决的 R1-1 所说的:日志那一段是本 PR 里唯一没有任何测试钉住的生产改动。

由于 R1-1 的论证依赖"兜底路径可达",我可以补一条实测澄清:对 MiniMax 的 chat-completions 会话来说它并不可达。会发出 chat completions 的生成器只有 OpenAIContentGenerator 及其 Qwen 子类,两者都走 ContentGenerationPipeline,而日志的两个入口(generateContent :423 与 generateContentStream :593)都把调用包在捕获上下文里 —— 所以 captured 必定有值,重建分支永远不会执行。实测:两条臂的日志工具数组与 wire 逐字节相同,把这一段整体删掉重新打包后仍然相同。只有当 buildRequest()pipeline.ts:1510 之前抛错,或者生成器根本不是 chat completions(Anthropic / Gemini / /responses)时,才会走到重建。这并不否定 R1-1 —— 它依然是唯一没被钉住的生产改动,而且建议补的那对参数断言成本极低 —— 只是把"这段代码到底在保护什么"说得更准确一些。

我这边的结论: 修复有实测支撑,wire 的变化被限制在该动的路由上,未命中的路由逐字节不变,CI 全绿。剩下唯一需要判断的,是 R1-1 那条测试是否作为合并前置条件。

Reverting the MiniMax opt-out in the OpenAI request logger left the suite green, so both mutations survived: dropping the options object, and reading the configured model instead of the wire model. One case with two calls pins both directions, and each revert now fails it.
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

Picks up main's review-runner-schedule helper-test fix (#11933), which writes a commonjs package.json into the fake-gh temp dir so the Lint lane's helper tests stop failing with 'require is not defined in ES module scope' when TMPDIR resolves inside the repo.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

Patrol-Run: qwen-pr-conflict/jmu2luc9n6d

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified the wire change and its blast radius at the PR head. No issues found.

  • Default path is byte-identical. options defaults to {}, so keepParameterlessParameters is falsy everywhere except the MiniMax gate, and the parameters = undefined drop from #11431 is preserved for every other provider — the two pre-existing converter.test.ts pins for llama.cpp/LM Studio are unedited, and the opt-in literal ({type:'object',properties:{}}) is the same shape the Anthropic wire substitutes for a missing inputSchema.
  • Exactly two production call sites, both updated. git grep convertLlmToolsToOpenAI finds pipeline.ts:1035 (the only chat-completions wire builder) and loggingContentGenerator.ts:1046 (the log-reconstruction path) — both pass the flag, so logged bodies match sent bodies. There is no third serializer.
  • The gate reads the right model. pipeline.ts passes context.model, which createRequestContext sets to request.model || contentGeneratorConfig.model — the true wire model, so a request-level override that points at MiniMax is honoured; the hostname branch and the /minimax/i model-id fallback in isMiniMaxRouting are both safe on a missing/invalid baseUrl (try/catch inside isMiniMaxProvider).
  • The absent-tools case is unchanged. The tools.length > 0 guard still skips an empty tools: [], and nothing downstream mutates baseRequest.tools after conversion.
  • The /minimax/i model-id match also catching an llama.cpp/LM-Studio server that happens to serve a MiniMax-named model is disclosed and measured under Risk & Scope, and the restored literal's additionalProperties: false drop is disclosed as a static inference — both read as accepted scope rather than defects.

qqqys
qqqys previously approved these changes Sep 15, 2026

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

APPROVE

核对基线:head fdf880a4ec41(8 个文件,+431/-1,最后提交 12:03:54Z)。

历史阻塞问题:无

本 PR 历史上没有出现过 CHANGES_REQUESTED:三次 review 分别是 DISMISSED(占位)、COMMENTED(2026-09-14T18:14:09Z)与 doudouOUC 在当前 head 上的 COMMENTED(2026-09-15T12:18:32Z)。1 条 review thread 且已 isResolved: true,Critical 级未解决数 0。

本轮独立扫描:未发现 Critical

生产改动共 4 个文件 +63 行,修的是 #11834:MiniMax 对不带 parameters 的零参数工具返回 400 invalid params, function parameters is empty (2013),而始终注册的 list_agents 正是这种工具。这类改动最大的风险是「为了一个 provider 把其他 provider 弄坏」,我按这条主线核对:

  1. 默认行为逐字未变,两个既有修复没有被回退。 convertLlmToolsToOpenAI 新增的第三参是 options: { keepParameterlessParameters?: boolean } = {},分支写成
    parameters = options.keepParameterlessParameters
      ? { type: 'object', properties: {} }
      : undefined;
    不传 option 时 undefined 为 falsy,走 parameters = undefined,与改动前完全一致。因此 llama.cpp 无法在空 properties 上编译 grammar(#10080)与 LM Studio 等严格校验器拒绝无 propertiesparameters#11410)这两条修复都保持有效——新行为是显式 opt-in,不是改默认值。代码注释把这三个 issue 的相互制约关系写得很清楚,这一点做得好。
  2. 门禁的判定来源选对了。 两个调用点都传「线上实际使用的模型」而非配置模型:pipeline.tscontext.modelloggingContentGenerator.tsrequest.model,注释说明理由——请求级 model override 决定由哪个后端应答,与同文件里 enable_thinking 门禁用的是同一个来源。这也保证了日志侧与实际发送侧用同一个谓词,调试日志里的 body 就是真正发出去的 body,不会出现「日志显示省略、实际发送保留」的偏差。
  3. isMiniMaxRouting 的误判面足够窄。 provider/minimax.ts:52-60
    return MiniMaxOpenAICompatibleProvider.isMiniMaxProvider(config)
      || /minimax/i.test(wireModel ?? config.model ?? '');
    主机侧沿用既有的 isMiniMaxProvider(精确匹配 api.minimaxi.com / api.minimax.io,外加 .minimaxi.com / .minimax.io 后缀,且该后缀宽松的取舍与其风险在 :14-23 已有注释说明,本 PR 未改动它);模型侧是对 wire model 做大小写不敏感的 minimax 子串匹配,用于覆盖「聚合网关用自己的域名代理 MiniMax 后端、只把 2013 拒绝原样转发」这种主机名毫无线索的情形。误判需要「模型 id 含 minimax 但后端不是 MiniMax」,而这种组合恰好就是网关代理 MiniMax 的常见命名;反过来,真正会被 {type:'object',properties:{}} 拒绝的 llama.cpp / LM Studio 是本地服务,其模型 id 不会含 minimax。?? '' 的兜底也让 model 缺失时安全落到不匹配,不会抛错。
  4. 改动没有越过 converter 的契约。 返回类型仍是 Promise<OpenAI.Chat.ChatCompletionTool[]>,替换出的 { type: 'object', properties: {} } 正是 Anthropic wire 在缺少 inputSchema 时使用的形状(注释已点明),属于合法 JSON Schema,不引入新的类型分支。

测试侧新增 4 个文件共 +368 行,其中 converter-parameterless-parameters.test.ts(+148)是为本行为专设的;当前 head 的 Lint & StaticTest (ubuntu-latest, Node 22.x)Integration Tests (no-AK, No Sandbox) 全部 pass,说明这些用例确实通过。

CI:上述三项 pass,review-pr pending,按策略不作为门禁,我没有等待或轮询;无失败项。

结论:无历史阻塞问题,本轮未发现可证明的 Critical,提交 APPROVE。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not explored to full depth (tool budget reached): "agent 1e": none — no check was cut short.; "agent 6c": I did not read baseLlmClient.ts:558 / client.ts:5042 to confirm whether request.model can actually be '' in production, which is the reachability premis….

Test Plan (not a blocker): src/core/openaiContentGenerator/converter-parameterless-parameters.test.tsno such file or directory; src/core/openaiContentGenerator/converter.test.tsno such file or directory; src/core/openaiContentGenerator/pipeline.test.tsno such file or directory; src/core/openaiContentGenerator/provider/minimax.test.tsno such file or directory; src/core/loggingContentGenerator/loggingContentGenerator.test.tsno such file or directory; and 1 more.

Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:

  • packages/core/src/core/openaiContentGenerator/provider/minimax.ts:57 — [review] host-blind model-id leg also changes the first-party Alibaba Coding Plan / Token Plan routing (MiniMax-M2.5 on coding.dashscope.aliyuncs.com), which the PR body…
中文说明

未探索到全部深度(达到工具调用预算):"agent 1e"none — no check was cut short."agent 6c"I did not read baseLlmClient.ts:558 / client.ts:5042 to confirm whether request.model can actually be '' in production, which is the reachability premis…

Test Plan(非阻断):src/core/openaiContentGenerator/converter-parameterless-parameters.test.tsno such file or directory; src/core/openaiContentGenerator/converter.test.tsno such file or directory; src/core/openaiContentGenerator/pipeline.test.tsno such file or directory; src/core/openaiContentGenerator/provider/minimax.test.tsno such file or directory; src/core/loggingContentGenerator/loggingContentGenerator.test.tsno such file or directory; and 1 more。

收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.23.3)

Comment thread packages/core/src/core/openaiContentGenerator/provider/minimax.ts Outdated
Comment thread packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts Outdated

@qwen-code-review-bot qwen-code-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

COMMENT

Independent pass over head fdf880a4ec41 (+431/-1, CI all green). I reviewed the two open threads from the round-2 review rather than duplicating them — both reproduce, and I would not approve while R2-1 stands.

R2-1 (Critical) reproduces from first principles. The gate is isMiniMaxProvider(config) || /minimax/i.test(wireModel), and the model-id leg carries no host condition. A self-hosted MiniMax deployment — llama.cpp serving MiniMax-M2-GGUF, where the served model id derives from the filename or --alias, so containing "minimax" is the normal spelling — is exactly the deployment #10080 stripped the empty parameters key for: llama.cpp cannot compile a grammar over an empty properties map. This PR switches that deployment back onto the empty-object shape, i.e. it fixes a 400 from the hosted MiniMax API by re-introducing the grammar-compile failure on a local engine. Reachable, a regression, and the fails-closed direction (the local engine breaks, not the cloud one). The deferred note about the same leg also firing for MiniMax-M2.5 on first-party Alibaba Coding Plan routing belongs in the same fix.

The hard part is real: hosted MiniMax rejects the omission, llama.cpp rejects the presence, and the hostname genuinely carries no hint behind an aggregating gateway. But "model id matches /minimax/i" cannot distinguish "gateway proxying hosted MiniMax" from "local llama.cpp running MiniMax weights" — both look identical at this layer. The fix probably wants one of: (a) exclude loopback/private/known-local hosts from the model-id leg, accepting that a remote self-hosted engine still misfires; (b) make the model-id leg opt-in via config for gateway users; or (c) key off a stronger signal than the bare id. Any of the three beats the current unconditional leg.

R2-2 (Suggestion) also reproduces. Both new gate tests reach keepParameterlessParameters: true only through the model-id leg; the host leg — isMiniMaxProvider, which covers the official api.minimaxi.com / api.minimax.io routings, i.e. the most common way this gate fires — has no test at either gated call site. Narrowing the predicate to a bare model-id regex would stay green. One case with baseUrl: https://api.minimaxi.com/v1 and a non-MiniMax model id expecting true pins it.

Everything else checks out: the default path stays byte-identical (options defaults to {}, the two pre-existing llama.cpp/LM Studio pins are unedited), the converter assertions are made on the serialized wire body rather than the returned object (the right level for a JSON.stringify bug), the logging mirror correctly uses the request-level model so the logged body matches the sent body, and the R1-1 thread is resolved with the test it asked for.

CI note: Test / Lint / Integration / web-shell E2E / Desktop x2 all green on this head.

@qwen-code-review-bot qwen-code-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Duplicate of the COMMENT review posted at 15:07:29Z (id 5211853255) — a flaky network made the client report failure after the review had actually been created, triggering retries. No content difference; please ignore this copy.

@qwen-code-review-bot qwen-code-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Duplicate of the COMMENT review posted at 15:07:29Z (id 5211853255) — a flaky network made the client report failure after the review had actually been created, triggering retries. No content difference; please ignore this copy.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

> 你好 ✕ [API Error: 400 invalid params, function parameters is empty (2013)] > /update ●︎ Qwen Code 0.23.3 已是最新!

7 participants