Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Dec 8, 2025

Summary

Consolidate reasoning effort UI components and fix the "disable" option to properly override model defaults.

Changes

UI Component Consolidation

  • Merge SimpleThinkingBudget into ThinkingBudget component
  • Add "none" option to reasoning effort dropdown for non-required models
  • Remove special case for Roo provider in ApiOptions.tsx

Bug Fix: "disable" Reasoning Effort

  • Fix model-params.ts to not fallback to model default when user explicitly selects "disable"
  • When customReasoningEffort is set (including "disable"), use it directly instead of falling back

Test Updates

  • Update tests to reflect component consolidation
  • Remove redundant assertions
  • Improve test descriptions

Files Changed

  • webview-ui/src/components/settings/ThinkingBudget.tsx - Enhanced with "none" option
  • webview-ui/src/components/settings/ApiOptions.tsx - Simplified rendering
  • webview-ui/src/components/settings/SimpleThinkingBudget.tsx - Deleted
  • src/api/transform/model-params.ts - Fix disable fallback logic
  • Test files updated accordingly

…atures

- Enforce explicit supportsReasoningEffort capability flag for models to use reasoning effort
- Having a reasoningEffort default property alone is no longer sufficient
- Remove SimpleThinkingBudget component, consolidate into ThinkingBudget with 'none' option
- Fix model-params to not fallback to model default when customReasoningEffort is 'disable'
- Remove defaultTemperature from Roo provider config
- Update all related tests for capability-based behavior
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Dec 8, 2025
@roomote
Copy link
Contributor

roomote bot commented Dec 8, 2025

Oroocle Clock   See task on Roo Cloud

Re-review complete for the latest commit (a74cd6e). No new issues were found in the updated reasoning-effort UI.

  • Review latest Roo provider and reasoning-effort changes
  • Verify tests and capability-based behavior for reasoning effort
  • Confirm default temperature handling for Roo provider
  • No issues requiring code changes in this PR
Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Dec 8, 2025
- Restore shouldUseReasoningEffort fallback logic for models with
  reasoningEffort but no supportsReasoningEffort (fixes fixed-effort
  model variants like o3-high, o3-low, o4-mini-high, etc.)
- Restore defaultTemperature: 0.7 for Roo provider
- Update tests to reflect correct behavior
@hannesrudolph hannesrudolph changed the title refactor: require supportsReasoningEffort capability for reasoning features refactor: consolidate ThinkingBudget components and fix disable handling Dec 9, 2025
- 'disable' turns off reasoning entirely (enableReasoningEffort=false)
- 'none' is a valid reasoning level that sends reasoning with value 'none'
- Both display as 'None' in the UI but behave differently
- Add 'disable' as synthetic option when reasoning is not required
- Handle 'none' from capability arrays as legitimate reasoning level
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 9, 2025
@mrubens mrubens merged commit de00ab1 into main Dec 9, 2025
13 checks passed
@mrubens mrubens deleted the refactor/require-reasoning-effort-capability branch December 9, 2025 01:23
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Dec 9, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants