Skip to content

Run Diffusers in a worker-v2 execution environment, with no heavy dependencies at edit time - #66

Open
cjkindel wants to merge 11 commits into
mainfrom
experiment/v2-exec-deps
Open

cjkindel wants to merge 11 commits into
mainfrom
experiment/v2-exec-deps

Conversation

@cjkindel

@cjkindel cjkindel commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

This is the Diffusers migration to worker v2, not a
test vehicle: it runs the Text2Image template end to end in a worker, with no heavy dependency on the
orchestrator.

Stack: this PR keeps the library's ModelCache, which now lives in the worker. Moving built pipelines
into the engine's local object store, and the new Release Pipeline node, are in #76 on top of this one.

The shape of it

Two environments, and the library has to be correct in both.

Orchestrator Worker
Installs pip_dependencies — 2 entries (griptape, huggingface-hub) pip_dependencies_exec — 36, including torch/diffusers
Does imports node modules, builds node classes, answers structural questions loads models, runs nodes, holds what cannot travel

Everything below follows from keeping those two honest.

Edit time: no heavy dependencies

The orchestrator imports every node module and constructs every node class to show a node in the editor.
Anything that reached from there had to be installed there, which cost a second multi-gigabyte install
and put every library's copy of a shared package on one sys.path.

  • Annotation-only uses moved into TYPE_CHECKING; module-scope type aliases became PEP 695 type
    statements; runtime uses moved into the function that needs them.
  • @torch.no_grad() is evaluated while the class body runs, so 11 sites now use no_grad /
    inference_mode from utils/torch_utils.py, which enter the context at call time.
  • Two import-time guards that validated tables by reaching for real diffusers classes became tests.
  • get_component_slots() read a pipeline's __init__ signature to decide which override ports to show —
    1730 heavy imports during node construction. Each pipeline type now declares _component_slots, and
    tests/test_component_slots.py checks all 19 against the real classes.
  • Driver class facts (produces_video, video_fps, supports_inpainting) are declared on DriverSpec,
    so asking "does this produce video" no longer imports a driver. tests/test_driver_specs.py pins them.

The dev environment enforces this too. [project] dependencies is the edit-time set alone; the
execution set is an exec extra. uv sync therefore gives a developer the same slim orchestrator a real
install gets, and scripts/sync_dependencies.py refuses to sync if anything heavy is declared edit-time.
make test/exec builds a separate .venv-test-exec rather than fattening .venv and leaving it that
way. This mattered: a fat dev venv is what let orchestrator-side code build a pipeline locally and
silently eat 23GB.

huggingface-hub is declared, because the orchestrator imports it. Reading a model's cached config is
how the loader and builder nodes answer questions about a component without diffusers, and four
orchestrator-reachable paths do it. On main the edit-time diffusers pulled it in; with that gone it was
arriving only because the host engine happens to depend on it, which is not a guarantee a library can rely
on. ruff now also enables TC004 — a TYPE_CHECKING import used at runtime raises NameError while
both default ruff and pyright stay green, and this change moves enough imports into TYPE_CHECKING to make
that worth gating.

Correctness across the boundary

Five classes of bug, each found and then gated so it cannot come back.

1. A value that cannot be encoded is silently stringified. The engine writes event payloads with
json.dumps(default=str). DiffusionPipelineArtifact defined __str__ as its config hash, so it
arrived from the worker as that hash string — no exception, a type error two nodes later. It now
travels via _cattrs_unstructure / _cattrs_structure, the engine's seam for a class that knows its own
wire form, and __str__ is gone so a recurrence is unmistakable.

2. A value that can travel must not be held. output_image and output_video were
serializable=False, so the editor received the cache's own envelope and rendered blank:

data:image/jpeg;base64,{"kind":"local_object_reference","worker":"663f...","key":"...output_image#031a4d68"}

Both are URL artifacts naming a file on the shared workspace. Removing the flag is the whole fix.
tests/test_parked_values.py checks the rule statically and through the real egress path.

3. Node code must not reach an engine manager. Those raise during worker execution: that process
holds its own copy of the state, so a local answer would be silently wrong.
scripts/check_worker_safe_managers.py reads the guarded list out of the engine rather than
restating it, and ignores construction-time reads, which a worker performs before the guarded scope
opens. It found a live failure the crash-driven approach had not reached yet — connection_utils read
connections off FlowManager from set_parameter_value, and hydration sets values with initial_setup
defaulting to False.

