fix(mistral): handle structured content from magistral reasoning models (#14805)

### What problem does this PR solve?

`MistralModel.ChatWithMessages` (in the driver merged via #14807)
assumes that `choices[0].message.content` from `/v1/chat/completions` is
always a string and falls through to `return nil, fmt.Errorf("invalid
content format")` on anything else.

That assumption breaks for the **magistral reasoning family**
(`magistral-small-*`, `magistral-medium-*`). When the model needs a
chain-of-thought to answer, Mistral returns `content` as a **structured
array of typed parts**:

```json
"content": [
  {"type": "thinking",
   "thinking": [{"type": "text", "text": "Combined speed is 150 mph. 300 / 150 = 2 hours."}],
   "closed": true},
  {"type": "text", "text": "They will meet after **2 hours**."}
]
```

Concretely, this is what the live API returns today (probed against
`api.mistral.ai/v1`):

```
$ curl -H "Authorization: Bearer <key>" -H "Content-Type: application/json" \
       -X POST https://api.mistral.ai/v1/chat/completions \
       -d '{"model":"magistral-medium-latest",
            "messages":[{"role":"user","content":"two trains 60mph and 90mph, 300mi apart, when do they meet? step by step."}],
            "max_tokens":1024}'
HTTP 200
{ "choices":[{"message":{
    "role":"assistant",
    "content":[
      {"type":"thinking","thinking":[{"type":"text","text":"Okay, let's see..."}],"closed":true},
      {"type":"text","text":"To determine when the two trains meet..."}
    ]}}] }
```

With the current driver, every call like that returns the generic
`"invalid content format"` error. Trivial prompts that happen to fit in
a string answer still succeed, so the breakage is **non-deterministic
from the tenant's POV**: same model, same provider, sometimes works,
sometimes 500s with no useful error.

A secondary issue: `conf/models/mistral.json` does not include any
magistral model. The picker hid the broken path, which is why this
wasn't caught during #14807's review.

### What this PR includes

- New helper `extractMistralContent(raw interface{}) (answer,
reasonContent string, err error)` in
`internal/entity/models/mistral.go`, which normalizes both shapes
Mistral can return:
- `string` → historical path. `Answer = content`, `ReasonContent = ""`.
Preserves behavior for every non-reasoning model (`mistral-large-*`,
`mistral-small-*`, `ministral-*`, `codestral-*`, `pixtral-*`,
`open-mistral-nemo`).
- `[]interface{}` → walk the parts. Concatenate every `{"type":"text",
"text":...}` part into `Answer`; concatenate the inner text inside every
`{"type":"thinking", "thinking":[...]}` part into `ReasonContent`.
- `ChatWithMessages` now calls the helper instead of doing the raw
`.(string)` cast.
- Unknown part types are **skipped, not failed**. Mistral has been
adding new content variants quickly (audio chunks, citations, etc.);
this driver should not 500 every call when a new part type appears.
- `conf/models/mistral.json`: add `magistral-medium-latest` and
`magistral-small-latest`. Both are visible in `/v1/models` today.

No interface change. No factory change. No new dependencies.

### How was this tested?

**Unit tests** — 5 new tests in `internal/entity/models/mistral_test.go`
on top of the 27 already shipped via #14807:

- `TestMistralChatHandlesStringContent` — regression net for the
historical path
- `TestMistralChatExtractsReasoningFromStructuredContent` — the fixture
body is a trimmed copy of the actual `magistral-medium-latest` response
captured above; asserts both `Answer` and `ReasonContent` are populated
correctly
- `TestMistralChatHandlesStructuredContentWithoutThinking` —
`magistral-*` with a trivial answer returns a structured shape that has
only a `text` part; `ReasonContent` must stay empty
- `TestMistralChatIgnoresUnknownContentPartTypes` — `audio_url` and
`future_part_type` parts are skipped, `text` parts still flow through
- `TestExtractMistralContent` — table-driven unit coverage of the helper
for string, empty string, nil, empty array, text-only, thinking+text,
unsupported root type

```
$ go test -vet=off -run "TestMistral|TestExtractMistralContent" -count=1 -v ./internal/entity/models/...
=== RUN   TestMistralChatHandlesStringContent
--- PASS: TestMistralChatHandlesStringContent (0.00s)
=== RUN   TestMistralChatExtractsReasoningFromStructuredContent
--- PASS: TestMistralChatExtractsReasoningFromStructuredContent (0.00s)
=== RUN   TestMistralChatHandlesStructuredContentWithoutThinking
--- PASS: TestMistralChatHandlesStructuredContentWithoutThinking (0.00s)
=== RUN   TestMistralChatIgnoresUnknownContentPartTypes
--- PASS: TestMistralChatIgnoresUnknownContentPartTypes (0.00s)
=== RUN   TestExtractMistralContent
=== RUN   TestExtractMistralContent/plain_string
=== RUN   TestExtractMistralContent/empty_string
=== RUN   TestExtractMistralContent/nil
=== RUN   TestExtractMistralContent/empty_array
=== RUN   TestExtractMistralContent/text_only
=== RUN   TestExtractMistralContent/thinking_then_text
=== RUN   TestExtractMistralContent/unknown_root_type
--- PASS: TestExtractMistralContent (0.00s)
PASS
ok      ragflow/internal/entity/models  0.046s
```

All 32 Mistral tests pass on go 1.25. `go build
./internal/entity/models/...` exits 0.

**Live integration test** — driver exercised against `api.mistral.ai/v1`
with the patched code:

```
=== RUN   TestMistralMagistralSmoke
    [OK] "magistral-small-latest" present upstream
    [OK] "magistral-medium-latest" present upstream
    [OK trivial]    Answer="7"  ReasonContent=""
    [OK reasoning]  Answer len=797   head="To determine when the two trains meet, we can follow these steps:\n\n1. **Identify..."
                    ReasonContent len=1069 head="Okay, let's see. There are two trains, one going 60 mph and the other going 90 mph. They're moving towards each other, s..."

MAGISTRAL SMOKE PASSED
--- PASS: TestMistralMagistralSmoke (18.09s)
PASS
ok      ragflow/internal/entity/models  18.112s
```

What the live run proves on the wire:

- `magistral-small-latest` with a trivial prompt still uses the
string-content shape; the regression-net path is exercised against the
real server, not just the mock.
- `magistral-medium-latest` with a reasoning prompt uses the
structured-array shape; the new code path extracts a 1069-character
reasoning trace into `ChatResponse.ReasonContent` and a 797-character
visible answer into `ChatResponse.Answer`. Before this fix, the same
call returned `"invalid content format"` and the caller saw nothing.

The smoke-test file itself is not committed (live tests live outside the
PR diff, same convention used for prior provider PRs).

### Type of change

- [x] Bug Fix (non-breaking change which fixes an issue)
This commit is contained in:
tmimmanuel
2026-05-20 21:19:04 -10:00
committed by Jin Hai
parent 9d37234953
commit 85d0b46d8e
3 changed files with 280 additions and 5 deletions

View File

@@ -89,6 +89,20 @@
"chat"
]
},
{
"name": "magistral-medium-latest",
"max_tokens": 40000,
"model_types": [
"chat"
]
},
{
"name": "magistral-small-latest",
"max_tokens": 40000,
"model_types": [
"chat"
]
},
{
"name": "mistral-embed",
"max_tokens": 8192,

View File

@@ -195,18 +195,80 @@ func (m *MistralModel) ChatWithMessages(modelName string, messages []Message, ap
return nil, fmt.Errorf("invalid message format")
}
content, ok := messageMap["content"].(string)
if !ok {
return nil, fmt.Errorf("invalid content format")
content, reasonContent, err := extractMistralContent(messageMap["content"])
if err != nil {
return nil, err
}
emptyReason := ""
return &ChatResponse{
Answer: &content,
ReasonContent: &emptyReason,
ReasonContent: &reasonContent,
}, nil
}
// extractMistralContent normalizes the two shapes Mistral can return in
// choices[0].message.content.
//
// 1. Plain string. The historical shape, used by every non-reasoning
// Mistral model (mistral-large, mistral-medium, ministral-*, etc.):
//
// "content": "Pong."
//
// 2. Structured array of typed parts. Used by the magistral reasoning
// family (magistral-small-*, magistral-medium-*) when the model
// actually produces a chain-of-thought:
//
// "content": [
// {"type": "thinking", "thinking": [{"type": "text", "text": "..."}]},
// {"type": "text", "text": "The final answer is ..."}
// ]
//
// The function concatenates the visible text parts into the assistant
// answer and the inner thinking text into the reasoning trace. Unknown
// part types are skipped rather than failing, so a new part shape from
// Mistral does not break the driver for tenants that don't use it.
func extractMistralContent(raw interface{}) (string, string, error) {
switch v := raw.(type) {
case string:
return v, "", nil
case []interface{}:
var answer, reasoning strings.Builder
for _, part := range v {
pm, ok := part.(map[string]interface{})
if !ok {
continue
}
switch pm["type"] {
case "text":
if t, ok := pm["text"].(string); ok {
answer.WriteString(t)
}
case "thinking":
// thinking is an array of inner text parts; concatenate
// any inner element with a non-empty text field.
inner, ok := pm["thinking"].([]interface{})
if !ok {
continue
}
for _, sub := range inner {
sm, ok := sub.(map[string]interface{})
if !ok {
continue
}
if t, ok := sm["text"].(string); ok {
reasoning.WriteString(t)
}
}
}
}
return answer.String(), reasoning.String(), nil
case nil:
return "", "", nil
default:
return "", "", fmt.Errorf("mistral: unsupported content type %T", raw)
}
}
// ChatStreamlyWithSender sends messages and streams the response via the
// sender function. The Mistral SSE stream uses the same shape as OpenAI:
// "data:" lines carrying JSON events, with a final "[DONE]" line.

View File

@@ -572,3 +572,202 @@ func TestMistralEmbedRejectsHTTPError(t *testing.T) {
t.Errorf("expected Mistral embeddings API error, got %v", err)
}
}
// --- structured-content (magistral reasoning) tests ---
// Regression net: the existing string-content path stays green for every
// non-reasoning Mistral model.
func TestMistralChatHandlesStringContent(t *testing.T) {
srv := newMistralServer(t, "/chat/completions", func(t *testing.T, _ map[string]interface{}, w http.ResponseWriter) {
_ = json.NewEncoder(w).Encode(map[string]interface{}{
"choices": []map[string]interface{}{{"message": map[string]interface{}{
"role": "assistant",
"content": "Pong.",
}}},
})
})
defer srv.Close()
m := newMistralForTest(srv.URL)
apiKey := "test-key"
resp, err := m.ChatWithMessages("ministral-3b-latest",
[]Message{{Role: "user", Content: "ping"}},
&APIConfig{ApiKey: &apiKey}, nil)
if err != nil {
t.Fatalf("Chat: %v", err)
}
if *resp.Answer != "Pong." {
t.Errorf("Answer=%q want %q", *resp.Answer, "Pong.")
}
if *resp.ReasonContent != "" {
t.Errorf("ReasonContent=%q want empty", *resp.ReasonContent)
}
}
// New path: magistral with a non-trivial answer returns a structured
// content array. Two part types — `thinking` and `text` — must be
// concatenated into ReasonContent and Answer respectively.
//
// The fixture body is a trimmed copy of the actual response captured
// from api.mistral.ai/v1/chat/completions against magistral-medium-latest
// with the prompt "When do two trains meet?".
func TestMistralChatExtractsReasoningFromStructuredContent(t *testing.T) {
srv := newMistralServer(t, "/chat/completions", func(t *testing.T, _ map[string]interface{}, w http.ResponseWriter) {
_ = json.NewEncoder(w).Encode(map[string]interface{}{
"choices": []map[string]interface{}{{"message": map[string]interface{}{
"role": "assistant",
"content": []map[string]interface{}{
{
"type": "thinking",
"thinking": []map[string]interface{}{
{"type": "text", "text": "Combined speed is 150 mph. "},
{"type": "text", "text": "300 / 150 = 2 hours."},
},
"closed": true,
},
{"type": "text", "text": "They will meet after **2 hours**."},
},
}}},
})
})
defer srv.Close()
m := newMistralForTest(srv.URL)
apiKey := "test-key"
resp, err := m.ChatWithMessages("magistral-medium-latest",
[]Message{{Role: "user", Content: "When do they meet?"}},
&APIConfig{ApiKey: &apiKey}, nil)
if err != nil {
t.Fatalf("Chat: %v", err)
}
wantAnswer := "They will meet after **2 hours**."
wantReason := "Combined speed is 150 mph. 300 / 150 = 2 hours."
if *resp.Answer != wantAnswer {
t.Errorf("Answer=%q want %q", *resp.Answer, wantAnswer)
}
if *resp.ReasonContent != wantReason {
t.Errorf("ReasonContent=%q want %q", *resp.ReasonContent, wantReason)
}
}
// magistral with a trivial answer that needed no reasoning returns the
// structured shape with only a `text` part. ReasonContent must be empty.
func TestMistralChatHandlesStructuredContentWithoutThinking(t *testing.T) {
srv := newMistralServer(t, "/chat/completions", func(t *testing.T, _ map[string]interface{}, w http.ResponseWriter) {
_ = json.NewEncoder(w).Encode(map[string]interface{}{
"choices": []map[string]interface{}{{"message": map[string]interface{}{
"role": "assistant",
"content": []map[string]interface{}{
{"type": "text", "text": "12"},
},
}}},
})
})
defer srv.Close()
m := newMistralForTest(srv.URL)
apiKey := "test-key"
resp, err := m.ChatWithMessages("magistral-small-latest",
[]Message{{Role: "user", Content: "15% of 80?"}},
&APIConfig{ApiKey: &apiKey}, nil)
if err != nil {
t.Fatalf("Chat: %v", err)
}
if *resp.Answer != "12" {
t.Errorf("Answer=%q want %q", *resp.Answer, "12")
}
if *resp.ReasonContent != "" {
t.Errorf("ReasonContent=%q want empty (no thinking part)", *resp.ReasonContent)
}
}
// Unknown part types must be skipped, not crash the driver. This makes
// the parser forward-compatible with new Mistral content variants
// (audio chunks, citations, etc.) that ragflow doesn't surface yet.
func TestMistralChatIgnoresUnknownContentPartTypes(t *testing.T) {
srv := newMistralServer(t, "/chat/completions", func(t *testing.T, _ map[string]interface{}, w http.ResponseWriter) {
_ = json.NewEncoder(w).Encode(map[string]interface{}{
"choices": []map[string]interface{}{{"message": map[string]interface{}{
"role": "assistant",
"content": []map[string]interface{}{
{"type": "audio_url", "audio_url": "ignored://x"},
{"type": "text", "text": "Hello"},
{"type": "future_part_type", "blob": "?"},
},
}}},
})
})
defer srv.Close()
m := newMistralForTest(srv.URL)
apiKey := "test-key"
resp, err := m.ChatWithMessages("magistral-small-latest",
[]Message{{Role: "user", Content: "x"}},
&APIConfig{ApiKey: &apiKey}, nil)
if err != nil {
t.Fatalf("Chat: %v", err)
}
if *resp.Answer != "Hello" {
t.Errorf("Answer=%q want %q", *resp.Answer, "Hello")
}
}
// Direct unit coverage of the helper, including the nil and bad-type
// edge cases that won't surface in the integration tests above.
func TestExtractMistralContent(t *testing.T) {
tests := []struct {
name string
input interface{}
wantAns string
wantReason string
wantErr bool
}{
{"plain string", "hi", "hi", "", false},
{"empty string", "", "", "", false},
{"nil", nil, "", "", false},
{"empty array", []interface{}{}, "", "", false},
{
"text only",
[]interface{}{
map[string]interface{}{"type": "text", "text": "a"},
map[string]interface{}{"type": "text", "text": "b"},
},
"ab", "", false,
},
{
"thinking then text",
[]interface{}{
map[string]interface{}{
"type": "thinking",
"thinking": []interface{}{
map[string]interface{}{"type": "text", "text": "why "},
map[string]interface{}{"type": "text", "text": "this"},
},
},
map[string]interface{}{"type": "text", "text": "answer"},
},
"answer", "why this", false,
},
{"unknown root type", 42, "", "", true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
ans, reason, err := extractMistralContent(tc.input)
if tc.wantErr {
if err == nil {
t.Errorf("want error, got nil")
}
return
}
if err != nil {
t.Errorf("unexpected err: %v", err)
}
if ans != tc.wantAns {
t.Errorf("answer=%q want %q", ans, tc.wantAns)
}
if reason != tc.wantReason {
t.Errorf("reason=%q want %q", reason, tc.wantReason)
}
})
}
}