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.
Closes #354
Summary
sceneas a first-class input and output for model extension nodes/generate/from-artifactroute while preserving/generate/from-imageScope
This PR deliberately supports the reviewable model-node shapes needed by scene workflows:
scene -> meshscene -> scene-> sceneIt fails closed for process extensions, mixed/multiple scene inputs,
capture, andvideo. 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.The integrated state passed an exact six-node Pixal3D registry check, 100 focused Python tests, 9 focused Node suites, TypeScript
--noEmit, and the Electron/Vite production build. Either PR can merge first; the later branch must apply the documented combined resolution during rebase.Security and compatibility
/generate/from-imagebehavior compatibleValidation
--noEmit: passedelectron-vite build: passedgit diff --check: passedThe full packaging build reached a pre-existing built-in dependency reinstall and was interrupted by DNS (
EAI_AGAIN); the authoritative Electron/Vite production build passed directly.