Improve model configuration resilience - #4
Conversation
📝 WalkthroughWalkthroughProfile variant parsing and runtime fallback behavior now support removed variants. Settings highlights affected Profiles, preserves references on save, and replaces modality text inputs with accessible checkbox controls alongside updated layout and interaction coverage. ChangesProfile variant configuration and runtime
Settings controls
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
|
We've triggered an ultrareview automatically — This PR changes core model configuration validation and resolution logic, including replacing a strict validation error with a silent fallback for missing profile variants; a subtle bug could cause incorrect model selection across all profiles, making multi-pass review valuable.. I'll post findings when complete. An ultrareview is cubic's deepest review, catching hard-to-find bugs in the most critical PRs. It runs a longer, multi-pass analysis using cubic's most capable review models, and typically takes around 30 minutes. It consumes your team's reviewed-lines allowance at 3× the standard rate. Automated ultrareviews are disabled by default. We triggered this run as part of your trial. Want cubic to do this for every high-risk PR? Enable auto-ultrareview in your settings. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/features/settings-panels.tsx`:
- Around line 34-73: Update the onChange handler in ModalityField to preserve
modality values from the original value that are not present in
MODEL_MODALITIES, while still applying the checkbox change and maintaining the
tuple’s ordering for recognized entries. Emit the combined result through
onChange so toggling a checkbox cannot discard unrecognized entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d7abcc7e-e311-40b3-b12d-f08d7bdcace3
📒 Files selected for processing (12)
apps/web/src/components/features/SettingsDialog.interaction.tsxapps/web/src/components/features/SettingsDialog.tsxapps/web/src/components/features/settings-helpers.tsapps/web/src/components/features/settings-panels.tsxdocs/configuration.mdpackages/agent-core/src/config/config.test.tspackages/agent-core/src/config/provider.tspackages/agent-core/src/config/schema.tspackages/agent-core/src/config/server-config-service.test.tspackages/agent-core/src/config/server-config-service.tspackages/agent-core/src/models/model-runtime-snapshot.tspackages/agent-core/src/models/model-selection-resolver.test.ts
💤 Files with no reviewable changes (1)
- packages/agent-core/src/config/server-config-service.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ultrareview completed in 12m 5s
All reported issues were addressed across 12 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
52ed6b5 to
68e1e58
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/agent-core/src/config/server-config-service.test.ts (1)
517-535: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winExercise Profile call-option preservation in both fallback paths.
Both tests use
fastwithoutoptions, so they would pass even if removed- or empty-variant resolution dropped Profile call options. Add a valid Profile option and assert it remains in the saved config and resolved runtime default.Also applies to: 538-554
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/agent-core/src/config/server-config-service.test.ts` around lines 517 - 535, Update both fallback tests, including “saves a missing Profile variant and publishes its model-default fallback” and the empty-variant test, to set a valid Profile call option on fast. Assert that the option is preserved in the saved config and in both resolved runtime defaults returned by getProfileDefault and catalog.profileDefaults, covering removed- and empty-variant resolution paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/agent-core/src/config/server-config-service.test.ts`:
- Around line 517-535: Update both fallback tests, including “saves a missing
Profile variant and publishes its model-default fallback” and the empty-variant
test, to set a valid Profile call option on fast. Assert that the option is
preserved in the saved config and in both resolved runtime defaults returned by
getProfileDefault and catalog.profileDefaults, covering removed- and
empty-variant resolution paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 053a9cd4-a958-40a8-8318-cf3677342291
📒 Files selected for processing (12)
apps/web/src/components/features/SettingsDialog.interaction.tsxapps/web/src/components/features/SettingsDialog.tsxapps/web/src/components/features/settings-helpers.tsapps/web/src/components/features/settings-panels.tsxdocs/configuration.mdpackages/agent-core/src/config/config.test.tspackages/agent-core/src/config/provider.tspackages/agent-core/src/config/schema.tspackages/agent-core/src/config/server-config-service.test.tspackages/agent-core/src/config/server-config-service.tspackages/agent-core/src/models/model-runtime-snapshot.tspackages/agent-core/src/models/model-selection-resolver.test.ts
💤 Files with no reviewable changes (1)
- packages/agent-core/src/config/server-config-service.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/agent-core/src/config/schema.ts
- packages/agent-core/src/config/provider.ts
- packages/agent-core/src/config/config.test.ts
- apps/web/src/components/features/SettingsDialog.tsx
- apps/web/src/components/features/settings-helpers.ts
- packages/agent-core/src/models/model-runtime-snapshot.ts
- apps/web/src/components/features/settings-panels.tsx
- packages/agent-core/src/models/model-selection-resolver.test.ts
- apps/web/src/components/features/SettingsDialog.interaction.tsx
1 participant
Summary
text,image,audio, andvideomulti-select controlsRoot cause
The apply notice and Settings workspace each claimed the dialog height, which pushed the footer beyond the visible boundary. The modality text input normalized its controlled value after every keystroke, so users could not naturally type a comma-separated second value. Variant replacement was rejected by whole-document validation when an existing Profile referenced a removed key, even though the model itself remained usable.
User impact
Users can always reach the save action, select modalities without editing delimiter syntax, and save a valid model variant map without first coordinating every Profile reference. Profiles that need repair remain explicit, while execution deterministically uses the model default until the reference is fixed.
Validation
git diff --checkpassedSummary by CodeRabbit