Skip to content

fix(custom): lazy exec_globals in validate.prepare_global_scope - #12786

Merged
ogabrielluiz merged 5 commits into
cold-start/01-measurement-foundationfrom
cold-start/04-imp-11-lazy-validate
May 6, 2026
Merged

fix(custom): lazy exec_globals in validate.prepare_global_scope#12786
ogabrielluiz merged 5 commits into
cold-start/01-measurement-foundationfrom
cold-start/04-imp-11-lazy-validate

Conversation

@ogabrielluiz

Copy link
Copy Markdown
Contributor

Summary

Rewrites prepare_global_scope in lfx/custom/validate.py to defer langchain_* imports until the component class body actually references them. Components that do not use AgentExecutor (or similar) no longer trigger the langchain_classic.agents -> transformers -> torch chain during instantiation.

Scope

  • src/lfx/src/lfx/custom/validate.py:
    • New _LazyImportProxy + _LazyExecGlobals(dict) types; prepare_global_scope builds the lazy mapping instead of eagerly calling importlib.import_module() per import node
    • Narrowed to langchain_* prefixes so pydantic-validated imports (e.g., string constants) stay eager
    • Preserves Windows _MissingModulePlaceholder path + star-import eager behavior
  • src/lfx/tests/unit/test_import_absence.py:
    • New TestLazyValidateExecGlobals — subprocess-isolated asserts that validate.prepare_global_scope does not eagerly import langchain_classic.agents until a name from that module is accessed

Why

Discovered during cold-start snapshot measurement: every lfx run invocation was failing with AttributeError: partially initialized module 'torch' has no attribute 'library' (most likely due to a circular import) because the eager langchain_classic chain pulled transformers -> torch and hit a known torch 2.x partial-init bug. The lazy rewrite bypasses the chain entirely for components that do not need it, and makes every lfx run invocation pay only for what the component actually uses — a structural cold-start win, not just a bug fix.

Stacked on

PR #12785 (import-time deferrals) → #12784 (component index) → #12783 (measurement harness)

Test plan

  • cd src/lfx && uv run pytest tests/unit/test_import_absence.py::TestLazyValidateExecGlobals -v
  • LFX_BENCHMARK_CHECKPOINTS=1 uv run lfx run src/backend/tests/benchmarks/fixtures/noop_flow.json --format text — exits cleanly without the torch AttributeError

Rewrites prepare_global_scope to build a LazyImportProxy-backed globals
mapping instead of eagerly calling importlib.import_module() for every
langchain_* name in the prepended DEFAULT_IMPORT_STRING. Components that
do not reference a given langchain symbol no longer trigger its import,
cutting transformers/torch off the component-instantiation path.

Narrowed scope: only langchain / langchain_core / langchain_classic /
langchain_text_splitters / langchain_community prefixes are deferred.
Everything else (stdlib, pydantic, lfx) resolves eagerly so class-body
validators get concrete values.
@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a2780021-1ef0-4b1a-929f-ea0e6ed6a9e0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cold-start/04-imp-11-lazy-validate

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the bug Something isn't working label Apr 20, 2026
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Apr 20, 2026
- Replace `_resolved = None` cache sentinel with `_UNSET = object()` so a
  legitimately-resolved `None` is no longer mistaken for an empty cache.
- Add `logger.exception` around `_LazyImportProxy._resolve()` so deferred
  import failures get attributable log context (the traceback otherwise
  points at the user's first usage site, not the originating import).
- Capture `last_error` per candidate in the module-binding fallback loop
  and re-raise it (mirroring `_eager_import_from`) instead of re-importing
  the canonical name, so the error names the package that actually failed.
- `_LazyExecGlobals.copy()` now returns `_LazyExecGlobals` rather than a
  plain `dict`, so the marker type survives common dict idioms.
- Honor `alias.asname` in `_handle_module_attributes` (`from X import Y as Z`
  was binding `Y` instead of `Z` on the eager path).
- Expand `from X import *` to the module's public surface (`__all__` or
  non-underscore `dir()`); previously raised `ImportError` because `*`
  was treated as a literal attribute name.