4. A held value cannot be read outside its process. Not even to check whether it is set. Every such
check moved to validate_in_execution_environment (engine #5650): the three pipeline-building dimension
checks, the input-latent checks, ControlNet's can_make_control_pipe_from_standard, and four
validate_before_node_run reads of a parked latent.

5. A value that encodes but cannot be rebuilt is silently dropped. A ComponentArtifact reaching the
worker as a bare parameter value arrived as a dict: get_component_overrides returned nothing for that
slot and get_override_config_kwargs left it out of the config hash, so the pipeline built with the
default component under a hash claiming the override. Each subclass now registers itself and
_cattrs_unstructure stamps its name into the wire form, so one function rebuilds the concrete class.
Naming the class is what makes this work — the converter cannot tell the subclasses apart on shape alone,
since every field of every one of them has a default and two share config_source and repo_ref. An
unknown tag raises rather than guessing. tests/test_pipeline_artifact_round_trip.py and
test_parked_values.py drive the engine's real encode path, and both fail if either the tag or the
registration is removed.

Manifest: the engine floor

metadata.engine_version moves from 0.100.2 to 0.102.0, the first release carrying
pip_dependencies_exec, BaseNode.execution_device and the validate_in_execution_environment hook
this change depends on. Below that floor an engine ignores the execution set, builds no .venv-exec,
and the nodes then fail only when someone runs them.

pip_install_flags stays on main's --preview --torch-backend=auto. An earlier revision pinned a cu128
--extra-index-url, on the reasoning that auto resolves per installing machine and cannot be reconciled
with a hard torch==2.7.0 pin. That is a real concern, but it is main's concern too, and this change is
meant to put the same dependencies in a new environment rather than to settle it.

Reviewer notes

  • CLAUDE.md changed twice, and both are policy rather than mechanics: the "do not use lazy imports"
    rule is now a deferral policy with its three mechanisms, and there is a new "Running in a worker"
    section stating the two rules above with their gates. Worth reading — leaving the old rule would have
    invited the next contributor to revert the whole change.
  • check/types runs against the execution venv. Pyright cannot resolve torch or diffusers in the
    edit-time environment, and that absence is deliberate rather than a setup error.
  • make check now runs test/exec too. CI runs make check and nothing else, so the suite that
    checks this library's declarations (driver specs, component slots) against real diffusers was gating
    nothing: an upstream signature change surfaced as wrong ports in the builder instead of a red build. It
    needs no extra install, since check/types already builds that venv.
  • History is four topics. The merge with main and the migration itself are one commit; the three
    after it are review fixes, each scoped to one claim. The last commit splits the object-cache migration
    out to Hold Diffusers pipelines in the engine's local object store #76.

Verification

End to end in the editor: Text2Image runs in a worker and the decoded image appears.

  • edit-time gate: 24/24 node modules imported, 23/23 node classes constructed, heavy packages reached:
    none
    — in both environments
  • a venv built from exactly the declared pip_dependencies imports all 24 with zero heavy packages
  • worker-safety gate: clean, and mutation-checked
  • suites: 58 passed, 19 skipped on the slim venv; 131 passed, 21 skipped in the execution venv.
    Both run in make check
  • pyright 0 errors; ruff clean
  • dependency audit against main (Windows, py3.12, 2026-09-28): the execution resolve and the installed
    .venv-exec match main's install exactly, 159 packages each. Declaring huggingface-hub directly does
    not move that count — it was already in the set, pulled in by diffusers and transformers

Known gaps

  • Indirect manager access — the gate only sees GriptapeNodes.X() written in this library. A call
    into an engine helper that itself uses the facade is invisible to it, as is a direct manager import.
  • Non-literal parameter reads — the held-value audit matched get_parameter_value("literal").
  • Thread safety — misc/partial_denoise.py states it assumes serial generation. Untouched, and
    parallel resolution exists.
  • OSManager.cleanup_directory_if_needed is reached only when enable_directory_cleanup is on, which
    defaults to false. It is imported directly rather than through the facade, since it is static and
    touches only the shared filesystem.
  • Component subclass registration is an import side effect, so rebuilding an override requires the
    receiving process to have imported the module declaring that subclass. Both processes import all 24
    node modules today, so this is latent rather than reachable; the error names the tag and the known set.
  • deps/sync is not checked in CI. pyproject.toml and the manifest can drift without failing a
    build; they agree today.
  • Text Config scheduler keys cannot be validated while editing. Listing a scheduler's accepted
    parameters needs diffusers, so the unknown-key warning now appears after a run rather than on edit.

@cjkindel cjkindel changed the title Run the Diffusers library in a worker-v2 execution environment (test vehicle) Run Diffusers in a worker-v2 execution environment, with no heavy dependencies at edit time Sep 23, 2026
@cjkindel cjkindel self-assigned this Sep 23, 2026

@griptapeops griptapeops Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aimed at the right place: the split keeps heavy imports off the orchestrator, and moving the cache onto local_objects is what lets pipelines coexist.

2 correctness findings, 1 LLMisms. Both correctness ones come from the new release paths:

  • nodes/clear_pipeline_cache_node.py:24: drop_all() also releases parked latents.
  • nodes/release_pipeline_node.py:49: releasing a base breaks the ControlNet pipeline built from it.

Comment thread modular_diffusion_nodes_library/nodes/clear_pipeline_cache_node.py Outdated
Comment thread modular_diffusion_nodes_library/nodes/release_pipeline_node.py Outdated
Comment thread modular_diffusion_nodes_library/artifact_utils/pipeline_artifact.py Outdated

@griptapeops griptapeops Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 of 3 findings fixed. 1 open:

  • release_pipeline_node.py:49 — releasing a base pipeline tears down the ControlNet pipeline built from it.

Commit 205ac48 since the last review is clean against the rubric.

@cjkindel
cjkindel requested review from joshbretag-fn, ladipobaruwa-fn and zachgiordano and removed request for zachgiordano September 28, 2026 16:07
@cjkindel
cjkindel marked this pull request as ready for review September 29, 2026 13:59
…v-exec

The orchestrator imports every node module and constructs every node class to populate the
editor, so anything it touches has to exist in the edit-time environment. This moves the heavy
set (torch, diffusers, transformers, accelerate, numpy, PIL and friends) out of
`pip_dependencies` into `pip_dependencies_exec`, which the engine installs additively into a
worker's `.venv-exec`, and keeps the orchestrator's environment to what building the node
classes actually needs.

What that required:

- Defer every heavy import. Annotation-only uses move into `TYPE_CHECKING` blocks, module-scope
  type aliases become lazily-evaluated `type` aliases, and runtime uses move into the function
  that needs them. Pipeline drivers resolve on lookup instead of at import.
- Stop deriving node shape from diffusers. A node's `__init__` runs on the orchestrator, so
  anything previously read off a pipeline's signature is declared per pipeline type
  (`_component_slots`) and pinned by a test.
- Hold built pipelines and latents in the worker's object store and pass them as references, so
  an unserializable value never has to cross the process boundary.
- Validate where the value lives. Checks that inspect a held payload move to
  `validate_in_execution_environment`; `validate_before_node_run` keeps only what the
  orchestrator can answer.
- Read config through requests rather than engine manager accessors, which refuse to answer
  during node execution in a worker.
- Pin the CUDA index and pool PyPI alongside it, and raise the engine floor to 0.102.0.

New gates, all wired into `make check`: `check/edit-time-imports` imports every node module and
constructs every node class, then fails if a heavy package was reached; `check/worker-safe`
fails on a manager access that is not a request; and the unit suite now runs from `check` so CI
gates it.
`MediaGenConditioningRuntimeParameter.validate_before_node_run` is now
`validate_in_execution_environment`, and these two still delegated to the old name, so validating
either node raised `AttributeError` instead of returning errors. Rename the enclosing hook rather
than just the call, matching the seven runtime-parameter classes already converted: the delegation
belongs with the hook the conditioning class now exposes.

Qwen Edit's own `image_references` check moves with it. Nothing here depends on the orchestrator --
the payload is URL artifacts and scalars and travels as data -- so running it where the node runs
costs only the earlier failure point.
`ast.Constant.value` is any literal, so a `name=` or `type=` that is not a string satisfied the
`isinstance(..., ast.Constant)` check and was returned where the signature promises `str` -- which
pyright rejected, and which would have reached `_is_sendable`'s `.endswith` as a non-string.
Main removed this repo from the catalog without updating the test that asserts how it is keyed, so
the lookup raised `KeyError`. Nothing in the library offers the repo any more. The surviving four
match what the docstring already claims.

This went unnoticed because main's `check` target does not run the tests; this branch adds that.
@cjkindel
cjkindel force-pushed the experiment/v2-exec-deps branch from 91d8bbf to 21a07d5 Compare October 1, 2026 16:13
`huggingface_hub` is imported from four orchestrator-reachable paths but was declared
nowhere. It arrived only because the host engine happens to depend on it, and on main
because the edit-time `diffusers` pulled it in. Declared in both dependency sets.

`pip_install_flags` goes back to main's `--preview --torch-backend=auto`. The pinned cu128
index was an elective change, and the goal is main's install in a new environment.

`make check` now runs `test/exec` as well as `test/unit`. CI runs `make check` and nothing
else, so the suite that checks this library's declarations against real diffusers was
gating nothing. It needs no extra install: `check/types` already builds that venv.
A `ComponentArtifact` reaching the worker as a bare parameter value arrived as a dict and was
silently dropped: `get_component_overrides` returned nothing for that slot and
`get_override_config_kwargs` left it out of the config hash, so the pipeline built with the
default component under a hash claiming the override.

Each subclass now registers itself and `_cattrs_unstructure` stamps its name into the wire
form, so `structure_component_artifact` can rebuild the concrete class. The converter cannot
tell them apart on shape alone -- every field of every subclass has a default and two of them
share `config_source` and `repo_ref` -- which is what made the polymorphic registration raise
on the production path. An unknown tag raises rather than guessing.

Reading an override's config no longer needs the pipeline class, so the builder's
compatibility check runs on the orchestrator, where it refreshes on every override edit.
`_find_denoiser_slot` reads the declared `_component_slots` rather than the pipeline's
`__init__` signature, and a scheduler config comes from the cached `scheduler_config.json`
rather than through a scheduler class's `load_config`.

Building a pipeline also releases the one already resident, which is what `ModelCache` did:
holding two models' worth of VRAM is not the same experience in a new environment. A derived
build keeps its base, whose components it shares.
`ComponentSpec`, `InputParam` and `OutputParam` were read in `ModularPipelineBlocks` property
bodies, `randn_tensor` and five `numpy` uses at module scope, and the LTX2 upsampler's classes
came from a `TYPE_CHECKING` block but were used at runtime. The orchestrator imports every node
module, so each of these was a crash there rather than a slow import.

`_sync_num_inference_steps_for_distillation` no longer runs from `after_value_set`: it reads
the sigma schedule, which needs diffusers. The driver derives the step count from that same
schedule at build time, so a value left stale by an edit never reaches the pipeline.

The three `build_if_needed` guards now say what the flag means, since False reads as a
performance hint rather than "this process cannot import diffusers".
@cjkindel

cjkindel commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

@griptapeops re-review

@griptapeops

griptapeops Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

✅ GriptapeOps finished @cjkindel's request in 2m 31s

  • Read open threads
  • Review commits since 205ac48
  • Post review + status

Review posted at 25bbb79.

@griptapeops griptapeops Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one open thread from the last cycle (release_pipeline_node.py:49) was accepted as a risk and resolved; taking that. The branch was squashed and rebased, so I read the 6 commits after 6543e07.

1 new correctness finding, in 5acc16b:

  • pipeline_artifact.py:532: keeping the base doesn't protect it when the sibling derived pipeline gets released.

Comment thread modular_diffusion_nodes_library/artifact_utils/pipeline_artifact.py Outdated
The builder node no longer puts _pipeline_cls in overrides-only build data, so _build_pipeline_from_overrides_only raised KeyError. Fall back to the params class's pipeline_cls(), as build_pipeline_from_build_data already does.
@cjkindel

cjkindel commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

@griptapeops re-review

@griptapeops

griptapeops Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

✅ GriptapeOps finished @cjkindel's request in 14m 55s

  • Read open threads from prior cycles
  • Check commits since 25bbb79
  • Verify candidate finding
  • Post review + status

Review posted at 5d5b720.

# Conflicts:
#	modular_diffusion_nodes_library/nodes/noise_latent_node.py
#	modular_diffusion_nodes_library/parameters/generate_latent_parameters.py

@griptapeops griptapeops Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No open findings from earlier cycles. The pipeline_artifact.py:532 thread was resolved as pre-existing behavior; taking that. The ModelCache split-out matches main, and the _pipeline_cls fallback in 5d5b720 is correct.

1 advisory, with no line to anchor to because it now matches main:

  • latent_diffusion_pipeline_builder_node.py:91: the restored state override can't see the worker's cache. It runs on the orchestrator, where model_cache stays empty (the DAG builder reads upstream_node.state there), so a resolved builder always reports UNRESOLVED and runs again. That run hits the worker's cache, so the cost is a redundant builder run rather than a model reload. Fine to leave for #76.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant