Release/v0.4.3 - #367
Merged
Merged
Release/v0.4.3#367
Conversation
A generation with enable_texture builds the texture pipeline lazily, and extensions free the shape pipeline first to make room for it. When that setup failed (missing xatlas being the common case) the generator was left with _model = None while the worker process stayed alive, and nothing reset the loaded state: - runner.py called gen.generate() unconditionally, with no loaded check. - ExtensionProcess._loaded stayed True, because it is only cleared by unload(), stop() and the cancel hard-kill path, not by a failed run. - GeneratorRegistry.get_active() therefore skipped load(). Every later generation then raised "TypeError: 'NoneType' object is not callable" until the worker was killed by hand. The runner now ensures the model is loaded before inference, mirroring what get_active() already does host-side, and reports its post-failure loaded state so ExtensionProcess can drop its cached flag and reload on the next run. The original failure is still surfaced unchanged. Fixes #239
Adds AMD GPU detection and a PyTorch/ROCm redirect for extension setup, on top of the existing NVIDIA/MPS/CPU paths. - electron/main/gpu-detect.ts: detects AMD GPUs (KFD topology on Linux, Win32_VideoController on Windows), resolves the ROCm pip index and requirements. NVIDIA keeps detection priority; explicit overrides (MODLY_TORCH_FLAVOR, MODLY_ROCM_GFX, MODLY_ROCM_INDEX, MODLY_ROCM_TORCH_SPEC) are available for machines the auto-detection gets wrong. - electron/main/setup-launcher.ts: extracted from ipc-handlers.ts, adds a compatibility shim that redirects an extension's pip torch install to ROCm wheels — needed because most third-party extension setup.py scripts predate AMD support and hardcode a CUDA index. - api/routers/extensions.py: the FastAPI-side GPU detection no longer mistakes a ROCm build's device capability for CUDA compute capability (both answer torch.cuda.get_device_capability the same way), and reads the AMD compute target from the same KFD topology as the Electron side. - electron/main/copy-runtime.ts: fixes an unrelated but blocking AppImage bug found while verifying this end-to-end — fs.cp rewrote the bundled Python runtime's relative symlinks into absolute paths pointing at the ephemeral AppImage mount, so every extension venv died on the next launch. verbatimSymlinks keeps them relative. - docs/running-on-amd-rocm.md, arch/decisions/AMD-ROCM-SUPPORT.md: usage, verified configuration, and known limitations. Verified end-to-end on a Radeon RX 9060 XT (gfx1200): detection, ROCm wheel install (torch 2.13.0+rocm7.2), and a full image-to-3D generation through hunyuan3d-mini all complete successfully on the GPU.
The unload_models tool discarded the response from POST /model/unload-all and always returned the success string, so when the unload failed (any 4xx/5xx) the assistant and user were told VRAM had been freed when it had not. Every other POST tool in execute_tool calls raise_for_status(), and the MCP server's modly_unload_models does too; this brings the tool in line so the shared HTTPStatusError handler reports the failure. Adds api/tests/test_agent_router.py covering the error path (fails before this change, passes after) and the success path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
agent_chat's VRAM-unload block sat *after* the tool-call `for` loop, so it only ran when all 10 iterations completed without an early return — i.e. the rare "Reached maximum tool iterations" path. In the normal flow (round 1 dispatches run_workflow, round 2 returns a final answer with no tool calls) the function returns from inside the loop, so the unload the comment promises — "so the workflow has full GPU memory" — never happened. On a single-GPU machine the LLM kept holding VRAM while the workflow tried to generate. Extract the unload into a helper and call it on both exits (the normal return and the loop-exhausted return) whenever a workflow was dispatched. It stays best-effort and only fires once the agent is done reasoning, so intermediate tool rounds still have the model loaded. Adds api/tests/test_agent_workflow_unload.py: asserts keep_alive:0 is sent on the normal post-workflow return (fails before, passes after) and not sent when no workflow ran. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolves conflicts in api/routers/extensions.py (imports) and electron/main/ipc-handlers.ts (extensions:repair now uses dev's runExtensionRepairTransaction with this branch's GpuInfo-based detectGpuInfo/runExtensionSetup signatures). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015PpXT4je2p281SvDxnb5oF
Blocking items: - copy-runtime.test.mjs: skip the symlink-based tests on Windows, where symlinkSync needs Developer Mode; the AppImage failure mode they guard is Linux-only anyway. - api/routers/extensions.py: _detect_gpu_sm imported torch, but Modly's main venv has no torch, so every NVIDIA machine silently read as CPU. Detection now parses nvidia-smi (compute cap + driver-derived CUDA version), mirroring gpu-detect.ts. - The setup.py JSON contract now carries the same keys on both sides: the FastAPI fallback adds arch, torch_index_url and a real cuda_version, matching runExtensionSetup in ipc-handlers.ts. - Priority matches gpu-detect.ts: NVIDIA first, then ROCm, then CPU. Should-fix items: - setup-launcher: _is_pip_command scans every token ahead of the pip subcommand (basename match + '-m pip'), so [sys.executable, '-u', '-m', 'pip', 'install', ...] is redirected too. - A foreign primary --index-url is demoted to --extra-index-url instead of staying primary, where pip's last-wins would shadow the injected ROCm index; _is_rocm_index now also recognises AMD's Windows index and the MODLY_ROCM_INDEX override. - The subprocess shim patches Popen itself, covering run, call, check_call, check_output, direct Popen and the args= keyword exactly once, instead of wrapping three functions positionally. - The PyPI rescue --extra-index-url is added whenever a mixed install ends on a ROCm primary index, including when the extension supplied that index itself. - parseKfdGfxTarget (and its Python mirror) picks the GPU node with the most SIMDs, so an APU + dGPU machine resolves the discrete card instead of the lower-numbered APU. Minor items: - MODLY_TORCH_FLAVOR=cuda now short-circuits before the Apple-Silicon MPS default, so the override really wins over auto-detection. - The Windows adapter probe runs one PowerShell round-trip; the unknown- target log reuses the adapters from the same query. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015PpXT4je2p281SvDxnb5oF
fix(agent): surface HTTP errors from the unload_models tool
fix(agent): free the LLM's VRAM on the normal path after a workflow runs
…I doesn't leak The /workflow-runs surface shares _jobs, _cancel_events and _completed_at with the /generate endpoints, but never participated in their TTL purge: - create_run_from_image never called _purge_old_jobs(), so terminal job records accumulated indefinitely unless a /generate/from-image call happened to sweep them. That is the opposite of the intended use — /workflow-runs is the headless automation surface, where nothing else triggers the purge. - cancel_run set status="cancelled" but never stamped _completed_at, so _purge_old_jobs() (which only sweeps entries that have a completion time) could never evict a cancelled run — a permanent leak of a JobStatus + threading.Event per cancellation. Mirror what cancel_job and generate_from_image already do: purge on create, and record the completion time on cancel. Collection routing is intentionally left untouched here. Adds api/tests/test_workflow_runs_lifecycle.py (both cases fail before, pass after). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…'t lost `_run_generation` imported `WORKSPACE_DIR` by name, so it kept the value bound at import time. `POST /settings/paths` rebinds the registry's global when the user relocates the workspace, so every generation after that was written under the old directory and its `/workspace/...` URL 404'd in the app. Import the module instead and read `registry.WORKSPACE_DIR` at call time. `sanitize_collection()` (added to dev after this branch was cut) checks the collection's containment against the same root before the job is filed, so it reads the live binding the same way; a test now drives `generate_from_image` through it after a relocation, which is where a stale or missing module-level name surfaces as a NameError on every request.
- Add showErrorNotification mirroring showCompletionNotification - Fire error notification from appStore.updateCurrentJob when status becomes error - Fire error toasts even when window has focus (was gated by hasFocus) - Disable backgroundThrottling so renderer polling + notifications fire when minimized
…eviewNode to use data URLs
ImagePreviewNode has both an input and output handle so nodes can be chained after it, but it was never added to the nodeBehaviors.ts BEHAVIORS registry. Without this, resolveDataSource() (used by workflowRunStore.ts to find the real upstream source at run time) stops at the ImagePreviewNode itself instead of walking through it, so any node wired downstream of an ImagePreviewNode silently receives no image when the workflow actually runs, even though the canvas and preflight validation say the connection is fine. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SDMd7LzfFJ7TXav5etBjRi
- Extract mimeFromPath() into a shared nodes/imageUtils.ts instead of
duplicating it verbatim between ImageNode.tsx and ImagePreviewNode.tsx.
- Export fromWorkspaceUrl() from workflowRunStore.ts, symmetric to the
existing toWorkspaceUrl(), and use it in ImagePreviewNode instead of
reimplementing the URL-to-path conversion inline.
- Add a doc comment to ImagePreviewNode.tsx distinguishing it from the
existing PreviewImageNode.tsx ("Preview Views") — same "Preview
Image(s)" naming pattern, different behavior (single-image passthrough
vs. terminal multi-view grid), easy to confuse when editing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDMd7LzfFJ7TXav5etBjRi
Reject readiness checks excluded by filters, require regular non-empty files, and recover zero-byte final targets.
…nodes ImagePreviewNode read its preview image from only the immediate incoming edge's source, unlike the execution engine's resolveDataSource(), which walks past passthrough nodes to find the real source. Since this PR registers imagePreviewNode itself as passthrough (to allow chaining it inline), any graph where the node's direct predecessor is itself a passthrough node (WaitNode, or another ImagePreviewNode) ran and validated fine, but the preview silently never updated. Reuse resolveDataSource() (already used by workflowRunStore.ts and preflight.ts) to find the real upstream source before looking up nodeImageOutputs / the source node's type. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SDMd7LzfFJ7TXav5etBjRi
feat/add single-image preview node, fix text-only detection for multi-input models
…etup-recovery fix(extensions): recover worker after failed texture setup
…ources
normalize_model_sources() only returns None when the "model_sources" key
is absent from the dict it receives, but the dict was always built with
that key present (even when body.get("sources") was None). The intended
"sources are required" error could never fire; callers instead got
normalize_model_sources' own "model_sources must be a non-empty array"
message, which references the internal field name instead of the API's
"sources" field.
Check body.get("sources") directly before normalizing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDMd7LzfFJ7TXav5etBjRi
… download model:cancelDownload unconditionally deleted the entire model directory after a cancel. That was harmless when a model was always a single, all-or-nothing HF repo, but a model node can now declare multiple model_sources sharing one directory, downloaded sequentially under one shared cancel control. The backend's own cancel path is already conservative and only removes in-progress *.part files, preserving any source that already finished — but the Electron handler wiped everything, including sources that had already completed. Add removePartialDownloadArtifacts(), mirroring the backend's *.part-only cleanup, and use it instead of rm-ing the whole directory. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SDMd7LzfFJ7TXav5etBjRi
feat(models): support multiple Hugging Face sources per node
Lets external contributors claim an issue without repo write access. Commenting /assign self-assigns via a github-script Action (GITHUB_TOKEN has the write permission the commenter doesn't); /unassign releases it. CONTRIBUTING.md documents the full flow: claim -> fork -> PR with `Closes #N` -> board moves through In progress / Ready to review / Ready to test / Done. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SDMd7LzfFJ7TXav5etBjRi
…command docs: add CONTRIBUTING.md and /assign command bot
The /assign bot moved GitHub assignees but never touched the Project v2 board itself, so the "In progress" column stayed empty. Same for PRs: opening one with `Closes #N` closed the issue on merge but never moved the card to "Ready to review". - assign-command.yml: on /assign, move the linked board item to "In progress"; on /unassign, move it back to "Backlog". Uses PROJECT_TOKEN since the default GITHUB_TOKEN has no Projects v2 scope. - pr-board-sync.yml (new): on PR opened/edited/ready_for_review, parse closing keywords (Closes/Fixes/Resolves #N) from the description and move each linked issue's card to "Ready to review". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YTLv6fJFA5MotMqWgStGrf
feat: sync project board status with /assign and linked PRs
Add AMD ROCm support (Linux + Windows)
…ce-dir fix(generate): read WORKSPACE_DIR dynamically so relocated output isn't lost
…-manifest-ui # Conflicts: # electron/preload/electron-api.ts
…nifest-ui Feat/progressbar value manifest UI
- 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
feat(workflows): support scene model artifacts
Resolve conflicts with the scene artifacts PR (#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
- Treat an unreachable backend (ECONNREFUSED) as unloaded when removing weights: no Python process can hold the files, and failing here blocked deleting weights and uninstalling extensions while the backend was down - Removing an extension's weights unloads only that extension's models instead of calling /model/unload-all - Compare shared model dirs with str(Path(...)) so the extension process test passes on Windows - Add regression tests for both removal cases and for unload_all
…groups feat(models): support extension-scoped shared weight groups
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
The capability destination is built from EXTENSIONS_DIR.resolve() (long form) but was compared to os.path.abspath(ext_dir), which keeps the configured form. With an 8.3 short EXTENSIONS_DIR (e.g. GitHub's Windows runners: C:\Users\RUNNER~1\...) the same folder never matched, so an authorized extension update was reported as an interrupted installation. Normalize the extension folder the same way (resolved parent + name), without resolving the folder itself, and cover it with a Windows test that reruns the authorization through a short path.
feat(models): install and remove weight variants per node
The run-scoped 'error'/'exit' listeners only cover a worker that dies mid-run. A worker can also die while idle -- e.g. a timer the processor left behind throws after it returned -- and stayed cached as ready, so the next run posted into a dead thread and hung. Forget the worker whenever it exits, from ensureReady(). The test now stubs electron: process-runner imports `app` since the mesh ops registry change, and the real package cannot load outside Electron, so the bundled module failed to load. Add a test for the idle-death case.
fix(process-runner): settle a JS process run when its worker dies
The containment checks compared string prefixes, so a sibling folder whose
name starts with the workspace's (e.g. "<workspace>-secret") passed them:
"../<workspace>-secret/file.glb" could be exported, edited or served by
/export/{fmt}, /optimize/export, /optimize/mesh, /smooth, /transform and
/optimize/ply-to-splat.
Add generator_registry.is_within_workspace(), which compares ancestry and
reads the current workspace, and use it in all five checks. transform_mesh
now builds its result URL from the resolved path, and ply_to_splat uses the
module-level registry import. Cover the sibling-folder case on every route.
…rkspace-dir fix(export,optimize): read WORKSPACE_DIR dynamically so edits work after the workspace moves
…lectron On dev, process-runner imports electron's `app` for the Python runner's spawn environment. Bundling the real electron package makes the module fail to load under plain Node, so every test in this file failed once merged. Mark electron external and hand the bundle a stub `app`, as process-runner-worker-exit.test.mjs already does, with `getAppPath` added because the Python runner calls it when it spawns.
Replace the Ollama-backed agent with a managed llama.cpp pool (one llama-server per loaded model, LRU + VRAM-budget eviction, idle reaper), a shared model library with resumable downloads, and external providers (OpenAI, Anthropic, Mistral, Groq, OpenRouter, Ollama, custom endpoint) with API keys encrypted through Electron safeStorage. Review fixes: - restart a crashed slot on a fresh port instead of killing the server that inherited its port - unload_all() spares slots answering a request or being started; the agent releases its slot before the post-workflow unload - "Free memory" also unloads the local LLMs - POSIX stale-server cleanup only kills llama-server processes - retry an external chat without images when a text-only model rejects them - only treat OSCrypt-tagged hex as ciphertext so legacy hex keys survive - external model picker no longer shows a model the draft does not hold - drop the CAD and Custom tabs from the model library
…pace fix(process-runner): rebuild a cached runner when its workspace moves
scripts/react-test-env.mjs imports jsdom, which no package declared, so llmModelsStore.react.test.mjs failed on any fresh install and npm test (and with it the pre-push hook) failed for everyone.
…lder The llama.cpp engine, GGUF models, logs and pool config lived in ~/.modly/llm on the system drive, apart from every other data folder the user picked at setup. They now go to an `agent` folder beside models/, extensions/, workflows/, workspace/ and dependencies/, passed to the API as MODLY_LLM_DIR. Setup writes <base>/agent. Installs that predate it get the folder next to their models folder, created and pinned in settings.json at startup so moving the models folder later does not take the agent folder with it.
Settings > Agent shows whether the llama.cpp engine is installed, with an install button when it is missing, and lists the models in the agent folder with the selected one first. Browse opens the model library, now a wide dialog with two sections: Installed (everything in agent/models, Select / delete) and Suggested (catalog models not downloaded yet). Add picks a local .gguf and copies it into the agent models folder; picking and copying both run in the main process. The engine install moved out of the dialog into the settings section, and the shared SSE progress bar lives in its own component.
Agent settings now use the Settings card kit in two columns: provider, local engine (status badge, install, selected model, simultaneous models) and thinking on the left; the MCP server setup on the right, with copy buttons and a tab per client. The shared Card gains an optional `aside` slot for header badges. The model library shows models as cards with a search box and filters (All, Fits my GPU, Vision, CAD), the card VRAM next to Add, and per-card tags, VRAM estimate and actions (Select, Download, Pause/Resume, Cancel, delete).
The llama-server archive is downloaded from the llama.cpp GitHub release and its files are executed, but nothing checked the bytes. GitHub reports a sha256 digest for every release asset; the download is now hashed as it streams and refused on a mismatch. An asset without a digest (older uploads) is accepted as before. A failed or refused download no longer leaves its temp archive behind.
The chat leaves code/CAD models out of its own picker, but the model library still offered Select on them, so a CAD coder could become the agent's chat model. Those cards now say they are for workflow nodes. Also drop LlmModelSelect, which nothing imports.
…ders Feat/llamacpp cloud providers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.