feat: Better multi-file consistency for File Component#8625
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 WalkthroughThis change updates the FileComponent in both backend code and multiple starter project configurations. It enables real-time refresh on the file path input and introduces a method to dynamically adjust output ports based on the number of files selected. Node metadata is also updated in several starter project JSON files for output selection consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant FileComponent
User->>Frontend: Selects file(s) via FileInput ("path")
Frontend->>FileComponent: Triggers update_outputs on "path" change
FileComponent->>Frontend: Returns updated node outputs (based on file count)
User->>Frontend: Initiates processing
Frontend->>FileComponent: Calls process_files with selected files
FileComponent->>Frontend: Returns processed file(s) output
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (6)
src/backend/base/langflow/initial_setup/starter_projects/Text Sentiment Analysis.json (2)
308-336:update_outputsmis-classifies single path strings as “multiple files”.
len(self.path) > 1is truthy for every non-empty string (e.g."/tmp/file.txt"→len(..)=14).
Consequently, a single file given as a string will be treated as a multi-file selection, hiding the “Structured File / Raw File” outputs.- if len(self.path) > 1: + # Detect true “multi-file” only when the incoming value is a list + if isinstance(field_value, list) and len(field_value) > 1:Leverage the already-provided
field_valueinstead of the potentially staleself.path.
This avoids UI confusion and keeps the dynamic-output feature reliable.
340-390: Parallel-processing threshold & deprecation toggle are drifting.
use_multithreadingis flagged [Deprecated] yet the concurrency logic still depends on it, forcing users to set two different knobs (use_multithreading=Trueandconcurrency_multithreading>1) to enable parallelism.
Consider simplifying:
- Drop the boolean entirely and infer intent from
concurrency_multithreading.- Or keep the flag but ignore the integer when it’s
<=1.- concurrency = 1 if not self.use_multithreading else max(1, self.concurrency_multithreading) + # Treat any value >1 in `concurrency_multithreading` as the single source of truth + concurrency = max(1, self.concurrency_multithreading)Reduces user friction and cleans up the deprecated path sooner.
src/backend/base/langflow/initial_setup/starter_projects/Portfolio Website Code Generator.json (1)
1867-1889:update_outputsuses staleself.pathand mis-detects single vs. multi-file cases
update_outputsrelies onlen(self.path)while the newfield_valuecontaining the current value of the path is passed as an argument.
If the method is triggered before thepathattribute is updated (which is usually the case in build-config hooks), the length check is performed on the previous value, leading to an incorrect set of outputs.- # Add outputs based on the number of files in the path - if len(self.path) > 1: + # Prefer `field_value` (freshest value) and gracefully fallback + paths = field_value or self.path + # Handle both list-of-files and single-string cases + file_count = len(paths) if isinstance(paths, (list, tuple)) else 1 + + if file_count > 1:This also fixes the subtle bug where a single file given as a string with more than one character (
"a","b", …) is treated as “multiple files”.src/backend/base/langflow/initial_setup/starter_projects/Vector Store RAG.json (2)
2475-2496:update_outputsrelies onself.pathinstead of the incomingfield_value
update_outputsis triggered by the front-end each time a field changes and you already receivefield_name&field_value.
Usingself.pathhere is racy:• When the user first picks a file, the attribute may not yet be updated, so
len(self.path)could still be0→ no outputs rendered.
• Ifpathis a string (single file path) itslen()counts characters, not the number of files, so every non-empty path is treated as “multiple files”.- if field_name == "path": + if field_name == "path": # Start with empty outputs frontend_node["outputs"] = [] - # Add outputs based on the number of files in the path - if len(self.path) > 1: + # Decide based on the *new* value coming from the UI + file_count = len(field_value) if isinstance(field_value, list) else 1 + + if file_count > 1: frontend_node["outputs"].append( Output(display_name="Raw Files", name="dataframe", method="load_files"), ) else: frontend_node["outputs"].append( Output(display_name="Structured File", name="dataframe", method="load_files"), ) frontend_node["outputs"].append( Output(display_name="Raw File", name="message", method="load_files_message"), )This guarantees deterministic behaviour and correct single-vs-multi file detection.
2543-2560: Concurrency flag logic is backwards‐compatible but confusing
concurrency = 1 if not self.use_multithreading else max(1, self.concurrency_multithreading)
- The new guidance is to ignore
use_multithreadingand rely solely onconcurrency_multithreading; however this keeps the old flag alive, forcing users to set two inputs for parallelism.- With the default
concurrency_multithreading = 1, the component never crosses theparallel_processing_threshold = 2, so even whenuse_multithreading=Truenothing happens.Recommend:
- concurrency = 1 if not self.use_multithreading else max(1, self.concurrency_multithreading) + # honour new field first, fall back to deprecated flag for BC + concurrency = max(1, self.concurrency_multithreading) + + # `use_multithreading` kept only for BC: if users untick it, force sequential + if not self.use_multithreading: + concurrency = 1and document this in the component description.
src/backend/base/langflow/initial_setup/starter_projects/Document Q&A.json (1)
835-870:update_outputsusesself.pathinstead of the just-edited value – can desync UI & backend
update_outputsis invoked while the “path” field is being changed, but the component attribute (self.path) has not yet been mutated.
If the user switches from one file to many (or vice-versa) the length check will be performed on the stale value and the frontend may receive the wrong port list.- if field_name == "path": + if field_name == "path": # Start with empty outputs frontend_node["outputs"] = [] - if len(self.path) > 1: + files = field_value if isinstance(field_value, (list, tuple)) else [field_value] + if len(files) > 1:Leaning on
field_valueguarantees we evaluate against the new selection and sidesteps any mutation-order edge-cases.
🧹 Nitpick comments (5)
src/backend/base/langflow/components/data/file.py (1)
60-70: Enhance robustness with defensive programming.Consider adding defensive checks to handle edge cases more gracefully:
def update_outputs(self, frontend_node: dict, field_name: str, field_value: Any) -> dict: # noqa: ARG002 """Dynamically show only the relevant output based on the number of files processed.""" if field_name == "path": # Start with empty outputs frontend_node["outputs"] = [] # Add outputs based on the number of files in the path - if len(self.path) > 1: + path_length = len(self.path) if self.path else 0 + if path_length > 1: frontend_node["outputs"].append( Output(display_name="Raw Files", name="dataframe", method="load_files"), ) - else: + elif path_length == 1: frontend_node["outputs"].append( Output(display_name="Structured File", name="dataframe", method="load_files"), ) frontend_node["outputs"].append( Output(display_name="Raw File", name="message", method="load_files_message"), ) + # If path_length == 0, outputs remain empty which is appropriate return frontend_nodesrc/backend/base/langflow/initial_setup/starter_projects/Portfolio Website Code Generator.json (2)
1889-1906: Output names & display names are inconsistent with the methods they callFor the multi-file branch an output is registered as
Output(display_name="Raw Files", name="dataframe", method="load_files")
load_filesreturns a DataFrame/structured table, not a raw byte/string blob, whereas the display name claims “Raw”. Conversely the single-file branch callsload_files_message(which is the raw path) but labels the first output “Structured File”.Consider aligning naming with behaviour:
- Output(display_name="Raw Files", name="dataframe", method="load_files"), + Output(display_name="Structured Files", name="dataframe", method="load_files"), … - Output(display_name="Structured File", name="dataframe", method="load_files"), - Output(display_name="Raw File", name="message", method="load_files_message"), + Output(display_name="Structured File", name="dataframe", method="load_files"), + Output(display_name="Raw Content", name="message", method="load_files_message"),This avoids confusing the UI/JSON consumers.
1920-1955:concurrencylogic still depends on deprecateduse_multithreadingflagconcurrency = 1 if not self.use_multithreading else max(1, self.concurrency_multithreading)This silently ignores a user-provided
concurrency_multithreading > 1whenever the (now deprecated) flag is false. Since the UI surfaces “Processing Concurrency” as the canonical switch, drop the flag and rely solely onconcurrency_multithreading:- concurrency = 1 if not self.use_multithreading else max(1, self.concurrency_multithreading) + concurrency = max(1, self.concurrency_multithreading)You can keep backward-compatibility by mapping
use_multithreading=True⇒concurrency_multithreading = max(2, concurrency_multithreading)during migration.src/backend/base/langflow/initial_setup/starter_projects/Document Q&A.json (2)
835-870: Port display names don’t match their semantic roleWhen multiple files are chosen the code adds
Output(display_name="Raw Files", name="dataframe", method="load_files")Yet
load_filesreturns a structuredDataFrame, not raw text. Conversely for single-file mode “Structured File” is used for the samedataframeoutput, while “Raw File” maps to themessageport.The mismatch is confusing in the UI. Consider:
-Output(display_name="Raw Files", name="dataframe", method="load_files") +Output(display_name="Structured Files", name="dataframe", method="load_files")(or flip the port names) so users immediately understand which handle carries structured vs. raw content.
835-870: Deprecateduse_multithreadinggate still drives concurrency
concurrency_multithreadingis ignored unless the deprecated flag isTrue:concurrency = 1 if not self.use_multithreading else max(1, self.concurrency_multithreading)Users setting only
concurrency_multithreading> 1 will be puzzled that parallelism never kicks in.
Either:
- Drop the boolean and treat
concurrency_multithreading > 1as the single source of truth, or- Flip the default of
use_multithreadingtoTrueand mark the field as hidden.This keeps behaviour aligned with the “[Deprecated]” label while preventing silent performance regressions.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/backend/base/langflow/components/data/file.py(3 hunks)src/backend/base/langflow/initial_setup/starter_projects/Document Q&A.json(6 hunks)src/backend/base/langflow/initial_setup/starter_projects/Portfolio Website Code Generator.json(1 hunks)src/backend/base/langflow/initial_setup/starter_projects/Text Sentiment Analysis.json(4 hunks)src/backend/base/langflow/initial_setup/starter_projects/Vector Store RAG.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`src/backend/base/langflow/components/**/*`: Add new backend components to the appropriate subdirectory under src/backend/base/langflow/components/.
src/backend/base/langflow/components/**/*: Add new backend components to the appropriate subdirectory under src/backend/base/langflow/components/.
src/backend/base/langflow/components/data/file.py
`src/backend/**/components/**/*.py`: In your component class, set the 'icon' attribute to a string matching the frontend icon mapping exactly (case-sensitive).
src/backend/**/components/**/*.py: In your component class, set the 'icon' attribute to a string matching the frontend icon mapping exactly (case-sensitive).
src/backend/base/langflow/components/data/file.py
🔇 Additional comments (4)
src/backend/base/langflow/components/data/file.py (2)
1-2: LGTM - Necessary imports added for new functionality.The imports for
deepcopyandAnyare correctly added to support the new input modification logic and type annotations.
53-72: Verify dynamic output behavior for edge cases.The
update_outputsmethod looks well-implemented for the main use cases, but there are a few considerations:
- The method accesses
self.pathwithout null checking- Empty file lists might cause unexpected behavior
Please run the following script to verify how this method handles edge cases:
#!/bin/bash # Description: Search for similar update_outputs implementations and FileInput usage patterns # Expected: Find patterns for handling edge cases in similar methods # Search for other update_outputs methods to understand common patterns ast-grep --pattern 'def update_outputs($_, $_, $_) { $$$ }' # Search for FileInput real_time_refresh usage rg -A 3 -B 3 "real_time_refresh" # Search for path attribute usage in FileComponent rg -A 2 -B 2 "self\.path" src/backend/base/langflow/components/data/src/backend/base/langflow/initial_setup/starter_projects/Vector Store RAG.json (1)
2508-2542: Nestedprocess_filebreaks parallel execution ifparallel_load_datauses multiprocessing
process_fileis declared insideprocess_files. A nested (local) function is not picklable, so anymultiprocessingor process-based executor insideparallel_load_datawill fail with aPicklingError.Two safe fixes:
@@ -class FileComponent(BaseFileComponent): +class FileComponent(BaseFileComponent): ... + # --- helpers ----------------------------------------------------------- + +def _process_single_file(file_path: str, *, silent_errors: bool = False) -> Data | None: + """Standalone helper so it can be pickled / used by process pools.""" + from langflow.base.data.utils import parse_text_file_to_data # local import to avoid circulars + try: + return parse_text_file_to_data(file_path, silent_errors=silent_errors) + except Exception as e: + return e # propagate error for caller to handle + class FileComponent(BaseFileComponent): @@ - def process_file(file_path: str, *, silent_errors: bool = False) -> Data | None: - ... + # use the top-level helperor switch
parallel_load_datato a thread pool exclusively.Confirm the implementation of
parallel_load_databefore this ships.src/backend/base/langflow/initial_setup/starter_projects/Document Q&A.json (1)
415-417: Nice addition – explicitselected_outputgreatly improves default UXThe new
selected_outputentries for ChatInput, File, Prompt, OpenAIModel and Parser nodes remove ambiguity in multi-port components and ensure the graph wires up predictably.Also applies to: 1033-1035, 1191-1193, 1586-1588, 1761-1763
…gflow into feat-file-output-changes
This pull request makes output changes to the File Component to be more intuitive in both the single and multi file case.
Summary by CodeRabbit
New Features
Enhancements