fix(openai-compatible): consistently apply configured reasoning effort - #1604
Conversation
Use the custom model effort selected in settings instead of stale top-level values, and propagate it consistently across streaming, non-streaming, O-series, and single-completion requests while respecting explicit disable. Keep inherited DeepSeek completions from sending V4 reasoning fields for unknown model IDs. Add regression coverage for request paths and support boundaries. Signed-off-by: JunyongParkDev <jun94.park@samsung.com>
Verify that the OpenAI-compatible selector updates custom model metadata, unsaved edits survive live state refreshes, and Save sends the cached value. Cover profile reload and storage-to-webview round trips with reasoning enabled, disabled, and unset while a stale top-level effort remains. Signed-off-by: JunyongParkDev <jun94.park@samsung.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📜 Recent review details
|
| Layer / File(s) | Summary |
|---|---|
Provider model resolution and requests src/api/providers/openai.ts, src/api/providers/deepseek.ts, src/api/providers/__tests__/* |
OpenAI requests use reasoning parameters resolved from custom model metadata. DeepSeek exposes reasoning metadata only for supported models. Tests cover streaming, non-streaming, completion, and custom-model cases. |
Provider settings persistence src/core/config/__tests__/ProviderSettingsManager.spec.ts, src/core/webview/__tests__/ClineProvider.spec.ts |
Tests verify that reasoning settings and custom model metadata survive save, reload, and webview state updates. |
Reasoning effort editing and unsaved state webview-ui/src/components/settings/__tests__/ApiOptions.spec.tsx, webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx |
Tests verify model-specific effort options, custom "max" selection, and preservation of edits until Save. |
Priority: ➖ Normal
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant SettingsView
participant ApiOptions
participant ProviderSettings
SettingsView->>ApiOptions: edit reasoning effort
ApiOptions->>ProviderSettings: cache enablement and custom effort
SettingsView->>ProviderSettings: save configuration
ProviderSettings-->>SettingsView: return persisted settings
Merge Risk: ⚪ Minimal · up to 426a1
The reasoning-effort propagation and settings-state changes have no identified merge-blocking risk.
🚥 Pre-merge checks | ✅ 8
✅ Passed checks (8 passed)
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check | ✅ Passed | Issue #993 requires the selected custom reasoning effort to reach local OpenAI-compatible requests instead of a stale top-level value. OpenAiHandler.getModel() resolves reasoningEffort from `openA… |
| Out of Scope Changes check | ✅ Passed | The changes stay within Issue #993. They update OpenAI-compatible request construction, preserve resolver and storage behavior, and add regression coverage for request construction and settings persis… |
| Regression Evidence | ✅ Passed | Focused coverage is present at the provider and settings layers. openai.spec.ts asserts SDK request bodies for streaming and non-streaming chat, both O3 paths, and completePrompt, including custom… |
| Security Boundaries | ✅ Passed | PASS. The two changed production files only adjust reasoning request construction. OpenAI spreads the resolver's literal reasoning payload into requests, and the reasoning values are constrained by … |
| Persistence Integrity | ✅ Passed | No changed persistence path exists. The only non-test changes are in src/api/providers/openai.ts and src/api/providers/deepseek.ts, and they change request parameter construction. The persistence … |
| Lifecycle Resource Cleanup | ✅ Passed | PASS. The changed production paths only compute model metadata and spread reasoning parameters into existing OpenAI/DeepSeek request objects. The diff adds no listener, watcher, timer, task, provider … |
| Title check | ✅ Passed | The title clearly identifies the main change: consistent application of configured reasoning effort for OpenAI-compatible providers. |
| Description check | ✅ Passed | The description follows the required template and provides linked issues, implementation details, test procedures, test results, checklist status, snapshot and documentation notes. The Get in Touch se… |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
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.
Comment @coderabbitai help to get the list of available commands.
Review statusThanks for contributing. This comment tracks the review sequence and the next action. Current step: The required review sequence passed. Remaining merge requirements apply. Review-state labels are managed by this workflow; do not edit them manually. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Related GitHub Issue
Closes: #993 #1524
Description
OpenAI Compatible could display a selected reasoning effort such as
maxwhile sending a stale top-level value such aslowormedium. The settings UI stores its selection inopenAiCustomModelInfo.reasoningEffort, but request construction previously gave the shared top-levelreasoningEffortpriority.This improves provider reliability without changing the settings UI, storage format, or the shared reasoning resolver's precedence for other providers.
Test Procedure
Validated locally on macOS with Node.js
22.23.1and pnpm10.8.1.The request tests inspect the arguments passed to the OpenAI SDK. They cover conflicting settings (
maxversuslow, andhigh/xhighversusmedium), stalenone/disablevalues, explicit disable, an unset enable flag, and missing custom effort across all five request paths. DeepSeek tests also verify omission for unknown models and disabled reasoning, andmaxfor a supported model.Settings tests verify that selecting
maxupdates custom model metadata, unsaved edits survive a live state refresh, Save sends the cached value, and persistence preserves the settings for enabled, disabled, and unset states.After installing dependencies, reproduce the full package checks from the repository root. Finish bundling before running the test suites:
Results:
Coverage from the five provider suites is 95.40% lines / 88.19% branches for
openai.tsand 100% lines / 84.37% branches fordeepseek.ts. The added DeepSeek condition is exercised in both directions. The core OpenAI request tests also detected all five independently reverted fix variants.Pre-Submission Checklist
Visual Snapshots
Not applicable. The UI changes in this PR are limited to tests.
Videos (interaction / animation only)
Not applicable. No new interaction or animation is introduced.
Documentation Updates
Additional Notes
Get in Touch