-
Notifications
You must be signed in to change notification settings - Fork 8.2k
fix: Support tool mode in File Component properly #10520
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 WalkthroughThe changes add a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 inconclusive)
✅ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (39.97%) 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 #10520 +/- ##
=======================================
Coverage 32.13% 32.13%
=======================================
Files 1364 1364
Lines 62496 62496
Branches 9249 9249
=======================================
Hits 20082 20082
Misses 41401 41401
Partials 1013 1013
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/base/langflow/initial_setup/starter_projects/Vector Store RAG.json (1)
2995-3035: Advanced-mode gating vs compatibility list: csv listed as Docling-compatible but UI forbids it.UI gating hides Advanced Parser when any path endswith .csv/.xlsx/.parquet, yet _is_docling_compatible treats “.csv” as compatible. This inconsistency can surprise users if advanced_mode is set programmatically.
Consider removing “.csv” (and optionally spreadsheet formats) from the Docling compatibility list, or allow advanced_mode for these in the gating check. Example patch (remove csv from tuple):
- docling_exts = ( + docling_exts = ( ".adoc", ".asciidoc", ".asc", ".bmp", - ".csv", ".dotx", ".dotm", ".docm", ".docx", ".htm", ".html", ".jpg", ".jpeg", ".json", ".md", ".pdf", ".png", ".potx", ".ppsx", ".pptm", ".potm", ".ppsm", ".pptx", ".tiff", ".txt", ".xls", ".xlsx", ".xhtml", ".xml", ".webp", )
🧹 Nitpick comments (2)
src/backend/base/langflow/initial_setup/starter_projects/Vector Store RAG.json (2)
3065-3110: Path “blacklist” is unnecessary and may reject valid filenames.Since subprocess.run uses an argv list (no shell), characters like $, &, ;, ` are not interpreted. The check can block legitimate filenames. Prefer allowing all paths or restrict via allowlist (e.g., existence + isfile) instead.
- if not isinstance(args["file_path"], str) or any(c in args["file_path"] for c in [";", "|", "&", "$", "`"]): - return Data(data={"error": "Unsafe file path detected.", "file_path": args["file_path"]}) + # Basic validation without disallowing valid characters; avoid shell, already using argv list. + if not isinstance(args["file_path"], str) or not args["file_path"]: + return Data(data={"error": "Invalid file path.", "file_path": args.get("file_path")})
2925-2960: Minor: simplify file_path retrieval in update_outputs.You already computed paths = self._path_value(template). Reusing paths[0] is enough regardless of field_name.
- file_path = paths[0] if field_name == "path" else frontend_node["template"]["path"]["file_path"][0] + file_path = paths[0]
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/backend/base/langflow/initial_setup/starter_projects/Document Q&A.json(2 hunks)src/backend/base/langflow/initial_setup/starter_projects/News Aggregator.json(1 hunks)src/backend/base/langflow/initial_setup/starter_projects/Portfolio Website Code Generator.json(2 hunks)src/backend/base/langflow/initial_setup/starter_projects/Text Sentiment Analysis.json(2 hunks)src/backend/base/langflow/initial_setup/starter_projects/Vector Store RAG.json(2 hunks)src/backend/tests/unit/components/data/test_file_component.py(1 hunks)src/lfx/src/lfx/components/data/file.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
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/data/test_file_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/data/test_file_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/data/test_file_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/data/test_file_component.py
src/backend/**/*component*.py
📄 CodeRabbit inference engine (.cursor/rules/icons.mdc)
In your Python component class, set the
iconattribute to a string matching the frontend icon mapping exactly (case-sensitive).
Files:
src/backend/tests/unit/components/data/test_file_component.py
src/backend/**/components/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/icons.mdc)
In your Python component class, set the
iconattribute to a string matching the frontend icon mapping exactly (case-sensitive).
Files:
src/backend/tests/unit/components/data/test_file_component.py
🧠 Learnings (5)
📚 Learning: 2025-07-18T18:25:54.486Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/backend_development.mdc:0-0
Timestamp: 2025-07-18T18:25:54.486Z
Learning: Applies to src/backend/tests/unit/components/**/*.py : Create comprehensive unit tests for all new components
Applied to files:
src/backend/tests/unit/components/data/test_file_component.py
📚 Learning: 2025-07-21T14:16:14.125Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-21T14:16:14.125Z
Learning: Applies to src/backend/tests/**/*.py : Test component configuration updates in backend Python tests by asserting correct updates to build configuration objects.
Applied to files:
src/backend/tests/unit/components/data/test_file_component.py
📚 Learning: 2025-07-18T18:25:54.486Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/backend_development.mdc:0-0
Timestamp: 2025-07-18T18:25:54.486Z
Learning: Applies to src/backend/tests/unit/components/**/*.py : Mirror the component directory structure for unit tests in src/backend/tests/unit/components/
Applied to files:
src/backend/tests/unit/components/data/test_file_component.py
📚 Learning: 2025-07-21T14:16:14.125Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-21T14:16:14.125Z
Learning: Applies to src/backend/tests/**/*.py : Validate input/output behavior and test component initialization and configuration in backend Python tests.
Applied to files:
src/backend/tests/unit/components/data/test_file_component.py
📚 Learning: 2025-07-18T18:25:54.486Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/backend_development.mdc:0-0
Timestamp: 2025-07-18T18:25:54.486Z
Learning: Starter project files auto-format after langflow run; these formatting changes can be committed or ignored
Applied to files:
src/backend/base/langflow/initial_setup/starter_projects/Vector Store RAG.jsonsrc/backend/base/langflow/initial_setup/starter_projects/Text Sentiment Analysis.jsonsrc/backend/base/langflow/initial_setup/starter_projects/Portfolio Website Code Generator.jsonsrc/backend/base/langflow/initial_setup/starter_projects/Document Q&A.json
🧬 Code graph analysis (1)
src/backend/tests/unit/components/data/test_file_component.py (1)
src/lfx/src/lfx/components/data/file.py (1)
update_outputs(201-245)
🪛 GitHub Actions: Ruff Style Check
src/lfx/src/lfx/components/data/file.py
[error] 216-216: E501 Line too long (128 > 120)
🪛 GitHub Check: Ruff Style Check (3.13)
src/lfx/src/lfx/components/data/file.py
[failure] 243-243: Ruff (E501)
src/lfx/src/lfx/components/data/file.py:243:121: E501 Line too long (128 > 120)
[failure] 229-229: Ruff (E501)
src/lfx/src/lfx/components/data/file.py:229:121: E501 Line too long (124 > 120)
[failure] 226-226: Ruff (E501)
src/lfx/src/lfx/components/data/file.py:226:121: E501 Line too long (135 > 120)
[failure] 216-216: Ruff (E501)
src/lfx/src/lfx/components/data/file.py:216:121: E501 Line too long (128 > 120)
⏰ 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: Run Frontend Tests / Determine Test Suites and Shard Distribution
- GitHub Check: Lint Backend / Run Mypy (3.13)
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
- 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: Run Backend Tests / LFX Tests - Python 3.10
- GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
- GitHub Check: Test Starter Templates
- GitHub Check: Optimize new Python code in this PR
- GitHub Check: Update Component Index
- GitHub Check: test-starter-projects
🔇 Additional comments (10)
src/backend/base/langflow/initial_setup/starter_projects/Document Q&A.json (2)
1208-1212: PR description vs. diff: this is a functional change, not just a code_hash updateThe File component’s embedded code and outputs have been altered (e.g., new tooling logic, tool_mode propagation). The PR description claims only metadata/hash changes. Please update the PR summary to reflect the functional changes so reviewers understand scope.
1225-1239: Good: Raw Content output exposes tool_mode for File componentAdding
tool_mode: trueon the "Raw Content" output aligns the starter with tool-mode agent flows and matches the stated objective.src/backend/base/langflow/initial_setup/starter_projects/Text Sentiment Analysis.json (3)
2342-2342: Code hash update reflects tool_mode propagation in File component.The
code_hashchange from85abc1094130toda942f5bd4d9is expected and aligns with the PR objective to addtool_mode=Trueto the File component's Output declarations. This hash update indicates the underlying FileComponent source code has been modified.Per the retrieved learning, starter project files auto-format after langflow run; these metadata changes can be committed.
2356-2371: File component outputs correctly declare tool_mode=True.The "Raw Content" output at line 2365 correctly sets
"tool_mode": true, which aligns with the PR objective to support tool mode in dynamic outputs. This change ensures the File component output can be used in tool mode contexts.
2394-2411: Embedded FileComponent source code reflects tool_mode additions.The embedded Python code in the File component's code field (lines 2394–2411) contains the source implementation of FileComponent with updated Output declarations that include
tool_mode=True. This is consistent with the PR's objective to propagate tool_mode across the File component's outputs.For example, at the end of the code snippet, the
outputsdefinition includes:Output(display_name="Raw Content", name="message", method="load_files_message", tool_mode=True),This confirms the tool_mode parameter is properly integrated into the component definition.
src/backend/base/langflow/initial_setup/starter_projects/Portfolio Website Code Generator.json (2)
934-934: LGTM: Metadata update reflects component changes.The
code_hashupdate correctly reflects the modified File component implementation that now includestool_modesupport in outputs.Based on learnings: Starter project files auto-format after langflow run; these formatting changes can be committed.
1002-1002: LGTM: Embedded code reflects updated File component.The embedded File component code correctly includes the
tool_mode=Trueparameter in Output declarations, consistent with the changes in the actual component implementation.src/backend/tests/unit/components/data/test_file_component.py (1)
144-188: LGTM: Comprehensive test coverage for tool_mode propagation.This test effectively validates that
tool_mode=Trueis consistently set across all dynamic output scenarios. The test covers:
- Single CSV/JSON files
- Multiple files
- Advanced mode enabled/disabled
The clear docstring and descriptive assertions make the test easy to understand and maintain.
As per coding guidelines: Test validates input/output behavior and component configuration, following the recommended patterns for comprehensive component testing.
src/backend/base/langflow/initial_setup/starter_projects/Vector Store RAG.json (2)
2746-3110: tool_mode enabled across File outputs and dynamic update paths — LGTM.Outputs include tool_mode=True for Raw Content and all branches in update_outputs (structured/json/advanced/multi-file). This aligns with tool mode behavior.
Please run the unit test that validates dynamic outputs have tool_mode=True to ensure parity with this embedded component.
2678-2679: No issues found—code_hash regeneration and tool_mode settings are correct.Verification confirms:
- Code hash "da942f5bd4d9" appears exactly once (correct, no duplicates)
- All outputs have
tool_mode=Trueproperly set—both the static output definition and all 7 dynamically generated outputs inupdate_outputs()method
This pull request makes a minor update to the
Document Q&A.jsonstarter project by changing thecode_hashin the metadata. This likely reflects a new build or version of the project, but no functional or structural changes are introduced.Summary by CodeRabbit
New Features
Updates
Tests