diff --git a/core/api/retry.py b/core/api/retry.py index 50f03b9..295e19d 100644 --- a/core/api/retry.py +++ b/core/api/retry.py @@ -2,7 +2,7 @@ Both ``AgnesImageAPI`` and ``OpenAICompatProvider`` funnel their HTTP retry loop through :func:`retryable_post` so that retry semantics, the backoff policy -(exponential, capped at 120s, with jitter), and error collection stay identical +(exponential, capped at 30s, with jitter), and error collection stay identical across providers. Previously the ~100-line retry block was copy-pasted into each provider with @@ -43,9 +43,9 @@ def compute_backoff(attempt: int, base_delay: float, cap: float = BACKOFF_CAP_SE """Exponential backoff with a hard cap and full jitter. ``attempt`` is 0-based (the current retry index). The raw delay grows as - ``base_delay * 2**attempt``, is clamped to ``cap`` (120s), then multiplied - by a uniform random factor in ``[0.5, 1.0]`` so that many clients do not - synchronize their retries into a collective spike. + ``base_delay * 2**attempt``, is clamped to ``cap`` (30s by default), then + multiplied by a uniform random factor in ``[0.5, 1.0]`` so that many + clients do not synchronize their retries into a collective spike. """ raw = base_delay * (2**attempt) capped = min(raw, cap) diff --git a/core/comic/layout.py b/core/comic/layout.py index 4cc8218..ce0b5d7 100644 --- a/core/comic/layout.py +++ b/core/comic/layout.py @@ -14,14 +14,12 @@ from PIL import Image, ImageDraw, ImageFont -from core.comic.fonts import resolve_font -from core.config import webtoon_max_pixels +from core.comic.fonts import resolve_font, text_requires_cjk +from core.config import DEFAULT_WEBTOON_MAX_PIXELS, webtoon_max_pixels -# Upper bound for a single webtoon canvas, in pixels. A webtoon strip is one -# giant RGB buffer (3 bytes/px), so an unbounded strip OOMs on long books: -# 180 panels at 1400x1500 each would need >1GB in a single allocation. +# Re-export for callers/tests that historically imported from this module. # Override with INKSTONE_WEBTOON_MAX_PIXELS; set it to 0 to disable the guard. -DEFAULT_WEBTOON_MAX_PIXELS = 200_000_000 +__all__ = ["DEFAULT_WEBTOON_MAX_PIXELS", "LayoutEngine", "PanelImage"] def _webtoon_max_pixels() -> int: @@ -102,7 +100,7 @@ def _compose_pages(self, panels: list[PanelImage], output_dir: Path) -> list[str @staticmethod def _paginate(panels: list[PanelImage], per_page: int = 4) -> list[list[PanelImage]]: if not panels: - return [[]] + return [] return [panels[i : i + per_page] for i in range(0, len(panels), per_page)] @staticmethod @@ -225,13 +223,44 @@ def _wrap_text(text: str, font: ImageFont.ImageFont, max_width: int) -> list[str if not paragraph: lines.append("") continue - cur = "" - for char in paragraph: - if font.getlength(cur + char) <= max_width: - cur += char - else: + if text_requires_cjk(paragraph): + lines.extend(LayoutEngine._wrap_chars(paragraph, font, max_width)) + else: + lines.extend(LayoutEngine._wrap_words(paragraph, font, max_width)) + return lines or [""] + + @staticmethod + def _wrap_chars(text: str, font: ImageFont.ImageFont, max_width: int) -> list[str]: + lines: list[str] = [] + cur = "" + for char in text: + if font.getlength(cur + char) <= max_width: + cur += char + else: + if cur: lines.append(cur) - cur = char + cur = char + if cur: + lines.append(cur) + return lines + + @staticmethod + def _wrap_words(text: str, font: ImageFont.ImageFont, max_width: int) -> list[str]: + lines: list[str] = [] + cur = "" + for word in text.split(" "): + candidate = word if not cur else f"{cur} {word}" + if font.getlength(candidate) <= max_width: + cur = candidate + continue if cur: lines.append(cur) - return lines or [""] + if font.getlength(word) <= max_width: + cur = word + else: + # Oversized token: fall back to character wrap for that word only. + lines.extend(LayoutEngine._wrap_chars(word, font, max_width)) + cur = "" + if cur: + lines.append(cur) + return lines diff --git a/core/comic/segmentation.py b/core/comic/segmentation.py index 5e6ea51..861c2e8 100644 --- a/core/comic/segmentation.py +++ b/core/comic/segmentation.py @@ -109,6 +109,35 @@ def segment_text( return chunks +def _mint_unnamed(merged: dict[str, CharacterAsset]) -> str: + """Allocate a unique fallback name for nameless extracts.""" + if "unnamed" not in merged: + return "unnamed" + index = 2 + while f"unnamed_{index}" in merged: + index += 1 + return f"unnamed_{index}" + + +def _fill_empty_appearance(target: CharacterAsset, source: CharacterAsset) -> None: + """Copy non-empty role/appearance fields from ``source`` into empty slots on ``target``.""" + if not (target.role or "").strip() and (source.role or "").strip(): + target.role = source.role + for field in ( + "hair", + "eyewear", + "outfit_top", + "outfit_bottom", + "shoes", + "body_type", + "distinguishing", + ): + cur = getattr(target.appearance, field) or "" + nxt = getattr(source.appearance, field) or "" + if not str(cur).strip() and str(nxt).strip(): + setattr(target.appearance, field, nxt) + + def merge_characters( existing: dict[str, CharacterAsset], new: "Iterable[CharacterAsset]", @@ -116,8 +145,9 @@ def merge_characters( """Merge extracted characters into the running character table by name. Characters already present (exact name match) are kept and reused so the - same portrait is not regenerated. New names are added and reported so the - pipeline can generate their portraits. + same portrait is not regenerated; empty appearance/role fields are filled + from later extractions. Multiple nameless extracts become ``unnamed``, + ``unnamed_2``, … so they do not share one identity. Args: existing: the project's current ``name -> CharacterAsset`` table. @@ -129,9 +159,15 @@ def merge_characters( merged = dict(existing) created: list[str] = [] for char in new: - if char.name not in merged: - merged[char.name] = char - created.append(char.name) + name = char.name + if name == "unnamed" and name in merged: + name = _mint_unnamed(merged) + char = char.model_copy(update={"name": name}) + if name not in merged: + merged[name] = char + created.append(name) + else: + _fill_empty_appearance(merged[name], char) return merged, created diff --git a/core/config.py b/core/config.py index a782d54..07f62d8 100644 --- a/core/config.py +++ b/core/config.py @@ -89,7 +89,12 @@ def webtoon_warn_mb(*, default: float = 50.0) -> float: return default -def webtoon_max_pixels(*, default: int = 200_000_000) -> int: +# Upper bound for a single webtoon canvas, in pixels. A webtoon strip is one +# giant RGB buffer (3 bytes/px), so an unbounded strip OOMs on long books. +DEFAULT_WEBTOON_MAX_PIXELS = 200_000_000 + + +def webtoon_max_pixels(*, default: int = DEFAULT_WEBTOON_MAX_PIXELS) -> int: raw = _get(ENV_WEBTOON_MAX_PIXELS, "").strip() if raw: try: diff --git a/core/pipelines/creative_comic.py b/core/pipelines/creative_comic.py index 61982ae..3965d3e 100644 --- a/core/pipelines/creative_comic.py +++ b/core/pipelines/creative_comic.py @@ -287,10 +287,13 @@ def _mark_chunk_done_if_complete( def _soft_invalidate_render(state: ProjectState) -> None: - """Drop render-owned assets while keeping structural cache (extract/storyboard).""" + """Drop render-owned assets while keeping structural cache (extract/storyboard). + + Content-policy ``skipped`` entries are preserved: the source text did not + change, so re-attempting those panels only burns quota. + """ state.panels_done = [] state.stale_panels = [] - state.skipped = [] state.generated.panels = {} state.generated.portraits = {} for asset in state.characters.values(): @@ -341,11 +344,13 @@ def _reconcile_state(state: ProjectState, state_path: Path, output_dir: Path) -> def _chunk_complete( state: ProjectState, board: Storyboard, output_dir: Path, chunk_index: int ) -> bool: - """True when every planned panel is generated, contained, and present on disk.""" + """True when every planned panel is generated or policy-skipped.""" for panel_index, _panel in enumerate(board.panels): state_key = _stored_panel_key(state, chunk_index, panel_index) if state_key in state.stale_panels: return False + if state_key in state.skipped: + continue rec = state.generated.panels.get(state_key) if ( rec is None @@ -365,7 +370,8 @@ def _ordered_generated_panels(state: ProjectState) -> list[tuple[str, GeneratedP (k for k in state.chunk_cache if str(k).isdigit()), key=lambda value: int(value), ) - for chunk_index, key in enumerate(digit_keys): + for key in digit_keys: + chunk_index = int(key) board = state.chunk_cache[key].storyboard if board is None: continue diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 62cf435..3e67d45 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -64,6 +64,10 @@ Keep these prototypes only as migration material until they conform to the targe - **Local (v0.1.2):** structure/render fingerprint split landed — style, model, and L3 changes soft-invalidate panels/portraits only; `chunk_cache` is reused. Legacy states still compare the combined hash until migrated. + - **Local (v0.1.3):** defect-review patch — sparse chunk panel ordering, preserve + content-policy `skipped` across soft-invalidate, Latin word-wrap, unique + `unnamed_*` identities, fill empty appearance on merge (see + `docs/superpowers/plans/2026-07-27-v0.1.3-defect-review-fixes.md`). - [ ] Reconcile README, configuration defaults, CLI help and historical docs so they do not contradict released behavior. - [ ] Isolate, rename or remove the old PageScript / coverage prototype so it diff --git a/docs/superpowers/plans/2026-07-27-v0.1.3-defect-review-fixes.md b/docs/superpowers/plans/2026-07-27-v0.1.3-defect-review-fixes.md new file mode 100644 index 0000000..050306d --- /dev/null +++ b/docs/superpowers/plans/2026-07-27-v0.1.3-defect-review-fixes.md @@ -0,0 +1,33 @@ +# v0.1.3 — Defect Review Verification & Fixes + +**Date:** 2026-07-27 +**Source:** `.issue/2026-07-27-16_22-v0.1-defect-review.md` + +## Verification summary + +| ID | Verdict | Action | +|----|---------|--------| +| #1 `WEBTOON_MAX_PIXELS=0` | **False positive** — `"0"` is truthy in Python; existing tests pass | None | +| #2 `_ordered_generated_panels` enum index | **Confirmed** (dialogue/key corruption under sparse keys; extras only partially masks omission) | Fix `int(key)` | +| #3 soft-invalidate clears `skipped` | **Confirmed** | Keep `skipped` | +| #4 Latin mid-word wrap | **Confirmed** | Word-wrap Latin; keep CJK char wrap | +| #5 unnamed collision | **Confirmed** when name+role blank | Mint unique `unnamed_N` in `merge_characters` | +| #6 `_chunk_complete` ignores skipped | **Confirmed** (wasted re-entry) | Treat `skipped` as complete | +| #7 merge discards appearance fills | **Confirmed** | Fill empty appearance/role like `merge_settings` | +| #8 `_is_within` duplicate | Drift | Defer (larger move; not a user-facing bug) | +| #9 dead `DEFAULT_WEBTOON_MAX_PIXELS` | Drift | Point tests/layout at `config` default | +| #10 backoff docstring 120s | **Confirmed** DOC | Fix docstring | +| #13 portrait scan all chars | Not a bug — proposed fix breaks soft-invalidate | None | +| #14 `_paginate` → `[[]]` | **Confirmed** EDGE | Return `[]` | +| #15–#20 | DESIGN / low-impact ROBUST / retracted | Defer | + +## Implementation order (TDD) + +1. Failing tests for #2, #3, #4, #5, #6, #7, #14, #10 +2. Minimal production fixes +3. Update soft-invalidate test expectation (`skipped` preserved) +4. Run targeted pytest + +## Out of scope + +Content-policy detection redesign (#15), PDF compression (#18), CJK Extension B token estimate (#20), dotenv quote edge (#17), L3 multi-face log (#19), coerce_size warning (#11), wrap O(n²) (#12). diff --git a/tests/test_estimate_progress.py b/tests/test_estimate_progress.py index 87139c3..47e7f87 100644 --- a/tests/test_estimate_progress.py +++ b/tests/test_estimate_progress.py @@ -84,3 +84,43 @@ def test_ordered_generated_panels_ignores_non_digit_chunk_keys(): ) ordered = _ordered_generated_panels(state) assert [key for key, _ in ordered] == ["c0000-p0000", "c0001-p0000"] + + +def test_ordered_generated_panels_uses_sparse_chunk_keys(): + """Chunk keys may skip indices after content-policy skips (0, 1, 5).""" + board0 = Storyboard(chapter_id="0", panels=[Panel(panel_id="p0", action="a", dialogue="d0")]) + board1 = Storyboard(chapter_id="1", panels=[Panel(panel_id="p1", action="a", dialogue="d1")]) + board5 = Storyboard(chapter_id="5", panels=[Panel(panel_id="p5", action="a", dialogue="d5")]) + state = ProjectState( + project_id="p", + chunk_cache={ + "0": ChunkCache(storyboard=board0), + "1": ChunkCache(storyboard=board1), + "5": ChunkCache(storyboard=board5), + }, + generated=GeneratedAssets( + panels={ + "c0000-p0000": GeneratedPanel(local="/tmp/p0.png", chunk_index=0, panel_index=0), + "c0001-p0000": GeneratedPanel(local="/tmp/p1.png", chunk_index=1, panel_index=0), + "c0005-p0000": GeneratedPanel(local="/tmp/p5.png", chunk_index=5, panel_index=0), + } + ), + ) + ordered = _ordered_generated_panels(state) + assert [key for key, _ in ordered] == ["c0000-p0000", "c0001-p0000", "c0005-p0000"] + assert [g.dialogue for _, g in ordered] == ["d0", "d1", "d5"] + assert [g.chunk_index for _, g in ordered] == [0, 1, 5] + + +def test_chunk_complete_true_when_all_panels_skipped(tmp_path): + from core.pipelines.creative_comic import _chunk_complete + + board = Storyboard( + chapter_id="0", + panels=[Panel(panel_id="p0", action="a"), Panel(panel_id="p1", action="a")], + ) + state = ProjectState( + project_id="p", + skipped=["c0000-p0000", "c0000-p0001"], + ) + assert _chunk_complete(state, board, tmp_path, 0) is True diff --git a/tests/test_fingerprint_split.py b/tests/test_fingerprint_split.py index 2438eed..6a1f863 100644 --- a/tests/test_fingerprint_split.py +++ b/tests/test_fingerprint_split.py @@ -118,7 +118,8 @@ def test_soft_invalidate_render_clears_panels_keeps_chunk_cache(): _soft_invalidate_render(state) assert state.panels_done == [] assert state.stale_panels == [] - assert state.skipped == [] + # Content-policy skips must survive render invalidation (style/model change). + assert state.skipped == ["c0000-p0002"] assert state.generated.panels == {} assert state.generated.portraits == {} assert state.characters["方鸿渐"].portrait_local is None diff --git a/tests/test_layout.py b/tests/test_layout.py index 6342b12..26f2de1 100644 --- a/tests/test_layout.py +++ b/tests/test_layout.py @@ -1,7 +1,7 @@ """tests/test_layout.py — page composition and dialogue bubbles (no network).""" import numpy as np -from PIL import Image, ImageDraw +from PIL import Image, ImageDraw, ImageFont from core.comic.layout import LayoutEngine, PanelImage @@ -103,3 +103,38 @@ def test_explicit_newlines_expand_dialogue_bubble(tmp_path): assert Image.open(page[0]).height > 100 assert Image.open(webtoon[0]).height > 100 + + +def test_wrap_text_latin_breaks_on_word_boundaries(): + eng = LayoutEngine() + font = ImageFont.load_default() + # Narrow width forces wrapping; words must stay intact. + lines = eng._wrap_text("The quick brown fox", font, max_width=40) + joined = " ".join(lines) + assert "The" in joined and "quick" in joined + for line in lines: + # No mid-word split of these tokens across a line boundary without space. + assert "quic" != line # would only appear if "quick" was split mid-word as start + assert all(" " not in w or True for w in lines) + # Every original word appears unbroken in some line (or as whole line). + for word in ("The", "quick", "brown", "fox"): + assert any(word in line for line in lines) + + +def test_wrap_text_cjk_still_breaks_per_character(): + eng = LayoutEngine() + font = ImageFont.load_default() + text = "雨水顺着梧桐叶滑落" + lines = eng._wrap_text(text, font, max_width=20) + assert "".join(lines) == text + assert len(lines) >= 2 + + +def test_paginate_empty_returns_no_pages(): + assert LayoutEngine._paginate([]) == [] + + +def test_compose_empty_page_mode_writes_nothing(tmp_path): + paths = LayoutEngine().compose([], tmp_path, layout_mode="page") + assert paths == [] + assert list(tmp_path.glob("*.png")) == [] diff --git a/tests/test_providers.py b/tests/test_providers.py index e67c77a..f08092b 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -163,11 +163,12 @@ def test_compute_backoff_exponential_capped_with_jitter(monkeypatch): # the production code still multiplies by random.uniform, we only fix the value. monkeypatch.setattr("core.api.retry.random.uniform", lambda _a, _b: 0.75) vals = [compute_backoff(i, 20.0) for i in range(10)] - # Every value is positive and clamped to the 120s cap. - assert all(0 < v <= 120.0 for v in vals) + # Every value is positive and clamped to the 30s cap. + assert all(0 < v <= 30.0 for v in vals) # Early attempts are strictly smaller than later ones (exponential growth # dominates before the cap is hit). assert vals[0] < vals[3] + assert "30" in compute_backoff.__doc__ assert RETRYABLE_STATUS == (429, 500, 502, 503, 504, 520, 521, 522, 523, 524) diff --git a/tests/test_segmentation.py b/tests/test_segmentation.py index f1907d4..ffc2fad 100644 --- a/tests/test_segmentation.py +++ b/tests/test_segmentation.py @@ -16,6 +16,36 @@ def test_merge_characters_dedups_by_exact_name(): assert created == ["b"] +def test_merge_characters_mints_unique_unnamed(): + first = CharacterAsset(name="unnamed", appearance={"outfit_top": "tray"}) + second = CharacterAsset(name="unnamed", appearance={"outfit_top": "rifle"}) + merged, created = merge_characters({}, [first, second]) + assert "unnamed" in merged + assert any(n.startswith("unnamed_") for n in merged) + assert len(merged) == 2 + assert len(created) == 2 + tops = {a.appearance.outfit_top for a in merged.values()} + assert tops == {"tray", "rifle"} + + +def test_merge_characters_fills_empty_appearance_fields(): + existing = { + "Da Shi": CharacterAsset(name="Da Shi", appearance={"hair": "short"}), + } + new = [ + CharacterAsset( + name="Da Shi", + role="detective", + appearance={"hair": "ignored", "eyewear": "aviator sunglasses"}, + ) + ] + merged, created = merge_characters(existing, new) + assert created == [] + assert merged["Da Shi"].appearance.hair == "short" + assert merged["Da Shi"].appearance.eyewear == "aviator sunglasses" + assert merged["Da Shi"].role == "detective" + + def test_segment_text_respects_chapter_headings(): text = "第一章\nintro.\n第二章\nmore text here." chunks = segment_text(text)