Skip to content

fix(core): filter Mistral reasoning content at request boundary#3882

Merged
tanzhenxin merged 1 commit into
QwenLM:mainfrom
cyphercodes:fix/mistral-reasoning-content-filter
May 9, 2026
Merged

fix(core): filter Mistral reasoning content at request boundary#3882
tanzhenxin merged 1 commit into
QwenLM:mainfrom
cyphercodes:fix/mistral-reasoning-content-filter

Conversation

@cyphercodes

Copy link
Copy Markdown
Contributor

Summary

  • What changed: Mistral requests now get a provider-aware outbound copy of chat history with reasoning_content removed from messages before the request is sent.
  • Why it changed: strict Mistral-compatible endpoints reject that non-standard message field after a user switches from a thinking/reasoning provider, but the shared in-memory history still needs to preserve it for providers such as DeepSeek.
  • Reviewer focus: confirm this is scoped to Mistral detection (official base URL plus model-name fallback) and that the original history object is not mutated.

Validation

  • Commands run:
    npm ci --ignore-scripts
    cd packages/core && npx vitest run src/core/openaiContentGenerator/provider/mistral.test.ts
    cd packages/core && npx vitest run src/core/openaiContentGenerator/provider/mistral.test.ts src/core/openaiContentGenerator/provider/deepseek.test.ts src/core/openaiContentGenerator/provider/minimax.test.ts
    cd packages/core && npx vitest run src/core/openaiContentGenerator/provider
    npm run build
    npm run typecheck --workspace=packages/core
    npx prettier --check packages/core/src/core/openaiContentGenerator/index.ts packages/core/src/core/openaiContentGenerator/provider/index.ts packages/core/src/core/openaiContentGenerator/provider/mistral.ts packages/core/src/core/openaiContentGenerator/provider/mistral.test.ts
    npx eslint packages/core/src/core/openaiContentGenerator/index.ts packages/core/src/core/openaiContentGenerator/provider/index.ts packages/core/src/core/openaiContentGenerator/provider/mistral.ts packages/core/src/core/openaiContentGenerator/provider/mistral.test.ts
    git diff --check
  • Prompts / inputs used: provider-unit regression with prior assistant history containing reasoning_content, then an outbound Mistral request.
  • Expected result: Mistral-bound request messages omit reasoning_content; non-Mistral providers keep it; the source history still contains it.
  • Observed result: matched expected; provider test directory passes (116 tests).
  • Quickest reviewer verification path: cd packages/core && npx vitest run src/core/openaiContentGenerator/provider/mistral.test.ts
  • Evidence (output, logs, screenshots, video, JSON, before/after, etc.): local Vitest output showed src/core/openaiContentGenerator/provider passing with 116/116 tests.

Scope / Risk

  • Main risk or tradeoff: model-name fallback may catch custom Mistral-family aliases behind strict OpenAI-compatible proxies, which is intended for this compatibility quirk.
  • Not covered / not validated: live Mistral API smoke test with real credentials was not run.
  • Breaking changes / migration notes: none; this only changes the outbound request copy for Mistral-compatible providers.

Testing Matrix

🍏 🪟 🐧
npm run ⚠️ ⚠️
npx ⚠️ ⚠️
Docker ⚠️ ⚠️ ⚠️
Podman ⚠️ N/A N/A
Seatbelt ⚠️ N/A N/A

Testing matrix notes:

  • Verified locally on Linux only; no sandbox/container surface was changed.

Linked Issues / Bugs

Fixes #3304

@tanzhenxin tanzhenxin added the type/bug Something isn't working as expected label May 7, 2026

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No review findings. Downgraded from Approve to Comment: CI failing: Test (windows-latest, 22.x). — gpt-5.5 via Qwen Code /review

@tanzhenxin tanzhenxin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

The fix is correct on what it sets out to do: a Mistral-only outbound copy of the chat history with reasoning_content stripped from messages, while the shared in-memory history is preserved for providers that still want it. The hostname guard parses the URL and matches api.mistral.ai exactly or as a dot-prefixed subdomain (nice — the api.mistral.ai.evil.example test pins this), the source-history objects are not mutated, and Mistral being last in determineProvider keeps OpenRouter / ModelScope / DashScope aliases routing to their existing providers. Two non-blocking notes for follow-ups:

1. Top-level reasoning body parameter is not stripped (severity: low · confidence: high)

When contentGeneratorConfig.reasoning is set, the pipeline injects a top-level reasoning field into every request, and the Mistral provider's buildRequest only filters messages[].reasoning_content. So a user with reasoning configured at the content-generator level who then points at api.mistral.ai is likely to hit the same kind of 400/422 this PR is meant to fix. Acknowledged as out of scope; a tracking issue would help.

2. magistral is on the marker list but is Mistral's reasoning model (severity: low · confidence: medium)

Magistral currently delivers the chain-of-thought inside <think> tags in content rather than the reasoning_content field, so today the strip is a harmless no-op for that family. If a future Magistral release moves the trace to reasoning_content (the de-facto OpenAI-compatible carrier), this strip would silently drop it on the wire and break multi-turn reasoning. Worth either a FIXME pointing at the assumption, or dropping magistral from the marker list and relying on the hostname check for that family.

Verdict

APPROVE — clean fix for the actual bug; the two notes above are forward-looking, not blocking.

@tanzhenxin tanzhenxin merged commit 7910a5b into QwenLM:main May 9, 2026
12 of 13 checks passed
TaimoorSiddiquiOfficial pushed a commit to TaimoorSiddiquiOfficial/HopCode that referenced this pull request May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Switching models mid-session causes API failures.

3 participants