Conversation
When a Mistral model returns empty content and hides the answer in a <response> block inside the thinking trace, the converter recovered the answer but trimmed the reasoning at </response>, so the reported reasoning still contained the opening tag and a duplicate of the answer. Trim at the opening tag instead. Co-authored-by: Tony Coder <407243179@qq.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 4 remain after this review. WalkthroughChangesMistral response extraction
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
9 tasks
tbille
pushed a commit
that referenced
this pull request
Aug 18, 2026
## Description Non-streaming _convert_response mapped only text, tool_use, and thinking, then raised ValueError on anything else. Anthropic returns redacted_thinking when safety flags part of an extended-thinking trace, so completion() crashed even though the rest of the answer was fine. The streaming converter already ignores unmapped block types. Skip redacted_thinking quietly; warn and skip other unmapped types. Shared by anthropic, azureanthropic, vertexaianthropic, and meta. Tests: uv run pytest tests/unit -> 2158 passed. Distinct from #1292 and #1296-#1302. ## PR Type - Bug Fix ## Relevant issues No open issue. ## Checklist - [x] I understand the code I am submitting. - [x] I have added unit tests that prove my fix/feature works - [x] I have run this code locally and verified it fixes the issue. - [x] New and existing tests pass locally - [x] Documentation was updated where necessary - [x] I have read and followed the contribution guidelines - [x] **AI Usage:** - [x] This is fully AI-generated. ## AI Usage Information - AI Model used: Claude Opus 5 - AI Developer Tool used: Cursor cloud agent - [x] I am an AI Agent filling out this form (check box if true) Co-authored-by: Cursor Agent <cursoragent@cursor.com>
tbille
approved these changes
Aug 18, 2026
tonycoder-hub
temporarily deployed
to
integration-tests
August 18, 2026 09:39 — with
GitHub Actions
Inactive
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 31 files with indirect coverage changes 🚀 New features to boost your workflow:
|
JamMaster1999
added a commit
to JamMaster1999/any-llm
that referenced
this pull request
Aug 18, 2026
Brings in upstream's merges of our mozilla-ai#1291/mozilla-ai#1292/mozilla-ai#1310 plus mozilla-ai#1297, mozilla-ai#1299, mozilla-ai#1301, mozilla-ai#1302, mozilla-ai#1303, mozilla-ai#1305. Carried-until-merged fork work stays: mozilla-ai#1294 (gemini reasoning_effort=none), mozilla-ai#1308 (aresponses timeout), mozilla-ai#1309 (gemini native tool dicts), and the mozilla-ai#1300 carry. One conflict in tests/unit/test_responses.py: kept our mozilla-ai#1308 timeout test next to upstream's flatten test. Unit suite: 2234 passed. Claude-Session: https://claude.ai/code/session_018D3FGNvb1hRZQmsXFoA44J
11 tasks
javiermtorres
pushed a commit
to shoemoney/any-llm
that referenced
this pull request
Aug 26, 2026
…ming path too 🔧 mozilla-ai#1302 fixed this for the non-streaming converter (_create_mistral_completion_from_response) but the streaming converter (_create_openai_chunk_from_mistral_chunk) never got the same treatment. With stream=True, a Magistral response that wraps its answer in <response>...</response> inside the thinking delta comes out with delta.content=None on every chunk - the whole answer, tags included, sits only in delta.reasoning.content. A caller following the standard OpenAI streaming contract (accumulate delta.content) gets an empty response with no error. Factored the split/trim logic out of the non-streaming function into a shared _split_response_tag_from_reasoning() helper and call it from both converters, so the two paths can't drift apart a third time. Streaming semantics note: this only catches the case where both <response> and </response> land in the same chunk's reasoning text; a marker split across two chunks is not reassembled, and that limitation is called out in the docstring.
javiermtorres
pushed a commit
that referenced
this pull request
Aug 26, 2026
…ming path too (#1336) ## What #1302 fixed a bug where some Mistral reasoning models (Magistral) sometimes return the real answer wrapped in `<response>...</response>` inside the reasoning/thinking content instead of in `content`. That fix only landed in `_create_mistral_completion_from_response` (the non-streaming converter), `src/any_llm/providers/mistral/utils.py:209-218` on main before this PR. `_create_openai_chunk_from_mistral_chunk` (the streaming converter, same file, ~256-334) never got the equivalent change. ## The bug With `stream=True`, when a `ThinkChunk` delta's thinking text contains the `<response>` markers, `choice.delta.content` stays `None` for that chunk and the whole answer, tags included, ends up only in `delta.reasoning.content`. A caller following the standard OpenAI streaming contract (accumulate `delta.content` across chunks) gets an empty response. No exception, no error - it just silently returns nothing. The identical request with `stream=False` returns the answer correctly today (because of #1302), which is what made this easy to miss. ## The fix Pulled the split/trim logic out of the non-streaming function into a shared `_split_response_tag_from_reasoning()` helper and call it from both converters. That's the part that actually matters here - the two converters had this same logic and one of them fell behind once already; sharing it is what stops that from happening a second time. ## Streaming semantics limitation Deltas arrive in fragments, so in principle `<response>` or `</response>` could be split across two separate chunks. This fix only handles the case where a single chunk's reasoning text contains both the opening and closing tag, mirroring what #1302 already does for a single non-streaming response body. It does not buffer or reassemble text across chunks to catch a tag boundary that lands mid-chunk-break. Noting this here rather than silently leaving it unhandled; happy to follow up with a stateful/buffered version if that's wanted, but wanted to keep this diff minimal and match #1302's scope. ## Testing Added to `tests/unit/providers/test_mistral_provider.py`: - `test_create_openai_chunk_strips_response_block_from_reasoning` - a streaming chunk whose thinking text contains `<response>...</response>` yields `delta.content` with the answer and `reasoning` trimmed to what came before the opening tag - `test_create_openai_chunk_leaves_plain_reasoning_unchanged` - regression: a normal streaming chunk with plain reasoning and no tags is unaffected Ran: - `uv run pytest tests/unit/providers/test_mistral_provider.py -p no:rerunfailures` - 64 passed (62 previously existing + 2 new) - Reverted the source change only (kept the new tests) and reran the two new tests to confirm they fail against the old code: `test_create_openai_chunk_strips_response_block_from_reasoning` failed with `AssertionError: assert None == 'The answer is 42.'` - `delta.content` stayed `None`, matching the reported bug exactly. Restored the fix afterward and reran the full file to confirm 64 passed again. - `uv run ruff check` / `uv run ruff format --check` on both changed files - clean - `uv run mypy src/any_llm/providers/mistral/utils.py` - no issues ## PR Type <!-- Delete the types that don't apply --> - 🐛 Bug Fix ## Relevant issues <!-- e.g. "Fixes #123" --> Follow-up to #1302, which fixed the same bug in the non-streaming converter but missed the streaming one. ## Checklist <!-- If this checklist is deleted from the PR submission it will be immediately closed --> - [x] I understand the code I am submitting. - [x] I have added unit tests that prove my fix/feature works - [x] I have run this code locally and verified it fixes the issue. - [x] New and existing tests pass locally - [ ] Documentation was updated where necessary - [x] I have read and followed the [contribution guidelines](https://github.com/mozilla-ai/any-llm/blob/main/CONTRIBUTING.md) - [x] **AI Usage:** - [ ] No AI was used. - [x] AI was used for drafting/refactoring. - [ ] This is fully AI-generated. ## AI Usage Information <!-- We welcome the use of AI to aid in contribution! Optional: We're interested in hearing about your setup. What LLM are you using (e.g. Opus 4.5, GPT-5, Minimax), and which tooling (Claude Code, VsCode, OpenCode, etc) --> - AI Model used: Claude Opus 5 - AI Developer Tool used: Claude Code - Any other info you'd like to share: Written in conjunction with my pair programmer Claude. The finding was verified against upstream before any code was written, and the fix is covered by tests that fail against the unpatched source. When answering questions by the reviewer, please respond yourself, do not copy/paste the reviewer comments into an AI system and paste back its answer. We want to discuss with you, not your AI :) - [x] I am an AI Agent filling out this form (check box if true) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Mistral response handling when answers are embedded within reasoning content. * Recovered wrapped answers consistently in both standard and streaming responses. * Prevented response markers from appearing in displayed reasoning. * Preserved existing behaviour for ordinary reasoning without response markers. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Some Mistral reasoning models hide the answer in a response block inside the thinking trace. The converter already extracted the answer but trimmed reasoning at the closing tag, so the opening tag and a duplicate of the answer stayed in reasoning.content.
Split on the opening tag instead. Tests: uv run pytest tests/unit/providers/test_mistral_provider.py -> 62 passed; tests/unit 2157 passed.
Distinct from #1296-#1301.
PR Type
Relevant issues
No open issue.
Checklist
AI Usage Information