fix: Prevent infinite loop when Loop component is frozen #10736
Conversation
|
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 WalkthroughAdds comprehensive unit test coverage for Loop component freeze behavior and modifies the build and evaluate logic to ensure frozen Loop components rebuild rather than using cached results, enabling proper iteration over their data. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant BuildVertex as build_vertex()
participant LoopCheck as is_loop_component<br/>(check)
participant Cache as Cache Path
participant Rebuild as Rebuild Path
Caller->>BuildVertex: Build frozen vertex
BuildVertex->>LoopCheck: Is vertex a Loop component?
alt Loop Component
LoopCheck-->>BuildVertex: Yes (is_loop flag or<br/>display_name="Loop")
BuildVertex->>Rebuild: Force rebuild
Rebuild-->>Caller: Return rebuilt result
else Non-Loop Component
LoopCheck-->>BuildVertex: No
BuildVertex->>Cache: Use cached result
Cache-->>Caller: Return cached result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
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
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (40.04%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #10736 +/- ##
==========================================
- Coverage 32.40% 31.53% -0.88%
==========================================
Files 1368 1368
Lines 63412 63414 +2
Branches 9373 9373
==========================================
- Hits 20551 19998 -553
- Misses 41829 42383 +554
- Partials 1032 1033 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py(1 hunks)src/lfx/src/lfx/graph/graph/base.py(1 hunks)src/lfx/src/lfx/graph/vertex/base.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
src/backend/**/*.py: Use FastAPI async patterns withawaitfor async operations in component execution methods
Useasyncio.create_task()for background tasks and implement proper cleanup with try/except forasyncio.CancelledError
Usequeue.put_nowait()for non-blocking queue operations andasyncio.wait_for()with timeouts for controlled get operations
Files:
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py
src/backend/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
src/backend/tests/**/*.py: Place backend unit tests insrc/backend/tests/directory, component tests insrc/backend/tests/unit/components/organized by component subdirectory, and integration tests accessible viamake integration_tests
Use same filename as component with appropriate test prefix/suffix (e.g.,my_component.py→test_my_component.py)
Use theclientfixture (FastAPI Test Client) defined insrc/backend/tests/conftest.pyfor API tests; it provides an asynchttpx.AsyncClientwith automatic in-memory SQLite database and mocked environment variables. Skip client creation by marking test with@pytest.mark.noclient
Inherit from the correctComponentTestBasefamily class located insrc/backend/tests/base.pybased on API access needs:ComponentTestBase(no API),ComponentTestBaseWithClient(needs API), orComponentTestBaseWithoutClient(pure logic). Provide three required fixtures:component_class,default_kwargs, andfile_names_mapping
Create comprehensive unit tests for all new backend components. If unit tests are incomplete, create a corresponding Markdown file documenting manual testing steps and expected outcomes
Test both sync and async code paths, mock external dependencies appropriately, test error handling and edge cases, validate input/output behavior, and test component initialization and configuration
Use@pytest.mark.asynciodecorator for async component tests and ensure async methods are properly awaited
Test background tasks usingasyncio.create_task()and verify completion withasyncio.wait_for()with appropriate timeout constraints
Test queue operations using non-blockingqueue.put_nowait()andasyncio.wait_for(queue.get(), timeout=...)to verify queue processing without blocking
Use@pytest.mark.no_blockbustermarker to skip the blockbuster plugin in specific tests
For database tests that may fail in batch runs, run them sequentially usinguv run pytest src/backend/tests/unit/test_database.pyr...
Files:
src/backend/tests/unit/components/flow_controls/test_loop_freeze.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)
Check that test files follow the project's naming conventions (test_*.py for backend, *.test.ts for frontend)
Files:
src/backend/tests/unit/components/flow_controls/test_loop_freeze.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 tests should follow pytest structure with proper test_*.py naming
For async functions, ensure proper async testing patterns are used with pytest for backend
Files:
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py
🧠 Learnings (4)
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Test component versioning and backward compatibility using `file_names_mapping` fixture with `VersionComponentMapping` objects mapping component files across Langflow versions
Applied to files:
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Test both sync and async code paths, mock external dependencies appropriately, test error handling and edge cases, validate input/output behavior, and test component initialization and configuration
Applied to files:
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Create comprehensive unit tests for all new backend components. If unit tests are incomplete, create a corresponding Markdown file documenting manual testing steps and expected outcomes
Applied to files:
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Use predefined JSON flows and utility functions from `tests.unit.build_utils` (create_flow, build_flow, get_build_events, consume_and_assert_stream) for flow execution testing
Applied to files:
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py
🧬 Code graph analysis (2)
src/lfx/src/lfx/graph/graph/base.py (1)
src/lfx/src/lfx/graph/vertex/base.py (1)
is_loop(123-127)
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py (2)
src/lfx/src/lfx/graph/vertex/base.py (1)
is_loop(123-127)src/lfx/src/lfx/components/flow_controls/loop.py (3)
LoopComponent(10-163)item_output(91-112)done_output(123-135)
🪛 GitHub Actions: Ruff Style Check
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py
[error] 10-10: Ruff I001 Import block is un-sorted or un-formatted.
🪛 GitHub Check: Ruff Style Check (3.13)
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py
[failure] 466-466: Ruff (F841)
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py:466:9: F841 Local variable should_build is assigned to but never used
[failure] 10-11: Ruff (I001)
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py:10:1: I001 Import block is un-sorted or un-formatted
⏰ 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). (10)
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
- GitHub Check: Run Backend Tests / LFX Tests - Python 3.10
- GitHub Check: Run Frontend Unit Tests / Frontend Jest Unit Tests
- GitHub Check: Test Starter Templates
🔇 Additional comments (3)
src/lfx/src/lfx/graph/vertex/base.py (1)
738-742: LGTM! Correctly prevents frozen Loop components from using cached results.The logic correctly identifies Loop components using both
display_name == "Loop"and theis_loopproperty (which checks forallows_loop=Truein outputs). This ensures Loop components always execute theirbuild()method to iterate through data correctly.The detection logic matches the corresponding change in
src/lfx/src/lfx/graph/graph/base.py, maintaining consistency across both build paths.src/lfx/src/lfx/graph/graph/base.py (1)
1522-1525: LGTM! Correctly forces rebuild for frozen Loop components at the graph level.The logic is consistent with the corresponding change in
vertex/base.py. This ensures that at both the graph build decision point and the vertex build execution point, Loop components always rebuild to properly iterate through their data.src/backend/tests/unit/components/flow_controls/test_loop_freeze.py (1)
1-8: Good test coverage for the Loop freeze fix.The test module provides comprehensive coverage including:
- Property detection logic
- Build decision logic at both graph and vertex levels
- All combinations of frozen/built/is_loop states
- Real component output configuration validation
- Edge cases for unusual inputs
The tests effectively validate the fix for frozen Loop components running infinitely.
| import pytest | ||
| from unittest.mock import Mock |
There was a problem hiding this comment.
Fix import order to resolve pipeline failure.
The imports are not sorted correctly. Standard library imports should come before third-party imports.
Apply this diff:
-import pytest
-from unittest.mock import Mock
+from unittest.mock import Mock
+
+import pytest📝 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.
| import pytest | |
| from unittest.mock import Mock | |
| from unittest.mock import Mock | |
| import pytest |
🧰 Tools
🪛 GitHub Actions: Ruff Style Check
[error] 10-10: Ruff I001 Import block is un-sorted or un-formatted.
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 10-11: Ruff (I001)
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py:10:1: I001 Import block is un-sorted or un-formatted
🤖 Prompt for AI Agents
In src/backend/tests/unit/components/flow_controls/test_loop_freeze.py around
lines 10 to 11, the import order is incorrect: standard library imports must
precede third-party imports. Reorder imports so any Python stdlib modules (e.g.,
unittest.mock) come before third-party ones (e.g., pytest); in this case import
Mock from unittest.mock before importing pytest, and ensure imports are sorted
and grouped consistently with project style.
| def test_none_values_handled(self): | ||
| """Test that None values are handled gracefully.""" | ||
| vertex = Mock() | ||
| vertex.display_name = None | ||
| vertex.is_loop = None | ||
| vertex.frozen = True | ||
| vertex.built = True | ||
|
|
||
| # Should not crash, should not be detected as loop | ||
| is_loop_component = vertex.display_name == "Loop" or vertex.is_loop | ||
| should_build = not vertex.frozen or is_loop_component | ||
|
|
||
| assert is_loop_component is None # None or None = None | ||
| # But in boolean context, None is falsy | ||
| should_build_bool = not vertex.frozen or bool(is_loop_component) | ||
| assert should_build_bool is False |
There was a problem hiding this comment.
Fix unused variable and clarify test intent.
The static analysis correctly flagged that should_build on line 466 is assigned but never used. Additionally, this test may not accurately reflect production behavior since the production code would treat None as falsy in boolean context.
Consider removing the unused variable or asserting on it directly:
def test_none_values_handled(self):
"""Test that None values are handled gracefully."""
vertex = Mock()
vertex.display_name = None
vertex.is_loop = None
vertex.frozen = True
vertex.built = True
# Should not crash, should not be detected as loop
is_loop_component = vertex.display_name == "Loop" or vertex.is_loop
- should_build = not vertex.frozen or is_loop_component
assert is_loop_component is None # None or None = None
# But in boolean context, None is falsy
- should_build_bool = not vertex.frozen or bool(is_loop_component)
- assert should_build_bool is False
+ should_build = not vertex.frozen or is_loop_component
+ # In production, `not True or None` evaluates to None, which is falsy
+ assert not should_build # None is falsy, so frozen non-loop won't build🧰 Tools
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 466-466: Ruff (F841)
src/backend/tests/unit/components/flow_controls/test_loop_freeze.py:466:9: F841 Local variable should_build is assigned to but never used
🤖 Prompt for AI Agents
In src/backend/tests/unit/components/flow_controls/test_loop_freeze.py around
lines 456-471, the local variable should_build is assigned but never used;
either remove the unused assignment or assert its expected value to reflect real
evaluation (not vertex.frozen evaluates to False so should_build becomes None
because False or None yields None). Update the test to either delete the
should_build line and keep the boolean-context assertion, or replace the unused
assignment with an assertion like assert should_build is None so the variable is
exercised and the test intent is explicit.
erichare
left a comment
erichare
left a comment
There was a problem hiding this comment.
LGTM Only question @Cristhianzl is do we need the package-lock.json update? maybe not a big deal?
|
@erichare not a big deal. |
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
…into cz/fix-freeze-loop
github-actions
bot
added
bug
github-actions
bot
added
bug
Summary
build_vertexandvertex.build()to always execute Loop components even when frozenbuild()method to properly iterate through data, regardless of frozen stateChanges
src/lfx/src/lfx/graph/graph/base.py: Add check for Loop component before skipping build on frozen verticessrc/lfx/src/lfx/graph/vertex/base.py: Add check to prevent returning cached result for frozen Loop componentsTest plan
is_loopproperty detectionbuild_vertexLoop exception logicvertex.build()Loop exception logicSummary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.