32 Commits
Author SHA1 Message Date
malkuthro 8447c05795 docs: add automation loop visual overview (#163) 2026-05-19 23:20:36 +02:00
malkuthroandClaude Opus 4.7 9372032b46 feat(automations): workflow validator + LTX 2.3 stage-2-only docs (#170)
* feat(automations): workflow JSON validator + LTX 2.3 stage-2-only findings

Adds scripts/validate_workflow.py — a stdlib-only CLI that checks a
ComfyUI workflow JSON for internal consistency: link-table declarations
vs node-IO references, slot index bounds and types, endpoint cross-
checks, last_node_id/last_link_id bounds, and duplicate IDs.

Catches the class of bugs that come from hand-editing workflow JSONs
(e.g. when an agent re-wires a graph to build a slim variant from a
Director workflow). Comfy's loader silently drops or mis-wires bad
parts, so the cost of an undetected drift is "graph looks fine but
behaves wrong" — exactly the failure mode this script blocks.

Exit codes: 0 clean, 1 problems on stderr, 2 file missing or bad JSON.

Docs:
- docs/automations/CONVENTIONS.md §11 — when and how to run the
  validator, full list of checks performed.
- docs/automations/LTX-2.3/findings.md — two new sections covering the
  spatial upscaler's architectural facts (ltx-2.3-spatial-upscaler-x2
  is hardcoded x2, the brief enters only via Director > guide_data >
  DirectorGuide, attention scales (W * H * T)^2) plus three open
  hypotheses for a stage-2-only refine path.
- CHANGELOG.md [Unreleased] entry.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(scripts): make validate_workflow.py robust against malformed JSON

Addresses PR #170 review (Codex GPT-5.3) — the validator crashed with
AttributeError on malformed-but-valid JSON, defeating its goal as a safe
preflight tool. Two repros from the review:

    {"nodes":[1],"links":[],"last_node_id":0,"last_link_id":0}
    -> AttributeError: 'int' object has no attribute 'get'

    [1,2]
    -> AttributeError: 'list' object has no attribute 'get'

Both confirmed locally. Now every shape assumption is guarded:

- Top-level must be a JSON object; non-object short-circuits with one
  clear problem line.
- 'nodes' and 'links' must be lists; non-list reported and treated as
  empty so subsequent checks still run.
- Each node entry must be a dict; bad entries reported and dropped.
- Each input/output entry must be a dict (extracted into a
  _sanitize_slots helper, cached per node so duplicates aren't reported
  when the same node is touched by multiple links).
- Each link entry must be a 6-element list; malformed link entries
  reported and skipped without crashing later phases.
- node_by_id only keys on int node IDs (non-int IDs can't match a
  link's int src/dst, so we don't mask bad data by accepting them).

Adds tests/scripts/test_validate_workflow.py — 24 tests covering both
review repros, every new guard, and a sampling of the existing
consistency checks (link-to-missing-node, type mismatch, endpoint
desync, duplicate IDs, last_node_id bound, CLI exit codes). Full suite:
168 passed in 1.59s.

CHANGELOG entry updated to reflect the defensive behavior + test
coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 22:52:35 +02:00
malkuthroandClaude Opus 4.7 092c204060 feat(automations): LTX Director iteration-loop framework + 4K investigation docs (#159)
* docs(investigations): kickoff for LTX Director 4K transition breakdown

Captures the maintainer's problem statement (HD multi-keyframe works,
4K single-keyframe works, 4K multi-keyframe degenerates into dissolves
between keyframes) and an initial conclusion about the WhatDreamsCost
LTXDirector node's implementation.

Traces the lineage to the Prompt Relay paper (Chen, Huang, Liu —
arXiv:2604.10030) via Kijai's ComfyUI port, separates the three
mechanisms the node bundles (cross-attention penalty, guide-image
keyframes, audio/image preprocessing), and flags the candidates for
the 4K dissolve: model-side limit at LTX sampling time vs. node-side
length-independent σ in the penalty matrix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(investigations): round 2 — 2K vs 4K + two-stage upscale findings

Adds the maintainer's round-2 observations:
 - 2K renders behave much better than 4K (segments morph instead of
   dissolving) — model is closer to its training distribution.
 - The two-stage workflow's upscale stage (stage 2) is overwriting
   stage 1's audio/animation: lip sync degrades and segment routing
   weakens.

Captures the hypothesis space:
 - Why 2K > 4K: model prior (out of distribution at 4K) is dominant;
   token-grid scaling with a length-independent sigma is secondary.
 - Why stage 2 overwrites stage 1: KSampler denoise re-applies
   conditioning from scratch; if LTX Director isn't wired into
   stage 2 the Prompt Relay + audio_attn2 patches go missing;
   latent shape change between stages forces a penalty rebuild.

Includes a 3-option pros/cons table (4K direct vs 2K direct vs
2-stage) and 6 investigation directions ordered cheapest-first,
starting with auditing the stage-2 wiring.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(investigations): round 3 — both stages run LTX Director; new prime suspect

Maintainer's screenshot confirms both stages route through LTX Director,
so round 2's missing-patch hypothesis is ruled out. Audited the stock
LTX nodes used in stage 2 against ComfyUI source:

 - LTXVLatentUpsampler explicitly pop()s the noise_mask and runs a
   learned (EXPERIMENTAL) neural upscaler — not interpolation.
 - LTXVConcatAVLatent fills a missing noise_mask with ones_like
   (denoise fully) for the side that's missing.
 - LTXVCropGuides removes keyframe slots and clears keyframe_idxs.

Traced the audio noise_mask end-to-end through stage 2 and found:
audio waveform stays preserved (noise_mask=0), video keyframes stay
pinned, but every non-keyframe video frame goes into stage 2's
sampler at noise_mask=1 — i.e. fully regenerated at denoise=0.80.
That's where stage 1's audio-aligned mouth shapes live, and that's
what's getting thrown away.

Prime suspect: denoise=0.80 is too high for "preserve stage 1"
semantics — most LTX two-stage reference workflows use 0.30–0.40.
Secondary suspect: the LTXVLatentUpsampler [BETA] is a learned model
that can hallucinate spatially-dense detail (faces, mouths) before
stage 2 even starts denoising.

Includes a 4-step cheapest-first test plan starting with dropping
stage 2 denoise to 0.30.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(investigations): round 4 — scheduler-shape finding + tracking plan

Captures two outputs from the planning session so they're preserved
for the next pickup:

1. Promotes the maintainer's empirical finding to a locked-in
   baseline: `linear_quadratic` scheduler at 8 steps produces the
   right sigma curve shape (flat plateau near sigma=1.0 for ~5
   steps, then steep drop in the last 3) for both phases. Any
   other scheduler degrades fast motion and keyframe transitions.

2. Records the agreed tracking-system plan (BASELINE.md +
   log.md + per-run screenshot folder, with three 1-5 score axes
   matching the live trade-off: motion / audio sync / Phase-2
   sharpness). Not yet scaffolded — explicit "next step" callout
   for whoever picks this up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(investigations): /make-card skill + PNG card renderer toolkit

Per-iteration tracking card for the LTX Director 4K investigation.
Solves the documentation-drift problem we hit on v01->v02 (multiline
summary diverged from actual node values) by treating the saved
workflow JSON as the source of truth.

Adds:
 - scripts/make_card.py — reads a ComfyUI workflow JSON, paints a
   self-contained 540px PNG card via Pillow. Extracts Phase 1 run
   params (Format/Mult/Scheduler/Steps/Denoise), locked base
   (Sampler/CFG/Seed/epsilon/Audio src), scene (duration/segments
   with prompt/audio/keyframe checkboard), free-text BASE notes
   from `OVERLAY - INFO` Text Multiline, FEEDBACK from `OVERLAY -
   FEEDBACK` Text Multiline, and a POST-RENDER block that
   auto-computes render duration from JSON->newest-video mtime
   delta in the same folder. Layout mirrors the project's
   snapshot-dialogs design language (panelled sections, sky/amber/
   green accents matching docs/designs/snapshot-dialogs.html).
 - scripts/watch_cards.py — background poller. Watches a working
   folder; auto-renders <stem>_card.png whenever a workflow JSON
   appears or is updated. Skips up-to-date PNGs.
 - .claude/skills/make-card/SKILL.md — /make-card slash command;
   globs the newest JSON in the working folder, calls make_card.py,
   shows the PNG inline.
 - .claude/skills/make-card/work-folder.txt — one-line config of
   the working folder path. Edit to switch projects.
 - docs/investigations/experiments/{baseline,combined-card,run-001-card}.html
   — design prototypes that fed into the PNG layout; kept as
   history.
 - docs/investigations/experiments/run-001-card.png and
   run-v04-card.png — sample card outputs from v03 and v04 of the
   workflow JSON, included for reference.

Issue #151 tracks the eventual native ComfyUI custom node so this
flow happens inside the graph; for now the external script + watcher
is the loop.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(launch): use absolute Windows python path for design-server

The default `python3` runtimeExecutable fails on this Windows host
(triggers the Microsoft Store install stub). Hard-coding
C:\Python313\python.exe lets the preview MCP and the design-server
target work on the maintainer's machine. Worktree-scoped change —
main keeps the cross-platform `python3` default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(automations): docs/loop scaffold + env-var working folder + _AI subdir

Promotes the LTX Director card-rendering loop from one-off scripts to a
documented, project-portable automation framework.

Adds `docs/automations/` as the home for AI-assisted iteration loops:
 - README.md — the pattern; how to start a loop for a new model
 - CONVENTIONS.md — cross-cutting: working folder, Text Multiline node
   contract (Working_Folder_PATH / OVERLAY - INFO / OVERLAY - FEEDBACK),
   card format, iterations log, skill, watcher, run-id rules, tool
   locations
 - LTX-2.3/README.md — entry point for the LTX 2.3 loop; per-iteration
   steps; full extraction table; active investigation status
 - LTX-2.3/handoff-checklist.md — 5-minute onboarding for a new machine
   or new person
 - LTX-2.3/findings.md — promoted findings (linear_quadratic scheduler
   locked-in; resolution behaviour; audio mode; open two-stage suspects)

Replaces the committed `.claude/skills/make-card/work-folder.txt` with
an env-var-driven path. Follows the project's existing `KOLOOK_*`
convention — `.env.example` documents the new
`KOLOOK_AUTOMATIONS_WORK_DIR`. Real path lives in `.env` (gitignored).
No more leaking client-specific absolute paths into committed files.

`scripts/make_card.py`:
 - Loads `.env` automatically; worktree-aware (walks the .git common-dir
   marker to find the main repo when run from a worktree).
 - Output folder priority: Working_Folder_PATH node in JSON → env var →
   JSON's parent dir.
 - Run with no args: auto-discovers newest *.json in the working folder.
 - Writes `_AI/card.png` + `_AI/iterations.md` inside the working folder,
   not the root. Lets the maintainer wipe AI artifacts or user content
   (JSONs/MP4s) independently.

`/make-card` skill simplified to a single command — `python
scripts/make_card.py` — the script handles env loading and discovery
itself.

baseline.html: stripped the project-specific label.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(automations): sync all path references to _AI/ + add layout diagram

Audit pass after introducing _AI/ subfolder. Updated every doc and
script that still referenced root-level card.png / iterations.md:

 - docs/automations/CONVENTIONS.md (3 spots)
 - docs/automations/LTX-2.3/README.md (per-iteration steps + composite)
 - docs/automations/LTX-2.3/handoff-checklist.md (4 spots, including
   the off-boarding section which conflated repo commits with working-
   folder artifacts)
 - docs/automations/LTX-2.3/findings.md (promotion criterion)
 - scripts/watch_cards.py (target PNG path now matches make_card.py's
   _AI/card.png — was writing to a stale <stem>_card.png location)

Adds a clear "Where things live" section to docs/automations/README.md
showing both layouts side-by-side:
 - the repo tree (where the tooling, skill, env-example, and per-model
   loop docs sit)
 - the per-project working-folder tree (user content at root, _AI/
   subfolder for managed artifacts)

So anyone picking this up can answer "what's in the repo vs. what's in
my project folder" without reading prose.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(automations): add CHEATSHEET.md — one-page dense reference

Compact reminder card for returning users. Structured by application
context: ComfyUI · Scripts · Folder structure · Conventions · Quick-fail
diagnostics. Skippable for first-timers; targets the maintainer who
already knows the loop and just needs the right node names, score
syntax, and command list at a glance.

Linked from docs/automations/README.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(automations): add Bootstrap section to CHEATSHEET — bare-bones setup

What's needed to get a fresh machine running the loop, separated from
the daily-loop reference. Three checklists:
 - Repo side: Python + Pillow + .env config
 - ComfyUI side: was-node-suite + the three required Text Multiline
   nodes, with exact titles
 - First card: /make-card → expect _AI/card.png

Deliberately skips model-specific setup (checkpoints, LoRAs, per-model
custom nodes) — those live in each model's handoff-checklist.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(automations): skip *loop* files in auto-discovery + watcher

The maintainer's workflow includes a "loop" stage in ComfyUI that
composites the card next to the video and writes the result back into
the working folder as `<base>_loop_<seq>.{mp4,json}`. Without a skip
rule, auto-discovery treats this composited-card output as the newest
input — the script then re-renders the card from yesterday's metadata.

Now: any JSON or video file with `loop` in its stem (case-insensitive)
is ignored by:
 - scripts/make_card.py — auto-discover newest JSON, newest video
 - scripts/watch_cards.py — polling glob

Explicit invocation still works: `python scripts/make_card.py
<wf_loop.json>` will render a card for that file specifically.

Documents the rule in docs/automations/CONVENTIONS.md (new §7) and adds
a one-line bullet to CHEATSHEET conventions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* style(automations): satisfy Ruff lint — split if-on-one-line, rename l, drop os

CI's Lint & Smoke step flagged 17 violations across the new scripts.
Cleaned up:

 - scripts/make_card.py: 11 × E701 (multi-statement lines)
   split every `if cond: stmt` onto two lines.
 - scripts/make_card.py: 5 × E741 (ambiguous variable `l`)
   renamed to `link` in the two link-dict comprehensions and their
   downstream lookups.
 - scripts/watch_cards.py: 1 × F401 (unused `os` import).

No functional change — `make_card.py` re-renders the working folder's
card identically post-cleanup. Verified `ruff check` returns
"All checks passed!" on both files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(automations): render time from ComfyUI log, not disk timestamps

Disk mtimes don't reliably bracket the model run — the maintainer's
workflow saves the workflow JSON manually after rendering, assembles
QuickTime from EXRs separately, and ComfyUI's metadata bundle writes
JSON + MP4 at the same instant. None of those give us actual render
duration.

Render time now sourced from ComfyUI's own log
(`<comfyui-root>/user/comfyui.log`):

 - Find the latest `saving images: 100%` line → pair it with the next
   `Prompt executed in X` → that's the most recent EXR-producing render
   wall-time.
 - Supports both `Prompt executed in 18.21 seconds` and
   `Prompt executed in HH:MM:SS` log formats.
 - Skips trivial follow-up prompts (e.g. audio-VAE load between render
   and /make-card invocation).

Log path resolution:
 1. `KOLOOK_COMFYUI_LOG` env var (explicit override).
 2. Inferred from existing `KOLOOK_COMFYUI_DEV_PATH` →
    `<comfyui-root>/user/comfyui.log`.

Fallbacks remain in priority order: EXR sequence span (oldest to newest
in the latest version subfolder) → MP4 mtime delta → "pending".

Docs:
 - .env.example: documents the optional KOLOOK_COMFYUI_LOG.
 - CONVENTIONS.md: new §4 "Render duration" describing the resolution
   chain; section numbers renumbered.
 - CHEATSHEET.md: updated the "Render time —" diagnostic.

Lint: ruff clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 22:44:52 +02:00
malkuthroandClaude Opus 4.7 4409d3c9d8 feat(ai_pipeline): no_subfolders toggle + path normalisation + tooltips (#156)
* feat(ai_pipeline): no_subfolders toggle + path normalisation + tooltips

Three EasyAIPipeline improvements with a 49-case test suite pinning
the behavior so the user-reported "phantom undefined/ folder" bug
can't regress.

no_subfolders (default off): write directly into base_directory_path
with no shot_name/ai_method/vNNN subfolders appended. Base folder is
still auto-created if missing.

Paste-input hardening: _normalize_base_path strips surrounding
whitespace, one matched pair of quotes ("..." from Explorer's Copy
as path), and trailing path separators. Drive roots (C:\, n:/) are
preserved. Plus a belt-and-suspenders rstrip on the final
output_directory — fixes the symptom where a trailing backslash on
the user's pasted base path caused downstream save nodes to split
on /, stringify the empty tail as "undefined", and write into
.../undefined/... on disk.

Smart filename: empty pieces of shot_name / ai_method / vNNN are
filtered before the underscore join, so blank ai_method yields
RTX-upscale.exr (not RTX-upscale_.exr). Same rule for subfolder
build.

Tooltips on every input explain interactions (e.g. ai_method
becomes both filename segment and subfolder; enable_overwrite only
blocks an existing file, not directory).

JS preview helpers in web/ai_pipeline.js mirror the Python so the
Get output directory / file path buttons stay accurate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ai_pipeline): sanitize shot_name/ai_method to prevent base-path escape

Reviewer-flagged blocker in PR #156: os.path.join's "last absolute path
wins" rule let absolute-looking shot_name or ai_method values (typos
like "/oops", or pasted full paths like "C:/Windows/junk") replace
base_directory_path entirely. The pre-#156 code had a partial defense
(.lstrip('/')) that the refactor dropped without replacing.

