Skip to content

Conversation

@OneZero-Y
Copy link
Contributor

What type of PR is this?

Fix - Resolves hardcoded values and improves configurability

What this PR does / why we need it:

This PR makes reasoning effort levels configurable instead of hardcoded, addressing a TODO comment in the codebase and improving system flexibility.
Changes:

  1. Removes hardcoded reasoning effort: Previously hardcoded to "high", now configurable per category
  2. Adds global default configuration: default_reasoning_effort in config.yaml for fallback behavior
  3. Adds category-specific configuration: reasoning_effort field for fine-grained control per category
  4. Maintains backward compatibility: Falls back to sensible defaults when configuration is missing
  5. Updates function signatures: Enhanced setReasoningModeToRequestBody to accept category context

Configuration Examples:

# Global default
default_reasoning_effort: medium
# Category-specific settings  
categories:
- name: math
  reasoning_effort: high    # Math problems need intensive reasoning
- name: business  
  reasoning_effort: low     # Business conversations need light reasoning

Which issue(s) this PR fixes:

Fixes hardcoded reasoning effort levels (addresses TODO in reason_mode_selector.go:94)

Release Notes: Yes/No

@github-actions
Copy link

github-actions bot commented Sep 2, 2025

👥 vLLM Semantic Team Notification

The following members have been identified for the changed files in this PR and have been automatically assigned:

📁 config

Owners: @rootfs
Files changed:

  • config/config.yaml

📁 src

Owners: @rootfs, @Xunzhuo, @wangchen615
Files changed:

  • src/semantic-router/pkg/config/config.go
  • src/semantic-router/pkg/extproc/reason_mode_config_test.go
  • src/semantic-router/pkg/extproc/reason_mode_selector.go
  • src/semantic-router/pkg/extproc/reasoning_integration_test.go
  • src/semantic-router/pkg/extproc/request_handler.go

This comment was automatically generated based on the OWNER files in the repository.

@netlify
Copy link

netlify bot commented Sep 2, 2025

Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit 2bdfc9d
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/68b7002050f469000840cfa5
😎 Deploy Preview https://deploy-preview-21--vllm-semantic-router.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@rootfs
Copy link
Collaborator

rootfs commented Sep 2, 2025

@OneZero-Y can you run make test to fix the test failure?

rootfs
rootfs previously approved these changes Sep 2, 2025
@Xunzhuo Xunzhuo dismissed rootfs’s stale review September 2, 2025 12:55

The merge-base changed after approval.

@Xunzhuo Xunzhuo force-pushed the main branch 2 times, most recently from fd7623d to 7b4eb2c Compare September 2, 2025 12:55
@rootfs
Copy link
Collaborator

rootfs commented Sep 2, 2025

test failed

2025/09/02 12:50:29 No category determined for query, defaulting to no reasoning mode
--- FAIL: TestReasoningModeIntegration (0.00s)
    --- PASS: TestReasoningModeIntegration/Math_query_enables_reasoning (0.00s)
    --- PASS: TestReasoningModeIntegration/Business_query_disables_reasoning (0.00s)
    --- FAIL: TestReasoningModeIntegration/addReasoningModeToRequestBody_adds_correct_fields (0.00s)
    --- FAIL: TestReasoningModeIntegration/getChatTemplateKwargs_returns_correct_values (0.00s)
    --- PASS: TestReasoningModeIntegration/Empty_query_defaults_to_no_reasoning (0.00s)
    --- PASS: TestReasoningModeIntegration/Unknown_category_defaults_to_no_reasoning (0.00s)

@OneZero-Y OneZero-Y force-pushed the fix/configurable-reasoning-effort branch from 8203687 to 2bdfc9d Compare September 2, 2025 14:33
@OneZero-Y OneZero-Y requested a review from rootfs September 2, 2025 14:51
@rootfs
Copy link
Collaborator

rootfs commented Sep 2, 2025

The config is currently manual, will have an automated process (to be listed in roadmap).
Test is green, merging now.

Thanks for contributing!

@rootfs rootfs merged commit 4b45157 into vllm-project:main Sep 2, 2025
9 checks passed
@OneZero-Y OneZero-Y deleted the fix/configurable-reasoning-effort branch September 4, 2025 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants