fix: ambiguous model id error #4306

elianiva · 2025-06-03T23:10:11Z

Related GitHub Issue

Closes: #2577

Description

Previously the model id error displayed is a bit ambiguous since it is placed below the provider selector, making users confused which input is actually needs correction. This PR puts the error message where it is more relevant.

There was also an issue where if you change the provider, the invalid one will still get selected. This PR makes it so that invalid values get reset into empty.

Test Procedure

Added some unit tests and tested it manually.

Type of Change

🐛 Bug Fix: Non-breaking change that fixes an issue.
✨ New Feature: Non-breaking change that adds functionality.
💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
📚 Documentation: Updates to documentation files.
⚙️ Build/CI: Changes to the build process or CI configuration.
🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

Screenshots / Videos

Before:

After:

Important

Improves error message placement and handling of invalid model selections in model configuration components.

Behavior:
- Moves error message in ModelPicker to be more contextually relevant.
- Resets invalid model selections to empty when provider changes in ApiOptions.
Components:
- Updates ModelPicker to display error messages below the model selector.
- Modifies ApiOptions to use validateApiConfigurationExcludingModelErrors() and getModelValidationError().
- Updates provider components (Glama.tsx, LiteLLM.tsx, OpenAICompatible.tsx, OpenRouter.tsx, Requesty.tsx, Unbound.tsx) to pass modelValidationError to ModelPicker.
Tests:
- Adds tests in ModelPicker.test.tsx for error message display.
- Adds tests in validate.test.ts for new validation functions.
Validation:
- Adds getModelValidationError() and validateApiConfigurationExcludingModelErrors() in validate.ts to separate model-specific errors from general API configuration errors.

^{This description was created by}^{for e27109e. You can customize this summary. It will automatically update as commits are pushed.}

webview-ui/src/components/settings/ModelPicker.tsx

webview-ui/src/components/settings/ApiOptions.tsx

elianiva · 2025-06-04T01:04:43Z

will fix lint errors in a bit, not in front of computer rn, somehow it didn't catch on earlier on my machine

elianiva · 2025-06-04T11:20:04Z

brought back glama since #4269 is closed, this PR should be ready to review

daniel-lxs

Hey @elianiva, Thank you for taking this issue! Sorry that it took so long to review your contribution!

I reviewed your PR and left some questions, I would like to know what you think.

I also tested the implementation and I see that while a default model is selected when I enter a non-existent model, the model I entered is persisted while the default is not:

2025-06-07.09-54-34.mp4

Might be worth taking a look.

.roomodes

webview-ui/src/components/settings/ApiErrorMessage.tsx

webview-ui/src/utils/validate.ts

webview-ui/src/components/settings/ApiOptions.tsx

webview-ui/src/components/settings/providers/OpenRouter.tsx

elianiva · 2025-06-10T16:54:41Z

i've addressed the comments, i ended up removing the (invalid) suffix since it is redundant when we already have the error message, less clutter is better

elianiva · 2025-06-14T15:58:33Z

Just a heads up, I've addressed all the change requests, so this PR should be labeled needs review instead 🙂

daniel-lxs · 2025-06-14T19:12:00Z

Hey @elianiva, I might be doing something wrong here but the invalid model is not resetting when switching providers.

2025-06-14_14-10-20.mp4

Is this intended?

elianiva · 2025-06-14T23:17:59Z

Hey @elianiva, I might be doing something wrong here but the invalid model is not resetting when switching providers.
2025-06-14_14-10-20.mp4

Is this intended?

Yes, I decided to remove it based on your previous feedback. I thought it was a good idea at first, but then decided to let it be invalid so the users know and then fix it themselves. It's better to have this behaviour rather than implicitly reset the model. That behaviour might leave users questioning "why this model got reset" without further explanation.

daniel-lxs

LGTM

cte · 2025-06-17T18:30:30Z

webview-ui/src/components/settings/ApiOptions.tsx

 					}
