ff79b0f418229fafaae32dbaf63355941e034bd0
26
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 malkuthroandClaude Opus 4.7
|
ff79b0f418 |
feat(video): add Easy_VideoCombine (#154)
* feat(video): add Easy_VideoCombine with absolute-path support Adds a Koolook subclass of VHS VideoCombine that bypasses ComfyUI's output-directory sandbox when filename_prefix is an absolute path — the VFX-pipeline missing piece for writing renders into a project tree (e.g. E:/projects/shot01/v003) instead of ComfyUI/output/. Implementation: - k_video_combine.py: ~60 LoC subclass + a pure _resolve_abs_target helper. Scoped-patches folder_paths.get_save_image_path for the duration of one combine_video() call so upstream's encoder, audio mux, batch manager, and progress bar run unchanged. No VHS code copied. Self-skips registration when VHS isn't importable (clean one-line stderr notice). - Discriminator: os.path.isabs(filename_prefix). No new input pin; pattern borrowed from spacepxl/ComfyUI-HQ-Image-Save's SaveEXR. - Optional create_path_if_missing BOOLEAN (default False) so typos surface as FileNotFoundError instead of spawning stray dirs. License audit (2026-05-16): - VHS upstream: GPL-3.0 (verified via raw LICENSE fetch). - HQ-Image-Save pattern source: MIT, copyright 2023 spacepxl. - Koolook: already GPL-3.0; both upstreams are compatible. - No source code from either upstream is incorporated. Verification: 18 pytest cases green (12 existing + 6 new covering relative passthrough, absolute resolution, create-on/off branches, trailing-separator normalization, and empty-basename rejection). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(starter-preset): pre-favourite Easy_VideoCombine Adds the new node to the shipped starter_preset.json picks so fresh installs surface it in the Kforge Labs sidebar's Nodes section without the user having to discover it via Comfy's stock search first. Pairs naturally with the existing LoadEXR / SaveEXRFrames picks from HQ-Image-Save. Existing installs with non-empty picks are unaffected (the seeder skips them by design); they can either Load -> "starter" in the Snapshot dialog or hit "+" once in the Nodes section to pull it in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dev-sync): include k_video_combine.py in RUNTIME_PATHS Without this entry the new Easy_VideoCombine module is never copied into the dev ComfyUI install, so the node fails to register on restart with a silent ImportError. Caught when the maintainer restarted ComfyUI and couldn't find the node in the search palette. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(starter-preset): include all Koolook nodes in default picks Fresh installs should land on a populated Nodes (favorites) panel that immediately surfaces what Koolook provides — otherwise new users see only third-party picks and have no entry point into the Koolook-native workflow. Adds the 8 currently-registered Koolook node IDs to the shipped starter preset alongside the existing third-party essentials: EasyAIPipeline, EasyResize_Koolook, EasyWan22Prompt, Easy_Pattern, Easy_VideoCombine, Easy_hdr_VAE_decode, Easy_hdr_VAE_encode, KoolookLoadCameraPosesAbsolute, easy_ImageBatch Also adds Easy_VideoCombine to the README's Included Nodes table. Existing installs with non-empty picks are unaffected by the seeder (by design). To pull the Koolook defaults into an existing install, Quick Save first, then Snapshot -> Load -> "starter". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(video): reach VHS_VideoCombine via nodes.NODE_CLASS_MAPPINGS The previous `from videohelpersuite.nodes import VideoCombine` fails inside ComfyUI's loader context because VHS uses relative imports throughout (`from .logger ...`, `from .utils ...`) that only resolve when VHS is loaded through ComfyUI's importlib spec — not through a plain absolute import path. The error surfaced as No module named 'utils.install_util'; 'utils' is not a package at Koolook load time, which caused the node to self-skip with an empty NODE_CLASS_MAPPINGS — invisible in ComfyUI's search palette. Fix: look up VHS_VideoCombine through `nodes.NODE_CLASS_MAPPINGS`, which is the documented cross-pack interface. ComfyUI's loader (custom_nodes/.../__init__.py exec_module → merge into global) runs to completion for each pack before moving to the next, and VHS sorts before Koolook alphabetically in every layout we've seen, so by the time Koolook's __init__.py runs the global registry already has VHS_VideoCombine in it. Verified against ComfyUI's nodes.py load_custom_node() at load_custom_node() lines 2196 (exec_module) and 2225-2228 (merge), which guarantee this ordering. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(video): add output_directory input for split path/name editing Overloading filename_prefix for absolute paths means losing the "just type a name" affordance — once you put the path there, there's nowhere to specify just the file root. Across many renders into the same project tree this becomes tedious: every iteration retypes the whole path. Adds an optional `output_directory` STRING input. When set: - filename_prefix is treated as just the filename root; any path components are stripped (so accidental typos like "sub/clip" don't double-up under the directory). - Absolute output_directory writes directly to that location. - Relative output_directory joins under ComfyUI's output/. When output_directory is empty, the original overloaded filename_prefix behavior is preserved unchanged (back-compat for the one-field flow). Implementation: - New pure helper `_compose_prefix(filename_prefix, output_directory)` combines the two fields into one effective prefix. The existing isabs discrimination then handles sandboxed vs absolute writes uniformly downstream. - 6 new tests cover all combinations: empty/abs/relative directory, path stripping from filename_prefix, trailing-separator trim, empty-name fallback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(video): coerce frontend 'undefined' STRING values to empty The ComfyUI frontend serializes a STRING widget with empty default as the literal string "undefined" when left untouched (a known long-running quirk). Without defensive normalization, an untouched output_directory was treated as a real directory name and prepended to the user's path: filename_prefix = "E:/renders/clip_v003" (typed by user) output_directory = "undefined" (frontend default, not typed) _compose_prefix(...) -> "undefined/clip_v003" (relative) _resolve_abs_target -> None (not absolute) super().combine_video -> writes to <output>/undefined/clip_v003_* The QuickTime ended up at <ComfyUI>/output/undefined/<name>.mov with a spurious `undefined/` subfolder, instead of at the absolute path the user actually specified. Fix: a `_normalize_text_input()` helper coerces the literal strings "undefined" / "null" / "None" (case-insensitive, post-strip) to "". Applied to both filename_prefix and output_directory at the top of combine_video() before any path composition. 4 new tests cover the sentinels and a regression test for the exact path-composition behavior that produced the bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(video-fe): wire format-specific widgets for Easy_VideoCombine VHS's frontend extension (web/js/VHS.core.js:2030) gates its format-widget machinery on `nodeData?.name == "VHS_VideoCombine"` — strict-equality on the registered node ID. Our subclass uses a different ID (`Easy_VideoCombine`), so none of VHS's per-format widget rendering ran. The user saw: [VideoHelperSuite] - WARNING - Missing input for p has been set to hq on every execution (profile defaulting to "hq" for ProRes), and had no way to pick anything other than the default — because the profile dropdown was never rendered. Fix: a minimal frontend extension at web/easy_video_combine.js (~80 LoC) that replicates `addFormatWidgets` for our node name. Hooks the format dropdown's callback to add/remove per-format widgets the same way upstream does. Self-contained — does not import from VHS's module (their helpers aren't exported as ES imports). Verifies via the actual ComfyUI install after dev-sync; static preview servers can't exercise it (depends on LiteGraph's registered_node_types at runtime). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(video): strip 'undefined' from anywhere + one-shot debug log The previous normalization only caught the EXACT literal "undefined" / "null" / "None" at field-level. Adds two stronger defenses for the case where a sentinel still slips through: 1. _normalize_text_input now also strips surrounding quotes (paste- from-Explorer artifact) and handles embedded newlines by taking the first non-empty non-sentinel line (defends against multi-line Text Multiline inputs with trailing blank or junk lines). 2. New _strip_sentinel_components helper drops any path component that is exactly a sentinel string from the composed effective_ prefix — belt-and-suspenders for any path that somehow carries `\undefined\` as a literal segment by the time it reaches us. Also adds a one-shot print() of fp_raw, od_raw, normalized values, and effective_prefix at the top of combine_video. The user is seeing an undefined/ folder under their custom path despite the field-level normalization, and pure code tracing doesn't reproduce it — the log will surface what kwargs actually contains at run time so we can fix the real cause if defensive stripping doesn't already handle it. To be removed in a follow-up once root cause is known. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(video): trim node docstring + tooltips, remove debug print - Class docstring collapsed from ~20 lines to a single sentence (the frontend was showing it as the node description on right-click, cluttering the UI). - Widget tooltips trimmed to one short sentence each — same intent, no walls of text. - Debug print() added in |
||
|
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> |
||
|
malkuthroandClaude Opus 4.7
|
9dab85c695 |
feat(easy_pattern): add test-pattern node + auto-restart dev-sync (#135)
* feat(easy_pattern): add test-pattern node + auto-restart dev-sync Adds an `Easy_Pattern` node (Koolook/Testing) for solid-color frame batches with optional per-frame index overlays — verifies sequence merging, frame insertion, batch indexing, and video-pipeline frame order at a glance. Color inputs use the same `White/Black/Gray/Custom` dropdown idiom `EasyResize_Koolook` already exposes for pad/panel colors, so "Gray" means the same 50% sRGB across nodes. `scripts/sync_to_dev.py` now restarts the live ComfyUI server after a successful sync (via ComfyUI-Manager's `/manager/reboot`). Default-on because dev-sync is only ever invoked to see new code, and Python custom-node files load once at server start — `--no-restart` is the escape hatch. The `RUNTIME_PATHS` manifest also picks up `k_easy_pattern.py` so adding a new top-level node doesn't silently break the pack at import time on the next sync. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sync-dev): restrict restart URL to http(s) for Bandit B310 `urllib.request.urlopen` accepts arbitrary schemes including `file://`, which Bandit B310 flags as a path-traversal vector. The script's typical caller is the maintainer's own shell, but a typo in `--restart-url` could surprise — and the static gate is right to flag it. Validate the scheme is http/https before the urlopen call and annotate it so Bandit accepts the now-truthful narrowing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
malkuthroandClaude Opus 4.7
|
93363cc625 |
feat(sidebar): snapshot library v2 — configurable filesystem path via KFORGELABS_PRESETS (#68)
* feat(sidebar): snapshot library — named presets for picks + workflows Implements #46 item 1 (custom save location + cross-machine portability) as a preset library: each preset is one named JSON file under /userdata/koolook-presets/, capturing the full Kforge Labs state. Use cases addressed ------------------- - **Personal portability:** copy `koolook-presets/` between machines, or sync ComfyUI's userdata folder via Dropbox/iCloud/Drive — your kit follows. - **Team / facility shared library:** multiple workstations on the same ComfyUI server share `/userdata/koolook-presets/` natively. One person publishes "facility kit", everyone else loads it. - **One-off transfer (different servers):** Manage tab's Download button saves a snapshot to local disk; Upload imports it into another machine's library. UI -- New top-level **Snapshot** action row above the search field, mirroring the Nodes/Workflows action-row pattern. Three icon buttons (Load / Save / Manage), each opens the same modal at a different starting tab. - Load tab: lists every preset with metadata (workflow count · pick count · export date). Click a row → "Replace current state?" confirm → restores both picks and workflows. - Save tab: input field defaults to `preset YYYY-MM-DD`, fully editable. Existing-name save prompts to overwrite. - Manage tab: per-row Download / Delete buttons + a top-of-tab Upload action (file picker imports a JSON snapshot from disk into the library). Schema ------ ```json { "kind": "koolook-snapshot", "version": 1, "name": "Wan video kit", "exportedAt": "2026-05-05T08:42:13.000Z", "picks": [...], "workflows": { "directories": {...} } } ``` `kind` + `version` for future schema migrations. Anything without `kind === "koolook-snapshot"` is rejected on import. Module shape ------------ - `web/sidebar/snapshot.js` (new) — server I/O on the preset directory, schema validation, gather/apply, download/upload helpers. UI-free. - `web/sidebar/modals.js` — new `showSnapshotModal({initialTab, ...})` that takes capability functions instead of importing snapshot.js directly, so the UI module stays UI-only. - `web/sidebar/workflows_store.js` — new `replaceAllWorkflows(rawStore)` for bulk-replace (bypasses persistMutation since the entire cache is being swapped) + `getAllWorkflowsForExport()` for the gather flow. - `web/sidebar/picks_store.js` — new `setAllPicks(picks)` exported helper. - `web/sidebar/tree.js` — new top action row + 3 toolbar buttons + one wiring helper. - `web/sidebar/constants.js` — CSS for tab strip, list rows, manage buttons. Deliberate scope cuts (per the design discussion) ------------------------------------------------- - No continuous sync. Each save is a deliberate act. - No conflict resolution. Same preset name written from two machines on the same server = last-write-wins. Documented as a known limit; revisit when it bites. - No selective import (preset = full state). Merge can be a v2. - No custom userdata path config. The `koolook-presets/` location is fixed; portability comes from the snapshot files themselves, not from rerouting storage. Verification ------------ - `node --input-type=module --check` on all modified + new JS files — clean. - `python tools/preflight_release.py --skip manager-meta -v` PASS. - Synced to live install on Mac for browser smoke-test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(sidebar): snapshot library now uses configurable filesystem path Reworks PR #67 (which served snapshots from /userdata/koolook-presets/) to expose the library through Koolook's own server-side endpoints, making the storage location configurable via an env var. The userdata namespace is per-install — fine for personal use, useless for the "facility shared library" use case the user explicitly called out. Architectural change -------------------- - The client no longer talks to ComfyUI's `/userdata/*` API for snapshot files. New JS-side endpoints: `/koolook/presets/{info,list,file}`. - Server side: new `koolook_routes.py` registers aiohttp routes on ComfyUI's PromptServer at custom-node load time. Path-traversal protection at the route boundary via a strict filename whitelist regex (`^[A-Za-z0-9 _.()\-]+\.json$`). - Storage location is read from the `KFORGELABS_PRESETS` env var on every request (so re-launching ComfyUI with a new value picks it up without any cache fuss). Default fallback: `<comfyui-userdata>/koolook-presets/`. - Auto-creates the configured directory on first save (parent must already exist — for NFS/SMB mounts the mount must be present). Use cases this enables vs. PR #67 --------------------------------- | Use case | PR #67 | This PR | |-------------------------|--------|---------| | Personal, single-machine | ✓ | ✓ | | Personal cross-machine | symlink-or-sync userdata | point env var at Dropbox folder | | NFS/SMB facility share | ✗ (locked to userdata) | ✓ point env var at mount | | Multiple parallel libs | ✗ | per-instance env var | | Read-only distribution | ✗ | ✓ (server detects, save fails cleanly) | UI difference vs. PR #67 ------------------------ - The Manage tab gains a small `i` icon at the right edge of the upload row. Hover tooltip shows the current library path, source (default vs `KFORGELABS_PRESETS` override), and writability status. No header line, no extra UI weight. Files added/changed ------------------- - `koolook_routes.py` (new, ~180 lines) — aiohttp routes + env-var config + filename validation. Loaded by `__init__.py` at startup; failure is non-fatal (the rest of Koolook still works, just the snapshot feature is unavailable). - `__init__.py` — registers `koolook_routes` import-time side-effect. - `scripts/sync_to_dev.py` — `koolook_routes.py` added to `RUNTIME_PATHS` so dev-sync includes it. - `web/sidebar/snapshot.js` — server I/O section rewritten to call `/koolook/presets/*` instead of `/userdata/koolook-presets/*`. Drops the URL-encoding workaround (we own the route now, so the `name=<file>` query param is clean and the server validates). - `web/sidebar/modals.js` — `showSnapshotModal` accepts a `getLibraryInfo` capability function; the Manage tab calls it and populates the info-icon tooltip. - `web/sidebar/tree.js` — passes `getLibraryInfo` from `snapshot.js` to the modal. - `.env.example` — new `KFORGELABS_PRESETS` block with macOS / Linux / Windows examples for personal-sync, NFS facility share, and SMB facility share patterns. - `CHANGELOG.md` — Added entry rewritten for the env-var design; Internal entry on the new server-side module. Why a separate v2 PR (instead of patching #67) ---------------------------------------------- The userdata-namespace design in #67 was the wrong architecture for what the user actually wanted. Patching it to the env-var design would have meant a near-total rewrite of #67's server I/O paths mid-review. Cleaner: branch fresh from main, cherry-pick #67's UI + stores work as a base, layer the new endpoint design on top. PR #67 will be closed as superseded once this lands. Verification ------------ - `node --input-type=module --check` on every modified JS file — clean. - `python -c "import ast; ast.parse(...)"` on `koolook_routes.py` + `__init__.py` — clean. - `python tools/preflight_release.py --skip manager-meta -v` PASS. - Synced to live install on Mac. User to smoke-test against a real ComfyUI session (the previous URL-encoding 405 should be gone since the new route handlers own the URL space). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(sidebar): redesign snapshot UX — single-click Save, real Settings panel User feedback on #68's first cut: tabbed modal is too complicated, can't see existing presets in the Save flow, the env var alone isn't a real settings UI, the default-path text was stale (still claimed `/userdata/koolook-presets/` after the configurable-path landed). UX changes ---------- - **Save button → one click.** If a preset is currently loaded (tracked in localStorage as `koolook.snapshot.currentPresetName.v1`, survives reloads), prompts "Save over '<name>'?" with three options: Save (overwrite) / Save as new… (rename) / Cancel. If nothing is loaded yet, prompts for a name with `preset YYYY-MM-DD` default, fully editable. - **Load → single dialog.** Preset list with per-row × Delete button. Header line shows the current library path so the user can see where they're loading from. No tabs. - **Cog → real Settings panel.** Single field for the library path (text input, absolute path). Save / Reset to default / Close. Read- only line shows the currently-resolved path + source label (settings panel / env var / built-in default). The "Manage" tab and its Download / Upload sub-actions are gone — the user can already reach the file on disk after a save, no in-app file ops needed. Server side ----------- - New `GET /koolook/presets/settings` returns the saved library path + resolved path + source. - New `POST /koolook/presets/settings` writes the saved library path to `<comfyui-userdata>/koolook-settings.json`. Empty value clears the override. - Resolution chain in `_configured_dir()`: settings file > KFORGELABS_PRESETS env var > built-in default Reads each on every call; no module-load caching. - `info` endpoint gains a `source` field ("settings" | "env" | "default") the UI uses for the readout line. Client side ----------- - `showSnapshotModal` (tabbed) replaced by three exports: showSaveSnapshotDialog, showLoadSnapshotDialog, showSnapshotSettingsDialog. Each is self-contained, no shared state. - `snapshot.js` adds `getSettings`/`saveSettings` for the Settings dialog and `getCurrentPresetName`/`setCurrentPresetName` for the one-click Save tracking. Settings ride on the new server endpoints; current-preset tracking is browser-local. - `applySnapshot` now also updates the current-preset tracker (via the dialog wrapper, not the function itself) on successful load. - `tree.js` toolbar wires three direct callbacks instead of one `openSnapshotModal(initialTab)` helper. Stale text fix -------------- - The Save dialog's hint text in #68's first cut hardcoded "/userdata/koolook-presets/<name>.json" — stale from the userdata- era PR #67. The redesigned Save flow doesn't show that text at all (the path is no longer relevant to the user-typed name); the Load dialog's header line and the Settings dialog's read-only field are the canonical surfaces for "where my presets actually live." Deliberately out of scope ------------------------- - No per-save path picker. The library is one location at a time, set in advance via Settings or env var. Saving to ad-hoc locations would fragment the library and break the "facility shared library" use case. - No in-app Download / Upload (was in #68's first cut). Files are on disk at the configured path; the OS file manager handles transfer. Verification ------------ - `node --input-type=module --check` on every modified JS file — clean. - `python -c "import ast; ast.parse(...)"` on `koolook_routes.py` — clean. - `python tools/preflight_release.py --skip manager-meta -v` PASS. - Synced to live install on Mac. User to smoke-test against a real ComfyUI session. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sidebar): address PR #68 review — atomicity, security, error UX Multi-agent review surfaced 2 BLOCKING + 5 HIGH + 4 MEDIUM + 3 LOW. Validated the BLOCKINGs against the actual code (both real) and applied the bundle of fixes that hold up. Modal-size HIGH (#7 file size) deferred to a follow-up — that's a wider refactor. BLOCKING fixes -------------- - **`applySnapshot` partial-failure tracker gating** (modals.js Load dialog). `setCurrentPresetName(...)` was called unconditionally before checking `{picksOk, workflowsOk}` from the apply. Partial apply (one of picks/workflows persisted, the other didn't) left the tracker pointing at the preset, so the next Save would offer "Save over '<name>'?" and silently overwrite the on-disk preset with the corrupted half-state. Now: tracker is set only on `picksOk && workflowsOk`; partial failure clears it (and the toast tells the user "Reload to recover prior state. Tracker cleared."). - **`replaceAllWorkflows` non-atomic rebind** (workflows_store.js). Rebinds `workflowsCache` before awaiting persist, with no rollback. Now: snapshots the cache via the existing `snapshotCache()` primitive that `persistMutation` uses; on persist failure, restore the prior cache and return false. Load is fully atomic. HIGH fixes ---------- - **Workflows-store invariant block updated.** Lists `replaceAllWorkflows` as the fourth legitimate rebind site (with rollback semantics). "Fifth rebind" warning bumped to "sixth." - **Explicit HEAD route on `/koolook/presets/file`** that just does `is_file()` + status, no body work. The auto-synthesis path was reading the full file from disk just to discard it — wasteful on multi-MB snapshots over Dropbox/iCloud/NFS. - **Client `sanitizeName` mirrors server whitelist regex** (`^[A-Za-z0-9 _.()\-]+$`). Characters outside the whitelist (`+`, `&`, `'`, accented letters, emoji, etc.) collapse to `_` rather than triggering an opaque HTTP 400 at save time. - **Error-reason surfacing via `await resp.text()`.** RFC 7540 strips HTTP/2 reason phrases; behind any HTTP/2-terminating proxy `resp.statusText` is empty. Reading the body always carries aiohttp's `reason=` text. Used by writePreset, deletePreset, saveSettings, readPreset. MEDIUM fixes ------------ - **Symlink follow-through hardening.** `_resolve_within_base` helper computes `(base / name).resolve()` and asserts it's still under `base.resolve()`. Defends against a symlink planted in a facility- shared library dir redirecting reads/writes off-share. - **Atomic settings write.** `_write_settings` writes to `<tmp>` then `os.replace`, so an interrupted process can't truncate the file and silently revert the user's saved library path on next load. - **`__init__.py` import wrapped in try/except.** The "non-fatal" comment promised that an import failure wouldn't break node registration; previously only `install()` returning False was guarded. Now any module-load exception is caught and logged with the `[Koolook]` prefix, and the rest of the plugin loads. LOW housekeeping ---------------- - Dropped dead exports `downloadSnapshotAsFile` + `readSnapshotFile` (left over from an earlier iteration that had Download/Upload UI). - Dropped 5 dead CSS rules (`.koolook-snapshot-tabs*`, `.koolook-snapshot-tab-pane`) from when the modal was tabbed. - `currentPresetName` cleared when the user deletes the currently- loaded preset (Load dialog's promptDelete now checks via the new `getCurrentPresetName` capability). - `loadPreview` warns to console on `kind` mismatch — non-snapshot JSON files in the library directory are now visible in DevTools rather than silently filtered. - `presetExists` is tri-state (`true` / `false` / `null`) — the Save flow refuses to write when the library is unreachable rather than silently skipping the overwrite-confirm prompt. Deferred -------- - modals.js size (1080 lines) — extracting the snapshot dialogs into their own file is a follow-up refactor. Not blocking merge. - Doc drift on maintainer docs / README env-var convention — separate documentation sweep. Verification ------------ - `python -c "import ast; ast.parse(...)"` on all .py files — clean. - `node --input-type=module --check` on every modified JS file — clean. - `python tools/preflight_release.py --skip manager-meta -v` PASS. - `grep -rn "downloadSnapshotAsFile\\|readSnapshotFile\\|koolook-snapshot-tab" web/` — zero hits (dead code gone). - Synced to live install for browser smoke-test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: nudge — re-trigger CI after webhook miss on d6d2bf8 The CI workflow didn't fire on the prior push (Socket Security ran but GitHub Actions Lint/Smoke/Pre-flight/Security Gate didn't). Empty commit to refire the pull_request webhook. * fix(routes): drop unnecessary f-string prefixes flagged by ruff (F541) Two adjacent string literals in the symlink-escape error message had an `f` prefix despite containing no placeholders. CI's ruff check fails on F541 — strip the prefixes so the lint gate passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
malkuthroandClaude Opus 4.7
|
428d4f538b |
chore(nodes): drop Easy_Version + bump 0.1.9 (#41)
Per maintainer direction: Easy_Version was a one-line node that turned
an INT N into the string `vNNN` (zero-padded). The first trivial node
ever written for the pack, never used in production workflows. Removed
to keep the pack's surface focused on the higher-value nodes.
Anyone needing the exact behavior can either inline `f"v{N:03d}"`
downstream of an INT primitive in their workflow, or pull a more
general string-format node from KJ Nodes / ComfyUI-Custom-Scripts.
BREAKING for any saved workflow that references the `Easy_Version` ID
(node will fail to load on import). Same migration shape as the v0.1.5
__koolook_v1_0_1 cleanup.
Also rolls the [Unreleased] PR #39 LATENT-dict-wrap fix into [0.1.9]
so it ships in the same release.
Touched:
- Deleted: k_easy_version.py
- __init__.py: removed version_mappings / version_display imports + merges
- README.md: removed node-table row, Pipeline category list update,
Features bullet, Usage section, Node Code Overview line, install
category list — Easy Version no longer mentioned anywhere
- pyproject.toml: 0.1.8 -> 0.1.9
- CHANGELOG.md: [0.1.9] entry covering the removal + the carried-over
PR #39 fix
Net effect: pack drops from 8 nodes to 7. Koolook/Pipeline subfolder
now has only EasyAIPipeline.
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
malkuthro
|
7f8eda4809 |
Restructure fork tracking and versioned Radiance layout.
Consolidate fork metadata into forks/, move Radiance custom code into versioned folders, and align external reference naming so upgrades and cross-machine parity are easier to manage. Made-with: Cursor |
||
|
malkuthro
|
13d78b776e | Make a Koolook cam-track-loader | ||
|
malkuthro
|
b41f6e17ca | renames | ||
|
malkuthro
|
8960af66c3 | add Easy batch | ||
|
malkuthro
|
0303343d06 | WORKING button | ||
|
malkuthro
|
92cfcaa7e9 | working - version - PIPELINE | ||
|
malkuthro
|
ccd091be82 | adding buttons - A | ||
|
malkuthro
|
949e0c52c2 | AI pipeline update - A | ||
|
malkuthro
|
d2301ee0a6 | init again | ||
|
malkuthro
|
e272606fa8 | add AI_pipline node | ||
|
malkuthro
|
94886fdf49 | Multiple files - new names | ||
|
malkuthro
|
789ebd35b1 | All working - A | ||
|
malkuthro
|
7a13bc1d03 | Size working -A | ||
|
malkuthro
|
a6a800a2a6 | Color RGB | ||
|
malkuthro
|
f3bd016ba4 | system-A | ||
|
malkuthro
|
f2c40b5a94 | Update __init__.py | ||
|
malkuthro
|
2dfd19deb0 | add in-out = positive Prompt tool | ||
|
malkuthro
|
6882ebc46b | add multiLine prompt box | ||
|
malkuthro
|
74713d0b24 | fixed init 2 comfyui | ||
|
malkuthro
|
86637d07da | init_ONLY | ||
|
malkuthro
|
bb114e48ac | add init |