Fix: Exclude mirostat parameter when disabled in Ollama component #10201

HzaRashid · 2025-10-09T16:28:56Z

Description

When using the Ollama component with mirostat set to "Disabled", the integration was sending "mirostat": 0 to the Ollama API, which caused errors with certain models and Ollama versions. This PR fixes the issue by setting mirostat to None when disabled, ensuring it gets filtered out and is not sent to the API.

Changes

Code Changes

src/lfx/src/lfx/components/ollama/ollama.py
- Changed mirostat_value default from 0 to None when "Disabled" is selected
- Updated logic to check if mirostat_value is None instead of if mirostat_value == 0
- Updated comment to reflect the new behavior

Test Changes

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py
- Removed mirostat=0 from test_build_model assertion (parameter is now excluded when disabled)
- Added test_build_model_with_mirostat_enabled to verify mirostat=1 is passed when "Mirostat" is selected
- Added test_build_model_with_mirostat_2_enabled to verify mirostat=2 is passed when "Mirostat 2.0" is selected

Root Cause

The Ollama Python client defines mirostat as Optional[int] = None, meaning the default/disabled state should be None, not 0. While 0 is technically valid according to Ollama's documentation (0=disabled, 1=Mirostat, 2=Mirostat 2.0), some models reject "mirostat": 0 as an invalid option. The safest approach is to omit the parameter entirely when disabled, matching the official client's behavior.

Testing

✅ Existing tests updated and passing
✅ New tests added for enabled states (Mirostat and Mirostat 2.0)
✅ Verified that mirostat parameter is excluded from API calls when disabled
✅ Verified that mirostat=1 and mirostat=2 are correctly passed when enabled

Before / After

Before:

# When mirostat is "Disabled"
llm_params = {
    "mirostat": 0,  # ❌ Sent to API, causes errors
    ...
}

After:

# When mirostat is "Disabled"
llm_params = {
    # mirostat is None, filtered out ✅
    ...
}

Summary by CodeRabbit

Bug Fixes
- Corrected handling of the mirostat setting: when disabled, related parameters are no longer applied; when enabled, parameters are preserved as configured. Improves reliability and consistency across configurations.
Tests
- Added unit tests covering mirostat enabled modes (Mirostat and Mirostat 2.0) to ensure correct parameter passing and model construction.

coderabbitai · 2025-10-09T16:29:15Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Updates Ollama component to treat disabled mirostat as None instead of 0, adjusting parameter forwarding (mirostat, mirostat_eta, mirostat_tau). Adds unit tests verifying correct parameter propagation when mirostat is enabled for modes “Mirostat” and “Mirostat 2.0”. Addresses invalid-parameter issue when mirostat is disabled.

Changes

Cohort / File(s)	Summary of Changes
Ollama component parameter handling `src/lfx/src/lfx/components/ollama/ollama.py`	Switches default disabled mirostat from 0 to None; updates condition to treat None as disabled and sets mirostat_eta/tau to None when disabled; preserves provided values when enabled.
Unit tests for mirostat propagation `src/backend/tests/unit/components/languagemodels/test_chatollama_component.py`	Adds tests asserting correct ChatOllama call args when mirostat is “Mirostat” (1) and “Mirostat 2.0” (2); confirms model instance passthrough; clarifies assertions when mirostat is disabled are filtered.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant LangFlow as LangFlow UI
  participant OllamaComp as Ollama Component
  participant ChatAPI as ChatOllama

  User->>LangFlow: Configure model (mirostat: Disabled/Mirostat/Mirostat 2.0)
  LangFlow->>OllamaComp: build_model(config)
  alt mirostat Disabled (None)
    Note over OllamaComp: Map mirostat=None<br/>mirostat_eta=None, mirostat_tau=None
    OllamaComp->>ChatAPI: create(model, mirostat=None, eta=None, tau=None)
  else mirostat = "Mirostat"
    Note over OllamaComp: Map mirostat=1<br/>preserve eta/tau
    OllamaComp->>ChatAPI: create(model, mirostat=1, eta, tau)
  else mirostat = "Mirostat 2.0"
    Note over OllamaComp: Map mirostat=2<br/>preserve eta/tau
    OllamaComp->>ChatAPI: create(model, mirostat=2, eta, tau)
  end
  ChatAPI-->>OllamaComp: model instance
  OllamaComp-->>LangFlow: return model
  LangFlow-->>User: ready

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (8 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly and accurately summarizes the primary change, which is excluding the mirostat parameter when disabled in the Ollama component, making it clear to reviewers what the fix addresses.
Linked Issues Check	✅ Passed	The changes update the default mirostat value to None and adjust logic to omit the parameter and its related settings when disabled while also adding tests to confirm correct behavior for enabled and disabled cases, fully addressing issue #8666's coding requirements to prevent sending invalid mirostat options.
Out of Scope Changes Check	✅ Passed	All modifications are confined to the Ollama component and its associated tests to adjust the mirostat handling and its validation, with no unrelated files or features altered, ensuring no out-of-scope changes have been introduced.
Test Coverage For New Implementations	✅ Passed	The PR modifies the Ollama component’s mirostat handling and adds two backend unit tests in src/backend/tests/unit/components/languagemodels/test_chatollama_component.py that verify the correct parameters are passed when mirostat is enabled, while the existing test now confirms the parameter is omitted when disabled. These tests target the regression described in issue #8666, follow the project’s backend naming convention (test_*.py), and meaningfully assert the new behavior rather than serving as placeholders. No additional untested functionality introduced by the code changes was identified.
Test Quality And Coverage	✅ Passed	The newly added backend unit tests exercise the updated mirostat handling by verifying that both mirostat variants (“Mirostat” and “Mirostat 2.0”) propagate the correct parameters, and the existing baseline test confirms the disabled case omits them, so the primary behavior change is well covered. The tests use pytest-style assertions consistent with the project’s backend testing practices and validate specific argument values rather than acting as superficial smoke tests. No async routines or API endpoints are involved, so the remaining checklist items are not applicable, and overall the test coverage is appropriate for the implemented fix.
Test File Naming And Structure	✅ Passed	The modified backend test file `src/backend/tests/unit/components/languagemodels/test_chatollama_component.py` follows the expected naming convention (`test_*.py`) and uses pytest-style functions with clear, descriptive names that explain the behavior under test, covering both default/disabled scenarios and the newly added mirostat modes, while leveraging fixtures for shared setup without requiring explicit teardown. No frontend or integration test files were introduced in this PR, so there are no additional structural concerns. Overall, the tests remain well organized and provide balanced coverage across the relevant configurations.
Excessive Mock Usage Warning	✅ Passed	The updated tests rely on a single fixture-based mock of `ChatOllama` to prevent real LLM calls, and the new cases merely assert that specific parameters are forwarded while reusing the same stubbed return, so there is no proliferation of mocks masking core logic; the tests continue to exercise the component’s parameter handling directly and remain appropriate unit coverage rather than over-mocked scenarios.

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d12bb50 and 90dff81.

📒 Files selected for processing (2)

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py (2 hunks)
src/lfx/src/lfx/components/ollama/ollama.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (8)

src/backend/tests/unit/components/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)

src/backend/tests/unit/components/**/*.py: Mirror the component directory structure for unit tests in src/backend/tests/unit/components/
Use ComponentTestBaseWithClient or ComponentTestBaseWithoutClient as base classes for component unit tests
Provide file_names_mapping for backward compatibility in component tests
Create comprehensive unit tests for all new components

Files:

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py

{src/backend/**/*.py,tests/**/*.py,Makefile}

📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)

{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code

Files:

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py

src/backend/tests/unit/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)

Test component integration within flows using create_flow, build_flow, and get_build_events utilities

Files:

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py

src/backend/tests/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)

