fix(lfx): include components path in flow builder registry - #13623
fix(lfx): include components path in flow builder registry#13623luochen211 wants to merge 4 commits into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR extends registry loading throughout the stack to accept optional custom components paths, enabling bundled component indexes to be merged with custom component registries from caller-provided paths for search, description, and flow-building operations. ChangesCustom Components Path Registry Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
894008c to
5ff02eb
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lfx/src/lfx/graph/flow_builder/builder.py (1)
61-104: 💤 Low valueConsider returning a shallow copy of the cached merged registry to prevent cache corruption.
The merged registry is stored directly in
_registry_with_components_path_cacheand returned to callers. If any caller mutates the returned dict (e.g., adds/removes keys), the cached entry is silently corrupted for subsequent lookups.Currently,
user_components_overlay.pyalready creates a shallow copy before modifications (line 131), and typical search/describe operations are read-only, so the practical risk is low. However, for defensive consistency with the base registry path (which benefits from_registry_cachebeing only read), consider returningdict(merged)at line 104.🛡️ Optional defensive copy
_registry_with_components_path_cache[cache_key] = merged - return merged + return dict(merged)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lfx/src/lfx/graph/flow_builder/builder.py` around lines 61 - 104, The merged registry stored in _registry_with_components_path_cache is returned directly and can be mutated by callers; modify load_local_registry so that whenever a merged registry is returned from the cache or after computing/setting merged (references: function load_local_registry, variable _registry_with_components_path_cache, local name merged), return a shallow copy (e.g., dict(merged)) instead of the original object to prevent cache corruption from caller-side mutations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lfx/src/lfx/graph/flow_builder/builder.py`:
- Around line 61-104: The merged registry stored in
_registry_with_components_path_cache is returned directly and can be mutated by
callers; modify load_local_registry so that whenever a merged registry is
returned from the cache or after computing/setting merged (references: function
load_local_registry, variable _registry_with_components_path_cache, local name
merged), return a shallow copy (e.g., dict(merged)) instead of the original
object to prevent cache corruption from caller-side mutations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 240255fc-2978-4c25-b645-c0ed24e2baba
📒 Files selected for processing (4)
src/backend/base/langflow/agentic/services/user_components_overlay.pysrc/lfx/src/lfx/graph/flow_builder/builder.pysrc/lfx/src/lfx/mcp/flow_builder_tools/_state.pysrc/lfx/tests/unit/test_flow_builder_tools.py
5ff02eb to
4b2b964
Compare
|
Addressed the CodeRabbit test-quality warning by adding coverage for components_path normalization/cache reuse and missing-settings fallback, alongside the existing search/describe/build path coverage. Re-verified with |
4b2b964 to
2c74a1e
Compare
|
Also addressed CodeRabbit’s cached-registry nit: components_path cache hits and newly merged registries now return shallow copies so caller mutations cannot corrupt cached entries. The registry test now covers that behavior. Re-verified with |
|
CI looks blocked by runner package repository setup rather than this PR change: Playwright shard 63 failed before tests ran while installing browsers because |
…builder-components-path
Fixes #13619.
Summary
settings.components_pathcustom components into the flow-builder local registryLANGFLOW_COMPONENTS_PATHTests
uv run pytest tests/unit/test_flow_builder_tools.py -quv run ruff check src/lfx/graph/flow_builder/builder.py src/lfx/mcp/flow_builder_tools/_state.py ../backend/base/langflow/agentic/services/user_components_overlay.py tests/unit/test_flow_builder_tools.pyuv run ruff format --check src/lfx/graph/flow_builder/builder.py src/lfx/mcp/flow_builder_tools/_state.py ../backend/base/langflow/agentic/services/user_components_overlay.py tests/unit/test_flow_builder_tools.pygit diff --checkSummary by CodeRabbit
Release Notes
New Features
Tests