Skip to content

fix: args name error in logic component#10375

Closed
nightosong wants to merge 0 commit into
langflow-ai:mainfrom
nightosong:main
Closed

fix: args name error in logic component#10375
nightosong wants to merge 0 commit into
langflow-ai:mainfrom
nightosong:main

Conversation

@nightosong
Copy link
Copy Markdown

@nightosong nightosong commented Oct 23, 2025

Fixed incorrect parameter names in logic router components.

  • Changed is_list=True to list=True in DataInput initialization
  • Changed Message(content=) to Message(text=) to match the correct API

Summary by CodeRabbit

  • Refactor

    • Updated message property naming and input-list parameter naming across components to align with the current public API.
  • Tests

    • Adjusted unit tests to reflect the updated message property naming.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 23, 2025

Walkthrough

Update caller code to use the renamed Message constructor parameter text instead of content, and change a DataInput declaration argument from is_list to list in the data conditional router. Tests updated accordingly.

Changes

Cohort / File(s) Summary
Conditional router — Message API
src/lfx/src/lfx/components/logic/conditional_router.py
Replaced Message(content=...) usages with Message(text=...) in branches that return default/plain responses.
Data conditional router — DataInput param
src/lfx/src/lfx/components/logic/data_conditional_router.py
Changed DataInput declaration argument from is_list=True to list=True.
Unit tests — Message constructor updates
src/backend/tests/unit/components/logic/test_conditional_router.py
Updated test instances to construct Message with text= instead of content= (three occurrences).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Potential attention points:

  • Ensure no other modules or tests still use Message(content=...) or DataInput(is_list=...).
  • Confirm Message signature change is backward-compatible or intentionally breaking.
  • Verify linter/type checks for the renamed list argument (reserved word concerns) and DataInput constructor.

Suggested labels

size:S

Suggested reviewers

  • erichare

Pre-merge checks and finishing touches

❌ Failed checks (1 error, 2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Test Coverage For New Implementations ❌ Error The PR definitively fails the test coverage check. The test file contains critical inconsistencies that prove tests were not properly updated to match the API changes described in the PR. Specifically: (1) the default_kwargs fixture (lines 23-24) still uses the old Message(content="...") syntax instead of Message(text=...), and (2) test assertions on lines 255 and 286 check result.content == "" when the Message class has no content attribute—only a text field. These tests will fail immediately when executed. The Message class definition clearly shows text: str | AsyncIterator | Iterator | None = Field(default="") but no content field. This indicates the PR changes to component files were made, but the corresponding test file was not properly updated, and no regression tests were added to validate the parameter name changes work correctly. To pass this check, immediately update the test file to: (1) change all Message(content=...) instances to Message(text=...) in the default_kwargs fixture and any other fixture definitions (lines 23-24), (2) change all assertions from result.content to result.text throughout the test file (at minimum lines 255, 286), (3) verify the DataInput parameter change is properly tested by checking assertions reference the correct parameter name, and (4) run the full test suite to confirm all tests pass with these corrections. Additionally, consider adding explicit regression tests that specifically validate the parameter name fixes—tests that would have caught this inconsistency before PR submission.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Test Quality And Coverage ⚠️ Warning
Test File Naming And Structure ❓ Inconclusive The test file test_conditional_router.py follows pytest conventions with the test_*.py naming pattern and is located in the appropriate directory structure matching the component being tested. According to Langflow's test conventions, test files should follow the same directory structure as the component being tested and use the pattern test_<file_name>.py. The file path src/backend/tests/unit/components/logic/test_conditional_router.py correctly mirrors the component structure. However, without examining the actual test content (test function names, class organization, and test coverage), it cannot be conclusively verified that all aspects of the custom check are fully met, particularly regarding descriptive test function names, test organization into classes, positive and negative scenarios, and edge case coverage. To complete the verification, examine the test file directly to confirm: (1) test functions follow the pattern test_<case_name> with descriptive names, (2) tests are organized into classes named Test<ClassName> as per Langflow conventions, (3) test coverage includes positive scenarios (correct message construction with text= parameter), negative scenarios (error conditions), and edge cases, and (4) tests follow the Arrange-Act-Assert pattern with proper setup and teardown.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix: args name error in logic component" is directly related to the main changes in the changeset. The PR fixes two specific argument/parameter naming issues: changing is_list=True to list=True in the DataInput configuration and updating Message(content=...) to Message(text=...) to match the correct API. The title accurately characterizes these changes as fixing argument name errors in logic components. It is concise, uses the appropriate "fix:" prefix convention, and clearly conveys that this is a bug fix addressing naming issues.
Excessive Mock Usage Warning ✅ Passed The test file test_conditional_router.py contains 19 test methods, of which 12 (63%) do not use any mocks at all, indicating generally sound test design. The 7 tests that do use mocks primarily target framework infrastructure dependencies (ctx, graph, update_ctx, stop, _id), which is appropriate for isolating component logic. The PR itself only updates Message constructor calls from content= to text= parameters and does not introduce new mocks or alter test structure. While 4 tests do mock the component's own iterate_and_stop_once() method, this pattern is justified because those tests verify the calling behavior of true_response() and false_response() rather than testing the implementation of the called method itself.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent 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 ec3b103 and dae847f.

📒 Files selected for processing (1)
  • src/backend/tests/unit/components/logic/test_conditional_router.py (3 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
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/logic/test_conditional_router.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/logic/test_conditional_router.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/logic/test_conditional_router.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/logic/test_conditional_router.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/logic/test_conditional_router.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 if tests have too many mock objects that obscure what’s actually being tested
Warn when mocks are used instead of testing real behavior and interactions
Suggest using real objects or simpler test doubles when mocks become excessive
Ensure mocks are used only for external dependencies, not core business logic
Recommend integration tests when unit tests become overly mocked
Check that test files follow the project’s naming conventions (backend: test_*.py; frontend: *.test.ts/tsx)
Verify that tests actually exercise the new or changed functionality, not placeholder assertions
Test files should have descriptive test function names explaining what is being tested
Organize tests logically with proper setup and teardown
Include edge cases and error conditions for comprehensive coverage
Verify tests cover both positive (success) and negative (failure) scenarios
Ensure tests are not mere smoke tests; they should validate behavior thoroughly
Ensure tests follow the project’s testing frameworks (pytest for backend, Playwright for frontend)

Files:

  • src/backend/tests/unit/components/logic/test_conditional_router.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 must be named test_*.py and use proper pytest structure (fixtures, assertions)
For async backend code, use proper pytest async patterns (e.g., pytest-asyncio)
For API endpoints, include tests for both success and error responses

Files:

  • src/backend/tests/unit/components/logic/test_conditional_router.py
🔇 Additional comments (3)
src/backend/tests/unit/components/logic/test_conditional_router.py (3)

26-27: LGTM! Message API update correctly applied.

The Message constructor calls have been properly updated to use the text parameter.


226-226: LGTM! Message constructor updated correctly.


265-265: LGTM! Message constructor updated correctly.


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.

@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Oct 23, 2025
@nightosong
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 29, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant