Repository navigation
fix(providers): gate MiMo bare-call recovery on a terminal signal and limit DeepSeek upload roles - #7396
Conversation
A MiMo stream that closed after a complete bare block but before finish_reason or [DONE] reached the EOF flush with user-facing output, and standalone recovery emitted an executable call that structured calls on the same path would have refused as possible truncation. Recover only after a terminal signal; otherwise the block stays visible text. Co-authored-by: shirruy <63145023+shirruy@users.noreply.github.com>
System and assistant message images were uploaded and persisted even though DeepSeek rejects images in those roles. Restrict uploads to user and developer messages plus tool outputs. Co-authored-by: fontanierh <14199823+fontanierh@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 5 files. (1 skipped: 1… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title accurately summarizes both primary changes: terminal-signal gating for MiMo bare-call recovery and role restrictions for DeepSeek image uploads. It is specific, concise, and clearly related … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title accurately summarizes both primary changes: terminal-signal gating for MiMo bare-call recovery and role restrictions for DeepSeek image uploads. It is specific, concise, and clearly related to the pull request.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 5 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a03be2f65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
| } | ||
| if ((yield* flushToolCalls()) === "terminate") return; | ||
| if ((yield* flushToolCalls(sawFinish)) === "terminate") return; // A cut stream never synthesizes a bare call. |
There was a problem hiding this comment.
When a provider emits finish_reason and then sends another content delta before EOF, finishReason remains set, so this passes true even though the candidate bare block arrived after the only terminal signal. The earlier finish flush finds no candidate and leaves standalone recovery enabled, allowing a later cut-off <tool_call> to execute—the truncated-response hazard this change is intended to prevent. Track terminal state relative to newly ingested content, or disable recovery after the first terminal flush, instead of using the response-wide sawFinish value.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
…covery decision Content that arrives after finish_reason kept standalone recovery enabled, so a later cut-off block could still execute at EOF or [DONE]. Co-authored-by: shirruy <63145023+shirruy@users.noreply.github.com>
1 participant
Summary
This is a follow-up to #7394 (B4a, already merged). It fixes the two Codex review findings on that PR, both confirmed real:
src/adapters/openai-chat.ts:341): a MiMo stream that closed after a complete bare<tool_call>block, but beforefinish_reasonor[DONE], reached the EOF flush with user-facing output. Standalone recovery then emitted an executable freeform call, such asexec, from a possibly truncated response, even though structured calls on the same path are refused as possible truncation. Recovery now runs only after a terminal signal; otherwise the block stays visible text.openai-chat.tsstays at 822 lines:flushToolCallstakes aterminalflag that defaults totrue, and the EOF site passessawFinish.src/server/responses/deepseek-image-files.ts:61): theitem.rolecheck also matchedsystemandassistantmessages, so their images were uploaded and kept on DeepSeek for an hour, although DeepSeek rejects images in those roles. Uploads are now limited touseranddevelopermessages plus the two tool-output types. This only matters when a model has opted in withdeepseekImageFiles.Verification
Based on
origin/dev74463a2. Tests ran with the pinned Bun 1.4.3.bun run typecheck,bun run structure:check,bun run privacy:scan: passbun test --isolate tests/adapters/openai/ tests/providers/deepseek-image-files.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts: 936 pass / 0 fail (42 files)a stream cut without a terminal signal never executes a bare blockandimages in roles DeepSeek rejects are never uploaded) fails with thesrc/changes stashed and passes with them.--no-verify: this host's local pre-push hook runs the fullbun run test, which cannot pass here because of environment-only launcher failures (described in fix(providers): carry Gemini, MiMo, DeepSeek, Mistral, opencode-go and audio adapter fixes for 2.90 #7394). A full suite was not run locally; CI covers it.src/adapters/openai-chat.ts: 822 lines (baseline cap 822).Co-authored-by: shirruy 63145023+shirruy@users.noreply.github.com
Co-authored-by: fontanierh 14199823+fontanierh@users.noreply.github.com
Review follow-ups after opening
Codex P1 on the first fix: content arriving after
finish_reasonkept standalone recovery enabled, so a later cut-off block could still execute at EOF or[DONE]. The first terminal flush is now the only recovery decision. A regression test covering both the EOF and[DONE]tails fails without the fix.bun test --isolate tests/adapters/openai/: 884 pass / 0 fail.CI's red
cold-spawn warm-up coverageshard is not from this PR. It fails the same way on cleanorigin/devdbb908f (fix(oauth): harden OAuth login/import, quota diagnostics, service/update paths and shim token handling (B5) #7397 addedcodex-shim-bypass-token.test.tswithout a disposition). Thecodex-shim-windows-revisionfailure in one shard passes locally ondev, and this PR touches no shim code.Checklist