mirror of
https://github.com/langflow-ai/langflow.git
synced 2026-07-24 05:39:16 +08:00
fix(extension): accept Output(method=...) in build-method validator
The validator's static AST check looked for a literal ``build`` method
on every Component subclass and emitted ``build-method-missing`` when
absent. Real Langflow components -- including the production
``DuckDuckGoSearchComponent`` and the in-tree ``ArXivComponent`` -- do
not declare a literal ``build``; they declare entry-points
declaratively via ``outputs = [Output(name=..., method=<name>)]`` and
the named method is what gets called. Result: every clean port hit a
spurious ``build-method-missing`` error from ``lfx extension validate``.
Fix: extend ``_has_build_method`` to also accept any class that names a
method via ``Output(method="X")`` AND defines that method in the class
body. The negative case ("Output(method=...) without a matching def")
still fires -- a typo'd method name would crash at runtime, so the
static check should keep flagging it.
Two new tests in ``test_validate.py`` lock the contract: positive case
mirroring duckduckgo / arxiv, negative case for typo'd method names.
All 26 validator tests pass.
PORTING.md updates fall out of running the recipe live against the
duckduckgo bundle on this branch:
* Validate path is ``src/bundles/<bundle>/src/lfx_<bundle>``, not the
bundle root (the manifest lives next to ``__init__.py``).
* Index regen needs ``LFX_DEV=1`` to skip the prebuilt-index fast path
and uses ``scripts/build_component_index.py``.
* Drop the migration-table JSON from the ruff invocation (ruff treats
it as Python and complains about the top-level expression).
This commit is contained in:
@ -250,13 +250,15 @@ The pre-built component index drives lazy loading; the moved component's
|
||||
old entry must be removed.
|
||||
|
||||
```bash
|
||||
cd src/lfx
|
||||
LFX_DEV=1 uv run python -m lfx._dev.build_component_index
|
||||
LFX_DEV=1 uv run python scripts/build_component_index.py
|
||||
```
|
||||
|
||||
(Or whichever one-shot is in your tree — see `scripts/`. The diff should
|
||||
only delete the `<bundle>` block; if it touches anything else, your local
|
||||
checkout has unrelated drift.)
|
||||
`LFX_DEV=1` forces dynamic discovery via `pkgutil.walk_packages`; without
|
||||
it the script reads the existing index and reproduces the stale entry
|
||||
even when the source module is gone.
|
||||
|
||||
The diff should only delete the `<bundle>` block; if it touches anything
|
||||
else, your local checkout has unrelated drift.
|
||||
|
||||
---
|
||||
|
||||
@ -286,8 +288,11 @@ Run, in order, the smallest commands that will fail loudly when a step is
|
||||
wrong:
|
||||
|
||||
```bash
|
||||
# 1. The bundle's manifest is structurally valid.
|
||||
uv run lfx extension validate src/bundles/<bundle>
|
||||
# 1. The bundle's manifest is structurally valid. Point ``validate`` at
|
||||
# the package directory (where extension.json lives), not the bundle
|
||||
# root -- the manifest is nested inside ``src/lfx_<bundle>/`` so the
|
||||
# wheel ships it.
|
||||
uv run lfx extension validate src/bundles/<bundle>/src/lfx_<bundle>
|
||||
|
||||
# 2. Workspace resolves and the bundle is importable.
|
||||
uv sync
|
||||
@ -307,8 +312,10 @@ print('discovered:', roots['lfx-<bundle>'])
|
||||
# 5. The integration test passes.
|
||||
uv run pytest src/lfx/tests/integration/extension/test_pilot_<bundle>_upgrade.py -q
|
||||
|
||||
# 6. Ruff is clean across the touched files.
|
||||
uv run ruff check src/bundles/<bundle> src/lfx/src/lfx/components/__init__.py src/lfx/src/lfx/extension/migration/migration_table.json
|
||||
# 6. Ruff is clean across the touched Python files. (Don't pass the
|
||||
# JSON migration table to ruff -- it lints it as Python and complains
|
||||
# about the top-level expression.)
|
||||
uv run ruff check src/bundles/<bundle> src/lfx/src/lfx/components/__init__.py src/lfx/tests/integration/extension/test_pilot_<bundle>_upgrade.py
|
||||
```
|
||||
|
||||
**End-to-end smoke test** (optional but cheap): start a dev server with
|
||||
|
||||
@ -376,8 +376,60 @@ def _is_component_class(node: ast.ClassDef) -> bool:
|
||||
|
||||
|
||||
def _has_build_method(node: ast.ClassDef) -> bool:
|
||||
"""Return True if the class body declares a ``build`` callable."""
|
||||
return any(isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)) and item.name == "build" for item in node.body)
|
||||
"""Return True if the class body declares an invocable entry-point.
|
||||
|
||||
Langflow Components reach the runtime in two shapes:
|
||||
|
||||
1. A literal ``build`` method on the class (the original convention).
|
||||
2. A method named in an ``Output(method="...")`` declaration in the
|
||||
class-level ``outputs = [...]`` assignment. This is the more
|
||||
common modern shape: a component can declare multiple outputs that
|
||||
each call a different method. Neither DuckDuckGo nor arXiv ships a
|
||||
literal ``build``; both are valid.
|
||||
|
||||
The validator accepts either form. If a class declares
|
||||
``Output(method="X")`` but no ``def X(...)`` in the class body,
|
||||
that's still flagged -- it would crash at run-time anyway.
|
||||
"""
|
||||
method_names = {item.name for item in node.body if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef))}
|
||||
if "build" in method_names:
|
||||
return True
|
||||
return any(invocable in method_names for invocable in _output_method_names(node))
|
||||
|
||||
|
||||
def _output_method_names(node: ast.ClassDef) -> set[str]:
|
||||
"""Collect method names referenced by ``Output(method="...")`` calls in the class body.
|
||||
|
||||
Walks the class-level assignment ``outputs = [...]`` and pulls the
|
||||
string value of every ``method=`` keyword argument on a call whose
|
||||
callee is the name ``Output`` (or anything ending in ``Output``).
|
||||
Static analysis only -- if the list is built dynamically, we miss
|
||||
those names and the literal-``build`` fallback still applies.
|
||||
"""
|
||||
names: set[str] = set()
|
||||
for item in node.body:
|
||||
if not isinstance(item, ast.Assign):
|
||||
continue
|
||||
if not any(isinstance(t, ast.Name) and t.id == "outputs" for t in item.targets):
|
||||
continue
|
||||
for sub in ast.walk(item.value):
|
||||
if not isinstance(sub, ast.Call):
|
||||
continue
|
||||
callee = sub.func
|
||||
if isinstance(callee, ast.Name):
|
||||
callee_name = callee.id
|
||||
elif isinstance(callee, ast.Attribute):
|
||||
callee_name = callee.attr
|
||||
else:
|
||||
continue
|
||||
if callee_name != "Output" and not callee_name.endswith("Output"):
|
||||
continue
|
||||
for kw in sub.keywords:
|
||||
if kw.arg != "method":
|
||||
continue
|
||||
if isinstance(kw.value, ast.Constant) and isinstance(kw.value.value, str):
|
||||
names.add(kw.value.value)
|
||||
return names
|
||||
|
||||
|
||||
def _find_top_level_io(tree: ast.AST) -> list[tuple[int, str]]:
|
||||
|
||||
@ -174,6 +174,54 @@ def test_build_method_missing_flagged(tmp_path: Path) -> None:
|
||||
assert "build-method-missing" in _codes(report)
|
||||
|
||||
|
||||
def test_output_method_satisfies_invocable_check(tmp_path: Path) -> None:
|
||||
"""A class with ``Output(method="X")`` + ``def X`` is invocable without literal ``build``.
|
||||
|
||||
Mirrors the production duckduckgo / arxiv bundles, which declare their
|
||||
entry-point via ``outputs = [Output(method="...")]`` rather than a
|
||||
literal ``build`` method. The validator must accept this shape;
|
||||
flagging it would force every modern Component to add a vestigial
|
||||
``build`` stub.
|
||||
"""
|
||||
src = (
|
||||
"class Component:\n"
|
||||
" pass\n\n"
|
||||
"class Output:\n"
|
||||
" def __init__(self, **kwargs):\n"
|
||||
" pass\n\n"
|
||||
"class OpenAIThing(Component):\n"
|
||||
" display_name = 'X'\n"
|
||||
" outputs = [Output(name='dataframe', method='fetch_content_dataframe')]\n\n"
|
||||
" def fetch_content_dataframe(self):\n"
|
||||
" return None\n"
|
||||
)
|
||||
_make_bundle(tmp_path, files={"text.py": src})
|
||||
report = validate_extension(tmp_path)
|
||||
assert "build-method-missing" not in _codes(report), report.errors.errors
|
||||
|
||||
|
||||
def test_output_method_without_matching_def_still_flagged(tmp_path: Path) -> None:
|
||||
"""``Output(method="X")`` without a ``def X`` does NOT satisfy the check.
|
||||
|
||||
Defense-in-depth: the static check passes only when the named method
|
||||
actually exists on the class. A typo'd method name would otherwise
|
||||
sneak past the validator and crash at run-time.
|
||||
"""
|
||||
src = (
|
||||
"class Component:\n"
|
||||
" pass\n\n"
|
||||
"class Output:\n"
|
||||
" def __init__(self, **kwargs):\n"
|
||||
" pass\n\n"
|
||||
"class OpenAIThing(Component):\n"
|
||||
" display_name = 'X'\n"
|
||||
" outputs = [Output(name='dataframe', method='typo_doesnt_exist')]\n"
|
||||
)
|
||||
_make_bundle(tmp_path, files={"text.py": src})
|
||||
report = validate_extension(tmp_path)
|
||||
assert "build-method-missing" in _codes(report)
|
||||
|
||||
|
||||
def test_import_star_flagged(tmp_path: Path) -> None:
|
||||
src = "from os.path import *\n" + _component_source()
|
||||
_make_bundle(tmp_path, files={"text.py": src})
|
||||
|
||||
Reference in New Issue
Block a user