New _sanitize_segment helper: lstrip leading separators → splitdrive →
lstrip again. Strips leading seps BEFORE splitdrive so multi-slash
input like "///oops" doesn't get swallowed as a UNC prefix on Windows
(empirically observed — the user's intended segment name would be lost).
Applied symmetrically to directory build and filename build. Raw
shot_name kept in the return tuple for downstream label-only consumers.

Also fixes a minor JS preview mismatch at web/ai_pipeline.js: empty
base + no_subfolders=true used to render "/name.exr" while Python
returns "name.exr". Filter empties before joining the file_path.

16 new tests pin both the helper and the end-to-end escape vectors:
- TestSanitizeSegment (7 unit tests, platform-aware drive handling)
- 4 parametrised absolute shot_name escape cases
- 4 parametrised absolute ai_method escape cases
- 1 filename-leak regression

Full pytest: 102 passed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ai_pipeline): flatten embedded separators in shot_name for filename

Maintainer-reported: with no_subfolders=true and an upstream node feeding
shot_name="ComfyUI-working-folder/LTX-Director-2K", the output was
base/ComfyUI-working-folder/LTX-Director-2K.%04d.exr — still creating
subfolders via the filename concat, defeating the toggle entirely.

Root cause: the prior fix sanitized only LEADING separators (path-escape
case). Internal separators in shot_name slipped through and became
subfolders when the filename string with embedded "/" was joined onto
output_directory by the filesystem.

Fix: filenames can't contain "/" or "\\" on any OS, so always flatten
them to "_" in the filename build. Directory build with
no_subfolders=false still uses the separators as subfolder delimiters
(intentional — users organizing into Project/v1/ai_method/vNNN want this).

With no_subfolders=true, shot_name="job/shot" now correctly produces
base/job_shot.ext (flat in base), not base/job/shot.ext.

3 new tests:
- Maintainer's exact case (no_subfolders=true + slash in shot_name)
- Belt-and-suspenders across 5 shot_name/ai_method/toggle combinations
- Verify no_subfolders=false still creates subfolders from slashes
  (preserves intended use case)

Full pytest: 110 passed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ai_pipeline): strip control chars + single-line Output Preview

Maintainer reported the file_path preview rendering as two visual lines
("n:/.../bear/...LTX-Director-2K" + newline + "_0.%04d.exr") and the
output looking like it contained a literal newline. Two contributing
causes:

1. The multi-line Output Preview widget word-wrapped long single-line
   paths at the textarea edge. The data was always one logical string,
   but the visual break made it look broken. Fix: set wrap="off",
   whiteSpace="pre", overflowX="auto" on the input element so long
   paths render on a single horizontal-scrollable line.

2. Upstream nodes (WAS Text Multiline etc.) can legitimately emit
   strings with embedded \n / \r / \t — a stray paragraph break in the
   widget content. Those would silently break filesystem saves and ALSO
   wrap the preview at the actual newline character (not just the
   textarea edge). Fix: new _strip_control_chars helper, applied at the
   top of _normalize_base_path and _sanitize_segment (covers
   base_directory_path, shot_name, ai_method) plus explicitly on
   extension in generate_pipeline. Mirrored in JS for preview parity.

9 new tests: 6 helper unit tests, 3 end-to-end (shot_name w/ newline,
base path w/ newline, extension w/ newline).

Full pytest: 119 passed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ai_pipeline): strict whitespace strip on extension + revert widget

Two issues, one commit:

1. Maintainer reported ComfyUI-HQ-Image-Save bailing with "Filepath
   needs to end in .exr" even though our preview showed a clean
   .%04d.exr suffix. Root cause: HQ-Image-Save validates via
   os.path.splitext(filepath)[1].lower() != ".exr" — a single trailing
   space on the extension widget (easy to acquire from a paste) makes
   splitext return ".exr " with the space, which fails the strict
   equality. Previous control-char strip only handled \r\n\t and
   left regular spaces alone.

   Fix: extension now runs through "".join(extension.split()) which
   removes ALL whitespace (internal + surrounding, including unicode
   whitespace and non-breaking spaces). Mirrored as cleanExtension
   in JS. Also added .strip() to _sanitize_segment so shot_name /
   ai_method with stray surrounding whitespace canonicalise too.

2. Reverted the Output Preview widget from the previous commit's
   single-line wrap=off + horizontal-scroll layout back to the
   original multi-line word-wrap (100px height, default textarea
   behavior). The earlier change traded mouse-drag-selectability for
   single-line display — wrong tradeoff. The path is one logical
   string anyway (cleanLine strips real newlines), so visual wrap is
   just cosmetic.

