Files
manga-recap-pipeline/caveats/audit-open.md
T
kami 18b49c43bd Record two GPU cycles: the gate works, coverage is the open question
No worker code changed. This is the evidence from the 2026-08-12 16:39 and
17:38 runs, and where each finding now lives.

The fourth session's four identity changes all work on real panels. Panel 7's
two wrong bindings are gone. The lead going unassigned there is correct and
was measured, not assumed: face_detect finds one face on the whole panel at
conf 0.599, nothing else above 0.056 even at a 0.04 threshold, and the crop
shows him drawn from behind.

Two decisions, both closed: a roster name is a guess so it never reaches
detection, and merged_into is exactly one hop deep. Two caveats, both open:
detection can order a bbox backwards (1 in 117), and identity coverage has
fallen on every run since the gate landed (70 -> 61 -> 50).

Coverage is the thing to settle next, and not by reading the number.
identity_labels already holds 145 rows of ground truth.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-12 21:51:36 +04:00

11 KiB

Open limits from the 2026-08-11 audit

Everything here was read in source during the audit and deliberately left unfixed in Phase 1. The fixed findings live in decisions/audit-phase1.md. Line numbers are from the audit and may drift.

Reconcile deletes the losing character irreversibly

Reconciliation used to delete the losing character row. Clearing the reconcile stage did not undo it, and nothing recorded which detections had been the loser's.

Half-closed 2026-08-12. The loss is no longer unrecoverable. merge_characters marks the loser merged_into = keeper instead of deleting it, so its embedding, description and gender survive, and every repointed assignment is stamped method = merged_from:<loser_id> in identity_assignment_sources. Those two records are enough to walk a merge backwards. Roster readers filter merged_into IS NULL. Lookup by id does not, so an assignment still pointing at a merged id resolves.

What is still missing is the mechanism that consumes them: there is no unmerge, and no split. Undoing a merge today means a manual SQL walk of the two records above.

