Strip old dubtitle at mux + isolate it from pipeline context #4

Open
xenarathon wants to merge 24 commits from feat/strip-and-isolate-old-dubtitles into main
Owner

What

The pipeline embedded its output as a subtitle track named Dubtitles and then treated that track's presence as "done" — so a regeneration silently no-op'd unless a separate heavy pre-strip pass ran first. Worse, the track is codec=ass, language=eng, so every stage that reads the fansub as context picked it up as if it were the human fansub and repaired / aligned / mined the new run against the previous run's own mistakes.

Context isolation — one shared marker, common.TRACK_NAME:

  • common.eng_sub_streams() now queries stream_tags=language,title and drops any stream titled Dubtitles. It is the single chokepoint for repair.py, tools/timing_compare.py and dub_signs_merge.py, which inherit the fix with no code change.
  • mine_glossary.eng_sub_text() has its own selector and got the same exclusion.
  • No fallback by design: an episode whose only English sub is our old dubtitle runs reference-free rather than reading itself.

Strip-at-mux:

  • mux.keep_sub() drops any track named TRACK_NAME as its first check, so build_cmd's single mkvmerge pass drops the old track and appends the new one — a replace, not a duplicate, and self-healing if a past run left several.
  • mux.process() skips on the stamp alone; the ffprobe "already-muxed" backstop is gone (re-mux is now idempotent). generate.py's SKIP_IF_MUXED guard and its has_dubtitles_track() go with it.

Regeneration trigger:

  • common.PIPELINE_VERSION (1) is recorded in .dubtitles.done; stamp_valid() rejects anything older. GRANDFATHER_VERSION (1) covers pre-versioning stamps, so the rollout regenerates nothing. Bumping PIPELINE_VERSION is the operator's deliberate signal for a global in-place regeneration.
  • scripts/migrate_write_v1_stamps.py stamps files that carry a Dubtitles track but no stamp (dry-run by default), so removing the backstop doesn't trigger a mass regeneration on deploy.

Review findings fixed in this branch