9 new tests:
- 6 parametrised extension whitespace cases (trailing/leading/internal
  space, tab, non-breaking space) — all verify HQ-Image-Save's
  splitext check passes after sanitization
- 3 parametrised shot_name surrounding-whitespace cases

Full pytest: 128 passed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(ai_pipeline): no_subfolders keeps version folder; v### in tooltips

Maintainer-clarified semantics for the no_subfolders toggle: it should
only drop shot_name / ai_method from the directory chain, NOT the
version folder. Previously, no_subfolders=true stripped everything,
which meant versioned outputs all collided in the same flat directory —
defeating versioning whenever the toggle was on.

New behavior matrix for no_subfolders=true:
- disable_versioning=true  → output_directory = base
- disable_versioning=false → output_directory = base/v###

So truly flat output (no subfolders at all) now requires BOTH toggles,
matching VFX convention. Versioned-but-no-name-subfolders is now
expressible with just no_subfolders=true, which is the common case.

Tooltip updates:
- no_subfolders: rewritten to describe new behavior
- base_directory_path: path-shape description updated to
  shot_name/ai_method/[v###]
- ai_method: clarifies subfolder application is gated by no_subfolders
- version, disable_versioning: vNNN → v### (Nuke-style convention
  consistent with the existing .%04d.exr default)

2 new tests:
- no_subfolders=True + disable_versioning=False → base/v###
- no_subfolders=True + disable_versioning=True  → base (truly flat)

Existing tests use the disable_versioning=True default with
no_subfolders=True so they continue to assert base-only — they still
pass under the new logic since version_str="" gets filtered out of the
join.

Full pytest: 130 passed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ai_pipeline): strip sentinel path components

* fix(ai_pipeline): resolve EasyUse get-set preview values

* docs: document ComfyUI preview button debugging

* fix(ai_pipeline): align drive segment sanitization

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 21:01:43 +02:00
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 943be67 removed (defensive strip handled
  the undefined/ bug; no further diagnosis needed).
- Local fp_raw / od_raw variables (only used by the debug print)
  collapsed back into direct call sites.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(video): suppress metadata PNG + silent intermediate by default

Upstream VHS writes 3 files per render when audio is connected:
  - <name>_NNNNN.png      (first-frame sidecar with workflow metadata)
  - <name>_NNNNN.mov      (silent intermediate)
  - <name>_NNNNN-audio.mov (final audio-muxed video)

Both extras are controlled by hidden VHS_MetadataImage and
VHS_KeepIntermediate flags in extra_pnginfo['workflow']['extra'],
both default-on upstream. Result: a single 500MB render lands as
three files (one needed, two clutter), nearly 2x the disk footprint
and a confusing folder.

Adds two optional BOOLEAN inputs (both default False on Koolook):
  - save_metadata_png
  - keep_silent_intermediate

combine_video injects them into extra_pnginfo before delegating to
super(), so the existing upstream flags do the actual work — no
post-hoc file deletion, no fork divergence from upstream's encoder
path. The extra_pnginfo dict is copied (not mutated in place) so
other nodes in the workflow that reference the same dict are
unaffected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* diag(video): log frame-count + audio length, restore PNG default-on

Two changes to investigate a doubled-length output report:

1. Default save_metadata_png back to True (was False since 946e170).
   The PNG sidecar carries the embedded workflow JSON — it's the
   only audit trail when a render comes out wrong. Removing it by
   default left no way to diagnose retroactively. Users who don't
   want the sidecar can still toggle it off explicitly; ones who do
   want auditability (most VFX pipelines) get it automatically.

2. One-shot diagnostic print() at the top of combine_video logging:
   - input image batch size + shape
   - audio sample count + sample rate -> seconds
   - frame_rate -> expected video seconds
   - pingpong + loop_count

This pins down whether a doubled-length output is the image stream
arriving already-doubled (workflow issue: ImageConcatMulti in batch
mode, an upstream pingpong, etc.) vs combine_video itself doing
something with the frames. To be removed in a follow-up once the
current report is diagnosed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(video): restore Easy_VideoCombine widgets by name

* feat(video): polish Easy_VideoCombine review workflow

* test(video): fix Easy_VideoCombine pytest coverage

* docs: require repo-local Python venv