Costs: a wrong merge needs hand-written SQL to undo, not a rebaseline. Revisit when: a reviewer reports a wrong merge, or the review gates from [#136] get a place to hang name/merge/split actions. No wrong merge has been seen since the crops were fixed, so building the unmerge before either trigger would be speculative.

Clearing a stage does not undo what it wrote

The dialogue and direct half of this is fixed and proven (decisions/storage-layout.md#clear-vision-blob). It cost a wasted rerun on 2026-08-11 first: the clear returned {"ok": true}, deleted nothing, and the stage skipped all 116 panels.

What remains: clearing identity preserves the per-manga registry by design, so a rerun inherits every character it ever minted (caveats/speaker-attribution.md#registry-pollution). Nothing verifies that a clear emptied what it claimed.

Costs: a rerun silently reuses stale output, which reads as a reproducible result. Revisit when: any stage is scheduled concurrently or resumed automatically. A stage must be idempotent before either is safe.

ComfyUI uses the GPU outside the session mutex

The layers stage calls ComfyUI directly and takes no lease. Another job can load gemma, siglip2, or dots while ComfyUI holds the same GPU.

Costs: out-of-memory failures that look random and land on an unrelated stage. Revisit when: layers is enabled on a real run, or a second concurrent job is allowed. Workaround: MAX_CONCURRENT_JOBS=1 keeps one pipeline at a time, which is the current default.

The JSON repair pass can fabricate dialogue

call_gemma4_json hands the model its own truncated text and asks for the JSON it should have been (worker_vision.py). The repair call carries no image. On a response truncated by max_tokens, the model completes dialogue it can no longer see. What it adds is indistinguishable downstream from transcribed text.

Costs: invented lines enter the script with normal provenance. Revisit when: schema-constrained generation lands, which removes most of this path. Workaround: resend the image on repair, or retry a truncated response instead of repairing it.

/review/preview pins a solo clip into the final video

review_preview renders one panel and calls save_clip(panel_id, ...). _render_one_beat returns early when a clip already exists for the leader. Previewing a beat leader therefore drops the rest of the beat's panels. review_retts gets this right by calling delete_clip first.

Costs: a reviewer silently corrupts the output by looking at it. Revisit when: the review UI is used on a real chapter. Workaround: never preview a beat leader, or delete the clip row afterwards.

Stored embeddings carry no model or pooling version

embed_crop uses pooler_output when present and falls back to mean-pooled patch tokens otherwise. The two paths produce different vector spaces, and the fixed 0.85 threshold is valid for one of them. Nothing beside a stored .npy records which model, revision, or pooling produced it.

Costs: a transformers upgrade mixes incompatible vectors into one gallery with no error. Revisit when: transformers or the siglip2 revision is upgraded. Before, not after. Workaround: none. Write the model id and pooling mode beside the vector and refuse cross-version comparison.

Worker endpoints block the event loop

Endpoints declared async def run blocking MinIO, OpenCV, torch, and ffmpeg calls directly on the event loop. A busy worker cannot answer /health or /unload.

Costs: /health/workers reports a working worker as unreachable, and the session manager's 30-second /unload can time out exactly when VRAM needs freeing. Revisit when: a stage stalls on /unload, or before any bounded parallelism lands. Workaround: def instead of async def moves each handler to the threadpool. Tracked as [#199] for the render worker.

SQLite has no busy timeout

get_conn opens a connection per call with no busy_timeout. WAL tolerates one writer. PIPELINE=1 already writes clips from concurrent tasks while TTS writes audio.

Costs: planned CPU parallelism will surface as database is locked before it surfaces as throughput. Revisit when: Phase 2 concurrency work starts. Set the timeout first. Workaround: keep PIPELINE off.

layers runs after tts

STAGES orders layers after tts, while run_stage_tts warns that eager rendering under PIPELINE=1 needs layers to run first.

Costs: with the flag on, solo beats always render without parallax. Revisit when: a real run enables PIPELINE=1. Workaround: keep PIPELINE off, or reorder STAGES.

completed means something different in each stage

A dropped vision panel fails the stage and halts the pipeline. A failed direction window counts its panels as done. Layers always finishes completed.

Costs: the acceptance metrics in ROADMAP.md cannot be read across stages. Revisit when: a baseline measurement is taken. The numbers are meaningless until then. Workaround: none.

Identity worker caches characters the orchestrator has deleted

worker_identity._known_cache is invalidated only by _persist_char. The reconcile stage deletes losing characters directly in the orchestrator database. A long-lived worker keeps shortlisting and assigning ids that no longer exist.

Costs: assignments point at rows that are gone. Revisit when: reconcile runs on a chapter without a worker restart between stages. Tracked as [#201], which proposes caching per manga_id. Workaround: restart the identity worker after reconcile.

A character seen once gets no assignment at all

worker_identity._pending holds full crop images in worker memory for a whole chapter, keyed by session. That is durable per-chapter state inside a worker documented as stateless, and it is lost on restart. A character seen exactly once receives no assignment, not even a chapter-local handle.

Costs: one-off characters vanish from the scene graph. Revisit when: chapter-local tracklet persistence lands (ROADMAP.md, Phase 3). Workaround: none.

MinIO credentials are hardcoded in committed source

Defaults live in transport.py and service.py.

Costs: the credentials are in git history for anyone who gets the repo. Revisit when: the repo leaves this machine, or MinIO is reachable outside the LAN. Workaround: the environment variables already override them. Set them and remove the defaults.

Assemble marks a job completed with no clips

run_stage_assemble does not check that clip_uris is non-empty before assembling, then marks the job completed.

Costs: a failed chapter reports success. Revisit when: any run reports completed without a video. One if not clip_uris guard fixes it. Workaround: none.

Reviewer timestamps drift against the crossfaded video

/review/panels sums per-panel audio durations. Assemble crossfades clips using the per-beat transitions, so every non-cut transition shortens the real video.

Costs: reviewer timestamps drift further out of sync the further into the chapter they scrub. Revisit when: the review UI is used for timing work. Workaround: subtract the transition overlaps by hand.

A completed job keeps the error from an earlier failure

/job/resume does not clear jobs.error. Job 778297bc finished every stage and still reports status: "completed" beside error: "partial: 112/116 completed", a message from three resumes earlier.

Costs: any reader of the error field sees a failure on a successful job. The review UI and any future alerting both read it. Revisit when: anything branches on jobs.error, or a run is judged by its status alone.

layers reports success on an empty bucket

Measured on job 778297bc: layers reported completed 116/116 while s3://layers/ held 0 objects. Every clip in that run therefore has no parallax. The stage is a sibling of #inconsistent-stage-policy, but this is the measured instance: worker_layers.py's own self-check already fails on the missing legacy/qwen_layered_workflow.json, and the stage still reports done for every panel.

Costs: a silent quality regression that no status field reveals. Revisit when: parallax matters for a deliverable, or before quoting this run as a full-pipeline pass.

Detection can order a bbox backwards

p007 person_1 came back as [226, 417, 130, 551] on the 2026-08-12 17:38 run: x1 greater than x2. One detection in 117. _bbox_to_pixels clamps every coordinate into the panel but never orders the corners, so the box survives as a zero-or-negative-width region. It crops to nothing, so that detection can never enroll, embed or match, and it is silently lost rather than reported.

The guard is two min/max pairs in _bbox_to_pixels. It was not written this session because it is a worker change and needs a vision worker restart plus a GPU cycle to prove.

Revisit trigger: the next vision run. Count degenerate boxes against 1 in 117.

Identity coverage has fallen on every run since the gate landed

70% -> 61% -> 50% across the 13:11 baseline, the 16:39 run and the 17:38 run. The has_face gate explains part of it and is stable, gating 39% of detections on both post-change runs. Against face-bearing detections only, the 17:38 run assigned 59 of 72, or 82%.

Nothing yet separates correct abstention from lost cast, and both fixes that could have caused the second drop landed together. A back-turned lead is a correct abstention. A real character the resolver refused is not, and the two are indistinguishable in the coverage number alone.

Revisit trigger: before trusting the registry for a downstream run. identity_labels already exists for exactly this and holds 145 rows of human ground truth, so eval_identity.py can score precision against recall instead of counting assignments.