src/backend/tests/**/*.py: Unit tests for backend code must be located in the 'src/backend/tests/' directory, with component tests organized by component subdirectory under 'src/backend/tests/unit/components/'.
Test files should use the same filename as the component under test, with an appropriate test prefix or suffix (e.g., 'my_component.py' → 'test_my_component.py').
Use the 'client' fixture (an async httpx.AsyncClient) for API tests in backend Python tests, as defined in 'src/backend/tests/conftest.py'.
When writing component tests, inherit from the appropriate base class in 'src/backend/tests/base.py' (ComponentTestBase, ComponentTestBaseWithClient, or ComponentTestBaseWithoutClient) and provide the required fixtures: 'component_class', 'default_kwargs', and 'file_names_mapping'.
Each test in backend Python test files should have a clear docstring explaining its purpose, and complex setups or mocks should be well-commented.
Test both sync and async code paths in backend Python tests, using '@pytest.mark.asyncio' for async tests.
Mock external dependencies appropriately in backend Python tests to isolate unit tests from external services.
Test error handling and edge cases in backend Python tests, including using 'pytest.raises' and asserting error messages.
Validate input/output behavior and test component initialization and configuration in backend Python tests.
Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
Be aware of ContextVar propagation in async tests; test both direct event loop execution and 'asyncio.to_thread' scenarios to ensure proper context isolation.
Test error handling by mocking internal functions using monkeypatch in backend Python tests.
Test resource cleanup in backend Python tests by using fixtures that ensure proper initialization and cleanup of resources.
Test timeout and performance constraints in backend Python tests using 'asyncio.wait_for' and timing assertions.
Test Langflow's Messag...

Files:

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py

src/backend/**/*component*.py

📄 CodeRabbit inference engine (.cursor/rules/icons.mdc)

In your Python component class, set the icon attribute to a string matching the frontend icon mapping exactly (case-sensitive).

Files:

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py

src/backend/**/components/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/icons.mdc)

In your Python component class, set the icon attribute to a string matching the frontend icon mapping exactly (case-sensitive).

Files:

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py

**/{test_*.py,*.test.ts,*.test.tsx}

📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)

**/{test_*.py,*.test.ts,*.test.tsx}: Check test files for excessive use of mocks that obscure what's actually being tested
Warn when mocks replace testing real behavior/interactions
Suggest using real objects or simpler test doubles when mocks become excessive
Use mocks primarily for external dependencies, not core logic
Recommend integration tests when unit tests are overly mocked
Verify tests actually exercise the new/changed functionality (not placeholders)
Test files should use descriptive test names explaining what is being tested
Organize tests logically with proper setup/teardown
Include edge cases and error conditions for comprehensive coverage
Cover both positive and negative scenarios where appropriate
Tests should cover the main functionality being implemented
Tests should not be mere smoke tests; they must validate behavior

Files:

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py

**/test_*.py

📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)

**/test_*.py: Backend test files must follow naming convention test_*.py
Backend tests should use proper pytest structure
For async backend functions, use proper async testing patterns with pytest
Backend tests should follow pytest conventions
For API endpoints, verify both success and error response tests are present

Files:

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py

🧬 Code graph analysis (1)

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py (1)

src/lfx/src/lfx/components/ollama/ollama.py (1)

build_model (144-210)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)

GitHub Check: Lint Backend / Run Mypy (3.10)
GitHub Check: Test Docker Images / Test docker images
GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
GitHub Check: Lint Backend / Run Mypy (3.13)
GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
GitHub Check: Lint Backend / Run Mypy (3.12)
GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
GitHub Check: Lint Backend / Run Mypy (3.11)
GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
GitHub Check: Test Starter Templates
GitHub Check: Run Ruff Check and Format
GitHub Check: Update Starter Projects

🔇 Additional comments (4)

src/lfx/src/lfx/components/ollama/ollama.py (1)

149-157: LGTM! The implementation correctly handles the disabled mirostat case.

The change from defaulting to 0 to defaulting to None ensures that when mirostat is "Disabled", the parameter is not sent to the Ollama API (filtered out on line 199). This fixes the issue where some Ollama models reject "mirostat": 0. The conditional logic properly preserves mirostat_eta and mirostat_tau values when mirostat is enabled.

src/backend/tests/unit/components/languagemodels/test_chatollama_component.py (3)

58-58: LGTM! Clear documentation of the disabled behavior.

The comment accurately explains that mirostat is excluded from the API call when disabled (set to None and filtered out).

86-108: LGTM! Comprehensive test for Mirostat mode.

This test properly verifies that when mirostat is set to "Mirostat", the correct value (mirostat=1) and related parameters (mirostat_eta, mirostat_tau) are passed to ChatOllama. The test follows best practices with clear documentation, proper mocking, and meaningful assertions.

109-130: LGTM! Comprehensive test for Mirostat 2.0 mode.

This test properly verifies that when mirostat is set to "Mirostat 2.0", the correct value (mirostat=2) and related parameters (mirostat_eta=0.2, mirostat_tau=10.0) are passed to ChatOllama. The test follows best practices with clear documentation, proper mocking, and meaningful assertions.