-					break
+				>
+			> = {


I'd probably pull this out into a constant and strengthen the typing using this: https://github.com/RooCodeInc/Roo-Code/blob/main/packages/types/src/provider-settings.ts#L263

cte

LGTM - thanks!

elianiva requested review from cte and mrubens as code owners June 3, 2025 23:10

github-project-automation bot added this to Roo Code Roadmap and Roo Code Roadmap Jun 3, 2025

github-project-automation bot moved this to New in Roo Code Roadmap Jun 3, 2025

github-project-automation bot moved this to Triage in Roo Code Roadmap Jun 3, 2025

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jun 3, 2025

ellipsis-dev bot reviewed Jun 3, 2025

View reviewed changes

webview-ui/src/components/settings/ModelPicker.tsx Outdated Show resolved Hide resolved

adamhill reviewed Jun 3, 2025

View reviewed changes

webview-ui/src/components/settings/ApiOptions.tsx Outdated Show resolved Hide resolved

elianiva force-pushed the fix/ambiguous-model-id-error branch from 871637c to 056eca6 Compare June 3, 2025 23:31

hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 3, 2025

daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 4, 2025

hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jun 5, 2025

daniel-lxs reviewed Jun 7, 2025

View reviewed changes

daniel-lxs moved this from PR [Needs Prelim Review] to PR [Draft / In Progress] in Roo Code Roadmap Jun 7, 2025

daniel-lxs marked this pull request as draft June 7, 2025 15:00

daniel-lxs added PR - Draft / In Progress and removed PR - Needs Preliminary Review labels Jun 7, 2025

elianiva added 7 commits June 10, 2025 14:51

fix: show model id error below model selector

fef46c9

test: validate models, model picker

13ecf4c

fix: set default model id when we have errors

6dfcdd3

chore: remove glama

1edefd4

test: use data-testid for querying

c799239

chore: update test mode to prioritize data-testid

7d538f4

chore: bring back glama

004cb26

elianiva force-pushed the fix/ambiguous-model-id-error branch from c8e1dce to 004cb26 Compare June 10, 2025 07:52

elianiva added 3 commits June 10, 2025 22:39

refactor: remove unnecessary switch cases

32e46de

fix: i18n error messages

7d9075e

refactor: remove invalid label, redundant

836a519

elianiva force-pushed the fix/ambiguous-model-id-error branch from 89faa67 to 836a519 Compare June 10, 2025 16:51

elianiva mentioned this pull request Jun 10, 2025

chore: add data-testid rule #4505

Merged

23 tasks

hannesrudolph added PR - Changes Requested and removed PR - Draft / In Progress labels Jun 11, 2025

daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jun 14, 2025

hannesrudolph added PR - Needs Preliminary Review and removed PR - Changes Requested labels Jun 14, 2025

daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jun 14, 2025

hannesrudolph added PR - Changes Requested and removed PR - Needs Preliminary Review labels Jun 14, 2025

daniel-lxs moved this from PR [Changes Requested] to PR [Needs Review] in Roo Code Roadmap Jun 14, 2025

daniel-lxs approved these changes Jun 14, 2025

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 14, 2025

hannesrudolph added PR - Needs Review and removed PR - Changes Requested labels Jun 15, 2025

cte reviewed Jun 17, 2025

View reviewed changes

cte approved these changes Jun 17, 2025

View reviewed changes

cte merged commit b8505fe into RooCodeInc:main Jun 17, 2025
25 checks passed

github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 17, 2025

github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 17, 2025

elianiva deleted the fix/ambiguous-model-id-error branch June 18, 2025 03:52

valekseev pushed a commit to valekseev/Roo-Code that referenced this pull request Jun 18, 2025

fix: ambiguous model id error (RooCodeInc#4306)

c99e8a2

cte pushed a commit that referenced this pull request Jun 24, 2025

fix: ambiguous model id error (#4306)

5bfe896

fix: ambiguous model id error #4306

fix: ambiguous model id error #4306

Uh oh!

Conversation

elianiva commented Jun 3, 2025 • edited by ellipsis-dev bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related GitHub Issue

Description

Test Procedure

Type of Change

Pre-Submission Checklist

Screenshots / Videos

Uh oh!

Uh oh!

Uh oh!

elianiva commented Jun 4, 2025

Uh oh!

elianiva commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daniel-lxs left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elianiva commented Jun 10, 2025

Uh oh!

elianiva commented Jun 14, 2025

Uh oh!

daniel-lxs commented Jun 14, 2025

Uh oh!

elianiva commented Jun 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daniel-lxs left a comment

Choose a reason for hiding this comment

Uh oh!

cte Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

cte left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

elianiva commented Jun 3, 2025 •

edited by ellipsis-dev bot

Loading

elianiva commented Jun 4, 2025 •

edited

Loading

daniel-lxs left a comment •

edited

Loading

elianiva commented Jun 14, 2025 •

edited

Loading