Skip to content

fix: make chat-template thinking configurable - #561

Merged
hedhoud merged 2 commits into
refactor/hexagonalfrom
fix/559-configurable-chat-template
Jun 24, 2026
Merged

fix: make chat-template thinking configurable#561
hedhoud merged 2 commits into
refactor/hexagonalfrom
fix/559-configurable-chat-template

Conversation

@hedhoud

@hedhoud hedhoud commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Why

Mistral-tokenizer deployments fail when OpenRAG sends Qwen-specific chat-template kwargs by default. Removing the option entirely is also a regression for deployments that rely on suppressing Qwen-style reasoning traces.

Change

This makes the thinking control opt-in across the affected LLM and VLM paths. Mistral deployments omit it by default, while Qwen-style deployments can enable the setting explicitly.

Fixes #559.

Verification

  • uv run --no-env-file pytest tests/unit -q
  • AUTH_TOKEN=test-admin-token OPENRAG_API_URL=http://localhost:8080 uv run --no-env-file pytest tests/integration/api -q --tb=short
  • uv run --no-env-file ruff check ...
  • uv run --no-env-file ruff format --check ...
  • git diff --check

Summary by CodeRabbit

  • New Features
    • Added optional “thinking” controls for LLM, VLM, and the OpenAI-compatible OCR loader.
    • These settings are now respected across chat, streaming, and image captioning request payloads.
  • Bug Fixes
    • Token counting and endpoint seeding no longer pass unsupported thinking parameters.
    • Environment overrides for the new options are consistently applied.
  • Documentation
    • Updated environment-variable docs and the example compose config with the new thinking toggles.
  • Tests
    • Added/expanded unit tests to verify request shaping and override behavior.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 445127da-8b9a-4571-9d06-e4f5c1bd54ad

📥 Commits

Reviewing files that changed from the base of the PR and between c3398ae and 29c932e.

📒 Files selected for processing (1)
  • openrag/services/workers/parsers/parser_dispatcher.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • openrag/services/workers/parsers/parser_dispatcher.py

📝 Walkthrough

Walkthrough

Adds optional enable_thinking config fields and environment-variable mappings for LLM, VLM, and OpenAI loader settings. The flag is threaded into vLLM request payloads, endpoint seeding, parser builders, and indexer fallback config, and removed before ChatOpenAI token counting.

Changes

enable_thinking configuration and propagation

Layer / File(s) Summary
Config fields and env mappings
openrag/core/config/endpoints.py, openrag/core/config/indexation.py, openrag/core/config/loader.py, conf/config.yaml
Adds enable_thinking to config models, maps the new env vars, and sets enable_thinking: null in YAML defaults.
VLLM request payload shaping
openrag/services/inference/vllm_client.py
Stores enable_thinking and moves it into chat_template_kwargs for chat, stream chat, and image caption requests.
Endpoint seeding and fallback config
openrag/services/orchestrators/model_endpoint_service.py, openrag/services/workers/indexer_pool.py
Conditionally adds enable_thinking to seeded endpoint extras and default LLM endpoint config.
Parser VLM builders
openrag/services/workers/parsers/parser_dispatcher.py
Threads enable_thinking through VLM builder construction for PDF and captioning paths.
Token counting kwargs cleanup
openrag/core/utils/text.py
Removes enable_thinking from ChatOpenAI kwargs before instantiation.
Docs, env example, and unit coverage
docs/content/docs/documentation/env_vars.md, infra/compose/.env.example, tests/unit/...
Updates env docs and examples, and adds tests for config overrides, payload shaping, seeding, parsing, and token-counting behavior.

Sequence Diagram(s)

sequenceDiagram
    participant Operator as Operator
    participant ConfigLoader as config loader
    participant Settings as Settings
    participant ModelEndpointService as model endpoint service
    participant VLLMClient as VLLMClient
    participant vLLMAPI as vLLM API

    Operator->>ConfigLoader: set enable_thinking in env / YAML
    ConfigLoader->>Settings: load config fields
    Settings->>ModelEndpointService: seed endpoint extras
    ModelEndpointService->>VLLMClient: pass enable_thinking
    VLLMClient->>VLLMClient: move flag into chat_template_kwargs
    VLLMClient->>vLLMAPI: POST /chat/completions
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • linagora/openrag#335: Both PRs touch the same chat_template_kwargs / enable_thinking request shaping.
  • linagora/openrag#524: Both PRs touch the global LLM fallback wiring in openrag/services/workers/indexer_pool.py.

Suggested reviewers

  • andyne13
  • EnjoyBacon7
  • Ahmath-Gadji

Poem

A bunny hops through config trees,
With enable_thinking set with ease.
Qwen can whisper, Mistral stay clear,
No hardcoded traces to fear 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.52% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately describes the main change: making chat-template thinking configurable.
Linked Issues check ✅ Passed The PR makes enable_thinking configurable and only injects it when set, matching the Mistral/Qwen fix requested in #559.
Out of Scope Changes check ✅ Passed The changes stay focused on configurable thinking flags, docs, env samples, and tests; no unrelated code paths are introduced.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/559-configurable-chat-template

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added the fix Fix issue label Jun 24, 2026
Comment thread openrag/services/workers/parsers/parser_dispatcher.py Outdated
The config models now define enable_thinking with a None default, so the
getattr fallback in the dispatcher can never trigger. Use direct attribute
access to match the surrounding sibling-field access.
@Ahmath-Gadji
Ahmath-Gadji force-pushed the fix/559-configurable-chat-template branch from dcdfdef to 29c932e Compare June 24, 2026 13:20

@Ahmath-Gadji Ahmath-Gadji 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.

LGTM.
I've tested it. Nothing is broken on my side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Fix issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants