Skip to content

feat(workflows): support scene model artifacts - #357

Open
DrHepa wants to merge 1 commit into
lightningpixel:devfrom
DrHepa:feat/354-scene-artifacts
Open

DrHepa wants to merge 1 commit into
lightningpixel:devfrom
DrHepa:feat/354-scene-artifacts

Conversation

@DrHepa

@DrHepa DrHepa commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Closes #354

Summary

  • add scene as a first-class input and output for model extension nodes
  • add a Load Scene workflow source and scene artifact registration
  • add a generic typed /generate/from-artifact route while preserving /generate/from-image
  • validate scene directories at submission and again inside the runner

Scope

This PR deliberately supports the reviewable model-node shapes needed by scene workflows:

  • scene -> mesh
  • scene -> scene
  • multi-image -> scene

It fails closed for process extensions, mixed/multiple scene inputs, capture, and video. 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, and ipc-handlers.ts. The combined resolution preserves scene/video artifact validation, ordered node inputs, 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

  • rejects traversal, absolute and encoded paths, symlink/reparse escapes, malformed manifests, missing referenced files, and oversized scenes
  • reserves and strips transport parameters so callers cannot forge the artifact kind or path
  • pins queued jobs to the requested composite model ID through the registry, subprocess, and runner boundaries
  • keeps existing image-model extensions and /generate/from-image behavior compatible
  • registers scene outputs as workflow artifacts rather than attempting to render them as meshes

Validation

  • Python: 146 tests passed
  • Node test suite: passed
  • TypeScript --noEmit: passed
  • electron-vite build: passed
  • git diff --check: passed

The 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.

@lightningpixel lightningpixel left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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:

  1. VRAM leak. /generate/from-artifact never calls switch_model(), and _run_generation now uses get_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-image does.

  2. Load Scene can run a stale scene. Editing the path field keeps the previously validated manifestPath, and preflight and the runner only read manifestPath. So the workflow runs on the old scene while the node shows the new path. Please clear manifestPath whenever the path changes.

  3. Rebase on dev. It conflicts with #359, which moved the multi-input slot loop into slotInputs.ts. The scene branch in that loop can't be reached anyway, since scene inside inputs is rejected, so it can simply be dropped.

  4. Windows test failure. test_rejects_symlinks_missing_assets_and_oversized_manifest fails on Windows without symlink privileges (WinError 1314). Please skip it on OSError, or use a junction on Windows, which is also the more realistic case there.

  5. Stricter IO type validation. Install, the Electron listing and the registry now reject any input/output outside image|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_artifact falls back to generate(artifact_path, ...), which passes a Path where bytes are expected. Raising NotImplementedError would be cleaner.
  • The getattr(generator_registry, "model_status", None) fallback isn't needed, since model_status is added in this same PR.
  • scene uses the same emerald color as audio for badges and handles.

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.

2 participants