-
Notifications
You must be signed in to change notification settings - Fork 229
fix(mistral): trim reasoning at the opening response tag in the streaming path too #1336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1351,6 +1351,69 @@ def test_create_openai_chunk_captures_thinking_signature() -> None: | |
| assert chunk.choices[0].delta.extra_content == {"mistral": {"signature": "sig-abc"}} | ||
|
|
||
|
|
||
| def test_create_openai_chunk_strips_response_block_from_reasoning() -> None: | ||
| """Streaming must recover a <response>-wrapped answer the same way non-streaming does. | ||
|
|
||
| #1302 fixed this for `_create_mistral_completion_from_response` but left the streaming | ||
| converter untouched, so `stream=True` silently returned an empty `delta.content` for the | ||
| same payload. This is the streaming half of that fix. | ||
| """ | ||
| pytest.importorskip("mistralai") | ||
| from mistralai.client.models import TextChunk, ThinkChunk | ||
|
Comment on lines
+1354
to
+1362
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add tests for the new guard paths. The tests cover a complete response block and reasoning without tags. They do not cover the As per coding guidelines, tests must cover every new branch, including error, raise, and edge paths. Also applies to: 1389-1392 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| from any_llm.providers.mistral.utils import _create_openai_chunk_from_mistral_chunk | ||
|
|
||
| choice = Mock() | ||
| choice.index = 0 | ||
| choice.delta.content = [ | ||
| ThinkChunk(thinking=[TextChunk(text="Let me work it out.<response>The answer is 42.</response>")]) | ||
| ] | ||
| choice.delta.role = "assistant" | ||
| choice.delta.tool_calls = None | ||
| choice.finish_reason = None | ||
|
|
||
| event = Mock() | ||
| event.data.id = "chatcmpl-abc" | ||
| event.data.created = 1_700_000_000 | ||
| event.data.model = "magistral-medium-latest" | ||
| event.data.choices = [choice] | ||
| event.data.usage = None | ||
|
|
||
| chunk = _create_openai_chunk_from_mistral_chunk(event) | ||
|
|
||
| assert chunk.choices[0].delta.content == "The answer is 42." | ||
| assert chunk.choices[0].delta.reasoning is not None | ||
| assert chunk.choices[0].delta.reasoning.content == "Let me work it out." | ||
|
|
||
|
|
||
| def test_create_openai_chunk_leaves_plain_reasoning_unchanged() -> None: | ||
| """Regression: a normal streaming chunk with plain reasoning and no tags is unaffected.""" | ||
| pytest.importorskip("mistralai") | ||
| from mistralai.client.models import TextChunk, ThinkChunk | ||
|
|
||
| from any_llm.providers.mistral.utils import _create_openai_chunk_from_mistral_chunk | ||
|
|
||
| choice = Mock() | ||
| choice.index = 0 | ||
| choice.delta.content = [ThinkChunk(thinking=[TextChunk(text="just thinking, nothing special")])] | ||
| choice.delta.role = "assistant" | ||
| choice.delta.tool_calls = None | ||
| choice.finish_reason = None | ||
|
|
||
| event = Mock() | ||
| event.data.id = "chatcmpl-abc" | ||
| event.data.created = 1_700_000_000 | ||
| event.data.model = "mistral-medium-3-5" | ||
| event.data.choices = [choice] | ||
| event.data.usage = None | ||
|
|
||
| chunk = _create_openai_chunk_from_mistral_chunk(event) | ||
|
|
||
| assert chunk.choices[0].delta.content is None | ||
| assert chunk.choices[0].delta.reasoning is not None | ||
| assert chunk.choices[0].delta.reasoning.content == "just thinking, nothing special" | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_timeout_is_translated_to_timeout_ms() -> None: | ||
| """The seconds-based any-llm ``timeout`` must become the SDK's ``timeout_ms``. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: mozilla-ai/any-llm
Length of output: 29860
🏁 Script executed:
Repository: mozilla-ai/any-llm
Length of output: 16327
🏁 Script executed:
Repository: mozilla-ai/any-llm
Length of output: 47804
🏁 Script executed:
Repository: mozilla-ai/any-llm
Length of output: 411
Buffer incomplete
<response>markers across streaming chunks. The converter processes each event independently. If either marker is split, the answer remains indelta.reasoningand is not emitted indelta.content. Keep incomplete marker text in stream-level state and add tests for split opening and closing markers.🤖 Prompt for AI Agents