diff --git a/HANDOFF.md b/HANDOFF.md index 8ec77b5..06a08cd 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -1,130 +1,110 @@ -# HANDOFF, 2026-08-12 (second session of the day) +# HANDOFF, 2026-08-12 (third session of the day) -Live state is in `NEXT.md`. This file is only what this session did. The previous handoff's content is -now in `JOURNAL.md` and in `decisions/`. +Live state is in `NEXT.md`. This file is only what this session did. The previous handoff is in +`JOURNAL.md`. ## Asked -Get up to speed from the previous handoff. Then: rebuild the chapter, and what about characters. +Move on from the chapter rebuild. Scope chosen mid-session: reset the registry, rerun `vision`, +`identity` and `reconcile` only, inspect, then decide about the ~80 minutes downstream. After the +inspection the user chose to fix extras-versus-cast before running downstream. ## Result -Two things landed. The A/V gap is fixed and the rebuilt `chapter.mp4` is in the bucket. The `bbox` -coordinate space is settled with measurements instead of comments, and converted at the boundary. - -One correction to carry forward. **The first fix of the day named the wrong cause.** Commit `1457556` -claimed the xfade offset drift was the shipped 72.7s gap. The rebuild came out byte-identical to the -broken file, which disproved it. Both are real defects. Only the second one shipped. - -## The chapter, rebuilt - -`s3://video/ef105a86-4b7e-4ac4-b45c-b7d83b8f5b5e/7c944dd4-e972-42c7-ba60-9f6939548e80/chapter.mp4` +The identity rebaseline ran and the numbers moved a long way. One code change on top of it is written +and self-checked but **not yet proven on a GPU run**. ``` -before v=436.392 a=363.675 nb_frames=9902 avg_frame_rate=22.69 gap +72.72 -after v=364.120 a=364.122 nb_frames=9101 r_frame_rate=25/1 gap -0.002 + before stale run after +x2 past panel width 47/113 46/110 0/110 +coords exactly on 1000 21 27 1 +max y2 1000 1000 2307 +characters 19 8 8 +coverage 26/113 = 23% 77/110 = 70% 77/110 = 70% +top character's share 96% Haeseon 47% 47% ``` -Cause: `assemble` sends an all-`cut` chapter down a `concat` demuxer with `-c copy`. That path writes -the output in the **first** input's `time_base` and reinterprets every later packet in it. 14 of the 49 clips -are `30/1` at `1/15360`, the other 35 are `25/1` at `1/12800`, so those 14 play `15360/12800 = 1.2` too -long with their audio untouched. `collage_cmd` hardcoded `-r 30`. -`decisions/chapter-assembly.md#mixed-rate-stream-copy`. +`Choi Haeseon`, which held 25 of 26 assignments, is gone from the registry. Panel 7 against the art: -Fixes: `collage_cmd` emits `-r FPS`. `assemble` probes `_fps_of` across the clips and routes mixed rates -through `_assemble_batched`, whose branches both normalize with `fps={FPS}`. +| box | who | before | after | +| --- | --- | --- | --- | +| `[457, 657, 642, 937]` | Seonho, foreground | nothing | `Seonho` | +| `[669, 591, 763, 822]` | the colleague, unnamed in the story | `Choi Haeseon` 0.9 | `character_f7a4fd` | +| `[428, 386, 496, 526]` | background extra | extra as cast | none | +| `[34, 414, 122, 564]` | background extra | extra as cast | none | +| `[498, 386, 568, 533]` | background extra | extra as cast | `character_d72710` 0.94 | +| `[31, 554, 94, 728]` | background extra | `Lim Seonho` | `Seonho` **1.00** | -A second, latent defect on the transition path was fixed and committed separately. `_xfade_chain` took -offsets from `format=duration`, which is `max(video, audio)`. The accumulator crept past the end of its -input, and ffmpeg discarded whole clips at `rc 0` with nothing on stderr. -`decisions/chapter-assembly.md#offsets-from-min-stream`. +## The run that did not count -The single-item passthrough theory from the previous handoff is dead, recorded void at -`decisions/chapter-assembly.md#passthrough-innocent`. The one-path rewrite it recommended is not needed. - -**Still open here.** The 14 clips in the bucket are still 30fps. Assembly normalizes them, so the chapter -is correct, but the fast stream-copy path stays off for this chapter until `render` re-runs. Nobody has -watched the rebuilt video yet. The 2:52 slide transition and the 28s static hold from 2:24 were both -supposed to be re-judged after the sync fix. - -## Characters: the `bbox` space, settled - -All 113 detections, straight from `/review/identity`: - -| test | result | -| --- | --- | -| boxes with `x2` past the 900px panel width | **47 of 113** | -| boxes with `y2` past 1000, on panels 1257 to 2307px tall | **0 of 113** | -| boxes clamped at exactly 1000 | 21 in x, 5 in y | -| coordinate range over every box | `[0, 1000]` | - -Gemma's native 0-1000 grid. Not pixels. `worker_vision.py` prompt text and the old -`worker_identity.py:91` comment both claimed pixels and both were wrong. - -`/vision` now calls `_bbox_to_pixels(characters, w, h)` before returning. Four consumers are fixed at -once: `_crop_bbox` in identity, `_pair_faces_to_present`, the set-of-mark boxes, and the review UI's -client-side crop. The pairing one was comparing real pixel face boxes against 0-1000 character boxes, -which is the likely mechanism behind 7 `unknown` out of 7 `som_face` lines. -`decisions/identity-bbox.md#bbox-is-normalized`. - -Checked by eye on panel 7, not just asserted. Five of six converted boxes land on their subject. That -includes `person_5`, who is Seonho in the foreground with headphones and carried no identity. `person_1` -still frames an empty window mullion, which is `caveats/speaker-attribution.md#extras-as-cast`. - -Converted boxes for panel 7, for whoever redraws the overlay: +The first full cycle completed 116/116/20 and reproduced the defect exactly. The fix was not wrong, it +was not loaded: ``` -person_1 [226, 414, 286, 553] window mullion, nobody -person_2 [ 34, 558, 106, 749] background extra, was assigned Lim Seonho -person_3 [428, 384, 494, 533] background extra -person_4 [498, 389, 561, 549] background extra -person_5 [460, 657, 631, 939] Seonho, foreground. was assigned nothing -person_6 [646, 591, 767, 794] the colleague, no name in the story. was assigned Choi Haeseon at 0.9 +vision worker started 12:00:09 +worker_vision.py modified 12:11:35 +8113bdf, carrying _bbox_to_pixels 12:16:22 ``` -**The registry is unchanged and still wrong.** Every stored box, embedding and `ref_image_uris` was -enrolled from the wrong space. `vision` and `identity` have to re-run before any of it means anything, -and that is GPU work nobody authorized. `caveats/speaker-attribution.md#bbox-wrong-space` is marked -resolved with the rerun pending. +Python binds a module once, at process start. The stage reported success and the orchestrator recorded +no error. Cost: one vision + identity + reconcile cycle and a second registry reset to undo the 8 +characters it minted. `decisions/identity-bbox.md#stale-worker-invalidates`. + +`./check_stale.sh` now compares every running worker's start time against its module mtime and exits +non-zero. Mutation-tested. **Run it before any stage run meant to prove a code change.** + +## Written this session + +- `db.reset_registry` + `POST /characters/reset` (`confirm=true`). `/stage/clear` spares `characters` + by design (`db.py:790`), so nothing could rebaseline the registry. Also clears + `identity_assignment_sources`, which no stage clear touches and where a leftover `manual` row makes + `assign_identity` refuse the next model assignment (`db.py:663`). + Covered by `test_db.py:TestResetRegistry`. **Orchestrator, committed on homesrv, image rebuilt.** +- `check_stale.sh`, `audit_registry.py` (runs inside `manga-orchestrator`, already `docker cp`'d). +- **`has_face`, the extras gate. This is the part not yet proven.** Fixing the coordinate space made + extras worse. With crops finally on their subjects, an extra bound to the lead at 1.00. `/vision` stamps + `has_face` per character using `face_detect` + `_pair_faces_to_present`, so containment and its + margin are the rules the speaker path already uses. `worker_identity.py` skips `has_face is False` + before it crops or embeds. Fails open on a missing or raising detector, and gates on `is False` so an + older vision blob behaves as before. `decisions/identity-bbox.md#face-gates-enrollment`. ## Checks -Every self-check runs from the repo root and passes: - ```bash -.venv/bin/python worker_render.py # about 4 minutes, real ffmpeg -.venv/bin/python worker_vision.py +.venv/bin/python worker_vision.py # includes the has_face gate + both fail-open paths .venv/bin/python worker_identity.py +./check_stale.sh # exits non-zero if a worker predates its file +cd /home/kami/docker-apps/manga-infra/orchestrator && python3 -m pytest -q --ignore=test_api.py # 115 passed, on homesrv ``` -Three checks were added, because the existing ones passed all the way through both shipped defects: - -- `_fps_of(collage clip) == "25/1"`, on a real collage encode. This is the one that would have caught the - mixed-rate bug at the source. -- three clips whose audio outlasts their video by 0.4s, assembled through the xfade branch. Mutation - tested by restoring `_audio_dur`: fires with `video=1.80 audio=3.56 expected=3.56`. -- `_bbox_to_pixels` against panel 7's real `person_5` box, asserting the result covers the lower half of a - 1650px panel, which a raw grid value cannot. - -`_check_assembled` now runs after every encode on both paths, because ffmpeg returns 0 while dropping -whole inputs. - ## Next command -Watch the rebuilt chapter before anything else. That is what found every real defect so far. +The `has_face` gate has never run on a GPU. Restart vision and identity, reset, rerun, and check whether +the two wrong bindings on panel 7 disappear without taking Seonho with them. ```bash -/usr/bin/ssh kami@192.168.1.104 'mc cat homesrv/video/ef105a86-4b7e-4ac4-b45c-b7d83b8f5b5e/7c944dd4-e972-42c7-ba60-9f6939548e80/chapter.mp4' > chapter.mp4 +cd /home/kami/Programs/n8n-worker && ./check_stale.sh # restart anything it flags +J=778297bc-e7ce-439d-91b5-8a027060d17f; M=ef105a86-4b7e-4ac4-b45c-b7d83b8f5b5e +/usr/bin/ssh kami@192.168.1.104 "curl -s -X POST http://127.0.0.1:9090/characters/reset -H 'Content-Type: application/json' -d '{\"manga_id\":\"$M\",\"confirm\":true}'" +/usr/bin/ssh kami@192.168.1.104 "curl -s -X POST http://127.0.0.1:9090/stage/clear -H 'Content-Type: application/json' -d '{\"job_id\":\"$J\",\"stage\":\"vision\"}'" +for S in vision identity reconcile; do /usr/bin/ssh kami@192.168.1.104 "curl -s --max-time 5400 -X POST http://127.0.0.1:9090/stage/run -H 'Content-Type: application/json' -d '{\"job_id\":\"$J\",\"stage\":\"$S\"}'"; done +/usr/bin/ssh kami@192.168.1.104 "docker exec manga-orchestrator python3 /app/audit_registry.py 7c944dd4-e972-42c7-ba60-9f6939548e80 6" ``` -Then, with a go-ahead, the vision and identity rerun in `NEXT.md` item 1. +Watch for coverage collapsing. 70% is the number to beat. A gate that abstains too hard shows up there +before it shows up on panel 7. Restart the identity worker after every reset: it caches the known list +in-process and only invalidates on enrollment. -## Traps confirmed again this session +## Gone -- `tmux` session `manga-workers` was gone and both systemd units were inactive. `./start_workers.sh` - starts 9 windows. The render worker is window 9 and must be restarted by hand to pick up an edit. -- Plain `ssh` is the kitty ssh kitten and refuses non-interactive stdin. Use `/usr/bin/ssh`. -- `cd $dir && .venv/bin/python` fails, because the venv path is relative to the repo root. Use the - absolute interpreter path when the working directory is elsewhere. -- The whole assembly investigation ran offline on 49 downloaded clips with no GPU and no orchestrator. - Re-download with the command in `NEXT.md`. +The rebuilt `chapter.mp4` and all 49 clips were deleted by the cascade from `/stage/clear vision`. The +user chose not to keep a copy. Nothing downstream of `reconcile` exists for this job now. + +## Traps confirmed again + +- A stage reporting `completed 116/116` says the code ran, not that the current code ran. +- The SSHFS mount at `/mnt/server/home/kami` was absent and needed remounting by the user, since the + mountpoint needs root. +- The orchestrator image bakes its source. Editing the repo on homesrv does nothing until + `docker compose up -d --build orchestrator`. +- `rtk grep` searches files, not stdin. Piping into it silently searches the repo instead. diff --git a/JOURNAL.md b/JOURNAL.md index c197905..f30bdf7 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -306,3 +306,93 @@ subject, including `person_5`, who is Seonho in the foreground with headphones a Not done: `vision` and `identity` have not re-run, so every box, embedding and `ref_image_uris` in the registry is still from the wrong space. That rerun is GPU work and was not started. + +## 2026-08-12, third session — the rerun, and the run that did not count + +Asked: move on from the chapter rebuild. Scope chosen mid-session: reset the registry, rerun vision, +identity and reconcile only, and inspect before spending the ~80 minutes downstream. + +**The registry had to be wiped first, and nothing could do it.** `clear_stage_data` maps `identity` to +`identity_assignments` only and spares `characters` on purpose (`db.py:790`), because the registry is +cross-run. So a rerun would have matched fresh crops against 19 stale grid-space embeddings and kept the +duplicate `Lim Seonho` / `Seonho` rows that raise `ambiguous-speaker`. Added `db.reset_registry` and +`POST /characters/reset` (`confirm=true` required), which also clears `identity_assignment_sources` — +untouched by any stage clear, and a leftover `manual` row there makes `assign_identity` refuse the next +model assignment (`db.py:663`). Covered by `test_db.py:TestResetRegistry`; 115 orchestrator tests pass. +The orchestrator image bakes its source, so it needed `docker compose up -d --build orchestrator`. + +**The first full cycle was void: the worker was serving pre-fix code.** vision + identity + reconcile +completed 116/116/20 and reproduced the defect exactly — 46 of 110 boxes past the 900px panel width, +coordinates clamping at 1000. The vision worker started 12:00:09, `worker_vision.py` changed 12:11:35, +and the commit carrying `_bbox_to_pixels` landed 12:16:22. Python had already bound the old module. +Nothing in the stage output said so (`decisions/identity-bbox.md#stale-worker-invalidates`). Added +`check_stale.sh`, which compares every worker's process start against its module mtime and exits +non-zero; mutation-tested by touching `worker_tts.py`. + +**The rerun against restarted workers.** 8 minutes for all three stages. + +``` + before stale run after +x2 past panel width 47/113 46/110 0/110 +coords exactly on 1000 21 27 1 +max y2 1000 1000 2307 +characters 19 8 8 +coverage 26/113 = 23% 77/110 = 70% 77/110 = 70% +top character's share 96% Haeseon 47% 47% +``` + +`Choi Haeseon`, which had absorbed 25 of 26 assignments, no longer exists in the registry. On panel 7 +Seonho is bound for the first time, and the unnamed colleague took an anonymous id instead of being +called `Choi Haeseon` at 0.9. + +**Fixing the boxes made the extras problem worse.** With crops finally landing on their subjects, a +background extra bound to `Seonho` at confidence 1.00, putting an extra into the lead's reference set. +`/vision` now stamps `has_face` per character via `face_detect` + `_pair_faces_to_present`, and identity +skips `has_face is False` before it crops or embeds +(`decisions/identity-bbox.md#face-gates-enrollment`). Fails open on a missing or raising detector. +Self-checked in both workers. **Not yet proven on a GPU run** — that is the next command. + +Deleted along the way and not recoverable: the rebuilt `chapter.mp4` and all 49 clips, by the cascade +from `/stage/clear vision`. The user chose not to keep a copy. + +## 2026-08-12, fourth session — identity 2b, the resolver NONE branch + +Asked: "how much will `has_face` help with character and identity problems?", then "fix 2b first". + +Answered the first honestly: `has_face` reaches 2 of 6 detections on panel 7 and nothing else. It does not +touch naming or merging, and it cannot touch the chibi at 1:35, because an anime face detector detects a +chibi face. Also flagged its real cost: the model is face-only by design, so back-turned cast lose +enrollment along with the extras, and coverage is where that shows up first. + +Measured before writing anything, read-only, no GPU: + +``` +registry: 8 characters, 1 named -> ['Seonho'] +detections: 110 assignments: 77 = 70% coverage +spread: Seonho 36, character_565c88 24, character_759e23 9, character_f7a4fd 3, + character_25f682 3, character_d72710 1, character_823aba 1 +``` + +That killed the assumed cause. Anonymous ids already recur, so the identity worker's own +pending-promote path gives stable anonymous identities. The defect was elsewhere. + +`/vision/resolve` can answer "none of these" and always could: `worker_vision.py:1071` returns +`state="new"` for `choice: 0` and `state="unresolved"` for an out-of-range index. `service.py` read only +`character_id` and unassigned every crop of the tracklet for either. The stale `ponytail:` comment above +that block named the real blocker and was right: minting needs an `embedding_uri` the orchestrator cannot +compute, since siglip and gemma cannot both be resident. + +Fixed by carrying the embedding, not by adding a GPU pass. `/identity/resolve` writes each crop's +embedding to the crop's key with a `.npy` suffix and returns `emb_uri`. `tracklets.resolve_outcome` holds +the three-way decision as a pure function. `service.py` mints via the existing `create_character` and +falls into the existing assign loop. `decisions/identity-bbox.md#none-mints-an-anonymous-character`. + +Checks: `worker_identity self-check ok`, `tracklets self-check ok`, 115 passed on homesrv. +Deployed: image rebuilt, `resolve_outcome` verified inside the running container, `audit_registry.py` +re-copied after the recreate. Vision and identity restarted, `./check_stale.sh` exits 0. + +New trap: `tmux respawn-window -k` does not re-run the window command. It leaves a bare shell and the +worker down. Both workers were dead for two minutes before `/health` caught it. + +Not run: the GPU cycle. `has_face` and the NONE mint are both unproven on real panels and now land in the +same run. diff --git a/NEXT.md b/NEXT.md index 7ff5af1..be20615 100644 --- a/NEXT.md +++ b/NEXT.md @@ -39,10 +39,12 @@ One number sets the agenda: checked by eye on panel 7, where five of six converted boxes land on their subject (`decisions/identity-bbox.md#bbox-is-normalized`). `person_1` still frames an empty window mullion, which is (c). - b. **Let identity abstain and stay abstained.** The colleague has no name in the story and was - labelled `Choi Haeseon` at 0.9. An unnamed recurring person needs a stable anonymous identity so - narration says "the colleague" every time. `match()` already returns `None` below threshold. Check - whether the Tier-2 gemma resolver can answer "none of these"; that was not verified. + b. ~~Let identity abstain and stay abstained.~~ **Done 2026-08-12, not yet run on a GPU** + (`decisions/identity-bbox.md#none-mints-an-anonymous-character`). The resolver could always answer + "none of these". The orchestrator was discarding the answer: it read only `character_id`, so a + deliberate NONE and a hallucinated index both unassigned every crop of the tracklet. A NONE now + mints an anonymous character from the crop, using the embedding `/identity/resolve` ships beside + it as `emb_uri`. Deployed: image rebuilt, `resolve_outcome` verified inside the container. c. **Separate extra from cast.** Four of the six detections on panel 7 are background extras or nothing at all, and all six reach identity as equal candidates. d. Only then merge `Seonho` into `Lim Seonho` and split `character_afa7623b`, which still needs the @@ -113,7 +115,11 @@ Read the state, or clear a stage and resume: /usr/bin/ssh kami@192.168.1.104 "curl -s -X POST 'http://127.0.0.1:9090/job/resume?job_id=778297bc-e7ce-439d-91b5-8a027060d17f'" ``` -Traps: plain `ssh` is the kitty ssh kitten and refuses non-interactive stdin, so use `/usr/bin/ssh`. +Traps: `tmux respawn-window -k` does NOT re-run the window's command, it drops to a bare shell and the +worker stays down. Restart one worker by re-sending the `send-keys` line from `start_workers.sh`, and +confirm with `/health` plus `./check_stale.sh`. `docker compose up --build orchestrator` recreates the +container, which drops any `docker cp`'d script such as `audit_registry.py`. +Plain `ssh` is the kitty ssh kitten and refuses non-interactive stdin, so use `/usr/bin/ssh`. `mc` aliases on homesrv are `homesrv` and `mio`. `local` returns Access Denied and `rfs` is the empty rustfs. `cp` is aliased to `cp -i` and hangs on overwrite, so use `/usr/bin/cp -f`. diff --git a/audit_registry.py b/audit_registry.py new file mode 100644 index 0000000..4c88149 --- /dev/null +++ b/audit_registry.py @@ -0,0 +1,93 @@ +"""Registry audit for one chapter, after vision + identity + reconcile and before anything downstream. + +Runs inside manga-orchestrator (reads /data/manga.db). `audit_speakers.py` answers the attribution +questions and needs the dialogue stage; this one answers the questions that decide whether dialogue is +worth running at all: + + 1. did the bbox fix land — are stored boxes pixels, or still gemma's 0-1000 grid, + 2. how many characters did the rebaseline mint, and did one of them absorb the chapter again, + 3. what happened on panel 7, the worked example. + +Usage: docker exec manga-orchestrator python3 /app/audit_registry.py [chapter_id] [panel_index] +""" +import collections +import json +import sqlite3 +import sys + +CHAPTER = sys.argv[1] if len(sys.argv) > 1 else "7c944dd4-e972-42c7-ba60-9f6939548e80" +WORKED_EXAMPLE = int(sys.argv[2]) if len(sys.argv) > 2 else 7 + +c = sqlite3.connect("/data/manga.db") +c.row_factory = sqlite3.Row +manga_id = c.execute("SELECT manga_id FROM chapters WHERE chapter_id=?", (CHAPTER,)).fetchone()[0] + +reg = {r["character_id"]: dict(r) for r in c.execute( + "SELECT character_id, name, aliases, gender, ref_image_uris, embedding_uri " + "FROM characters WHERE manga_id=?", (manga_id,))} + +panels = c.execute( + 'SELECT panel_id, panel_index, page_index, bbox FROM panels WHERE chapter_id=? ORDER BY panel_order', + (CHAPTER,)).fetchall() + +# 1. coordinate space. A 0-1000 grid box on a panel wider or taller than 1000px cannot exceed 1000, +# and clamps AT 1000. Real pixel boxes track the panel and scatter past it. The tell is the ratio of +# the largest coordinate to the panel dimension, plus how many boxes sit exactly on 1000. +detections = 0 +past_1000 = at_1000 = 0 +max_ratio = 0.0 +assigned_total = 0 +per_char = collections.Counter() +worked = None + +for p in panels: + row = c.execute("SELECT result_json FROM vision_results WHERE panel_id=?", (p["panel_id"],)).fetchone() + if not row: + continue + v = json.loads(row["result_json"]) + # the vision blob carries no panel size. panels.bbox is the panel's box on its page and is + # [x, y, w, h], not corners — panel 3 of this chapter is [0, 615, 900, 106]. + pb = json.loads(p["bbox"] or "null") + pw, ph = (pb[2], pb[3]) if pb and len(pb) == 4 else (None, None) + assigns = {a["local_id"]: (a["character_id"], a["confidence"]) for a in c.execute( + "SELECT local_id, character_id, confidence FROM identity_assignments WHERE panel_id=?", + (p["panel_id"],))} + assigned_total += len(assigns) + for cid, _ in assigns.values(): + per_char[reg.get(cid, {}).get("name") or cid[:16]] += 1 + people = [ch for ch in (v.get("characters") or []) if ch.get("bbox")] + detections += len(people) + for ch in people: + x1, y1, x2, y2 = ch["bbox"] + past_1000 += 1 if max(x2, y2) > 1000 else 0 + at_1000 += 1 if 1000 in (x2, y2) else 0 + if pw and ph: + max_ratio = max(max_ratio, x2 / pw, y2 / ph) + if p["panel_index"] == WORKED_EXAMPLE: + worked = (p, v, people, assigns, pw, ph) + +named = [r for r in reg.values() if (r["name"] or "").strip()] +print(f"registry: {len(reg)} characters, {len(named)} named -> {sorted((r['name'] or '') for r in named)}") +print(f"detections: {detections} assignments: {assigned_total} " + f"= {100*assigned_total/max(detections,1):.0f}% coverage") +if per_char: + top, n = per_char.most_common(1)[0] + print(f"assignment spread: {dict(per_char.most_common(8))}") + print(f" top character holds {n}/{assigned_total} = {100*n/max(assigned_total,1):.0f}% " + f"({'ABSORBING, same signature as before' if n > 0.5 * assigned_total else 'ok'})") +print(f"bbox space: {past_1000}/{detections} boxes exceed 1000, {at_1000} sit exactly on 1000, " + f"largest coord/panel-dimension = {max_ratio:.2f}") +print(f" verdict: {'PIXELS' if past_1000 or max_ratio > 0.02 and at_1000 == 0 else 'STILL 0-1000 GRID'}") +missing_refs = [k for k, r in reg.items() if not r["ref_image_uris"] or not r["embedding_uri"]] +print(f"characters missing a ref crop or embedding: {len(missing_refs)}") + +if worked: + p, v, people, assigns, pw, ph = worked + print(f"\npanel_index {WORKED_EXAMPLE} ({p['panel_id']}), {pw}x{ph}:") + for ch in people: + cid, conf = assigns.get(ch["local_id"], (None, None)) + name = reg.get(cid, {}).get("name") or (cid[:16] if cid else "-- none --") + print(f" {ch['local_id']:10} {str(ch['bbox']):28} {name:22} " + f"{'' if conf is None else f'{conf:.2f}'}") +else: + print(f"\npanel_index {WORKED_EXAMPLE} not found in this chapter") diff --git a/caveats/speaker-attribution.md b/caveats/speaker-attribution.md index ec6d824..f821975 100644 --- a/caveats/speaker-attribution.md +++ b/caveats/speaker-attribution.md @@ -116,10 +116,13 @@ evidence. A recurring unnamed person needs a stable anonymous identity, so narra colleague" every time. `match()` at `worker_identity.py:69` does abstain, returning `None` below threshold, so the 0.9 came from -cosine clearing the threshold on a wrong crop. Whether the Tier-2 gemma resolver can answer "none of -these" was not verified. +cosine clearing the threshold on a wrong crop. -Revisit trigger: immediately after the `bbox` space is settled. +**Resolved 2026-08-12, `decisions/identity-bbox.md#none-mints-an-anonymous-character`.** The gemma resolver +can answer "none of these" and always could. The orchestrator was discarding the answer. It read only +`character_id`, so a deliberate NONE and a hallucinated index both unassigned every crop of the tracklet. +A NONE now mints an anonymous character from the crop, using the embedding identity ships beside it. Not +yet proven on a GPU run. ## Vision does not separate a background extra from cast {#extras-as-cast} @@ -142,7 +145,9 @@ The visual comparison people reach for as the fix is **already implemented**, so `/vision/resolve` at `worker_vision.py:963` sends the query crop plus up to 3 labelled reference images per candidate. `build_resolve_prompt` already tells the model to judge face shape first, to treat hair and outfit as secondary, that two people sharing a hair colour are not the same, and to answer `0` for -NONE when unsure. `choice: 0` becomes a new character and an out-of-range index becomes `unresolved`. The +NONE when unsure. `choice: 0` returns `state="new"` and an out-of-range index returns `unresolved`. What +the orchestrator does with each is +`decisions/identity-bbox.md#none-mints-an-anonymous-character`. The `ref_image_uris` column is republished as `reference_image_uris` at `worker_identity.py:152` and `:161`, so the references reach the model. diff --git a/decisions/CLAUDE.md b/decisions/CLAUDE.md index 4542463..ba8fc24 100644 --- a/decisions/CLAUDE.md +++ b/decisions/CLAUDE.md @@ -41,3 +41,6 @@ still live belongs in `caveats/`. | [A vision `bbox` is gemma's 0-1000 grid, converted to pixels at `/vision`](identity-bbox.md#bbox-is-normalized) | closed | | [Assembly verifies its own output instead of trusting ffmpeg's exit code](chapter-assembly.md#check-assembled) | closed | | [The single-item passthrough is not the assembly bug](chapter-assembly.md#passthrough-innocent) | void | +| [A stage result proves nothing until the worker is newer than the edit](identity-bbox.md#stale-worker-invalidates) | closed | +| [A detection with no detected face never enrolls or binds](identity-bbox.md#face-gates-enrollment) | closed | +| [A resolver NONE mints an anonymous character, it does not clear the crop](identity-bbox.md#none-mints-an-anonymous-character) | closed | diff --git a/decisions/identity-bbox.md b/decisions/identity-bbox.md index 0c637e1..c2308a9 100644 --- a/decisions/identity-bbox.md +++ b/decisions/identity-bbox.md @@ -43,3 +43,113 @@ that. It also records the trap: a model that really answered in pixels would be `ref_image_uris` are enrolled on balloons and window frames. Re-running identity is what makes the registry mean anything. The anonymous-identity and extra-versus-cast work cannot be judged until that rerun happens. + +## A stage result proves nothing until the worker is newer than the edit {#stale-worker-invalidates} + +**Closed, 2026-08-12.** The first rerun after the bbox fix reproduced the defect exactly: 46 of 110 +boxes past the 900px panel width, coordinates clamping at 1000, `y2` never once past 1000 on panels up +to 2307px tall. The same fingerprint as `#bbox-is-normalized` measured before the fix. + +The fix was not wrong. It was not loaded. + +``` +vision worker process started 12:00:09 +worker_vision.py modified 12:11:35 +8113bdf, which contains _bbox_to_pixels, committed 12:16:22 +``` + +Python binds a module once, at process start. `./start_workers.sh` had launched the worker eleven +minutes before the file changed, so `/vision` served pre-fix code for the whole run and returned raw +grid boxes. Nothing in the result said so. The stage reported `completed 116/116`, the orchestrator +recorded no error, and identity and reconcile ran to completion on top of it. Cost: one full +vision + identity + reconcile cycle, plus a registry reset to undo the 8 characters it minted. + +The rerun against a restarted worker gives the opposite reading over the same 110 detections: 0 boxes +past the width, 0 past the height, one coordinate on 1000 which is now a real pixel value, and a +deepest box reaching 100% down its panel with `max y2 = 2307`. Boxes track the panel, so they are +pixels. + +`check_stale.sh` compares every running worker's process start against its module's mtime and exits +non-zero if any is stale. This failure mode was already known as advice — the render worker "must be +restarted by hand to pick up an edit" — and advice did not stop it happening. Run the check before any +stage run that is meant to prove a code change. + +Forbids: citing a stage result as evidence about a code change without establishing that the worker +serving it postdates the change. + +## A detection with no detected face never enrolls or binds {#face-gates-enrollment} + +**Closed, 2026-08-12. Written and self-checked, not yet proven on a GPU run.** + +Fixing the coordinate space made the extras problem worse, not better. With the boxes finally landing +on their subjects, panel 7's four background extras became four good crops of four irrelevant people, +and one of them bound to `Seonho` at confidence 1.00. Before the fix the same detection was a crop of +scenery and matched nothing much. Correct geometry turned a harmless failure into a poisoned reference +set for the lead. + +The measured panel 7 outcome, converted boxes, against the art: + +| box | who | assigned | +| --- | --- | --- | +| `[457, 657, 642, 937]` | Seonho, foreground | `Seonho` | +| `[669, 591, 763, 822]` | the colleague, unnamed in the story | `character_f7a4fd`, anonymous | +| `[428, 386, 496, 526]` | background extra | none | +| `[34, 414, 122, 564]` | background extra | none | +| `[498, 386, 568, 533]` | background extra | `character_d72710` at 0.94 | +| `[31, 554, 94, 728]` | background extra | `Seonho` at 1.00 | + +`/vision` now stamps `has_face` on every character by running `face_detect.detect_faces` on the panel +and reusing `_pair_faces_to_present` for containment, so the gate uses the same margin and the same +global shortest-first assignment as the speaker path. `worker_identity.py` skips a character with +`has_face is False` before it crops, embeds, matches or mints. + +Two properties are deliberate. It **fails open**: a missing or raising detector marks every character +`True`, because dropping a whole panel's cast is worse than the over-detection the gate exists to trim. +And it gates on `is False`, not falsiness, so a vision blob written before this change (no key) behaves +as it did rather than silently dropping every character. + +Cost: a cast member drawn from behind, or in a style the detector misses, now takes no identity on that +panel. That is the abstain this pipeline already prefers to a wrong bind +(`caveats/speaker-attribution.md#no-anonymous-identity`). + +Forbids: enrolling a reference crop, or binding a character, from a region no face detector confirms. + +## A resolver NONE mints an anonymous character, it does not clear the crop {#none-mints-an-anonymous-character} + +**Closed, 2026-08-12. Written and self-checked, not yet proven on a GPU run.** + +`caveats/speaker-attribution.md#no-anonymous-identity` asked whether the Tier-2 gemma resolver can answer +"none of these". It can, and it always could. `/vision/resolve` at `worker_vision.py:1071` maps `choice: 0` +to `state="new"`, an out-of-range index to `state="unresolved"`, and a parse failure to `unresolved` as +well. The abstain path was never the defect. + +The defect was one branch on the other side of the contract. The orchestrator read only +`v.get("character_id")` and treated every falsy value the same way: `unassign_identity` on every crop of +the tracklet. So a deliberate "this is a real person the roster does not hold" and a hallucinated index +both produced nothing, and the unnamed colleague was `unknown` on every panel she appeared on. The stale +`ponytail:` comment above that block named the reason nobody fixed it, and the reason was real: minting a +character needs an `embedding_uri`, and the orchestrator cannot compute one. siglip is resident in the +identity worker, gemma is resident in the vision worker, and `session_manager` forbids both at once. + +What removes the blocker is carrying the embedding, not a third GPU pass. `/identity/resolve` already +computes an embedding per crop and already uploads the crop to +`s3://manga/{manga}/characters/_crops/{panel}_{local}.png`. It now writes the embedding to the same key +with a `.npy` suffix and returns `emb_uri` in each shortlist entry. The mint is then a local +`create_character(manga_id, None, appearance, [crop_uri], emb_uri, gender)`, and the existing per-tracklet +assign loop binds every member to it. + +`tracklets.resolve_outcome` holds the three-way decision as a pure function, so the branch that runs is +the branch the self-check covers: `known` on a named answer, `mint` on `state="new"` with an `emb_uri`, +`clear` on `unresolved`, on a NONE with no embedding, and on an older worker that sends no `state` at all. + +Two limits are deliberate. A tracklet's candidate gallery is built before the loop mints anything, so one +person split across two unlinked tracklets still gets two anonymous ids; `run_stage_reconcile` merges +unnamed twins on appearance overlap and is what folds them. And an anonymous character's text sheet +(`worker_vision._sheet`) carries no name, so gemma re-recognising it on a later panel leans on the +reference images rather than the description. + +Contract: `shortlists[].emb_uri` is new in the `/identity/resolve` response. Invariant 7 — both repos +changed in the same session. + +Forbids: treating an absent `character_id` as one outcome. A resolver that answered and a resolver that +failed are different facts. diff --git a/worker_identity.py b/worker_identity.py index 18677bb..477412e 100644 --- a/worker_identity.py +++ b/worker_identity.py @@ -200,6 +200,13 @@ async def resolve(data: IdentityInput): assignments, backfill, new_chars, shortlists = [], [], [], [] for ch in data.vision_characters: + # no detected face inside the box -> a background extra, a figure on a poster, or scenery + # gemma called a person. Embedding it pollutes the registry and, once the boxes were pixels, + # bound an extra to the lead at confidence 1.00. Abstain instead. `/vision` stamps this and + # fails open, so a panel it could not gate arrives with has_face=True on every character + # (decisions/identity-bbox.md#face-gates-enrollment). Absent key = an older vision blob. + if ch.get("has_face") is False: + continue crop = _crop_bbox(img, ch["bbox"]) if crop.size == 0: # degenerate/out-of-bounds bbox -> nothing to embed, skip continue @@ -207,10 +214,17 @@ async def resolve(data: IdentityInput): # shortlist for gemma's decider, from the roster as it stood before this crop's own outcome. sl = shortlist(emb, known, data.k, ch.get("gender")) - crop_uri = f"s3://manga/{data.manga_id}/characters/_crops/{data.panel_id}_{ch['local_id']}.png" + # the embedding ships with the crop. gemma's decider can answer "none of these", and the + # orchestrator has to mint a character from that crop — which needs an embedding it cannot + # compute (siglip is resident here, gemma is resident there, and the mutex forbids both). + # Uploading it now is what removes the third siglip pass + # (`decisions/identity-bbox.md#none-mints-an-anonymous-character`). + key = f"{data.manga_id}/characters/_crops/{data.panel_id}_{ch['local_id']}" + crop_uri, emb_uri = f"s3://manga/{key}.png", f"s3://manga/{key}.npy" cp = f"{SHM}/cc_{uuid.uuid4().hex[:8]}.png"; cv2.imwrite(cp, crop) transport.put(cp, crop_uri); os.remove(cp) - shortlists.append({"local_id": ch["local_id"], "crop_uri": crop_uri, + _save_npy(emb, emb_uri) + shortlists.append({"local_id": ch["local_id"], "crop_uri": crop_uri, "emb_uri": emb_uri, "candidates": [{"character_id": c["character_id"], "name": c.get("name"), "gender": c.get("gender"), "species": c.get("species"), "appearance": c.get("description"), "cosine": c["cosine"], diff --git a/worker_vision.py b/worker_vision.py index 5e076c6..dcc40ce 100644 --- a/worker_vision.py +++ b/worker_vision.py @@ -330,6 +330,40 @@ def _bbox_to_pixels(chars: list, w: int, h: int) -> list: return chars +def _mark_has_face(img, chars: list) -> list: + """Stamp `has_face` on every character a real detected face lands inside. + + gemma calls every person-shaped region a character. On panel 7 of job 778297bc that was two + people and four background extras, one of which is an empty window mullion. All six reached + identity as equal candidates, and after the bbox fix an extra took `Seonho` at confidence 1.00, + which puts an extra's crop into the lead's reference set. A faceless crop is a bad embedding as + well as a bad reference: the back of a head or a patch of coat matches almost anything. + + Reuses `_pair_faces_to_present`, so containment, its margin, and the global shortest-first + assignment are exactly the rules the speaker path already uses. Requires pixel boxes, so call it + after `_bbox_to_pixels`. + + Fails open. A missing or broken detector marks everything `True`, because dropping every + character is worse than the over-detection this gate exists to trim. + """ + def _all(v): + for c in chars: + c["has_face"] = v + return chars + + if face_detect is None or not chars: + return _all(True) + try: + faces = face_detect.detect_faces(img) + except Exception as e: + print(f"[vision/detect] face detect failed, has_face gate off for this panel: {e}", flush=True) + return _all(True) + paired = {p["local_id"] for p in _pair_faces_to_present(faces, chars) if p.get("local_id")} + for c in chars: + c["has_face"] = c.get("local_id") in paired + return chars + + def _panel_size(path: str) -> tuple: """(width, height) of a panel image, or (0, 0) when it cannot be read.""" import cv2 @@ -355,11 +389,14 @@ async def vision(data: VisionInput): print(f"[vision/detect] parse failed for {data.panel_id} after repair retry: {e}", flush=True) result = {"skip": False, "parse_failed": True, "characters": [], "scene": {}} finally: - pw, ph = _panel_size(local) + import cv2 + img = cv2.imread(local) # read once: the size and the face gate both need it os.remove(local) + pw, ph = (img.shape[1], img.shape[0]) if img is not None else (0, 0) result.setdefault("characters", []) if pw and ph: _bbox_to_pixels(result["characters"], pw, ph) + _mark_has_face(img, result["characters"]) else: print(f"[vision/detect] panel size unreadable for {data.panel_id}, boxes left normalized", flush=True) @@ -1187,6 +1224,35 @@ if __name__ == "__main__": # the box must now cover the lower half of a tall panel, which the raw grid value never can assert _ch[0]["bbox"][3] > 1000 > _ch[0]["bbox"][1] + # the has_face gate, on panel 7's real converted boxes. Seonho and the colleague carry a detected + # face; the two background extras do not, and identity must skip them rather than embed scenery. + _p7 = [{"local_id": "person_1", "bbox": [457, 657, 642, 937]}, # Seonho, foreground + {"local_id": "person_2", "bbox": [669, 591, 763, 822]}, # the colleague + {"local_id": "person_5", "bbox": [31, 554, 94, 728]}, # extra that took Seonho at 1.00 + {"local_id": "person_6", "bbox": [34, 414, 122, 564]}] # extra + _faces = [{"bbox": [520, 690, 590, 770]}, {"bbox": [695, 610, 740, 660]}] + class _FD: + @staticmethod + def detect_faces(_img): + return _faces + _real_fd, globals()["face_detect"] = face_detect, _FD + try: + _mark_has_face(None, _p7) + assert [c["has_face"] for c in _p7] == [True, True, False, False], _p7 + # fail open: a detector that raises must not silently drop every character in the panel + class _Boom: + @staticmethod + def detect_faces(_img): + raise RuntimeError("onnx died") + globals()["face_detect"] = _Boom + _mark_has_face(None, _p7) + assert all(c["has_face"] for c in _p7), _p7 + globals()["face_detect"] = None + _mark_has_face(None, _p7) + assert all(c["has_face"] for c in _p7), _p7 + finally: + globals()["face_detect"] = _real_fd + # face->identity pairing is GATED on containment: a face outside every gemma bbox stays unknown. faces = [{"bbox": [10, 10, 30, 30]}, {"bbox": [900, 900, 920, 920]}] present = [{"local_id": "person_1", "name": "Teto", "bbox": [0, 0, 100, 200]}]