fix: restore LLM model values in reapply_all_settings fallback (#1587) by hunterxtang · Pull Request #1850 · langflow-ai/openrag

hunterxtang · 2026-06-11T22:01:34Z

Fixes #1587. The fallback branch of _update_langflow_model_values() only refreshed embedding providers, so LLM selections reverted to stale/default models after a flow reset. Add an LLM provider loop mirroring the embedding logic: current provider gets config.agent.llm_model, others get None (first available), all with force_llm_update=True. Guards config.agent.llm_provider against None before .lower().

Summary by CodeRabbit

Bug Fixes
- Improved “reapply all settings” behavior so Langflow updates language model values for all configured LLM providers when no explicit overrides are provided.
Tests
- Added unit tests covering the fallback update path (including a regression for a previously failing scenario), edge cases around provider/config combinations, and graceful handling of null/empty provider inputs.

…low-ai#1587) The fallback branch of _update_langflow_model_values() only refreshed embedding providers, so LLM selections reverted to stale/default models after a flow reset. Add an LLM provider loop mirroring the embedding logic: current provider gets config.agent.llm_model, others get None (first available), all with force_llm_update=True. Guards config.agent.llm_provider against None before .lower().

coderabbitai · 2026-06-11T22:01:47Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: de89bc6d-61f3-490a-99b8-181ebe236b66

📥 Commits

Reviewing files that changed from the base of the PR and between a147707 and 0874ce8.

📒 Files selected for processing (3)

tests/unit/api/test_langflow_sync_bug_1587.py
tests/unit/api/test_langflow_sync_edge_cases.py
tests/unit/api/test_none_crash.py

💤 Files with no reviewable changes (2)

tests/unit/api/test_none_crash.py
tests/unit/api/test_langflow_sync_edge_cases.py

🚧 Files skipped from review as they are similar to previous changes (1)

tests/unit/api/test_langflow_sync_bug_1587.py

Walkthrough

This PR fixes bug #1587 by implementing fallback logic in _update_langflow_model_values to update all configured LLM providers when no explicit model or provider overrides are specified. The implementation iterates configured providers and calls the flows service for each, using the configured model only for the current provider. Comprehensive test coverage includes regression validation and extensive edge case scenarios.

Changes

LLM Provider Fallback for Bug #1587

Layer / File(s)	Summary
Fallback LLM provider update logic `src/api/settings/langflow_sync.py`	Adds conditional iteration over all configured LLM providers (openai, anthropic, watsonx, ollama) to call `change_langflow_model_value` for each when no explicit overrides are provided, using `config.agent.llm_model` only for the current `config.agent.llm_provider` and `None` for others.
Bug `#1587` regression validation `tests/unit/api/test_langflow_sync_bug_1587.py`	Introduces module structure, test fixtures with mocked config and flows service, a skipped test documenting unfixed behavior, and a passing test validating four LLM provider calls with anthropic receiving the configured model and others receiving `None`, plus three embedding provider calls.
Edge case and robustness test coverage `tests/unit/api/test_langflow_sync_edge_cases.py`, `tests/unit/api/test_none_crash.py`	Comprehensive test suite validating empty-string and `None` provider handling without crashes, unconfigured provider behavior, single-provider configurations, provider set consistency between LLM and embedding updates, exception propagation mid-loop, and explicit override bypass behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

lucaseduoli

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: fixing the LLM model values restoration in the reapply_all_settings fallback path, addressing bug `#1587`.
Linked Issues check	✅ Passed	The PR implements all coding requirements from issue `#1587`: adding LLM provider iteration loop, calling change_langflow_model_value for each configured provider, setting llm_model correctly per provider, using force_llm_update=True, and checking None guard.
Out of Scope Changes check	✅ Passed	All changes are scoped to the fallback branch of _update_langflow_model_values and comprehensive test coverage; no extraneous modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 91.67% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/api/settings/langflow_sync.py (1)

154-157: ⚠️ Potential issue | 🟠 Major

Guard LLM provider normalization when config.agent.llm_provider can be None (explicit update path).

src/api/settings/langflow_sync.py lines 155-156 call .lower() on config.agent.llm_provider without a None-guard; if llm_model is provided while the stored provider is None, this branch can throw AttributeError instead of using the null-safe fallback logic.

Suggested fix

         if llm_model or llm_provider:
-            effective_llm_provider = (llm_provider or config.agent.llm_provider).lower()
-            if llm_provider and llm_provider.lower() != config.agent.llm_provider.lower():
+            configured_llm_provider = (config.agent.llm_provider or "").lower()
+            effective_llm_provider = (llm_provider or configured_llm_provider).lower()
+            if not effective_llm_provider:
+                raise ValueError("LLM provider is required when updating LLM model values")
+            if llm_provider and llm_provider.lower() != configured_llm_provider:
                 effective_llm_model = llm_model  # do not fall back; force caller to specify
             else:
                 effective_llm_model = llm_model or config.agent.llm_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/api/settings/langflow_sync.py` around lines 154 - 157, The branch that
computes effective_llm_provider and compares llm_provider to
config.agent.llm_provider can raise AttributeError because
config.agent.llm_provider may be None; update the logic in the block that uses
llm_model/llm_provider to null-guard config.agent.llm_provider before calling
.lower() (for example use a None-safe expression or explicit None check) so both
the assignment to effective_llm_provider and the comparison llm_provider.lower()
!= config.agent.llm_provider.lower() handle a None stored provider, and keep the
existing behavior where effective_llm_model is forced when the caller supplies a
differing llm_provider.

🤖 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.

Outside diff comments:
In `@src/api/settings/langflow_sync.py`:
- Around line 154-157: The branch that computes effective_llm_provider and
compares llm_provider to config.agent.llm_provider can raise AttributeError
because config.agent.llm_provider may be None; update the logic in the block
that uses llm_model/llm_provider to null-guard config.agent.llm_provider before
calling .lower() (for example use a None-safe expression or explicit None check)
so both the assignment to effective_llm_provider and the comparison
llm_provider.lower() != config.agent.llm_provider.lower() handle a None stored
provider, and keep the existing behavior where effective_llm_model is forced
when the caller supplies a differing llm_provider.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dbd0555e-6821-4bdc-806d-d175a89061c1

📥 Commits

Reviewing files that changed from the base of the PR and between 5bf8fe4 and a147707.

📒 Files selected for processing (4)

src/api/settings/langflow_sync.py
tests/unit/api/test_langflow_sync_bug_1587.py
tests/unit/api/test_langflow_sync_edge_cases.py
tests/unit/api/test_none_crash.py

Vchen7629

Added some comments for some light code changes.

Vchen7629

👍

github-actions Bot added community backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) tests labels Jun 11, 2026

github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels Jun 11, 2026

coderabbitai Bot reviewed Jun 11, 2026

View reviewed changes

Vchen7629 requested changes Jun 16, 2026

View reviewed changes

Comment thread tests/unit/api/test_langflow_sync_bug_1587.py

Comment thread tests/unit/api/test_langflow_sync_edge_cases.py

Comment thread tests/unit/api/test_none_crash.py

github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels Jun 16, 2026

hunterxtang added 2 commits June 16, 2026 10:43

test: extract mock_config and mock_flows_service into fixtures

b272fe8

chore: remove "Made with Bob" markers

0874ce8

github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels Jun 16, 2026

Vchen7629 approved these changes Jun 16, 2026

View reviewed changes

hunterxtang mentioned this pull request Jun 18, 2026

fix: restore LLM model values in reapply_all_settings fallback (#1587) #1926

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: restore LLM model values in reapply_all_settings fallback (#1587)#1850

fix: restore LLM model values in reapply_all_settings fallback (#1587)#1850
hunterxtang wants to merge 3 commits into
langflow-ai:mainfrom
hunterxtang:fix/1587-llm-fallback-update

hunterxtang commented Jun 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Vchen7629 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Vchen7629 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

hunterxtang commented Jun 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Vchen7629 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Vchen7629 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hunterxtang commented Jun 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading