fix(llm): detect mistral.ai provider and support reasoning_effort (#4698)

* fix(llm): detect mistral.ai provider and support reasoning_effort

Four coupled bugs broke Mistral thinking model support:

1. _detect_provider() had no mistral.ai host check, so all Mistral
   endpoints fell through to the generic 'openai' provider string.
   _provider_display_name() correctly identified them as 'Mistral',
   making any 'if provider == "Mistral"' check elsewhere dead code.

2. reasoning_effort parameter was never sent in the request payload,
   so Mistral never activated thinking mode even when the user
   configured a thinking-capable model (mistral-small-latest,
   mistral-medium-latest, magistral-*).

3. Mistral returns content as a typed array
   ([{"type":"thinking",...},{"type":"text",...}]) when
   reasoning is on, not as a plain string. Both the streaming and
   non-streaming parsers expected strings and silently dropped the
   thinking content.

4. _THINKING_MODEL_PATTERNS didn't include magistral or mistral-*
   model prefixes, so the frontend wouldn't tag reasoning output
   as thinking even after the above were fixed.

Fix:
- Add mistral.ai to _detect_provider() host checks
- Add a _normalize_mistral_content() helper that splits the typed
  array into (text, thinking) strings
- Inject payload["reasoning_effort"] = "high" when provider is
  Mistral and _supports_thinking(model) is true, in both stream_llm
  and llm_call_async payload construction
- Wire the normalizer into both response parsers
- Extend _THINKING_MODEL_PATTERNS to include magistral,
  mistral-small, mistral-medium, mistral-large

Tested on Docker install with mistral-small-latest +
reasoning_effort=high. Reasoning streams correctly into the
thinking panel after the fix.

Fixes #4678

* fix(llm): address review — lowercase provider id, configurable effort, tests

Addresses vdmkenny's review on PR #4698:

1. Removed duplicate 'if provider == "mistral"' block in stream_llm
   — two back-to-back copies, one was dead-redundant.

2. Dropped personal-context comment ('free-tier limits are generous
   for this user') and made reasoning_effort configurable via env var
   ODYSSEUS_MISTRAL_REASONING_EFFORT (high / medium / low / none).
   Default remains 'high' for backward compat with the tested behavior.

3. Recased provider id from 'Mistral' to 'mistral' to match the
   lowercase convention used by every other provider id in the file
   (openai, anthropic, ollama, copilot, ...). _provider_display_name()
   still returns the Title-Case 'Mistral' for UI labels — only the
   runtime id used in 'if provider == ...' checks was recased.

4. Added tests/test_llm_core_mistral_content.py with 13 tests pinning
   _normalize_mistral_content()'s contract: string passthrough, the
   Mistral array format (thinking + text blocks), and edge cases
   (empty, garbage, None, wrong types, missing fields, string-vs-array
   inner thinking field).

Also fixed a gap the review didn't catch: the non-streaming paths
(llm_call sync + llm_call_async) were missing the reasoning_effort
injection entirely. Added the same injection to both, so Deep Research
and agent tool calls also activate Mistral thinking.

All 13 new tests pass. Existing reasoning/streaming/ollama-thinking
tests still pass (38 tests, no regressions).

Fixes #4678
This commit is contained in:
aubrey
2026-06-23 05:28:17 -03:00
committed by GitHub
parent fef08ed114
commit bd9149f79a
2 changed files with 226 additions and 2 deletions
+156
View File
@@ -0,0 +1,156 @@
"""Tests for _normalize_mistral_content() — Mistral's structured content parser.
Mistral's chat completions API returns content as a typed array when reasoning
is enabled, instead of the plain string most OpenAI-compat servers use:
"content": [
{"type": "thinking", "thinking": [{"type": "text", "text": "..."}], "closed": true},
{"type": "text", "text": "..."}
]
_normalize_mistral_content() splits that into (text, thinking) plain strings.
The function is called from three sites:
- llm_call (sync, non-streaming response parser)
- llm_call_async (async, non-streaming response parser)
- stream_llm (streaming delta parser)
These tests pin the contract: string passthrough, the array shape, and the
edge cases (empty, garbage, missing fields) so a refactor doesn't silently
drop thinking content or break non-Mistral providers.
"""
from src.llm_core import _normalize_mistral_content
def test_string_passthrough_returns_text_with_empty_thinking():
"""Plain string content (the common case) passes through unchanged."""
text, thinking = _normalize_mistral_content("hello world")
assert text == "hello world"
assert thinking == ""
def test_empty_string_passthrough():
text, thinking = _normalize_mistral_content("")
assert text == ""
assert thinking == ""
def test_array_with_thinking_and_text_blocks():
"""Mistral's documented format: thinking block + text block."""
content = [
{
"type": "thinking",
"thinking": [{"type": "text", "text": "Let me work through this..."}],
"closed": True,
},
{"type": "text", "text": "The answer is 42."},
]
text, thinking = _normalize_mistral_content(content)
assert text == "The answer is 42."
assert thinking == "Let me work through this..."
def test_array_with_only_thinking_block():
"""Streaming deltas often contain only a thinking fragment (no text block yet)."""
content = [
{
"type": "thinking",
"thinking": [{"type": "text", "text": "Okay, let's"}],
"closed": True,
}
]
text, thinking = _normalize_mistral_content(content)
assert text == ""
assert thinking == "Okay, let's"
def test_array_with_only_text_block():
"""Final answer delta — only the text block, no thinking."""
content = [{"type": "text", "text": "Final answer."}]
text, thinking = _normalize_mistral_content(content)
assert text == "Final answer."
assert thinking == ""
def test_array_concatenates_multiple_text_blocks():
"""Multiple text blocks are concatenated in order."""
content = [
{"type": "text", "text": "part 1 "},
{"type": "text", "text": "part 2"},
]
text, thinking = _normalize_mistral_content(content)
assert text == "part 1 part 2"
def test_array_concatenates_multiple_thinking_fragments():
"""Multiple thinking sub-blocks are concatenated in order."""
content = [
{
"type": "thinking",
"thinking": [
{"type": "text", "text": "first "},
{"type": "text", "text": "second"},
],
"closed": True,
}
]
text, thinking = _normalize_mistral_content(content)
assert text == ""
assert thinking == "first second"
def test_empty_array_returns_empty_strings():
text, thinking = _normalize_mistral_content([])
assert text == ""
assert thinking == ""
def test_array_with_garbage_entries_skips_them():
"""Non-dict entries, missing type, missing text — all silently skipped."""
content = [
"not a dict",
None,
{"type": "unknown_type", "text": "should be ignored"},
{"type": "text"}, # missing text key
{"type": "thinking"}, # missing thinking key
{"type": "text", "text": "valid text"},
]
text, thinking = _normalize_mistral_content(content)
assert text == "valid text"
assert thinking == ""
def test_none_returns_empty_strings():
"""Defensive: None content (server bug or schema drift) doesn't crash."""
text, thinking = _normalize_mistral_content(None)
assert text == ""
assert thinking == ""
def test_int_returns_empty_strings():
"""Defensive: wrong-typed content doesn't crash."""
text, thinking = _normalize_mistral_content(42)
assert text == ""
assert thinking == ""
def test_thinking_block_with_string_inner():
"""Some Mistral API versions may use a string instead of an array for
the inner 'thinking' field. Accept both shapes."""
content = [
{"type": "thinking", "thinking": "inline string thinking"},
{"type": "text", "text": "answer"},
]
text, thinking = _normalize_mistral_content(content)
assert text == "answer"
assert thinking == "inline string thinking"
def test_thinking_block_with_empty_text_field():
"""Empty text fields don't pollute the output."""
content = [
{"type": "thinking", "thinking": [{"type": "text", "text": ""}], "closed": True},
{"type": "text", "text": ""},
]
text, thinking = _normalize_mistral_content(content)
assert text == ""
assert thinking == ""