fix: correct deprecated model settings behavior#14048
Open
erichare wants to merge 3 commits into
Open
Conversation
Contributor
WalkthroughChangesModel availability enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ModelSelection
participant update_enabled_models
participant UnifiedCatalog
Client->>ModelSelection: Select model
ModelSelection->>UnifiedCatalog: Read model metadata
UnifiedCatalog-->>ModelSelection: Return availability flags
ModelSelection-->>Client: Disable unavailable model toggle
Client->>update_enabled_models: Submit enabled model update
update_enabled_models->>UnifiedCatalog: Check model availability
UnifiedCatalog-->>update_enabled_models: Return unavailable reason
update_enabled_models-->>Client: Return HTTP 400
Suggested reviewers: 🚥 Pre-merge checks | ✅ 9✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
github-actions
Bot
added
bug
Contributor
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-1.11.0 #14048 +/- ##
==================================================
+ Coverage 60.16% 60.55% +0.39%
==================================================
Files 2333 2342 +9
Lines 228353 229953 +1600
Branches 32038 32275 +237
==================================================
+ Hits 137386 139250 +1864
+ Misses 89351 89041 -310
- Partials 1616 1662 +46
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
github-actions
Bot
added
bug
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/backend/tests/unit/api/v1/test_models_enabled_providers.py (1)
368-380: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for
not_supportedmodel rejection.The test covers the
deprecatedrejection path but not thenot_supportedbranch (line 848-849 inmodels.py). Theelifcreates a distinct code path with a different error message ("Cannot enable not supported model: ...") that should be verified. As per coding guidelines, API endpoint tests should verify both success and error responses, and test coverage should cover the new or changed behavior.🧪 Suggested test for not_supported model rejection
`@pytest.mark.usefixtures`("active_user") async def test_cannot_enable_deprecated_model(client: AsyncClient, logged_in_headers): """Deprecated models cannot be persisted as explicitly enabled.""" response = await client.post( "api/v1/models/enabled_models", json=[{"provider": "OpenAI", "model_id": "gpt-3.5-turbo", "enabled": True}], headers=logged_in_headers, ) assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json()["detail"] == "Cannot enable deprecated model: gpt-3.5-turbo" + + +@pytest.mark.usefixtures("active_user") +async def test_cannot_enable_not_supported_model(client: AsyncClient, logged_in_headers): + """Not-supported models cannot be persisted as explicitly enabled.""" + # Use a model that is marked not_supported in the catalog + response = await client.post( + "api/v1/models/enabled_models", + json=[{"provider": "<Provider>", "model_id": "<not_supported_model>", "enabled": True}], + headers=logged_in_headers, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == "Cannot enable not supported model: <not_supported_model>"🤖 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 `@src/backend/tests/unit/api/v1/test_models_enabled_providers.py` around lines 368 - 380, Add a unit test alongside test_cannot_enable_deprecated_model covering an explicitly enabled model marked not_supported. Post the same enabled-model payload through the endpoint with logged_in_headers, assert HTTP 400, and verify the response detail is exactly "Cannot enable not supported model: ..." for the chosen model.Source: Coding guidelines
src/frontend/src/modals/modelProviderModal/__tests__/ModelSelection.test.tsx (1)
535-553: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for
not_supportedmodel toggle.The test verifies the
deprecatedtoggle is disabled but does not cover thenot_supportedcondition added at line 162 ofModelSelection.tsx. As per coding guidelines, frontend tests should verify the new or changed behavior rather than acting as placeholders, and thenot_supporteddisabling is part of this PR's changes.🧪 Suggested test for not_supported model toggle
it("should not allow deprecated models to be enabled", async () => { const user = userEvent.setup(); const onModelToggle = jest.fn(); render( <ModelSelection {...defaultProps} availableModels={withDeprecated} modelType="llm" onModelToggle={onModelToggle} />, ); await user.click(screen.getByTestId("llm-deprecated-summary")); const deprecatedToggle = screen.getByTestId("llm-toggle-gpt-old"); expect(deprecatedToggle).toBeDisabled(); await user.click(deprecatedToggle); expect(onModelToggle).not.toHaveBeenCalled(); }); + + it("should not allow not_supported models to be enabled", async () => { + const user = userEvent.setup(); + const onModelToggle = jest.fn(); + const withNotSupported = [ + ...defaultModels, + { model_name: "gpt-unsupported", metadata: { model_type: "llm", not_supported: true } }, + ]; + render( + <ModelSelection + {...defaultProps} + availableModels={withNotSupported} + modelType="llm" + onModelToggle={onModelToggle} + />, + ); + + // not_supported models may not appear in the deprecated disclosure; + // adjust the selector based on how they are rendered in the UI. + const unsupportedToggle = screen.getByTestId("llm-toggle-gpt-unsupported"); + expect(unsupportedToggle).toBeDisabled(); + + await user.click(unsupportedToggle); + expect(onModelToggle).not.toHaveBeenCalled(); + });🤖 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 `@src/frontend/src/modals/modelProviderModal/__tests__/ModelSelection.test.tsx` around lines 535 - 553, Extend the ModelSelection tests around the existing deprecated-model toggle case to cover a model marked not_supported. Render the component with that model in availableModels, open the appropriate summary, assert its toggle is disabled, and verify clicking it does not call onModelToggle, matching the behavior implemented in ModelSelection.Source: Coding guidelines
🤖 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 `@src/backend/tests/unit/api/v1/test_models_enabled_providers.py`:
- Around line 368-380: Add a unit test alongside
test_cannot_enable_deprecated_model covering an explicitly enabled model marked
not_supported. Post the same enabled-model payload through the endpoint with
logged_in_headers, assert HTTP 400, and verify the response detail is exactly
"Cannot enable not supported model: ..." for the chosen model.
In
`@src/frontend/src/modals/modelProviderModal/__tests__/ModelSelection.test.tsx`:
- Around line 535-553: Extend the ModelSelection tests around the existing
deprecated-model toggle case to cover a model marked not_supported. Render the
component with that model in availableModels, open the appropriate summary,
assert its toggle is disabled, and verify clicking it does not call
onModelToggle, matching the behavior implemented in ModelSelection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fc659f04-dc2f-403c-a0ea-32fb675ea0ab
📒 Files selected for processing (7)
src/backend/base/langflow/api/v1/models.pysrc/backend/tests/unit/api/v1/test_models_enabled_providers.pysrc/backend/tests/unit/api/v1/test_variable.pysrc/frontend/src/modals/modelProviderModal/__tests__/ModelSelection.test.tsxsrc/frontend/src/modals/modelProviderModal/components/ModelSelection.tsxsrc/lfx/src/lfx/base/models/models_dev_catalog.pysrc/lfx/tests/unit/base/models/test_models_dev_catalog.py
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.
Release-1.11.0 companion to #14047.
Summary
Root cause
The settings UI allowed deprecated models to enter its optimistic toggle queue, and the update endpoint accepted and persisted that state. A later
GET /enabled_modelsalways returned deprecated models as disabled, so the next refetch appeared to undo the user's toggle.Separately, the models.dev fallback marks catalog entries older than 900 days as deprecated. That heuristic also applied to embeddings. The OpenAI
text-embedding-3-*models reached that threshold in July 2026 even though age alone is not a reliable deprecation signal for embedding models.Explicit catalog deprecations remain unchanged, including provider-specific embedding models that are known to be unavailable.
Validation
uv run pytest tests/unit/base/models/test_models_dev_catalog.py -q(23 passed)uv run pytest src/backend/tests/unit/api/v1/test_models_enabled_providers.py -k cannot_enable_deprecated_model -q(1 passed)npm test -- src/modals/modelProviderModal/__tests__/ModelSelection.test.tsx --runInBand(31 passed)Summary by CodeRabbit
New Features
Bug Fixes