Commit Graph
3 Commits
Author SHA1 Message Date
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 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 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