main
19
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 malkuthroandCursor
|
7050f2328e |
Release v0.5.0 (#283)
* Release v0.5.0: stability fixes for drift status and draft toasts Co-authored-by: Cursor <cursoragent@cursor.com> * fix(tests): stop hardcoding pack version in version-route assertion Co-authored-by: Cursor <cursoragent@cursor.com> * fix(tests): parse pyproject independently in version-route assertion Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
malkuthro
|
7b00a551d9 |
feat(publish): independent output type for published setups; fix(pipeline): auto-versioning re-runs each queue (#267)
Publish: Koolook_PublishOutput output_mode/input_switch/switch; setupSurface.app.outputSwitch; routers auto-bound as the output selector; unwired output types hidden; 'Same as input' offered only when every input type has a wired writer; result path follows the output switch. Pipeline: EasyAIPipeline IS_CHANGED keeps version=auto/next perpetually dirty so the vNNN disk scan re-runs every queue; literal versions stay cacheable. |
||
|
malkuthro
|
24857d5e9c |
Release v0.4.4 (#263)
* Release v0.4.4: funding metadata * docs: add GitHub footer to v0.4.4 notes |
||
|
malkuthro
|
575cfb7f63 | Release v0.4.3: publish latest updates (#256) | ||
|
malkuthro
|
543998e026 |
Release v0.4.2 (#253)
Bump 0.4.1 -> 0.4.2. Ships #251 (Comfy Registry node-list parsing fix) and records #252 (tag-driven automated release pipeline). First release cut via release.yml. |
||
|
malkuthro
|
bda4df576a |
Release v0.4.1 (#250)
* Release v0.4.0: update footer notice * Release v0.4.1 |
||
|
|
6f158c9020 |
fix(nodes): Easy_LoadVideo empty-branch degrade (bundles #243 publish-tab cleanup) (#244)
* docs(setups): add setup runner quickstart runbook Standalone reference for opening the published-setup mockup frontend (setup runner simulator): the `<comfy-address>/koolook/setup_runner_simulator.html` URL pattern, the live-registry vs setup-file (inspect-only) load paths, and demo mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(sidebar): close throwaway workflow tabs left by publish capture Publishing a setup captures ComfyUI's API-format prompt by loading the graph onto the canvas (graphToPrompt) and reloading the prior canvas. In current Comfy each loadGraphData spawns an "Unsaved Workflow" tab, so every publish left two stray tabs behind. captureWorkflowApiPrompt now snapshots the open workflows before capture and, afterward, re-activates the user's original tab and closes whatever the capture opened, via app.extensionManager.workflow. All of it is guarded and best-effort: the workflow-store API is Comfy-version specific, so if it is absent capture still succeeds and behavior falls back to today (no crash, no broken publish). Covered by a source-pattern test. Needs live verification against the 1.45 frontend (dev-sync) to confirm the closeWorkflow API shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(nodes): Easy_LoadVideo degrades to empty on an unselected folder branch When an upstream mode switch routes a path into Easy_LoadVideo, both the EXR sequence loader and the video loader execute eagerly. On the EXR branch the video loader was handed a folder (no filename) and raised ValueError, crashing the whole prompt -- even though that branch is discarded downstream. Lenient sequence loaders just ignore a wrong-type path; the video loader was the only one that hard-failed. _compose_input_video_path now returns EMPTY_BRANCH_SENTINEL instead of raising when input_path resolves to an existing directory. load_video turns that into a correctly-shaped empty result (IMAGE zeros, frame_count 0, empty AUDIO, empty video_info) matching VHS_LoadVideoPath returns, so the dead branch no longer crashes the prompt. VALIDATE_INPUTS and IS_CHANGED handle the sentinel too. A path that does not exist is still a genuine mistake and keeps raising. torch is imported lazily in the empty-result builder so the path logic stays importable/testable without torch. Covered by unit tests for the sentinel (absolute + relative dirs), the still-raises case, and a load_video integration test that asserts the empty result and that the loader never runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(setups): expose prompt as an always-on field in the publish frontend The publish surface modelled `prompt` as a 4th source mode (mutually exclusive with EXR/QT/Img) and hid it (visible:false), so a published setup couldn't offer "pick a source AND describe the shot". This makes prompt a standalone, always-on input. - published_surface.js: prompt becomes visible + standalone + multiline, rendered independently of the source switch; the author's prompt-widget text is surfaced as a `placeholder` hint (submitted default is empty so an untouched hint is never sent), with a `help` line. The switch excludes standalone fields from its source options. - setup_runner_simulator.js: renders always-on standalone inputs alongside the selected source (textarea + placeholder + help), includes them in the run payload, and updates the demo setup. - setup_runner_simulator.html: .field-help styling. - contract doc: documents the always-on prompt behaviour. - launch.json: portability fix (hardcoded C:\Python313 -> python). Contract test updated + a new test asserting prompt is always-on, carries the placeholder, and is excluded from the switch source options (runs on CI where Node is available). Behaviour verified in the runner simulator (demo mode) in a browser: textarea + hint, EXR/QT/Img-only switch, prompt stays visible across source changes, payload includes the typed prompt. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(setups): mirror always-on prompt field in the Python publish inference The publish surface is inferred server-side in Python (koolook_setups.py _infer_app_surface, called by publishSetup), which overrides the client. The earlier change only updated the JS (published_surface.js), so re-publishing still produced the old surface with a hidden prompt. Mirror the JS here: prompt is a standalone, always-on input (visible + multiline) with the author's prompt-widget text as a placeholder hint and an empty submitted default; it is excluded from the source switch options. Keeps koolook_setups.py and published_surface.js in sync. Registry test updated to the new shape. Server + publish-contract suites green (115); full non-sidebar suite green (530). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(setups): keep the runner simulator polling through long renders The poller used a fixed 30s total budget, so a multi-minute render (e.g. 428s) timed out mid-run with "Timed out waiting for run" even though ComfyUI was still rendering and the run later landed as succeeded. timeoutMs is now a "no-progress" window: while ComfyUI keeps reporting the run as queued/running, the deadline is pushed forward, so a long-but-active render never spuriously times out; only a stalled/vanished run gives up. Default raised 30s -> 120s. The run status line shows elapsed seconds while active so a long wait visibly ticks instead of looking frozen. Happy path (queued -> running -> succeeded) unchanged; verified in the runner simulator (demo mode) in a browser. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(setups): serve simulator assets with no-store so dev-syncs take effect The runner-simulator HTML/JS route returned no cache headers, so browsers cached the assets and kept running stale JS after a dev-sync -- e.g. the old 30s poller persisted client-side even though the server was serving the long-render fix, which looked like "the fix didn't land". Serve these dev-synced assets with Cache-Control: no-store so a normal reload always picks up the current code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(setups): add live run/poll smoke check for the published-setup API tools/check_setup_run.py queues a published setup through the same routes the runner simulator uses and polls the run endpoint with the same deadline-extension logic, printing the status timeline. Lets us self-verify end-to-end run tracking against a live ComfyUI (e.g. "does a long render reach succeeded?") without driving the browser UI. Used it to confirm the run/poll server flow is correct (terminal status detected in ~2s; run-000001 reported succeeded), isolating the simulator's earlier "Timed out" to stale browser cache rather than a server bug. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(nodes): Koolook_NextVersion auto-detects the next output version Adds filesystem-aware version auto-detection so a re-run lands on a fresh version instead of failing with "Output file already exists and overwrite is disabled" (the real EasyAIPipeline failure seen on the RMGB setup). - koolook_versioning.next_version_token(directory, name, ...): scans for existing <name>_vNNN files AND sequence-folders (handles name_v002.png, name_v002.0001.exr, name_v003/), returns highest+1 zero-padded; missing/ empty dir -> start (v001); exact base-name match so other shots never bump this one; empty name matches bare vNNN; configurable prefix/padding. - Koolook_NextVersion node (Koolook/Publish): folder + name -> next version token. Wire once into both the EXR and video version inputs so branches stay in sync. IS_CHANGED returns NaN to re-scan every run. Tested: helper edge cases + node registration/run. Preflight clean (20 node IDs). Full non-sidebar suite green (539). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(nodes): 'auto' version keyword for EasyAIPipeline + Easy_VideoCombine No new node or wiring needed: type 'auto' (or 'next') in the version field and the node scans its own output folder for existing <name>_vNNN entries and writes the next free version -- fixing "Output file already exists and overwrite is disabled" without manual version bumps. - koolook_versioning.is_auto_version(): detects the auto/next keyword. - EasyAIPipeline (build_pipeline_outputs): on auto, scans the version-folder parent (base, or base/shot/method when subfolders are on) for vNNN dirs and picks the next; version computed after path segments so the scan dir is known. - Easy_VideoCombine: on auto, resolves the token after the output dir + name root are composed, scanning that dir for <name>_vNNN outputs. - Non-auto/typed/wired tokens and disable_versioning are unchanged. Tests: is_auto_version keywords + EasyAIPipeline auto (v003 after v001/v002, v001 first run). Full non-sidebar suite green (542), preflight clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(nodes): drop redundant Koolook_NextVersion node The 'auto' version keyword on EasyAIPipeline + Easy_VideoCombine covers auto-detection without any new node or wiring, so the standalone Koolook_NextVersion node is redundant. Remove it (and its node test) to keep the node list clean; the shared next_version_token helper stays (the 'auto' keyword uses it). Node-ID count back to 19; full non-sidebar suite green (541), preflight clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(tools): satisfy security gate for setup smoke check * fix(pr244): address verified review findings * fix(video): fail if relative auto-version scan root is unavailable --------- Co-authored-by: Cristian Predut <cristian.predut@x.baconcph.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: AI Machine <ai.machine@x.baconcph.com> |
||
|
malkuthroandClaude Opus 4.8
|
ae2480eee9 |
feat(setups): publish success feedback + published-setup discoverability (#227) (#239)
* feat(setups): publish success feedback + published-setup discoverability (#227) Post-publish feedback (issue #227 Part 1): - POST /koolook/api/setups now returns the on-disk storagePath - new POST /koolook/api/setups/reveal opens the published-setups folder (shared _open_dir_in_file_manager helper; the snapshot reveal route was refactored onto it, behaviour-identical) - the publish modal renders a success card reusing the snapshot Save dialog's "Saved to ... Open folder" language (Open folder / Copy path / Close) instead of closing silently; a draft publish still gets a confirmation Published discoverability (reuses the tag system, no new schema): - publishing auto-tags the source workflow `published` -> a "published" badge in the Workflows tree + the Tags-section "published" pool - a new "P" toggle in the sidebar Tools row prunes the Workflows tree to published-only, folders force-expanded, so setups show in their origin structure; toggling off restores the full tree - workflowHasTag() shared helper (unit-tested) Deferred, still open on #227: separate destination/library setting (Part 2), relocating the storage folder beside the snapshot library, and clearing the filter when entering Compare mode. Tests: +server (storagePath, reveal route happy + 404), +client (reveal client, buildPublishSuccessView view-model), +sidebar (workflowHasTag). Full suite: 610 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(setups): store published setups beside the snapshot library (#227) - the published-setup registry path now derives from the configured snapshot library (`_configured_dir().parent / koolook-published-setups/setups.json`) instead of a fixed user-dir folder, so it follows libraryPath / KFORGELABS_PRESETS. The default config resolves to the same path as before (target == legacy), so existing default installs are unchanged. - non-destructive one-time migration: copy a legacy user-dir registry to the new location when the new one is absent, leaving the original in place. - sidebar: clear the "P" published filter when entering Compare mode so it doesn't render both compare columns filtered with no toggle to undo it. Tests: +tests/server/test_published_setups_relocation.py (path beside library; migration copy / no-op-when-present / no-op-when-no-legacy). Full suite: 614 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(tests): remove unused pathlib.Path import (ruff F401) The relocation test only uses the `tmp_path` fixture, not `Path` directly, so `from pathlib import Path` was unused and failed the Lint & Smoke (ruff) gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(setups): address PR #239 review — migration masking + tag persist - HIGH: a failed legacy->new migration no longer masks real setups with the bundled sample. The default registry keeps the legacy file as its primary storage when migration could not produce the new file, so stranded data stays visible. (+2 tests: keeps-legacy-on-copy-failure / uses-new-on-success) - MEDIUM/LOW: the publish flow now awaits the `published`-tag persist so the success card reflects a committed tag (persist failures already surface via persistMutation's critical toast), and warns when the source workflow could not be tagged (vanished mid-publish) rather than on a harmless republish. Full suite: 616 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(setups): harden migration against partial/corrupt relocation target Addresses the remaining HIGH from the PR #239 consolidated re-review: a zero-byte/partial/corrupt setups.json at the relocated path could shadow the intact legacy registry (existence was treated as proof of migration). - migration now copies via temp file + atomic os.replace (the same pattern FileSetupStorage.save_setups uses), so an interrupted copy can never leave a partial setups.json at the target; failed-copy temp files are cleaned up. - primary selection is corruption-aware: when the relocated file exists but does not parse as a setups document while the legacy file is intact, the registry keeps serving the legacy file and logs a loud diagnostic. The unreadable file is left untouched (never overwritten) as evidence. A valid relocated file stays authoritative; if both are corrupt, the relocated path stays primary so storage diagnostics point at the current location. Tests (+4): corrupt-target-prefers-intact-legacy (file left untouched), valid-target-stays-authoritative, both-corrupt-keeps-target, and interrupted-copy-leaves-no-partial-target-and-no-tmp-litter. Full suite: 620 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
malkuthroandClaude Fable 5
|
5da590120c |
fix(tests): canonicalize expected writer filepath on Windows (#238)
* fix(tests): canonicalize expected writer filepath on Windows build_pipeline_outputs canonicalizes output paths to forward slashes on every platform, but the execution-map writer test built its expectation from str(tmp_path), which yields backslashes on Windows — failing there while ubuntu CI stayed green. Normalize the expected prefix the same way test_easy_ai_pipeline.py already does. No-op on POSIX. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: run pytest job on a windows-latest matrix lane The suite is now Windows-clean (previous commit), but only ubuntu ran in CI - the path-separator regression this PR fixes was invisible until a local Windows run. Add windows-latest to the pytest job so that class of break gates PRs. fail-fast off so a Windows-only failure still reports alongside the green ubuntu lane. Safe rename: Protect-main requires only "Lint & Smoke" and "Security Gate", so Pytest -> "Pytest (os)" breaks no required check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
malkuthro
|
1732030175 |
fix(setups): resolve switch-selected run results (#235)
* fix(setups): resolve switch-selected run results * fix(setups): harden switch result reporting * fix(setups): reject stale execution map writers |
||
|
malkuthro
|
85206ab321 |
feat(setups): add external runner simulator (#231)
* feat(setups): add external runner simulator * feat(setups): render app-surface simulator form * docs(setups): capture publish-to-run frontend plan * docs(setups): preserve sidebar inventory breadcrumbs * fix(setups): address runner simulator review findings * fix(setups): validate simulator run payload shape |
||
|
malkuthro
|
16c1b42760 |
Add published setup execution API (#230)
* feat(setups): add published setup execution API * fix(setups): expose app surface run results * test(setups): harden execution API failures |
||
|
malkuthro
|
83b170bdae |
feat(setups): add publish contract nodes (#228)
* feat(setups): infer app contract from named fields * feat(setups): add publish contract nodes * feat(setups): split publish result contract * fix(setups): resolve publish output reroutes in preview * fix(setups): validate app surface on read * fix(setups): harden app contract validation |
||
|
malkuthro
|
7cffe0702e |
fix(setups): publish simple widget-only workflows (#223)
* fix(setups): publish simple widget-only workflows * fix(setups): read subgraph and named widget defaults * fix(setups): allow optional publish descriptions * fix(setups): reject incomplete widget-only values |
||
|
malkuthro
|
3b35e6a195 |
feat(setups): infer setup surface from workflow groups (#222)
* feat(setups): infer setup surface from workflow groups * fix(setups): validate inferred setup surfaces on read * fix(setups): reject boolean setup geometry |
||
|
malkuthro
|
98c204b119 |
feat(setups): convert visual workflows to API prompts (#217)
* feat(setups): convert visual workflows to api prompts * fix(setups): reject corrupt visual graph links |
||
|
malkuthro
|
16023662e6 | feat(setups): publish sidebar workflows (#216) | ||
|
malkuthro
|
1cf2fd31a2 |
feat(setups): add published setup catalog API (#214)
* feat(setups): add published setup catalog API * fix(setups): report registry storage diagnostics |
||
|
malkuthroandClaude Opus 4.7
|
8e489ad37a |
feat(snapshot): redesigned Save/Load dialogs + folder picker (#137) (#144)
* feat(snapshot): row-augment max(periodic, pre_load_*) + pytest infra (#137) Server-side preparation for the snapshot dialogs redesign. The Load dialog's "newer auto-save available" row flag now reflects the freshest recovery file regardless of which mechanism wrote it — before this we only stat'd ``periodic.json``, missing cases where the user's most recent Load wrote a fresher ``pre_load_*.json`` snapshot inside the same autosave subfolder. Extracts ``_latest_autosave_mtime(autosave_dir, named_mtime)`` from the inline block in ``list_presets`` so the scan is unit-testable. The helper walks ``periodic.json`` and every ``pre_load_*.json`` once, returns the max mtime if strictly newer than the named file, else ``None``. Same shape as before, broader source set. Introduces pytest as a first-class test runner: * ``tests/server/test_listing_autosave_mtime.py`` — 5 row-augment test cases (periodic-only, pre_load-only, max-of-both, no-newer, missing-dir). * ``[tool.pytest.ini_options]`` in ``pyproject.toml`` scopes discovery to ``tests/`` and uses ``importmode=importlib``. * ``[project.optional-dependencies] test = [...]`` defines the test extra (pytest + aiohttp, which is a runtime dep of ``koolook_routes`` that must be importable even when only a pure helper is under test). * ``.github/workflows/ci.yml`` gains a ``pytest`` job that installs via ``pip install .[test]`` and runs ``pytest -q``. Guards ``__init__.py``'s relative imports in a ``try / except ImportError / else`` block. Pytest's package collector walks up from test files looking for ``__init__.py`` and tries to import the one at the repo root — but the relative imports inside it only resolve under ComfyUI's custom-node loader, not under a standalone Python import. Without the guard, every pytest run aborts during collection with ``attempted relative import with no known parent package``. The fallback emits a one-line diagnostic and registers empty mappings so the package directory can still be imported by tooling. ComfyUI's loader still hits the ``else`` branch and behaviour is unchanged for real users. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(snapshot): folder picker + agent-driven visual harness (#137) Commit 2 of the snapshot dialogs redesign. Introduces the navigate- into folder picker from mockup section 6 — the picker that opens when the user clicks Save to… / Load from… in the redesigned dialogs (and that the Settings cog will be removed in favour of, once commit 6 lands). Path input is the source of truth: type/paste + Enter navigates, clicking a folder row drills in, ↑ Up climbs one level. Files appear greyed below the directories as a "yes, this is the folder I expected" affordance. End-visible path overflow: the path input uses ``direction: rtl`` + ``unicode-bidi: plaintext`` so when the path exceeds the input width the leading ``/Users/…`` clips on the left, never the trailing folder name. Per mockup section 6. Visual harness — the path the issue called out as "visual verification is a hard gate": * ``.claude/launch.json`` defines ``design-server`` (``python3 -m http.server 8765``). Agent / maintainer can spin this up via ``preview_start`` or directly from a terminal. * ``docs/designs/_harness/folder-picker.html`` mounts the real ``showFolderPicker`` from ``web/sidebar/modals.js`` against a canned filesystem, with one trigger button per mockup-illustrated state (default / new-folder-input / empty / error). Cache-busts module imports so editing modals.js while the harness is open picks up fresh code on reload. * ``docs/maintainers/visual-harness.md`` documents the workflow, when to use the harness vs. dev-sync, and the backtick-in-CSS- comments pitfall that broke this commit twice during development. * ``CLAUDE.md`` now lists the harness as the preferred agent-driven path; dev-sync stays the integration-level gate. Server: ``/koolook/presets/browse`` gains an opt-in ``?files=1`` parameter that returns child JSON files alongside the directories. Default response shape unchanged so the legacy Settings dialog keeps working. ``_list_child_files`` mirrors ``_list_child_dirs``'s swallow-OSError-per-entry policy so a broken symlink mid-listing doesn't sink the whole response. Tests: six new pytest cases in ``tests/server/test_browse_list_children.py`` covering directory sorting, autosave-subfolder hiding, JSON-only filter, case-insensitive extension match, and the empty-result paths. ``pytest`` runs all 11 tests cleanly under the existing CI job from commit 1. What's still verified via dev-sync (not via the harness): * Real Comfy theme tokens (the harness fakes ``--comfy-menu-bg`` etc. with dark-theme defaults). * End-to-end Save → Load round-trip across the new library path. * Picker interaction with ComfyUI's keyboard manager. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(snapshot): modal headers stay title-only; route gloss to tooltip (#137) Follow-up to the folder picker commit. Maintainer flagged that the "Where snapshots are saved and loaded from." subtitle below the title was extra information the user doesn't need at first glance — if the title isn't self-explanatory, fix the title, don't pad it with a sentence. The mockup at docs/designs/snapshot-dialogs.html section 6 shows a title-only header for exactly this reason. Locks the rule in as a project convention so every dialog box behaves the same way going forward: * docs/maintainers/conventions.md — new "Modal headers" section spells out the rule, what counts as a "descriptive subtitle" (remove) vs. a "functional info row" (keep), and how to plumb explanatory copy via a hover tooltip on the title instead. * web/sidebar/modals.js — makeModalShell now accepts ``titleTooltip``, applied as the ``title`` attribute on the modal title element so the gloss is mouseover-discoverable but never visible by default. showFolderPicker drops its ``subtitle`` param (it was added one commit ago, no other callers) in favour of ``titleTooltip``. * docs/designs/_harness/folder-picker.html — harness re-points its explanatory text to ``titleTooltip``. Verified in the harness: the visible header is title-only, the tooltip surfaces on hover. Applies prospectively to the Save / Load dialogs (commits 3 and 4) — they will also be title-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(snapshot): redesigned Save dialog with inline library row + Save to... + 4-state button (#137) Commit 3 of the snapshot dialogs redesign. The Save dialog is now a single modal (was three: three-way overwrite / input prompt / confirm) that matches mockup section 2: * Inline library-path info row at the top of the body — "Saved to" label on the left, "Open folder ↗" link on the right, full path below. Pure information; every action lives in the bottom command bar. * Bottom command bar with split layout: Save to... on the left, Cancel + Save as new... + Save on the right (spacer in between). * Save to... opens the folder picker from commit 2. On Use this folder, the dialog calls saveSettings() to persist the new library path, refreshes its in-place library row, and flips the primary button into Dirty state. * The primary Save button cycles through the four states from mockup section 5: - Default ("Save") — clean state, primary blue. - Dirty ("Save to new folder") — folder differs from the one the dialog opened with; the label spells out the consequence. - In progress ("Saving...") — disabled, dimmed primary. - Done ("Saved") — disabled, subtle green, brief pause before the dialog closes. * Save as new... still routes through the existing showInputModal plus presetExists guard, but the prompt no longer carries a library-path subtitle — the parent dialog already shows the target. Convention from the previous commit applied. tree.js now passes the new capability functions (saveSettings, browseDirectories, createBrowseDirectory, revealPresetFolder) into showSaveSnapshotDialog so the dialog can invoke the folder picker end-to-end inside the Save flow. Visual harness: docs/designs/_harness/save-dialog.html exercises all five mockup states (default tracked, untracked Save-as-new, dirty, in-progress, done) so the maintainer (or future agent) can re-verify without dev-syncing. Verified all states render correctly in the preview server. Static guard against the recurring backtick-in-CSS-comments trap: tests/sidebar/test_css_template_literal.py asserts that the CSS template literal in web/sidebar/constants.js contains exactly two backticks (open + close) and nothing in between. The trap bit this work three times (commits 2 and 3) — a literal backtick inside a comment closes the template early and silently bricks the entire sidebar. The test runs under the same pytest job introduced in commit 1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(snapshot): redesigned Load dialog with scoped recovery + inline delete (#137) Commit 4 of the snapshot dialogs redesign. The Load dialog is now a single modal matching mockup section 3: * Inline library row at the top, same shape as the Save dialog — "Loaded from" label + Open folder ↗ link + leaf folder name + full path. No 📂 emoji button (the link supersedes it). * Preset rows: click to load. The "Replace current state?" confirm modal is gone — clicking Load is the intent, the dialog title already says Load snapshot, no double-confirm. * Scoped recovery: when a preset has a newer autosave (per the server-side row-augment from commit 1), the first click DOES NOT load. It expands exactly ONE recovery row directly beneath the parent, showing the freshest entry (newest of periodic.json or pre_load_*.json) with its kind badge + meta line. The user then chooses: re-click the named row to load the named version, OR click the recovery row to restore the auto-save. Both options are surfaced inline; the user picks by clicking the one they want. * Inline delete confirm: × outlines the target row red, the bottom Close button transforms to 'Yes — delete "<name>"' (danger styling). A second click commits; Escape cancels and reverts. No second modal. * Bottom command bar matches mockup section 3 split layout: [Load from…] on the left, [Close] on the right. Load from… opens the folder picker from commit 2; on Use this folder it persists via saveSettings() and refreshes the listing. The pre-#137 YES/NO choice modal (``promptApplyChoice``) and the collapsible Recovery auto-saves <details> section are both removed — the scoped-recovery inline row + the click flow above subsume their job. (Commit 5 in the issue plan was specifically for dropping the YES/NO modal, which happens here as a natural consequence of the routing change; commit 5 will be a small follow-up if needed.) Behavior preserved from the pre-#137 implementation: * doNamedLoad — pre-load auto-save backup chain, partial-failure tracker handling, identical toast messages. * doAutosaveRestore — same backup chain, same markStateAutosaved() semantics, same restored-name derivation from subdir. Tree.js plumbs the three new caps (saveSettings, browseDirectories, createBrowseDirectory) into showLoadSnapshotDialog so the dialog can invoke the folder picker end-to-end. Visual harness: docs/designs/_harness/load-dialog.html exercises default, empty, scoped-recovery, and pending-delete states. Verified all four render correctly in the preview server. Net diff: -56 lines in the sidebar source (the redesign deletes more than it adds because the YES/NO modal + collapsible recovery disclosure + confirm modal for plain loads are all gone). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(designs): reconcile snapshot redesign sketch with shipped behaviour (#137) Commit 5 of the snapshot dialogs redesign. The code-level removal of the autosave-newer YES/NO modal happened in commit 4 as a natural consequence of the scoped-recovery inline row taking over its job; this commit reconciles the design docs so they describe what was actually built. docs/designs/snapshot-dialogs-redesign.md: * "What changes" item 4 reworded — the YES/NO modal is dropped; the affordance is now an inline scoped recovery row that opens directly beneath the clicked preset. * "What changes" gains items 2, 3, 5 covering the Save/Load dialog redesigns and the inline delete confirm that landed in commits 3 and 4. * Mermaid flow rewritten to reflect the new routing (preset click → scoped recovery expand → user picks named or recovery row), matching commit 4's onPresetClick implementation. docs/designs/snapshot-dialogs.html "Locked-in decisions": * Updated the autosave-modal item from "modal head reads X" to "modal — dropped" and explains the inline scoped recovery row replacement. * Added the title-only modal header convention (commit 2 follow-up) pointing to docs/maintainers/conventions.md. * Added a pointer to the visual harness workflow added in commit 2. No code changes — the YES/NO modal was already gone after commit 4. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(snapshot): remove Settings cog + showSnapshotSettingsDialog + CHANGELOG (#137) Final commit of the snapshot dialogs redesign. The Settings cog disappears from the Snapshot toolbar — the snapshot row is now status indicator | Load | Quick Save | Save, matching mockup section 1. Library-path management now lives inside the Save dialog ("Save to…") and Load dialog ("Load from…"), both of which open the folder picker introduced in commit 2 and persist via the existing `saveSettings` route. Removals: * `showSnapshotSettingsDialog` (~325 lines) — the dialog body, its inline directory browser, and the "save folder location" state machine. The same browse functionality lives in `showFolderPicker` now; the same persistence happens via `saveSettings()` called from inside Save / Load. * `showThreeWayDialog` (~45 lines) — only consumer was the pre-#137 Save flow (overwrite / Save as new / Cancel), which the redesigned Save dialog absorbed into its primary action bar. * `renderLibraryLocation` (~17 lines) — replaced by inline rendering in the Save and Load dialogs' library rows. * Settings cog toolbar button + `openSettingsDialog` wiring in tree.js (~12 lines). * Orphan `.koolook-browse-*`, `.koolook-path-input-row`, `.koolook-load-library-row*`, `.koolook-settings-save-note` CSS rules in constants.js (~16 lines) — only consumed by the removed dialog. `.koolook-settings-folder-name` and `.koolook-settings- folder-path` stay; the new Save / Load library rows reuse them verbatim (git-blame continuity for the visual style). Adds: * CHANGELOG.md [Unreleased] entry summarising the whole #137 feature — picker, redesigned Save/Load dialogs, scoped recovery, inline delete, removed Settings cog, pytest infrastructure. Net diff for this commit: -377 lines. Cumulative PR diff (commits 1–6 + harness/convention follow-ups): all six issue ACs satisfied. The maintainer should dev-sync once before merging to confirm real-Comfy theme tokens render correctly (harness fakes the dark-theme defaults) and end-to-end Save → Load round-trips against the live preset folder behave. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): install pytest + aiohttp directly, not via pip install . (#137) The Pytest job introduced in commit 1 of this PR was failing on CI because ``pip install .[test]`` tries to build a wheel of the entire project. setuptools's flat-layout discovery then trips on the non-Python ``web/`` and ``forks/`` directories at the repo root and errors out with "Multiple top-level packages discovered in a flat- layout: ['web', 'forks']". The project is a ComfyUI custom node loaded directly from its source directory, not a pip-installable package. There's no wheel to build. Install the test deps explicitly instead — the ``[project.optional- dependencies] test`` block in pyproject.toml is the spec, CI mirrors it by hand. Worked locally because the venv install path went straight to ``pip install pytest aiohttp`` and never touched the project. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(snapshot): untracked dirty state + Load dialog Escape listener leak (#137) Two findings from the external review on PR #144: **Blocker — Save dialog's primary action with no tracked preset + folder changed via Save to...** ``updatePrimaryLabel`` was entering the "dirty" state whenever the library path diverged from the original, regardless of whether a tracked preset existed. The primary click handler then routed "dirty" to ``doOverwrite(current)`` — and with ``current === null``, ``sanitizeName(null)`` would throw / silently coerce to an empty string instead of asking the user for a name. Fix: ``isDirty()`` only matters when there is something to overwrite. With no tracked preset, the primary stays in ``default-new`` regardless of folder divergence. The Save to... button still updates the library row, so the user has feedback the destination changed; the eventual primary click then routes to the Save-as-new name prompt and writes into the new folder cleanly. Verified in the visual harness: opening Save as untracked, switching the library to ``/Volumes/Shared/different-library`` via the picker, then inspecting the primary button: * Label stays "Save" (was "Save to new folder" pre-fix) * Tooltip "Choose a name and write the snapshot." (was "Write into /Volumes/Shared/different-library.") * Library row still reflects the new path (saveSettings DID run) **Major — Load dialog leaked a document keydown listener per session** The inline-delete Escape-to-cancel handler was registered with ``document.addEventListener`` but never removed. Each open/close cycle of the Load dialog accumulated another stale handler; long sessions ended up firing the cancelDelete check on every Escape press across every dialog (no-op because ``pendingDelete`` was null in closed dialogs, but the listener was still alive and called). Fix: capture the handler by name, attach on open, tear down on close by wrapping ``overlay.remove`` (mirrors the pattern the modal shell already uses at ``modals.js:52-66`` for its own Escape handler). Verified by instrumenting ``document.{add,remove}EventListener`` and opening + closing the Load dialog five times in a row: every keydown listener added during the cycle was removed before the next cycle opened. No accumulation. CI gates: ruff, compileall, bandit (via preflight equivalent), pytest 12/12, preflight 4/4 — all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |