refactor: M1 polish pass (I2, I3, I5, I7)
Four minor improvements flagged in the M1 holistic review: - I7: Extract core/image_ops.py with arr_to_pil / pil_to_arr / hex_to_rgb. Removes the three duplicated copies scattered across compositor, batch_generator, and page_composer. Sets up cleaner reuse for M2/M3 when validator and polygon mask code need the same helpers. - I5: Move `import torch` out of module top-level in both node files, into the methods that actually use it. Keeps the ComfyUI node scan cheap — torch is the heaviest dependency by far. - I3: Rename the `retries` widget to `max_attempts` (default 3, min 1). The old name was misleading: retries=2 meant 3 total attempts. Log lines now read "attempt N/M" for clarity. - I2: Add manifest_version: 1 to cache manifests. manifest_matches() now treats missing/mismatched versions as stale, so a future hash schema change can safely invalidate old caches by bumping the constant in core/cache_manager.py. Tests updated to new kwargs, plus 2 new regression tests covering the manifest-version behaviour. 74/74 pass, ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -8,6 +8,11 @@ from pathlib import Path
|
||||
|
||||
from .types import SolvedPanel
|
||||
|
||||
MANIFEST_VERSION = 1
|
||||
"""Bump when the panel_hash input schema or manifest layout changes in a way
|
||||
that invalidates older caches. manifest_matches() treats version mismatch as
|
||||
a miss so stale manifests are regenerated safely."""
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class PanelPaths:
|
||||
@@ -45,7 +50,7 @@ def panel_paths(out_dir: Path, panel: SolvedPanel) -> PanelPaths:
|
||||
|
||||
def write_manifest(path: Path, hash_hex: str, extra: dict | None = None) -> None:
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
payload = {"hash": hash_hex}
|
||||
payload = {"manifest_version": MANIFEST_VERSION, "hash": hash_hex}
|
||||
if extra:
|
||||
payload.update(extra)
|
||||
path.write_text(json.dumps(payload, indent=2), encoding="utf-8")
|
||||
@@ -62,4 +67,8 @@ def manifest_matches(manifest_path: Path, panel: SolvedPanel) -> bool:
|
||||
m = read_manifest(manifest_path)
|
||||
except (OSError, json.JSONDecodeError):
|
||||
return False
|
||||
# Older manifests without the version field are treated as stale —
|
||||
# hash semantics may have shifted between releases.
|
||||
if m.get("manifest_version") != MANIFEST_VERSION:
|
||||
return False
|
||||
return m.get("hash") == panel_hash(panel)
|
||||
|
||||
@@ -6,20 +6,10 @@ from collections.abc import Iterable
|
||||
import numpy as np
|
||||
from PIL import Image, ImageDraw
|
||||
|
||||
from .image_ops import arr_to_pil, hex_to_rgb
|
||||
from .types import SolvedPage, SolvedPanel
|
||||
|
||||
|
||||
def _hex_to_rgb(hex_str: str) -> tuple[int, int, int]:
|
||||
h = hex_str.lstrip("#")
|
||||
return (int(h[0:2], 16), int(h[2:4], 16), int(h[4:6], 16))
|
||||
|
||||
|
||||
def _arr_to_pil(arr: np.ndarray) -> Image.Image:
|
||||
if arr.dtype != np.uint8:
|
||||
arr = np.clip(arr * 255.0, 0, 255).astype(np.uint8)
|
||||
return Image.fromarray(arr, mode="RGB")
|
||||
|
||||
|
||||
def compose_page(
|
||||
page: SolvedPage,
|
||||
panel_images: Iterable[tuple[SolvedPanel, np.ndarray]],
|
||||
@@ -28,10 +18,10 @@ def compose_page(
|
||||
|
||||
`panel_images` yields (SolvedPanel, HxWx3 float32 in [0,1] or uint8) pairs.
|
||||
"""
|
||||
canvas = Image.new("RGB", (page.page_width, page.page_height), _hex_to_rgb(page.background))
|
||||
canvas = Image.new("RGB", (page.page_width, page.page_height), hex_to_rgb(page.background))
|
||||
|
||||
for panel, img_arr in panel_images:
|
||||
pil = _arr_to_pil(img_arr)
|
||||
pil = arr_to_pil(img_arr)
|
||||
# In M1 every panel is rect; paste directly.
|
||||
canvas.paste(pil, panel.bbox_topleft)
|
||||
|
||||
@@ -39,7 +29,7 @@ def compose_page(
|
||||
draw = ImageDraw.Draw(canvas)
|
||||
x0, y0 = panel.bbox_topleft
|
||||
w, h = panel.bbox_size
|
||||
color = _hex_to_rgb(panel.border["color"])
|
||||
color = hex_to_rgb(panel.border["color"])
|
||||
width = panel.border["width_px"]
|
||||
draw.rectangle([x0, y0, x0 + w - 1, y0 + h - 1], outline=color, width=width)
|
||||
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
"""Shared image conversion helpers used across core and nodes."""
|
||||
from __future__ import annotations
|
||||
|
||||
import numpy as np
|
||||
from PIL import Image
|
||||
|
||||
|
||||
def hex_to_rgb(hex_str: str) -> tuple[int, int, int]:
|
||||
h = hex_str.lstrip("#")
|
||||
return (int(h[0:2], 16), int(h[2:4], 16), int(h[4:6], 16))
|
||||
|
||||
|
||||
def arr_to_pil(arr: np.ndarray) -> Image.Image:
|
||||
"""Convert an HxWx3 numpy array (float32 in [0,1] or uint8) to a PIL RGB image."""
|
||||
if arr.dtype != np.uint8:
|
||||
arr = np.clip(arr * 255.0, 0, 255).astype(np.uint8)
|
||||
return Image.fromarray(arr, mode="RGB")
|
||||
|
||||
|
||||
def pil_to_arr(img: Image.Image) -> np.ndarray:
|
||||
"""Convert a PIL image to HxWx3 float32 numpy array in [0, 1]."""
|
||||
return np.asarray(img.convert("RGB"), dtype=np.float32) / 255.0
|
||||
@@ -6,7 +6,6 @@ import time
|
||||
from pathlib import Path
|
||||
|
||||
import numpy as np
|
||||
import torch
|
||||
from PIL import Image
|
||||
|
||||
from ..core.cache_manager import (
|
||||
@@ -16,20 +15,11 @@ from ..core.cache_manager import (
|
||||
read_manifest,
|
||||
write_manifest,
|
||||
)
|
||||
from ..core.image_ops import arr_to_pil, pil_to_arr
|
||||
from ..core.sampler_runner import SamplerBackend, run_panel_sampler
|
||||
from ..core.types import COMIC_SCRIPT_TYPE, SolvedPanel, SolvedScript
|
||||
|
||||
|
||||
def _arr_to_pil(arr: np.ndarray) -> Image.Image:
|
||||
arr_u8 = np.clip(arr * 255.0, 0, 255).astype(np.uint8) if arr.dtype != np.uint8 else arr
|
||||
return Image.fromarray(arr_u8, mode="RGB")
|
||||
|
||||
|
||||
def _pil_to_arr(img: Image.Image) -> np.ndarray:
|
||||
arr = np.asarray(img.convert("RGB"), dtype=np.float32) / 255.0
|
||||
return arr
|
||||
|
||||
|
||||
def _red_x(w: int, h: int) -> np.ndarray:
|
||||
arr = np.full((h, w, 3), 1.0, dtype=np.float32)
|
||||
# crude X in red
|
||||
@@ -85,7 +75,8 @@ class ComicBatchGenerator:
|
||||
"clip": ("CLIP",),
|
||||
"vae": ("VAE",),
|
||||
"on_failure": (["retry_then_skip", "halt"], {"default": "retry_then_skip"}),
|
||||
"retries": ("INT", {"default": 2, "min": 0, "max": 10}),
|
||||
# Total attempts per panel, including the first try.
|
||||
"max_attempts": ("INT", {"default": 3, "min": 1, "max": 11}),
|
||||
},
|
||||
"optional": {
|
||||
"positive_suffix": ("STRING", {"default": "", "multiline": True}),
|
||||
@@ -123,11 +114,12 @@ class ComicBatchGenerator:
|
||||
|
||||
def generate(
|
||||
self, comic_script: SolvedScript, model, clip, vae,
|
||||
on_failure, retries,
|
||||
on_failure, max_attempts,
|
||||
positive_suffix: str = "", negative_suffix: str = "",
|
||||
_backend: SamplerBackend | None = None,
|
||||
_out_dir: Path | None = None,
|
||||
):
|
||||
import torch # lazy — keeps ComfyUI node scan cheap
|
||||
backend = _backend if _backend is not None else _ComfyUIBackend()
|
||||
out_dir = self._resolve_out_dir(comic_script.job_id, _out_dir)
|
||||
|
||||
@@ -148,7 +140,7 @@ class ComicBatchGenerator:
|
||||
|
||||
if manifest_matches(paths.manifest, run_panel):
|
||||
manifest = read_manifest(paths.manifest)
|
||||
img = _pil_to_arr(Image.open(paths.image))
|
||||
img = pil_to_arr(Image.open(paths.image))
|
||||
status = manifest.get("status", "ok")
|
||||
log_lines.append(f"{tag} ... cached"
|
||||
+ (" (skipped placeholder)" if status == "skipped" else ""))
|
||||
@@ -157,7 +149,7 @@ class ComicBatchGenerator:
|
||||
|
||||
img: np.ndarray | None = None
|
||||
last_err: BaseException | None = None
|
||||
for attempt in range(1 + retries):
|
||||
for attempt in range(max_attempts):
|
||||
t0 = time.time()
|
||||
try:
|
||||
img = run_panel_sampler(backend, model=model, clip=clip, vae=vae, panel=run_panel)
|
||||
@@ -167,12 +159,12 @@ class ComicBatchGenerator:
|
||||
f"sampler returned shape {img.shape}, expected ({expected_h}, {expected_w}, 3)"
|
||||
)
|
||||
log_lines.append(f"{tag} ... generated ({time.time() - t0:.1f}s)"
|
||||
+ (f" retry#{attempt}" if attempt else ""))
|
||||
+ (f" attempt {attempt + 1}/{max_attempts}" if attempt else ""))
|
||||
break
|
||||
except Exception as e: # noqa: BLE001
|
||||
last_err = e
|
||||
img = None
|
||||
log_lines.append(f"{tag} ... FAILED attempt {attempt + 1}: {e}")
|
||||
log_lines.append(f"{tag} ... FAILED attempt {attempt + 1}/{max_attempts}: {e}")
|
||||
continue
|
||||
|
||||
if img is None:
|
||||
@@ -180,14 +172,14 @@ class ComicBatchGenerator:
|
||||
raise RuntimeError(f"panel generation failed: {tag}") from last_err
|
||||
# retry_then_skip — persist placeholder so re-runs stay skipped.
|
||||
img = _red_x(*run_panel.bbox_size)
|
||||
_arr_to_pil(img).save(paths.image, format="PNG")
|
||||
arr_to_pil(img).save(paths.image, format="PNG")
|
||||
write_manifest(paths.manifest, panel_hash(run_panel),
|
||||
extra={"status": "skipped"})
|
||||
log_lines.append(f"{tag} ... skipped with placeholder")
|
||||
images.append(img)
|
||||
continue
|
||||
|
||||
_arr_to_pil(img).save(paths.image, format="PNG")
|
||||
arr_to_pil(img).save(paths.image, format="PNG")
|
||||
write_manifest(paths.manifest, panel_hash(run_panel),
|
||||
extra={"status": "ok"})
|
||||
images.append(img)
|
||||
|
||||
@@ -4,16 +4,12 @@ from __future__ import annotations
|
||||
import json
|
||||
|
||||
import numpy as np
|
||||
import torch
|
||||
|
||||
from ..core.compositor import compose_page
|
||||
from ..core.image_ops import pil_to_arr
|
||||
from ..core.types import COMIC_SCRIPT_TYPE, SolvedScript
|
||||
|
||||
|
||||
def _pil_to_tensor(img) -> np.ndarray:
|
||||
return np.asarray(img.convert("RGB"), dtype=np.float32) / 255.0
|
||||
|
||||
|
||||
def _bboxes_metadata(script: SolvedScript) -> dict:
|
||||
return {
|
||||
"job_id": script.job_id,
|
||||
@@ -52,8 +48,9 @@ class ComicPageComposer:
|
||||
}
|
||||
}
|
||||
|
||||
def compose(self, generated_panels: torch.Tensor, comic_script: SolvedScript,
|
||||
def compose(self, generated_panels, comic_script: SolvedScript,
|
||||
debug_overlay: bool):
|
||||
import torch # lazy — keeps ComfyUI node scan cheap
|
||||
panels_np = generated_panels.cpu().numpy() # (N, Hmax, Wmax, 3)
|
||||
|
||||
out_pages: list[np.ndarray] = []
|
||||
@@ -72,7 +69,7 @@ class ComicPageComposer:
|
||||
panel_imgs.append((panel, cropped))
|
||||
idx += 1
|
||||
pil = compose_page(page, panel_imgs)
|
||||
out_pages.append(_pil_to_tensor(pil))
|
||||
out_pages.append(pil_to_arr(pil))
|
||||
|
||||
# Pad composed pages to same shape if sizes differ (M1: all same)
|
||||
max_h = max(p.shape[0] for p in out_pages)
|
||||
|
||||
@@ -34,7 +34,7 @@ def _gen(node, script, backend, out_dir, on_failure="halt"):
|
||||
return node.generate(
|
||||
comic_script=script, model="M", clip="C", vae="V",
|
||||
positive_suffix="", negative_suffix="",
|
||||
on_failure=on_failure, retries=1,
|
||||
on_failure=on_failure, max_attempts=2,
|
||||
_backend=backend, _out_dir=out_dir,
|
||||
)
|
||||
|
||||
@@ -98,7 +98,7 @@ def test_retry_then_skip_mode_tolerates_transient_failure(tmp_path: Path):
|
||||
images, _, log = _gen(ComicBatchGenerator(), script, backend, tmp_path,
|
||||
on_failure="retry_then_skip")
|
||||
assert images.shape[0] == 4
|
||||
assert "retry" in log.lower()
|
||||
assert "attempt 2/2" in log
|
||||
|
||||
|
||||
def test_halt_mode_raises_on_failure(tmp_path: Path):
|
||||
@@ -127,7 +127,7 @@ def test_generator_does_not_mutate_input_panels(tmp_path: Path):
|
||||
node.generate(
|
||||
comic_script=script, model="M", clip="C", vae="V",
|
||||
positive_suffix="MASTERPIECE", negative_suffix="BLURRY",
|
||||
on_failure="halt", retries=0,
|
||||
on_failure="halt", max_attempts=1,
|
||||
_backend=RecordingBackend(), _out_dir=tmp_path,
|
||||
)
|
||||
after = _snapshot_panels(script)
|
||||
|
||||
@@ -94,3 +94,25 @@ def test_manifest_matches_false_when_no_file(tmp_path: Path):
|
||||
p = _panel()
|
||||
paths = panel_paths(tmp_path, p)
|
||||
assert manifest_matches(paths.manifest, p) is False
|
||||
|
||||
|
||||
def test_manifest_matches_false_when_version_mismatch(tmp_path: Path):
|
||||
"""Manifests from older releases (or manifests with no version) are stale
|
||||
— the panel_hash input schema may have shifted and we must re-render."""
|
||||
import json as _json
|
||||
|
||||
p = _panel()
|
||||
paths = panel_paths(tmp_path, p)
|
||||
paths.image.parent.mkdir(parents=True, exist_ok=True)
|
||||
# Simulate a legacy manifest: correct hash but no manifest_version.
|
||||
paths.manifest.write_text(_json.dumps({"hash": panel_hash(p)}), encoding="utf-8")
|
||||
assert manifest_matches(paths.manifest, p) is False
|
||||
|
||||
|
||||
def test_write_manifest_includes_version(tmp_path: Path):
|
||||
p = _panel()
|
||||
paths = panel_paths(tmp_path, p)
|
||||
paths.image.parent.mkdir(parents=True, exist_ok=True)
|
||||
write_manifest(paths.manifest, panel_hash(p))
|
||||
m = read_manifest(paths.manifest)
|
||||
assert m["manifest_version"] == 1
|
||||
|
||||
@@ -37,7 +37,7 @@ def test_full_pipeline_basic_script(tmp_path: Path):
|
||||
gen = ComicBatchGenerator()
|
||||
panels, pass_script, log = gen.generate(
|
||||
comic_script=script, model="M", clip="C", vae="V",
|
||||
on_failure="halt", retries=0,
|
||||
on_failure="halt", max_attempts=1,
|
||||
positive_suffix="", negative_suffix="",
|
||||
_backend=GreyBackend(), _out_dir=tmp_path,
|
||||
)
|
||||
@@ -55,7 +55,7 @@ def test_full_pipeline_basic_script(tmp_path: Path):
|
||||
gen2 = ComicBatchGenerator()
|
||||
panels2, _, log2 = gen2.generate(
|
||||
comic_script=script, model="M", clip="C", vae="V",
|
||||
on_failure="halt", retries=0,
|
||||
on_failure="halt", max_attempts=1,
|
||||
positive_suffix="", negative_suffix="",
|
||||
_backend=GreyBackend(), _out_dir=tmp_path,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user