feat(platform): add auto model selection and expand error categories#1441
Merged
Conversation
Add "Auto" option to model selector that picks the best available model with fallback, replacing the previous governance-default logic. Model overrides now expire after 24 hours to prevent stale selections. Expand chat error sanitization with auth, model-not-found, and provider error categories. Known error types no longer expose rawMessage to avoid leaking stack traces to the UI.
…#1432) Add pattern to sanitize-chat-error so missing auth header errors show a user-friendly message instead of raw stack traces.
📝 WalkthroughWalkthroughThis PR introduces automatic model selection mode in the chat feature by adding an Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
services/platform/app/features/chat/utils/sanitize-chat-error.ts (1)
25-58:⚠️ Potential issue | 🟠 MajorBare 401/403/404 matches now mask tool failures.
These status-code patterns run before
toolFailure, so errors likeTool error: upstream returned 404orTool error: 401 unauthorizedwill be shown as auth/model issues instead of tool failures. Tighten the auth/model regexes or movetoolFailureahead of the raw status-code rules.💡 Minimal fix
{ + pattern: /tool.*error|tool.*fail|unable to complete/i, + category: 'toolFailure', + }, + { pattern: /\b401\b|\b403\b|invalid.*key|expired.*key|api.?key.*invalid|unauthorized|forbidden|authentication.*fail|user not found|missing.*authentication/i, category: 'authError', }, { pattern: /model.*not found|model.*not available|invalid model|\b404\b/i, category: 'modelNotFound', }, @@ - { - pattern: /tool.*error|tool.*fail|unable to complete/i, - category: 'toolFailure', - },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/chat/utils/sanitize-chat-error.ts` around lines 25 - 58, The current regex order in sanitize-chat-error.ts causes bare status-code rules (authError and modelNotFound) to match messages coming from tools, masking tool failures; update the rules so the object with category 'toolFailure' is evaluated before the status-code-based rules (the entries with category 'authError' and 'modelNotFound'), or alternatively tighten those regexes to avoid matching when the message contains tool context (e.g., require word boundaries plus absence of "tool" or "upstream" so expressions like "Tool error: 404" don't match auth/model patterns); locate the pattern array in sanitize-chat-error.ts and either move the { category: 'toolFailure', pattern: /tool.*error|.../ } entry above the authError/modelNotFound objects or constrain the auth/model regexes accordingly.services/platform/app/features/chat/components/model-selector.tsx (1)
141-170:⚠️ Potential issue | 🟠 MajorDon't hide the
Autostate when only one concrete model is available.After adding
autoOption,filteredModels.length <= 1no longer means there is nothing to choose. In the current flow, a user who previously pinned the lone accessible model can never switch back to Auto, because the component falls through to the read-only branch before the new option is usable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/chat/components/model-selector.tsx` around lines 141 - 170, Move creation of autoOption and modelOptions (the autoOption constant and the modelOptions mapping using modelInfoMap and getDisplayName) before the single-model read-only check and change the guard from checking filteredModels.length <= 1 to checking the total options length (e.g. compute options = [autoOption, ...modelOptions] first and then if options.length <= 1 return the read-only span). This ensures AUTO_MODEL (AUTO_MODEL / autoOption) is considered when deciding whether to render the select so a user can switch back to Auto even when there is one concrete model.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/app/features/chat/context/chat-layout-context.tsx`:
- Around line 100-118: The current selectedModelOverrides memo drops legacy
string entries (Record<string,string>) which wipes users' saved selections;
update the logic that reads rawModelOverrides (the useMemo for
selectedModelOverrides) to detect string values, convert them into
ModelOverrideEntry objects with a fresh expiresAt (e.g. Date.now() +
24*60*60*1000) and persist the migration back via setRawModelOverrides so legacy
entries are upgraded once; keep the existing expiry check for ModelOverrideEntry
values and ensure you reference rawModelOverrides, selectedModelOverrides,
setRawModelOverrides and modelOverridesKey when implementing the one-time
migration.
- Around line 104-118: The memoized selectedModelOverrides uses Date.now() but
only recalculates when rawModelOverrides changes, so expirations won't take
effect over time; update the logic to re-evaluate periodically by introducing a
time tick into the dependency list (e.g., a state like now updated via a
useEffect setInterval) or by using a timeout that triggers a state update at the
nearest expiresAt, then reference that state in the useMemo dependency array so
selectedModelOverrides (and any consumers) recompute when entries actually
expire; touch symbols: selectedModelOverrides, rawModelOverrides, useMemo,
Date.now(), and add a useEffect/setInterval or scheduled timeout to drive
updates.
---
Outside diff comments:
In `@services/platform/app/features/chat/components/model-selector.tsx`:
- Around line 141-170: Move creation of autoOption and modelOptions (the
autoOption constant and the modelOptions mapping using modelInfoMap and
getDisplayName) before the single-model read-only check and change the guard
from checking filteredModels.length <= 1 to checking the total options length
(e.g. compute options = [autoOption, ...modelOptions] first and then if
options.length <= 1 return the read-only span). This ensures AUTO_MODEL
(AUTO_MODEL / autoOption) is considered when deciding whether to render the
select so a user can switch back to Auto even when there is one concrete model.
In `@services/platform/app/features/chat/utils/sanitize-chat-error.ts`:
- Around line 25-58: The current regex order in sanitize-chat-error.ts causes
bare status-code rules (authError and modelNotFound) to match messages coming
from tools, masking tool failures; update the rules so the object with category
'toolFailure' is evaluated before the status-code-based rules (the entries with
category 'authError' and 'modelNotFound'), or alternatively tighten those
regexes to avoid matching when the message contains tool context (e.g., require
word boundaries plus absence of "tool" or "upstream" so expressions like "Tool
error: 404" don't match auth/model patterns); locate the pattern array in
sanitize-chat-error.ts and either move the { category: 'toolFailure', pattern:
/tool.*error|.../ } entry above the authError/modelNotFound objects or constrain
the auth/model regexes accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6cff8e40-5cac-414c-9b46-b2013b64f242
📒 Files selected for processing (6)
services/platform/app/features/chat/components/model-selector.tsxservices/platform/app/features/chat/context/chat-layout-context.tsxservices/platform/app/features/chat/utils/__tests__/sanitize-chat-error.test.tsservices/platform/app/features/chat/utils/sanitize-chat-error.tsservices/platform/messages/de.jsonservices/platform/messages/en.json
Comment on lines
+100
to
+118
| const [rawModelOverrides, setRawModelOverrides] = usePersistedState< | ||
| Record<string, ModelOverrideEntry | string> | ||
| >(modelOverridesKey, {}); | ||
|
|
||
| // Expose a flat Record<string, string> to consumers, filtering out expired entries. | ||
| const selectedModelOverrides = useMemo(() => { | ||
| const now = Date.now(); | ||
| const result: Record<string, string> = {}; | ||
| for (const [key, value] of Object.entries(rawModelOverrides)) { | ||
| if (typeof value === 'string') { | ||
| // Legacy format (no expiry) — treat as expired | ||
| continue; | ||
| } | ||
| if (value.expiresAt > now) { | ||
| result[key] = value.modelId; | ||
| } | ||
| } | ||
| return result; | ||
| }, [rawModelOverrides]); |
There was a problem hiding this comment.
Don't discard legacy overrides on first load.
Legacy Record<string, string> entries are skipped here, which resets every pre-existing saved model selection as soon as users load this build. If the intended rollout behavior is “expire after 24 hours,” these values need a one-time migration to ModelOverrideEntry with a fresh expiresAt instead of being treated as already expired.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/context/chat-layout-context.tsx` around
lines 100 - 118, The current selectedModelOverrides memo drops legacy string
entries (Record<string,string>) which wipes users' saved selections; update the
logic that reads rawModelOverrides (the useMemo for selectedModelOverrides) to
detect string values, convert them into ModelOverrideEntry objects with a fresh
expiresAt (e.g. Date.now() + 24*60*60*1000) and persist the migration back via
setRawModelOverrides so legacy entries are upgraded once; keep the existing
expiry check for ModelOverrideEntry values and ensure you reference
rawModelOverrides, selectedModelOverrides, setRawModelOverrides and
modelOverridesKey when implementing the one-time migration.
Comment on lines
+104
to
+118
| // Expose a flat Record<string, string> to consumers, filtering out expired entries. | ||
| const selectedModelOverrides = useMemo(() => { | ||
| const now = Date.now(); | ||
| const result: Record<string, string> = {}; | ||
| for (const [key, value] of Object.entries(rawModelOverrides)) { | ||
| if (typeof value === 'string') { | ||
| // Legacy format (no expiry) — treat as expired | ||
| continue; | ||
| } | ||
| if (value.expiresAt > now) { | ||
| result[key] = value.modelId; | ||
| } | ||
| } | ||
| return result; | ||
| }, [rawModelOverrides]); |
There was a problem hiding this comment.
The 24-hour TTL won't expire in an already-open tab.
Date.now() is only consulted inside a useMemo keyed by rawModelOverrides, so time passing alone never removes an override. A session left open overnight will keep using the pinned model past expiresAt until the override map changes or the page remounts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/context/chat-layout-context.tsx` around
lines 104 - 118, The memoized selectedModelOverrides uses Date.now() but only
recalculates when rawModelOverrides changes, so expirations won't take effect
over time; update the logic to re-evaluate periodically by introducing a time
tick into the dependency list (e.g., a state like now updated via a useEffect
setInterval) or by using a timeout that triggers a state update at the nearest
expiresAt, then reference that state in the useMemo dependency array so
selectedModelOverrides (and any consumers) recompute when entries actually
expire; touch symbols: selectedModelOverrides, rawModelOverrides, useMemo,
Date.now(), and add a useEffect/setInterval or scheduled timeout to drive
updates.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1432
Summary
rawMessageto avoid leaking stack traces to the UI.enanddelocales.Test plan
sanitize-chat-errorunit tests pass with new categories and no-rawMessage assertions