Skip to content

feat(vlm): add configurable enable_thinking flag#1514

Open
yeyitech wants to merge 2 commits intovolcengine:mainfrom
yeyitech:feat/issue-983-enable-thinking-config
Open

feat(vlm): add configurable enable_thinking flag#1514
yeyitech wants to merge 2 commits intovolcengine:mainfrom
yeyitech:feat/issue-983-enable-thinking-config

Conversation

@yeyitech
Copy link
Copy Markdown
Contributor

@yeyitech yeyitech commented Apr 17, 2026

Summary

  • add tri-state vlm.enable_thinking config so callers can force, suppress, or defer backend-specific extra_body.enable_thinking
  • share the resolution logic across OpenAI-compatible and LiteLLM VLM backends
  • document the new config and add focused config/backend tests

Closes #983

Testing

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 pytest -o addopts='' -q tests/unit/test_vlm_thinking_param.py tests/unit/test_extra_headers_vlm.py::TestVLMExtraHeaders::test_dashscope_text_completion_passes_enable_thinking_in_extra_body tests/unit/test_extra_headers_vlm.py::TestVLMExtraHeaders::test_official_openai_text_completion_does_not_set_enable_thinking tests/unit/test_extra_headers_vlm.py::TestVLMExtraHeaders::test_explicit_enable_thinking_forces_extra_body_on_openai_backend tests/unit/test_extra_headers_vlm.py::TestVLMExtraHeaders::test_azure_text_completion_does_not_set_enable_thinking tests/unit/test_extra_headers_vlm.py::TestVLMExtraHeaders::test_explicit_disable_thinking_suppresses_dashscope_extra_body tests/misc/test_config_validation.py

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

983 - Partially compliant

Compliant requirements:

  • Added configurable enable_thinking flag in VLM config
  • Implemented tri-state logic (force, suppress, auto) shared across backends
  • Updated OpenAI and LiteLLM VLM backends to use the new config
  • Added tests for the new functionality
  • Updated English and Chinese documentation

Non-compliant requirements:

  • Japanese documentation not updated
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 85
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Tri-state Logic Clarification

The current _resolve_enable_thinking_value treats enable_thinking: false as "suppress the field entirely". However, some models (like GLM-5) may require explicitly sending enable_thinking: false (not omitting it) to disable reasoning. There is no way to explicitly set the field to false with the current config.

def _resolve_enable_thinking_value(
    self, thinking: bool, auto_supported: bool
) -> Optional[bool]:
    """Return the enable_thinking payload value, or None to omit the field."""
    if self.enable_thinking is True:
        return bool(thinking)
    if self.enable_thinking is False:
        return None
    if auto_supported:
        return bool(thinking)
    return None

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ yeyitech
❌ Codex


Codex seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

Support enable_thinking config for reasoning models (GLM-5, etc.)

2 participants