Declare networkx as a runtime dependency - #9457
Conversation
lstein
left a comment
There was a problem hiding this comment.
Adversarial review at 0cff84a33d. I attacked the change empirically in a full InvokeAI environment (all of invokeai importable, networkx blocked via an import hook).
What holds up
| Attack | Result |
|---|---|
Import invokeai.app.services.shared.graph with networkx blocked |
✅ succeeds |
Import invokeai.app.api_app with networkx blocked (the PR's actual claim) |
✅ succeeds — no other module pulls networkx, and torch._dynamo.trace_rules only probes it via find_spec, which tolerates absence |
scripts/generate_openapi_schema.py with vs. without networkx |
✅ byte-identical output (3,011,949 bytes both) — quoting the annotations causes no schema drift |
Does the new test actually fail on main? |
✅ yes — the eager import networkx as nx at graph.py:25 raises under the blocker, so it is a valid regression test |
Shim recursion (__getattr__ re-entering on _module / _load) |
✅ no recursion; both resolve through normal class lookup |
Any module-level nx usage left at import time |
✅ none — all 20 nx. sites are inside function bodies, including _get_type_tree_root_types |
Pydantic / FastAPI resolving the quoted "nx.DiGraph" forward refs |
✅ all are method annotations; no model field, validator, or route handler touches them |
tests/test_node_graph.py + tests/test_graph_execution_state.py |
✅ 161 passed, 2 xfailed |
ruff@0.11.2 check |
✅ clean |
The mechanism itself is correct and behavior-preserving when networkx is present. My findings are about the premise and the test.
Findings
1. networkx is an undeclared direct dependency — that's the real bug
graph.py imports networkx directly, but pyproject.toml never declares it. I traced every networkx entry in uv.lock: all four sit under the torch package. We are relying on torch's transitive dependency for a module we import ourselves.
The correct fix is one line in [project].dependencies. This PR instead makes a required dependency look optional.
2. The change turns a loud startup failure into a deferred runtime failure
With networkx genuinely absent, on this branch:
DEFERRED FAILURE: ModuleNotFoundError No module named 'networkx' # raised from Graph.nx_graph()
Before: the server refuses to start.
After: the server boots, the UI loads, and the first invoke dies with a bare ModuleNotFoundError from deep inside graph materialization.
For a genuinely-required dependency that is strictly worse. If the deferral is kept, _load() should catch ModuleNotFoundError and re-raise with an actionable message.
3. The stated CI root cause does not reproduce
typegen-checks.yml installs with uv pip install --editable ., which installs torch and therefore networkx (the lock marker platform_machine == 'x86_64' and sys_platform == 'linux' and extra != cpu/cuda/rocm includes it). The generate step also sets shell: bash, which GitHub runs as bash -eo pipefail, so the pipe into pnpm typegen does not mask a schema-generation failure — the job fails either way; Unexpected end of JSON input is just layered over the real traceback in the same log.
Could you link the failing run? If the goal is only a clearer error message, redirecting the schema to a file instead of piping is the targeted fix.
4. The regression test can pass vacuously
It asserts nothing beyond returncode == 0 and never checks that the blocker took effect. builtins.__import__ also does not intercept importlib.import_module or importlib.util.find_spec, so a networkx import performed that way would slip through silently and the test would still be green.
Suggest making the child prove laziness:
import sys
import invokeai.app.services.shared.graph # noqa: F401
assert "networkx" not in sys.modules
print("LAZY_OK")and asserting "LAZY_OK" in result.stdout in the parent.
5. timeout=60 is a real flake risk
The child spawns a fresh interpreter that imports the entire invocation registry (torch + transformers + diffusers). It takes ~5 s warm on my machine, but python-tests also runs windows-cpu and macos-default with cold caches, where that import routinely takes tens of seconds. There is no global pytest timeout to compare against. Suggest ~300 s.
6. Non-blocking: collapse the shim after first load
Every nx.X access permanently routes through __getattr__ → _load() → getattr, and nx never becomes a real module (tracebacks render it as <_LazyNetworkX object at 0x...>). Since the class is defined in graph.py, _load can rebind the module global and get out of the way:
def _load(self) -> Any:
if self._module is None:
import networkx
self._module = networkx
globals()["nx"] = networkx # every later lookup in this module hits the real module
return self._moduleVerdict
No defects in the change itself. But findings 1 and 2 together say the wrong problem is being solved: networkx should be added to pyproject.toml. If you still want the lazy import as defense-in-depth, it should land alongside that declaration rather than instead of it, plus the test hardening in 4 and 5.
|
Agreed. Rreworked this PR so the primary fix is declaring I also updated the PR title/body and added validation for the dependency lock, graph import behavior, node graph tests, and frontend typegen. |
lstein
left a comment
There was a problem hiding this comment.
Re-reviewed at 73efce7245. All requested changes landed, and I re-ran the full attack set. Approving.
Verified at this head
The dependency declaration is correct and complete. networkx is in [project].dependencies, and uv.lock picks it up in both places it needs to — InvokeAI's own dependencies block and requires-dist — with an unconditional marker (aarch64-linux ∪ x86_64-linux ∪ darwin ∪ win32, plus the conflicting-extra combinations). No extra != ... gating, so every supported install gets it. This is the fix.
The shim now self-retires. globals()["nx"] = networkx works as intended — type(graph.nx) goes _LazyNetworkX → module after the first access, so there is no per-access overhead and nx becomes a real module for tracebacks and introspection.
Test hardening is in. The LAZY_OK sentinel plus assert "networkx" not in sys.modules closes the vacuous-pass hole, and timeout=300 removes the cold-Windows/macOS flake risk. Passes in 4.8 s locally.
No regressions. ruff@0.11.2 check clean; tests/test_node_graph.py + tests/test_graph_execution_state.py = 161 passed, 2 xfailed; generated OpenAPI schema still byte-identical to the pre-PR output.
Withdrawing my finding 2
My request that _load() re-raise with an actionable message assumed a real deferred-failure scenario. With networkx declared, reaching that path requires --no-deps or a hand-broken environment, where the app is already unusable for other reasons. Not worth the code — please disregard it.
One note for the record (not blocking, no change requested)
The lazy import is now pure hygiene rather than a functional optimisation. In a normal environment networkx is already loaded by the time graph.py finishes importing, via torchvision:
import torch -> networkx in sys.modules: False
import torchvision -> networkx in sys.modules: True
import invokeai.app.services.shared.graph -> True # before any nx.* access
The chain is torchvision -> torch._functorch.aot_autograd:135 -> .partitioners -> _activation_checkpointing/graph_info_provider.py:3: import networkx as nx. So the deferral saves ~0 ms of startup time in the app (standalone import networkx is ~50 ms cumulative); it only decouples our import from the package.
Relatedly, the new test's assert "networkx" not in sys.modules holds only inside the blocked child process — under the block, torch quietly skips the aot_autograd subtree that would otherwise pull networkx in. That is still a valid guard on graph.py's own import behaviour; it just shouldn't be read as describing production.
Keeping the shim is a maintainer taste call at this point, not a correctness one — it is harmless and locked in by a test. Happy either way.
Summary
networkxas a direct runtime dependency becauseinvokeai.app.services.shared.graphimports and uses it directly.graph.pynetworkximport lazy so app/schema import does not load graph traversal dependencies until graph functionality is used.networkx.Context
PR #9091 surfaced an existing dependency declaration gap during the frontend typegen job.
scripts/generate_openapi_schema.pyimports the FastAPI app, which importsinvokeai.app.services.shared.graph. That module usesnetworkxdirectly, but the dependency was not declared inpyproject.tomland was only available when pulled transitively by other packages.The primary fix here is the dependency declaration. The lazy import remains as import hygiene/defense-in-depth for schema generation and app import; it is not a substitute for the runtime dependency.
Validation
uv lock --checkuv tool run ruff@0.11.2 format invokeai/app/services/shared/graph.py tests/test_imports.pyuv tool run ruff@0.11.2 check invokeai/app/services/shared/graph.py tests/test_imports.pyuv run pytest tests/test_imports.py -k graph_module_import_does_not_require_networkx -quv run pytest tests/test_node_graph.py -qcd invokeai/frontend/web && uv run ../../../scripts/generate_openapi_schema.py | pnpm typegen