-
Notifications
You must be signed in to change notification settings - Fork 8.3k
fix: use lfx versions instead of langflow for comp index #11260
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
base: main
Are you sure you want to change the base?
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 pull request updates terminology from Langflow to LFX in the build component index script through function and constant renames, docstring updates, and type syntax modernization. Additionally, a new unit test module is added to verify starter project JSON files do not contain internal hash_history metadata. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (5 passed)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (40.80%) 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 #11260 +/- ##
==========================================
+ Coverage 34.12% 34.13% +0.01%
==========================================
Files 1408 1408
Lines 66770 66770
Branches 9858 9858
==========================================
+ Hits 22783 22792 +9
+ Misses 42794 42785 -9
Partials 1193 1193
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: 2
🤖 Fix all issues with AI agents
In @scripts/build_component_index.py:
- Around line 149-150: The rename of VERSION_FIRST_KEY/VERSION_LAST_KEY to
"v_from"/"v_to" breaks compatibility with existing component_index.json files;
update _load_existing_index to detect the legacy keys
"version_first"/"version_last" and migrate them to the new keys (or populate
both formats), and update _merge_hash_history to accept both key names during
the transition; ensure migration logic normalizes entries to use
VERSION_FIRST_KEY and VERSION_LAST_KEY constants so subsequent code (including
_merge_hash_history) works without raising KeyError, and add a brief comment
documenting the compatibility behavior.
In @src/backend/tests/unit/test_starter_projects_no_hash_history.py:
- Around line 80-87: The test_all_starter_projects_loaded test is missing the
visibility print mentioned in the comment; update the test to print the count
(and optionally file names) of starter projects after calling
get_starter_project_files(); e.g., invoke print(f"Found {len(project_files)}
starter project files") (or print the list) before the final assertions so test
output shows how many/which starter projects were discovered while keeping the
existing assertion using len(project_files) > 0.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
scripts/build_component_index.pysrc/backend/tests/unit/test_starter_projects_no_hash_history.pysrc/lfx/src/lfx/_assets/component_index.json
🧰 Additional context used
📓 Path-based instructions (3)
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/test_starter_projects_no_hash_history.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/test_starter_projects_no_hash_history.py
**/test_*.py
📄 CodeRabbit inference engine (Custom checks)
**/test_*.py: Review test files for excessive use of mocks that may indicate poor test design - 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, and suggest using real objects or test doubles when mocks become excessive
Ensure mocks are used appropriately for external dependencies only, not for core logic
Backend test files should follow the naming convention test_*.py with proper pytest structure
Test files should have descriptive test function names that explain what is being tested
Tests should be organized logically with proper setup and teardown
Consider including edge cases and error conditions for comprehensive test coverage
Verify tests cover both positive and negative scenarios where appropriate
For async functions in backend tests, ensure proper async testing patterns are used with pytest
For API endpoints, verify both success and error response testing
Files:
src/backend/tests/unit/test_starter_projects_no_hash_history.py
🧠 Learnings (6)
📓 Common learnings
Learnt from: ogabrielluiz
Repo: langflow-ai/langflow PR: 0
File: :0-0
Timestamp: 2025-06-26T19:43:18.260Z
Learning: In langflow custom components, the `module_name` parameter is now propagated through template building functions to add module metadata and code hashes to frontend nodes for better component tracking and debugging.
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/langflow PR: 0
File: :0-0
Timestamp: 2025-08-07T20:23:23.569Z
Learning: Some Langflow starter project files and components still use `from loguru import logger` instead of the centralized structlog logger from `langflow.logging.logger`. These should be updated to ensure consistent structured logging across the entire codebase.
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
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/langflow PR: 0
File: :0-0
Timestamp: 2025-08-07T20:23:23.569Z
Learning: The Langflow codebase has an excellent structlog implementation that follows best practices, with proper global configuration, environment-based output formatting, and widespread adoption across components. The main cleanup needed is updating starter project templates and documentation examples that still contain legacy `from loguru import logger` imports.
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/docs_development.mdc:0-0
Timestamp: 2025-11-24T19:46:26.770Z
Learning: Applies to docs/docs/**/*.{md,mdx} : Use sentence case for headers and proper capitalization for terminology: Langflow, Component, Flow, API, JSON
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/docs_development.mdc:0-0
Timestamp: 2025-06-23T12:46:29.953Z
Learning: All terminology such as 'Langflow', 'Component', 'Flow', 'API', and 'JSON' must be capitalized or uppercased as specified in the terminology section.
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
📚 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/test_starter_projects_no_hash_history.py
📚 Learning: 2025-12-19T18:04:08.938Z
Learnt from: Jkavia
Repo: langflow-ai/langflow PR: 11111
File: src/backend/tests/unit/api/v2/test_workflow.py:10-11
Timestamp: 2025-12-19T18:04:08.938Z
Learning: In the langflow-ai/langflow repository, pytest-asyncio is configured with asyncio_mode = 'auto' in pyproject.toml. This means you do not need to decorate test functions or classes with pytest.mark.asyncio; async tests are auto-detected and run by pytest-asyncio. When reviewing tests, ensure they rely on this configuration (i.e., avoid unnecessary pytest.mark.asyncio decorators) and that tests living under any tests/ path (e.g., src/.../tests/**/*.py) follow this convention. If a test explicitly requires a different asyncio policy, document it and adjust the config accordingly.
Applied to files:
src/backend/tests/unit/test_starter_projects_no_hash_history.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 component versioning and backward compatibility using `file_names_mapping` fixture with `VersionComponentMapping` objects mapping component files across Langflow versions
Applied to files:
scripts/build_component_index.py
📚 Learning: 2025-11-24T19:46:09.104Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/backend_development.mdc:0-0
Timestamp: 2025-11-24T19:46:09.104Z
Learning: Applies to src/backend/base/langflow/components/**/__init__.py : Update `__init__.py` with alphabetically sorted imports when adding new components
Applied to files:
scripts/build_component_index.py
📚 Learning: 2025-08-01T20:12:57.303Z
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/langflow PR: 0
File: :0-0
Timestamp: 2025-08-01T20:12:57.303Z
Learning: The LangFlow components system has 47 modules with identical lazy loading patterns that can be optimized using a centralized lazy loading utility, eliminating over 1,000 lines of duplicated code while maintaining backward compatibility.
Applied to files:
scripts/build_component_index.py
⏰ 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). (27)
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 17/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 16/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 4/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 15/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 10/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 9/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 13/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 11/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 7/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 8/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 5/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 14/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 6/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 2/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 3/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 12/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 1/17
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- 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 / Integration Tests - Python 3.10
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
- GitHub Check: Lint Backend / Run Mypy (3.11)
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Test Starter Templates
- GitHub Check: Update Component Index
🔇 Additional comments (7)
src/backend/tests/unit/test_starter_projects_no_hash_history.py (3)
1-10: LGTM! Well-documented test module.The module docstring clearly explains the purpose and rationale for these tests. The imports are appropriate and minimal.
13-40: LGTM! Solid recursive search implementation.The function correctly handles nested dictionaries and lists, with proper path tracking for error reporting. The logic is clean and handles edge cases appropriately.
43-57: LGTM! Appropriate use of pytest.skip for missing resources.The path construction is correct, and the function properly handles cases where starter project files might not be present, using
pytest.skipinstead of failing the test.scripts/build_component_index.py (4)
260-260: LGTM! Docstring updates are consistent with terminology changes.The docstrings have been appropriately updated from "langflow" to "lfx" to match the overall PR objective of using lfx versions for component indexing.
Also applies to: 337-337
378-378: LGTM! Function call updated consistently.The function call has been updated to match the renamed
_get_lfx_version()function. However, ensure the package name "lfx" is verified as mentioned in the earlier comment on lines 22-31.
22-31: Verify that "lfx" is declared as a project dependency.The function now queries
version("lfx")instead ofversion("langflow"). While "lfx" (Langflow Executor CLI) is a real PyPI package, you need to confirm that your project'spyproject.tomlorsetup.pydeclares "lfx" as a dependency. If "lfx" is not listed as a dependency and is not installed in your environment, this will raise aPackageNotFoundErrorat runtime.
128-128: Union syntax is appropriate for this project.The project requires Python 3.10+ (as specified in
pyproject.toml:python = ">=3.10,<3.14"), and the modern PEP 604 union syntaxlist | tupleis valid and works correctly withisinstance()at runtime for non-parameterized types.
scripts/build_component_index.py
Outdated
| VERSION_FIRST_KEY = "v_from" # Hash valid from Version (inclusive) | ||
| VERSION_LAST_KEY = "v_to" # Hash valid to version (inclusive) |
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.
Breaking change: Verify backward compatibility with existing indexes.
The constant values changed from "version_first"/"version_last" to "v_from"/"v_to". This is a breaking change if an existing component_index.json file uses the old keys. When _load_existing_index() loads the old format and then _merge_hash_history() tries to access these keys, it will fail.
Consider:
- Adding migration logic to convert old keys to new keys when loading
- Supporting both key formats during a transition period
- Documenting this as a breaking change requiring index rebuild
#!/bin/bash
# Description: Check if existing component_index.json uses old key format
# Find component_index.json and check for old keys
echo "=== Checking for existing component_index.json with old key format ==="
fd "component_index.json" --exec sh -c 'echo "File: {}" && grep -o "version_first\|version_last" {} | head -5'🤖 Prompt for AI Agents
In @scripts/build_component_index.py around lines 149 - 150, The rename of
VERSION_FIRST_KEY/VERSION_LAST_KEY to "v_from"/"v_to" breaks compatibility with
existing component_index.json files; update _load_existing_index to detect the
legacy keys "version_first"/"version_last" and migrate them to the new keys (or
populate both formats), and update _merge_hash_history to accept both key names
during the transition; ensure migration logic normalizes entries to use
VERSION_FIRST_KEY and VERSION_LAST_KEY constants so subsequent code (including
_merge_hash_history) works without raising KeyError, and add a brief comment
documenting the compatibility behavior.
| def test_all_starter_projects_loaded(): | ||
| """Sanity check that we're actually testing starter projects.""" | ||
| project_files = get_starter_project_files() | ||
|
|
||
| # We should have multiple starter projects | ||
| assert len(project_files) > 0, "No starter project files found to test" | ||
|
|
||
| # Print count for visibility |
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.
Complete the implementation.
The comment on line 87 suggests there should be a print statement for visibility, but the implementation is incomplete.
📝 Suggested completion
def test_all_starter_projects_loaded():
"""Sanity check that we're actually testing starter projects."""
project_files = get_starter_project_files()
# We should have multiple starter projects
assert len(project_files) > 0, "No starter project files found to test"
# Print count for visibility
+ print(f"Testing {len(project_files)} starter project files")📝 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.
| def test_all_starter_projects_loaded(): | |
| """Sanity check that we're actually testing starter projects.""" | |
| project_files = get_starter_project_files() | |
| # We should have multiple starter projects | |
| assert len(project_files) > 0, "No starter project files found to test" | |
| # Print count for visibility | |
| def test_all_starter_projects_loaded(): | |
| """Sanity check that we're actually testing starter projects.""" | |
| project_files = get_starter_project_files() | |
| # We should have multiple starter projects | |
| assert len(project_files) > 0, "No starter project files found to test" | |
| # Print count for visibility | |
| print(f"Testing {len(project_files)} starter project files") |
🤖 Prompt for AI Agents
In @src/backend/tests/unit/test_starter_projects_no_hash_history.py around lines
80 - 87, The test_all_starter_projects_loaded test is missing the visibility
print mentioned in the comment; update the test to print the count (and
optionally file names) of starter projects after calling
get_starter_project_files(); e.g., invoke print(f"Found {len(project_files)}
starter project files") (or print the list) before the final assertions so test
output shows how many/which starter projects were discovered while keeping the
existing assertion using len(project_files) > 0.
Comps live in lfx, so use lfx for versioning.
Summary by CodeRabbit
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.