feat(workflows): support scene model artifacts - #357
Conversation
lightningpixel
left a comment
There was a problem hiding this comment.
Thanks for the thorough work on this, especially the path validation on the Python side. A few things need to change before this can be merged:
-
VRAM leak.
/generate/from-artifactnever callsswitch_model(), and_run_generationnow usesget_ready_generator(), which loads the model without unloading the active one. After a scene run, both models stay loaded. Switching back to the previous model is then a no-op, so the scene model is never unloaded. Please switch models the same way/from-imagedoes. -
Load Scene can run a stale scene. Editing the path field keeps the previously validated
manifestPath, and preflight and the runner only readmanifestPath. So the workflow runs on the old scene while the node shows the new path. Please clearmanifestPathwhenever the path changes. -
Rebase on
dev. It conflicts with #359, which moved the multi-input slot loop intoslotInputs.ts. Thescenebranch in that loop can't be reached anyway, sincesceneinsideinputsis rejected, so it can simply be dropped. -
Windows test failure.
test_rejects_symlinks_missing_assets_and_oversized_manifestfails on Windows without symlink privileges (WinError 1314). Please skip it onOSError, or use a junction on Windows, which is also the more realistic case there. -
Stricter IO type validation. Install, the Electron listing and the registry now reject any
input/outputoutsideimage|text|mesh|audio|scene. That's a breaking change for existing third-party extensions and should be called out. The allowed list is also hardcoded in four places, which is why this conflicts with #358. Could it live in one shared place?
Smaller points:
BaseGenerator.generate_artifactfalls back togenerate(artifact_path, ...), which passes aPathwhere bytes are expected. RaisingNotImplementedErrorwould be cleaner.- The
getattr(generator_registry, "model_status", None)fallback isn't needed, sincemodel_statusis added in this same PR. sceneuses the same emerald color asaudiofor badges and handles.
f46086d to
10cdc15
Compare
|
Addressed the review in
Validation: 154 Python tests + 29 Node tests passed, production build passed, I did not claim real model inference, literal UI execution, or Windows runtime validation from this environment. |
- Read-only registry calls (model/active/all status, params_schema) no longer take the lifecycle lock, which load() holds for its whole duration (first-run downloads included); /model/status, /model/all and /model/params stay responsive while a model loads - unload_all, reload and update_paths now take the lock so the lifecycle is actually serialized; their routes run them off the event loop - Show "Waiting for the previous generation..." while a job is queued behind another on the single pinned worker - Share RESERVED_ARTIFACT_PARAMS between the generation router and runner, and the scene-shape rule between preflight and the workflow runner - Cover Windows junction escapes with a real test, and add a regression test for status reads during an in-progress load
# Conflicts: # src/areas/workflows/workflowRunStore.ts
Resolve conflicts with the scene artifacts PR (lightningpixel#357): - generator_registry: keep both the weight-group and scene-shape manifest validation; move the shared-weight readiness check into get_ready_generator; recompute shared_model_dirs inside the lifecycle lock in update_paths; unload_all unloads every model before reporting the ones that stayed loaded - extension-install-utils / ipc-handlers: apply the scene-shape check, then the weight-group validation - test_generator_registry: keep the tests from both sides
Resolve conflicts with the scene artifacts (#357) and shared weight groups (#348) work, and address the review: Conflict resolution - Keep weight_groups and weight_variants side by side in the manifest validation (Python registry, model-sources, download plan, install utils, IPC listing), the preload API, the shared types and the Models UI - Rebuild model-sources.ts and the interleaved tests from dev, re-adding the variant code and tests unchanged - Download: keep dev's leased, target-aware flow and run the variant passes (shared files first, then the variant) in the legacy branch - ModelsPage: keep dev's install queue and shared groups, pass the variant id through, and use installedNodeIds everywhere Review fixes - Check the job's model for a missing weight variant (assert_weight_variant_installed(params, model_id)): pinned jobs only switch to their model once they run, so the active model is not a stand-in - deleteWeightVariant goes through the weight lease with a confirmed unload, and lists the variant files only once the node root is reserved - Picking a missing variant no longer navigates away from the graph or the Generate panel; an "install it" link is shown instead - Reject weight_variants combined with weight_groups with an explicit message - Disable other variant installs while one variant of the node downloads - README: undeclared repository variants are fetched by the shared pass
Closes #354
Summary
sceneas a first-class input and output for model extension nodes/generate/from-artifactroute while preserving/generate/from-imageScope
This PR supports the reviewable model-node shapes needed by scene workflows:
scene -> meshscene -> scene-> sceneScene inputs and outputs remain restricted to model nodes with the supported single-scene shape. This PR does not implement
captureorvideoartifact execution, but it no longer rejects unrelated third-party custom I/O declarations. Shared weight groups remain independent in #348.Shared-weight integration
The Pixal3D consumer requires both this scene contract and shared-weight PR #348. An isolated merge of #348 + #357 + video PR #358 found three overlapping validation conflicts in
generator_registry.py,extension-install-utils.ts, andipc-handlers.ts. The combined resolution preserves scene/video artifact validation, ordered nodeinputs, and node-specific shared-weight projection.Either PR can merge first; the later branch must preserve those combined behaviors during rebase.
Security and compatibility
/generate/from-imagebehavior compatibleMaintainer review fixes
devand retained the fix(workflows): multi-input nodes drop their audio slot #359 slot-input implementationNotImplementedErrormodel_statusfallback and give scenes a distinct pink colorValidation
upstream/devgit diff --check: passedNo real model inference, literal UI run, or Windows execution was performed in this environment.