Fix Xiaomi MiMo reasoning-only completions#60304
Conversation
Greptile SummaryThis PR adds a Xiaomi-specific stream compatibility wrapper in Confidence Score: 5/5Safe to merge — no P0/P1 issues, all remaining findings are minor style suggestions. The core normalization logic is correct and well-guarded. Both the iterator and result() rewrite paths are exercised by tests. The only open items are a stylistic early-break asymmetry and missing direct coverage of mimo-v2-omni, neither of which affects runtime behavior. No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/xiaomi/stream.ts
Line: 62-65
Comment:
**Early `break` may shadow `hasToolCalls` detection**
The `break` fires as soon as any renderable text block is found, so subsequent `toolCall` blocks are never visited — `hasToolCalls` stays `false` for those iterations. Correctness is preserved because `hasRenderableText = true` already causes the function to bail on line 78, but the loop's three booleans read as symmetric flags when they aren't. If the loop body ever grows (e.g. another bypass condition), this implicit ordering could silently create a bug. Consider removing the `break` or adding a comment explaining that only `hasRenderableText` needs to short-circuit.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/xiaomi/index.test.ts
Line: 62-104
Comment:
**`mimo-v2-omni` not covered by tests**
The `XIAOMI_REASONING_AS_TEXT_MODEL_IDS` Set lists both `mimo-v2-pro` and `mimo-v2-omni` as target models, but the tests only exercise `mimo-v2-pro`. Adding a parallel case for `mimo-v2-omni` (or parameterizing the existing test) would lock in the full set of affected models against future `XIAOMI_REASONING_AS_TEXT_MODEL_IDS` edits.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Normalize Xiaomi reasoning-only completi..." | Re-trigger Greptile Re-review progress:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ) { | ||
| hasRenderableText = true; | ||
| break; | ||
| } |
There was a problem hiding this comment.
break may shadow hasToolCalls detection
The break fires as soon as any renderable text block is found, so subsequent toolCall blocks are never visited — hasToolCalls stays false for those iterations. Correctness is preserved because hasRenderableText = true already causes the function to bail on line 78, but the loop's three booleans read as symmetric flags when they aren't. If the loop body ever grows (e.g. another bypass condition), this implicit ordering could silently create a bug. Consider removing the break or adding a comment explaining that only hasRenderableText needs to short-circuit.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/xiaomi/stream.ts
Line: 62-65
Comment:
**Early `break` may shadow `hasToolCalls` detection**
The `break` fires as soon as any renderable text block is found, so subsequent `toolCall` blocks are never visited — `hasToolCalls` stays `false` for those iterations. Correctness is preserved because `hasRenderableText = true` already causes the function to bail on line 78, but the loop's three booleans read as symmetric flags when they aren't. If the loop body ever grows (e.g. another bypass condition), this implicit ordering could silently create a bug. Consider removing the `break` or adding a comment explaining that only `hasRenderableText` needs to short-circuit.
How can I resolve this? If you propose a fix, please make it concise.| it("normalizes reasoning-only final assistant messages into text for MiMo reasoning models", async () => { | ||
| const stream = runWrappedXiaomiStream({ | ||
| modelId: "mimo-v2-pro", | ||
| events: [ | ||
| { | ||
| type: "done", | ||
| reason: "stop", | ||
| message: { | ||
| role: "assistant", | ||
| content: [{ type: "thinking", thinking: "MiMo final answer" }], | ||
| stopReason: "stop", | ||
| }, | ||
| }, | ||
| ], | ||
| resultMessage: { | ||
| role: "assistant", | ||
| content: [{ type: "thinking", thinking: "MiMo final answer" }], | ||
| stopReason: "stop", | ||
| }, | ||
| }); | ||
|
|
||
| const events: unknown[] = []; | ||
| for await (const event of stream) { | ||
| events.push(event); | ||
| } | ||
|
|
||
| await expect(stream.result()).resolves.toEqual({ | ||
| role: "assistant", | ||
| content: [{ type: "text", text: "MiMo final answer" }], | ||
| stopReason: "stop", | ||
| }); | ||
| expect(events).toEqual([ | ||
| { | ||
| type: "done", | ||
| reason: "stop", | ||
| message: { | ||
| role: "assistant", | ||
| content: [{ type: "text", text: "MiMo final answer" }], | ||
| stopReason: "stop", | ||
| }, | ||
| }, | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
mimo-v2-omni not covered by tests
The XIAOMI_REASONING_AS_TEXT_MODEL_IDS Set lists both mimo-v2-pro and mimo-v2-omni as target models, but the tests only exercise mimo-v2-pro. Adding a parallel case for mimo-v2-omni (or parameterizing the existing test) would lock in the full set of affected models against future XIAOMI_REASONING_AS_TEXT_MODEL_IDS edits.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/xiaomi/index.test.ts
Line: 62-104
Comment:
**`mimo-v2-omni` not covered by tests**
The `XIAOMI_REASONING_AS_TEXT_MODEL_IDS` Set lists both `mimo-v2-pro` and `mimo-v2-omni` as target models, but the tests only exercise `mimo-v2-pro`. Adding a parallel case for `mimo-v2-omni` (or parameterizing the existing test) would lock in the full set of affected models against future `XIAOMI_REASONING_AS_TEXT_MODEL_IDS` edits.
How can I resolve this? If you propose a fix, please make it concise.
|
Codex review: needs maintainer review before merge. Summary Reproducibility: yes. at source level. Current main exposes the target MiMo models as reasoning OpenAI-completions models, maps Real behavior proof Next step before merge Security Review detailsBest possible solution: Land the scoped Xiaomi/provider-family normalization after normal maintainer review, preserving the existing replay fix and keeping the final-output promotion limited to MiMo V2 reasoning-only terminal responses. Do we have a high-confidence way to reproduce the issue? Yes, at source level. Current main exposes the target MiMo models as reasoning OpenAI-completions models, maps Is this the best way to solve the issue? Yes. A Xiaomi-owned wrapper plus a shared provider-family helper for custom proxy routes is the narrow maintainable fix, and the force-pushed version preserves tool-call reasoning replay and already-visible text. Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 6de8563827ad. |
|
Hit this on v2026.5.7 with xiaomi/mimo-v2-pro thinking=medium. Same error: Param Incorrect: The reasoning_content in the thinking mode must be passed back to the API. Disabling thinking works around it. Bumping for visibility — this PR has been open for a month. |
efe7922 to
1056c81
Compare
openclaw-barnacle
Bot
added
agents
|
Maintainer rewrite pushed in 0de9f69. Changed:
Code refs:
Tested:
Not live-tested:
|
Co-authored-by: HiddenPuppy <[email protected]>
clawsweeper
Bot
added
the
proof: sufficient
openclaw-barnacle
Bot
removed
the
proof: sufficient
1056c81 to
0de9f69
Compare
openclaw-barnacle
Bot
added
proof: supplied
Co-authored-by: Peter Steinberger <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
3 participants
Summary
reasoning_content.mimo-v2-pro/mimo-v2-omnioutputs, including custom Xiaomi-compatible OpenAI-completions proxy routes.Changed
extensions/xiaomi/stream.ts: direct Xiaomi wrapper composes replay/passthrough with terminal reasoning-only visible-text promotion.src/plugin-sdk/provider-stream-shared.ts: shared stream helper promotes final thinking-only assistant output to text while preserving existing text and tool-call turns.src/agents/pi-embedded-runner/extra-params.ts: custom proxy fallback applies the same MiMo V2 response-side promotion when the Xiaomi plugin wrapper is not in the provider path.extensions/xiaomi/index.test.tsandsrc/agents/pi-embedded-runner-extraparams.test.ts: regression coverage for direct Xiaomi and proxy routes.Real behavior proof
Behavior addressed: Xiaomi MiMo
mimo-v2-pro/mimo-v2-omnican return the final user-visible answer inreasoning_content; OpenClaw parsed that as thinking-only content, so the rendered reply could appear blank.Real environment tested: local OpenClaw checkout plus a real Xiaomi console API key created temporarily for this verification and revoked afterward.
Exact steps or command run after this patch:
OPENCLAW_LIVE_TEST=1 XIAOMI_API_KEY=<temporary Xiaomi key> pnpm test extensions/xiaomi/xiaomi-chat.tmp.test.tspnpm test extensions/xiaomi/index.test.ts src/agents/pi-embedded-runner-extraparams.test.ts src/plugin-sdk/provider-stream-shared.test.tspnpm check:changedpnpm buildcodex-review --mode local --parallel-tests "pnpm test extensions/xiaomi/index.test.ts src/agents/pi-embedded-runner-extraparams.test.ts src/plugin-sdk/provider-stream-shared.test.ts"Evidence after fix: live
mimo-v2-proreturned the requested markerXIAOMI_VISIBLE_E2E_OKas visible assistant text; focused tests passed; Codex review reported no actionable findings; changed gate passed on Blacksmith Testboxtbx_01krnvk8eq4ncvaqhnkg4swy2awith Actions run https://github.com/openclaw/openclaw/actions/runs/25919175341.Observed result after fix: reasoning-only terminal MiMo V2 outputs are converted to visible text, while replay/tool-call reasoning and already-visible text remain unchanged.
What was not tested: live
mimo-v2-omni; it is covered by the same unit wrapper path asmimo-v2-pro.Linked Issue
Closes #60261