- Strip internal planning vocabulary from comments / docstrings; fix
  stale claims (sorts-into-three-lists, 3-arg exec example, "pre-populated"
  on `_LazyExecGlobals.__init__`); add WHY comments on `__mro_entries__`,
  `object.__setattr__` use, `__repr__` non-resolution, and the explicit-
  dunders block. Clarify that `__instancecheck__` / `__subclasscheck__`
  only fire when the proxy is the right-hand side.

Tests: add `TestLazyExecGlobalsBinding` (10) + `TestLazyImportProxyResolution`
(3) covering prefix-narrowing, asname binding (lazy + eager), star-import
expansion, mixed lazy+eager on a single import node, `__repr__` non-
resolution, `ModuleNotFoundError` surfacing, and resolution caching.

Also revert accidental `langchain_openai` 1.1.13 -> 1.1.12 downgrade in
two starter project JSONs (autofix.ci noise unrelated to this PR).
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels May 5, 2026
@github-actions github-actions Bot added the lgtm This PR has been approved by a maintainer label May 5, 2026
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels May 6, 2026
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels May 6, 2026
Base automatically changed from cold-start/03-import-deferrals to cold-start/01-measurement-foundation May 6, 2026 16:59
@ogabrielluiz
ogabrielluiz merged commit 2ece3a3 into cold-start/01-measurement-foundation May 6, 2026
30 of 31 checks passed
@ogabrielluiz
ogabrielluiz deleted the cold-start/04-imp-11-lazy-validate branch May 6, 2026 16:59
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels May 6, 2026
@ogabrielluiz

Copy link
Copy Markdown
Contributor Author

Consolidated into #12783 for QA. The chain merges that brought this branch's content up to the base PR caused GitHub to auto-mark this as merged; the actual landing happens when #12783 merges into release-1.10.0. Original review thread preserved here.

ogabrielluiz added a commit that referenced this pull request May 25, 2026
Sub-PR #12786 removed DEFAULT_IMPORT_STRING auto-injection and added an
actionable NameError hint as compensation. The runtime NameError handler in
create_class only fired for class-body references; the common case (symbol
referenced inside def build(), def run(), etc.) compiled silently and crashed
at runtime with no guidance.

Adds a static analysis pass _check_function_body_name_resolution that walks
every FunctionDef / AsyncFunctionDef inside every ClassDef in the parsed
module and raises ValueError (same actionable hint as the runtime path) when
it sees a free Name reference that is not in exec_globals, function locals,
or builtins, AND that the legacy / langchain hint tables can map to a known
import. Names with no hint entry are passed through silently to avoid false
positives on graph-level runtime-injected globals.

Refactors the inline hint logic into _resolve_import_module_for_name and
_format_undefined_name_message so both the static and runtime paths phrase
the fix identically. Adds an except ValueError: raise branch in create_class
so deliberately-raised ValueErrors reach the caller unchanged.

Refs: LE-1229.
ogabrielluiz added a commit that referenced this pull request May 25, 2026
…ort_graph

QA Bug 3 flagged that test_import_graph.py and the TestLazyValidateExecGlobals
class were referenced by the original sub-PR descriptions (#12785, #12786) but
absent from the consolidation branch, leaving a regression-coverage gap on the
two centerpieces of the cold-start refactor.

TestLazyValidateExecGlobals (appended to test_import_absence.py):
- _LazyExecGlobals is a dict subclass usable as exec globals
- .copy() preserves the marker type (the dict.copy() default would silently
  strip it and break the laziness contract on nested execs)
- prepare_global_scope returns _LazyExecGlobals regardless of imports
- langchain imports bind _LazyImportProxy and do NOT touch sys.modules
- non-langchain imports stay eager and resolve to real module objects

test_import_graph.py (new file, mirrors test_import_absence.py subprocess pattern):
- Graph class import stays clean of langchain_core, langchain_classic,
  langchain_text_splitters, langchain_community, transformers, and torch
- Graph payload helpers stay clean
- Empty Graph instantiation stays clean
- Vertex module import stays clean

All 5 + 4 tests run green under uv sync inside src/lfx.

Refs: LE-1229 QA Bug 3 (2026-05-18).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants