fix: batch Enabled Models for Model Provider #11472
Open
+151
−16
Conversation
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
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughThe changes introduce batching functionality for model enable/disable updates and optimistic UI support. A parent component accumulates model toggle changes in a pendingUpdates map and flushes them as a batch upon provider switch or unmount. A child component checks pending updates to display optimistic UI states before server confirmation. Changes
Sequence DiagramsequenceDiagram
participant User
participant ModelProvidersContent
participant ModelSelection
participant API
User->>ModelProvidersContent: Toggle model enabled/disabled
ModelProvidersContent->>ModelProvidersContent: Add to pendingUpdates map
ModelProvidersContent->>ModelSelection: Pass pendingUpdates prop
ModelSelection->>ModelSelection: Check pendingUpdates for optimistic state
ModelSelection->>User: Display optimistic UI state
alt On Provider Switch or Unmount
ModelProvidersContent->>ModelProvidersContent: flushPendingUpdates()
ModelProvidersContent->>API: updateEnabledModels(batched updates)
API->>ModelProvidersContent: Response confirmation
ModelProvidersContent->>ModelProvidersContent: Clear pendingUpdates
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11472 +/- ##
==========================================
- Coverage 34.89% 34.88% -0.01%
==========================================
Files 1420 1420
Lines 68217 68244 +27
Branches 9984 9992 +8
==========================================
+ Hits 23804 23808 +4
- Misses 43179 43202 +23
Partials 1234 1234
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/frontend/src/modals/modelProviderModal/components/ModelProvidersContent.tsx (1)
41-145: Don’t drop pendingUpdates on failed batch + keep invalidation consistent.
flushPendingUpdatesclears state immediately; if the mutation fails, optimistic changes are lost and the UI silently falls back to stale server data. The unmount flush also bypasses invalidation/error handling, so downstream views can stay stale. Please clear only on success, surface errors, and invalidate consistently for the unmount path.✅ Suggested fix
const flushPendingUpdates = useCallback(() => { if (pendingUpdates.size === 0) return; const updates = Array.from(pendingUpdates.values()); updateEnabledModels( { updates }, { onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["useGetEnabledModels"] }); + setPendingUpdates(new Map()); }, + onError: () => { + setErrorData({ + title: "Error updating models", + list: ["Please retry."], + }); + }, }, ); - setPendingUpdates(new Map()); -}, [pendingUpdates, updateEnabledModels, queryClient]); +}, [pendingUpdates, updateEnabledModels, queryClient, setErrorData]); // Flush pending updates on unmount (modal close) useEffect(() => { return () => { if (pendingUpdates.size > 0) { const updates = Array.from(pendingUpdates.values()); - updateEnabledModels({ updates }); + updateEnabledModels( + { updates }, + { + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["useGetEnabledModels"] }); + }, + }, + ); } }; -}, [pendingUpdates, updateEnabledModels]); +}, [pendingUpdates, updateEnabledModels, queryClient]);
github-actions
bot
added
bug
viktoravelino
approved these changes
Jan 27, 2026
github-actions
bot
added
bug
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The Bug
Screen.Recording.2026-01-27.at.3.33.35.PM.mov
The Fix
Screen.Recording.2026-01-27.at.3.32.42.PM.mov
Testing Strategy
select language model component and add a provider with api key and select models.
Description
This pull request introduces optimistic UI updates for toggling model enablement in the model provider modal. Now, when users toggle models, the UI immediately reflects their changes before the server responds, and multiple toggles are batched and sent together. The changes also ensure pending updates are flushed when switching providers or closing the modal, and add comprehensive tests for this new behavior.
Optimistic UI and batching for model toggles:
pendingUpdatesstate toModelProvidersContent.tsxto accumulate model enable/disable changes locally and batch them before sending to the server. Pending updates are flushed when the provider changes or the modal closes. (src/frontend/src/modals/modelProviderModal/components/ModelProvidersContent.tsx) [1] [2] [3]ModelSelection.tsxto accept apendingUpdatesprop and prioritize pending updates for UI state, enabling immediate feedback when toggling models. (src/frontend/src/modals/modelProviderModal/components/ModelSelection.tsx) [1] [2]Testing improvements:
pendingUpdates, including cases for enabled/disabled states, server fallback, and prioritization. (src/frontend/src/modals/modelProviderModal/__tests__/ModelSelection.test.tsx)Type and error handling enhancements:
ResponseErrorDetailAPItype for provider activation and validation errors. (src/frontend/src/modals/modelProviderModal/components/ModelProvidersContent.tsx) [1] [2]Imports and type usage:
ModelStatusUpdateand improved usage of types in affected files for clarity and maintainability. (src/frontend/src/modals/modelProviderModal/components/ModelProvidersContent.tsx,src/frontend/src/modals/modelProviderModal/components/ModelSelection.tsx,src/frontend/src/modals/modelProviderModal/__tests__/ModelSelection.test.tsx) [1] [2] [3]Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.