4f18eefa69fe5fcfb14ebc2979c315d37cdc334f
166
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 digital-garbageandClaude Sonnet 5
|
4f18eefa69 |
feat: picker wheel -- customizable radial menu, replaces v4's pinned buttons
[[project_v5_ui_requirements]]: "any action in the UI can be assigned to a
slot... full wheel or half wheel... half wheel positioned left or right...
both draggable... default: middle mouse button, anywhere."
app/legacy/actions.js: ported the composer tree's tiny action registry
(app/shell/actions.js, an ES module, unreferenced from boot) as a classic
global window.FunPackActions -- offerAction({id,label,icon,run}), offered(),
run(id). Whatever announces itself is what the wheel (or anything later --
a command palette, a keymap) can show.
app/legacy/wheel.js: middle-mouse (anywhere) opens a radial menu of
whatever's currently on it. Full wheel spaces items evenly around 360°;
half wheel spaces them over a 180° arc on the chosen side, with drag
locked to the vertical axis only (full wheel drags both axes) per the
spec's "half wheel moves up/down, full wheel up/down and left/right" line.
Dragging repositions the CURRENTLY OPEN wheel only, not persisted -- read
"draggable" as "don't get stuck somewhere awkward this time" rather than a
remembered dock position, since the spec doesn't say which and this is the
simpler reading. Customize dialog (reuses pinned_buttons.js's existing
.pin-modal/.pin-slot CSS) toggles shape, side, and which offered actions
are on the wheel. pinned_buttons.js is NOT removed -- the spec says this
supersedes it, but retiring a working control felt like a separate
decision from adding a new one; both coexist for now, confirmed to not
conflict (separate storage keys, no shared DOM, never both open at once).
app/legacy/wheel_actions_seed.js: seeds 9 starter actions (Generate,
Generate selected, Render, Auto Montage, Undo, Redo, Models & Pipeline,
Engine settings, Sampler quick-access) so the wheel isn't empty on first
use.
Two rounds of extensive_testing, plus a bug I found myself in live
testing before either round: the half wheel's fixed radius, sized for a
full wheel's 360° spread, packed the same item count into half the
angular room and produced heavily overlapping items -- fixed by growing
the radius only as far as needed to keep adjacent items from touching,
confirmed visually before/after.
Round 1: no functional break found in outside-click-vs-item-click
ordering, listener leaks on repeated opens, or coexistence with
pinned_buttons.js.
Round 2: found the real defect -- enabledIds()'s first-call snapshot
("default to everything offered so far") could run in the gap between
wheel.js executing and wheel_actions_seed.js finishing (classic <script>
tags block sequentially, and this window widens on a slow/cold-cache
load), permanently persisting an empty or partial action list with no
error, no indication -- the user would just experience "the wheel doesn't
work" and have no way to know why. Fixed by deferring the snapshot to the
window `load` event (guaranteed after every script has run); before that,
enabledIds() shows whatever's offered so far live without writing it
down as the permanent default. Also cleaned up a dead if/else in the
half-wheel angle math flagged as a "looks like a bug that got half-fixed"
smell.
Live-verified after both fixes: real physical drag (manual mousedown/
mousemove/mouseup sequences, not just JS calls) confirms half-wheel drag
is genuinely locked to the y-axis (x stays exactly put) while full-wheel
drag moves both, and neither drag opens Customize -- only a press that
never crosses the drag threshold does. npm test: 691 pass, 1 pre-existing
unrelated flake (shell/tests/projects.test.js:285).
No test file exists yet for actions.js/wheel.js/wheel_actions_seed.js --
a real, named test gap, not fixed here. Real hardware middle-click and
cross-browser autoscroll-blocking are also unverified (browser automation
has no native middle-click) -- declared, not chased further without
physical hardware.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
4a1f60bda6 |
feat: sampler quick-access -- steps/scheduler/sampler in one click
[[project_v5_ui_requirements]]: "steps, scheduler and sampler are reached for constantly, and in v4 they are several clicks deep... a thin hovering window above the timeline, which keeps them visible without spending timeline height." Nothing in v5 built this yet -- the only existing path was Settings > Engine > Sampling, a modal several clicks deep. app/legacy/sampler_quickbar.js: a toggle button in the timeline header (next to Assets/Preview/Properties/Composer) opens a fixed-position panel above the timeline showing only category="sampling" module settings. Hidden until PipelineState confirms at least one installed module has a sampling setting (the "absent, not greyed" convention this codebase already uses for a module/role that doesn't apply). The quickbar needed to read/write the SAME module-values tree engine_settings.js already edited. That tree gets serialized into one opaque JSON blob and written into a pipeline slot by the server (core/graph.py's place()) with no server-side merge -- a second, naive editor sending a partial tree would silently erase every other module's settings. To avoid two independent caches of the same server data diverging (the exact bug class pipeline_state.js's own header comment already describes for `slots`, from an earlier fix), extracted the shared pieces instead of duplicating them: - PipelineState gained currentValues() (always-fresh: defaults + valuesAlreadyPlaced()) and setModuleValue(moduleId, name, value) (patches one field, sends the whole tree). Single source of truth, used by both editors. - app/legacy/settings_field.js: the field-widget builder (checkbox/ slider/enum/color/text, the `when`-clause visibility gate) extracted out of engine_settings.js verbatim, parameterized on a values tree and an onChange callback instead of a private cache -- one place renders a setting, one place saves one, for every editor of this tree from now on. - engine_settings.js refactored to use both instead of its own private values/seeded/setValue/ensureSeeded/commit/controlFor. Two rounds of extensive_testing. Round 1 found a real, live-reproduced defect: setModuleValue's "always fresh, no cache" design had its own race -- two calls fired before either's round-trip resolved could silently lose the earlier edit, because the second call's "read the whole tree" snapshot didn't yet include the first call's in-flight write. Fixed with a `pendingValues` overlay in pipeline_state.js, written synchronously before every read, reconciled (per-key, not wholesale) once valuesAlreadyPlaced() actually confirms it server-side. Round 2 re-verified the original race closes -- checked against valuesAlreadyPlaced() specifically (decoded straight from the server's own response, not the optimistic overlay) for both a two-different- modules race and a three-calls-same-field race -- and found one more thing, currently inert: setModuleValue's returned promise doesn't mean "this specific edit is confirmed" for every concurrent caller, only for whichever one owns save()'s queue at that moment. Documented in place (both real callers only re-render from shared state, which is already correct regardless of promise timing) rather than reworking save()'s shared queue for a guarantee nothing currently needs. Live-verified: button appears only when a sampling setting exists; opens positioned above the timeline; editing in the quickbar changes only the target field (confirmed via full before/after tree diff); same change visible in the Engine Settings modal and vice versa; real outside-click closes it; 5 open/close cycles leave no DOM leak. npm test: 691 pass, 1 pre-existing unrelated flake (shell/tests/projects.test.js:285). No test file exists yet for pipeline_state.js/engine_settings.js/settings_field.js/ sampler_quickbar.js -- a real, named test gap, not fixed here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
bac2f3c5af |
feat: overlay tracks -- no default lane until the user adds one
v4 always presented an overlay lane whether or not anything used it.
The user's own v5 UI requirements explicitly reject this: "the
timeline starts with no overlay tracks. One appears when the user
adds it explicitly." store.js's normalizeOverlayLanes() force-created
a default "Overlay 1" lane the moment overlay_lanes was empty -- on
every project load AND every timeline render -- so a fresh, empty
project always showed one anyway.
Fixed at the two real sources:
- normalizeOverlayLanes() now only auto-creates a lane to house
ORPHANED tracks (old-format projects, or a lane removed out from
under its tracks) -- never just because nothing asked for one yet.
- defaultOverlayLaneId() (reached only from addImageOverlay/
addTextOverlay when the user is explicitly adding their first
overlay) now creates the first lane itself at that exact moment --
the "user adds it explicitly" trigger the spec asks for.
Traced every overlay_lanes/overlay_tracks consumer across store.js,
timeline.js, inspector.js, view_bus.js, tour.js: none assume >=1 lane
always exists (removeOverlayLane's own guard already handles "never
below 1 once lanes exist"; drag-drop onto a lane row is naturally
unreachable before one exists, matching intended behavior).
Live-verified: a fresh project (window.Store.newProject) renders zero
overlay rows; addTextOverlay("Hello") creates exactly one lane, wires
the new track to it, and the row appears with a live text-overlay
preview on the canvas. JS suite: 691 pass, 1 pre-existing unrelated
flake (shell/tests/projects.test.js:285).
One round of extensive_testing (adversarial review) came back clean.
Declared, not fixed: a project saved under the old behavior keeps its
already-persisted empty "Overlay 1" lane -- a forward-only fix, no
migration was asked for and the lane is now an ordinary, removable one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
0a71e0b929 |
fix: tour -- clear tour-s1's dead rating: "good" left over from round-9's scene_renders fix
Round 9 emptied demoProject()'s scene_renders to stop a real unmocked
network fetch against the live backend. Round 10's adversarial review
caught the sibling: tour-s1 still had rating: "good" in the demo data,
but the rating badge (timeline.js's sceneRatingRaw -> genUnitHasRender)
is gated on hasRender(), which now reads false for every scene since
scene_renders is empty -- so the badge silently never renders on the
scene the tour spotlights, self-contradicting the stored rating.
Cleared the rating to "" to match the no-render state honestly. Traced
every step/FAQ reference to tour-s1 and ratings: the FAQ answer is
generic ("after a render, rate a generative scene"), no step claims
tour-s1 specifically shows a badge, so this is a pure consistency fix
with no user-visible regression.
Live-verified: fresh tour boot, tour-s1.rating reads "" via
window.Store.get(). JS suite: 691 pass, 1 pre-existing unrelated flake
(shell/tests/projects.test.js:285).
Round 10 of the tour extensive_testing chain otherwise came back clean:
no dangling onCancel overlay (PipelineSetup's cancel-button variant is
unreachable in tour mode -- onboarding.js explicitly skips opening
during the tour), no crash from the empty scene_renders object across
every reader (store/timeline/inspector/player/montage_dialog/view_bus/
history), and the patchApi()/patchStore() mock surfaces still match
their consumers' expected shapes on a fresh full read.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
ecbdc42785 |
fix: tour -- restart mock was reloading the real page, plus a stray player fetch
Round 9's module-definition sweep came back clean (the four standalone modules found in rounds 5/7/8 are all still correctly covered, and no fifth exists) -- but it caught two real bugs my own round-8 live verification missed by not waiting long enough: - Both restartComfy() implementations call FunPackRestart.waitForReload() after API.restart() resolves. That function polls API.health() starting 3.5s later and calls location.reload() once it reports ok -- and API.health is ALREADY mocked to ok:true for the Settings About panel's own unrelated reasons, so the very first poll after a restart click reloads the real page mid-tour, silently resetting all tour progress. Nothing actually restarted, so there's nothing to wait for -- fixed by short-circuiting waitForReload() in tour mode instead of polling toward a reload. - demoProject() pointed a fake scene_renders entry at a real-shaped ComfyUI /view URL for a file that doesn't exist. Since bootstrap() notifies immediately on tour start, player.js's render() (a plain Store subscriber, never blocked or mocked) picks it up and issues a real GET, then a real HEAD retry, against the live backend -- zero clicks, always 404s, but still a genuine unmocked reach into the real install. Fixed at the source instead of patching player.js's internals: an always- unrendered demo scene is an honest trade-off the tour's own FAQ text already accounts for. Verified live: clicked both restartComfy implementations, waited the full 5s past the 3.5s poll delay this time (my round-8 verification hadn't waited long enough to catch this) -- confirmed no reload, still on ?mode=tour; confirmed zero /view?filename= requests; full tour sequence still completes cleanly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
e9a570405f |
fix: tour -- mock restart/git/refinement-key ops, the most severe gap yet
Round 8's module-definition sweep found a fourth standalone module (after
GenerateBridge, PipelineState): window.FunPackGit (git_actions.js) and an
independent window.FunPackMaintenance.restartComfy (menubar.js) both call
window.MovieEditorAPI.restart() directly -- a REAL server restart, killing
any generation in progress -- one click from the same .settings-win
sidebar rounds 5-7 already proved reachable (Custom Nodes' inline
"Restart ComfyUI" button, and the built-in "Updates & ComfyUI" section,
which also fires a real GET /api/git/status the instant it mounts, zero
clicks needed -- same shape as pipeline_state.js's ensureLoaded()). That
same section's "Switch branch"/"Update FunPack" reach real git
checkout/pull. Worse than any prior finding: not a stray read, not even
round 7's directory-delete, but restarting the live production server and
mutating the FunPack install's own git state.
While fixing this, found two more real, unmocked, irreversible operations
in the same file (menubar.js's FunPackMaintenance, not caught by round 8's
own report): deleteRefinementKey and clearGlobalTaste, both described by
their own confirm dialogs as "This cannot be undone." Currently dormant --
v5's own api.js has these as stubs ("not built in v5 yet") -- same shape
as saveClipToMediaBin/renameMedia, closed pre-emptively rather than
waiting for a future backend to wire them up for real.
Fixed by mocking all ten functions in patchApi(): restart, gitStatus (also
cleanly short-circuits git_actions.js's own _ensureStatus() gate via its
existing alert path), gitUpdate/gitCheckout/gitRollback, and the five
refinement-key/taste functions.
Verified live in a genuinely fresh tab: called both restartComfy
implementations directly (auto-accepting their confirm dialogs) plus
refresh/update/switchBranch/deleteRefinementKey/clearGlobalTaste -- zero
/api/git/* requests in the network log, zero errors, page stayed on
?mode=tour rather than reloading, full tour sequence still completes
cleanly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
6658a95928 |
fix: tour -- mock pipeline_state.js and custom_nodes.js, real files were at risk
Round 7 finished round 6's admittedly-incomplete api.js-vs-patchApi() sweep and found two more real, live gaps -- both on the same reachable surface (the models-modal step's sidebar) rounds 5-6 already proved live: - pipeline_state.js (window.PipelineState) is a standalone module, like GenerateBridge -- talks to MovieEditorAPI directly, never through window.Store, invisible to every Store-shaped sweep. ensureLoaded() fires real, unmocked API.modules()+API.pipeline() the INSTANT the models-modal step's panel mounts, zero clicks needed. Any ordinary widget edit on that same screen then calls API.editPipeline(), a real POST. - custom_nodes.js registers its own Settings section in the same sidebar. Its panel calls API.customNodes() on mount, and its buttons reach a real git clone (customNodeInstall), git pull (customNodeUpdate), and an irreversible directory delete the panel's own confirm dialog describes as "cannot be undone" (customNodeRemove) -- worse than any prior finding, since it can destroy real files on disk rather than just make a stray network call. Also closed temp_browser.js's listTemp/downloadTempFile (lower severity, read-only, but still a real read of the live server's actual temp files). Fixed by mocking all eight functions in patchApi(), matching each one's real response shape (pipeline_state.js needs slots/incomplete/refused/ queueable; editPipeline's mock echoes back whatever slots were sent so the demo panel behaves sanely rather than erroring). Verified live in a genuinely fresh tab (a stale tab's request log falsely showed these firing before the fix landed -- reproduced clean in a new tab to be certain): opened the Models modal directly (the zero-click trigger) and the Custom Nodes panel, zero /api/modules, /api/pipeline, or custom-node requests in the network log, zero console errors, full tour sequence still completes cleanly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
97e1029ca0 |
fix: tour -- mock settingsCard, guard applyGlobalPromptQuiet at its root
Round 6's sweep of raw API.* calls that bypass Store entirely (a layer none of the prior five rounds checked) found the most easily-triggered gap yet: models.js's "Export settings..." button calls API.settingsCard directly, a real unmocked POST /api/settings-card -- and it's on the FIRST screen the tour's own "models-modal" step shows, no sidebar navigation needed (unlike round 5's Combo Sweep panel, which needed a click into Settings first). Fixed by mocking API.settingsCard with a real Blob (the function resolves a Blob and feeds it straight to URL.createObjectURL, so a JSON mock like every other entry uses would break the button instead of blocking it) -- a 1x1 transparent PNG is enough for the card to render. Also closed the secondary finding: applyGlobalPromptQuiet has TWO internal closure-scoped callers (setEditorSetting's anchorEnabled toggle, applyPromptTemplate) besides its exported Store property -- the same "block the export, miss the internal call" shape as commit()'s original vulnerability, so a patchStore() block-list entry alone couldn't have closed it. It reaches a real, unmocked API.parsePrompt call. Fixed with the same root-cause pattern as commit()'s guard: a window.__FUNPACK_TOUR__ check directly inside the function, which also covers scheduleGlobalPromptApply transitively since that just debounces into a call to this same function. Verified live: API.settingsCard resolves a real 68-byte PNG Blob (not an error); toggling anchorEnabled and calling scheduleGlobalPromptApply directly, waited past both debounce windows, zero prompt/expand or settings-card requests in the network log; full tour sequence still completes with zero thrown errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
37574ac44c |
fix: tour -- block runComboSweep, the real gap round 5's sweep found
Round 5's exhaustive sweep of store.js's Store export found the serious
one: runComboSweep ("Run sweep" in the Settings window's "H3 Combo Sweep"
panel) reaches a real ComfyUI job queue completely outside every mock and
guard built so far. It calls window.GenerateBridge -> app/shell/session.js's
real queue-and-poll pipeline directly -- a separate module tree from
window.MovieEditorAPI that patchApi() never touches, and with zero
__FUNPACK_TOUR__ awareness anywhere in that chain. The "models-modal" tour
step spotlights the entire .settings-win element (sidebar included), so
the Combo Sweep panel is genuinely openable and runnable mid-tour -- not
theoretical.
This is a fourth caller of the same generation funnel generate/
generateMontage/generateSelected already block, just reached through
Settings instead of the timeline -- the kind of sibling path this whole
review chain keeps finding one at a time. Also closed renameMedia, a
second dormant landmine of the exact shape saveClipToMediaBin was
(currently inert -- API.renameMedia is a stub -- but already reachable
from the media bin's rename control).
Verified live: both return the block toast directly (confirmed the exact
Generate toast fires for runComboSweep, not a queued job), zero
prompt-queue network requests, full tour sequence still completes with
zero thrown errors.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
cd1d07d96c |
fix: tour -- close the second funnel round 4 found, correct the overclaim
Round 4's review confirmed the commit() guard genuinely closes every project-save path (keyboard shortcuts included), but caught the fix's own comment overreaching: "every mutating path in this module funnels through here" is false. store.js's shortcut/category/transition CRUD functions (saveShortcut, deleteShortcut, addCategory, importShortcuts, clearShortcuts, saveTransition, deleteTransition, importTransitions, clearTransitions) each call their own real, unmocked API.* endpoint directly and never touch commit() at all -- a second, separate funnel the guard doesn't cover. Currently dormant (no Settings > Shortcuts panel is wired up in v5 yet, confirmed by grep -- these functions have no caller anywhere outside store.js/api.js today), same shape as the saveClipToMediaBin landmine rounds 2-3 already closed pre-emptively rather than waiting for it to go live. These write GLOBAL state (the on-disk shortcut/transition library), not sandboxed per-project state like everything commit() guards, so if left open they'd be worse than a stray project PUT once a Shortcuts UI ships. Fixed with the same block-list pattern, and corrected both comments (store.js's commit() guard, tour.js's blocklist) to state their actual scope instead of the overclaim. Verified live: all nine now return the block toast instead of reaching Store's real implementation; full tour sequence still completes with zero thrown errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
61a4817460 |
fix: tour -- close the save-bypass at its root, not by enumerating callers
Round 3's review found the enumeration approach (patchStore()'s blocked list) was still incomplete, and worse, live rather than dormant: keyboard shortcuts (Delete/Backspace/S) bypass the tour's click-blocking backdrop entirely, since CSS pointer-events does nothing to a window-level keydown listener. timeline.js's shortcut handler calls removeSelectedScenes(), removeAudioTrack(), removeSelectedOverlay(), and splitScene() -- none of them in the blocked list -- and every one ends in scheduleSave(), whose setTimeout calls the closure-scoped commit() directly, reaching a real, unmocked PUT /funpack/api/projects/<id>. tour-s1 is pre-selected from the moment the tour boots, so pressing Delete at any point immediately triggers this -- no click needed, no specific step required. The blocked-list pattern (three rounds of fixes: parsed.scenes crash, syncFromPreview, saveClipToMediaBin) was closing this one caller at a time and kept finding another. Fixed at the root instead: every mutating path in store.js funnels through the single closure-scoped commit() function before it can reach the network, so that's the one place a tour-mode guard covers every current AND future caller by construction, without needing tour.js to know the name of anything that might call it. Verified live: dispatched a real Delete keydown during the tour with tour-s1 selected (the default boot state) -- the scene still gets removed locally (harmless, it's fake demo data), but zero requests to /api/projects appear in the network log across the full debounce window after the fix, versus a real PUT before it. Confirmed the guard is a no-op outside tour mode (window.__FUNPACK_TOUR__ is false there) and the full 16-step tour sequence still completes with zero thrown errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
da04063a77 |
fix: tour -- also block Save video, a dormant landmine round 2 flagged
Round 2's review of the syncFromPreview fix came back clean but flagged an
adjacent, currently-dormant gap while tracing the same pattern: the "Save
video" button (Store.saveClipToMediaBin -> API.importClipToMediaBin) is
neither in patchStore()'s block list nor patchApi()'s mock list, and it
already renders during the tour (tour-s2 satisfies
clipSaveableToMediaBin()). It's harmless today only because
importClipToMediaBin is itself a stub ("no render/stitch stage yet") --
the moment that stage lands, this becomes a real, unblocked PUT during the
tour with nothing prompting anyone to notice. Closing it now with the same
established pattern rather than leaving it for a future regression.
Verified live: Store.saveClipToMediaBin now returns the block toast
directly, and a full tour run completes with zero thrown errors.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
c020ee8ca8 |
fix: tour -- Sync scenes from preview could reach a real save, and a demo text mismatch
Adversarial review of
|
||
|
digital-garbageandClaude Sonnet 5
|
ee2c0d55ea |
fix: tour demo -- parsed.scenes must be an array, not a count
Verified the ported Tour feature live end-to-end (entry via ?mode=tour and
via Help > Welcome tour..., all 15 spotlight steps, FAQ landing, exit) and
found the one real bug: tour.js's canned preview.parsed was `{scenes: 3}`
-- a count, not an array. inspector.js's renderSplit() does
`(parsed.scenes || []).forEach(...)`, which throws the moment the Scene
preview panel renders during the tour. Confirmed identical in v4's
original tour.js -- a pre-existing bug carried over verbatim by the
byte-for-byte port, not something the port introduced.
Fixed by giving the demo preview a real 3-scene array matching the demo
project's own scene texts. Re-verified live: zero thrown errors across
the full tour sequence after the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
8e0d02363e |
fix: settings card export -- round 5 found unwrapped head titles cost real CPU
extensive_testing round 5 did a systematic audit rather than one more spot check, and found the one remaining gap: row VALUES are wrapped to card width before drawing (_wrap()), bounding each draw call's cost, but "head" block titles (built from slot id/group/node, each individually _cap()-allowed up to 400 chars per round 4) were drawn as one unwrapped string. PIL's text-render cost scales with string length regardless of visible width, so 2000 slots each with a 1200-char (id+group+node) concatenated title -- every documented cap individually honored, nothing near any threshold -- cost 20.5s of pure font-render CPU. Fixed by truncating head-block text to the same rendered width row values already get, before it's ever handed to the font renderer. Round 5's own systematic pass (tracing every slots-derived value to its draw/embed destination, checking the wired-link path, non-string id/group/node, by_id construction cost, host reachability, and the render kwarg) found nothing else -- everything else that reaches the raster or the JSON embed is capped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
5214dd7a86 |
fix: settings card export -- cap dict KEYS, not just values, in two more places
Found while re-reading collect() after round 4's fix, same pattern as every
prior round: an unaudited site where caller data reaches the card uncapped.
- collect()'s per-input loop caps `value` via _short() but never capped
`name` -- the input dict's KEY is caller data too (the whole `inputs`
dict comes from the client), and flows verbatim into the drawn row label
and the embedded JSON.
- _rows_from_list's per-entry join (`f"{k}={_short(v)}"`) caps `v` but not
`k` -- a huge key still gets fully materialized into the string before
the joined result's own _short() call ever runs, so it's real per-row
cost even though the final printed string comes out short.
Both fixed with the same _cap() helper already used for slot id/group/node.
Added regression tests for both.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
eda757e103 |
fix: settings card export -- round 4 found slot metadata bypassed every cap
extensive_testing round 4 found every cap added in rounds 1-3 bounded ROW content only. A slot's id/group/node class, and project_name, were never run through any length cap -- they reach render_png as "head"/"title" block kinds, which the round-2 line-truncation loop skips entirely (it only counted kind == "row"). Worse, collect()'s row-budget check never fires for a slot with zero rows (empty inputs), so slot COUNT itself was unbounded whenever each slot carries no rows. Reproduced: 3000 slots with empty inputs and 4000-char id/group strings -- total_rows stayed at 0 the entire time, so none of the three existing caps ever engaged -- cost 193.6s render time and produced a 12.1MB embedded tEXt JSON, with zero truncation declared anywhere on the card. Fixed by: capping slot id/group/node/project_name/wired-from-label length via a new _cap() helper (same truncation as _short(), without its display-value formatting so "no id"/"no group" fallbacks still work); capping slot COUNT directly in collect() rather than relying on row count; and making render_png's line-truncation loop count "head" blocks toward the same budget as "row" blocks, as defense in depth against any other block kind reaching render with no accompanying row. Added a test reproducing the exact empty-input/huge-id shape, asserting on render_png's wall-clock time and both the PNG size and the embedded JSON size. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
ffb40d271f |
fix: settings card export -- round 3 found the DoS bound in a new place
extensive_testing round 3 found round 2's fix was itself incomplete: it
bounded row count and per-key value length, but not the JOINED row string
_rows_from_list builds from a dict with many keys. A funpack_list entry
like {"k1": "x"*400, ..., "k150": "x"*400} produced one row whose value had
no length cap of its own -- row count and per-key length both stayed under
every existing budget while the report JSON embedded in the PNG's tEXt
chunk (which scales 1:1 with it) reached 123MB, reproducing round 2's exact
number through a different attack shape, with zero truncation declared
anywhere on the resulting card.
Fixed by running the joined row string back through _short() (closing the
length gap) and capping how many keys get processed per row at 50 before
that join even happens (closing a related CPU-amplification gap: an absurd
key count still cost one f-string+join per key before any length cap could
apply). Added a test reproducing the exact many-key-row shape and asserting
on render_png's actual wall-clock time and output size, not just collect()'s
data.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
7005de86e5 |
fix: settings card export -- round 2 found the DoS bound still had gaps
extensive_testing round 2 found the round-1 render-cost fix was broken: - _MAX_ROWS and _MAX_VALUE_CHARS each bounded one factor but never their product -- 2000 rows of 400-char no-space values honored both caps individually and still cost ~14s/~10MB per render. - collect()'s card-wide row budget was checked once per SLOT, so one slot with many funpack_list inputs (each independently capped at _MAX_ROWS) could blow the total budget by 50x with no declared truncation -- 67.8s, ~123MB, and a PNG that trips Pillow's own decompression-bomb guard on reopen. Worse than the original round-1 finding, not fixed by it. - none of round 1's three new tests called render_png -- the function whose cost was the actual finding -- so both regressions passed all 21 tests. Fixed by bounding the actual cost driver directly: render_png now caps total rendered lines at 1200 regardless of how collect() produced them (defense in depth), and collect()'s row-budget check now runs per input, not per slot, so one slot's inputs can no longer accumulate past the card budget before the next slot's check fires. Added tests that call render_png against both worst cases and assert on wall-clock time, not just collect()'s output data. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
691f17168b |
fix: settings card export -- bound render cost, stop a blob-URL leak
Round 1 of extensive_testing on
|
||
|
digital-garbageandClaude Sonnet 5
|
6d496251f9 |
feat: settings card export -- render the live pipeline as a PNG
MEDIUM-tier v4 feature ("Which model was that?" answered three days
later): a button in Models & Pipeline renders the currently loaded
pipeline as a PNG -- every slot's node class and typed-in values, wired
inputs marked as wired rather than printed as a stale value, and the
host facts (torch/CUDA/GPU/attention) that decide whether the setup
reproduces elsewhere. Downloadable, copyable to clipboard, and the
report is embedded in the PNG's own tEXt chunk for later machine
reading.
core/settings_card.py is a hard cut of v4's version, not a verbatim
port: v4 spent most of its length (_OWNED_BY, _EDITOR_DEFAULTS,
_pass_rows, _sampling_sections) describing FunPackStudio and the
LTX-AV Scene Chain Sampler's own hardcoded knob tables -- neither node
exists in v5's module-driven pipeline, so there is nothing left to
describe there. What's genuinely generic (walking a slot's real
inputs, detecting a wired one via core.graph.is_link, the PNG
renderer) is what's kept, adapted to v5's slot shape ({id, node,
group, inputs}, a wire arriving as [source_id, index] rather than a
separate input_sources/wires lookup) and v5's existing core/sysinfo.py
for host facts (whose field names already matched v4's needs).
New POST /api/settings-card route: stateless like /api/pipeline, the
CLIENT sends its own live pipeline (window.PipelineState.slots()) since
that's the one copy that includes any unsaved edit; the server supplies
only what it alone knows (host facts) and the render.
Frontend: app/legacy/api.js's settingsCard stub (present since the
Phase A port, explicitly marked "not built in v5 yet") replaced with a
real POST+blob call; app/legacy/models.js's Export-settings modal
ported from v4 near-verbatim, reading PS.slots()/project name instead
of a project id the old per-project models config needed. Verified
live: real pipeline, real host facts (this Mac's actual Apple MPS/
Python/ComfyUI versions), wired inputs correctly marked.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
06621bb2e3 |
docs: note has_block's contains-not-is limit on SLA's H3 gate
Round 2 of extensive_testing on
|
||
|
digital-garbageandClaude Sonnet 5
|
548d99a39b |
fix: SLA's H3 gate now matches the qualified class, not a bare name
Round 1 of extensive_testing on
|
||
|
digital-garbageandClaude Sonnet 5
|
79ca8cfb6b |
feat: SLA block-sparse attention for MiniMax H3, ported from v4
Loader-level attention option: mean-pool Q and a smoothed K into blocks, score them with one small matmul, attend only the top 1-sparsity fraction of key blocks per query block, via a vendored Triton kernel. Runs alongside the chosen attention backend rather than replacing it (one override slot; the backend becomes SLA's dense fall-through for the text refiner, masked calls, and trailing dense steps), and refuses cleanly -- naming why -- on anything that is not MiniMax H3 or a machine without CUDA+Triton. modules/loaders/sla_block_map.py and sla_kernel.py are near-verbatim ports of v4's vendored files (no FunPack-specific imports, so nothing to adapt); sla_attention.py is the loader-facing glue (H3 gate, override composition, per-step wrapper), same content as v4's with a "not a module" note added to match gguf_support.py's convention. Wired into FunPackDiffusionModelLoader as six new optional widgets, following v4's loaders.py wiring but expressed in v5's io.Schema style. The Triton kernel itself is unexercisable on this Mac (no CUDA) -- matches how real generation is verified in this project (rented GPU, not local). Ported the CPU-testable half of v4's test suite (the override contract, the H3 gate, the per-step wrapper); the three kernel-correctness tests carry the same CUDA+Triton skip guard v4 used. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
fe8d530a13 |
fix: reference-image wiring -- derive stale-render notice ownership from text, not a hand-tracked flag
Round 3 of extensive_testing found ae319cb's _noticeIsStaleRenders flag had the exact footgun it was built to avoid: exportSelected() and saveSelectedToMediaBin() also write state.notice (through the generic set() helper, spelled `notice:` in an object literal -- invisible to a `state.notice =` grep) and never cleared the flag, so a pending stale-render flag could survive into an unrelated export/save confirmation and have _recordSegment wrongly clear THAT instead. Every future writer of state.notice would have needed to remember to maintain a parallel boolean by hand -- two already didn't. Replaced the flag with a regex test against the stale-render sweep's own known message template: _recordSegment now clears state.notice only when its current text actually IS that message, derived from data instead of tracked by convention. No writer needs to know about any other writer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
ae319cb3bd |
fix: reference-image wiring -- _recordSegment was a third clobberer of state.notice
Round 2 of extensive_testing on
|
||
|
digital-garbageandClaude Sonnet 5
|
2e738a831e |
fix: reference-image wiring -- notice race and dangling-source masking
Round-1 adversarial review of
|
||
|
digital-garbageandClaude Sonnet 5
|
1abf136e03 |
feat: wire per-scene reference-image picking into the real generate flow
Fills in the placeholder left in _buildQueueInputs: v5's real per-scene
fields (core/projects.py's Scene.source_image/.references) already
round-trip through api.js's getProject/saveProject today -- it's a raw
whole-object PUT/GET, and the server's own Scene.from_dict()/asdict()
extract and return them regardless of what else v4's project shape carries.
There was just no UI to set them and nothing reading them at generate time.
Added:
- store.js: setSourceImage/setReferences, thin patchScene wrappers (same
pattern as the existing setSourceTrim).
- inspector.js: a "Reference media" section on the scene panel, right after
v4's own i2v anchor/Source field, using the same window.MediaPicker
thumbnail-grid component already used throughout this file for image
picks -- a single Resolution-source picker plus an add/remove list for
References.
- generate_bridge.js: exposes reference_wiring.js's wireReferences()
(untouched, already tested, no DOM dependency).
- store.js's _buildQueueInputs: sets assets.source_image from the run's
first/anchor scene, and calls wireReferences() for its references --
same "first scene stands for the whole run" convention its prompt-text
branch already uses for a multi-scene (whole-timeline) run. An unwired
reference (no matching slot in the current pipeline) surfaces as a real
notice in the timeline header, not a silent drop.
Live-verified against the dev server end to end: picked a real reference
image from the Media bin, watched it persist server-side
(GET /funpack/api/projects/:id showed it in the saved file), then clicked
Generate and confirmed the pipeline check ran with it -- the current
default pipeline has no reference slot, and the app correctly said so
("1 reference could not be wired into this pipeline and will not be
used.") rather than silently dropping it. 691/692 JS tests and 548/548
Python tests pass (the one JS failure is the pre-existing, unrelated
projects.test.js import-race flake).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
68df7ba866 |
fix: replay the "adopt" event to a listener that registers after it already fired
Round 5 of adversarial review found a real ordering race: generate_bridge.js's
module-level wire() call starts reattach() (a /queue or /history round trip)
the moment the module evaluates -- before store.js's classic script has even
run, let alone reached _wireGenerateBridge() (called deep inside init(), after
seven sequential awaited setup calls). On a reload during or just after a
generation, "adopt" routinely fired and was gone before store.js ever called
GB.on("adopt", ...): _genRunSceneIds stayed [], so the adopted run's eventual
result was never recorded onto its scene (_recordSegment's own length guard
silently skipped it), and _genRunActive/_genAdoptedRun never got set, leaving
that run invisible to the overlap guard this whole review thread has been
hardening.
Fixed by giving "adopt" -- and only "adopt", since it carries the sole copy
of which scene(s) a reattached run belongs to, unlike the transient "say"/
"warn"/"hold"/"release" status text, which is fine to miss -- replay
semantics: the bridge remembers the last adopt payload and hands it to any
listener that registers afterward, immediately and synchronously, the same
way it would have if the listener had been there in time. Verified with a
standalone reproduction of the exact closure logic (late subscribers,
including a second one, both receive the replayed payload correctly) and
against the real dev server (boots clean, no regressions). 691/692 JS tests
and 548/548 Python tests pass (the one JS failure is the pre-existing,
unrelated projects.test.js import-race flake).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
a742318cc0 |
fix: give the in-flight guard its own dedicated flag instead of reusing state.gen
Round 4 of adversarial review found that the previous commit's overlap
guard, keyed off state.gen.state, produced false refusals: Export, Save to
Media bin and Render Final all set state.gen to "running" for their own
entirely local work, so clicking H3 Combo Sweep's "Run sweep" button while
any of those was in progress got refused with a message borrowed from that
unrelated job ("Stopped at ...: Exporting 3 clips…") -- not silent, but
actively misleading about the real cause.
Switching the check to GenerateBridge's own run.state.phase (considered as
the obvious fix) would have reopened the very race round 3 closed: phase
stays "idle" for the whole check() network round trip, before run.start()
ever fires, so a second call could still slip through during that window.
Fixed with `_genRunActive`, a flag dedicated to exactly one question --
"does GenerateBridge have a run of this page's making in flight" -- set
synchronously the instant a call is accepted (before any await, so two
calls fired back to back still serialize) and cleared either by the
terminal-phase subscription (covers a run that actually started, click or
adopted) or by _generateRun's own `finally` (covers a run refused before
ever leaving "idle", which that subscription never sees).
Live-verified: a state.gen "running" left over from an unrelated job no
longer blocks a real generate() call, while two genuinely concurrent
generate() calls still correctly serialize (one proceeds, one is refused
with the accurate reason). 691/692 JS tests and 548/548 Python tests pass
(the one JS failure is the pre-existing, unrelated projects.test.js
import-race flake).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
f803b1a329 |
fix: refuse an overlapping generate() instead of corrupting the in-flight run's state
Round 3 of adversarial review found that GenerateBridge now holds exactly ONE run's shared state (_genRunPrefix/_genRunSceneIds/_genAdoptedRun, window.GenerateBridge's own module-level ranFor/ranForList), but only the main Generate/Selected/Render buttons are gated against starting a second one while the first is still queuing/running (actionbar.js's busy(st)). H3 Combo Sweep's "Run sweep" button has no such gate and calls _generateRun directly -- if clicked while another generation (including one adopted on reload) is already in flight, it silently overwrote that run's scene attribution and status message, got refused deeper inside (session.js's own starting/phase guard), and left the REAL run's eventual result recorded onto the sweep's scene instead of its own when it finished -- persistent data corruption, not just a UI glitch -- and could leave _genAdoptedRun stuck false, reopening round 1's "adopted run's clock never stops" bug. Fixed at the root: _generateRun itself now checks state.gen.state before touching any of that shared state, and refuses (loudly, via console.warn) rather than clobbering it. Fixes every current and future caller in one place, not just block_sweep.js -- consistent with every other caller already being a sequential, awaited loop that this guard doesn't affect. Live-verified: two overlapping Store.generate() calls now produce exactly one real attempt and one logged refusal, with the first call's status never overwritten. 691/692 JS tests and 548/548 Python tests pass (the one JS failure is the pre-existing, unrelated projects.test.js import-race flake). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
c67e223937 |
fix: don't stop the batch wall clock after the first run of a montage/selection
Round 2 of adversarial review on the previous commit found that _genClockStop() firing unconditionally on every run's terminal phase (added to fix an adopted run's clock never stopping) broke the OTHER case it didn't account for: generateMontage()/generateSelected() call _generateRun several times under one press for a chain-sampler pipeline (_runs() splits the timeline into several runs whenever PipelineCaps.usesChainSampler() is true, which is the default). genWallStart is documented to span the WHOLE batch, started once by _clockedMontage/_clockedSelected's own wrapper -- stopping it after run 1 of N left genElapsed() returning null, and nothing in the batch loop ever restarted it, so the visible timer next to Generate disappeared for the rest of an in-progress batch. Fixed with a `_genAdoptedRun` flag: only a run actually ADOPTED on reload (nothing else will ever stop its clock) clears it from the persistent subscription; a normally-clicked run's terminal phase leaves the clock for its own caller's `finally` to stop, same as before this whole feature existed. 691/692 JS tests and 548/548 Python tests pass (the one JS failure is the pre-existing, unrelated projects.test.js import-race flake). Live-verified against the dev server. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
aa1cec7817 |
fix: carry every scene a multi-scene run covers through reattach, not just the first
Adversarial review of the previous commit found four real defects, all confirmed: 1. A run covering more than one scene (the whole-timeline H3 shape, or any single-scene click whose gen-unit has sibling subclips) only ever sent the FIRST scene id as extra_data.funpack_scene_id. On reload mid-run, reattach recovered just that one id, so _recordSegment only ever wrote the finished media onto scene 1 -- every other scene the job actually rendered silently got nothing recorded, no error shown. Fixed by adding funpack_scene_ids (plural) alongside the existing singular field, threaded through client.js's queuedFor and session.js's reattach() (both additive -- a new optional third onAdopt argument and a new optional response field, ignored by every existing caller such as app/boot.js) down to generate_bridge.js/store.js, which now record onto every adopted scene. 2 & 3. A run adopted on reload never primed pollStart/genWallStart, so _elapsed() and the transport button's own clock read time since the Unix epoch instead of real elapsed time, and never stopped afterward -- wedging genElapsed() for the rest of the session (its own guard is "if already started, do nothing"). Fixed: the "adopt" handler now primes both clocks (flagged approximate, same as the deleted resumeRunningGeneration used to), and every terminal run phase now stops them. 4. check()'s `notes` field (e.g. "N settings didn't reach the pipeline") was fired through transport.warn() but nothing ever subscribed to GenerateBridge's "warn" event -- silently dropped, reproducing the exact bug class the deleted code's own comment said must not recur. Fixed: wired into state.notice, same as the old code did. New tests added for both the plural-scene-id path (client.test.js, session.test.js) and the fallback-to-one-scene case. 691/692 JS tests and 548/548 Python tests pass (the one JS failure is the pre-existing, unrelated projects.test.js import-race flake). Live-verified against the dev server: a multi-scene GenerateBridge.generate() call still correctly surfaces a real pipeline refusal with no console errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
bb404d988d |
feat: wire Generate/Cancel/reattach through v5's real check->queue->terminal flow
Adds generate_bridge.js, a small ES-module shim over the untouched app/shell/run.js + session.js + pipeline.js + client.js, and rewires store.js's generate()/generateMontage()/generateSelected()/interrupt()/ init() reattach to go through it instead of the now-permanently-stubbed API.generate/status/active. PipelineState (pipeline_state.js) stays the one client-side copy of the live pipeline; the bridge only adds the run/queue/socket layer on top of it. Single scene (or the whole active timeline, for a non-chain pipeline like H3) per real queue -- v5 has no chain-sampler/montage stage. REINS captures and the anchor-guide i2v bypass are said loudly (console.warn) rather than silently dropped, since neither has a v5 equivalent yet. Reference-image wiring is left as a placeholder: legacy's saveProject/getProject adapter doesn't round-trip source_image/references yet, so there is nothing to wire in from there today. Live-verified against the real dev server: a real Generate click built the scene's prompt, POSTed it through /funpack/api/pipeline's check, and surfaced the genuine refusal reason (an unset model loader) in the existing status UI, with the button correctly re-enabled after. 688/689 JS tests and 548/548 Python tests pass (the one JS failure is the pre-existing, unrelated projects.test.js import-race flake). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
c6559765b7 |
fix: pipeline_state.js -- deep-merge queued saves, close a load race
Review found the save() queue's pendingBody merge was shallow: two queued
edits that both touch the top-level "inputs" key (two different slots, or
even two fields on the same slot from models.js's per-call-fresh objects)
had the second's inputs object silently REPLACE the first's rather than
combine -- the earlier edit vanished with no error, the field just
reverted on the next render. engine_settings.js's own calls happened to be
immune by accident (one long-lived mutated `values` object, so the last
queued snapshot already contained every earlier edit) -- that immunity
was never a property of save() itself, and models.js's fresh-object-per-
call shape hit it immediately. Fixed with a one-level-deeper merge for
"inputs"/"values" specifically. Verified live via two real race
reproductions run through the browser console: two edits on different
slots queued with no await between them, and two edits on the SAME slot --
both survive now, confirmed against the actual saved graph state.
Also closed a load race: ensureLoaded()'s only guard was `slots !== null`,
which doesn't cover the window while a first load is still in flight -- two
panels mounting close together could both start a fetch. Now every caller
awaits the same in-flight promise, so exactly one fetch ever goes out per
session.
Documented (not fixed, deliberately) that Models & Pipeline's node-level
pin target is currently unreachable: pinTarget() only ever returns a
group-level pin today, since there's no "currently open node" concept in
this flat slot list yet -- the openNode/ctx.sub("node:...") plumbing stays
for whatever old pin data might still reference it, but nothing produces
a fresh one. Real gap against v4's per-node pinning, not a regression.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
770d58a315 |
feat: schema-driven Models & Pipeline, plus shared pipeline state
models.js's ~2700 lines were entirely built around v4's Studio "roles" and "candidates per role" concept, neither of which exists in v5 -- the pipeline is a flat, swappable slot list, not a fixed named-role set. Replaced with a renderer over the live slots (GET /api/pipeline) grouped the way the pipeline itself groups them (Loaders/Preparation/Sampling/ Render), each slot's real widget inputs read live from GET /api/nodes and edited through the same /api/pipeline POST. The "not ready to generate yet" / incomplete-inputs list is now real and visible, not implied. Node add/remove/swap/wire is NOT built here -- v4's graph-editing machinery assumed nodes that no longer exist, and a real editor is its own piece of work; this establishes the actual working file-picker surface first. Extracted `pipeline_state.js`: engine_settings.js and models.js both edit the SAME live pipeline, and each independently caching its own `slots` copy (as Engine Settings' fix earlier this session did) would let one panel's save silently revert the other's un-saved edits the moment the user switched between them -- the exact stale-state bug already fixed once this session, about to recur in a second file. One shared module now owns `slots`/the manifest/the save queue; both panels are thin renderers over it. engine_settings.js refactored to consume it (no behavior change, confirmed live). Live-verified end to end: picked real model/clip/vae files scanned off disk, watched "incomplete" clear field by field down to "ready to generate", and got back a genuine queueable ComfyUI prompt graph. Then confirmed the cross-panel fix specifically: saved an Engine Settings toggle afterward and the POST still carried every file pick Models & Pipeline had made -- neither panel clobbers the other. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
2831a004b6 |
fix: scope valuesAlreadyPlaced() to known module ids
Round-2 review: the reopen-persistence fix scanned every string input on every slot for anything that JSON-decodes to a plain object, with no check that it's actually the real settings blob -- a coincidental match (any future node with an unrelated JSON-object string input) could inject a key under a module id that doesn't exist, which then round-trips forever through every future save via place()'s blind overwrite. No route exposes the real sink's (node, input) identity to the client to look it up properly (core doesn't name an implementation, by design), so the fix narrows the heuristic instead: a decoded key is only merged when it names a module this session already knows is installed. Not reachable with any real module today (verified), but was a live hazard for the first one that collides. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
99f3062ada |
feat: schema-driven Engine Settings, replacing v4's hardcoded Studio/Chain panel
engine_settings.js's ~1100 lines were entirely hardcoded around
FunPackStudio/Chain Sampler fields that don't exist in v5. Replaced with a
renderer over the real module manifest (GET /api/modules): groups every
loaded module's settings by category, builds a typed control per setting
(bool/int/float/enum/color/text), honors `when` conditional visibility, and
saves through the real /api/pipeline POST -- the same settings_sink
mechanism core/graph.py already had, just never had a caller. Outer
registration/DOM shell (SettingsWindow section, EngineSettingsModal.open/
close) kept identical so every existing entry point still works. Added
three new real api.js functions (modules, pipeline, editPipeline) -- v5-
native capability, no v4 equivalent existed.
Adversarial review found and fixed two real defects before this landed:
(1) every reopen re-fetched the (stateless) server default and reseeded
from schema defaults, silently discarding every previously-set value
across every module -- fixed by only fetching once per session and
holding `slots`/`values` in memory across mount/unmount, since the server
itself remembers nothing between GETs; (2) a second edit made while the
first's save was still in flight was dropped rather than queued -- fixed
with a trailing-edge retry so `commit()` re-sends after the in-flight
request resolves if anything changed meanwhile. Also fixed api.js's error
reader missing /api/pipeline's one inconsistently-named 400 field
("refused" instead of "problems"). Live-verified: toggled settings on two
different modules across two categories, closed and reopened the panel,
both survived; the real graph's FunPackModifierSettings node's `settings`
input genuinely updated end to end.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
776529e0d8 |
feat: assign category to every remaining module
The port plan calls for every module under modules/ to declare a CATEGORY so Settings/Engine panels can group by it. Only alg, sharpen, appearance, and full_control had one; the other 12 (loaders, empty latent, media load, minimax_h3, output decode, sampling modifiers/ sampler, system pipeline) are plain infrastructure with no user-facing settings dial, so they land in "system" except output_decode (post, matching its STAGE) and the two sampling modules (sampling). Verified against core/tests (548 passed) -- category validation and the registry's field-mapping catch a typo here, per the earlier fixture-gap fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
ebd7a94215 |
fix: api.js -- stubs reject instead of resolving, stop double-unwrapping lists
Round-1 adversarial review found the resolve-based unsupported() sentinel
broke every existing try/catch in v4's ~60 caller files (they were written
for a rejecting promise, same as any other failed fetch) -- worst case was
the Models & Pipeline panel hard-crashing on open, the one caller api.js's
own comment claimed was safe. Switched unsupported() to reject; the
existing defensive code across the callers now does exactly what it always
did. Also fixed a self-inflicted double-unwrap in listProjects/listMedia/
shortcuts (api.js unwrapped {field:[...]} to the array, but every real
caller still destructures .field off the result -- onboarding's "resume a
project" list was silently always empty), and switched five stubs that
threw synchronously (escaping before a caller's .then/.catch chain could
even attach) to async so the throw becomes a normal rejection. Live-
verified: Models & Pipeline now shows "Could not load" instead of
crashing, "Continue with existing project" reappears in onboarding,
Export Settings shows its real failure message instead of hanging.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
b7acc2e91a |
feat: rewire api.js against v5's real routes, stub the rest safely
Three buckets per the port plan: (a) same-shape routes just renamed
(projects, media, shortcuts, packs, git/system, temp, log), (b) a small
adapter where v5's shape differs (parsePrompt -> /api/prompt/expand,
resultUrl -> ComfyUI's own /view), (c) no v5 equivalent yet -- refinement
keys, REINS, probes, render/stitch, Models/pipeline-roles, Generate's
actual queue flow.
Every stub resolves to {unsupported, reason} instead of throwing, so a
module nobody has built yet can't take a caller down with it -- the
"feature-independent" backend the port asked for. Live-verified: wizard
through project creation into the real editor, zero console errors, every
real route round-tripping 200 OK.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
digital-garbageandClaude Sonnet 5
|
6303f1312c |
fix: retire the stale composer layout suite, close a registry test gap
Two real findings from adversarial review of the previous commit (the v4 UI port's route change), both fixed: - app/tests-layout/*.spec.js (20 files, 133 tests): every one of these navigated to /funpack/ and asserted on composer-specific DOM (.cx-workspace*, window.FunPack) that no longer exists there now that the ported v4 frontend is served at that URL instead. Left as-is they would silently time out on every run. Each file now carries an explicit test.skip(true, "...") with a comment explaining why and pointing at the plan's pixel-exactness step, which is what will eventually rewrite these against the new DOM -- declared and tracked, not quietly broken. - core/tests/test_registry.py's fixture-based tests never exercised the new category field through the real announcement_of()/scan() path -- only test_schema.py's direct validate() calls did, which bypass registry.py's FIELDS name-mapping entirely. A typo there (e.g. "Category" instead of "CATEGORY") would have passed every existing test. The "good module" fixture gained CATEGORY = "guidance" and test_the_good_module_loads now asserts on it, closing the gap. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
29ac44ffcd |
feat: v4 UI port Phase A start -- module categories, legacy frontend served live
Two pieces of the "port v4's exact UI onto v5's backend" plan: - core/contract.py: new optional ModuleSpec.category field (closed set: continuity, guidance, conditioning, sampling, post, system), validated in core/schema.py, threaded through core/registry.py's FIELDS and to_manifest(). Presentation grouping, independent of STAGE (execution order) -- a module can be stage="sampling" and category="guidance" at once. Only the 4 existing modules that actually render a settings panel (alg, sharpen, appearance, full_control) got a category assigned; the other 12 have none to categorize (no SETTINGS block) and stay uncategorised, which is valid. - v4's frontend (movie_editor/frontend/*, ~29k lines, 61 files) copied verbatim into app/legacy/. core/routes.py's "/" route now serves legacy/index.html instead of the composer entry point; a <base> tag in that file resolves its own relative script/link tags against /funpack/app/legacy/ regardless of what URL it's served at, so every file moved as a unit with zero per-file path edits. Verified live: all 61 assets load 200 OK, the real v4 onboarding screen renders pixel-for- pixel (it's the actual DOM/CSS, not a reimplementation), and several v4 API calls (/api/media, /api/projects, /api/git/status) already succeed against v5's real backend by coincidence of shared route naming -- the rest 404/405 as expected, since api.js's ~90 functions haven't been rewired to v5's actual routes yet (next piece of this plan). The old composer entry point (app/index.html, app/composer/**) stays in the tree, unreferenced, until a later cleanup pass once nothing imports it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
a600fe3e29 |
fix: real timeline -- full-width layout, playhead reentrancy, panel overlap
Round 1 + round 2 of adversarial review on the single-track timeline
(
|
||
|
digital-garbageandClaude Sonnet 5
|
2092c8b67e |
feat: a real single-track timeline -- seconds, playhead, click-to-seek
Replaces the scene-index-tick ruler with composer's new track.default element: clips sized by real duration (frames/fps, not equal-width boxes), a ruler in genuine elapsed seconds, and a playhead that follows the selected scene but can be dragged to any exact second within a clip via click-to-seek. Also fixes a real crash found while live-testing this: addScene() built a scene object missing source_image/references, which crashed the Inspector the moment it tried to draw the new scene. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
9e3adf250d |
feat: an "Add group/node" card in the pipeline window
Groups the pipeline window into a card grid with a "+ Add group/node" card beside it -- matching v4's card-per-loader layout instead of a separate toolbar button. Picking it offers a new empty group or a node outside any group (landed in the existing "Other" bucket). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbageandClaude Sonnet 5
|
7a226830b1 |
feat: GGUF loader support, and fix silent pipeline-editor state loss
GGUF: port v4's gguf_support.py (backend selection, content-based .gguf detection, dequantize fallback) and wire it into both the diffusion-model and CLIP loaders. Pipeline editor: reopening the Models-and-pipeline settings window was silently discarding the whole edited pipeline (a loaded preset, every loader file, every sampler setting) because GET /api/pipeline is deliberately stateless server-side and boot.js never fed its own already-saved slots back into a freshly remounted editor -- found live while walking through the MiniMax-H3 setup flow. Fixed by re-validating the saved slots through check() on reopen instead of always replaying the raw stateless default. Also fixes, found via the extensive_testing adversarial loop over both of the above: a stale file-listing cache in the pipeline window (no way to see a newly-added model file without a full page reload), and a chain of races/leaks in its own refresh/commit error-banner handling that surfaced across five review rounds -- a rejection from a superseded request overwriting a fresher success, a failure banner never clearing on success, banners bleeding across groups on navigation, and a commit()'s refusal landing on the wrong view after the user backs out before it answers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
digital-garbage
|
4744d30087 |
fix: H3 preset combo values rejected by real node schemas
keep_proportion/extra_padding/invert_crop (ImageTransformKJ) and format
(SaveVideo) were wrapped in a redundant {"field": "value"} dict instead of
the plain string ComfyUI's own DynamicCombo expansion expects at the wire
level -- every one of them silently failed graph validation and fell back
to the node's own default, undermining the "total_pixels" resolution-fit
behavior this preset exists for. Found by loading the preset in a real
browser against real node schemas; the dev server never had comfy_extras
or KJNodes registered before now, so this was invisible locally until
tools/devserver.py also gained real node registration (init_extra_nodes +
KJNodes' load_custom_node, matching what the pytest `registered` fixture
already did).
The two pipeline_preset tests that were supposed to catch this had too
broad a filter -- it excluded ANY "is not one of" message as an assumed
"combo not chosen yet" state, which in this codebase actually shows up as
"needs X and nothing fills it" instead. Tightened to exempt only the three
loader-file combos genuinely left unset on a fresh install.
Verified live: preset loads with zero combo-validation problems, only the
three expected unset-loader-file messages remain, and Generate correctly
refuses with an honest reason rather than a false success.
|
||
|
digital-garbage
|
febb67c1ab |
fix: reference-wiring silent drops/overwrites, found by adversarial review
wireReferences() (new, extracted from boot.js's queueInputs) now reports an unwired count instead of silently losing a reference when: the pipeline has fewer reference slots than the scene has references, two references' roles resolve to the same slot or downstream destination, or a reference's slot was already claimed by the scene's source-image assignment. boot.js toasts whenever unwired > 0, worded to cover both the capacity and collision cases. None of these are reachable through the one shipped preset (H3's R2V, one role per slot) -- all are latent contract violations a hand-edited or future preset could trigger. 10 new tests in reference_wiring.test.js. |
||
|
digital-garbage
|
33e2b02f4d |
app: a "Load preset..." button in the pipeline window, wired to the new route
The last missing piece connecting the backend preset mechanism to something a person can actually click: pipeline.js gets a presets() fetcher matching load/describe/check/search's own shape, and pipeline_window.js's group-card index offers a "Load preset..." button (only when a caller passes one) that lists what /api/pipeline/presets returns and replaces the pipeline through the same commit()/check() path every other structural edit already goes through -- a preset that does not actually build is refused here, same as a hand-drawn bad wire would be, never shown as if it had loaded. Writing the test for this caught a real bug before it shipped: open()'s own destructuring silently dropped `presets` before ever reaching mount(), so the button would never have appeared no matter what boot.js passed it. |