coderabbitai · 2025-10-09T16:32:08Z

src/lfx/src/lfx/components/ollama/ollama.py

        # Mapping mirostat settings to their corresponding values
        mirostat_options = {"Mirostat": 1, "Mirostat 2.0": 2}

        # Default to 0 for 'Disabled'


⚠️ Potential issue | 🟡 Minor

Update the outdated comment.

The comment states "Default to 0 for 'Disabled'" but the code now uses None as the default value (line 149). Update the comment to reflect the current behavior.

Apply this diff to update the comment:

- # Default to 0 for 'Disabled' + # Default to None for 'Disabled' (will be filtered out before sending to API)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

# Default to 0 for 'Disabled'

# Default to None for 'Disabled' (will be filtered out before sending to API)

🤖 Prompt for AI Agents

In src/lfx/src/lfx/components/ollama/ollama.py around line 148, the inline comment "Default to 0 for 'Disabled'" is outdated because the code now uses None as the default; update the comment to reflect that the default is None (meaning 'Disabled' or unset) so it matches current behavior and avoid confusion.

- Set mirostat to None instead of 0 when disabled - Ensures parameter is not sent to Ollama API - Update test to verify mirostat is excluded when disabled - Add tests for mirostat enabled states (1 and 2) Fixes langflow-ai#8666

sonarqubecloud · 2025-10-09T16:44:23Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

erichare

LGTM! Nice work @HzaRashid

…ngflow-ai#10201) * Fix: Exclude mirostat parameter when disabled in Ollama component - Set mirostat to None instead of 0 when disabled - Ensures parameter is not sent to Ollama API - Update test to verify mirostat is excluded when disabled - Add tests for mirostat enabled states (1 and 2) Fixes langflow-ai#8666 * correct inline comment * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes --------- Co-authored-by: Hamza Rashid <[email protected]> Co-authored-by: Eric Hare <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

HzaRashid requested a review from edwinjosechittilappilly October 9, 2025 16:29

coderabbitai bot reviewed Oct 9, 2025

View reviewed changes

HzaRashid added 2 commits October 9, 2025 16:42

correct inline comment

025abab

HzaRashid force-pushed the fix/ollama-mirostat-disabled-issue-8666 branch from 790595b to 025abab Compare October 9, 2025 16:43

carlosrcoelho requested a review from erichare October 29, 2025 14:14

erichare approved these changes Oct 29, 2025

View reviewed changes

erichare and others added 2 commits October 29, 2025 13:06

Merge branch 'main' into fix/ollama-mirostat-disabled-issue-8666

f94644b

[autofix.ci] apply automated fixes

4deb068

erichare enabled auto-merge October 29, 2025 20:21

erichare added this pull request to the merge queue Oct 29, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 29, 2025

carlosrcoelho added this pull request to the merge queue Oct 30, 2025

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Oct 30, 2025

Merge branch 'main' into pr/10201

a1324dc

erichare enabled auto-merge October 30, 2025 15:01

[autofix.ci] apply automated fixes

ada9e31

erichare added this pull request to the merge queue Oct 30, 2025

Merged via the queue into langflow-ai:main with commit 6a8f8e2 Oct 30, 2025
47 checks passed

carlosrcoelho added the bug Something isn't working label Oct 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix: Exclude mirostat parameter when disabled in Ollama component #10201

Fix: Exclude mirostat parameter when disabled in Ollama component #10201

Uh oh!

HzaRashid commented Oct 9, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 9, 2025 •

edited

Loading

Review skipped

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Oct 9, 2025

Uh oh!

sonarqubecloud bot commented Oct 9, 2025

Uh oh!

erichare left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	# Default to 0 for 'Disabled'
	# Default to None for 'Disabled' (will be filtered out before sending to API)

Fix: Exclude mirostat parameter when disabled in Ollama component #10201

Fix: Exclude mirostat parameter when disabled in Ollama component #10201

Uh oh!

Conversation

HzaRashid commented Oct 9, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Code Changes

Test Changes

Root Cause

Testing

Before / After

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 9, 2025

Quality Gate passed

Uh oh!

erichare left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HzaRashid commented Oct 9, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 9, 2025 •

edited

Loading