Files
malkuthro-ComfyUI-Koolook/docs/maintainers/dev-iteration-loop.md
T
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

6.9 KiB
Raw Blame History

Iterative dev loop — sidebar work

How the Curated Nodes / Workflows sidebar was built, in case you come back to it after a break. Optimized for fast iteration without the publish round-trip.

First-time setup on a new machine

KOLOOK_COMFYUI_DEV_PATH should point at the eventual Koolook subdirectory inside your live ComfyUI custom_nodes/ folder — not the custom_nodes/ parent itself.

Platform Typical path
macOS /Volumes/Data/ComfyUI/custom_nodes/ComfyUI-Koolook
Linux /home/<user>/ComfyUI/custom_nodes/ComfyUI-Koolook
Windows C:/ComfyUI_portable/ComfyUI/custom_nodes/ComfyUI-Koolook (forward slashes work fine)
cp .env.example .env                            # if you don't have one yet
$EDITOR .env                                    # set KOLOOK_COMFYUI_DEV_PATH
python scripts/sync_to_dev.py --init            # creates the leaf folder + first sync
# restart ComfyUI to pick up the new install

After that first run, drop the --init — plain python scripts/sync_to_dev.py is enough for every subsequent edit. The --init flag is a one-shot guard against typos: it refuses to create a Koolook folder unless the parent looks like a ComfyUI custom_nodes/ directory (named custom_nodes or sitting inside a ComfyUI parent).

The loop

  1. Edit web/koolook_sidebar.js or any module under web/sidebar/ (or related JSON) in the worktree.
  2. python scripts/sync_to_dev.py — copies runtime files into your live ComfyUI custom_nodes/<koolook>/. Reads KOLOOK_COMFYUI_DEV_PATH from .env.
  3. Restart ComfyUI; verify visually.
  4. If a design mockup exists for what you just implemented (look under docs/designs/), screenshot the running result and diff against the mockup. Don't request review until visual parity is reached or each deviation is documented. See the project CLAUDE.md "Visual verification for design-driven implementation" section for the full rule.
  5. Iterate.

End-to-end: ~10–30 seconds. No tag/publish round-trip.

Trigger phrases when working with the agent

You say Agent does
dev-sync (or "sync to dev", "copy those files") runs scripts/sync_to_dev.py
go implements the most-recently proposed plan as code
paste exported JSON from the ↓ button replaces the matching web/<feature>_defaults.json

What survives across re-syncs and ComfyUI restarts

Data Lives in Persistence
Node favorites (per user) localStorage["koolook.curated.userPicks.v1"] survives JS updates, ComfyUI restarts, browser refreshes
Saved workflows incl. archive (per install) ComfyUI /userdata/koolook_workflows.json survives JS updates and ComfyUI restarts
Distributed defaults — starter preset web/starter_preset.json (in repo) copied once into the user's snapshot library as starter.json on first ComfyUI load (replaces the legacy curated_defaults.json localStorage seed)
Distributed defaults — workflows web/workflow_defaults.json (in repo) seeded once into /userdata on first ComfyUI load

dev-sync + restart never wipes user state. Only localStorage.clear(), deleting /userdata/koolook_workflows.json, or wiping browser data does.

Commit / push / publish gates

  • Commit after a stable batch of working changes — not after every edit.
  • Pushing a feature/PR branch never triggers a Comfy Registry publish.
  • Publish fires only when pyproject.toml changes and the push lands on main. Full procedure: releasing.md.

Tips that paid off in this build

  • Annotated screenshots (a red arrow on the spot you're talking about) beat text descriptions for UI feedback every time.
  • Verify uncertain frontend APIs against Comfy-Org/ComfyUI_frontend source before guessing. E.g. the loadGraphData(graph, true, true, wfName, {}) 4th-arg trick that names the workflow tab was pulled from upstream src/scripts/app.ts, not guessed.
  • Defensive fallbacks when calling a newer ComfyUI API surface — multiple property paths inside try/catch so older frontends still work.
  • Persist folder expansion state across re-renders so saving doesn't collapse the user's view (the pathStates Map in web/sidebar/tree.js).
  • For frontend preview buttons, inspect the virtual-node implementation before guessing. ComfyUI custom nodes such as EasyUse GET/SET can be frontend-only tunnels: render-time Python receives the resolved value, but a JS preview button may only see the visible GET key unless it follows the tunnel itself. Read the installed custom node JS, then simulate the relevant LiteGraph shape locally before asking the maintainer to test.

Debugging frontend preview buttons

Frontend preview buttons, such as Easy AI Pipeline's Get output file path, are not the same execution path as the node's Python outputs. The button code runs in browser JS and often peeks at connected widgets; the real render path runs through ComfyUI execution and may resolve virtual or dynamic nodes that the preview cannot see by default.

When a preview disagrees with render-time output:

  1. Confirm whether the fix is Python, JS, or both. Python changes need a ComfyUI restart; JS changes need the browser tab to reload the extension module. Restarting ComfyUI alone does not guarantee an already-open tab has re-imported web/*.js.
  2. Verify the served extension file directly, e.g. http://127.0.0.1:8188/extensions/koolook/ai_pipeline.js, and add a short temporary marker when needed so the loaded frontend version is provable.
  3. Inspect the connected custom node's frontend source in the live custom_nodes/ install. EasyUse GET/SET nodes live in comfyui-easy-use/web_version/v1/js/getset.js.
  4. If the connected node is a virtual tunnel, follow its graph semantics manually. EasyUse easy getNode stores the visible key in its Constant widget; the actual value lives upstream of the matching easy setNode input link.
  5. Build a small local JS simulation of the LiteGraph shape (GET -> SET -> source) before dev-syncing another guess.

Do not assume the in-browser automation can always read app.graph from the modern ComfyUI frontend. If the graph object is not exposed, use source inspection, served-file checks, and small simulations instead.

Files this loop touches most