Audit Phase 1: correctness and scheduling safety
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
This commit is contained in:
@@ -0,0 +1,29 @@
|
||||
# decisions/
|
||||
|
||||
One entry per settled question about this pipeline. An entry names the claim, the evidence, the date,
|
||||
and what it forbids or permits next.
|
||||
|
||||
No entry is a plan. `NEXT.md` holds the plan, `ROADMAP.md` holds the horizon, and a limitation that is
|
||||
still live belongs in `caveats/`.
|
||||
|
||||
## Rules for this directory
|
||||
|
||||
* One file per topic, one `##` section per question, so the index can link an anchor.
|
||||
* An entry cites a file and line, a commit, or a test. No citation means it is an opinion, and an
|
||||
opinion gets deleted rather than defended.
|
||||
* **Closed** means acting on it is safe. **Open** means investigated and undecided. **Void** means the
|
||||
evidence turned out to be invalid and the claim must not be cited again.
|
||||
* Rewrite an entry when something contradicts it, and say what changed.
|
||||
|
||||
## Index
|
||||
|
||||
| decision | state |
|
||||
| --- | --- |
|
||||
| [A dialogue row's speaker is `speaker_ref`, not `speaker`](audit-phase1.md#speaker-ref-is-canonical) | closed |
|
||||
| [Script verification must not fail on sentence-initial capitals or short quotes](audit-phase1.md#verifier-false-positives) | closed |
|
||||
| [Tracklet links are transitive over similarity, never across a hard constraint](audit-phase1.md#tracklet-hard-constraints) | closed |
|
||||
| [A detected face takes an identity only when it falls inside that character's box](audit-phase1.md#gated-face-pairing) | closed |
|
||||
| [A 409 from the GPU mutex is a queue signal, not a stale lease](audit-phase1.md#no-lease-stealing) | closed |
|
||||
| [Correctness flags resolve by flag id, and never block autonomous TTS](audit-phase1.md#flag-resolution) | closed |
|
||||
| [An out-of-range resolver index is `unresolved`, never a new character](audit-phase1.md#hallucinated-index) | closed |
|
||||
| [The session manager holds no lock across a model load](audit-phase1.md#unlocked-model-load) | closed |
|
||||
@@ -0,0 +1,144 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user