Skip to content

Conversation

mrubens
Copy link
Collaborator

@mrubens mrubens commented Jul 16, 2025

This fixes an issue where every time you open provider settings with an OpenRouter model selected you need to "discard changes" before saving, even though you didn't change anything.


Important

Fixes issue in SettingsView.tsx where initial sync of model ID was incorrectly marked as a change, requiring unnecessary discard action.

  • Behavior:
    • Fixes issue in SettingsView.tsx where opening provider settings with an OpenRouter model required discarding changes before saving, even if no changes were made.
    • Prevents marking initial sync from undefined to defined value as a change in setApiConfigurationField.
  • Components:
    • Updates useEffect in ApiOptions.tsx to include apiConfiguration.apiModelId in dependency array to prevent unnecessary updates.

This description was created by Ellipsis for 405c7b5. You can customize this summary. It will automatically update as commits are pushed.

@mrubens mrubens requested review from cte and jr as code owners July 16, 2025 14:38
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Jul 16, 2025
Copy link

delve-auditor bot commented Jul 16, 2025

No security or compliance issues detected. Reviewed everything up to 405c7b5.

Security Overview
  • 🔎 Scanned files: 2 changed file(s)
Detected Code Changes
Change Type Relevant files
Bug Fix ► ApiOptions.tsx
    Fix model selection state update condition
► SettingsView.tsx
    Improve settings dirty check logic

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Jul 16, 2025
Copy link
Collaborator

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

For future consideration: I think we should look into the useEffect in webview-ui/src/components/settings/ApiOptions.tsx (lines 169-173) that syncs all providers' models to apiModelId. Many providers use their own fields:

  • OpenRouter → openRouterModelId
  • Glama → glamaModelId
  • OpenAI → openAiModelId
  • etc.

This might be causing unnecessary updates when these providers are selected. It could be worth exploring a more targeted approach that checks the provider type and only syncs for those that actually use apiModelId (anthropic, gemini, deepseek, etc.).

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 16, 2025
@mrubens mrubens merged commit 0f994fc into main Jul 16, 2025
26 checks passed
@mrubens mrubens deleted the fix_settings_dirty_check branch July 16, 2025 14:57
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jul 16, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 16, 2025
fxcl added a commit to tameslabs/Roo-Cline that referenced this pull request Jul 16, 2025
* main:
  fix: Resolve confusing auto-approve checkbox states (RooCodeInc#5602)
  fix: prevent empty mode names from being saved (RooCodeInc#5766) (RooCodeInc#5794)
  Format time in ISO 8601 (RooCodeInc#5793)
  fix: resolve DirectoryScanner memory leak and improve file limit handling (RooCodeInc#5785)
  Fix settings dirty check (RooCodeInc#5779)
  feat: increase Ollama API timeout values and extract as constants (RooCodeInc#5778)
  fix: Exclude Terraform and Terragrunt cache directories from checkpoints (RooCodeInc#4601) (RooCodeInc#5750)
  Move less commonly used provider settings into an advanced dropdown (RooCodeInc#5762)
  feat: Add configurable error & repetition limit with unified control (RooCodeInc#5654) (RooCodeInc#5752)
  list-files must include at least the first-level directory contents (RooCodeInc#5303)
  Update evals repo link (RooCodeInc#5758)
  Feature/vertex ai model name conversion (RooCodeInc#5728)
  fix(litellm): handle baseurl with paths correctly (RooCodeInc#5697)
  Add telemetry for todos (RooCodeInc#5746)
  feat: add undo functionality for enhance prompt feature (fixes RooCodeInc#5741) (RooCodeInc#5742)
  Fix max_tokens limit for moonshotai/kimi-k2-instruct on Groq (RooCodeInc#5740)
  Changeset version bump (RooCodeInc#5735)
  Add changeset for v3.23.12 patch release (RooCodeInc#5734)
  Update the max-token calculation in model-params to use the shared logic (RooCodeInc#5720)
  Changeset version bump (RooCodeInc#5719)
  chore: add changeset for v3.23.11 patch release (RooCodeInc#5718)
  Add Kimi K2 model and better support (RooCodeInc#5717)
  Fix: Remove invalid skip-checkout parameter from GitHub Actions workflows (RooCodeInc#5676)
  feat: add Cmd+Shift+. keyboard shortcut for previous mode switching (RooCodeInc#5695)
  Changeset version bump (RooCodeInc#5708)
  chore: add changeset for v3.23.10 patch release (RooCodeInc#5707)
  Add padding to the index model options (RooCodeInc#5706)
  fix: prioritize built-in model dimensions over custom dimensions (RooCodeInc#5705)
  Update CHANGELOG.md
  Changeset version bump (RooCodeInc#5702)
  chore: add changeset for v3.23.9 patch release (RooCodeInc#5701)
  Tweaks to command timeout error (RooCodeInc#5700)
  Update contributors list (RooCodeInc#5639)
  feat: enable Claude Code provider to run natively on Windows (RooCodeInc#5615)
  feat: Add configurable timeout for command execution (RooCodeInc#5668)
  feat: add gemini-embedding-001 model to code-index service (RooCodeInc#5698)
  fix: resolve vector dimension mismatch error when switching embedding models (RooCodeInc#5616) (RooCodeInc#5617)
  fix: [5424] return the cwd in the exec tool's response so that the model is not lost after subsequent calls (RooCodeInc#5667)
  Changeset version bump (RooCodeInc#5670)
  chore: add changeset for v3.23.8 patch release (RooCodeInc#5669)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:S This PR changes 10-29 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants