kami
0cc6302245
Implements every P0 from AUDIT.md plus four P1s, across both halves of the pipeline. Verified by CPU-only self-checks and the orchestrator test suite. No GPU work ran and no pipeline ran. workpc: - worker_scene: read speaker_ref, not the rewritten speaker field. Every line narrated as "Someone" before this. Emit `actions` for the verifier. - worker_script: declare beat + verifier_feedback (pydantic dropped both, so the retry was blind) and render them as a repair prompt. - worker_vision: gate face->identity pairing on containment, assign globally shortest-first, map an out-of-range resolver index to `unresolved` instead of minting a character, parse JSON with raw_decode. - session_manager: tear down a server whose lease vanished mid-load, and spawn the supervisor respawn unlocked. orchestrator (edited in place, NOT committed there): - tracklets: canonicalize gender, add co-presence cannot-links, block transitive bridges across a hard constraint. - correctness: stop failing valid narration on sentence-initial capitals and short quotes; read action evidence from the singular key. - db: stop orphan flags leaking into every chapter; resolve by flag id. - service: TTS returns instead of raising under GATES, auto-resolves under autonomous mode; job admission control; registry names on dialogue resume. - session_proxy: queue on 409 instead of stealing the lease; run heartbeats. Docs restructured per the repo-structure layout: CLAUDE.md is a pointer table, NEXT.md replaces HANDOFF.md, plus ROADMAP.md, JOURNAL.md, decisions/ and caveats/. AUDIT.md now points at those instead of restating them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XD7cAy81MZrc7gCr6aZGWr
145 lines
8.0 KiB
Markdown
145 lines
8.0 KiB
Markdown
# Audit Phase 1: correctness and scheduling safety
|
|
|
|
Settled 2026-08-11 from `AUDIT.md`. Every entry was verified by a runnable check in the same commit.
|
|
No GPU work ran and no pipeline run was executed. Each claim rests on source and on the CPU-only
|
|
self-checks named below.
|
|
|
|
Files: `worker_scene.py`, `worker_script.py`, `worker_vision.py`, `session_manager.py` on workpc, and
|
|
`tracklets.py`, `correctness.py`, `db.py`, `service.py`, `session_proxy.py` in the homesrv orchestrator
|
|
(`/mnt/server/home/kami/docker-apps/manga-infra/orchestrator/`).
|
|
|
|
## Speaker ref is canonical {#speaker-ref-is-canonical}
|
|
|
|
**Closed.** A dialogue row's speaker is `speaker_ref` when its kind is `character_id`. The flat
|
|
`speaker` field is a compatibility value. The orchestrator already rewrote it from a panel-local id to
|
|
a `character_id`.
|
|
|
|
Evidence: `correctness.py:normalize_dialogue` rewrites `row["speaker"]`. `worker_scene.build_scene`
|
|
mapped that value through `id_by_local`, whose keys are panel-local ids. Every lookup returned `None`,
|
|
so `worker_script._render_line` narrated every line as `Someone`.
|
|
|
|
Forbids: reading `speaker` as a panel-local id anywhere downstream of the dialogue stage.
|
|
Check: `python worker_scene.py`, the `p007` case.
|
|
|
|
The same field leaked into the dialogue resume path, where a stored `character_id` was pushed into
|
|
`recent` as a speaker name. It now renders the registry name (`service.py`, `run_stage_dialogue`).
|
|
|
|
## Verifier false positives {#verifier-false-positives}
|
|
|
|
**Closed.** `verify_script` must stay quiet on valid narration. A failure is not soft. The script stage
|
|
retries once and then raises, so one false positive halts the whole chapter.
|
|
|
|
Two rules fired on correct output.
|
|
|
|
1. `unsupported-proper-noun` flagged any capitalized token outside a 20-word stop list. That caught
|
|
ordinary sentence openers (`Suddenly`, `Behind`, `Inside`, `Both`, `Everyone`, `After`, `Two`) and
|
|
`Someone`. A capital opening a sentence is now grammar unless the word falls outside a real
|
|
narration vocabulary. A capital in the middle of a sentence still counts as name evidence.
|
|
2. `misquote` compared each quoted span against a WHOLE source line at ratio 0.82. The narration prompt
|
|
asks for a short quote of the actual words. A 15-character quote from a 40-character line scored
|
|
about 0.55. Grounding now matches the longest contiguous span instead.
|
|
|
|
Forbids: adding a verifier rule without a test that a correct beat passes it.
|
|
Check: `pytest test_script_verify.py` in the orchestrator.
|
|
|
|
Verifier feedback now reaches the retry. `worker_script.ScriptInput` declares `beat` and
|
|
`verifier_feedback`. Pydantic v2 dropped both silently before, so the retry was another blind sample.
|
|
The repair prompt names each failure and lists the exact quotable lines from the beat.
|
|
|
|
## Tracklet hard constraints {#tracklet-hard-constraints}
|
|
|
|
**Closed.** Tracklet linking is transitive over similarity evidence and never across a hard constraint.
|
|
|
|
The hard constraints are gender and co-presence. Gender is canonicalized first. Vision emits
|
|
`m|f|unknown` while the registry says `male|female`, and the old gate tested only the second spelling,
|
|
so it never fired on real data. Co-presence means two detections in one panel are two people by
|
|
construction. A merge is rejected when any cross pair between the two groups violates either rule.
|
|
A weak chain can no longer bridge two people seen together.
|
|
|
|
Forbids: comparing a raw gender string against a literal enum anywhere in the identity path.
|
|
Check: `python tracklets.py` and `pytest test_tracklets.py`.
|
|
|
|
## Gated face pairing {#gated-face-pairing}
|
|
|
|
**Closed.** A detected face takes a character's identity only when its centre falls inside that
|
|
character's gemma bbox. The box is grown by 25% first. An unpaired face stays `unknown`.
|
|
|
|
This matters because the label becomes `speaker_method="som_face"`, the highest-trust provenance the
|
|
pipeline records. Unconditional nearest-neighbour pairing laundered a geometric guess into evidence.
|
|
Pairs are now taken globally shortest-first, so the first face processed cannot claim a character that
|
|
fits a later face better.
|
|
|
|
Set-of-mark attribution stays ON by default now that the pairing is gated. The docstring said "off by
|
|
default" while the flag read `SOM_ATTRIBUTION` with default `"1"`. The code and the comment now agree.
|
|
|
|
Forbids: trusting a `som_face` speaker without the containment gate.
|
|
Check: `python worker_vision.py`.
|
|
|
|
## No lease stealing {#no-lease-stealing}
|
|
|
|
**Closed.** A 409 from `/session/open` means another job legitimately holds the GPU. The proxy queues
|
|
on it. Only the session manager's TTL reaper clears a dead lease, because only it can tell a dead
|
|
lease from a busy one.
|
|
|
|
`session_proxy.open_session` previously closed the active lease and retried, so two jobs terminated
|
|
each other's models. Job admission control now bounds concurrent pipelines through
|
|
`MAX_CONCURRENT_JOBS`, default 1. A lease queue alone still lets two pipelines interleave stages
|
|
against one GPU.
|
|
|
|
`heartbeat_session` is finally called. `open_session` starts a beat task and `close_session` cancels
|
|
it, so a stage running past its TTL is no longer reaped mid-run. Beating from open and close keeps all
|
|
nine stage call sites unchanged.
|
|
|
|
Forbids: closing a session the caller did not open.
|
|
|
|
## Flag resolution {#flag-resolution}
|
|
|
|
**Closed.** Correctness flags resolve by flag id, and unresolved flags never wedge an autonomous run.
|
|
|
|
Two defects combined into a permanent block. `get_correctness_flags` accepted rows whose panel no
|
|
longer existed. A flag raised against a panel that a crop rerun deleted became visible to every
|
|
chapter and could never be resolved. `resolve_correctness_flags` could only touch flags whose
|
|
panel still existed. The view no longer admits orphans, and resolution takes explicit flag ids.
|
|
|
|
The TTS block also had no reachable exit. It refused to start on four flag kinds that only
|
|
`/review/approve` clears, and `GATES` defaults to off. With gates on, TTS now returns rather than
|
|
raises, so `awaiting_review` survives instead of being overwritten by `failed` in the pipeline's
|
|
catch-all. With gates off, the flags are logged and cleared, because no reviewer exists to clear them.
|
|
|
|
Forbids: raising out of a stage that has just set `awaiting_review`.
|
|
Check: `pytest test_name_binding.py` in the orchestrator.
|
|
|
|
## Hallucinated index {#hallucinated-index}
|
|
|
|
**Closed.** An out-of-range `choice` from `/vision/resolve` is `unresolved`, like a parse failure. Only
|
|
an explicit `0` means NONE and mints a new character. Mapping a bad index to NONE created a brand new
|
|
entry in the permanent registry from a hallucination.
|
|
|
|
Check: `python worker_vision.py`.
|
|
|
|
## Unlocked model load {#unlocked-model-load} [#203]
|
|
|
|
**Closed.** The session manager never holds `_lock` across a model load or a health wait.
|
|
|
|
`open_session` was already written that way, but it could orphan the server it spawned. A
|
|
`/session/close` arriving during the load found `proc = None`, tore down nothing, and cleared
|
|
`_active`. The finished server then held its VRAM unreferenced, and the next open spawned a second one
|
|
on the same port. Open now tears down its own process and reports 409 when its lease vanished mid-load.
|
|
|
|
`_supervise_once` had the opposite asymmetry. It called `_start_subprocess` while holding `_lock`,
|
|
which blocked `/session/active`, `/session/close`, and `/session/open` for the full health wait. It now
|
|
claims the respawn by clearing `proc`, spawns unlocked, and tears the new process down if the lease
|
|
disappeared meanwhile.
|
|
|
|
Check: `python session_manager.py`.
|
|
|
|
## Related
|
|
|
|
`_extract_json` in both `worker_vision.py` and `worker_script.py` now uses
|
|
`json.JSONDecoder().raw_decode` from the first brace. The greedy `\{.*\}` ran to the LAST brace in the
|
|
reply. A second object or trailing braced prose burned a repair call on a response that parsed fine.
|
|
|
|
`worker_scene.build_scene` emits `actions` as a list beside the joined `action` string, because
|
|
`correctness.build_beat_artifact` reads the plural key. The verifier was receiving no action evidence
|
|
at all. The beat builder also falls back to splitting the singular string.
|