Two review passes ran against the first cut; both found real defects, each now covered by a test:

  • -s is a whitelist and mkvmerge's default is copy-every-subtitle-track. Omitting -s on an empty keep list copied back the very track dropped reported as removed — so every mp4-origin episode, and the "only English sub is ours" case, would have ended up with two Dubtitles tracks. verify() is presence-only, so it would have passed and been stamped. An empty keep list now emits an explicit -S. (The test covering that path had asserted the broken command shape.)
  • A version bump could be silently defeated. The sidecar-existence skips aren't version-aware, so a file with a stale stamp and a leftover sidecar had generate return already-ass forever while mux re-embedded the old subtitle and stamped it current. Fixed via common.stale_version_stamp() + generate.discard_stale_sidecars(). A second pass caught that the first cut of that fix deleted freshly transcribed sidecars — so a sidecar now counts as a leftover only if it predates the stamp, and .dubtitles.fail episodes are exempt.
  • common.is_our_track() is now the one predicate for both track shapes (ffprobe tags.title, mkvmerge properties.track_name) so the exclude-from-context and drop-at-mux tests can't drift.
  • A failed write_stamp returned a bare error after the remux had already landed — a silent per-sweep full remux, forever. Now stamp-write-failed, logged loudly, sidecars kept for retry.
  • stamp_valid() no longer raises on a non-int version (that check runs outside mux.process()'s try, so one corrupt sidecar aborted a whole sweep); properties: None hardening across mux.

Testing

395 tests pass; ruff clean on the CI target (now including common.py, mine_glossary.py, scripts/). Every new guard was verified honest by reverting its hunk and watching the test fail.

Still pending — needs real media on the server:

  • Dry-run mux.py on an already-dubbed episode; expect drop-tracks=['sub:Dubtitles(old)'].
  • After a real --apply, confirm mkvmerge -J shows exactly one Dubtitles track. Use an mp4-origin episode — that's the shape the -S bug hid in.
  • Run scripts/migrate_write_v1_stamps.py (dry run, then --apply) before deploying, so no-stamp files don't mass-regenerate.
  • Bump PIPELINE_VERSION on a test show and confirm in-place regeneration.

Notes

  • strip_op.py (referenced in the spec's non-goals) does not exist in this repo and never has — nothing to deprecate.
  • The new Dubtitles track is appended last, so if an old one sat mid-list the dubtitle moves to the end. Players use the default-track flag; index-based scripts may see a different position. Documented in the README.

🤖 Generated with Claude Code

## What The pipeline embedded its output as a subtitle track named `Dubtitles` and then treated that track's presence as "done" — so a regeneration silently no-op'd unless a separate heavy pre-strip pass ran first. Worse, the track is `codec=ass, language=eng`, so every stage that reads the fansub *as context* picked it up as if it were the human fansub and repaired / aligned / mined the new run against the previous run's own mistakes. **Context isolation** — one shared marker, `common.TRACK_NAME`: - `common.eng_sub_streams()` now queries `stream_tags=language,title` and drops any stream titled `Dubtitles`. It is the single chokepoint for `repair.py`, `tools/timing_compare.py` and `dub_signs_merge.py`, which inherit the fix with no code change. - `mine_glossary.eng_sub_text()` has its own selector and got the same exclusion. - No fallback by design: an episode whose only English sub is our old dubtitle runs **reference-free** rather than reading itself. **Strip-at-mux:** - `mux.keep_sub()` drops any track named `TRACK_NAME` as its first check, so `build_cmd`'s single mkvmerge pass drops the old track and appends the new one — a replace, not a duplicate, and self-healing if a past run left several. - `mux.process()` skips on the stamp alone; the ffprobe "already-muxed" backstop is gone (re-mux is now idempotent). `generate.py`'s `SKIP_IF_MUXED` guard and its `has_dubtitles_track()` go with it. **Regeneration trigger:** - `common.PIPELINE_VERSION` (1) is recorded in `.dubtitles.done`; `stamp_valid()` rejects anything older. `GRANDFATHER_VERSION` (1) covers pre-versioning stamps, so **the rollout regenerates nothing**. Bumping `PIPELINE_VERSION` is the operator's deliberate signal for a global in-place regeneration. - `scripts/migrate_write_v1_stamps.py` stamps files that carry a `Dubtitles` track but no stamp (dry-run by default), so removing the backstop doesn't trigger a mass regeneration on deploy. ## Review findings fixed in this branch Two review passes ran against the first cut; both found real defects, each now covered by a test: - **`-s` is a whitelist and mkvmerge's default is copy-every-subtitle-track.** Omitting `-s` on an empty keep list copied back the very track `dropped` reported as removed — so every mp4-origin episode, and the "only English sub is ours" case, would have ended up with **two** `Dubtitles` tracks. `verify()` is presence-only, so it would have passed and been stamped. An empty keep list now emits an explicit `-S`. (The test covering that path had asserted the broken command shape.) - **A version bump could be silently defeated.** The sidecar-existence skips aren't version-aware, so a file with a stale stamp *and* a leftover sidecar had `generate` return `already-ass` forever while `mux` re-embedded the old subtitle and stamped it current. Fixed via `common.stale_version_stamp()` + `generate.discard_stale_sidecars()`. A second pass caught that the first cut of *that* fix deleted freshly transcribed sidecars — so a sidecar now counts as a leftover only if it **predates the stamp**, and `.dubtitles.fail` episodes are exempt. - `common.is_our_track()` is now the one predicate for both track shapes (ffprobe `tags.title`, mkvmerge `properties.track_name`) so the exclude-from-context and drop-at-mux tests can't drift. - A failed `write_stamp` returned a bare `error` after the remux had already landed — a silent per-sweep full remux, forever. Now `stamp-write-failed`, logged loudly, sidecars kept for retry. - `stamp_valid()` no longer raises on a non-int `version` (that check runs outside `mux.process()`'s `try`, so one corrupt sidecar aborted a whole sweep); `properties: None` hardening across `mux`. ## Testing 395 tests pass; `ruff` clean on the CI target (now including `common.py`, `mine_glossary.py`, `scripts/`). Every new guard was verified honest by reverting its hunk and watching the test fail. **Still pending — needs real media on the server:** - [ ] Dry-run `mux.py` on an already-dubbed episode; expect `drop-tracks=['sub:Dubtitles(old)']`. - [ ] After a real `--apply`, confirm `mkvmerge -J` shows exactly **one** `Dubtitles` track. Use an **mp4-origin** episode — that's the shape the `-S` bug hid in. - [ ] Run `scripts/migrate_write_v1_stamps.py` (dry run, then `--apply`) before deploying, so no-stamp files don't mass-regenerate. - [ ] Bump `PIPELINE_VERSION` on a test show and confirm in-place regeneration. ## Notes - `strip_op.py` (referenced in the spec's non-goals) does not exist in this repo and never has — nothing to deprecate. - The new `Dubtitles` track is appended last, so if an old one sat mid-list the dubtitle moves to the end. Players use the default-track flag; index-based scripts may see a different position. Documented in the README. --- 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Design for retiring the separate pre-strip pass: mux drops the old Dubtitles
track when embedding the new one, a pipeline-version stamp triggers in-place
regeneration, and every context reader (repair/timing-compare/signs-merge via
eng_sub_streams, plus mine_glossary) excludes the Dubtitles track so old-version
errors never influence the new output.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The pipeline embedded its output as a subtitle track named Dubtitles and then
treated that track's presence as done -- so a regeneration silently no-op'd
unless a separate heavy pre-strip pass ran first. Worse, the track is
codec=ass/language=eng, so every stage that reads the fansub as context picked
it up as if it were the human fansub and repaired/aligned/mined the new run
against the previous run's own mistakes.

Context isolation (one marker, common.TRACK_NAME):
- common.eng_sub_streams() now fetches stream_tags=title and drops any stream
  titled Dubtitles -- the single chokepoint for repair.py, timing_compare.py
  and dub_signs_merge.py, which inherit it with no code change.
- mine_glossary.eng_sub_text() has its own selector; same exclusion added.
- No fallback by design: an episode whose only English sub is our old dubtitle
  runs reference-free rather than reading itself.

Strip-at-mux:
- mux.keep_sub() drops any track named TRACK_NAME as its FIRST check, so
  build_cmd's single pass drops the old track and appends the new one --
  a replace, not a duplicate, and self-healing if a past run left several.
- mux.process() skips on the stamp alone; the ffprobe already-muxed backstop
  is gone (re-mux is now idempotent). generate.py's SKIP_IF_MUXED guard and its
  has_dubtitles_track() go with it.

Regeneration trigger:
- common.PIPELINE_VERSION (1) is recorded in .dubtitles.done; stamp_valid()
  rejects anything older. GRANDFATHER_VERSION (1) covers pre-versioning stamps,
  so the rollout regenerates nothing. Bumping PIPELINE_VERSION is the operator's
  deliberate signal for a global in-place regeneration.
- scripts/migrate_write_v1_stamps.py stamps files that carry a Dubtitles track
  but no stamp (dry-run by default), so removing the backstop doesn't trigger a
  mass regeneration on deploy.

377 tests pass; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six fixes from review, each covered by a test:

1. CRITICAL: build_cmd omitted -s when the keep list was empty. mkvmerge's -s is a
   WHITELIST and its default is copy-every-subtitle-track, so the old Dubtitles track
   was copied back and the file ended up with TWO — exactly the mp4-origin and
   only-sub-is-ours cases the feature exists for. verify() is presence-only, so it
   passed and got stamped. An empty keep list now emits an explicit -S.
   (The test for this path previously asserted the broken command shape.)

2. common.is_our_track() is now the one predicate for both track shapes (ffprobe
   tags.title via stream_title(), mkvmerge properties.track_name). keep_sub compared
   raw, so a padded track name was excluded from context but KEPT at mux — a silent
   duplicate. Also replaces mine_glossary's import of the private _track_title.

3. A version bump could be silently defeated: the sidecar-existence skips are not
   version-aware, so a file with a stale stamp AND a leftover sidecar (mux crashed
   after stamping) had generate return already-ass forever while mux re-embedded the
   OLD subtitle and stamped it current. common.stale_version_stamp() identifies that
   state; generate discards those sidecars and needs_work() lets process() reach them.

4. A failed stamp write returned a bare error after the remux had already landed —
   with the ffprobe backstop retired that means redoing a multi-GB mkvmerge every
   sweep, forever, invisibly. Now stamp-write-failed, logged loudly, sidecars kept.

5. stamp_valid() no longer raises TypeError on a non-int version; the check runs
   outside mux.process()'s try, so one corrupt sidecar aborted the whole sweep.

6. properties: None hardening across mux's track accessors; a present-but-null
   language tag no longer raises in mine_glossary.

Spec corrected: its orphaned-sidecar recovery claim (#3) was wrong, and the strip_op.py
non-goal referenced a file that never existed.

391 tests pass; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(strip-isolate): only discard sidecars that predate the stamp; exempt poison files
Some checks failed
tests / test (push) Failing after 3s
CI / test (push) Failing after 27s
tests / test (pull_request) Failing after 4s
CI / test (pull_request) Failing after 30s
146a6d1d32
Follow-up review found the first cut of the stale-sidecar fix rested on circular
reasoning -- it claimed generate could not have written a fresh sidecar because the
stale one blocks it, when unblocking generate is precisely what that fix does. The
stamp does not advance until mux succeeds, so a freshly transcribed sidecar sits
beside a still-stale stamp for at least one MERGE_INTERVAL, and indefinitely if the
mux keeps failing (skip-no-room, verify-*). Discarding it there re-ran Whisper on
every gen_loop.sh resume pass and, since the stall detector counts .srt files, the
deletions read as "no progress" and abandoned the show mid-regeneration.

- A sidecar is a LEFTOVER only if it predates the stamp: the run that stamped wrote
  its sidecars first and deleted them just after, so older-than-stamp belongs to that
  finished run and newer-than-stamp is this regeneration's own work.
- A .dubtitles.fail episode is exempt: it is never transcribed, so discarding its
  sidecars is pure destruction -- mux would have nothing to embed until the marker is
  cleared by hand. needs_work() checks the marker first too, so the two agree and a
  poisoned file no longer drags in the ~40s model load.
- Spec corrected: the circular safety argument is replaced with the real predicate.

Both guards verified honest by reverting each hunk in a scratch copy.
395 tests pass; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(migrate): distinguish a failed ffprobe from "no Dubtitles track"
Some checks failed
tests / test (push) Failing after 6s
tests / test (pull_request) Failing after 7s
CI / test (push) Failing after 43s
CI / test (pull_request) Failing after 42s
f29f30f293
has_dubtitles_track() wrapped everything in `except Exception: return False`, so an
unreadable or corrupt file was silently filed under `no-dubtitles` -- "not muxed yet,
the normal pipeline owns it". A run over a partly-broken library would therefore
report zero errors and read as clean while skipping exactly the files worth looking
at; the `error` status was only reachable from a failed stamp WRITE, so a 0 count
proved nothing about the 3587 files probed.

Now tri-state: True/False when ffprobe answers, None when it could not be run, timed
out, exited nonzero, or emitted unparseable output. process() maps None to its own
`probe-failed` status -- counted separately in the SUMMARY, never stamped -- and
main() prints an explicit warning when any occur.

Found by a dry run against the live 3587-file library.
399 tests pass; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The documented --raw path needs a dump_whisper.py capture that this repo does not
ship, so running a bake-off meant re-transcribing an episode on the GPU first --
which is why no bake-off had been run since the models changed.

--conf takes a production <stem>.dubtitles.conf.json instead. Those rows are already
post-reflow, post-hallucination-gate and post-collapse and carry every field
repair.is_target() reads (text, avg_logprob, no_speech_prob, word_probs) -- and it is
the very file repair.py consumes in production, so the models are judged on precisely
the lines the live pipeline would have sent them. No GPU, no reflow re-derivation.

Exactly one of --raw/--conf is required; a malformed conf (not a list, or a row with
no "text") exits loudly rather than silently producing empty prompts.

Verified against a real episode: 772 cards -> 163 targets.
407 tests pass; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Some models cannot be served by Ollama at all. Nanbeige 4.2-3B's GGUF needs a patched
llama.cpp -- `ollama create` refuses it with "failed to validate GGUF with
llama-quantize without compatibility patches" -- which is why it runs in its own
container behind a llama.cpp server. repair.py can already run such a model
(REPAIR_BACKEND=llamacpp + REPAIR_LLAMACPP_URL), so the bake-off had a gap: it could
not evaluate a model the pipeline is capable of using.

--llamacpp NAME=URL routes those model names to a llama.cpp /completion endpoint while
everything else still goes to Ollama. The request body mirrors repair.llm_llamacpp
exactly -- no model selector (the server has one model loaded), n_predict/stop bounded,
reply read from "content" -- so the bake-off measures the configuration that would
actually ship rather than an approximation of it.

ask() split into ask_ollama/ask_llamacpp behind a dispatching ask(); shared _post_json
and _first_line so both backends parse replies identically.

411 tests pass; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
repair.llm_llamacpp posts a RAW prompt to /completion, which applies no chat template.
Verified against the live Nanbeige 4.2-3B server: that path returns nothing but
newlines -- 200 tokens of "\n" -- because the instruct model never sees its template.
This is a latent bug in repair.py's llamacpp backend, not just a bake-off gap; it can
only work for a base/completion model.

--llamacpp-chat routes a candidate through /v1/chat/completions instead, which applies
the template, and passes chat_template_kwargs {enable_thinking: false}. Both are
required for this fork: with the template but thinking still on, the model fills
reasoning_content and returns an empty message (measured: empty after 114s at
max_tokens=512; correct output in 4.3s with thinking off).

An empty reply is surfaced as "<EMPTY thinking not disabled?>" rather than "" --
an empty string would score as "the model correctly left the line unchanged", turning
a broken configuration into a silent perfect no-op.

--llamacpp (raw) is kept so the shipping configuration can still be measured as-is.

414 tests pass; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(repair): restructure the prompt to stop glossary-name fabrication
Some checks failed
tests / test (push) Failing after 3s
tests / test (pull_request) Failing after 6s
CI / test (push) Failing after 46s
CI / test (pull_request) Failing after 43s
a4f7dd29de
Measured on real conf.json targets across three shows, qwen3.5:9b was rewriting a
large fraction of lines under the old prompt and pasting glossary names over text
that was already correct:

  This is Border Control.  -> This is Cipher Pol.
  Hey, Sonny,              -> Hey, Shanks,
  Oh, Neptune!             -> Oh, Nefertari Vivi!     (Neptune was already right)
  Straw Hat was innocent.  -> Straw Hat Pirates were innocent.
  Uchihime!                -> Uchiha!                 (a name from another franchise)

The old prompt already contained "NEVER replace a name in the line with a different
name", so restating the rule is not what fixes this. Two alternatives were tested and
rejected: stating it more forcefully, and removing the glossary from the prompt
entirely (still 38% -- the name list is NOT the trigger, and its removal made the
conservative model worse, 8% -> 18%).

What worked was structure:
  * the name list framed as VERIFICATION ONLY, not as material to apply
  * two worked examples -- one correcting a misspelling, one LEAVING an unlisted name
    alone, which is the behaviour prose failed to secure
  * nothing after the ASR line; context and reference now precede it

That last point is its own bug: an intermediate version put a trailing "Remember:"
reminder after the ASR line and the model echoed the rule text straight into the
subtitle output ("...friends in jail We're victims here, Remember: do not introduce
or swap names..."). A regression test now pins that nothing follows the ASR line.

Edit rate, old -> new (qwen3.5:9b, 40 targets each):
  One Pace              42% -> 25%
  Jujutsu Kaisen        28% -> 22%
  Delicious in Dungeon  22% -> 15%

Every glossary-name fabrication above is gone; the good repairs (human -centric ->
human-centric, sentence-boundary splits) survive.

NOT fixed: the model still invents phonetic names ("Syrahose" -> "Shyarros" on one
line and "Shirahoshi" on another -- same token, two answers). Prompt tuning has
plateaued there; tracked separately for a deterministic guard.

Also carries the bakeoff tooling used to measure this: --conf input, llama.cpp raw
and chat backends, and per-model incremental output so a timeout cannot discard
completed results.

421 tests pass; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat(repair): prompt that works for both models + fix the llama.cpp backend
Some checks failed
tests / test (push) Failing after 5s
tests / test (pull_request) Failing after 31s
CI / test (push) Failing after 50s
CI / test (pull_request) Failing after 53s
cc9469eaff
Goal: make a 3B model viable so the pipeline consolidates onto one small model that
others can actually run. Two things blocked that.

1. THE PROMPT. The previous one balanced badly across models:
     qwen3.5:9b     told only what NOT to do -> rewrote 42% of lines, pasting glossary
                    names over correct text (Neptune -> "Nefertari Vivi", Uchihime ->
                    "Uchiha", a name from a different franchise)
     nanbeige4.2-3b same prohibitions -> inert. 0 safe fixes across 120 targets,
                    returning input verbatim, losing the real repairs it had made

   A 6-variant sweep (3 shows x 40 targets, temperature 0, GPU residency verified before
   each run) found one prompt that fixes both. Measured, 120 targets:

     qwen3.5:9b      safe fixes  6 -> 23    name edits 17 -> 14
     nanbeige4.2-3b  safe fixes  0 -> 16    name edits  1 ->  2
     zero prompt leaks or length blowups for either

   What did it:
     - explicit POSITIVE DUTY. Rules phrased only as prohibitions produce a model that
       does nothing, which is not a repair stage.
     - REMOVING the worked example that showed a name being left alone. Counter-intuitive,
       but it over-anchored inaction; removing it was the single biggest gain AND name
       edits fell for both models.
     - dropping the blanket "if unsure, return UNCHANGED" escape, which a cautious model
       takes on every line. Restraint now attaches specifically to proper nouns, where the
       damage actually was.

2. THE LLAMA.CPP BACKEND, which nanbeige needs, was broken. llm_llamacpp posted a RAW
   prompt to /completion, applying no chat template; against a live Nanbeige server that
   returns nothing but newlines (200 tokens of "\n"). It could only ever have worked for a
   base/completion model. Now uses /v1/chat/completions with
   chat_template_kwargs.enable_thinking=false -- both required for this fork, which
   otherwise fills reasoning_content and returns an empty message (empty after 114s at
   max_tokens=512; correct output in 4.3s with thinking off). An empty reply is treated as
   "no repair", never as text to embed.

   REPAIR_LLAMACPP_URL default updated to the chat endpoint. The pre-existing test pinned
   the raw-/completion body, i.e. pinned the broken shape; updated with the reason.

Verified end to end: REPAIR_BACKEND=llamacpp against the live server repairs
"it worked Now we run" -> "It worked. Now we run.", fixes "human -centric", and leaves
"Sonny" alone.

Not fixed: qwen still invents names under this prompt (14 edits/120, incl. one glossary
paste, "catch Hirohoshi" -> "catch Crocodile"). nanbeige makes 2 name edits/120. That gap,
plus 4.13 GB vs 6.59 GB and ~3.4x faster, is the case for consolidating on nanbeige.

425 tests pass; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
recreate_srt.py rebuilds <stem>.eng.dubtitles.srt from the conf.json. mux removes
sidecars on success, and repair.py returns "skip" when the srt is absent -- so without
this tool in the image there is no way to re-run repair on an already-muxed episode
except by re-transcribing it from scratch.

That makes it a hard blocker for rolling any model or prompt change out to the existing
library: 295 of One Pace's 469 episodes are muxed with their sidecars gone, but all 461
conf.json files survive, and conf.json holds the ORIGINAL pre-repair transcription
(repair.py reads it and never writes it back). recreate_srt -> repair therefore re-repairs
cleanly from the original text, with no compounding of previous edits and no GPU time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(mux): verify the VIDEO track duration, not the container
Some checks failed
tests / test (push) Failing after 8s
tests / test (pull_request) Failing after 14s
CI / test (push) Failing after 58s
CI / test (pull_request) Failing after 1m7s
335bd88ea8
verify()'s truncation canary compared ffprobe's format=duration between source and
remux. In Matroska that is the LONGEST TRACK, not the video -- so it counts tracks this
stage deliberately drops.

Releases routinely ship a foreign subtitle that outlasts the picture. JUJUTSU KAISEN
S02E04 carries a Polish fansub track ending at 24:12.74 while the video ends at 23:54.85.
mux correctly drops it (not a keep language), the longest remaining track becomes audio,
and the container duration falls 18.9s -- eight times DUR_TOL. Measured on the real file:

  container: orig=1453.880  out=1434.952   delta=18.928s  -> rejected
  VIDEO:     orig=1434.850  out=1434.850   delta= 0.000s  -> correct

The remux was perfect every time. verify() rejected it, deleted the temp, and the merge
sweep retried it 10 minutes later, forever: that one episode's sidecars were assembled
2026-07-02 and it had been failing for 25 days. It was not alone -- a single sweep showed
11 consecutive verify-duration-mismatch failures across Demon Slayer and JUJUTSU KAISEN,
all multi-language AV1 releases, i.e. exactly the shape that ships an over-long sub.

video_duration() reads the first video stream: the duration field, then Matroska's
DURATION tag (usually where it actually lives, the field is N/A), then the container as a
last resort. duration() is kept -- it is still the right answer for "how long is this
file", just not for this check.

Note the irony: this duration check replaced a half-size heuristic that was removed for
false-positiving on compact muxes. It had the same defect, on a different axis.

431 tests pass; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(merge): dedup on layer+tags so stacked signs keep their white top layer
Some checks failed
tests / test (push) Failing after 4s
tests / test (pull_request) Failing after 12s
CI / test (push) Failing after 52s
CI / test (pull_request) Failing after 51s
53840abf84
Fansub typesetters build a sign from several events stacked on the same
\pos: a black backing copy supplying the stroke/drop-shadow on the lower
layer, and the visible copy above it (\bord0 plus a \t() animating the
fill to white). They share start, end, style and -- because the colour
lives entirely in override tags -- identical plaintext.

The dedup key was (start, end, style, plaintext), which made those two
events indistinguishable and kept only the first: the black backing.
Every credit, caption and title rendered as solid black text instead of
white-with-black-stroke.

Proven on One Pace S01E01, whose source tracks halve exactly under the
old key -- Credits-207+ 32->16, Captions-207+ 6->3, Credits 32->16 --
and come back whole under the new one (80 sign events, matching source).

Keying on the full override text plus the layer keeps compositions intact
while still collapsing byte-identical events, which is what the dedup was
for: the same sign carried by both the full track and the signs/songs
track. Both behaviours are now covered by tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
39 of the library's 79 releases ship more than one English ASS track, and
the same signs are typeset into each of them -- a full dialogue track, a
signs/songs track, often a CC track. build() read all of them, so every
sign rendered two or three times a few pixels apart. One Pace S01E01
carried the episode title three times over.

Add common.signs_sub_streams(): when any track names itself as the signs
track, that one is the release's curated signs list and the only one read;
several rival ones (DAN DA DAN ships MALD's and CR's) collapse to the
first. Titles that say nothing useful ("inid4c + SFX") fall back to the
historic scan of every track classified per event, which is how a single
mixed dialogue+signs track has always worked.

SIGNS_TITLE covers the spellings actually present in the library, taken
from a probe of all 79 releases: "Signs & Songs", "S&S", "Signs/Songs",
"English[Signs]", "Songs + Signs", "Signs and Songs(Hydes)", and "Forced"
(a forced track is signs plus foreign-dialogue captions, and releases that
ship ['Forced', ''] mean it as the signs track). Not "karaoke" -- FLE's
"Karaoke / English / ASS / FLE" is one mixed track, not a signs-only one.

eng_sub_streams() keeps its exact contract for repair.py and
timing_compare.py, which want the DIALOGUE track; both now share one probe
via eng_sub_tracks() so the is_our_track() exclusion cannot drift.

Verified on the real One Pace mkv: 80 sign events across three tracks ->
40 from the signs track alone, one title instead of three.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every v1 dubtitle has broken signs (plaintext dedup collapsing stacked
compositions to their black backing layer; signs lifted from every English
track at once), so the whole library needs regenerating. Bump
PIPELINE_VERSION -- the deliberate operator trigger -- to 2.

Roughly a third of built episodes no longer have their conf.json, so
recreate_srt.py cannot rebuild their sidecar and they would go back through
Whisper: ~850 episodes on a 6 GB 1060, days of GPU. Their dialogue is not
lost though -- it is sitting in the muxed track, carrying the "Dubtitles"
style, already transcribed/corrected/repaired. The signs beside it are
exactly what the rebuild replaces.

tools/recover_dub_srt.py lifts those events back into a sidecar, turning a
days-long GPU job into hours of remuxing. It is the one place the pipeline
reads its own output on purpose; that is not the leak eng_sub_streams()
guards against, since nothing is treated as a fansub reference and the
lines go straight back out as dialogue. It refuses to write an empty
sidecar (merge_pass discovers work BY the sidecar, so an empty one would
mux a dubtitle track with no dialogue) and never clobbers an existing one.

repair.process() now treats a missing conf.json as "skip" instead of
raising FileNotFoundError: merge_pass.sh calls repair.py unconditionally,
and a recovered episode has no conf. That text was repaired when first
built, so there is nothing to redo.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The migration ran while PIPELINE_VERSION == GRANDFATHER_VERSION, which is
what made its stamps read as current. The v2 bump deliberately makes them
stale again -- which the very next test already asserts -- so this one has
to pin the version it is actually describing rather than assume the
constants stay equal forever.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
build: ship tools/ in the image
Some checks failed
tests / test (push) Failing after 10s
tests / test (pull_request) Failing after 10s
CI / test (pull_request) Failing after 1m9s
CI / test (push) Failing after 1m21s
c20ff33902
recover_dub_srt.py has to run inside the container to reach the media, for
the same reason recreate_srt.py does: it is the only route that regenerates
an episode whose conf.json is gone without sending it back through Whisper.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
docs(review): status pass — most outstanding items were already fixed
Some checks failed
tests / test (push) Failing after 6s
tests / test (pull_request) Failing after 39s
CI / test (push) Failing after 51s
CI / test (pull_request) Failing after 52s
54c2b8de15
Verified each item in the latest addendum against the branch rather than
taking it at face value: the addendum was written where pytest could not
run, and most of what it lists as outstanding is already in-tree. Records
what is genuinely still open (the phonetic-name guard, whose specified
design is unsound), what is deliberate (default-track demotion), and the
two signs bugs found this pass that no earlier review caught.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The ollama/llamacpp dispatch lived only in repair.py, so glossary_verify
could reach nothing but Ollama -- the pipeline had to keep a second model
resident on a shared 8 GB GPU purely for adjudication.

Hoist the client into common.llm_chat(), with the llama.cpp specifics the
repair backend had to learn the hard way: /v1/chat/completions rather than
/completion (which applies no chat template -- a templated instruct model
answers it with nothing but newlines), enable_thinking=false (or the fork
spends its whole budget on reasoning_content and returns an empty message),
and no model selector (one model per server).

Two knobs adjudication needs that repair does not: max_tokens, since a JSON
object needs a real budget where a subtitle line needs 80; and
first_line=False, since taking the first line of a JSON object truncates it
to its opening brace.

VERIFY_BACKEND/VERIFY_LLAMACPP_URL default to the REPAIR_* values, so
pointing repair at a backend moves adjudication with it unless overridden.

Checked against the live servers on four real One Pace adjudications rather
than assumed: nanbeige and qwen3:8b agree on all four, and nanbeige answers
the hard one in 1.2s against qwen's 16.1s. Small sample, and the one case
they differ on ("Zolo" -> "Zolo" vs "Roronoa Zoro") is arguably right both
ways, since this stage picks the DUB-preferred spelling.

repair.py keeps its own dispatch for now -- it is the quality-critical path
and a library-wide regeneration is mid-flight; folding it onto llm_chat()
is a follow-up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An episode whose release ships no embedded signs never gets an .ass: the
merge returns "no-signs" and mux embeds the .srt directly, so its Dubtitles
track is codec subrip. pysubs2 styles every event of an SRT "Default", so
filtering on the "Dubtitles" style discarded those episodes wholesale --
37 of 50 in one stretch of the live seeding run came back "no-dialogue".

Such a track holds nothing but our dialogue (there are no signs in it to
confuse with it), so every event counts. The .ass path is unchanged: there
the non-Dubtitles events ARE the signs being replaced.

srt-ness has to come from ffprobe, not from the parsed file -- an extracted
SRT and a style-less ASS are indistinguishable by the time pysubs2 is done
with them -- so our_track_index() now returns the codec alongside the index.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(merge_pass): a missing shell/lib.sh killed the whole pass silently
Some checks failed
tests / test (push) Failing after 7s
tests / test (pull_request) Failing after 13s
CI / test (push) Failing after 57s
CI / test (pull_request) Failing after 58s
0c571707db
`.` is a POSIX special builtin: sourcing a file that does not exist is a
fatal error that terminates a non-interactive shell before `|| true` is
ever considered. So `. "$APP/shell/lib.sh" 2>/dev/null || true` did not
degrade to the inline fallback it sits next to -- it ended the script.

APP defaults to /scripts and only container_run.sh sets APP_DIR=/app, so
running merge_pass.sh directly (a targeted one-season pass, say) produced
no output at all, exited 2, and assembled nothing. The fallback regex the
comment promises for exactly that case was unreachable.

Guard with a file test, which is the POSIX-safe form.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(repair): reject repairs that lift their wording from the fansub reference
Some checks failed
tests / test (push) Failing after 4s
tests / test (pull_request) Failing after 5s
CI / test (push) Failing after 34s
CI / test (pull_request) Failing after 32s
6e94939922
The reference exists to disambiguate a garbled ASR line, not to supply its
text. A dubtitle has to match the DUB AUDIO, so a repair that swaps the
dub's phrasing for the fansub's yields a subtitle that reads perfectly and
is wrong against the sound -- the worst failure available here, because
nothing about it looks broken.

Measured across every repair summary the library had accumulated:

  qwen3:8b   2520 repairs -- 84.1% imported words from the reference (29.2% imported 3+)
  nanbeige   8456 repairs -- 52.5% imported words from the reference (17.1% imported 3+)

Both models do it; qwen far worse. Real lines that shipped:
  "That's enough of that, idiots!" -> "Hold it, you brats!"   (the reference, verbatim)
  "Let's go, Chopper."            -> "Well then, shall we go, Chopper?"

The only gate was a 0.4-2.5 length band, which a same-length rewrite sails
straight through, and which admitted "Huh?" -> "Huh? Help!" at exactly 2.5.

accept_repair() now also rejects a repair importing MAX_REF_BORROW (3) or
more NEW words present in the reference, and tightens the band to 0.6-1.5.
Both env-tunable so they can be adjusted without rebuilding the image.
Rejections are counted into the per-episode summary as rejected_guard, so
the guard's effect stays visible rather than silently shrinking the numbers.

Replayed over the 10971 repairs already on disk: blocks 26.0% of nanbeige's
and 37.1% of qwen's, while keeping 2733 of 2736 punctuation-only fixes and
name corrections like "Naya Yaskirihara" -> "Kirihara Naoyasu". The known
cost is a 3-word proper-noun correction ("Chinlong Tong" -> "Qing Long
Tang") now being refused; the deterministic glossary layer is the right
place for those anyway.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Some checks failed
tests / test (push) Failing after 4s
tests / test (pull_request) Failing after 5s
CI / test (push) Failing after 34s
CI / test (pull_request) Failing after 32s
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/strip-and-isolate-old-dubtitles:feat/strip-and-isolate-old-dubtitles
git switch feat/strip-and-isolate-old-dubtitles

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff feat/strip-and-isolate-old-dubtitles
git switch feat/strip-and-isolate-old-dubtitles
git rebase main
git switch main
git merge --ff-only feat/strip-and-isolate-old-dubtitles
git switch feat/strip-and-isolate-old-dubtitles
git rebase main
git switch main
git merge --no-ff feat/strip-and-isolate-old-dubtitles
git switch main
git merge --squash feat/strip-and-isolate-old-dubtitles
git switch main
git merge --ff-only feat/strip-and-isolate-old-dubtitles
git switch main
git merge feat/strip-and-isolate-old-dubtitles
git push origin main
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
xenarathon/DubTitlerr!4
No description provided.