* fix(video): handle existing silent mux intermediates

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 14:53:18 +02:00
malkuthro 55dab39c9b Trim workflow context menu move list (#153) 2026-05-16 23:22:23 +02:00
malkuthro f74b30603e Release v0.3.2 (#149) 2026-05-16 01:25:39 +02:00
malkuthro 119a1e3e6c docs: refresh release guide screenshots (#148) 2026-05-16 01:05:02 +02:00
malkuthro c7961998aa feat(snapshot): refine recovery choices and snapshot dialog styling (#146)
* Refine snapshot load dialog recovery UI

* Make snapshot load recovery choices explicit

* Align snapshot picker styling

* Respect Comfy theme variables in snapshot dialogs

* Document superseded snapshot recovery design

* Fix load recovery selection races

* Prevent duplicate snapshot dialog actions

* Harden snapshot recovery UI fallbacks
2026-05-15 10:35:13 +02:00
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>
2026-05-14 21:00:34 +02:00
malkuthroandClaude Opus 4.7 e87d111521 docs(designs): snapshot dialogs redesign mockup + Mermaid flow (WIP) (#136)
* docs(designs): snapshot dialogs redesign mockup + Mermaid flow (WIP)

Design exploration for consolidating the Snapshot Settings cog into the
Save and Load dialogs, plus the related autosave-newer fix. Discussion
artifacts only — no code changes.

- docs/designs/snapshot-dialogs.html: side-by-side mockups across six
  sections (toolbar, Save dialog, Load dialog with default / Recovery
  expanded / inline delete-confirm states, autosave-newer modal,
  button-state pattern, folder-browse picker)
- docs/designs/snapshot-dialogs-redesign.md: scope sketch + Mermaid flow
- docs/designs/README.md: register the two new design files in the index

Single-library design stays (documented intent per
docs/maintainers/curated-sidebar.md). Redesign only consolidates surface;
implementation lands in a separate PR after layout details are locked in.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(designs): refine autosave-newer modal + single-row recovery groups

Two scoped tweaks to the snapshot dialogs mockup:

- Section 4 (autosave-newer): drop the "Replace current state?" question
  since the user already chose Load; promote "Auto-save is newer than
  the saved version" to the only head message. Render the two options
  as standard preset-rows (filename + picks/workflows/date meta) so the
  layout mirrors the Load dialog list. Buttons collapse to NO /
  YES — load latest on one row.
- Section 3 (recovery expanded): show only the newest row per
  <preset>_autosave/ group — whichever of Pre-load or Periodic is most
  recent — instead of listing both kinds. Annotation also notes the
  Recovery section auto-expands when a preset row is clicked.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(designs): lock recovery behavior — hidden by default, one row per clicked preset

When the user clicks a preset row (Koolook_v03, starter, etc.) the recovery
section opens and reveals only that preset's <preset>_autosave/ folder,
with a single row inside — whichever of Pre-load or Periodic is newest
(the system picks). _unsaved_autosave/ no longer renders inside this
preset-scoped view since it isn't tied to the clicked preset.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(designs): polish section 6 + recovery row layout

Folder-browse picker (section 6):
- Switch to standard navigate-into model — clicking a folder drills in,
  body lists that folder's contents (subfolders clickable, files greyed-
  out for context). Path input shows where you are; "Use this folder"
  commits the current path. Drops the sibling-list + selected-tag model.
- Single-line toolbar: ↑ Up next to the path input; secondary parent-
  path strip removed.
- Path input keeps the END visible when overflowing (direction:rtl +
  text-align:left), so a long path shows the meaningful trailing folder
  instead of /Users/… — and example path is now long enough to
  demonstrate it.
- Header restyled to match other modal heads (15px title, dark bg,
  bigger padding); descriptive subtitle removed.
- Bottom bar: New folder… stays left; Cancel + Use this folder push
  right (.picker-foot .spacer rule, since the existing flex-spacer was
  scoped to .modal-actions).
- Redundant "SELECTED FOLDER" stripe removed — selected-row blue accent
  is sufficient confirmation; folder list gains the freed vertical room.
- Example folder rename _unsaved_autosave -> default_autosave (label
  only — on-disk constant unchanged; rename is an open suggestion to
  drop the self-contradicting "unsaved autosave" term).

Recovery row (section 3):
- Layout: badge on top, single combined meta line below in order
  "<picks> · <workflows> · <date>" — matches preset-row shape so both
  rows read the same way.
- Padding bumped to 10px 12px so recovery-row and preset-row are the
  same box size; badge nudged 1px up + meta nudged 1px down for
  breathing room.
- Recovery section now scopes to the clicked preset only: orphan
  _unsaved_autosave/ group removed from the "Koolook_v03 clicked" view,
  since when a preset row is clicked the dropdown should only show that
  preset's auto-saves.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: require visual verification before requesting design-driven review

When implementing a UI change that has a corresponding mockup in
docs/designs/ (e.g. snapshot-dialogs.html, sidebar-icon-proposals.html),
the design file is the spec. Before the work is brought to the
maintainer for review, the implementing agent must:

  1. Render the implementation in a real browser — dev-sync for runtime
     sidebar work, python3 -m http.server (or the Launch preview panel)
     for standalone HTML mockups.
  2. Screenshot the implemented UI at every state the mockup shows.
  3. Diff each screenshot against the mockup. Match it or document the
     deviation with a rationale.
  4. Only then request review.

The maintainer should never be the first set of eyes on rendered
output. Type checks and tests verify code correctness, not visual
fidelity to the design — if a mockup exists for the change, visual
verification is a hard gate, not optional polish.

Rule lives in three places, each hit from a different entry point:
- CLAUDE.md (loaded at every session start; canonical)
- docs/designs/README.md (discovered when opening the designs folder)
- docs/maintainers/dev-iteration-loop.md (step 4 in the iteration loop)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* skills(tdd): add project-specific TDD skill modeled on ForgeFlow's

When implementation work on docs/designs/ mockups begins, /tdd on this
repo should pick up a project-level skill that knows ComfyUI-Koolook's
actual machinery — not just generic TDD principles.

Two files:

- SKILL.md — keeps the global TDD philosophy (vertical slices,
  behaviour-not-implementation, refactor only after GREEN) and adds a
  project-specific tail covering:
  • code surface: where Python nodes vs sidebar JS vs design specs live
  • test surface: what CI runs today (ruff, compileall, bandit,
    preflight_release.py) and explicit guidance on introducing pytest /
    vitest "only when there's a behaviour that genuinely needs unit
    testing" — same PR as the first test, never speculative
  • the dev-sync iteration loop for runtime sidebar work
  • high-value test targets (node registration, route validation, VAE
    dispatch, snapshot row augmentation)
  • drift patterns specific to this codebase (vendored upstream into
    MAIN, renamed namespaced node IDs in saved workflows, hardcoded
    user paths, dev-sync triggered from an automated flow, …)
  • final scope-alignment check routing (clean → proceed; drift → show
    report, never revert)

- visual-verification.md — the secondary procedure for cycles whose
  assertion is CSS / layout / structural. Procedure split for the two
  shapes of work in this repo:
  • runtime sidebar (web/sidebar/*.js): dev-sync → restart ComfyUI →
    screenshot → diff against docs/designs/<feature>.html
  • standalone HTML mockups (docs/designs/*.html): Launch preview panel
    or python3 -m http.server (no new package installs per
    supply-chain rules)
  Plus a structured definition of what "diff against the mockup" means
  (spatial, hierarchy, states, chrome, text — not pixel-equality), so
  it isn't a vague gate.

Generic companions (deep-modules, mocking, refactoring, interface-
design, scope-check, tests) intentionally not copied — agents follow
the global ~/.claude/skills/tdd/ pointers in SKILL.md for those. Avoids
sync-drift on principle docs and keeps this skill focused on what's
genuinely project-specific.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-14 17:43:55 +02:00
bf02db8ad8 feat(easy_image_batch): source_batch + source_frames list + selected_… (#134)
* feat(easy_image_batch): source_batch + source_frames list + selected_* outputs

Three-mode input system, two new "selected only" outputs, and an alpha
output convention that's explicit about its role.

Inputs
- source_batch (IMAGE, optional): pre-batched stack, e.g. straight from
  Load Video.
- source_frames (multiline STRING): VFX frame numbers separated by
  commas, newlines, spaces or any mix. Picks from source_batch and
  places at the same timeline position. Removes the 4-keyframe ceiling.
- placeholder_color: now offers Black / Gray / White (was Black / Gray).
- invert_alpha (BOOLEAN, default off = "inpaint"): flips alpha to
  compositing convention (selected=white) when on.
- image1 is now optional (you can drive the node entirely from
  source_batch + source_frames). At least one of image1 or source_batch
  must be connected.
- start_frame default changed from 0 -> 1 to match VFX 1-based numbering.

Mode rules
- Mode 1 -- per-slot images: connect imageN, set imageN_frame.
- Mode 2 -- source_batch + 4 slots: when imageN is unconnected,
  imageN_frame doubles as the source pick index AND timeline target.
- Mode 3 -- source_batch + source_frames list: when the list is
  non-empty, the 4 slots contribute ONLY where imageN is explicitly
  connected -- the imageN_frame defaults (0, 4, 8, 12) do NOT pick from
  source_batch in this mode, so the list fully controls the selection
  unless an imageN override is wired.
- Priority: source_frames list runs first; manual slots run after and
  override at the same target index. Output dedup is automatic
  (set-based).

Outputs
- image_batch (IMAGE): full timeline with placeholders. Unchanged.
- alpha_batch (MASK): renamed from mask_batch; same default convention
  (selected=0.0/black, empty=1.0/white) so existing wires work
  unchanged. Toggle invert_alpha for compositing-style.
- selected_image_batch (IMAGE, NEW): only the keyframes that landed,
  packed back-to-back in timeline order. Empty (length 0) if none
  placed.
- selected_frames (STRING, NEW): comma-separated VFX frame numbers,
  sorted ascending and deduped. Same format as the source_frames input
  -- round-trips into another easy_ImageBatch instance.

Docs
- Updated class docstring with input modes + worked example (surfaces
  in ComfyUI's node tooltip).
- New per-node user guide:
  docs/user_guide/nodes/koolook_image/easy_image_batch.md.
- docs/user_guide/README.md graduates easy_ImageBatch from "TBD" to a
  real link.
- CHANGELOG: Unreleased Added/Changed entries cover the full change set.

Backward compatibility
- Existing workflows wired to image_batch + mask_batch keep working;
  the second output slot position is unchanged.
- The 4-keyframe screenshot pattern (source_batch + 4 slots, no list)
  is unchanged in behaviour.
- start_frame default change affects only newly added nodes; saved
  workflows keep the value they were saved with.

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(easy_image_batch): cut window — rename start_frame -> cut_start_frame, fix source-pick math

The 2nd output of Easy Image Batch (Koolook) gets a built-in cut
window, replacing the workflow pattern of feeding the output through
an external ImageFromBatch (batch_index, length) slicer.

Conceptual rename
- `start_frame` -> `cut_start_frame` everywhere.
- The output now represents VFX frames
  [cut_start_frame .. cut_start_frame + total_frames - 1].
- Frames placed outside the cut window are dropped from the cut
  silently and listed in a single end-of-run summary log (replaces
  the previous per-frame "out of range" warnings).

Math fix (real bug)
The previous code conflated source pick index and output index under a
single `target_index = vfx - start_frame`. That happened to work when
start_frame=1 (source pick index = vfx - 1, by coincidence), but
produced a wrong source pick whenever start_frame != 1. Now:

    source pick index   = vfx_frame - 1                  # hardcoded VFX 1-based
    output (cut) index  = vfx_frame - cut_start_frame    # cut window

The two are validated independently:
- source-side range error -> per-frame warning ("VFX frame N doesn't
  exist in source_batch")
- output-side range -> silent drop, collected into the end-of-run
  summary

Defaults
- cut_start_frame=1 (was start_frame=1, so default behaviour for
  source_batch starting at VFX frame 1 is identical).
- total_frames default 16 -> 81 (matches typical VFX shot length).
- imageN_frame defaults 0/4/8/12 -> 1/5/9/13 (valid 1-based).
- imageN_frame min 0 -> 1 (frame 0 is invalid under VFX 1-based).

Console summary
Always logged so it's easy to confirm what the node produced. Shape:

    [easy_ImageBatch] cut window: frames 41..121 (81 frames).
    6 placed; 2 outside cut: 1, 27.

Docs
- Class docstring rewritten with the cut-window model + Examples A
  (cut_start_frame=1, no cut) and B (cut_start_frame=41, cut).
- docs/user_guide/nodes/koolook_image/easy_image_batch.md fully
  updated to match.
- CHANGELOG: new "Changed" entry under [Unreleased] explaining the
  rename, math fix, defaults bump, and migration notes.

Migration notes for users of the previous PR commit
- Saved workflows that had start_frame=1 (the default) keep producing
  identical output -- the math is unchanged in that case.
- Saved workflows that set start_frame to a non-1 value will see
  different source picks (the old code was actually picking the wrong
  source frame, the new code picks correctly). Re-check those.

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(easy_image_batch): shift imageN_frame defaults to 5/9/13/17

The previous 1/5/9/13 cluster put all four default keyframes inside the
first 13 frames of an 81-frame timeline. Shifting to 5/9/13/17 keeps the
same stride-of-4 but leaves a small gap at the start, which reads
cleaner in the node UI and gives a more representative spread of where
keyframes commonly land.

Updates the doc table and CHANGELOG bullet to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-13 19:37:49 +02:00
malkuthro 27705be037 feat(sidebar): add onboarding guide (#107)
* feat(sidebar): add onboarding guide

* docs(guide): clarify tag search casing

* docs(guide): clarify workflow save copy

* docs(guide): refine sidebar visuals

* docs(guide): refresh save snapshot visual

* docs(guide): refresh load snapshot visual

* docs(guide): refresh module save visual

* docs(guide): use real sidebar screenshot

* docs(guide): render sidebar overview screenshot directly

* docs(guide): soften sidebar overview annotations

* docs(guide): clarify nodes action bar callout

* docs(guide): clarify workflow action bar callout

* docs(guide): simplify sidebar overview markers

* docs(guide): align sidebar overview intro

* docs(guide): use real snapshot recipe screenshots

* docs(guide): simplify project workflow diagram

* docs(guide): make function cards scannable

* docs(guide): sketch module recipe icon focus

* docs(guide): use real icon screenshot for module recipe

* docs(guide): add cropped workflow action screenshot

* docs(guide): use full toolbar screenshot for module recipe

* docs(guide): use real search screenshot

* docs(guide): use real node search screenshot

* docs(guide): standardize screenshot recipe cards

* docs(guide): tighten recipe callout styling

* docs: require visual QA for UI work

* docs(guide): address pass-2 review nits
2026-05-08 13:44:55 +02:00
malkuthro b185118950 feat(sidebar): refresh action icons (#108)
* feat(sidebar): refresh action icons

* fix(sidebar): label glyph toolbar buttons
2026-05-08 13:10:35 +02:00
malkuthro e5e07f6c50 feat(sidebar): preserve subgraph definitions in modules (#103) 2026-05-06 22:13:34 +02:00
malkuthro 6f1025f66b fix(sidebar): preserve module links on insert (#102) 2026-05-06 19:08:57 +02:00
malkuthroandClaude Opus 4.7 413b153702 docs(maintainers): refresh stale references + add conventions.md
Audit triggered while debugging the v0.2.0 release surfaced drift in
the maintainer docs vs current code: dead curated_defaults.json
references (file replaced by starter_preset.json in PR #72), the
v1_0_1 fork example (removed in v0.1.5), and a "tooling we don't have
yet" entry that's now covered by tools/preflight_release.py. Adds a
new conventions.md for cross-cutting rules — currently the
Mermaid-for-diagrams convention.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-06 10:38:07 +02:00
malkuthroandClaude Opus 4.7 cbf2cb4d09 chore(ci): drop push-to-main publish trigger — dispatch-only (#91)
The push trigger fired on every release-PR squash-merge to main,
attempting to publish a version that workflow_dispatch on the prep
branch had already published — producing a 400 "node version already
exists" failure and a cosmetic red badge on the merge commit. With
the dispatch-from-prep-branch flow now standard (per CLAUDE.md and
releasing.md), the push trigger is redundant.

Updates docs/maintainers/releasing.md to match: drops the stale
"trade-off" callout (whose premise was the auto-publish on merge),
replaces the two anti-patterns whose rationale was the auto-trigger
with a single "don't forget to dispatch" reminder.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-06 10:34:41 +02:00
malkuthroandCursor ed6f3dad02 feat(sidebar): Modules — splice a saved cluster into the live canvas
Tag any saved workflow with the literal tag `module` and the Kforge
Labs sidebar starts treating it as a reusable building block: a green
`pi pi-plus-circle` icon replaces the file glyph, left-click
**inserts** the cluster at the viewport center (with internal links
re-created and the new nodes left selected), and a distinct hover
tooltip surfaces the changed semantics. Designed for the
`EXR-output stack` / `depth-only render` / `Wan 2.2 prompt
module` pattern: small clusters the maintainer wants to drop into
many workflows without rebuilding them by hand.

What changed:

1. New non-destructive primitive `insertWorkflowOntoCanvas` in
   `web/sidebar/canvas_io.js`. Pre-flights every referenced node type
   against `LiteGraph.registered_node_types` and aborts cleanly with a
   toast naming the missing types — a partial insert with stub nodes
   is worse than no insert. Then deep-clones the saved graph, calls
   `node.configure(...)` per node with `node.id = -1` so
   `app.graph.add()` allocates fresh ids that can never collide with
   the live canvas, and finally walks the saved `links[]` to
   recreate internal connections via `originNode.connect(...)` (which
   auto-allocates fresh link ids — no manual link remap needed). Bbox
   placement reuses the existing CSS-pixel correction so the cluster
   lands at the visible viewport center on HiDPI displays.

2. Right-click on any workflow row now offers `Insert into canvas`
   alongside the existing `Load` — Load remains available for every
   row regardless of tag, so a module can still be loaded as a full
   session if the user wants.

3. Convention-driven module classification in
   `web/sidebar/constants.js` (`MODULE_TAG = module`). Picked a
   tag instead of a new `isModule` schema field so existing saved
   entries can be promoted to modules with one right-click → Tags add,
   no migration needed, and the existing Tags section groups them for
   free. Tree rendering looks up the tag at row-render time
   (`isModuleWorkflow` helper) and switches both the leaf icon
   (green `pi pi-plus-circle`) and the left-click handler (Insert
   instead of Load). Active rows in the Workflows section AND every
   row under the Tags section honor the flip; archived rows stay on
   Load (they're old versions, `review` not `splice`).

4. `showSaveWorkflowModal` gains a `Save as module` checkbox. The
   selection-save toolbar button passes `defaultModule: true` (the
   typical `module` candidate is a small cluster the user just
   selected); the whole-canvas save passes `defaultModule: false`.
   The tag is added in the SAME `persistMutation` as the entry write
   (folded into the `mutate` callback in `saveAndToast`), so a
   commit failure rolls back BOTH the save and the tag together — no
   `module`-but-untagged ghosts.

Docs: new `Modules` section in
`docs/maintainers/workflows-sidebar.md` covering the convention, the
checkbox flow, per-surface behavior, and how to promote an existing
saved workflow to a module without re-saving.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-05 21:32:06 +02:00
malkuthroandClaude Opus 4.7 a6a31c8086 feat(sidebar): Tools row + starter preset replaces curated_defaults (#72)
* fix(sidebar): merge preset HEAD route into GET handler

PR #68 shipped koolook_routes.py with @routes.get + @routes.head on the
same path. aiohttp's add_get auto-registers HEAD by default, so the
explicit @routes.head collided with the auto-synthesized HEAD and
crashed ComfyUI startup with "Added route will never be executed,
method HEAD is already registered".

Trying allow_head=False on the GET to opt out triggered a second crash:
ComfyUI's server.py mirrors every PromptServer route into a parallel
/api/... table via api_routes.route(method, path)(handler, **kwargs),
and RouteTableDef.route's inner closure rejects unknown kwargs with
TypeError. So no kwarg-based opt-out is viable.

Drop the dedicated HEAD handler. Aiohttp's auto-HEAD-from-GET delivers
HEAD probes to get_preset; we branch on request.method to skip the
file read for HEAD — preserving the cheap-existence optimization the
client's presetExists relies on for multi-MB JSON over Dropbox / iCloud
/ NFS-mounted libraries. CHANGELOG [Unreleased] updated to match.

This bug never reached users — koolook_routes.py was added in #68
(post-v0.2.0) and was never published to Comfy Registry.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(sidebar): Tools row + starter preset replaces curated_defaults

Two coordinated UX changes for fresh-install distribution + admin
ergonomics — designed together and shipped as one unit.

1. New "Tools" row sits between the Snapshot row and the search field.
   Moves the Export and "Install missing for picks" buttons up out of
   the Nodes row (admin/advanced operations don't belong in the
   everyday + Add toolbar). Adds a new "Drop placeholders onto canvas"
   button — the security_level=normal escape hatch for installing
   missing packs: instantiates one placeholder per missing pack on a
   fresh canvas tab so ComfyUI/Manager's standard "Install Missing
   Custom Nodes" detection takes over via Manager's UI flow (which
   uses different gates than our programmatic /customnode/install/
   git_url call). Nodes row keeps only the everyday + button.

2. curated_defaults.json retired in favor of starter_preset.json.
   Fresh installs no longer get picks seeded directly into localStorage
   on first load. Instead the bundled web/starter_preset.json (full
   snapshot format — picks + workflows + tags + archive) is copied
   into the user's snapshot library directory as starter.json. The
   user opens Snapshot -> Load -> starter to apply in one click.
   Unifies fresh-install distribution with the per-user snapshot
   library (#68); maintainer flow becomes "build state in ComfyUI ->
   click the (down-arrow) Tools-row button -> paste over
   web/starter_preset.json -> commit." Existing users with non-empty
   picks are explicitly skipped by the new seeder, so their state is
   untouched.

Removed: seedDefaultsIfNeeded, exportPicks, web/curated_defaults.json,
SEEDED_KEY, DEFAULTS_URL.

Added: seedStarterPresetIfNeeded, exportStarterPreset (snapshot.js),
STARTER_SEEDED_KEY, STARTER_URL, STARTER_PRESET_FILENAME,
discoverMissingPacks (installer.js), dropPlaceholdersForPacks
(canvas_io.js), web/starter_preset.json.

Docs: CLAUDE.md "Curated Nodes sidebar — distributed defaults" ->
"Kforge Labs sidebar — starter preset distribution".
docs/maintainers/curated-sidebar.md rewritten.
CHANGELOG.md [Unreleased] updated.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-05 17:18:15 +02:00
malkuthroandClaude Opus 4.7 0d4c81295b chore(sync): --init flag for first-run dev-sync setup + Mac path docs (#61)
Background
----------
Setting up `scripts/sync_to_dev.py` on a fresh machine required two
pre-steps the script didn't help with: (1) editing `.env` to point at
a path that didn't exist yet, then (2) `mkdir -p` on that path before
the first sync would work. The script errored out cleanly with
"target does not exist" but didn't tell the user the obvious next
move was to create it. This was particularly silent on Mac where the
ComfyUI install path differs from the Windows examples in `.env.example`.

Changes
-------
- **`--init` flag** on `scripts/sync_to_dev.py`. When the target
  directory doesn't exist and `--init` is passed, the script creates
  it and proceeds with the sync. Without `--init`, it errors out with
  the same "first time on this machine?" hint pointing at the new flag.
- **Safety guards** on `--init`. Refuses (exit 3) if:
  - the parent directory doesn't exist, OR
  - the parent doesn't look like a ComfyUI `custom_nodes/` folder
    (heuristic: parent is named `custom_nodes` OR sits inside a
    directory named `ComfyUI`).
  This prevents a typo in `KOLOOK_COMFYUI_DEV_PATH` from quietly
  creating a `ComfyUI-Koolook/` somewhere unexpected on disk. The
  refusal message includes a manual escape hatch (`mkdir -p` + retry
  without `--init`) for the case where the parent really IS the
  ComfyUI install but doesn't match the heuristic.
- **Drop stale `k_easy_version.py`** from `RUNTIME_PATHS`. The file
  was deleted in v0.1.10 (PR #41); the script's `if not src.exists()`
  guard meant it was a no-op every run, but the entry was misleading.
- **`.env.example`** gains explicit macOS / Linux / Windows path
  examples for `KOLOOK_COMFYUI_DEV_PATH` and a 3-step first-run
  recipe (cp .env.example → edit → `--init`).
- **`docs/maintainers/dev-iteration-loop.md`** gains a "First-time
  setup on a new machine" section with the per-OS path table and the
  same 3-step recipe.

Verification
------------
- `python scripts/sync_to_dev.py --dry-run` against a missing target
  → exit 2, hint at `--init`.
- `python scripts/sync_to_dev.py --init` against a real ComfyUI
  `custom_nodes/` parent → creates the leaf, copies 9 entries, exits 0.
- `python scripts/sync_to_dev.py` (no flag) re-run after `--init` →
  re-syncs cleanly, idempotent.
- `python scripts/sync_to_dev.py --init` against `/tmp/not-custom-nodes`
  → exit 3, "parent does not exist".
- `python scripts/sync_to_dev.py --init` against an existing
  `/tmp/wrong-parent` (not custom_nodes) → exit 3, "doesn't look like
  a ComfyUI custom_nodes/ folder", manual escape hatch in the message.
- `python -c "import ast; ast.parse(open('scripts/sync_to_dev.py').read())"`
  → syntax OK.
- `python tools/preflight_release.py --skip manager-meta -v` → PASS.

No CHANGELOG entry — dev-tooling under `scripts/` doesn't ship to
users of the published package. Avoiding noise on the [Unreleased]
section that PRs #59 / #60 also touch.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-05 08:58:24 +02:00
malkuthroandClaude Opus 4.7 c024a94e38 feat(sidebar): recursive subdirectories under workflows + rebrand to Kforge Labs (#54)
* feat(sidebar): recursive subdirectories under workflows + rebrand to Kforge Labs

## Recursive subdirectories

Workflow directories can now host nested subdirectories at arbitrary
depth. Each nested directory behaves like a top-level one: it can hold
workflows, its own Archive subfolder, and further subdirectories.

- **Create**: right-click a directory → "Create subdirectory…"
- **Save into**: pick the path from the save modal's flat directory
  dropdown (`UP-scale / Type-A`)
- **Move across**: right-click a workflow → "→ <other path>" submenu
  lists every other path in the tree
- **Reserved name**: subdirectory names cannot be "Archive" (case-
  insensitive) at non-root levels — collides with the synthetic Archive
  folder rendering. Top-level "Archive" still allowed (root has no
  archived workflows of its own).

### Schema (recursive, back-compat)

  workflowsCache = { directories: { [name]: DirNode } }
  DirNode = { workflows: { [wfName]: WorkflowEntry }, directories: { [name]: DirNode } }

Pre-v0.3 stores without the nested `directories` field still load fine —
`normalizeWorkflowsStore` treats a missing `directories` as `{}` and
adds the empty field during normalization. No data loss on upgrade.

### Path-based store API

Every mutator/lookup now takes a `path: string[]`:

  addDirectory(parentPath, name)
  renameDirectory(parentPath, old, new)
  deleteDirectory(parentPath, name)
  saveWorkflowEntry(path, wfName, graph)
  archiveWorkflow(path, wfName) / unarchiveWorkflow(...)
  renameWorkflow(path, old, new) / deleteWorkflow(path, wfName)
  moveWorkflow(srcPath, wfName, dstPath)
  getWorkflowGraph(path, wfName)

New helpers: `listDirectoryNames(parentPath = [])`,
`listAllDirectoryPaths()` (DFS), `pathsEqual(a, b)`. `dirOf([])` returns
a synthetic root wrapper so callers don't need a special-case for root.

### UI changes

- **Save modal**: directory dropdown is now a flat list of every path in
  the tree (`JSON.stringify(path)` values, `path.join(" / ")` labels).
  "+ New directory…" creates a top-level dir; subdirs go via right-click.
- **Directory header counts**: now show recursive descendant total
  (active + archived + all subtree). A parent with empty direct
  workflows but populated subdirs no longer shows "0".
- **Save flow**: `saveAndToast` pins every ancestor (`workflows`,
  `workflows/seg1`, `workflows/seg1/seg2`, …) so a save into a deeply
  nested path opens the whole chain.
- **Render order per directory**: subdirs first → Archive folder →
  active workflows last (so the freshest version sits closest to the
  bottom).

## Rebrand: "Curated Nodes" → "Kforge Labs"

- `TAB_TITLE`: "Curated Nodes" → "Kforge Labs"
- `TAB_TOOLTIP`: "Curated selection of ComfyUI nodes" →
  "Kforge Labs — curated nodes & workflows"
- Canvas right-click menu item: "Add to Curated Sidebar" → "Add to Kforge Labs"
- Empty-state hint in the curated-nodes section follows the new label

`TAB_ID` (`koolook.curatedNodes`) and the `app.registerExtension` name
(`koolook.curated_sidebar`) are deliberately unchanged so existing
per-user tab state (pinning, tab order, idempotency sentinel) is
preserved.

## Files

- web/sidebar/constants.js (rebrand)
- web/sidebar/canvas_menu.js (rebrand)
- web/sidebar/canvas_io.js (loadWorkflowOntoCanvas takes path)
- web/sidebar/workflows_store.js (recursive schema + path-based API)
- web/sidebar/modals.js (save modal flat-path picker)
- web/sidebar/tree.js (gather recursion + recursive renderer + path-based context menus)
- docs/maintainers/workflows-sidebar.md (subdirectories section + recursive JSON shape)
- CHANGELOG.md ([Unreleased] entry)

Encapsulation invariants preserved: `workflowsCache` only in
`workflows_store.js`; `pathStates` only in `tree.js`.

Closes the maintainer's outstanding feature request for subfolders under
workflow directories.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sidebar): show empty directories in tree when no search is active

User-reported: right-click → Create subdirectory → name → Submit
appeared to do nothing. The directory was actually created in the cache
(and showed up correctly in the save modal's path picker), but
gatherDirsAt was dropping any directory with zero matched workflows AND
zero matched descendants — including freshly-created empty subdirs.

Fix: scope the empty-drop check to "search active" only. With no search,
every directory in the cache renders. With a search, the filter still
collapses paths that contributed no hits, so search results stay clean.

Verified via Node dry-test (.tmp_subdir_test.js, since deleted):
- Pre-fix: addDirectory(['UP-scale'], 'Test') succeeds; gather('') drops Test
- Post-fix: gather('') includes Test; gather('hires') still drops empties

Also confirmed there is NO case-sensitivity restriction — 'Test' and
'test' coexist as separate directories. The user's earlier observation
that "only the lowercase one shows" was the empty-dir bug masquerading
as a case issue (whichever directory had been saved into earlier was
visible; the other was hidden).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sidebar): cascading directory picker in save modal (multi-step)

User feedback: the flat path list was unusable with deep nesting — every
existing path appeared in one giant dropdown, you couldn't pick a parent
and then narrow to a child. Replaced with cascading selects.

Behavior:
- Top-level select shows existing top-level dirs + "+ New directory…"
- Picking a top-level with children reveals a level-2 select. The first
  option is "(save in <path>)" which stops drilling at this level.
  Following options are the children.
- Picking a level-2 child with grandchildren reveals a level-3 select
  with the same shape, etc., to arbitrary depth.
- "(save here)" at any level commits the save at that depth.
- Picking a different top-level / parent collapses every cascade level
  below it and rebuilds.
- `defaultDir` (a path array) auto-drills the cascade to that path on
  open, with onCascadeChange recursing through pre-selected levels.

The save modal's Base-on-existing / Action / Workflow-name sections
re-derive from `getSelectedPath()` whenever any cascade level changes,
so they reflect the current target dir.

Verified via Node dry-test (.tmp_cascade_test.js, since deleted) over 6
scenarios: fresh open, deep defaultDir pre-selection, level-2 collapse,
top-change-to-leafless-dir, "+ New directory…" + name input, manual
drill into a no-children leaf.

The store API is unchanged — `saveWorkflowEntry(path, wfName, graph)`
still takes a path array, and the cascade just produces the same array
through interactive selection instead of a flat dropdown.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(sidebar): right-click Archive folder → Delete archive (N items)

Adds a context menu to the synthetic Archive folder that bulk-removes
every archived workflow in its parent directory. Active workflows at the
same level are untouched. The menu only appears when archivedCount > 0
(Archive folder is synthetic and only renders when there are archived
workflows present), so the no-op path is unreachable in practice — the
clearArchive function still defends against it (returns false if no
archived entries to remove or directory missing).

User-asked gap from the subdirectories review. Other adjacent gaps
considered:
- Restore-all from archive: SKIP. Each archived entry has a
  "(archived <ts>)" name suffix; bulk-restoring leaves messy long names
  AND can collide with the still-active version. Per-item is the right
  granularity.
- Move a real Directory across paths: SKIP for this PR. Useful but needs
  cycle prevention + path-prefix updates; separate scope.
- Move workflow to a NEW directory inline: SKIP. Two-step workaround
  (right-click → Create subdirectory, then move) works.
- Move synthetic Archive folder: N/A. The Archive folder is auto-derived
  from `archived: true` flags; nothing to move.

Verified via Node dry-test (deleted): clearArchive correctly removes
only archived entries, preserves active ones, returns false on
empty-archive and missing-dir paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sidebar): pull 'Base on existing' candidates from ancestors too

Reported regression: navigating into an empty subdirectory (e.g.
UP-scale → seedvr2) disabled "Use existing name" / "Modify existing
name" because getActiveWorkflowsInCurrentDir only looked at the leaf
dir's workflows. With nested dirs, leaf dirs are often empty (just
created via right-click), making those actions unreachable for the
common "save a variant of an existing workflow into a new subdir" flow.

Fix: rename to getCandidatesForBase and walk the destination path from
deepest up to root, collecting active workflows at every level. Dedup
by name (deepest wins). The dropdown shows the bare name when the
candidate is from the current dir, or "name  ·  in <path>" when it
comes from an ancestor — so the user knows which dir they're basing on.

The submit path is unchanged: baseSelect.value is still the bare name,
which feeds the workflow-name field for "Use existing"/"Modify existing"
actions. The Save destination path is whatever the cascade resolves to,
not the ancestor path — so basing UP-scale/seedvr2's new save on
"Hires-FIX-wanT2V-v01  ·  in UP-scale" creates UP-scale/seedvr2/Hires-
FIX-wanT2V-v01 (no archive happens because the destination didn't have
that name before).

Verified via Node dry-test (deleted): empty subdirs now expose the
parent's active workflows; deep empties expose every ancestor's actives;
non-empty leaves show their own first then ancestors with path
disambiguation; archived entries excluded; alphabetical within each
level.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(sidebar): drag-and-drop moves in workflows tree (Tier 1)

User: "if we can fix this [DnD] then everything is fixed". Tier 1 ships
the moves-only piece — drag-to-move workflows, drag-to-nest dirs, drag
onto Archive to archive. No custom-ordering schema change (sort within a
level stays alphabetical); that would be Tier 2.

Drag/drop semantics:
  workflow row  → directory row    move workflow (no-op if same dir)
  workflow row  → Archive folder    archive in that dir (move first if
                                    cross-dir, all in one persist)
  directory row → directory row    nest dragged dir as child of target

Cycle prevention in moveDirectory: rejects same-parent, dropping a dir
into itself, or into any of its descendants. Plus collision (target
already has a sibling with that name) and reserved-name "Archive" at
non-root.

Implementation:
  - workflows_store.js: new moveDirectory(srcParentPath, name,
    dstParentPath) with isPathDescendantOrSame() helper.
  - constants.js CSS: grab/grabbing cursor on draggable rows; blue
    outline + tinted background on .koolook-drop-target.
  - tree.js: decorateDraggable(row, payload) and decorateDropTarget(row,
    target) helpers; handleDndDrop dispatcher; threaded through
    makeFolderRow / makeWorkflowLeafRow / buildFolder / makeSectionCtx
    so renderGatheredDir can attach DnD opts via the section ctx.

DnD uses the native HTML5 Drag-and-Drop API (no libs). MIME type
"application/x-koolook-row" filters out file/external drags.

Verified via Node dry-test (deleted) over 7 scenarios:
  1. legal cross-parent move (Type-A from UP-scale to Depth)
  2. cycle: parent → its child → blocked
  3. same-parent → blocked (no-op)
  4. cycle: dir → its own descendant → blocked
  5. name collision in destination → blocked
  6. legal sibling-becomes-child nest
  7. moving a dir literally named "Archive" to non-root → blocked

Visual feedback: drop targets get .koolook-drop-target on dragover
(blue outline + tinted background), removed on dragleave/drop. Failed
drops surface "Could not move (cycle, collision, …)" toast through the
existing persistMutation onNoOp path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sidebar): hide Action dropdown entirely when no base to act on

User feedback: when saving into a directory whose path has zero active
workflows in any ancestor (genuinely empty case), the Action dropdown
showed three options with "Use existing name" and "Modify existing
name" disabled. Disabled <option> renders subtly across browsers — most
users just see "New name" with no obvious why. They proposed two
options: (1) gray out clearly, or (2) hide the dropdown entirely. Going
with (2) — Action only matters when a base candidate exists; without
one, the only useful flow is "type a name and save," which the
Workflow Name field already provides.

So now:
- hasBase=true (current dir or any ancestor has active workflows) →
  full Action picker, all three options enabled.
- hasBase=false (truly empty path, all the way up to root) →
  Action label + dropdown hidden; underlying actionSelect.value pinned
  to "new" so submit takes the by-name path through the Workflow Name
  field.

The ancestor-pull fix from e7da864 means this empty branch only fires
for genuinely-fresh paths (a brand-new top-level + subdir created
together with no workflows anywhere). Common case is hasBase=true.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sidebar): apply verified PR-review findings on #54

Verified review (3 review agents → synthesizer → verifier) on the
recursive-subdirectories + Kforge Labs PR flagged 3 HIGH / 1 MEDIUM /
1 LOW. All confirmed; no false positives. Applying every actionable
fix in one commit.

[HIGH] CHANGELOG disclosure for two undocumented Save-modal shifts:
  1) Base on existing now walks ancestor paths leaf-up, dedupes deepest-
     wins, labels ancestor entries "<name>  ·  in <path>".
  2) Action dropdown is hidden (not just disabled) when no base
     candidate exists; underlying value pinned to "new" so submit takes
     the by-name path.
Both behaviors landed in late commits (e7da864 and cd9b2c3); the [Unreleased]
entry now describes them under ### Changed alongside the Kforge Labs
rebrand bullets.

[HIGH] Self-drop directory misleading toast (tree.js handleDndDrop):
The same-parent guard caught dir-on-its-own-parent but missed dir-on-
its-own-row, because the dir's row carries `target.path =
[...srcParentPath, name]` which doesn't equal `srcParentPath`.
moveDirectory's cycle check would still reject the move, but the user
got "Could not move directory (cycle, name collision, or invalid
target)" for what is just a no-op gesture. Adds a second early-return
matching `[...srcParentPath, dirName]` against `target.path` so
self-drops are silent.

[MEDIUM] addDirectory(undefined, "x") crash hardening: moveDirectory
already had `Array.isArray` guards on its parent-path inputs;
addDirectory lacked the equivalent and would throw on
`parentPath.length` if a future caller ever passed `undefined`. Adds
the symmetric guard.

[LOW] Drops the dead `isArchiveReservedName` export — the predicate
was never imported. Reserved-name enforcement remains inline in every
mutator that creates or renames a directory. A short comment notes the
removal so future contributors know to revive the export only when a
caller (e.g. modal pre-validation) actually needs it.

Verified via Node dry-test (deleted) covering:
  - addDirectory(undefined / null / "string" / []) — all behave correctly
  - Self-drop on root 'A' → "no-op-self-drop" (silent)
  - Legal cross-parent move 'A' → 'B' → "moved"
  - Self-drop on nested 'B/A' → "no-op-self-drop"
  - Drop 'B/A' onto own parent 'B' → "no-op-same-parent"

Spec alignment: every fix was verified against PR intent. The two
Save-modal shifts are organic late-stage refinements, not regressions —
disclosure is the only ask and is now delivered. The self-drop guard
and Array.isArray hardening tighten contracts that were already the
clear intent. The dead-export drop is housekeeping.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 20:33:54 +02:00
malkuthroandClaude Opus 4.7 24dee7ee1b chore(skills): rename project /menu to /menu-comfy to avoid global collision (#53)
The project's `menu` skill at `.claude/skills/menu/` shared the same `/menu`
trigger as the user-level `~/.claude/skills/menu/` skill, so the global one
won and the project inventory never surfaced. Renamed the project skill to
`menu-comfy` so both can coexist.

- `git mv .claude/skills/menu .claude/skills/menu-comfy`
- Frontmatter `name`, header, trigger phrases, output example, and procedure-
  step reminder all switched to `menu-comfy` / `/menu-comfy`.
- Two intentional references to the global `/menu` retained — they document
  the deliberate split so future contributors don't "fix" them back.
- `docs/maintainers/README.md` companion-skills table row updated, with a
  one-line note about the collision split.

Behavior change: `/menu-comfy` now lists project skills; `/menu` continues
to list the global curated set. Docs-only — no runtime/code change.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 18:07:57 +02:00
malkuthroandClaude Opus 4.7 e03523faf9 refactor(web): split koolook_sidebar.js into 7 sidebar modules (#51)
* refactor(web): split koolook_sidebar.js into 7 sidebar modules

Pure mechanical split — zero behavior change. Implements the design from
issue #49 (common-case layout with patchCanvasMenu extracted).

Old: web/koolook_sidebar.js (1973 lines, monolithic)
New: web/koolook_sidebar.js (48-line entry) + 7 modules in web/sidebar/

  - constants.js (130) — REPOS, TAB_*, storage keys, event names, URLs,
    ensureStyle, toast, compareNames
  - picks_store.js (117) — localStorage user picks: load/save/add (tri-state)
    /remove/notify/seed/export
  - workflows_store.js (347) — workflowsCache + persistMutation + snapshotCache
    + commit + persist*/load*/seed* + in-memory ops; cache is module-private
  - canvas_io.js (192) — serialize*/canvasIsNonEmpty/loadWorkflowOntoCanvas
    /insertNode/getSelectedNodeTypes
  - modals.js (384) — makeModalShell + showInputModal/showConfirmModal
    /showSaveWorkflowModal/showContextMenu
  - canvas_menu.js (45) — patchCanvasMenu (idempotent LGraphCanvas patch)
  - tree.js (825) — pathStates (module-private) + gatherers + DOM row
    factories + buildFolder + workflow/dir context menus + rebuildTree
    + renderPanel

Encapsulation invariants verified:
  - workflowsCache: zero references outside workflows_store.js
  - pathStates: zero references outside tree.js
  - All 8 files pass node --check
  - CSS string byte-identical to original
  - URL constants resolve to same targets after the extra `..` adjustment

Documentation updated: dev-iteration-loop.md and workflows-sidebar.md
now point pathStates references at web/sidebar/tree.js. Other doc
references to koolook_sidebar.js as the entry remain accurate.

Closes #49.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(web): drop unreachable defensive guard in gatherWorkflows

Review feedback on #51: the `if (!dir) continue;` after `dirOf(dirName)`
is unreachable — `listDirectoryNames()` enumerates the same module-private
`workflowsCache.directories` keys that `dirOf()` reads, synchronously,
within a single event-loop turn. Removing the guard restores strict
"byte-identical move" semantics for the file-split refactor.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 17:47:20 +02:00
malkuthroandClaude Opus 4.7 cc6bc47866 docs+tooling: VAE user guide + versioning ref + 3 skills + preflight CI (#48)
* docs+tooling: VAE user guide + versioning ref + 3 skills + preflight CI

Documentation
- New user-guide tree: docs/user_guide/nodes/radiance_koolook_v2_3_3/
  with README, per-node pages for Easy_hdr_VAE_encode/decode, an
  encode/decode pairing guide (4 worked examples + ASCII connectivity
  diagram), and an HDR modes deep-dive (per-pixel comparison table).
  Folder name mirrors the code path under forks/radiance_koolook/versions/.
- New cross-cutting reference: docs/reference/versioning.md explains the
  three independent version axes (pack version, fork wrapper version,
  upstream pinned commit), when each one bumps, and which skill/doc
  owns it. Plus the v1/v2 node-ID suffix policy.
- Cross-links updated in docs/user_guide/README.md, docs/reference/
  README.md, docs/maintainers/README.md.

Skills (all under .claude/skills/)
- docs-sync — controlled find-and-replace for version tags across docs,
  code, and manifests; --scope and --dry-run flags.
- menu — lists all project skills with one-liners.
- preflight-release — companion doc for tools/preflight_release.py.

Pre-flight tooling
- tools/preflight_release.py — four checks, exit-code-driven for CI:
  (1) static AST extraction of NODE_CLASS_MAPPINGS keys (no torch needed),
  (2) stub-VAE dispatch test verifying all 5 rank/VAE-type branches,
  (3) drift check against ComfyUI-Manager's extension-node-map.json
      (catches the issue #44 phantom-nodes class of bug automatically),
  (4) saved-workflow regression check across tests/workflows/*.json.
- .github/workflows/ci.yml — new "Pre-flight Release" job runs checks
  1, 2, and 4 on every PR; check 3 (manager-meta) skipped pending the
  upstream issue #44 fix.
- tests/workflows/ — 2 starter fixtures (Depth-Anything3, Hires-FIX-wanT2V)
  + README documenting the contribution convention. Coverage grows as
  new fixtures are added; no code changes needed.

All four offline pre-flight checks pass on this branch. The
manager-meta check correctly identifies the 39 phantom IDs documented
in issue #44.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(preflight): satisfy ruff lint + bandit security gate

* fix(preflight): address blocking review findings

Three fixes from /review-pr feedback on this PR:

1. Sanitize tests/workflows/*.json — removed 4 unique absolute paths that
   leaked internal job/client info ('baconx/Toy-Hix/9110',
   'menneskehavn_3029', etc.). Replaced with placeholders like
   'path/to/sequence_a' and 'path/to/source_video'. Both fixtures still
   parse as valid JSON; grep for the old patterns returns zero hits.

2. Broaden the Koolook-ID heuristic regex from '^easy_[a-z]' to
   '^easy_[A-Za-z]'. The old pattern excluded 'easy_ImageBatch' (capital I
   after the underscore), which would have silently bypassed the
   workflow-fixture stability check for that node. Verified the new
   pattern correctly matches it.

3. Wrap check_dispatch() body in try/finally and track every sys.modules
   key the function injects in a list, so cleanup runs on the success
   path AND on any exception during exec_module / VAE-constructor
   execution. Previously only the three torch* stubs were popped, never
   the three _pf_pkg* entries — and even those weren't popped on
   exception paths. Verified zero leaked modules after the function
   returns.

The five dispatch test cases were factored into a helper
_check_dispatch_run_cases() so the try/finally wrapping stays focused on
module injection.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(preflight): address remaining review findings

Follow-up to 7844037 — picks up all the HIGH / MEDIUM / LOW review
findings that were deferred. Pre-flight still passes locally.

HIGH
- Extract check_dispatch (~195 lines) to tools/_preflight_dispatch.py.
  Main script drops from 680 → 508 lines. Module exposes a single
  run(verbose, repo_root) -> (bool, list[str]) entry; main script keeps
  the [2/4] header + PASS/FAIL line.
- Install _HttpsOnlyRedirectHandler that re-validates each redirect
  hop's scheme. Closes the gap that urlopen would otherwise follow a
  30x to http:// or file:// silently. The Bandit B310 suppression
  comment now accurately describes the protection.
- check_static docstring now honestly states it only walks
  NODE_CLASS_MAPPINGS literal dicts and does NOT apply
  NAMESPACE_SUFFIX / SKIP_VERSION_SUFFIX logic. Notes that the workflow
  check (4) is the safety net that surfaces drift the day a non-skipped
  ID lands.
- check_workflows now FAILS when the fixtures dir exists with a README
  but contains no *.json files — previously silently SKIPPED. If
  someone deletes fixtures or typoes the dir, that's a regression in
  test coverage and should fail loudly.

MEDIUM
- check_manager_meta validates the upstream entry shape before diffing.
  Previous code silently produced an empty set if the schema changed,
  reporting every one of our IDs as 'missing'. Now SKIPs with a clear
  message when shape is unexpected.
- check_workflows accumulates total_koolook_refs inside the main loop;
  drops the redundant second-pass that re-read every fixture file just
  to compute the summary count.
- _extract_workflow_node_types uses explicit detection: a canvas-format
  file with nodes: null no longer falls through to API-format and pulls
  class_type from unrelated keys.
- check_static return type simplified from (bool, set, list) to
  (bool, set). Third element was discarded at every call site.
- check_workflows now does two-pronged Koolook-ID matching: heuristic
  (catches drift even if static missed an ID) plus actual_ids (catches
  IDs that don't fit the heuristic, e.g. a future Koolook node without
  Easy / easy_ / koolook in its name).

LOW
- ANSI escape codes named in a constants dict (_ANSI["green"] etc.)
  instead of bare string literals.
- Removed dead 'cases: list[tuple[str, callable]] = []' initializer
  (was shadowed three statements later).
- check_static's per-ID listing now gated behind --verbose, matching
  check_dispatch and check_workflows.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 17:31:36 +02:00
malkuthroandClaude Opus 4.7 b7b34de7e2 feat(sidebar): curated nodes + workflows tab with archive (#45)
* feat(sidebar): add curated nodes sidebar tab with favorites

New "Curated Nodes" sidebar tab. Shows Koolook nodes auto-grouped by
subcategory under a "Nodes (favorites)" root. Users add favorites from
any installed pack from inside ComfyUI via:

- "+" button: adds the currently selected canvas node(s)
- Right-click any canvas node -> "Add to Curated Sidebar"

Picks persist in browser localStorage (per-browser, survives ComfyUI
restarts). Hover any entry -> "x" to remove. Search filter auto-expands
all folders so matches stay visible; pack/category folders are
collapsed by default otherwise to keep the panel compact.

"Download" export button copies current picks as JSON suitable for
web/curated_defaults.json -- committed to ship as the package's
defaults. On first ComfyUI load with empty localStorage, the seeder
reads curated_defaults.json into localStorage so fresh installs get the
curated set pre-loaded. Existing users keep their localStorage
untouched.

Pure JS, no build step. Uses the documented
app.extensionManager.registerSidebarTab API (stable since
ComfyUI_frontend v1.2.4).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(sidebar): document curated-defaults workflow + seed initial 9 picks

Adds docs/maintainers/curated-sidebar.md covering the full Export ->
paste -> commit flow for shipping distributed favorites with the
package, including storage-key reference, reset procedure, and the
intentional "seed once per browser" semantics.

CLAUDE.md gets a short pointer so the trigger ("update curated
defaults" or pasting an exported picks JSON) is discoverable on the
agent side too. Maintainer docs index linked.

Seeds web/curated_defaults.json with the maintainer's first 9 picks
(GetImageSizeAndCount, GetNode, ImageScaleBy, LoadEXR, NukeConstant,
NukeOCIOColorSpace, SaveEXRFrames, Text Multiline, easy showAnything).
Fresh installs from this version forward will get them pre-loaded into
their localStorage on first ComfyUI launch; existing users' picks are
untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(sidebar): trim curated-sidebar.md to a one-screen quick reference

Drops the verbose data-flow diagram and the localStorage-vs-userdata
rationale. Keeps the four action steps, the storage-key map, the reset
block, and the four "things that surprised us" footnotes capturing the
non-obvious bits (paste is manual, other users can't reach the repo,
seed-once-per-browser, no publish on web/ pushes). 81 lines removed,
20 added; same file path so existing links still resolve.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(sidebar): workflows section with archive + iteration docs

Adds a Workflows section to the Curated Nodes sidebar tab, sharing the
search field and tree pattern with the existing nodes feature.

Capabilities:
- Save entire canvas or current selection as a workflow.
- Save modal: New name / Use existing name (archive previous) /
  Modify existing name. The Workflow-name field is shown only when
  needed; "Base on existing" dropdown drives prefill for Use/Modify.
- Auto-archive on same-name save: previous version renamed with
  timestamp suffix and flagged archived; new entry takes the slot.
  Archive sub-folder appears in each directory only when populated and
  is rendered above the active list, so the latest sits at the bottom.
- Right-click on workflow: Load / Rename / Move to other directory /
  Move to archive (or Restore from archive) / Delete.
- Right-click on directory: Rename / Delete (with confirm if non-empty).
- Loading a workflow names the tab via loadGraphData's 4th arg
  (verified against ComfyUI_frontend src/scripts/app.ts) so the tab
  flips from "Unsaved Workflow (N)" to the saved name.

Storage: ComfyUI /userdata/koolook_workflows.json (primary), with a
localStorage fallback if /userdata is unreachable. Distributed defaults
seeded from web/workflow_defaults.json on first ComfyUI load with empty
workflow data -- same one-time-per-install pattern as
curated_defaults.json.

Other panel changes:
- Three labeled rows above the tree (search; NODES action bar;
  WORKFLOWS action bar), with thin dividers between them and before
  the tree.
- Folder expansion state preserved across re-renders via a pathStates
  Map; saving never collapses the directory you were viewing.
- Alphabetical (case-insensitive) sort everywhere via a single
  compareNames helper -- pack names, subcategories, directories,
  workflow names, dropdown options.
- "Use existing name (archive previous)" action label includes the
  parenthetical hint about archive behavior.

Docs:
- New docs/maintainers/dev-iteration-loop.md captures the
  dev-sync->restart->verify pattern, agent trigger phrases, persistence
  boundaries, and push/publish gates. First read when returning to UI
  work after a break.
- New docs/maintainers/workflows-sidebar.md covers workflow storage,
  action semantics, archive behavior, and reset procedure.
- docs/maintainers/README.md indexes both.
- CLAUDE.md "dev-sync" section now points at the iteration-loop doc.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sidebar): address review findings — seed gating + commit checks

Targets the data-integrity findings from the multi-agent PR review:

- BLOCKING: seed flag is now only set after the underlying save/persist
  succeeded. `saveUserPicks` returns a boolean; both seeders bail out
  (leaving the flag unset for next-load retry) on storage failure.
  Previously a transient storage failure during first-load could
  permanently strand a user with an empty store and the seeded flag set.

- BLOCKING: `normalizeWorkflowsStore` now strictly validates each
  workflow entry — drops entries missing a graph object, coerces
  `archived` to a strict boolean (so a stray "false" string can't
  accidentally hide a workflow), and logs the dropped count.

- HIGH: save handlers now check `commit()`'s return value and surface a
  "Save failed" toast when both /userdata and the localStorage fallback
  rejected the write. Previously the user saw "Saved" even though
  nothing was persisted. The canvas and selection handlers share a
  `saveAndToast` helper to keep the failure path consistent.

- HIGH: `saveCanvasBtn` now checks `canvasIsNonEmpty()` before
  serializing. A `serialize()` throw is no longer misreported as
  "Canvas is empty." — distinct toasts for distinct failure modes.

- HIGH: `loadWorkflowsStore` now logs (instead of silently swallowing)
  parse failures on the localStorage fallback, and warns when /userdata
  is reachable but a stale fallback blob still exists from an earlier
  outage — gives the user a recovery breadcrumb.

- HIGH: `loadUserPicks` logs parse failures so a corrupt picks blob is
  visible in the console before the next save overwrites it.

Skipped from the review with rationale:
- multi-tab seed race: sub-second window, both tabs write identical
  defaults, no observable wrong state — navigator.locks not worth it
- splitting `koolook_sidebar.js` and decomposing `renderPanel`:
  refactor scope, deferred to follow-up
- `loadGraphData` rename comparison: agent's claim was wrong; existing
  `wf.filename !== wfName` matches the upstream `filename` shape (bare
  name, no extension) we verified earlier against ComfyUI_frontend
  v1.44.x source

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sidebar): second-pass review fixes — rollback, idempotency, corrupt-file handling

Addresses the residual findings from the post-625aa97 review pass.

BLOCKING:
- patchCanvasMenu is now idempotent. Adds a __koolookCuratedPatched
  sentinel on LGraphCanvas.prototype so a soft-refresh / re-import
  doesn't stack duplicate "Add to Curated Sidebar" entries on every
  node's right-click menu. (LGraphCanvas is shared with every other
  pack, so non-idempotent prototype mutation was visible globally.)

HIGH:
- persistWorkflowsToServer now returns "server" | "fallback" | false
  instead of just true/false. seedWorkflowDefaultsIfNeeded only marks
  WORKFLOWS_SEEDED_KEY when the seed actually reached /userdata —
  previously a fallback-only seed (when /userdata was unreachable on
  first install) marked the user as "seeded" and they would have an
  empty server file forever after recovery.

- All 8 commit() callsites that mutate workflowsCache (saveAndToast,
  move, archive, restore, rename workflow, delete workflow, rename
  directory, delete directory + delete-non-empty-directory, create
  directory) now go through a new persistMutation helper that:
  - snapshots the cache before mutation
  - bails (toast no-op) if mutate returns false
  - rolls back the cache + re-renders if commit() returns false
  - toasts persistFailedMessage so the user isn't told a write
    succeeded when both /userdata and the localStorage fallback
    rejected it
  Pairs with snapshotCache() helper for explicit snapshot/restore.

- addToMyPicks tri-state ("added" | "duplicate" | "failed") so callers
  can distinguish "already in favorites" from "save failed" on
  localStorage quota / private-mode failure. Both call sites (the +
  toolbar button and the right-click "Add to Curated Sidebar" menu
  entry) updated to handle all three states.

- fetchWorkflowsFromServer now distinguishes a network failure from a
  parse failure of an existing-but-corrupt /userdata file. Returns
  SERVER_FILE_CORRUPT (a Symbol) on parse error; loadWorkflowsStore
  surfaces a toast and returns { corrupt: true }; setup() skips
  seedWorkflowDefaultsIfNeeded entirely so we don't auto-overwrite a
  recoverable-but-unreadable file with stock defaults.

- loadWorkflowOntoCanvas snapshots the current canvas before
  app.loadGraphData and restores it on throw. Previously a workflow
  with a now-unregistered node type could destroy the canvas partway
  through and leave the user with neither the old graph nor the new.

- Stale localStorage fallback detection (when /userdata is reachable
  but a fallback blob from a previous outage is still present) now
  surfaces a toast in addition to the console warning, so a user who
  doesn't have DevTools open gets a recovery breadcrumb.

MEDIUM:
- Modal escape-handler leak fixed via centralized close(). Every
  dismissal path (Escape key, overlay click, Cancel/OK calling
  overlay.remove()) now tears down the document-level keydown listener.
  Existing call sites that use overlay.remove() keep working — the
  remove method itself routes through cleanup.

Skipped from the review with rationale:
- subcategoryFor cosmetic mismatch fallback at L1000-1008 — silent
  rewrite is rare-path defensive, not load-bearing; defer.
- Optional toast on seedDefaultsIfNeeded storage failure — console
  warning is sufficient for a development-time scenario; defer.
- File size / module split (1864 lines) — refactor scope, deferred to
  follow-up per prior commit message.
- Magic sentinel strings ("__new__", action enum strings) — pure
  cleanup; defer.
- dataset.expanded boolean-string compare — pathStates Map already
  carries booleans; cleanup defer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 16:33:53 +02:00
malkuthroandClaude Opus 4.7 ee8ccefdf8 docs(reorg) + chore: roll [Unreleased] forward + bump 0.1.7 (#37)
Bundles two changes for v0.1.7:

1. PR #36 (already on main, just rolling [Unreleased] -> [0.1.7]):
   suffix all node display names with " (Koolook)" so they group
   together in ComfyUI's node-add search. Class IDs unchanged.

2. Docs reorg:
   - git mv Glossary.md            -> docs/reference/glossary.md
   - git mv RELEASING.md           -> docs/maintainers/releasing.md
   - Three audience buckets under docs/: user_guide/ (end users),
     reference/ (lookups), maintainers/ (project ops). Each gets a
     short index README.md.
   - Top-level docs/README.md as the navigation entry point.
   - New: docs/maintainers/registry-api.md captures the Comfy Registry
     undocumented version-management API (deprecate/yank/status enum,
     auto-deprecation behavior, curl recipes, optional GH Actions
     workflow YAML for codified mgmt without exposing the token).
   - New: docs/maintainers/node-versioning.md codifies the rules from
     the v0.1.5 PR-review discussion for changing INPUT_TYPES /
     RETURN_TYPES / class names without breaking saved workflows.
   - Imported the existing untracked docs/ images that the maintainer
     started locally (node inventory screenshot, Easy Image Batch
     helper image).
   - Cross-references updated in README.md, CLAUDE.md, the issue
     template, and .claude/skills/add-external-fork/SKILL.md.

Root drops from 6 -> 4 markdown files. The four that stay (README,
LICENSE, CHANGELOG, CLAUDE.md) are the ones that *must* live there for
GitHub / Comfy Registry / tooling / Claude Code reasons.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-03 23:26:46 +02:00
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
2026-04-24 15:45:04 +02:00
kflame99 7c34d902f4 Address cleanup feedback: ignore RunPod re-additions and tidy changelog 2026-02-15 15:10:57 +00:00
kflame99 075ef223e8 Remove RunPod scaffold from public repo after private migration 2026-02-15 14:54:53 +00:00
malkuthro 39fb15f549 chore: add release templates and expand third-party manifest 2026-02-14 22:49:32 +00:00
malkuthro 7e159780fe chore: add release workflow and third-party tracking scaffold 2026-02-14 22:42:12 +00:00