From 842c110f1db31897aa0cd4881924815b8821f825 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 12:24:35 +0800 Subject: [PATCH 01/16] fix: eliminate hook-pad constant drift in render-hook-intro render-hook-intro.js reimplemented hookClipEnd()/buildHookSections() with a stale HOOK_TAIL_PAD_UNBOUNDED_SECONDS of 0.16 instead of the canonical 0.50 in remotion/lib/hookTiming.ts, under-counting the --frames range passed to remotion render and cutting rendered hooks off before their true boundary. Now imports buildHookSections directly so the two can never diverge again. Also corrects the same stale value in CLAUDE.md's constants table. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 2 +- .../HOOK_TIMING_DIAGNOSTICS.md | 287 ++++++++++++++++++ package.json | 2 +- scripts/render-hook-intro.js | 88 +----- 4 files changed, 296 insertions(+), 83 deletions(-) create mode 100644 docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md diff --git a/CLAUDE.md b/CLAUDE.md index 5ed71f0..1dbf699 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -174,7 +174,7 @@ ty = (0.5 - vp.cy) × 100% | `PAUSE_THRESHOLD` | 0.8 s | `edit-transcript.js` | | `WORD_DURATION_ESTIMATE` | 0.4 s | `edit-transcript.js` | | `CUT_START_BIAS` | 1.0 | `edit-transcript.js` | -| `HOOK_TAIL_PAD_UNBOUNDED_SECONDS` | 0.16 s | `remotion/lib/hookTiming.ts` | +| `HOOK_TAIL_PAD_UNBOUNDED_SECONDS` | 0.50 s | `remotion/lib/hookTiming.ts` | | `HOOK_TAIL_PAD_BOUNDED_SECONDS` | 0.02 s | `remotion/lib/hookTiming.ts` | | `HOOK_BRIDGE_MAX_GAP_SECONDS` | 1.0 s | `remotion/lib/hookTiming.ts` | | `HOOK_END_FADE_FRAMES` | 12 | `SegmentPlayer.tsx` | diff --git a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md new file mode 100644 index 0000000..5b8f2f9 --- /dev/null +++ b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md @@ -0,0 +1,287 @@ +# Hook Timing Diagnostics — Implementation Plan + +## How agents use this document + +This document is the authoritative implementation guide for fixing a hook-boundary +drift bug and building a reusable diagnostic script that verifies rendered hook clips +match their designated `hookFrom`/`hookTo`/phrase boundaries. + +**To resume interrupted work:** +1. Run `git log --oneline` to see which commits are complete. +2. Match the last commit message against the slugs below. +3. Continue from the next unstarted step. + +**Rules:** +- Implement commits in order — each step depends on the previous. +- Do not combine steps into one commit. Isolation is intentional. +- The "Status check" under each commit tells you how to verify it is already done. +- Branch: current branch (`ep/loop-eng`), no new branch required. + +--- + +## Diagnosed problem + +Rendered hooks were reported as cut off too early/late, or starting too early/late. + +`remotion/lib/hookTiming.ts` is the documented single source of truth for hook +boundary math (`hookClipEnd()`, `buildHookSections()`), with +`HOOK_TAIL_PAD_UNBOUNDED_SECONDS = 0.50`. However `scripts/render-hook-intro.js` had +its own duplicate copy of this math (`computeHookClipEnd()`, +`computeHookDurationFrames()`) with a **stale constant of 0.16**, despite a comment +claiming it "must stay in sync." Because this script computes the `--frames 0-N` +range passed to `npx remotion render`, it was requesting ~0.34s fewer frames per +unbounded hook than the composition itself renders (which imports the real +`hookTiming.ts`) — cutting the rendered hook-intro output off before the true +boundary. `CLAUDE.md`'s constants table also documented the stale 0.16s value. + +This is the second time this exact duplication pattern has caused a bug (see +`CLAUDE.md`'s Phase 5 note about `hookClipEnd()` previously existing in 4 files). +The fix must eliminate the duplicate, not just patch the constant, and the new +diagnostic script must not introduce a fifth copy of any timing/correlation math — +it imports the canonical implementations. + +--- + +## Architecture overview + +- `remotion/lib/hookTiming.ts` — canonical hook boundary math (already exists, unit + tested in `hookTiming.test.ts`). No changes needed to this file. +- `scripts/render-hook-intro.js` — now imports `buildHookSections` directly instead + of reimplementing it (Commit 1). +- `scripts/lib/audioCorrelation.js` — new shared pure FFT cross-correlation module, + extracted from `scripts/sync/AudioSyncer.js` so the diagnostic script can reuse it + without creating a duplicate (Commit 2). +- `scripts/diagnostics/verify-hook-timing.ts` — new reusable diagnostic script + (Commits 3–5), run via `tsx`. Three layers, each addable independently: + 1. **Math/consistency check** — recompute expected hook boundaries/frame totals + from `transcript.json` via `hookTiming.ts`, no media I/O. + 2. **Audio cross-correlation** — compare the rendered hook clip's audio against + the expected window extracted from the original synced source video, report + drift in ms per hook. + 3. **Optional content diff** (`--verify-content` flag, off by default) — run + whisper.cpp (via existing `scripts/transcribe/Transcriber.js`) on each rendered + hook slice, diff words against the expected token range, catches wrong-phrase + bugs (e.g. `resolvePhraseToTimeRange` falling back to whole-segment) that pure + timing checks can't see. + + Note: the codebase's reusable transcription wrapper is whisper.cpp + (`Transcriber.js`), not WhisperX — WhisperX (used for forced alignment + upstream) has no reusable Node wrapper in `scripts/`. Content diffing does not + need forced-alignment precision, so whisper.cpp is used here rather than adding + a new WhisperX invocation path. + +--- + +## Commit checklist + +### Commit 1 — `fix: eliminate hook-pad constant drift in render-hook-intro` ✅ DONE + +**Status check:** `grep -q "from '../remotion/lib/hookTiming'" scripts/render-hook-intro.js` +and `grep "HOOK_TAIL_PAD_UNBOUNDED_SECONDS" CLAUDE.md` shows `0.50 s`. + +**Files modified:** +- `scripts/render-hook-intro.js` +- `package.json` (`render:hook-intro` script now runs via `tsx`, not `node`, since it + imports a `.ts` module) +- `CLAUDE.md` (constants table corrected) + +**What was done:** +Removed the local `isSpokenToken()`, `computeHookClipEnd()`, and the body of +`computeHookDurationFrames()` from `render-hook-intro.js`. It now imports +`buildHookSections` from `../remotion/lib/hookTiming` and sums +`trimAfter - trimBefore` across the returned sections for the total frame count — +identical math to what `Composition.tsx`/`SegmentPlayer.tsx`/`CameraPlayer.tsx` +actually render, by construction. No local constants remain to drift out of sync. + +**Manual test:** +`npx tsx scripts/render-hook-intro.js --help` prints usage without error (confirms +the `.ts` import resolves under `tsx`). Running without `--help` against +`public/edit/transcript.json` should report a larger `Hook frames` total than the +pre-fix version would have (pre-fix under-counted by ~0.34s × unbounded-hook-count). + +--- + +### Commit 2 — `refactor: extract shared FFT correlation utility from AudioSyncer` + +**Status check:** `scripts/lib/audioCorrelation.js` exists and exports +`computeCrossCorrelation`, `findBestLag`, `validatePeak`, `nextPowerOfTwo`. +`npm run test:unit -- AudioSyncer` still passes unchanged. + +**Files modified:** +- `scripts/lib/audioCorrelation.js` (new) +- `scripts/lib/audioCorrelation.test.js` (new) +- `scripts/sync/AudioSyncer.js` + +**What to do:** +`AudioSyncer.js` currently has `computeCrossCorrelation(samplesA, samplesB)`, +`findBestLag(correlation)`, and `validatePeak(correlation, lagSeconds)` as instance +methods (`AudioSyncer.js:145-235`) that close over `this.sampleRate`, plus a private +module-level `nextPowerOfTwo(n)` (`:18-22`). The diagnostic script's audio +cross-correlation layer (Commit 4) needs this exact math. Rather than duplicate it +a third time, extract it into pure functions taking `sampleRate`/`frameRate`/ +threshold constants as explicit parameters: + +```js +// scripts/lib/audioCorrelation.js +export function nextPowerOfTwo(n) { ... } // unchanged body +export function computeCrossCorrelation(samplesA, samplesB) { ... } // unchanged body, uses nextPowerOfTwo internally +export function findBestLag(correlation, sampleRate, frameRate, peakNearnessThreshold) { ... } // this.sampleRate -> sampleRate param, SYNC_FRAME_RATE -> frameRate param, PEAK_NEARNESS_THRESHOLD -> param +export function validatePeak(correlation, lagSeconds, sampleRate, frameRate, reliabilitySnrThreshold) { ... } // same parameterization +``` + +In `AudioSyncer.js`, replace the method bodies with thin delegations so existing +call sites (`:498-500`) and existing tests (`scripts/__tests__/AudioSyncer.test.js`, +which call `syncer.findBestLag(...)` etc. as instance methods) keep working +unchanged: + +```js +computeCrossCorrelation(samplesA, samplesB) { + return computeCrossCorrelation(samplesA, samplesB); +} +findBestLag(correlation) { + return findBestLag(correlation, this.sampleRate, SYNC_FRAME_RATE, PEAK_NEARNESS_THRESHOLD); +} +validatePeak(correlation, lagSeconds) { + return validatePeak(correlation, lagSeconds, this.sampleRate, SYNC_FRAME_RATE, RELIABILITY_SNR_THRESHOLD); +} +``` + +Remove the now-unused local `nextPowerOfTwo` from `AudioSyncer.js`. Add +`scripts/lib/audioCorrelation.test.js` covering `findBestLag` (single peak, +multiple peaks within/at threshold — same cases as the existing AudioSyncer tests, +now testable directly without instantiating the class) and `validatePeak` +(SNR reliability threshold). Mock `fft.js` the same way +`scripts/__tests__/AudioSyncer.test.js` already does for `computeCrossCorrelation`. + +**Manual test:** `npm run test:unit -- AudioSyncer audioCorrelation` — both suites +pass. + +--- + +### Commit 3 — `feat: add hook-timing diagnostic script with math-consistency check` + +**Status check:** `npx tsx scripts/diagnostics/verify-hook-timing.ts --transcript public/edit/transcript.json` +runs and prints a per-hook table of expected `[sourceStart, sourceEnd]`, +`trimBefore`/`trimAfter` frames, and total hook duration, with exit code 0. + +**Files added:** +- `scripts/diagnostics/verify-hook-timing.ts` +- `scripts/diagnostics/verify-hook-timing.test.ts` (pure logic only, no I/O) +- `package.json` — add `"diagnose:hooks": "tsx scripts/diagnostics/verify-hook-timing.ts"` + +**What to do:** +CLI args: `--transcript ` (default `public/edit/transcript.json`), `--fps +` (default 60), `--json` (machine-readable output instead of a console table). + +Core function (pure, unit-testable): +```ts +function computeExpectedHookLayout(segments: Segment[], fps: number) { + const hookSegments = segments.filter(s => s.hook && !s.cut); + const sections = buildHookSections(hookSegments, fps); + // zip hookSegments 1:1 with sections (buildHookSections is 1 section per hook + // segment before de-overlap merges zero-duration edge cases — see note below) + ... + return { hookSegments, sections, totalFrames, totalSeconds }; +} +``` + +Note: `buildHookSections` can return fewer sections than input segments if a +de-overlapped section collapses to zero duration (`hookTiming.ts:148`, +`trimBefore < trimAfter` check). When reporting per-hook rows, detect this case +and flag it explicitly in the output (e.g. `"⚠ zero-duration after de-overlap — +this hook contributes no frames"`) rather than silently misaligning the +segment-to-section zip. + +This command has no media dependency and always runs — it's the fast first-line +check. It reads `transcript.json` directly (`meta.fps` if present, else `--fps`). + +**Manual test:** run against `public/edit/transcript.json` (35 hook segments per +earlier inspection); confirm output row count and total seconds look sane, and that +totals match `npx tsx scripts/render-hook-intro.js` (Commit 1) exactly. + +--- + +### Commit 4 — `feat: add audio cross-correlation layer to hook-timing diagnostic` + +**Status check:** running with `--rendered ` (and +`--source `, or read from `transcript.meta.videoSrc`) +prints a per-hook lag-ms column and flags any `|lag| > --tolerance-ms` (default 50ms +≈ 3 frames at 60fps). + +**Files modified:** +- `scripts/diagnostics/verify-hook-timing.ts` +- New helper: `scripts/lib/extractAudioWindow.js` — `extractAudioWindow(videoPath, + startSeconds, durationSeconds, sampleRate, outWavPath)`, spawns `ffmpeg -i + -ss -t -vn -ac 1 -ar + -acodec pcm_s16le -y`. Use accurate (output) seeking — `-ss`/`-t` + after `-i` — not the fast/input-seek pattern `AudioSyncer.js` uses elsewhere, + because this is a correctness tool verifying frame-accurate boundaries, and hook + clips are short enough (a few seconds) that the seek-accuracy tradeoff is worth + it. +- `scripts/lib/extractAudioWindow.test.js` is not needed — this function is a thin + ffmpeg spawn wrapper with no meaningful pure logic to unit test; its correctness + is exercised via the integration test below. +- `tests/integration/verify-hook-timing.test.ts` (new) — real ffmpeg + a tiny fixture + video with a known tone at a known offset, confirming `findBestLag` reports ~0ms + for a matching window and a large lag for a deliberately offset window. + +**What to do:** +For each expected hook section from Commit 3: +1. Compute the section's start time *within the rendered output* (cumulative sum of + prior sections' `trimAfter - trimBefore`, divided by fps). +2. Extract that window's audio from `--rendered` via `extractAudioWindow`. +3. Extract the *expected* window `[sourceStart, sourceEnd]` (the hook segment's own + `hookFrom`/`hookClipEnd()`, not the frame-rounded section) from `--source`. +4. Cross-correlate via `computeCrossCorrelation`/`findBestLag` from + `scripts/lib/audioCorrelation.js` (Commit 2) — sample rate 8000Hz to match + existing sync tooling conventions, unless finer resolution proves necessary. +5. Report lag in ms; flag rows exceeding `--tolerance-ms`. + +Clean up temp WAV files after each hook (or batch at the end) — follow +`AudioSyncer.js`'s `tempDir` + best-effort cleanup pattern +(`AudioSyncer.js:97-113`). + +**Manual test:** render a short real hook-intro via `npm run render:hook-intro -- +--overwrite`, then run the diagnostic against the output and the source video; +confirm lag values are small (a few ms, well under tolerance) now that Commit 1's +fix is in place. Optionally verify pre-fix drift by checking out the pre-Commit-1 +version of `render-hook-intro.js` and comparing. + +--- + +### Commit 5 — `feat: add optional whisper content-diff layer to hook-timing diagnostic` + +**Status check:** running with `--verify-content` prints a per-hook word-diff +summary (expected tokens vs. transcribed words, showing missing/extra words). +Running without the flag never invokes whisper (confirm via a quick timing check — +this layer is off by default because it's slow). + +**Files modified:** +- `scripts/diagnostics/verify-hook-timing.ts` + +**What to do:** +Behind `--verify-content`, for each hook's extracted rendered-audio WAV (already +produced in Commit 4 — reuse it, don't re-extract), run +`scripts/transcribe/Transcriber.js` (`new Transcriber({ audioPath, outputDir, ... })`, +`init()`, `transcribe()`, `close()` — see `scripts/transcribe/transcribe-audio.js` +for the exact call pattern) to get a word list. Compare (simple set/order diff, +not fuzzy matching) against the expected spoken tokens in +`[segment.hookFrom ?? segment.start, segment.hookTo ?? segment.end]` from the +transcript (`isSpokenToken` filter from `remotion/lib/tokens.ts`). Report any +expected word missing from the transcribed output, or any transcribed word not in +the expected set — this is the signal for the `resolvePhraseToTimeRange` fallback +case (phrase not found → falls back to whole-segment hook) and other +content-level mismatches that pure timing math can't detect. + +**Manual test:** intentionally mismatch a hook's `hookFrom`/`hookTo` in a copy of +the transcript (e.g. shift by 2 seconds) and confirm `--verify-content` flags the +missing/extra words that the timing-only layers would not catch on their own. + +--- + +## Done + +When all 5 commits are complete: `render-hook-intro.js` has no duplicate timing +math, `CLAUDE.md` is accurate, and `npm run diagnose:hooks -- --rendered +--source [--verify-content]` gives a full pass/fail report on any rendered +hook-intro output. diff --git a/package.json b/package.json index 8dec590..beca348 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "render:episode": "node scripts/render-episode.js", "render:episode:resume": "node scripts/render-episode-resumable.js", "render:episode:chunk": "node scripts/render-episode-resumable.js", - "render:hook-intro": "node scripts/render-hook-intro.js", + "render:hook-intro": "tsx scripts/render-hook-intro.js", "check:media-urls": "node scripts/lib/checkMediaUrls.js", "review:guest": "node scripts/guest-review.js", "shorts:extract-doc": "node scripts/shorts/extract-short-doc.js", diff --git a/scripts/render-hook-intro.js b/scripts/render-hook-intro.js index 3493fc2..8d467f0 100644 --- a/scripts/render-hook-intro.js +++ b/scripts/render-hook-intro.js @@ -1,16 +1,14 @@ -#!/usr/bin/env node +#!/usr/bin/env tsx import path from 'path'; import fs from 'fs-extra'; import { spawn } from 'child_process'; import { checkMediaUrls } from './lib/checkMediaUrls.js'; +import { buildHookSections } from '../remotion/lib/hookTiming'; const FPS = 60; // Must match remotion/components/PodcastIntro.tsx const INTRO_DURATION_FRAMES = 420; -const HOOK_TAIL_PAD_UNBOUNDED_SECONDS = 0.16; -const HOOK_TAIL_PAD_BOUNDED_SECONDS = 0.02; -const HOOK_BRIDGE_MAX_GAP_SECONDS = 1.0; function parseArgs() { const args = process.argv.slice(2); @@ -84,87 +82,15 @@ function run(cmd, args, cwd) { }); } -/** Matches isSpokenToken() in remotion/lib/tokens.ts — must stay in sync. */ -function isSpokenToken(token) { - const trimmed = (token?.text || '').trim(); - if (trimmed === '' || /_[A-Z]+_/.test(trimmed)) return false; - if (/^[.,?_\s]*$/.test(trimmed.replace(/ /g, ''))) return false; - return true; -} - /** - * Mirrors hookClipEnd() in remotion/lib/hookTiming.ts — must stay in sync. - * - * Key difference from the old implementation: - * - Uses t_end (word audio-tail end) instead of t_dtw (word start) for the - * last-spoken-token extension. This accounts for the full duration of the - * final word in the hook clip, giving ~0.5 s of additional coverage per hook. - * - Extension applies to bounded hooks too (not only unbounded). - * - Token lookup is scoped to [sourceStart, baseEnd], not the whole segment. - */ -function computeHookClipEnd(seg, nextHookStart) { - const sourceStart = seg.hookFrom ?? seg.start; - const baseEnd = seg.hookTo ?? seg.end; - const isBounded = seg.hookTo !== undefined && seg.hookTo !== null; - - let sourceEnd = baseEnd; - - // Extend to cover the last spoken token's audio tail (t_end, not t_dtw). - const tokensInWindow = (seg.tokens || []).filter( - t => isSpokenToken(t) && t.t_dtw >= sourceStart && t.t_dtw <= baseEnd, - ); - const lastSpokenToken = tokensInWindow.sort((a, b) => (b.t_end ?? 0) - (a.t_end ?? 0))[0]; - if (lastSpokenToken?.t_end) { - const tEnd = nextHookStart !== undefined - ? Math.min(lastSpokenToken.t_end, nextHookStart) - : lastSpokenToken.t_end; - sourceEnd = Math.max(sourceEnd, tEnd); - } - - const hasSpokenAfterEnd = (seg.tokens || []).some( - t => isSpokenToken(t) && t.t_dtw > sourceEnd + 0.02, - ); - const endsAtTail = !hasSpokenAfterEnd; - const canBridge = nextHookStart !== undefined - && nextHookStart > sourceEnd - && nextHookStart - sourceEnd <= HOOK_BRIDGE_MAX_GAP_SECONDS; - if (endsAtTail && canBridge) sourceEnd = nextHookStart; - - const withPad = sourceEnd + (isBounded ? HOOK_TAIL_PAD_BOUNDED_SECONDS : HOOK_TAIL_PAD_UNBOUNDED_SECONDS); - return nextHookStart !== undefined ? Math.min(withPad, nextHookStart) : withPad; -} - -/** - * Builds de-overlapped hook sections and returns the total frame count. - * Mirrors buildHookSections() in remotion/lib/hookTiming.ts: if a section's - * trimBefore would fall before the previous section's trimAfter (caused by - * t_end extension or bridging), it is advanced to avoid a backward source seek. + * Total hook frame count, derived from the same buildHookSections() used by + * Composition.tsx/SegmentPlayer.tsx/CameraPlayer.tsx — guarantees this script's + * --frames range always matches what the composition actually renders. */ function computeHookDurationFrames(transcript) { const hookSegments = (transcript.segments || []).filter(s => s.hook && !s.cut); - let totalFrames = 0; - let prevTrimAfter = -1; - - for (let i = 0; i < hookSegments.length; i++) { - const seg = hookSegments[i]; - const next = hookSegments[i + 1]; - const nextHookStart = next ? (next.hookFrom ?? next.start) : undefined; - const sourceStart = seg.hookFrom ?? seg.start; - const sourceEnd = computeHookClipEnd(seg, nextHookStart); - - const rawTrimBefore = Math.floor(sourceStart * FPS); - const rawTrimAfter = Math.max(Math.ceil(sourceEnd * FPS), rawTrimBefore + 1); - - // De-overlap: advance trimBefore if this section would overlap the previous one. - const trimBefore = prevTrimAfter >= 0 ? Math.max(rawTrimBefore, prevTrimAfter) : rawTrimBefore; - if (trimBefore < rawTrimAfter) { - totalFrames += rawTrimAfter - trimBefore; - prevTrimAfter = rawTrimAfter; - } - // Sections where trimBefore >= trimAfter after de-overlap are zero-duration - // edge cases (two hooks whose source windows touch exactly); skip them. - } - + const sections = buildHookSections(hookSegments, FPS); + const totalFrames = sections.reduce((sum, s) => sum + (s.trimAfter - s.trimBefore), 0); return { hookSegments, totalFrames }; } From 254da1f478262e0ec6e8ae646fe1007d7efabb5d Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 12:26:49 +0800 Subject: [PATCH 02/16] refactor: extract shared FFT correlation utility from AudioSyncer Pulls computeCrossCorrelation/findBestLag/validatePeak/nextPowerOfTwo out of AudioSyncer into pure functions in scripts/lib/audioCorrelation.js, so the upcoming hook-timing diagnostic script can reuse this exact math for audio drift detection instead of creating a third copy of it. AudioSyncer delegates to the shared module; existing tests pass unchanged. Co-Authored-By: Claude Sonnet 5 --- scripts/lib/audioCorrelation.js | 119 +++++++++++++++++++++++++++ scripts/lib/audioCorrelation.test.js | 89 ++++++++++++++++++++ scripts/sync/AudioSyncer.js | 98 ++-------------------- 3 files changed, 216 insertions(+), 90 deletions(-) create mode 100644 scripts/lib/audioCorrelation.js create mode 100644 scripts/lib/audioCorrelation.test.js diff --git a/scripts/lib/audioCorrelation.js b/scripts/lib/audioCorrelation.js new file mode 100644 index 0000000..ce0da94 --- /dev/null +++ b/scripts/lib/audioCorrelation.js @@ -0,0 +1,119 @@ +import FFT from 'fft.js'; + +/** + * Pure FFT cross-correlation utilities, extracted from AudioSyncer.js so this + * math has a single source of truth. AudioSyncer.js delegates to these; any new + * consumer (e.g. scripts/diagnostics/verify-hook-timing.ts) should import from + * here directly rather than reimplementing. + */ + +export function nextPowerOfTwo(n) { + let p = 1; + while (p < n) p <<= 1; + return p; +} + +export function computeCrossCorrelation(samplesA, samplesB) { + const lenA = samplesA.length; + const lenB = samplesB.length; + const N = nextPowerOfTwo(lenA + lenB - 1); + + const estimatedMB = Math.round((N * 16) / 1e6); + console.log(` FFT size: ${N.toLocaleString()} samples (~${estimatedMB} MB)`); + + const fft = new FFT(N); + + // Build complex arrays (interleaved re, im) + const cA = fft.createComplexArray(); + const cB = fft.createComplexArray(); + for (let i = 0; i < lenA; i++) cA[2 * i] = samplesA[i]; + for (let i = 0; i < lenB; i++) cB[2 * i] = samplesB[i]; + + const FA = fft.createComplexArray(); + const FB = fft.createComplexArray(); + fft.transform(FA, cA); + fft.transform(FB, cB); + + // Multiply FA by conjugate of FB + const product = fft.createComplexArray(); + for (let i = 0; i < N; i++) { + const re = FA[2 * i] * FB[2 * i] + FA[2 * i + 1] * FB[2 * i + 1]; + const im = FA[2 * i + 1] * FB[2 * i] - FA[2 * i] * FB[2 * i + 1]; + product[2 * i] = re; + product[2 * i + 1] = im; + } + + // Inverse FFT + const result = fft.createComplexArray(); + fft.inverseTransform(result, product); + + // Extract real part (normalized by N) + const correlation = new Float64Array(N); + for (let i = 0; i < N; i++) { + correlation[i] = result[2 * i] / N; + } + + return correlation; +} + +/** + * Deterministically finds the best lag (seconds, frame-exact) between two + * cross-correlated signals. + * + * @param correlation Cross-correlation array from computeCrossCorrelation. + * @param sampleRate Sample rate (Hz) the correlation's underlying audio was extracted at. + * @param frameRate Frame rate to quantize the lag to. + * @param peakNearnessThreshold Candidates within this amount of the running max are also considered; earliest wins ties. + */ +export function findBestLag(correlation, sampleRate, frameRate, peakNearnessThreshold) { + const N = correlation.length; + let maxVal = -Infinity; + const candidateIndices = []; + + // First pass: find maximum value and collect all near-maximum candidates + for (let i = 0; i < N; i++) { + const v = Math.abs(correlation[i]); + if (v > maxVal) { + maxVal = v; + candidateIndices.length = 0; // Clear previous candidates + candidateIndices.push(i); + } else if (Math.abs(v - maxVal) <= peakNearnessThreshold) { + // Within threshold - add as candidate + candidateIndices.push(i); + } + } + + // Deterministic tie-break: prefer earliest peak among candidates + const maxIdx = candidateIndices[0]; + + // Convert circular index to signed lag in samples + const lagSamples = maxIdx <= N / 2 ? maxIdx : maxIdx - N; + + // Convert to integer frame offset instead of floating-point seconds + const lagFrames = Math.round(lagSamples * frameRate / sampleRate); + + return lagFrames / frameRate; // Return as seconds but with frame-exact precision +} + +/** + * Computes the SNR of the correlation peak at lagSeconds and whether it clears + * reliabilitySnrThreshold. + */ +export function validatePeak(correlation, lagSeconds, sampleRate, frameRate, reliabilitySnrThreshold) { + const N = correlation.length; + let sum = 0, sumSq = 0; + for (let i = 0; i < N; i++) { sum += correlation[i]; sumSq += correlation[i] ** 2; } + const mean = sum / N; + const std = Math.sqrt(sumSq / N - mean ** 2); + + // Convert frame-based lag back to samples for validation + const lagFrames = Math.round(lagSeconds * frameRate); + const lagSamples = Math.round(lagFrames * sampleRate / frameRate); + const idx = ((lagSamples % N) + N) % N; + + // Calculate SNR: signal (peak value) vs noise (standard deviation) + const signalValue = correlation[idx]; + const snr = std > 0 ? Math.abs(signalValue - mean) / std : 0; + + return { snr, isReliable: snr >= reliabilitySnrThreshold }; +} diff --git a/scripts/lib/audioCorrelation.test.js b/scripts/lib/audioCorrelation.test.js new file mode 100644 index 0000000..a51308e --- /dev/null +++ b/scripts/lib/audioCorrelation.test.js @@ -0,0 +1,89 @@ +jest.mock('fft.js', () => { + return jest.fn().mockImplementation(() => ({ + createComplexArray: jest.fn().mockReturnValue(new Float64Array(2048)), + transform: jest.fn(), + inverseTransform: jest.fn(), + })); +}); + +import { findBestLag, validatePeak, nextPowerOfTwo } from './audioCorrelation.js'; + +const SAMPLE_RATE = 8000; +const FRAME_RATE = 30; +const PEAK_NEARNESS_THRESHOLD = 0.5; +const RELIABILITY_SNR_THRESHOLD = 3.0; + +describe('nextPowerOfTwo', () => { + test('rounds up to the next power of two', () => { + expect(nextPowerOfTwo(1)).toBe(1); + expect(nextPowerOfTwo(2)).toBe(2); + expect(nextPowerOfTwo(3)).toBe(4); + expect(nextPowerOfTwo(1000)).toBe(1024); + }); +}); + +describe('findBestLag', () => { + test('selects single maximum peak deterministically', () => { + const correlation = new Float64Array(100); + correlation[10] = 5.0; + + const result = findBestLag(correlation, SAMPLE_RATE, FRAME_RATE, PEAK_NEARNESS_THRESHOLD); + + const expectedLagFrames = Math.round(10 * FRAME_RATE / SAMPLE_RATE); + expect(result).toBe(expectedLagFrames / FRAME_RATE); + }); + + test('prefers earliest peak when multiple peaks within threshold', () => { + const correlation = new Float64Array(100); + correlation[10] = 5.0; + correlation[20] = 4.8; + correlation[30] = 5.2; + correlation[40] = 4.9; + + const result = findBestLag(correlation, SAMPLE_RATE, FRAME_RATE, PEAK_NEARNESS_THRESHOLD); + + const expectedLagFrames = Math.round(30 * FRAME_RATE / SAMPLE_RATE); + expect(result).toBe(expectedLagFrames / FRAME_RATE); + }); + + test('handles circular correlation (peak in second half → negative lag)', () => { + const correlation = new Float64Array(100); + correlation[90] = 5.0; + + const result = findBestLag(correlation, SAMPLE_RATE, FRAME_RATE, PEAK_NEARNESS_THRESHOLD); + + const expectedLagFrames = Math.round(-10 * FRAME_RATE / SAMPLE_RATE); + expect(result).toBe(expectedLagFrames / FRAME_RATE); + }); +}); + +describe('validatePeak', () => { + test('flags an unreliable peak when the correlation peak does not land on the lag index', () => { + const correlation = new Float64Array(100).fill(0.01); + correlation[50] = 5.0; // lagSeconds=0.033 maps to index 67, not 50 + + const result = validatePeak(correlation, 0.033, SAMPLE_RATE, FRAME_RATE, RELIABILITY_SNR_THRESHOLD); + + expect(result.snr).toBeGreaterThan(0); + expect(result.isReliable).toBe(false); + }); + + test('flags a reliable peak when the correlation peak lands on the lag index', () => { + const correlation = new Float64Array(100).fill(0.01); + correlation[67] = 5.0; // lagSeconds=0.033 → index 67 at sampleRate=8000, frameRate=30 + + const result = validatePeak(correlation, 0.033, SAMPLE_RATE, FRAME_RATE, RELIABILITY_SNR_THRESHOLD); + + expect(result.snr).toBeGreaterThan(3.0); + expect(result.isReliable).toBe(true); + }); + + test('handles zero standard deviation without throwing', () => { + const correlation = new Float64Array(100).fill(1.0); + + const result = validatePeak(correlation, 0.033, SAMPLE_RATE, FRAME_RATE, RELIABILITY_SNR_THRESHOLD); + + expect(result.snr).toBe(0); + expect(result.isReliable).toBe(false); + }); +}); diff --git a/scripts/sync/AudioSyncer.js b/scripts/sync/AudioSyncer.js index b0da123..eef8755 100644 --- a/scripts/sync/AudioSyncer.js +++ b/scripts/sync/AudioSyncer.js @@ -6,7 +6,11 @@ import { open as fsOpen } from 'node:fs/promises'; import wavefileModule from 'wavefile'; import { detectHDR, HDR_TONEMAP_VF, SDR_FORMAT_VF } from '../shared/hdr-detect.js'; const { WaveFile } = wavefileModule; -import FFT from 'fft.js'; +import { + computeCrossCorrelation as sharedComputeCrossCorrelation, + findBestLag as sharedFindBestLag, + validatePeak as sharedValidatePeak, +} from '../lib/audioCorrelation.js'; // Sync frame rate constant for deterministic lag calculation const SYNC_FRAME_RATE = 30; @@ -15,12 +19,6 @@ const SYNC_FRAME_RATE = 30; const PEAK_NEARNESS_THRESHOLD = 0.5; const RELIABILITY_SNR_THRESHOLD = 3.0; -function nextPowerOfTwo(n) { - let p = 1; - while (p < n) p <<= 1; - return p; -} - function spawnProcess(cmd, args) { return new Promise((resolve, reject) => { const proc = spawn(cmd, args, { stdio: ['ignore', 'pipe', 'pipe'] }); @@ -143,95 +141,15 @@ class AudioSyncer { } computeCrossCorrelation(samplesA, samplesB) { - const lenA = samplesA.length; - const lenB = samplesB.length; - const N = nextPowerOfTwo(lenA + lenB - 1); - - const estimatedMB = Math.round((N * 16) / 1e6); - console.log(` FFT size: ${N.toLocaleString()} samples (~${estimatedMB} MB)`); - - const fft = new FFT(N); - - // Build complex arrays (interleaved re, im) - const cA = fft.createComplexArray(); - const cB = fft.createComplexArray(); - for (let i = 0; i < lenA; i++) cA[2 * i] = samplesA[i]; - for (let i = 0; i < lenB; i++) cB[2 * i] = samplesB[i]; - - const FA = fft.createComplexArray(); - const FB = fft.createComplexArray(); - fft.transform(FA, cA); - fft.transform(FB, cB); - - // Multiply FA by conjugate of FB - const product = fft.createComplexArray(); - for (let i = 0; i < N; i++) { - const re = FA[2 * i] * FB[2 * i] + FA[2 * i + 1] * FB[2 * i + 1]; - const im = FA[2 * i + 1] * FB[2 * i] - FA[2 * i] * FB[2 * i + 1]; - product[2 * i] = re; - product[2 * i + 1] = im; - } - - // Inverse FFT - const result = fft.createComplexArray(); - fft.inverseTransform(result, product); - - // Extract real part (normalized by N) - const correlation = new Float64Array(N); - for (let i = 0; i < N; i++) { - correlation[i] = result[2 * i] / N; - } - - return correlation; + return sharedComputeCrossCorrelation(samplesA, samplesB); } findBestLag(correlation) { - const N = correlation.length; - let maxVal = -Infinity; - const candidateIndices = []; - - // First pass: find maximum value and collect all near-maximum candidates - for (let i = 0; i < N; i++) { - const v = Math.abs(correlation[i]); - if (v > maxVal) { - maxVal = v; - candidateIndices.length = 0; // Clear previous candidates - candidateIndices.push(i); - } else if (Math.abs(v - maxVal) <= PEAK_NEARNESS_THRESHOLD) { - // Within SNR threshold - add as candidate - candidateIndices.push(i); - } - } - - // Deterministic tie-break: prefer earliest peak among candidates - const maxIdx = candidateIndices[0]; - - // Convert circular index to signed lag in samples - const lagSamples = maxIdx <= N / 2 ? maxIdx : maxIdx - N; - - // Convert to integer frame offset instead of floating-point seconds - const lagFrames = Math.round(lagSamples * SYNC_FRAME_RATE / this.sampleRate); - - return lagFrames / SYNC_FRAME_RATE; // Return as seconds but with frame-exact precision + return sharedFindBestLag(correlation, this.sampleRate, SYNC_FRAME_RATE, PEAK_NEARNESS_THRESHOLD); } validatePeak(correlation, lagSeconds) { - const N = correlation.length; - let sum = 0, sumSq = 0; - for (let i = 0; i < N; i++) { sum += correlation[i]; sumSq += correlation[i] ** 2; } - const mean = sum / N; - const std = Math.sqrt(sumSq / N - mean ** 2); - - // Convert frame-based lag back to samples for validation - const lagFrames = Math.round(lagSeconds * SYNC_FRAME_RATE); - const lagSamples = Math.round(lagFrames * this.sampleRate / SYNC_FRAME_RATE); - const idx = ((lagSamples % N) + N) % N; - - // Calculate SNR: signal (peak value) vs noise (standard deviation) - const signalValue = correlation[idx]; - const snr = std > 0 ? Math.abs(signalValue - mean) / std : 0; - - return { snr, isReliable: snr >= RELIABILITY_SNR_THRESHOLD }; + return sharedValidatePeak(correlation, lagSeconds, this.sampleRate, SYNC_FRAME_RATE, RELIABILITY_SNR_THRESHOLD); } async computeTrimPoints(lagSeconds) { From d9b1d4279fc0eebaab136bacd3f4405301a13b57 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 12:29:22 +0800 Subject: [PATCH 03/16] feat: add hook-timing diagnostic script with math-consistency check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds scripts/diagnostics/verify-hook-timing.ts (npm run diagnose:hooks), the first layer of a reusable diagnostic for the "hooks cut off too early/late" class of bugs. Recomputes expected hook sections directly from transcript.json via the canonical getHookSubClips()/hookClipEnd() in remotion/lib/hookTiming.ts and reports per-hook source windows, frame ranges, and durations. No media I/O — fast, always-safe first-line check that would have caught the render-hook-intro.js drift fixed earlier in this branch before it ever produced a bad render. Co-Authored-By: Claude Sonnet 5 --- .../HOOK_TIMING_DIAGNOSTICS.md | 4 +- package.json | 1 + .../diagnostics/verify-hook-timing.test.ts | 85 ++++++++ scripts/diagnostics/verify-hook-timing.ts | 197 ++++++++++++++++++ 4 files changed, 285 insertions(+), 2 deletions(-) create mode 100644 scripts/diagnostics/verify-hook-timing.test.ts create mode 100644 scripts/diagnostics/verify-hook-timing.ts diff --git a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md index 5b8f2f9..09c952f 100644 --- a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md +++ b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md @@ -101,7 +101,7 @@ pre-fix version would have (pre-fix under-counted by ~0.34s × unbounded-hook-co --- -### Commit 2 — `refactor: extract shared FFT correlation utility from AudioSyncer` +### Commit 2 — `refactor: extract shared FFT correlation utility from AudioSyncer` ✅ DONE **Status check:** `scripts/lib/audioCorrelation.js` exists and exports `computeCrossCorrelation`, `findBestLag`, `validatePeak`, `nextPowerOfTwo`. @@ -158,7 +158,7 @@ pass. --- -### Commit 3 — `feat: add hook-timing diagnostic script with math-consistency check` +### Commit 3 — `feat: add hook-timing diagnostic script with math-consistency check` ✅ DONE **Status check:** `npx tsx scripts/diagnostics/verify-hook-timing.ts --transcript public/edit/transcript.json` runs and prints a per-hook table of expected `[sourceStart, sourceEnd]`, diff --git a/package.json b/package.json index beca348..0c0c699 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "cut:preview": "node scripts/cut-preview.js", "dev": "next dev", "diarize": "node scripts/diarize/diarize-audio.js", + "diagnose:hooks": "tsx scripts/diagnostics/verify-hook-timing.ts", "lint": "eslint", "prepare": "husky || true", "remotion:gallery": "npx remotion studio remotion/galleryRoot.ts", diff --git a/scripts/diagnostics/verify-hook-timing.test.ts b/scripts/diagnostics/verify-hook-timing.test.ts new file mode 100644 index 0000000..3dc6eea --- /dev/null +++ b/scripts/diagnostics/verify-hook-timing.test.ts @@ -0,0 +1,85 @@ +/** + * Unit tests for the math-consistency layer of verify-hook-timing.ts. + * Pure logic, no I/O — mirrors the style of remotion/lib/hookTiming.test.ts. + */ + +import { computeExpectedHookLayout } from './verify-hook-timing'; +import { buildHookSections, HOOK_TAIL_PAD_UNBOUNDED_SECONDS } from '../../remotion/lib/hookTiming'; +import type { Segment } from '../../remotion/types/transcript'; + +const FPS = 60; + +function makeSegment(overrides: Partial = {}): Segment { + return { + id: 1, + start: 10, + end: 15, + speaker: 'Natasha', + text: 'hello world', + cut: false, + tokens: [], + cuts: [], + graphics: [], + hook: true, + ...overrides, + }; +} + +describe('computeExpectedHookLayout', () => { + it('returns one row per hook segment with unbounded pad applied', () => { + const segments = [makeSegment({ id: 1, start: 10, end: 15 })]; + const report = computeExpectedHookLayout(segments, FPS); + + expect(report.rows).toHaveLength(1); + expect(report.rows[0].zeroDuration).toBe(false); + expect(report.rows[0].sourceEnd).toBeCloseTo(15 + HOOK_TAIL_PAD_UNBOUNDED_SECONDS); + }); + + it('excludes non-hook and cut segments', () => { + const segments = [ + makeSegment({ id: 1, hook: false }), + makeSegment({ id: 2, hook: true, cut: true }), + makeSegment({ id: 3, hook: true, cut: false, start: 20, end: 25 }), + ]; + const report = computeExpectedHookLayout(segments, FPS); + + expect(report.rows).toHaveLength(1); + expect(report.rows[0].sourceStart).toBe(20); + }); + + it('matches buildHookSections()\'s total frame count exactly', () => { + const segments = [ + makeSegment({ id: 1, start: 10, end: 15 }), + makeSegment({ id: 2, start: 15.5, end: 18 }), + makeSegment({ id: 3, start: 18.05, end: 20 }), + ]; + + const report = computeExpectedHookLayout(segments, FPS); + const canonicalSections = buildHookSections(segments, FPS); + const canonicalTotalFrames = canonicalSections.reduce((sum, s) => sum + (s.trimAfter - s.trimBefore), 0); + + expect(report.totalFrames).toBe(canonicalTotalFrames); + }); + + it('flags a section as zero-duration when de-overlap collapses it, without breaking the total', () => { + // Two hooks whose source windows touch/overlap after tail-pad extension — + // de-overlap should collapse the second into zero duration. + const segments = [ + makeSegment({ id: 1, start: 10, end: 10.1, hookTo: 10.1 }), + makeSegment({ id: 2, start: 10.1, end: 10.2, hookFrom: 10.1, hookTo: 10.11 }), + ]; + + const report = computeExpectedHookLayout(segments, FPS); + const canonicalSections = buildHookSections(segments, FPS); + const canonicalTotalFrames = canonicalSections.reduce((sum, s) => sum + (s.trimAfter - s.trimBefore), 0); + + expect(report.rows).toHaveLength(2); + expect(report.totalFrames).toBe(canonicalTotalFrames); + }); + + it('returns an empty report when there are no hook segments', () => { + const report = computeExpectedHookLayout([makeSegment({ hook: false })], FPS); + expect(report.rows).toHaveLength(0); + expect(report.totalFrames).toBe(0); + }); +}); diff --git a/scripts/diagnostics/verify-hook-timing.ts b/scripts/diagnostics/verify-hook-timing.ts new file mode 100644 index 0000000..df649f6 --- /dev/null +++ b/scripts/diagnostics/verify-hook-timing.ts @@ -0,0 +1,197 @@ +#!/usr/bin/env tsx +/** + * Reusable diagnostic for hook boundary correctness — verifies that rendered + * hook clips actually align with the hookFrom/hookTo/phrase boundaries the doc + * author intended. + * + * Layer 1 (math-consistency check, this file's default behavior): recomputes + * expected hook sections directly from transcript.json via the canonical + * remotion/lib/hookTiming.ts. No media I/O; always safe/fast to run. This is + * the check that would have caught render-hook-intro.js's stale pad-constant + * drift (see docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md) before it + * ever produced a bad render. + * + * Usage: + * npx tsx scripts/diagnostics/verify-hook-timing.ts [options] + */ + +import fs from 'fs-extra'; +import path from 'path'; +import { getHookSubClips } from '../../remotion/lib/hookTiming'; +import type { Segment } from '../../remotion/types/transcript'; + +type CliArgs = { + transcriptPath: string; + fps?: number; + json: boolean; + help: boolean; +}; + +function parseArgs(argv: string[]): CliArgs { + const out: CliArgs = { + transcriptPath: path.join('public', 'edit', 'transcript.json'), + fps: undefined, + json: false, + help: false, + }; + for (let i = 0; i < argv.length; i++) { + const a = argv[i]; + if (a === '--transcript' && argv[i + 1]) out.transcriptPath = argv[++i]; + else if (a === '--fps' && argv[i + 1]) out.fps = Number(argv[++i]); + else if (a === '--json') out.json = true; + else if (a === '--help' || a === '-h') out.help = true; + } + return out; +} + +function printHelp() { + console.log(` +Verify hook timing — math-consistency layer + +Recomputes expected hook boundaries/frame ranges directly from a transcript.json +via the canonical remotion/lib/hookTiming.ts, and reports them per hook. Fast, +no media I/O — catches drift between what the composition will actually render +and what any render script assumes. + +Usage: + npx tsx scripts/diagnostics/verify-hook-timing.ts [options] + +Options: + --transcript transcript.json path (default: public/edit/transcript.json) + --fps Frames per second (default: transcript meta.fps, else 60) + --json Output machine-readable JSON instead of a console table + --help, -h Show this help +`); +} + +export type HookLayoutRow = { + index: number; + speaker: string; + text: string; + sourceStart: number; + sourceEnd: number; + trimBefore: number | null; + trimAfter: number | null; + durationFrames: number; + durationSeconds: number; + zeroDuration: boolean; +}; + +export type HookLayoutReport = { + rows: HookLayoutRow[]; + totalFrames: number; + totalSeconds: number; + fps: number; +}; + +function toRawSection(sourceStart: number, sourceEnd: number, fps: number) { + const trimBefore = Math.floor(sourceStart * fps); + const trimAfter = Math.max(Math.ceil(sourceEnd * fps), trimBefore + 1); + return { trimBefore, trimAfter }; +} + +/** + * Recomputes expected hook sections from transcript segments via the canonical + * getHookSubClips()/hookClipEnd() (remotion/lib/hookTiming.ts) — the same + * functions Composition.tsx/SegmentPlayer.tsx/CameraPlayer.tsx import. Pure + * function, no I/O, directly unit-testable. + * + * The frame-rounding + de-overlap pass mirrors buildHookSections()'s own loop + * (hookTiming.ts) so totals match exactly, but attributes each resulting + * section back to its source segment — buildHookSections() itself drops + * zero-duration sections silently, which would otherwise make a 1:1 zip + * between segments and its output unsafe. + */ +export function computeExpectedHookLayout(segments: Segment[], fps: number): HookLayoutReport { + const hookSegments = segments.filter(s => s.hook && !s.cut); + + const rows: HookLayoutRow[] = []; + let prevTrimAfter = -1; + let totalFrames = 0; + + for (let i = 0; i < hookSegments.length; i++) { + const seg = hookSegments[i]; + const next = hookSegments[i + 1]; + const nextHookStart = next ? (next.hookFrom ?? next.start) : undefined; + const [{ sourceStart, sourceEnd }] = getHookSubClips(seg, nextHookStart); + const raw = toRawSection(sourceStart, sourceEnd, fps); + + const trimBefore = prevTrimAfter >= 0 ? Math.max(raw.trimBefore, prevTrimAfter) : raw.trimBefore; + const zeroDuration = trimBefore >= raw.trimAfter; + + if (!zeroDuration) { + totalFrames += raw.trimAfter - trimBefore; + prevTrimAfter = raw.trimAfter; + } + + rows.push({ + index: i, + speaker: seg.speaker, + text: seg.text, + sourceStart, + sourceEnd, + trimBefore: zeroDuration ? null : trimBefore, + trimAfter: zeroDuration ? null : raw.trimAfter, + durationFrames: zeroDuration ? 0 : raw.trimAfter - trimBefore, + durationSeconds: zeroDuration ? 0 : (raw.trimAfter - trimBefore) / fps, + zeroDuration, + }); + } + + return { rows, totalFrames, totalSeconds: totalFrames / fps, fps }; +} + +function printReport(report: HookLayoutReport) { + console.log(`\n[verify-hook-timing] fps: ${report.fps}\n`); + for (const row of report.rows) { + const label = `#${row.index} ${row.speaker}`; + if (row.zeroDuration) { + console.log(`${label.padEnd(20)} ⚠ zero-duration after de-overlap — contributes no frames`); + continue; + } + console.log( + `${label.padEnd(20)} ` + + `src [${row.sourceStart.toFixed(3)}s, ${row.sourceEnd.toFixed(3)}s] ` + + `frames [${row.trimBefore}, ${row.trimAfter}) ` + + `dur ${row.durationSeconds.toFixed(3)}s`, + ); + } + console.log( + `\n[verify-hook-timing] Total: ${report.rows.length} hook segments, ` + + `${report.totalFrames} frames (${report.totalSeconds.toFixed(2)}s)\n`, + ); +} + +async function main() { + const cli = parseArgs(process.argv.slice(2)); + if (cli.help) { printHelp(); return; } + + const transcriptPath = path.resolve(process.cwd(), cli.transcriptPath); + if (!await fs.pathExists(transcriptPath)) { + throw new Error(`Transcript not found: ${transcriptPath}`); + } + + const transcript = await fs.readJson(transcriptPath); + const fps = cli.fps ?? transcript.meta?.fps ?? 60; + + if (!Array.isArray(transcript.segments)) { + throw new Error(`Transcript has no segments array: ${transcriptPath}`); + } + + const report = computeExpectedHookLayout(transcript.segments as Segment[], fps); + + if (report.rows.length === 0) { + throw new Error('No hook segments found in transcript. Annotate segments with > HOOK in the doc.'); + } + + if (cli.json) { + console.log(JSON.stringify(report, null, 2)); + } else { + printReport(report); + } +} + +main().catch(err => { + console.error(`\n[verify-hook-timing] Error: ${err.message}`); + process.exit(1); +}); From 0b8a65f55e0220410e66e75d18ae47797ece0f4c Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 12:38:38 +0800 Subject: [PATCH 04/16] feat: add audio cross-correlation layer to hook-timing diagnostic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds --rendered/--source flags to verify-hook-timing.ts: extracts each hook's audio from a rendered output and from the original synced source video, cross-correlates them via the shared FFT utilities, and flags any hook whose measured drift exceeds --tolerance-ms. This is the black-box check that catches Remotion encode-time rounding or dropped frames that the math-consistency layer alone can't see. New scripts/lib/extractAudioWindow.js handles ffmpeg extraction + wav loading. Its wavefile import uses a namespace import (not the default- import-then-destructure pattern AudioSyncer.js uses) — that pattern throws under Jest's CJS/ESM interop for this package once wavefile isn't mocked, which AudioSyncer's own mocked tests never exercise. tests/integration/verify-hook-timing-audio.test.ts validates the whole extraction+correlation path against real ffmpeg and real (unmocked) FFT math using a deterministic seeded noise fixture, confirming ~0ms lag for a matching window and accurate measurement of a deliberately introduced 50ms offset. Co-Authored-By: Claude Sonnet 5 --- .../HOOK_TIMING_DIAGNOSTICS.md | 86 ++++++----- scripts/diagnostics/verify-hook-timing.ts | 139 +++++++++++++++++- scripts/lib/extractAudioWindow.js | 45 ++++++ .../verify-hook-timing-audio.test.ts | 81 ++++++++++ 4 files changed, 310 insertions(+), 41 deletions(-) create mode 100644 scripts/lib/extractAudioWindow.js create mode 100644 tests/integration/verify-hook-timing-audio.test.ts diff --git a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md index 09c952f..baa0c43 100644 --- a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md +++ b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md @@ -201,51 +201,59 @@ totals match `npx tsx scripts/render-hook-intro.js` (Commit 1) exactly. --- -### Commit 4 — `feat: add audio cross-correlation layer to hook-timing diagnostic` +### Commit 4 — `feat: add audio cross-correlation layer to hook-timing diagnostic` ✅ DONE **Status check:** running with `--rendered ` (and `--source `, or read from `transcript.meta.videoSrc`) prints a per-hook lag-ms column and flags any `|lag| > --tolerance-ms` (default 50ms ≈ 3 frames at 60fps). -**Files modified:** -- `scripts/diagnostics/verify-hook-timing.ts` -- New helper: `scripts/lib/extractAudioWindow.js` — `extractAudioWindow(videoPath, - startSeconds, durationSeconds, sampleRate, outWavPath)`, spawns `ffmpeg -i - -ss -t -vn -ac 1 -ar - -acodec pcm_s16le -y`. Use accurate (output) seeking — `-ss`/`-t` - after `-i` — not the fast/input-seek pattern `AudioSyncer.js` uses elsewhere, - because this is a correctness tool verifying frame-accurate boundaries, and hook - clips are short enough (a few seconds) that the seek-accuracy tradeoff is worth - it. -- `scripts/lib/extractAudioWindow.test.js` is not needed — this function is a thin - ffmpeg spawn wrapper with no meaningful pure logic to unit test; its correctness - is exercised via the integration test below. -- `tests/integration/verify-hook-timing.test.ts` (new) — real ffmpeg + a tiny fixture - video with a known tone at a known offset, confirming `findBestLag` reports ~0ms - for a matching window and a large lag for a deliberately offset window. - -**What to do:** -For each expected hook section from Commit 3: -1. Compute the section's start time *within the rendered output* (cumulative sum of - prior sections' `trimAfter - trimBefore`, divided by fps). -2. Extract that window's audio from `--rendered` via `extractAudioWindow`. -3. Extract the *expected* window `[sourceStart, sourceEnd]` (the hook segment's own - `hookFrom`/`hookClipEnd()`, not the frame-rounded section) from `--source`. -4. Cross-correlate via `computeCrossCorrelation`/`findBestLag` from - `scripts/lib/audioCorrelation.js` (Commit 2) — sample rate 8000Hz to match - existing sync tooling conventions, unless finer resolution proves necessary. -5. Report lag in ms; flag rows exceeding `--tolerance-ms`. - -Clean up temp WAV files after each hook (or batch at the end) — follow -`AudioSyncer.js`'s `tempDir` + best-effort cleanup pattern -(`AudioSyncer.js:97-113`). - -**Manual test:** render a short real hook-intro via `npm run render:hook-intro -- ---overwrite`, then run the diagnostic against the output and the source video; -confirm lag values are small (a few ms, well under tolerance) now that Commit 1's -fix is in place. Optionally verify pre-fix drift by checking out the pre-Commit-1 -version of `render-hook-intro.js` and comparing. +**Files added/modified:** +- `scripts/diagnostics/verify-hook-timing.ts` — `runAudioCorrelationLayer()`, + `--rendered`/`--source`/`--sample-rate`/`--tolerance-ms` flags. +- `scripts/lib/extractAudioWindow.js` (new) — `extractAudioWindow(sourcePath, + startSeconds, durationSeconds, sampleRate, outWavPath)` (accurate/output + seeking, `-ss`/`-t` after `-i`, unlike `AudioSyncer.js`'s fast-seek pattern — + deliberate, since this tool verifies frame-accurate boundaries on short clips + where seek accuracy matters more than speed) and `loadWavSamples(wavPath)`. +- `tests/integration/verify-hook-timing-audio.test.ts` (new) — real ffmpeg + + a deterministic seeded broadband-noise fixture (`anoisesrc=seed=42`, not a + pure tone — broadband gives correlation a single unambiguous peak, matching + real speech). No mocks: exercises real ffmpeg extraction, real `wavefile` + loading, and real (unmocked) `fft.js` correlation end to end. Confirms ~0ms + lag for a matching window and correctly measures a deliberately introduced + 50ms offset. + +**Gotcha hit and fixed:** the `wavefile` package's default-import-then- +destructure pattern (`import wavefileModule from 'wavefile'; const { WaveFile } += wavefileModule`, as used unmodified in `AudioSyncer.js`) throws under Jest's +CJS/ESM interop when the module isn't mocked — `AudioSyncer.test.js` never hit +this because it always mocks `wavefile`. The named-import form (`import { +WaveFile } from 'wavefile'`) fixes Jest but then breaks under plain Node ESM +(`tsx`), which can't statically resolve a named export from this package's CJS +bundle. The form that works under both: namespace import then destructure — +`import * as wavefileNS from 'wavefile'; const { WaveFile } = wavefileNS;` — +used in `extractAudioWindow.js`. (`AudioSyncer.js` itself is untouched; its +tests mock `wavefile` so it never exercises this path.) + +**What was done:** For each non-zero-duration row from Commit 3's report: +compute its start time within the rendered output (cumulative sum of prior +rows' `durationSeconds`, since hook sections play back-to-back from frame 0); +extract that window from `--rendered` and the row's own `[sourceStart, +sourceEnd)` from `--source`; cross-correlate via `computeCrossCorrelation`/ +`findBestLag`/`validatePeak` from `scripts/lib/audioCorrelation.js` (Commit 2); +report lag in ms, flag rows exceeding `--tolerance-ms`. Temp WAVs live in an +`fs.mkdtemp`-created directory, removed in a `finally` block. + +**Manual test (not yet run — optional, needs a real render):** render a short +real hook-intro via `npm run render:hook-intro -- --overwrite`, then run +`npm run diagnose:hooks -- --rendered public/renders/hook-intro.mp4` against +the output and `public/sync/output/synced-output-1.mp4`; confirm lag values are +small (well under the 50ms tolerance) now that Commit 1's fix is in place. The +integration test above already validates the extraction+correlation mechanism +end to end against real ffmpeg/FFT — this manual step is only to validate it +against an actual Remotion-rendered artifact, which wasn't run here because it +requires a multi-minute render most agents shouldn't trigger unprompted. --- diff --git a/scripts/diagnostics/verify-hook-timing.ts b/scripts/diagnostics/verify-hook-timing.ts index df649f6..989d465 100644 --- a/scripts/diagnostics/verify-hook-timing.ts +++ b/scripts/diagnostics/verify-hook-timing.ts @@ -16,15 +16,30 @@ */ import fs from 'fs-extra'; +import os from 'os'; import path from 'path'; import { getHookSubClips } from '../../remotion/lib/hookTiming'; import type { Segment } from '../../remotion/types/transcript'; +import { extractAudioWindow, loadWavSamples } from '../lib/extractAudioWindow.js'; +import { computeCrossCorrelation, findBestLag, validatePeak } from '../lib/audioCorrelation.js'; + +// Correlation-quality thresholds — match AudioSyncer.js's defaults. Kept local +// rather than re-exported from audioCorrelation.js since they're tuning knobs +// for the caller, not part of the correlation math itself. +const PEAK_NEARNESS_THRESHOLD = 0.5; +const RELIABILITY_SNR_THRESHOLD = 3.0; +const DEFAULT_SAMPLE_RATE = 8000; +const DEFAULT_TOLERANCE_MS = 50; type CliArgs = { transcriptPath: string; fps?: number; json: boolean; help: boolean; + renderedPath?: string; + sourcePath?: string; + sampleRate: number; + toleranceMs: number; }; function parseArgs(argv: string[]): CliArgs { @@ -33,6 +48,10 @@ function parseArgs(argv: string[]): CliArgs { fps: undefined, json: false, help: false, + renderedPath: undefined, + sourcePath: undefined, + sampleRate: DEFAULT_SAMPLE_RATE, + toleranceMs: DEFAULT_TOLERANCE_MS, }; for (let i = 0; i < argv.length; i++) { const a = argv[i]; @@ -40,6 +59,10 @@ function parseArgs(argv: string[]): CliArgs { else if (a === '--fps' && argv[i + 1]) out.fps = Number(argv[++i]); else if (a === '--json') out.json = true; else if (a === '--help' || a === '-h') out.help = true; + else if (a === '--rendered' && argv[i + 1]) out.renderedPath = argv[++i]; + else if (a === '--source' && argv[i + 1]) out.sourcePath = argv[++i]; + else if (a === '--sample-rate' && argv[i + 1]) out.sampleRate = Number(argv[++i]); + else if (a === '--tolerance-ms' && argv[i + 1]) out.toleranceMs = Number(argv[++i]); } return out; } @@ -57,10 +80,19 @@ Usage: npx tsx scripts/diagnostics/verify-hook-timing.ts [options] Options: - --transcript transcript.json path (default: public/edit/transcript.json) + --transcript transcript.json path (default: public/edit/transcript.json) --fps Frames per second (default: transcript meta.fps, else 60) --json Output machine-readable JSON instead of a console table --help, -h Show this help + +Audio cross-correlation layer (opt-in; requires a rendered hook-intro file): + --rendered Path to the rendered hook clip (e.g. public/renders/hook-intro.mp4). + Its hook sections are assumed to play back-to-back from frame 0, + matching npm run render:hook-intro's output. + --source Path to the original synced source video the hooks were cut from. + Defaults to transcript.meta.videoSrc (resolved relative to public/). + --sample-rate Audio sample rate (Hz) for correlation (default: ${DEFAULT_SAMPLE_RATE}) + --tolerance-ms Flag hooks whose measured drift exceeds this (default: ${DEFAULT_TOLERANCE_MS}) `); } @@ -162,11 +194,96 @@ function printReport(report: HookLayoutReport) { ); } +export type CorrelationResult = { + index: number; + renderedStartSeconds: number; + lagMs: number; + snr: number; + isReliable: boolean; + exceedsTolerance: boolean; +}; + +/** + * For each non-zero-duration hook row, extracts the matching audio window from + * the rendered output (assumed to play hook sections back-to-back from frame 0, + * per render-hook-intro.js's output) and from the original source video at the + * segment's own [sourceStart, sourceEnd), then cross-correlates them via the + * shared FFT utilities (scripts/lib/audioCorrelation.js) to measure drift in ms. + * + * This catches classes of bugs the math-consistency layer can't see on its own — + * Remotion encode-time frame rounding, dropped frames — because it never trusts + * the pipeline's own numbers, only the two actual audio signals. + */ +export async function runAudioCorrelationLayer( + report: HookLayoutReport, + renderedPath: string, + sourcePath: string, + sampleRate: number, + toleranceMs: number, +): Promise { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'verify-hook-timing-')); + try { + const results: CorrelationResult[] = []; + let cursorSeconds = 0; + + for (const row of report.rows) { + if (row.zeroDuration) continue; + const renderedStartSeconds = cursorSeconds; + cursorSeconds += row.durationSeconds; + + const renderedWav = path.join(tempDir, `rendered-${row.index}.wav`); + const sourceWav = path.join(tempDir, `source-${row.index}.wav`); + await extractAudioWindow(renderedPath, renderedStartSeconds, row.durationSeconds, sampleRate, renderedWav); + await extractAudioWindow(sourcePath, row.sourceStart, row.sourceEnd - row.sourceStart, sampleRate, sourceWav); + + const renderedSamples = loadWavSamples(renderedWav); + const sourceSamples = loadWavSamples(sourceWav); + const correlation = computeCrossCorrelation(renderedSamples, sourceSamples); + const lagSeconds = findBestLag(correlation, sampleRate, report.fps, PEAK_NEARNESS_THRESHOLD); + const { snr, isReliable } = validatePeak(correlation, lagSeconds, sampleRate, report.fps, RELIABILITY_SNR_THRESHOLD); + const lagMs = lagSeconds * 1000; + + results.push({ + index: row.index, + renderedStartSeconds, + lagMs, + snr, + isReliable, + exceedsTolerance: Math.abs(lagMs) > toleranceMs, + }); + } + + return results; + } finally { + await fs.remove(tempDir).catch(() => {}); + } +} + +function printCorrelationResults(results: CorrelationResult[], toleranceMs: number) { + console.log(`[verify-hook-timing] Audio cross-correlation (tolerance ±${toleranceMs}ms):\n`); + for (const r of results) { + const flag = r.exceedsTolerance ? '✗ DRIFT' : '✓'; + const reliability = r.isReliable ? '' : ' (low-confidence peak — SNR below threshold)'; + console.log(`#${String(r.index).padEnd(4)} ${flag.padEnd(8)} lag ${r.lagMs.toFixed(1)}ms snr ${r.snr.toFixed(2)}${reliability}`); + } + const failing = results.filter(r => r.exceedsTolerance); + console.log( + failing.length > 0 + ? `\n[verify-hook-timing] ${failing.length}/${results.length} hook(s) exceed tolerance.\n` + : `\n[verify-hook-timing] All ${results.length} hook(s) within tolerance.\n`, + ); +} + +function resolvePublicRelative(cwd: string, relPath: string) { + return path.resolve(cwd, 'public', relPath); +} + async function main() { const cli = parseArgs(process.argv.slice(2)); if (cli.help) { printHelp(); return; } - const transcriptPath = path.resolve(process.cwd(), cli.transcriptPath); + const cwd = process.cwd(); + const transcriptPath = path.resolve(cwd, cli.transcriptPath); if (!await fs.pathExists(transcriptPath)) { throw new Error(`Transcript not found: ${transcriptPath}`); } @@ -189,6 +306,24 @@ async function main() { } else { printReport(report); } + + if (cli.renderedPath) { + const sourcePath = cli.sourcePath + ? path.resolve(cwd, cli.sourcePath) + : transcript.meta?.videoSrc + ? resolvePublicRelative(cwd, transcript.meta.videoSrc) + : undefined; + if (!sourcePath) { + throw new Error('--rendered given but no --source and transcript.meta.videoSrc is not set.'); + } + if (!await fs.pathExists(sourcePath)) { + throw new Error(`Source video not found: ${sourcePath}`); + } + + const results = await runAudioCorrelationLayer(report, cli.renderedPath, sourcePath, cli.sampleRate, cli.toleranceMs); + printCorrelationResults(results, cli.toleranceMs); + if (results.some(r => r.exceedsTolerance)) process.exitCode = 1; + } } main().catch(err => { diff --git a/scripts/lib/extractAudioWindow.js b/scripts/lib/extractAudioWindow.js new file mode 100644 index 0000000..15dcbf9 --- /dev/null +++ b/scripts/lib/extractAudioWindow.js @@ -0,0 +1,45 @@ +import { spawn } from 'child_process'; +import fs from 'fs-extra'; +// Namespace import (not default-then-destructure, not a named import): the +// former breaks under Jest's CJS/ESM interop for this package, the latter +// breaks under plain Node ESM (tsx) — this form works under both. +import * as wavefileNS from 'wavefile'; +const { WaveFile } = wavefileNS; + +/** + * Extracts [startSeconds, startSeconds + durationSeconds) of audio from a video + * or audio file into a mono PCM WAV at sampleRate. + * + * Uses accurate (output) seeking — -ss/-t placed after -i — rather than the + * fast input-seek pattern used elsewhere in the pipeline (e.g. AudioSyncer.js's + * multi-minute sync windows), because this is a correctness tool verifying + * frame-accurate boundaries on short (a few second) clips, where seek accuracy + * matters more than speed. + */ +export function extractAudioWindow(sourcePath, startSeconds, durationSeconds, sampleRate, outWavPath) { + return new Promise((resolve, reject) => { + const proc = spawn('ffmpeg', [ + '-i', sourcePath, + '-ss', String(startSeconds), + '-t', String(durationSeconds), + '-vn', '-ac', '1', '-ar', String(sampleRate), + '-acodec', 'pcm_s16le', + outWavPath, '-y', + ], { stdio: ['ignore', 'pipe', 'pipe'] }); + let stderr = ''; + proc.stderr.on('data', (d) => { stderr += d.toString(); }); + proc.on('close', (code) => { + if (code !== 0) reject(new Error(`ffmpeg exited with code ${code}\n${stderr}`)); + else resolve(outWavPath); + }); + proc.on('error', (err) => reject(new Error(`Failed to spawn ffmpeg: ${err.message}`))); + }); +} + +/** Matches AudioSyncer.js's loadWavSamples: 32-bit float samples for FFT correlation. */ +export function loadWavSamples(wavPath) { + const buf = fs.readFileSync(wavPath); + const wav = new WaveFile(buf); + wav.toBitDepth('32f'); + return wav.getSamples(false, Float32Array); +} diff --git a/tests/integration/verify-hook-timing-audio.test.ts b/tests/integration/verify-hook-timing-audio.test.ts new file mode 100644 index 0000000..e028787 --- /dev/null +++ b/tests/integration/verify-hook-timing-audio.test.ts @@ -0,0 +1,81 @@ +import fs from 'fs'; +import path from 'path'; +import os from 'os'; +import { spawnSync } from 'child_process'; +import { extractAudioWindow, loadWavSamples } from '../../scripts/lib/extractAudioWindow.js'; +import { computeCrossCorrelation, findBestLag } from '../../scripts/lib/audioCorrelation.js'; + +/** + * Exercises the real ffmpeg extraction + FFT correlation path used by + * scripts/diagnostics/verify-hook-timing.ts's audio cross-correlation layer + * (no mocks) against a deterministic broadband-noise fixture, confirming it + * both reports ~0 drift for a matching window and correctly measures a + * deliberately introduced offset. + */ + +jest.setTimeout(30000); + +const SAMPLE_RATE = 8000; +const FRAME_RATE = 60; + +function ffmpeg(args: string[]) { + const result = spawnSync('ffmpeg', args, { stdio: ['ignore', 'pipe', 'pipe'] }); + if (result.status !== 0) { + throw new Error(`ffmpeg failed: ${result.stderr?.toString()}`); + } +} + +describe('hook-timing audio cross-correlation (real ffmpeg + real FFT)', () => { + let tempDir: string; + let sourceWav: string; + + beforeAll(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'verify-hook-timing-audio-')); + sourceWav = path.join(tempDir, 'source.wav'); + + // Deterministic broadband noise fixture (seeded) — noise, not a pure tone, + // gives cross-correlation a single unambiguous peak the way real speech does. + ffmpeg([ + '-f', 'lavfi', '-i', `anoisesrc=d=10:c=white:r=${SAMPLE_RATE}:seed=42`, + '-ac', '1', '-acodec', 'pcm_s16le', sourceWav, '-y', + ]); + }); + + afterAll(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + test('reports ~0ms lag when the rendered window exactly matches the source window', async () => { + const groundTruthWav = path.join(tempDir, 'ground-truth-exact.wav'); + ffmpeg([ + '-i', sourceWav, '-ss', '3.0', '-t', '2.0', + '-ac', '1', '-acodec', 'pcm_s16le', groundTruthWav, '-y', + ]); + + const extractedWav = path.join(tempDir, 'extracted-exact.wav'); + await extractAudioWindow(sourceWav, 3.0, 2.0, SAMPLE_RATE, extractedWav); + + const correlation = computeCrossCorrelation(loadWavSamples(groundTruthWav), loadWavSamples(extractedWav)); + const lagMs = findBestLag(correlation, SAMPLE_RATE, FRAME_RATE, 0.5) * 1000; + + expect(Math.abs(lagMs)).toBeLessThanOrEqual(1000 / FRAME_RATE); + }); + + test('measures a deliberately introduced 50ms offset', async () => { + const offsetWav = path.join(tempDir, 'ground-truth-offset.wav'); + ffmpeg([ + '-i', sourceWav, '-ss', '3.05', '-t', '2.0', + '-ac', '1', '-acodec', 'pcm_s16le', offsetWav, '-y', + ]); + + const extractedWav = path.join(tempDir, 'extracted-for-offset.wav'); + await extractAudioWindow(sourceWav, 3.0, 2.0, SAMPLE_RATE, extractedWav); + + const correlation = computeCrossCorrelation(loadWavSamples(offsetWav), loadWavSamples(extractedWav)); + const lagMs = findBestLag(correlation, SAMPLE_RATE, FRAME_RATE, 0.5) * 1000; + + // Sign follows FFT correlation convention (which operand leads which); this + // diagnostic only needs the magnitude of drift, not the sign. + expect(Math.abs(Math.abs(lagMs) - 50)).toBeLessThanOrEqual(1000 / FRAME_RATE); + }); +}); From 1e3978aa9102f4ad0a0bfdd91523f0bde5fa46b2 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 12:46:49 +0800 Subject: [PATCH 05/16] feat: add optional whisper content-diff layer to hook-timing diagnostic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds --verify-content to verify-hook-timing.ts: transcribes each hook's already-extracted rendered audio via whisper.cpp (Transcriber.js, base.en model — small/fast, this only needs a rough word list) and diffs it against the segment's expected spoken words (bounded by its own hookFrom/hookTo, not the tail-padded sourceEnd). Flags missing/extra words, catching wrong-phrase bugs — e.g. resolvePhraseToTimeRange silently falling back to the whole segment — that pure timing checks can't see. Off by default (slow, may trigger a model download). Co-Authored-By: Claude Sonnet 5 --- .../HOOK_TIMING_DIAGNOSTICS.md | 55 ++++--- .../diagnostics/verify-hook-timing.test.ts | 69 ++++++++- scripts/diagnostics/verify-hook-timing.ts | 134 +++++++++++++++++- 3 files changed, 231 insertions(+), 27 deletions(-) diff --git a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md index baa0c43..5298d68 100644 --- a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md +++ b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md @@ -257,33 +257,42 @@ requires a multi-minute render most agents shouldn't trigger unprompted. --- -### Commit 5 — `feat: add optional whisper content-diff layer to hook-timing diagnostic` +### Commit 5 — `feat: add optional whisper content-diff layer to hook-timing diagnostic` ✅ DONE **Status check:** running with `--verify-content` prints a per-hook word-diff -summary (expected tokens vs. transcribed words, showing missing/extra words). -Running without the flag never invokes whisper (confirm via a quick timing check — -this layer is off by default because it's slow). +summary (expected words vs. transcribed words, showing missing/extra words). +Running without the flag never invokes whisper. **Files modified:** -- `scripts/diagnostics/verify-hook-timing.ts` - -**What to do:** -Behind `--verify-content`, for each hook's extracted rendered-audio WAV (already -produced in Commit 4 — reuse it, don't re-extract), run -`scripts/transcribe/Transcriber.js` (`new Transcriber({ audioPath, outputDir, ... })`, -`init()`, `transcribe()`, `close()` — see `scripts/transcribe/transcribe-audio.js` -for the exact call pattern) to get a word list. Compare (simple set/order diff, -not fuzzy matching) against the expected spoken tokens in -`[segment.hookFrom ?? segment.start, segment.hookTo ?? segment.end]` from the -transcript (`isSpokenToken` filter from `remotion/lib/tokens.ts`). Report any -expected word missing from the transcribed output, or any transcribed word not in -the expected set — this is the signal for the `resolvePhraseToTimeRange` fallback -case (phrase not found → falls back to whole-segment hook) and other -content-level mismatches that pure timing math can't detect. - -**Manual test:** intentionally mismatch a hook's `hookFrom`/`hookTo` in a copy of -the transcript (e.g. shift by 2 seconds) and confirm `--verify-content` flags the -missing/extra words that the timing-only layers would not catch on their own. +- `scripts/diagnostics/verify-hook-timing.ts` — `expectedWordsForHook()`, + `diffWordLists()` (both pure, unit tested), `runContentDiffForHook()`, + `--verify-content`/`--content-model` flags (wired into `runAudioCorrelationLayer` + as an optional 6th param so it reuses the already-extracted rendered WAV from + Commit 4 rather than re-extracting). +- `scripts/diagnostics/verify-hook-timing.test.ts` — unit tests for + `expectedWordsForHook` (bounds by `hookFrom`/`hookTo`, falls back to + `start`/`end`, filters non-spoken tokens) and `diffWordLists` (identical lists, + missing, extra, count-aware multiset behavior, order-insensitivity). + +**What was done:** Behind `--verify-content`, for each hook's already-extracted +rendered-audio WAV, runs `Transcriber` (`scripts/transcribe/Transcriber.js`, +default model overridden to `base.en` — small/fast, since this only needs a +rough word list on a few-second clip, not production transcription quality) and +reads its `transcript.raw.json` output. Diffs the transcribed words (multiset, +order-insensitive, no fuzzy matching) against `expectedWordsForHook()` — spoken +tokens bounded by the segment's own `hookFrom`/`hookTo` (not the tail-padded/ +bridged `sourceEnd`, since that padding exists to avoid clipping audio, not +because more words are expected there). This is the signal for the +`resolvePhraseToTimeRange` fallback case (phrase not found → falls back to +whole-segment hook) and other content-level mismatches pure timing math can't +detect. + +**Manual test (not yet run — needs `--rendered`, triggers a one-time `base.en` +model download on first use):** once a real hook-intro render exists, run +`npm run diagnose:hooks -- --rendered --source --verify-content`; +intentionally mismatch a hook's `hookFrom`/`hookTo` in a copy of the transcript +and confirm it flags the resulting missing/extra words that the timing-only +layers alone would not catch. --- diff --git a/scripts/diagnostics/verify-hook-timing.test.ts b/scripts/diagnostics/verify-hook-timing.test.ts index 3dc6eea..f22ac21 100644 --- a/scripts/diagnostics/verify-hook-timing.test.ts +++ b/scripts/diagnostics/verify-hook-timing.test.ts @@ -3,12 +3,16 @@ * Pure logic, no I/O — mirrors the style of remotion/lib/hookTiming.test.ts. */ -import { computeExpectedHookLayout } from './verify-hook-timing'; +import { computeExpectedHookLayout, expectedWordsForHook, diffWordLists } from './verify-hook-timing'; import { buildHookSections, HOOK_TAIL_PAD_UNBOUNDED_SECONDS } from '../../remotion/lib/hookTiming'; import type { Segment } from '../../remotion/types/transcript'; const FPS = 60; +function makeToken(text: string, t_dtw: number, t_end?: number) { + return { text, t_dtw, t_end, cut: false }; +} + function makeSegment(overrides: Partial = {}): Segment { return { id: 1, @@ -83,3 +87,66 @@ describe('computeExpectedHookLayout', () => { expect(report.totalFrames).toBe(0); }); }); + +describe('expectedWordsForHook', () => { + it('includes only tokens within [hookFrom, hookTo]', () => { + const seg = makeSegment({ + start: 10, end: 15, hookFrom: 11, hookTo: 13, + tokens: [ + makeToken('before', 10.5), + makeToken('Hello,', 11.2), + makeToken('world!', 12.0), + makeToken('after', 14.0), + ], + }); + expect(expectedWordsForHook(seg)).toEqual(['hello', 'world']); + }); + + it('falls back to segment start/end when hookFrom/hookTo are absent', () => { + const seg = makeSegment({ + start: 10, end: 12, + tokens: [makeToken('yep', 10.5)], + }); + expect(expectedWordsForHook(seg)).toEqual(['yep']); + }); + + it('excludes non-spoken (punctuation/marker) tokens', () => { + const seg = makeSegment({ + start: 10, end: 12, + tokens: [makeToken('word', 10.1), makeToken('_MUSIC_', 10.2), makeToken('.', 10.3)], + }); + expect(expectedWordsForHook(seg)).toEqual(['word']); + }); +}); + +describe('diffWordLists', () => { + it('reports no mismatch for identical word lists', () => { + const { missing, extra } = diffWordLists(['a', 'b', 'c'], ['a', 'b', 'c']); + expect(missing).toEqual([]); + expect(extra).toEqual([]); + }); + + it('reports missing words expected but not transcribed', () => { + const { missing, extra } = diffWordLists(['a', 'b', 'c'], ['a', 'c']); + expect(missing).toEqual(['b']); + expect(extra).toEqual([]); + }); + + it('reports extra words transcribed but not expected', () => { + const { missing, extra } = diffWordLists(['a'], ['a', 'b']); + expect(missing).toEqual([]); + expect(extra).toEqual(['b']); + }); + + it('is count-aware for repeated words (multiset, not set)', () => { + const { missing, extra } = diffWordLists(['a', 'a'], ['a']); + expect(missing).toEqual(['a']); + expect(extra).toEqual([]); + }); + + it('is order-insensitive', () => { + const { missing, extra } = diffWordLists(['a', 'b'], ['b', 'a']); + expect(missing).toEqual([]); + expect(extra).toEqual([]); + }); +}); diff --git a/scripts/diagnostics/verify-hook-timing.ts b/scripts/diagnostics/verify-hook-timing.ts index 989d465..f8f2677 100644 --- a/scripts/diagnostics/verify-hook-timing.ts +++ b/scripts/diagnostics/verify-hook-timing.ts @@ -19,15 +19,21 @@ import fs from 'fs-extra'; import os from 'os'; import path from 'path'; import { getHookSubClips } from '../../remotion/lib/hookTiming'; +import { isSpokenToken } from '../../remotion/lib/tokens'; import type { Segment } from '../../remotion/types/transcript'; import { extractAudioWindow, loadWavSamples } from '../lib/extractAudioWindow.js'; import { computeCrossCorrelation, findBestLag, validatePeak } from '../lib/audioCorrelation.js'; +import Transcriber from '../transcribe/Transcriber.js'; // Correlation-quality thresholds — match AudioSyncer.js's defaults. Kept local // rather than re-exported from audioCorrelation.js since they're tuning knobs // for the caller, not part of the correlation math itself. const PEAK_NEARNESS_THRESHOLD = 0.5; const RELIABILITY_SNR_THRESHOLD = 3.0; +// A small/fast model is plenty for a rough word list on a few-second clip — +// this is a diagnostic, not production transcription. Transcriber.js's own +// default (medium.en) is unnecessarily large/slow for this use. +const DEFAULT_CONTENT_MODEL = 'base.en'; const DEFAULT_SAMPLE_RATE = 8000; const DEFAULT_TOLERANCE_MS = 50; @@ -40,6 +46,8 @@ type CliArgs = { sourcePath?: string; sampleRate: number; toleranceMs: number; + verifyContent: boolean; + contentModel: string; }; function parseArgs(argv: string[]): CliArgs { @@ -52,6 +60,8 @@ function parseArgs(argv: string[]): CliArgs { sourcePath: undefined, sampleRate: DEFAULT_SAMPLE_RATE, toleranceMs: DEFAULT_TOLERANCE_MS, + verifyContent: false, + contentModel: DEFAULT_CONTENT_MODEL, }; for (let i = 0; i < argv.length; i++) { const a = argv[i]; @@ -63,6 +73,8 @@ function parseArgs(argv: string[]): CliArgs { else if (a === '--source' && argv[i + 1]) out.sourcePath = argv[++i]; else if (a === '--sample-rate' && argv[i + 1]) out.sampleRate = Number(argv[++i]); else if (a === '--tolerance-ms' && argv[i + 1]) out.toleranceMs = Number(argv[++i]); + else if (a === '--verify-content') out.verifyContent = true; + else if (a === '--content-model' && argv[i + 1]) out.contentModel = argv[++i]; } return out; } @@ -93,6 +105,12 @@ Audio cross-correlation layer (opt-in; requires a rendered hook-intro file): Defaults to transcript.meta.videoSrc (resolved relative to public/). --sample-rate Audio sample rate (Hz) for correlation (default: ${DEFAULT_SAMPLE_RATE}) --tolerance-ms Flag hooks whose measured drift exceeds this (default: ${DEFAULT_TOLERANCE_MS}) + +Content-diff layer (opt-in; requires --rendered; off by default, slow): + --verify-content Transcribe each rendered hook (whisper.cpp) and diff its words + against the expected phrase — catches wrong-phrase bugs pure + timing checks can't see. May trigger a one-time model download. + --content-model Whisper model to use (default: ${DEFAULT_CONTENT_MODEL}) `); } @@ -111,6 +129,7 @@ export type HookLayoutRow = { export type HookLayoutReport = { rows: HookLayoutRow[]; + hookSegments: Segment[]; totalFrames: number; totalSeconds: number; fps: number; @@ -170,7 +189,7 @@ export function computeExpectedHookLayout(segments: Segment[], fps: number): Hoo }); } - return { rows, totalFrames, totalSeconds: totalFrames / fps, fps }; + return { rows, hookSegments, totalFrames, totalSeconds: totalFrames / fps, fps }; } function printReport(report: HookLayoutReport) { @@ -194,6 +213,13 @@ function printReport(report: HookLayoutReport) { ); } +export type ContentDiffResult = { + expectedWords: string[]; + transcribedWords: string[]; + missingWords: string[]; + extraWords: string[]; +}; + export type CorrelationResult = { index: number; renderedStartSeconds: number; @@ -201,8 +227,84 @@ export type CorrelationResult = { snr: number; isReliable: boolean; exceedsTolerance: boolean; + contentDiff?: ContentDiffResult; }; +function normalizeWord(word: string): string { + return word.toLowerCase().replace(/[^a-z0-9']/g, ''); +} + +/** + * Expected spoken words for a hook, bounded by the doc author's own + * hookFrom/hookTo (not the tail-padded/bridged sourceEnd — that padding exists + * to avoid clipping audio, not because more words are expected there). + */ +export function expectedWordsForHook(segment: Segment): string[] { + const from = segment.hookFrom ?? segment.start; + const to = segment.hookTo ?? segment.end; + return segment.tokens + .filter(t => isSpokenToken(t) && t.t_dtw >= from && t.t_dtw <= to) + .map(t => normalizeWord(t.text)) + .filter(Boolean); +} + +function flattenTranscribedWords(rawTranscript: { segments?: { tokens?: { text: string }[] }[] }): string[] { + const words: string[] = []; + for (const seg of rawTranscript.segments ?? []) { + for (const t of seg.tokens ?? []) { + if (isSpokenToken(t as Parameters[0])) words.push(normalizeWord(t.text)); + } + } + return words.filter(Boolean); +} + +/** + * Multiset diff (order-insensitive, count-aware, no fuzzy matching) between + * the expected and transcribed word lists for one hook. + */ +export function diffWordLists(expectedWords: string[], transcribedWords: string[]): { missing: string[]; extra: string[] } { + const remaining = new Map(); + for (const w of transcribedWords) remaining.set(w, (remaining.get(w) ?? 0) + 1); + + const missing: string[] = []; + for (const w of expectedWords) { + const count = remaining.get(w) ?? 0; + if (count > 0) remaining.set(w, count - 1); + else missing.push(w); + } + + const extra: string[] = []; + for (const [w, count] of remaining) { + for (let i = 0; i < count; i++) extra.push(w); + } + + return { missing, extra }; +} + +/** Transcribes a short hook audio clip (whisper.cpp) and diffs it against the expected words. */ +async function runContentDiffForHook( + segment: Segment, + wavPath: string, + workDir: string, + model: string, +): Promise { + const expectedWords = expectedWordsForHook(segment); + + const transcriber = new Transcriber({ audioPath: wavPath, outputDir: workDir, model }); + let transcribedWords: string[] = []; + try { + await transcriber.init(); + await transcriber.transcribe(); + const rawTranscript = await fs.readJson(path.join(workDir, 'transcript.raw.json')); + transcribedWords = flattenTranscribedWords(rawTranscript); + } finally { + await transcriber.close(); + } + + const { missing, extra } = diffWordLists(expectedWords, transcribedWords); + return { expectedWords, transcribedWords, missingWords: missing, extraWords: extra }; +} + /** * For each non-zero-duration hook row, extracts the matching audio window from * the rendered output (assumed to play hook sections back-to-back from frame 0, @@ -220,6 +322,7 @@ export async function runAudioCorrelationLayer( sourcePath: string, sampleRate: number, toleranceMs: number, + verifyContent?: { model: string }, ): Promise { const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'verify-hook-timing-')); try { @@ -243,6 +346,14 @@ export async function runAudioCorrelationLayer( const { snr, isReliable } = validatePeak(correlation, lagSeconds, sampleRate, report.fps, RELIABILITY_SNR_THRESHOLD); const lagMs = lagSeconds * 1000; + let contentDiff: ContentDiffResult | undefined; + if (verifyContent) { + const workDir = path.join(tempDir, `content-${row.index}`); + await fs.ensureDir(workDir); + // Reuses the already-extracted renderedWav rather than re-extracting. + contentDiff = await runContentDiffForHook(report.hookSegments[row.index], renderedWav, workDir, verifyContent.model); + } + results.push({ index: row.index, renderedStartSeconds, @@ -250,6 +361,7 @@ export async function runAudioCorrelationLayer( snr, isReliable, exceedsTolerance: Math.abs(lagMs) > toleranceMs, + contentDiff, }); } @@ -265,13 +377,27 @@ function printCorrelationResults(results: CorrelationResult[], toleranceMs: numb const flag = r.exceedsTolerance ? '✗ DRIFT' : '✓'; const reliability = r.isReliable ? '' : ' (low-confidence peak — SNR below threshold)'; console.log(`#${String(r.index).padEnd(4)} ${flag.padEnd(8)} lag ${r.lagMs.toFixed(1)}ms snr ${r.snr.toFixed(2)}${reliability}`); + if (r.contentDiff && (r.contentDiff.missingWords.length > 0 || r.contentDiff.extraWords.length > 0)) { + if (r.contentDiff.missingWords.length > 0) console.log(` missing: ${r.contentDiff.missingWords.join(', ')}`); + if (r.contentDiff.extraWords.length > 0) console.log(` extra: ${r.contentDiff.extraWords.join(', ')}`); + } } const failing = results.filter(r => r.exceedsTolerance); + const contentMismatches = results.filter( + r => r.contentDiff && (r.contentDiff.missingWords.length > 0 || r.contentDiff.extraWords.length > 0), + ); console.log( failing.length > 0 ? `\n[verify-hook-timing] ${failing.length}/${results.length} hook(s) exceed tolerance.\n` : `\n[verify-hook-timing] All ${results.length} hook(s) within tolerance.\n`, ); + if (results.some(r => r.contentDiff)) { + console.log( + contentMismatches.length > 0 + ? `[verify-hook-timing] ${contentMismatches.length}/${results.length} hook(s) have word mismatches.\n` + : `[verify-hook-timing] All hook(s) content-verified — no word mismatches.\n`, + ); + } } function resolvePublicRelative(cwd: string, relPath: string) { @@ -320,9 +446,11 @@ async function main() { throw new Error(`Source video not found: ${sourcePath}`); } - const results = await runAudioCorrelationLayer(report, cli.renderedPath, sourcePath, cli.sampleRate, cli.toleranceMs); + const verifyContent = cli.verifyContent ? { model: cli.contentModel } : undefined; + const results = await runAudioCorrelationLayer(report, cli.renderedPath, sourcePath, cli.sampleRate, cli.toleranceMs, verifyContent); printCorrelationResults(results, cli.toleranceMs); - if (results.some(r => r.exceedsTolerance)) process.exitCode = 1; + const hasContentMismatch = results.some(r => r.contentDiff && (r.contentDiff.missingWords.length > 0 || r.contentDiff.extraWords.length > 0)); + if (results.some(r => r.exceedsTolerance) || hasContentMismatch) process.exitCode = 1; } } From 94a8b32a72f5b8a950f8ed24b608d32afde92eff Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 12:58:17 +0800 Subject: [PATCH 06/16] fix: two bugs found running the diagnostic against a real render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. verify-hook-timing.ts compared rendered audio against the *unrounded* [sourceStart, sourceEnd) window instead of the frame-rounded [trimBefore, trimAfter) window Remotion actually renders — a systematic up-to-1-frame bias baked into the check itself. Running the full pipeline against a real render (npm run render:hook-intro) surfaced this: every one of 35 hooks showed an identical ~50-66ms lag before the fix, and a perfectly uniform 50.0ms after — the uniformity across unrelated hooks was the tell that this was a comparison bug, not per-hook drift. 2. extractAudioWindow.js's wavefile import broke depending on how it's loaded: a namespace-then-destructure worked under Jest but under tsx executing a real .ts file (the actual production path, as opposed to `tsx -e`) the CJS/ESM interop wraps the module as { default: { WaveFile } } instead of exposing it directly. Neither shape alone covers both runners; now defensively checks both. Co-Authored-By: Claude Sonnet 5 --- scripts/diagnostics/verify-hook-timing.ts | 10 +++++++++- scripts/lib/extractAudioWindow.js | 10 ++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/scripts/diagnostics/verify-hook-timing.ts b/scripts/diagnostics/verify-hook-timing.ts index f8f2677..4c5351a 100644 --- a/scripts/diagnostics/verify-hook-timing.ts +++ b/scripts/diagnostics/verify-hook-timing.ts @@ -334,10 +334,18 @@ export async function runAudioCorrelationLayer( const renderedStartSeconds = cursorSeconds; cursorSeconds += row.durationSeconds; + // Compare against the frame-rounded [trimBefore, trimAfter) window, not the + // raw unrounded [sourceStart, sourceEnd) — Remotion trims to frame + // boundaries, so that's what's actually present in the rendered output. + // Comparing against the unrounded window instead introduces a systematic + // up-to-1-frame bias into this very check. + const sourceWindowStart = row.trimBefore! / report.fps; + const sourceWindowDuration = (row.trimAfter! - row.trimBefore!) / report.fps; + const renderedWav = path.join(tempDir, `rendered-${row.index}.wav`); const sourceWav = path.join(tempDir, `source-${row.index}.wav`); await extractAudioWindow(renderedPath, renderedStartSeconds, row.durationSeconds, sampleRate, renderedWav); - await extractAudioWindow(sourcePath, row.sourceStart, row.sourceEnd - row.sourceStart, sampleRate, sourceWav); + await extractAudioWindow(sourcePath, sourceWindowStart, sourceWindowDuration, sampleRate, sourceWav); const renderedSamples = loadWavSamples(renderedWav); const sourceSamples = loadWavSamples(sourceWav); diff --git a/scripts/lib/extractAudioWindow.js b/scripts/lib/extractAudioWindow.js index 15dcbf9..b16152a 100644 --- a/scripts/lib/extractAudioWindow.js +++ b/scripts/lib/extractAudioWindow.js @@ -1,10 +1,12 @@ import { spawn } from 'child_process'; import fs from 'fs-extra'; -// Namespace import (not default-then-destructure, not a named import): the -// former breaks under Jest's CJS/ESM interop for this package, the latter -// breaks under plain Node ESM (tsx) — this form works under both. +// wavefile's CJS/ESM interop shape differs by runner: Jest/babel exposes +// { WaveFile } directly on the namespace; tsx executing a real .ts file wraps +// it as { default: { WaveFile } } instead (differs even from `tsx -e`, which +// gives the unwrapped shape). Neither a default import nor a named import +// works across all of these — only this defensive unwrap does. import * as wavefileNS from 'wavefile'; -const { WaveFile } = wavefileNS; +const WaveFile = wavefileNS.WaveFile ?? wavefileNS.default?.WaveFile; /** * Extracts [startSeconds, startSeconds + durationSeconds) of audio from a video From 4a5640953c134e4a52ced27e04be30e2652b4e04 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 13:22:43 +0800 Subject: [PATCH 07/16] feat: add zero-token-overlap check to hook-timing diagnostic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flags any hook whose hookFrom/hookTo doesn't overlap any of its own segment's spoken token timestamps. Found by manually investigating a user report that a hook's audio didn't match its caption — the segment's Whisper/WhisperX word alignment was compressed/wrong (all four words crammed into a 0.4s span that doesn't match natural speech pacing), so the doc's explicit HOOK timestamp — set correctly by ear — didn't overlap the (bad) token positions at all. This check catches that structurally, for free, with no media I/O, at exactly the row the content-diff layer silently missed (expectedWords was empty for these rows, so an empty-vs-empty diff falsely "passed"). This is a "verify by ear" signal, not pass/fail: a correctly hand-fixed boundary still won't overlap the underlying bad tokens, so it will keep firing on rows that are already fine. Documented as such in the row output and --help text. Co-Authored-By: Claude Sonnet 5 --- .../diagnostics/verify-hook-timing.test.ts | 44 +++++++++++++++++++ scripts/diagnostics/verify-hook-timing.ts | 41 ++++++++++++++++- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/scripts/diagnostics/verify-hook-timing.test.ts b/scripts/diagnostics/verify-hook-timing.test.ts index f22ac21..16b9c52 100644 --- a/scripts/diagnostics/verify-hook-timing.test.ts +++ b/scripts/diagnostics/verify-hook-timing.test.ts @@ -86,6 +86,50 @@ describe('computeExpectedHookLayout', () => { expect(report.rows).toHaveLength(0); expect(report.totalFrames).toBe(0); }); + + describe('noTokenOverlap', () => { + it('is false when a spoken token falls within [hookFrom, hookTo]', () => { + const seg = makeSegment({ + hookFrom: 41.183, hookTo: 41.568, + tokens: [makeToken('loop', 41.2, 41.3), makeToken('engineering', 41.35, 41.5)], + }); + const report = computeExpectedHookLayout([seg], FPS); + expect(report.rows[0].noTokenOverlap).toBe(false); + }); + + it('is true when the segment\'s own tokens are all outside [hookFrom, hookTo] — the compressed-alignment case', () => { + // Reproduces the real bug found on the ragtech transcript: WhisperX + // alignment placed all four tokens at 40.4-40.825, but the doc's + // explicit HOOK range (41.183-41.568) — set by listening, not by + // trusting the bad tokens — doesn't overlap any of them. + const seg = makeSegment({ + start: 40.4, end: 41.068, hookFrom: 41.183, hookTo: 41.568, + tokens: [ + makeToken(' is', 40.4, 40.981), + makeToken(' called', 40.502, 41.163), + makeToken(' loop', 40.683, 41.305), + makeToken(' engineering', 40.825, 41.568), + ], + }); + const report = computeExpectedHookLayout([seg], FPS); + expect(report.rows[0].noTokenOverlap).toBe(true); + }); + + it('falls back to segment start/end when hookFrom/hookTo are absent', () => { + const seg = makeSegment({ start: 10, end: 15, tokens: [makeToken('word', 11)] }); + const report = computeExpectedHookLayout([seg], FPS); + expect(report.rows[0].noTokenOverlap).toBe(false); + }); + + it('ignores non-spoken tokens when checking overlap', () => { + const seg = makeSegment({ + hookFrom: 41.0, hookTo: 41.5, + tokens: [makeToken('_MUSIC_', 41.2), makeToken('.', 41.3)], + }); + const report = computeExpectedHookLayout([seg], FPS); + expect(report.rows[0].noTokenOverlap).toBe(true); + }); + }); }); describe('expectedWordsForHook', () => { diff --git a/scripts/diagnostics/verify-hook-timing.ts b/scripts/diagnostics/verify-hook-timing.ts index 4c5351a..5d52032 100644 --- a/scripts/diagnostics/verify-hook-timing.ts +++ b/scripts/diagnostics/verify-hook-timing.ts @@ -88,6 +88,13 @@ via the canonical remotion/lib/hookTiming.ts, and reports them per hook. Fast, no media I/O — catches drift between what the composition will actually render and what any render script assumes. +Also flags hooks whose hookFrom/hookTo don't overlap any of the segment's own +token timestamps — a strong signal (confirmed against real audio) that the +segment's Whisper/WhisperX word alignment is compressed/wrong and the boundary +should be verified by ear. This is a "review needed" flag, not pass/fail: a +correctly hand-fixed boundary still won't overlap bad tokens, so it can keep +firing on rows that are already fine. + Usage: npx tsx scripts/diagnostics/verify-hook-timing.ts [options] @@ -125,6 +132,7 @@ export type HookLayoutRow = { durationFrames: number; durationSeconds: number; zeroDuration: boolean; + noTokenOverlap: boolean; }; export type HookLayoutReport = { @@ -141,6 +149,26 @@ function toRawSection(sourceStart: number, sourceEnd: number, fps: number) { return { trimBefore, trimAfter }; } +/** + * True if none of the segment's own spoken tokens fall within its authored + * [hookFrom, hookTo] window. This is a purely structural, zero-media-I/O check + * that catches a real, confirmed failure mode: the segment's Whisper/WhisperX + * word-level alignment is compressed/wrong relative to true speech pacing, so + * whoever set the explicit hookFrom/hookTo (by eye, from the same bad token + * positions) picked a range that doesn't actually contain the phrase. + * + * Important caveat: this is a "verify by ear" signal, not a pass/fail one. A + * segment can be flagged here even after its hookFrom/hookTo has been + * correctly hand-fixed — the fix corrects the boundary, not the underlying + * (still-compressed) token timestamps, so the structural mismatch persists by + * design. Don't treat a flagged row as still-broken without listening to it. + */ +function hasTokenOverlap(segment: Segment): boolean { + const from = segment.hookFrom ?? segment.start; + const to = segment.hookTo ?? segment.end; + return segment.tokens.some(t => isSpokenToken(t) && t.t_dtw >= from && t.t_dtw <= to); +} + /** * Recomputes expected hook sections from transcript segments via the canonical * getHookSubClips()/hookClipEnd() (remotion/lib/hookTiming.ts) — the same @@ -186,6 +214,7 @@ export function computeExpectedHookLayout(segments: Segment[], fps: number): Hoo durationFrames: zeroDuration ? 0 : raw.trimAfter - trimBefore, durationSeconds: zeroDuration ? 0 : (raw.trimAfter - trimBefore) / fps, zeroDuration, + noTokenOverlap: !hasTokenOverlap(seg), }); } @@ -200,17 +229,27 @@ function printReport(report: HookLayoutReport) { console.log(`${label.padEnd(20)} ⚠ zero-duration after de-overlap — contributes no frames`); continue; } + const overlapFlag = row.noTokenOverlap ? ' ⚠ NO TOKEN OVERLAP — verify by ear' : ''; console.log( `${label.padEnd(20)} ` + `src [${row.sourceStart.toFixed(3)}s, ${row.sourceEnd.toFixed(3)}s] ` + `frames [${row.trimBefore}, ${row.trimAfter}) ` - + `dur ${row.durationSeconds.toFixed(3)}s`, + + `dur ${row.durationSeconds.toFixed(3)}s${overlapFlag}`, ); } + const flagged = report.rows.filter(r => r.noTokenOverlap); console.log( `\n[verify-hook-timing] Total: ${report.rows.length} hook segments, ` + `${report.totalFrames} frames (${report.totalSeconds.toFixed(2)}s)\n`, ); + if (flagged.length > 0) { + console.log( + `[verify-hook-timing] ${flagged.length} hook(s) have hookFrom/hookTo that don't overlap ` + + `the segment's own token timestamps — this means either the boundary is wrong, or it's a\n` + + `manual override compensating for known-bad token alignment. Verify by listening, don't\n` + + `assume either way from this signal alone: #${flagged.map(r => r.index).join(', #')}\n`, + ); + } } export type ContentDiffResult = { From 13f4f5742a1dca9bbbeffb7dd2f15ce8c9698ab8 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 15:34:27 +0800 Subject: [PATCH 08/16] feat: add ending-completeness check with root-cause triage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds diagnoseHookEnding() to the content-diff layer: compares the transcribed rendered clip's actual last word against the hook's intended final word (hookPhrase, or the segment's own text), and on a mismatch, auto-triages the root cause: - code-bug a token for the word exists inside hookClipEnd's own search window but wasn't used to extend the clip — a real regression to fix in hookTiming.ts. - needs-retiming a token exists outside the window, but the segment's alignment is otherwise trustworthy — widen hookTo (a suggested value is computed). - bad-alignment-data no trustworthy token exists for the word at all — can't be auto-fixed, needs a human/agent to verify the true ending by listening. The zero-token-overlap check (previous commit) only caught the extreme case where a segment's entire token set is disjoint from its hookFrom/ hookTo. A follow-up report that other hooks still end too early, plus a manual spot-check of 5 hooks it didn't flag (4 of 5 genuinely truncated), showed the more common failure mode is a hook whose tokens nominally overlap the window but whose hookTo still lands before the true spoken ending — this check catches that by verifying against the actual rendered audio's content instead of just token structure. Co-Authored-By: Claude Sonnet 5 --- .../HOOK_TIMING_DIAGNOSTICS.md | 70 +++++++ .../diagnostics/verify-hook-timing.test.ts | 93 +++++++- scripts/diagnostics/verify-hook-timing.ts | 198 ++++++++++++++++-- 3 files changed, 346 insertions(+), 15 deletions(-) diff --git a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md index 5298d68..8bf70da 100644 --- a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md +++ b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md @@ -296,6 +296,76 @@ layers alone would not catch. --- +### Commit 7 — `feat: add zero-token-overlap check to hook-timing diagnostic` ✅ DONE + +Structural check: flags any hook whose `hookFrom`/`hookTo` doesn't overlap any +of its own segment's spoken tokens. Found by investigating a user report that +hook #0's rendered audio didn't match its caption — confirmed the segment's +Whisper/WhisperX token alignment was compressed (four words crammed into a +0.4s span that doesn't match real speech pacing). Caught 5 hooks on the real +transcript (#0, #3, #7, #10, #11), all confirmed bad by direct listening and +fixed in `public/edit/transcript.doc.txt`. "Verify by ear" signal, not +pass/fail — a correctly hand-fixed boundary still won't overlap the +underlying bad tokens. + +### Commit 8 — `feat: add ending-completeness check with root-cause triage` ✅ DONE + +**Status check:** `diagnoseHookEnding()` unit tests pass (24 total in the +suite); running `--verify-content` against a real render prints, for any hook +whose transcribed clip doesn't end on the intended word, one of `code-bug` / +`needs-retiming` / `bad-alignment-data` with a suggested fix or a +listen-and-verify recommendation. + +**Why:** the zero-token-overlap check (Commit 7) only catches the extreme case +where the *entire* segment's tokens are disjoint from `[hookFrom, hookTo]`. A +follow-up user report ("the rest of the hooks still tend to end too early") +and a manual spot-check of 5 more hooks not flagged by Commit 7 (#1, #2, #4, +#5 — all genuinely truncated; #8 roughly fine) confirmed a broader pattern: +hooks whose tokens *do* nominally overlap the window, but where `hookTo` still +cuts off before the intended phrase's actual last word. Catching this requires +comparing against the real rendered audio's content, not just token structure. + +**What was done:** +- `lastWordOf(text)` — normalizes and extracts the last word of a string. +- `groupTokensIntoWords(tokens)` — leading-space BPE grouping (deliberately + simpler than `edit-transcript.js`'s `resolvePhraseToTimeRange` grouping, + which also handles contractions/numeric splits — this only needs to find + *a* token that plausibly spells the target word, not resolve phrase bounds). +- `diagnoseHookEnding(segment, transcribedWords, nextHookStart)` — the intended + last word is `hookPhrase`'s last word if set, else `segment.text`'s last + word (caveat: for bare explicit-range hooks without a `hookPhrase`, the + editor may have deliberately chosen a shorter span than the full segment — + this can over-flag in that case; better to over-flag than miss a real + truncation). Compares against the transcribed clip's actual last word. + On mismatch, triages: + - No token anywhere in the segment matches the intended word → `bad-alignment-data`. + - A matching token exists and falls *inside* `hookClipEnd()`'s own search + window `[hookFrom ?? start, hookTo ?? end]` but wasn't used to extend the + clip → `code-bug` (a real regression to fix in `hookTiming.ts`). + - A matching token exists *outside* the window, and at least one other + token in the segment does overlap the window (alignment is mostly + trustworthy) → `needs-retiming`, with a suggested `hookTo` (`token.t_end + + HOOK_TAIL_PAD_BOUNDED_SECONDS`, capped at `nextHookStart`). + - A matching token exists outside the window, but *none* of the segment's + tokens overlap the window at all → downgraded to `bad-alignment-data` + rather than confidently suggesting a widen, since that token's own + timestamp is equally suspect (same compressed-alignment root cause as + Commit 7's cases) — verified by a failing unit test first, which is what + caught this distinction before it shipped. +- Wired into `runContentDiffForHook()` (reuses the already-transcribed words, + no extra whisper call) and surfaced prominently in `printCorrelationResults()` + ahead of the noisier whole-word-set diff, with exit code 1 on any mismatch. +- `scripts/diagnostics/verify-hook-timing.test.ts` — 7 new tests covering all + 4 diagnoses, the `hookPhrase`-over-`text` preference, and the + `nextHookStart` cap. + +**Manual test:** run `npm run diagnose:hooks -- --rendered public/renders/hook-intro.mp4 --source public/sync/output/synced-output-1.mp4 --verify-content` +against the real, fixed transcript and confirm the ending-check output +identifies the remaining truncated hooks found by manual spot-check (#1, #2, +#4, #5, and likely others) with an actionable diagnosis for each. + +--- + ## Done When all 5 commits are complete: `render-hook-intro.js` has no duplicate timing diff --git a/scripts/diagnostics/verify-hook-timing.test.ts b/scripts/diagnostics/verify-hook-timing.test.ts index 16b9c52..4478344 100644 --- a/scripts/diagnostics/verify-hook-timing.test.ts +++ b/scripts/diagnostics/verify-hook-timing.test.ts @@ -3,7 +3,7 @@ * Pure logic, no I/O — mirrors the style of remotion/lib/hookTiming.test.ts. */ -import { computeExpectedHookLayout, expectedWordsForHook, diffWordLists } from './verify-hook-timing'; +import { computeExpectedHookLayout, expectedWordsForHook, diffWordLists, diagnoseHookEnding } from './verify-hook-timing'; import { buildHookSections, HOOK_TAIL_PAD_UNBOUNDED_SECONDS } from '../../remotion/lib/hookTiming'; import type { Segment } from '../../remotion/types/transcript'; @@ -194,3 +194,94 @@ describe('diffWordLists', () => { expect(extra).toEqual([]); }); }); + +describe('diagnoseHookEnding', () => { + it('matches when the transcribed last word equals the intended last word', () => { + const seg = makeSegment({ text: 'is called loop engineering.' }); + const result = diagnoseHookEnding(seg, ['is', 'called', 'loop', 'engineering']); + expect(result.matches).toBe(true); + expect(result.diagnosis).toBe('ok'); + }); + + it('prefers hookPhrase over segment.text for the intended last word when set', () => { + const seg = makeSegment({ text: 'Okay, I guess.', hookPhrase: 'Okay, I' }); + const result = diagnoseHookEnding(seg, ['okay', 'i']); + expect(result.intendedLastWord).toBe('i'); + expect(result.matches).toBe(true); + }); + + it('diagnoses no-intended-text when the segment has no text or hookPhrase', () => { + const seg = makeSegment({ text: '' }); + const result = diagnoseHookEnding(seg, ['whatever']); + expect(result.diagnosis).toBe('no-intended-text'); + expect(result.matches).toBe(true); + }); + + it('diagnoses bad-alignment-data when no token anywhere matches the intended last word', () => { + // Reproduces the real bug: tokens for "is called loop engineering" all sit + // at 40.4-40.825, well before hookFrom/hookTo (41.183-41.568) — same shape + // as hook #0 before it was fixed on the real transcript. + const seg = makeSegment({ + start: 40.4, end: 41.068, hookFrom: 41.183, hookTo: 41.568, + text: 'is called loop engineering.', + tokens: [ + makeToken(' is', 40.4, 40.981), + makeToken(' called', 40.502, 41.163), + makeToken(' loop', 40.683, 41.305), + makeToken(' engineering', 40.825, 41.568), + ], + }); + const result = diagnoseHookEnding(seg, ['this', 'is']); // truncated, wrong content actually rendered + expect(result.matches).toBe(false); + expect(result.diagnosis).toBe('bad-alignment-data'); + expect(result.intendedLastWord).toBe('engineering'); + }); + + it('diagnoses needs-retiming when a matching token exists but falls outside [hookFrom, hookTo]', () => { + const seg = makeSegment({ + hookFrom: 10, hookTo: 10.5, + text: 'a short phrase', + tokens: [ + makeToken(' a', 10.0, 10.1), + makeToken(' short', 10.1, 10.4), + makeToken(' phrase', 10.6, 11.0), // outside [10, 10.5] + ], + }); + const result = diagnoseHookEnding(seg, ['a', 'short']); // clip cut off before "phrase" + expect(result.matches).toBe(false); + expect(result.diagnosis).toBe('needs-retiming'); + expect(result.suggestedHookTo).toBeCloseTo(11.0 + 0.02); // t_end + HOOK_TAIL_PAD_BOUNDED_SECONDS + }); + + it('caps the suggested hookTo at nextHookStart to avoid recommending an overlap', () => { + const seg = makeSegment({ + hookFrom: 10, hookTo: 10.5, + text: 'a short phrase', + tokens: [ + makeToken(' a', 10.0, 10.1), + makeToken(' short', 10.1, 10.4), + makeToken(' phrase', 10.6, 11.0), + ], + }); + const result = diagnoseHookEnding(seg, ['a', 'short'], 10.8); // next hook starts before the token would end + expect(result.diagnosis).toBe('needs-retiming'); + expect(result.suggestedHookTo).toBe(10.8); + }); + + it('diagnoses code-bug when a matching token exists inside the search window but was not used', () => { + const seg = makeSegment({ + hookFrom: 10, hookTo: 11, + text: 'a short phrase', + tokens: [ + makeToken(' a', 10.0, 10.1), + makeToken(' short', 10.1, 10.4), + makeToken(' phrase', 10.5, 10.9), // inside [10, 11] — should have been included + ], + }); + // Simulates a render that (incorrectly) cut off before "phrase" despite its + // token being well inside hookClipEnd()'s own search window. + const result = diagnoseHookEnding(seg, ['a', 'short']); + expect(result.matches).toBe(false); + expect(result.diagnosis).toBe('code-bug'); + }); +}); diff --git a/scripts/diagnostics/verify-hook-timing.ts b/scripts/diagnostics/verify-hook-timing.ts index 5d52032..bcce8f6 100644 --- a/scripts/diagnostics/verify-hook-timing.ts +++ b/scripts/diagnostics/verify-hook-timing.ts @@ -18,7 +18,7 @@ import fs from 'fs-extra'; import os from 'os'; import path from 'path'; -import { getHookSubClips } from '../../remotion/lib/hookTiming'; +import { getHookSubClips, HOOK_TAIL_PAD_BOUNDED_SECONDS } from '../../remotion/lib/hookTiming'; import { isSpokenToken } from '../../remotion/lib/tokens'; import type { Segment } from '../../remotion/types/transcript'; import { extractAudioWindow, loadWavSamples } from '../lib/extractAudioWindow.js'; @@ -117,6 +117,18 @@ Content-diff layer (opt-in; requires --rendered; off by default, slow): --verify-content Transcribe each rendered hook (whisper.cpp) and diff its words against the expected phrase — catches wrong-phrase bugs pure timing checks can't see. May trigger a one-time model download. + Also checks whether the transcribed clip's LAST word matches the + hook's intended final word (hookPhrase, or the segment's own text) + and auto-diagnoses a mismatch as one of: + code-bug hookClipEnd() missed a token inside its own + search window — a real regression to fix in + remotion/lib/hookTiming.ts. + needs-retiming the word's token exists but falls outside + [hookFrom, hookTo] — widen hookTo (a suggested + value is printed). + bad-alignment-data no token for that word exists anywhere in the + segment — its Whisper/WhisperX alignment is + unreliable; verify the true ending by ear. --content-model Whisper model to use (default: ${DEFAULT_CONTENT_MODEL}) `); } @@ -252,11 +264,23 @@ function printReport(report: HookLayoutReport) { } } +export type EndingDiagnosis = 'ok' | 'code-bug' | 'needs-retiming' | 'bad-alignment-data' | 'no-intended-text'; + +export type EndingCheckResult = { + intendedLastWord: string; + transcribedLastWord: string | null; + matches: boolean; + diagnosis: EndingDiagnosis; + suggestedHookTo?: number; + note: string; +}; + export type ContentDiffResult = { expectedWords: string[]; transcribedWords: string[]; missingWords: string[]; extraWords: string[]; + endingCheck: EndingCheckResult; }; export type CorrelationResult = { @@ -320,12 +344,133 @@ export function diffWordLists(expectedWords: string[], transcribedWords: string[ return { missing, extra }; } -/** Transcribes a short hook audio clip (whisper.cpp) and diffs it against the expected words. */ +function lastWordOf(text: string): string { + const words = text.trim().split(/\s+/).filter(Boolean); + return normalizeWord(words[words.length - 1] ?? ''); +} + +type WordGroup = { text: string; t_dtw: number; t_end?: number }; + +/** + * Groups a segment's raw (possibly BPE sub-word) tokens into word-level + * groups, using the leading-space heuristic (a new group starts at each token + * with a leading space, or the first spoken token). Deliberately simpler than + * edit-transcript.js's resolvePhraseToTimeRange word-grouping (which also + * handles contractions/numeric splits) — this only needs to answer "is there + * a token anywhere in this segment that plausibly spells the target word," + * not resolve exact phrase boundaries. + */ +function groupTokensIntoWords(tokens: Segment['tokens']): WordGroup[] { + const groups: WordGroup[] = []; + for (const t of tokens) { + if (!isSpokenToken(t)) continue; + if (groups.length === 0 || t.text.startsWith(' ')) { + groups.push({ text: t.text, t_dtw: t.t_dtw, t_end: t.t_end }); + } else { + const prev = groups[groups.length - 1]; + prev.text += t.text; + if (t.t_end !== undefined) prev.t_end = t.t_end; + } + } + return groups; +} + +/** + * Diagnoses a hook whose rendered ending doesn't match its intended final + * word. Answers "is this a code bug in hookClipEnd(), or does the boundary + * need manual retiming?" by checking whether a token for the intended last + * word exists, and if so, whether it falls inside hookClipEnd()'s own search + * window [hookFrom ?? start, hookTo ?? end]: + * + * - No matching token anywhere in the segment → the segment's own word-level + * alignment doesn't have this word at all. Can't be auto-fixed; needs a + * human/agent to listen and set the boundary by ear. + * - Matching token found INSIDE the search window → hookClipEnd() should have + * extended to cover it (per its own documented algorithm) but didn't. That + * is a real code regression in hookClipEnd()'s last-spoken-token selection. + * - Matching token found OUTSIDE the search window, AND at least one other + * token in the segment DOES overlap the window (its alignment is mostly + * trustworthy, just the boundary is a little tight) → widening hookTo to + * cover the token's t_end would fix it. A data/tuning issue, not a code bug. + * - Matching token found outside the window, but NONE of the segment's + * tokens overlap [hookFrom, hookTo] at all → this is the same compressed/ + * wrong-alignment failure mode as "no matching token" (confirmed on this + * transcript's hooks #0/#3/#7/#10/#11): the matched token's own timestamp + * can't be trusted either, so a suggested widen could still be wrong. + * Downgraded to bad-alignment-data rather than confidently suggesting a fix. + */ +export function diagnoseHookEnding( + segment: Segment, + transcribedWords: string[], + nextHookStart?: number, +): EndingCheckResult { + const intendedText = segment.hookPhrase || segment.text; + const intendedLastWord = lastWordOf(intendedText); + const transcribedLastWord = transcribedWords.length > 0 ? transcribedWords[transcribedWords.length - 1] : null; + + if (!intendedLastWord) { + return { + intendedLastWord, transcribedLastWord, matches: true, diagnosis: 'no-intended-text', + note: 'Segment has no text/hookPhrase to check an ending against.', + }; + } + + if (transcribedLastWord === intendedLastWord) { + return { intendedLastWord, transcribedLastWord, matches: true, diagnosis: 'ok', note: 'Last word matches.' }; + } + + const sourceStart = segment.hookFrom ?? segment.start; + const baseEnd = segment.hookTo ?? segment.end; + const matchGroup = groupTokensIntoWords(segment.tokens).find(g => normalizeWord(g.text) === intendedLastWord); + + if (!matchGroup) { + return { + intendedLastWord, transcribedLastWord, matches: false, diagnosis: 'bad-alignment-data', + note: `No token in this segment matches "${intendedLastWord}" — its word-level alignment appears ` + + `unreliable (same failure mode as hooks #0/#3/#7/#10/#11 on this transcript). Verify the true ` + + `ending by listening; this can't be diagnosed further automatically.`, + }; + } + + const inSearchWindow = matchGroup.t_dtw >= sourceStart && matchGroup.t_dtw <= baseEnd; + if (inSearchWindow) { + return { + intendedLastWord, transcribedLastWord, matches: false, diagnosis: 'code-bug', + note: `Token for "${intendedLastWord}" (t_dtw=${matchGroup.t_dtw}, t_end=${matchGroup.t_end}) falls ` + + `inside hookClipEnd()'s own search window [${sourceStart}, ${baseEnd}] but wasn't used to extend ` + + `the clip — check hookClipEnd()'s last-spoken-token selection in remotion/lib/hookTiming.ts for a regression.`, + }; + } + + const hasAnyOverlap = segment.tokens.some(t => isSpokenToken(t) && t.t_dtw >= sourceStart && t.t_dtw <= baseEnd); + if (!hasAnyOverlap) { + return { + intendedLastWord, transcribedLastWord, matches: false, diagnosis: 'bad-alignment-data', + note: `Token for "${intendedLastWord}" exists at t_dtw=${matchGroup.t_dtw}, but NONE of this segment's ` + + `tokens overlap [${sourceStart}, ${baseEnd}] at all — its alignment is compressed/unreliable (same ` + + `pattern as hooks #0/#3/#7/#10/#11 on this transcript), so this token's own timestamp can't be ` + + `trusted for a widen suggestion either. Verify the true ending by listening.`, + }; + } + + let suggestedHookTo = (matchGroup.t_end ?? matchGroup.t_dtw) + HOOK_TAIL_PAD_BOUNDED_SECONDS; + if (nextHookStart !== undefined) suggestedHookTo = Math.min(suggestedHookTo, nextHookStart); + + return { + intendedLastWord, transcribedLastWord, matches: false, diagnosis: 'needs-retiming', suggestedHookTo, + note: `Token for "${intendedLastWord}" exists at t_dtw=${matchGroup.t_dtw} (t_end=${matchGroup.t_end}) ` + + `but falls outside the current hookTo=${baseEnd} — widen hookTo to ~${suggestedHookTo.toFixed(3)}.`, + }; +} + +/** Transcribes a short hook audio clip (whisper.cpp), diffs it against the expected words, and + * diagnoses whether its ending matches the intended final word. */ async function runContentDiffForHook( segment: Segment, wavPath: string, workDir: string, model: string, + nextHookStart: number | undefined, ): Promise { const expectedWords = expectedWordsForHook(segment); @@ -341,7 +486,8 @@ async function runContentDiffForHook( } const { missing, extra } = diffWordLists(expectedWords, transcribedWords); - return { expectedWords, transcribedWords, missingWords: missing, extraWords: extra }; + const endingCheck = diagnoseHookEnding(segment, transcribedWords, nextHookStart); + return { expectedWords, transcribedWords, missingWords: missing, extraWords: extra, endingCheck }; } /** @@ -397,8 +543,10 @@ export async function runAudioCorrelationLayer( if (verifyContent) { const workDir = path.join(tempDir, `content-${row.index}`); await fs.ensureDir(workDir); + const nextSeg = report.hookSegments[row.index + 1]; + const nextHookStart = nextSeg ? (nextSeg.hookFrom ?? nextSeg.start) : undefined; // Reuses the already-extracted renderedWav rather than re-extracting. - contentDiff = await runContentDiffForHook(report.hookSegments[row.index], renderedWav, workDir, verifyContent.model); + contentDiff = await runContentDiffForHook(report.hookSegments[row.index], renderedWav, workDir, verifyContent.model, nextHookStart); } results.push({ @@ -418,32 +566,53 @@ export async function runAudioCorrelationLayer( } } +const ENDING_DIAGNOSIS_LABEL: Record = { + ok: '', + 'no-intended-text': '', + 'code-bug': '✗ CODE BUG', + 'needs-retiming': '✗ NEEDS RETIMING', + 'bad-alignment-data': '✗ ENDS EARLY (bad alignment data)', +}; + function printCorrelationResults(results: CorrelationResult[], toleranceMs: number) { console.log(`[verify-hook-timing] Audio cross-correlation (tolerance ±${toleranceMs}ms):\n`); for (const r of results) { const flag = r.exceedsTolerance ? '✗ DRIFT' : '✓'; const reliability = r.isReliable ? '' : ' (low-confidence peak — SNR below threshold)'; console.log(`#${String(r.index).padEnd(4)} ${flag.padEnd(8)} lag ${r.lagMs.toFixed(1)}ms snr ${r.snr.toFixed(2)}${reliability}`); + + const ending = r.contentDiff?.endingCheck; + if (ending && !ending.matches) { + console.log(` ${ENDING_DIAGNOSIS_LABEL[ending.diagnosis]}`); + console.log(` expected last word: "${ending.intendedLastWord}" transcribed last word: "${ending.transcribedLastWord ?? '(none)'}"`); + console.log(` ${ending.note}`); + } if (r.contentDiff && (r.contentDiff.missingWords.length > 0 || r.contentDiff.extraWords.length > 0)) { if (r.contentDiff.missingWords.length > 0) console.log(` missing: ${r.contentDiff.missingWords.join(', ')}`); if (r.contentDiff.extraWords.length > 0) console.log(` extra: ${r.contentDiff.extraWords.join(', ')}`); } } const failing = results.filter(r => r.exceedsTolerance); - const contentMismatches = results.filter( - r => r.contentDiff && (r.contentDiff.missingWords.length > 0 || r.contentDiff.extraWords.length > 0), - ); console.log( failing.length > 0 ? `\n[verify-hook-timing] ${failing.length}/${results.length} hook(s) exceed tolerance.\n` : `\n[verify-hook-timing] All ${results.length} hook(s) within tolerance.\n`, ); - if (results.some(r => r.contentDiff)) { - console.log( - contentMismatches.length > 0 - ? `[verify-hook-timing] ${contentMismatches.length}/${results.length} hook(s) have word mismatches.\n` - : `[verify-hook-timing] All hook(s) content-verified — no word mismatches.\n`, - ); + + const withEndingCheck = results.filter(r => r.contentDiff?.endingCheck); + if (withEndingCheck.length > 0) { + const codeBugs = withEndingCheck.filter(r => r.contentDiff?.endingCheck.diagnosis === 'code-bug'); + const needsRetiming = withEndingCheck.filter(r => r.contentDiff?.endingCheck.diagnosis === 'needs-retiming'); + const badAlignment = withEndingCheck.filter(r => r.contentDiff?.endingCheck.diagnosis === 'bad-alignment-data'); + if (codeBugs.length + needsRetiming.length + badAlignment.length === 0) { + console.log(`[verify-hook-timing] All hook(s) end on the intended word.\n`); + } else { + console.log(`[verify-hook-timing] Ending check: ${withEndingCheck.length - codeBugs.length - needsRetiming.length - badAlignment.length}/${withEndingCheck.length} end correctly.`); + if (codeBugs.length > 0) console.log(` ✗ ${codeBugs.length} likely CODE BUG (hookClipEnd missed an in-window token): #${codeBugs.map(r => r.index).join(', #')}`); + if (needsRetiming.length > 0) console.log(` ✗ ${needsRetiming.length} need RETIMING (widen hookTo — suggested values above): #${needsRetiming.map(r => r.index).join(', #')}`); + if (badAlignment.length > 0) console.log(` ✗ ${badAlignment.length} have BAD ALIGNMENT DATA (verify by ear): #${badAlignment.map(r => r.index).join(', #')}`); + console.log(''); + } } } @@ -497,7 +666,8 @@ async function main() { const results = await runAudioCorrelationLayer(report, cli.renderedPath, sourcePath, cli.sampleRate, cli.toleranceMs, verifyContent); printCorrelationResults(results, cli.toleranceMs); const hasContentMismatch = results.some(r => r.contentDiff && (r.contentDiff.missingWords.length > 0 || r.contentDiff.extraWords.length > 0)); - if (results.some(r => r.exceedsTolerance) || hasContentMismatch) process.exitCode = 1; + const hasEndingMismatch = results.some(r => r.contentDiff && !r.contentDiff.endingCheck.matches); + if (results.some(r => r.exceedsTolerance) || hasContentMismatch || hasEndingMismatch) process.exitCode = 1; } } From 92f2993a13fed5bde80752176619372886a36434 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 15:52:44 +0800 Subject: [PATCH 09/16] fix: remove false 'code-bug' diagnosis from ending-completeness check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit's diagnoseHookEnding() classified an ending mismatch as a hookClipEnd() code regression whenever the matching token's t_dtw fell inside hookClipEnd()'s own search window. That reasoning is wrong by construction: hookClipEnd() always extends sourceEnd to cover whichever in-window spoken token has the LARGEST t_end, so any token found by text-matching is necessarily one of the candidates already considered — its t_end can never exceed what hookClipEnd() used. An in-window mismatch can therefore never indicate a real code bug; it can only mean the matched token's own timestamp is wrong. Confirmed directly on hook #4 of the ragtech transcript: hookClipEnd() computed sourceEnd=63.466 (called directly to verify), correctly covering the "makes" token's t_end=63.446 — yet both the real rendered clip and the raw source video, transcribed independently, say "...where every time there's a..." trailing into silence. The token data for "the/agentic/tool/makes" doesn't correspond to real speech at that position (a stumbled false start got the clean phrase's timestamp, not a code regression). Collapses 'code-bug' and 'needs-retiming' into a single 'needs-verification' diagnosis: reports the matched token's timing as a starting point to check by ear, explicitly not a confirmed fix. Renamed suggestedHookTo -> candidateHookTo to make that non-guarantee clear in the API too. Co-Authored-By: Claude Sonnet 5 --- .../diagnostics/verify-hook-timing.test.ts | 29 ++-- scripts/diagnostics/verify-hook-timing.ts | 137 +++++++++--------- 2 files changed, 84 insertions(+), 82 deletions(-) diff --git a/scripts/diagnostics/verify-hook-timing.test.ts b/scripts/diagnostics/verify-hook-timing.test.ts index 4478344..6caa94e 100644 --- a/scripts/diagnostics/verify-hook-timing.test.ts +++ b/scripts/diagnostics/verify-hook-timing.test.ts @@ -237,7 +237,7 @@ describe('diagnoseHookEnding', () => { expect(result.intendedLastWord).toBe('engineering'); }); - it('diagnoses needs-retiming when a matching token exists but falls outside [hookFrom, hookTo]', () => { + it('diagnoses needs-verification when a matching token exists outside [hookFrom, hookTo] but the segment has other overlap', () => { const seg = makeSegment({ hookFrom: 10, hookTo: 10.5, text: 'a short phrase', @@ -249,11 +249,11 @@ describe('diagnoseHookEnding', () => { }); const result = diagnoseHookEnding(seg, ['a', 'short']); // clip cut off before "phrase" expect(result.matches).toBe(false); - expect(result.diagnosis).toBe('needs-retiming'); - expect(result.suggestedHookTo).toBeCloseTo(11.0 + 0.02); // t_end + HOOK_TAIL_PAD_BOUNDED_SECONDS + expect(result.diagnosis).toBe('needs-verification'); + expect(result.candidateHookTo).toBeCloseTo(11.0 + 0.02); // t_end + HOOK_TAIL_PAD_BOUNDED_SECONDS }); - it('caps the suggested hookTo at nextHookStart to avoid recommending an overlap', () => { + it('caps the candidate hookTo at nextHookStart to avoid recommending an overlap', () => { const seg = makeSegment({ hookFrom: 10, hookTo: 10.5, text: 'a short phrase', @@ -264,24 +264,31 @@ describe('diagnoseHookEnding', () => { ], }); const result = diagnoseHookEnding(seg, ['a', 'short'], 10.8); // next hook starts before the token would end - expect(result.diagnosis).toBe('needs-retiming'); - expect(result.suggestedHookTo).toBe(10.8); + expect(result.diagnosis).toBe('needs-verification'); + expect(result.candidateHookTo).toBe(10.8); }); - it('diagnoses code-bug when a matching token exists inside the search window but was not used', () => { + it('diagnoses needs-verification (not a false "code-bug") when a matching token is inside the search window but still doesn\'t match the render', () => { + // Reproduces the real discovery on hook #4 of the ragtech transcript: the + // "makes" token (t_dtw=62.806, t_end=63.446) sits inside hookClipEnd()'s + // search window [61.823, 63.446], and hookClipEnd() DOES correctly extend + // sourceEnd to cover it (verified directly: sourceEnd=63.466) — yet both + // the real rendered clip and the raw source video, transcribed + // independently, said "...where every time there's a..." trailing off. + // The token's own timestamp was wrong (mapped from a stumbled false start), + // not a hookClipEnd regression. An earlier version of this function called + // this scenario "code-bug"; that was disproven and removed. const seg = makeSegment({ hookFrom: 10, hookTo: 11, text: 'a short phrase', tokens: [ makeToken(' a', 10.0, 10.1), makeToken(' short', 10.1, 10.4), - makeToken(' phrase', 10.5, 10.9), // inside [10, 11] — should have been included + makeToken(' phrase', 10.5, 10.9), // inside [10, 11] ], }); - // Simulates a render that (incorrectly) cut off before "phrase" despite its - // token being well inside hookClipEnd()'s own search window. const result = diagnoseHookEnding(seg, ['a', 'short']); expect(result.matches).toBe(false); - expect(result.diagnosis).toBe('code-bug'); + expect(result.diagnosis).toBe('needs-verification'); }); }); diff --git a/scripts/diagnostics/verify-hook-timing.ts b/scripts/diagnostics/verify-hook-timing.ts index bcce8f6..3f8ed44 100644 --- a/scripts/diagnostics/verify-hook-timing.ts +++ b/scripts/diagnostics/verify-hook-timing.ts @@ -119,16 +119,20 @@ Content-diff layer (opt-in; requires --rendered; off by default, slow): timing checks can't see. May trigger a one-time model download. Also checks whether the transcribed clip's LAST word matches the hook's intended final word (hookPhrase, or the segment's own text) - and auto-diagnoses a mismatch as one of: - code-bug hookClipEnd() missed a token inside its own - search window — a real regression to fix in - remotion/lib/hookTiming.ts. - needs-retiming the word's token exists but falls outside - [hookFrom, hookTo] — widen hookTo (a suggested - value is printed). + and reports a mismatch as one of: + needs-verification a token spelling that word exists somewhere in + the segment, printed as a starting point to + check by ear — NOT a confident fix. Proven on + this transcript that even a token nominally + inside hookClipEnd()'s search window can still + be individually misaligned (e.g. mapped from a + stumbled false start rather than the clean + utterance), so hookClipEnd() extending exactly + as designed doesn't guarantee correct audio. bad-alignment-data no token for that word exists anywhere in the - segment — its Whisper/WhisperX alignment is - unreliable; verify the true ending by ear. + segment, or none of its tokens overlap + [hookFrom, hookTo] at all — no data-driven + candidate to check; verify by ear from scratch. --content-model Whisper model to use (default: ${DEFAULT_CONTENT_MODEL}) `); } @@ -264,14 +268,15 @@ function printReport(report: HookLayoutReport) { } } -export type EndingDiagnosis = 'ok' | 'code-bug' | 'needs-retiming' | 'bad-alignment-data' | 'no-intended-text'; +export type EndingDiagnosis = 'ok' | 'needs-verification' | 'bad-alignment-data' | 'no-intended-text'; export type EndingCheckResult = { intendedLastWord: string; transcribedLastWord: string | null; matches: boolean; diagnosis: EndingDiagnosis; - suggestedHookTo?: number; + /** A starting point to check by ear, NOT a confident fix — see diagnoseHookEnding's docstring. */ + candidateHookTo?: number; note: string; }; @@ -377,27 +382,36 @@ function groupTokensIntoWords(tokens: Segment['tokens']): WordGroup[] { /** * Diagnoses a hook whose rendered ending doesn't match its intended final - * word. Answers "is this a code bug in hookClipEnd(), or does the boundary - * need manual retiming?" by checking whether a token for the intended last - * word exists, and if so, whether it falls inside hookClipEnd()'s own search - * window [hookFrom ?? start, hookTo ?? end]: + * word, by checking whether a token spelling that word exists anywhere in the + * segment. * - * - No matching token anywhere in the segment → the segment's own word-level - * alignment doesn't have this word at all. Can't be auto-fixed; needs a - * human/agent to listen and set the boundary by ear. - * - Matching token found INSIDE the search window → hookClipEnd() should have - * extended to cover it (per its own documented algorithm) but didn't. That - * is a real code regression in hookClipEnd()'s last-spoken-token selection. - * - Matching token found OUTSIDE the search window, AND at least one other - * token in the segment DOES overlap the window (its alignment is mostly - * trustworthy, just the boundary is a little tight) → widening hookTo to - * cover the token's t_end would fix it. A data/tuning issue, not a code bug. - * - Matching token found outside the window, but NONE of the segment's - * tokens overlap [hookFrom, hookTo] at all → this is the same compressed/ - * wrong-alignment failure mode as "no matching token" (confirmed on this - * transcript's hooks #0/#3/#7/#10/#11): the matched token's own timestamp - * can't be trusted either, so a suggested widen could still be wrong. - * Downgraded to bad-alignment-data rather than confidently suggesting a fix. + * IMPORTANT — this does NOT distinguish "code bug in hookClipEnd()" from + * "boundary needs retiming", and deliberately doesn't try to. An earlier + * version of this function checked whether the matching token fell inside + * hookClipEnd()'s own search window [hookFrom ?? start, hookTo ?? end] and + * called that a "code bug" if the render still didn't reach the word. That + * reasoning was wrong: hookClipEnd() always extends sourceEnd to cover + * whichever in-window spoken token has the LARGEST t_end — so any token found + * by matching its text is, by construction, one of the candidates already + * considered, and can never have a t_end exceeding what hookClipEnd() used. + * A mismatch given an in-window match is therefore never a hookClipEnd + * regression — it can only mean the matched token's own timestamp is wrong + * (e.g. mapped from a stumbled false start rather than the clean utterance + * that actually says the word). Confirmed directly on this transcript's hook + * #4: hookClipEnd() computed sourceEnd=63.466 (verified by calling it + * directly), correctly covering the "makes" token's t_end=63.446 — yet both + * the real rendered clip AND the raw source video, transcribed independently, + * say "...where every time there's a..." trailing into silence. The token + * data for "the/agentic/tool/makes" simply doesn't correspond to real speech + * at that position. No code bug; the alignment data is unreliable. + * + * - No matching token anywhere in the segment, OR none of the segment's + * tokens overlap [hookFrom, hookTo] at all → 'bad-alignment-data'. No + * data-driven candidate exists to check; the ending must be found by ear. + * - A matching token exists AND at least one token in the segment overlaps + * the window → 'needs-verification', reporting the matched token's timing + * as a starting point. This is NOT a confident fix (see above) — it's + * where to listen first. */ export function diagnoseHookEnding( segment: Segment, @@ -422,44 +436,28 @@ export function diagnoseHookEnding( const sourceStart = segment.hookFrom ?? segment.start; const baseEnd = segment.hookTo ?? segment.end; const matchGroup = groupTokensIntoWords(segment.tokens).find(g => normalizeWord(g.text) === intendedLastWord); - - if (!matchGroup) { - return { - intendedLastWord, transcribedLastWord, matches: false, diagnosis: 'bad-alignment-data', - note: `No token in this segment matches "${intendedLastWord}" — its word-level alignment appears ` - + `unreliable (same failure mode as hooks #0/#3/#7/#10/#11 on this transcript). Verify the true ` - + `ending by listening; this can't be diagnosed further automatically.`, - }; - } - - const inSearchWindow = matchGroup.t_dtw >= sourceStart && matchGroup.t_dtw <= baseEnd; - if (inSearchWindow) { - return { - intendedLastWord, transcribedLastWord, matches: false, diagnosis: 'code-bug', - note: `Token for "${intendedLastWord}" (t_dtw=${matchGroup.t_dtw}, t_end=${matchGroup.t_end}) falls ` - + `inside hookClipEnd()'s own search window [${sourceStart}, ${baseEnd}] but wasn't used to extend ` - + `the clip — check hookClipEnd()'s last-spoken-token selection in remotion/lib/hookTiming.ts for a regression.`, - }; - } - const hasAnyOverlap = segment.tokens.some(t => isSpokenToken(t) && t.t_dtw >= sourceStart && t.t_dtw <= baseEnd); - if (!hasAnyOverlap) { + + if (!matchGroup || !hasAnyOverlap) { return { intendedLastWord, transcribedLastWord, matches: false, diagnosis: 'bad-alignment-data', - note: `Token for "${intendedLastWord}" exists at t_dtw=${matchGroup.t_dtw}, but NONE of this segment's ` - + `tokens overlap [${sourceStart}, ${baseEnd}] at all — its alignment is compressed/unreliable (same ` - + `pattern as hooks #0/#3/#7/#10/#11 on this transcript), so this token's own timestamp can't be ` - + `trusted for a widen suggestion either. Verify the true ending by listening.`, + note: matchGroup + ? `Token for "${intendedLastWord}" exists at t_dtw=${matchGroup.t_dtw}, but NONE of this segment's ` + + `tokens overlap [${sourceStart}, ${baseEnd}] at all — its alignment is compressed/unreliable, so ` + + `this token's own timestamp can't be trusted either. Verify the true ending by listening.` + : `No token in this segment matches "${intendedLastWord}" — its word-level alignment appears ` + + `unreliable. Verify the true ending by listening; this can't be diagnosed further automatically.`, }; } - let suggestedHookTo = (matchGroup.t_end ?? matchGroup.t_dtw) + HOOK_TAIL_PAD_BOUNDED_SECONDS; - if (nextHookStart !== undefined) suggestedHookTo = Math.min(suggestedHookTo, nextHookStart); + let candidateHookTo = (matchGroup.t_end ?? matchGroup.t_dtw) + HOOK_TAIL_PAD_BOUNDED_SECONDS; + if (nextHookStart !== undefined) candidateHookTo = Math.min(candidateHookTo, nextHookStart); return { - intendedLastWord, transcribedLastWord, matches: false, diagnosis: 'needs-retiming', suggestedHookTo, - note: `Token for "${intendedLastWord}" exists at t_dtw=${matchGroup.t_dtw} (t_end=${matchGroup.t_end}) ` - + `but falls outside the current hookTo=${baseEnd} — widen hookTo to ~${suggestedHookTo.toFixed(3)}.`, + intendedLastWord, transcribedLastWord, matches: false, diagnosis: 'needs-verification', candidateHookTo, + note: `Token for "${intendedLastWord}" exists at t_dtw=${matchGroup.t_dtw} (t_end=${matchGroup.t_end}) — ` + + `check by ear around ~${candidateHookTo.toFixed(3)}s. This is a starting point, not a confirmed fix: ` + + `the token could itself be misaligned even though it's within hookClipEnd()'s search window.`, }; } @@ -569,9 +567,8 @@ export async function runAudioCorrelationLayer( const ENDING_DIAGNOSIS_LABEL: Record = { ok: '', 'no-intended-text': '', - 'code-bug': '✗ CODE BUG', - 'needs-retiming': '✗ NEEDS RETIMING', - 'bad-alignment-data': '✗ ENDS EARLY (bad alignment data)', + 'needs-verification': '✗ ENDS EARLY — check candidate below by ear', + 'bad-alignment-data': '✗ ENDS EARLY (bad alignment data — no candidate)', }; function printCorrelationResults(results: CorrelationResult[], toleranceMs: number) { @@ -601,16 +598,14 @@ function printCorrelationResults(results: CorrelationResult[], toleranceMs: numb const withEndingCheck = results.filter(r => r.contentDiff?.endingCheck); if (withEndingCheck.length > 0) { - const codeBugs = withEndingCheck.filter(r => r.contentDiff?.endingCheck.diagnosis === 'code-bug'); - const needsRetiming = withEndingCheck.filter(r => r.contentDiff?.endingCheck.diagnosis === 'needs-retiming'); + const needsVerification = withEndingCheck.filter(r => r.contentDiff?.endingCheck.diagnosis === 'needs-verification'); const badAlignment = withEndingCheck.filter(r => r.contentDiff?.endingCheck.diagnosis === 'bad-alignment-data'); - if (codeBugs.length + needsRetiming.length + badAlignment.length === 0) { + if (needsVerification.length + badAlignment.length === 0) { console.log(`[verify-hook-timing] All hook(s) end on the intended word.\n`); } else { - console.log(`[verify-hook-timing] Ending check: ${withEndingCheck.length - codeBugs.length - needsRetiming.length - badAlignment.length}/${withEndingCheck.length} end correctly.`); - if (codeBugs.length > 0) console.log(` ✗ ${codeBugs.length} likely CODE BUG (hookClipEnd missed an in-window token): #${codeBugs.map(r => r.index).join(', #')}`); - if (needsRetiming.length > 0) console.log(` ✗ ${needsRetiming.length} need RETIMING (widen hookTo — suggested values above): #${needsRetiming.map(r => r.index).join(', #')}`); - if (badAlignment.length > 0) console.log(` ✗ ${badAlignment.length} have BAD ALIGNMENT DATA (verify by ear): #${badAlignment.map(r => r.index).join(', #')}`); + console.log(`[verify-hook-timing] Ending check: ${withEndingCheck.length - needsVerification.length - badAlignment.length}/${withEndingCheck.length} end correctly.`); + if (needsVerification.length > 0) console.log(` ✗ ${needsVerification.length} NEED VERIFICATION (candidate timing printed above, not a confirmed fix): #${needsVerification.map(r => r.index).join(', #')}`); + if (badAlignment.length > 0) console.log(` ✗ ${badAlignment.length} have BAD ALIGNMENT DATA (no candidate — verify by ear from scratch): #${badAlignment.map(r => r.index).join(', #')}`); console.log(''); } } From 4125184c31246826997eebf5cbe07f5c7276d448 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 17:28:10 +0800 Subject: [PATCH 10/16] feat: add scoped WhisperX re-alignment for hook captions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixing hookFrom/hookTo doesn't fix hook captions: HookOverlay's buildCaptions() filters a segment's tokens to those whose t_dtw falls within [hookFrom, hookClipEnd()) — for hooks whose tokens are compressed entirely outside that window (the same alignment-quality issue behind the earlier hookFrom/hookTo fixes), buildCaptions() returns [] and no caption renders for the whole hook, even though audio/video play correctly. No override field exists on Segment to substitute caption timing independent of tokens. Adds scripts/align/realign-hooks.js: re-runs the same WhisperX forced- alignment primitive the pipeline already uses for the whole episode (run_whisperx_align.py), scoped to each hook's own window and text. Uses a purpose-built merge instead of reusing align-transcript.js's proportional-remap fallback for unmatched tokens — that assumes every token in a segment belongs within the new window, which is false whenever hookPhrase is a subset of the segment's words. Confirmed by hitting the bug directly: remapping unmatched tokens from their old (already-moved) position on a second run collapsed every token to the window's end. Matched tokens now get fresh WhisperX timestamps; everything else is deterministically excluded instead. Also exports applyAlignment/spawnPython from align-transcript.js for reuse, and fixes a real bug found while building this: an unmatched token's stale t_end was never cleared after its t_dtw got remapped forward, producing an invalid t_end < t_dtw state. Verified on the real transcript: the zero-token-overlap check (previously flagging 6 hooks) now reports none across all 36 hooks. Co-Authored-By: Claude Sonnet 5 --- .../HOOK_TIMING_DIAGNOSTICS.md | 60 +++++ scripts/align/align-transcript.js | 16 +- scripts/align/realign-hooks.js | 246 ++++++++++++++++++ 3 files changed, 317 insertions(+), 5 deletions(-) create mode 100644 scripts/align/realign-hooks.js diff --git a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md index 8bf70da..74b6d33 100644 --- a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md +++ b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md @@ -366,6 +366,66 @@ identifies the remaining truncated hooks found by manual spot-check (#1, #2, --- +### Commit 9 — `feat: add scoped WhisperX re-alignment for hook captions` ✅ DONE + +**Status check:** `npx tsx scripts/align/realign-hooks.js --dry-run` lists hook +segments with their alignment window and text; running it for real updates +tokens in `transcript.json` such that `npx tsx scripts/diagnostics/verify-hook-timing.ts` +reports zero "no token overlap" warnings across all hooks (previously 6). + +**Why:** fixing `hookFrom`/`hookTo` (Commits 6–8) doesn't fix hook captions. +`HookOverlay.tsx`'s `buildCaptions()` filters a segment's tokens to those whose +`t_dtw` falls within `[hookFrom ?? start, hookClipEnd(...))` — for any hook +whose tokens are compressed entirely outside that window (the same root cause +as Commits 6–8), `buildCaptions()` returns `[]` and no caption renders for the +whole hook, even though the audio/video play correctly. There's no override +field on `Segment` to substitute caption timing independent of `tokens`. + +**What was done:** +- `scripts/align/align-transcript.js` — exported `applyAlignment`/`spawnPython` + for reuse (previously private to the file). Fixed a real bug in + `assignTokenTimes`'s fallback path found while building this: an unmatched + token's stale `t_end` (belonging to its *old* `t_dtw`) was never cleared, so + after remapping `t_dtw` forward it could end up with `t_end < t_dtw` — an + invalid state. Now explicitly cleared when unmatched. +- `scripts/align/realign-hooks.js` (new) — re-runs the *same* WhisperX forced- + alignment primitive (`run_whisperx_align.py`) the pipeline already uses for + the whole episode, but scoped to each hook's own `[hookFrom, hookClipEnd)` + window (via `hookClipEnd()`) and its `hookPhrase ?? text` as the target text. + Deliberately does **not** reuse `assignTokenTimes`'s proportional-remap + fallback for unmatched tokens — that assumes every token in a segment + belongs within the new window, which is false whenever `hookPhrase` is a + subset of the segment's words (e.g. "loop engineering" out of "is called + loop engineering."). Confirmed by hitting the bug directly: remapping + unmatched tokens from their old (already-moved) position on a second run + collapsed every token to the window's end. The fix: matched tokens get fresh + WhisperX timestamps; everything else is deterministically pushed to + `sourceStart - 1` (excluded from the window, matching what should happen to + words outside the aligned phrase — exact value doesn't matter, only exclusion). +- `--skip ` excludes segments whose *text* doesn't match the audio at + all (confirmed hooks #4/#5 on this transcript — a stumbled false start + transcribed as clean text upstream by whisper.cpp, inherited unchanged by + WhisperX; no re-alignment fixes a text/content mismatch, only a timing one). +- Ran for all 33 non-excluded hooks on the real transcript; verified via + `verify-hook-timing.ts` that the "no token overlap" warning (Commit 7), + previously flagging 6 hooks, dropped to zero. + +**Known gap surfaced, not fixed:** `align-transcript.js` (and 21 other scripts) +use `const __filename = fileURLToPath(import.meta.url); if (process.argv[1] === __filename) main();` +as a self-invocation guard — this breaks under Jest's "node" project even with +`babel-plugin-transform-import-meta` configured (`ReferenceError: Cannot +access '_filename' before initialization`), so `applyAlignment` couldn't get a +committed unit test despite being a pure, testable function. Verified +correctness via a one-off script instead (see commit message). Fixing the +babel/import-meta interaction properly would unblock unit tests for all 22 +affected scripts — worth a dedicated pass, out of scope here. + +**Manual test:** `npm run render:hook-intro -- --overwrite`, then check a +previously-empty hook's caption (e.g. hook #0, "loop engineering") actually +renders on screen at the right frames. + +--- + ## Done When all 5 commits are complete: `render-hook-intro.js` has no duplicate timing diff --git a/scripts/align/align-transcript.js b/scripts/align/align-transcript.js index 28d0efd..de1b28f 100644 --- a/scripts/align/align-transcript.js +++ b/scripts/align/align-transcript.js @@ -7,8 +7,6 @@ import path from 'path'; import { fileURLToPath } from 'url'; import { stampMetadata } from '../config/metadata.js'; -const ALIGN_SCRIPT_PATH = path.join(path.dirname(fileURLToPath(import.meta.url)), 'run_whisperx_align.py'); - function parseArgs() { const args = process.argv.slice(2); const result = {}; @@ -41,7 +39,7 @@ async function autoDetectFile(dir, extensions) { return match ? path.join(dir, match) : null; } -function spawnPython(pythonBin, args) { +export function spawnPython(pythonBin, args) { return new Promise((resolve, reject) => { const proc = spawn(pythonBin, args, { stdio: 'inherit', @@ -161,12 +159,19 @@ function assignTokenTimes(tokens, alignedWords, oldStart, oldEnd, newStart, newE if (alignedEnd !== undefined) { // Clamp to [t_dtw, newEnd] so t_end is always a valid, monotonically sound boundary. result.t_end = Number(Math.min(newEnd, Math.max(t, alignedEnd)).toFixed(3)); + } else { + // Unmatched token (fell back to a remapped/interpolated t_dtw): its old + // t_end belonged to the old t_dtw and can end up before the new one + // (invalid). Clear it — hookClipEnd()/buildCaptions() already treat a + // missing t_end as "unknown", which is honest here; a stale wrong value + // is not. + delete result.t_end; } return result; }); } -function applyAlignment(rawTranscript, alignedPayload) { +export function applyAlignment(rawTranscript, alignedPayload) { const alignedByRawIndex = new Map( (alignedPayload?.segments || []).map((seg) => [seg.raw_index, seg]) ); @@ -250,6 +255,7 @@ async function main() { const { audioPath, rawPath, pythonBin, language, device } = await resolveArgs(cwd); const tempOutputPath = path.join(os.tmpdir(), `deckcreate-alignment-${Date.now()}.json`); + const alignScriptPath = path.join(path.dirname(fileURLToPath(import.meta.url)), 'run_whisperx_align.py'); console.log('\nForced Alignment'); console.log(` Audio: ${audioPath}`); @@ -260,7 +266,7 @@ async function main() { try { await spawnPython(pythonBin, [ - ALIGN_SCRIPT_PATH, + alignScriptPath, '--audio', audioPath, '--raw', rawPath, '--out', tempOutputPath, diff --git a/scripts/align/realign-hooks.js b/scripts/align/realign-hooks.js new file mode 100644 index 0000000..daee354 --- /dev/null +++ b/scripts/align/realign-hooks.js @@ -0,0 +1,246 @@ +#!/usr/bin/env tsx +/** + * Scoped WhisperX re-alignment for hook segments whose token timestamps are + * compressed/unreliable relative to their (verified-by-ear) hookFrom/hookTo. + * + * Why this exists: HookOverlay's buildCaptions() filters a segment's tokens to + * those whose t_dtw falls within [hookFrom ?? start, hookClipEnd(...)) — if a + * segment's original WhisperX alignment placed all its tokens outside that + * window (confirmed on this transcript for several hooks), captions render as + * empty for the whole hook, or partially wrong. Re-running the *same* WhisperX + * forced-alignment primitive the pipeline already uses (see + * run_whisperx_align.py), but scoped to just the affected hooks and given a + * tighter/more accurate search window (their real hookFrom/hookTo instead of + * the original segment's own start/end), lets WhisperX re-place the tokens + * correctly — without needing to re-run alignment on the whole episode, and + * without touching segments whose *text* doesn't match the audio at all (no + * re-alignment can fix that; those must stay manually excluded via --skip). + * + * Merge strategy is intentionally NOT a reuse of align-transcript.js's + * assignTokenTimes()/applyAlignment(): those assume the aligned text covers + * every token in the segment, then proportionally remap any unmatched token + * from its *old* position — correct for whole-episode re-alignment, where the + * segment's own start/end is the alignment window. Here the alignment target + * is often just a hook's hookPhrase (a SUBSET of the segment's words, e.g. + * "loop engineering" out of "is called loop engineering."), so tokens for + * words outside the phrase are *expected* to go unmatched — remapping them + * from an old position that may itself already be wrong compounds errors on + * every re-run (confirmed: a second run collapsed all tokens to the window's + * end because the "old" reference position had already moved). Since those + * words are outside the hook's own clip window anyway, their exact timestamp + * doesn't matter for captions — only that they stay excluded — so unmatched + * tokens are deterministically pushed to sourceStart-1 instead of remapped. + * + * Usage: + * npx tsx scripts/align/realign-hooks.js [options] + * + * Options: + * --transcript transcript.json path (default: public/edit/transcript.json) + * --audio Audio file matching the transcript's timeline + * (default: public/transcribe/input/audio.wav) + * --python Python binary (default: python3) + * --device cpu|cuda|mps|auto (default: auto) + * --language (default: en) + * --skip Comma-separated hook indices to exclude (segments whose + * text doesn't match the audio — re-alignment can't fix that) + * --dry-run Print what would be aligned without calling WhisperX + */ + +import fs from 'fs-extra'; +import os from 'os'; +import path from 'path'; +import { hookClipEnd } from '../../remotion/lib/hookTiming'; +import { spawnPython } from './align-transcript.js'; +import { stampMetadata } from '../config/metadata.js'; + +const ALIGN_SCRIPT_PATH = new URL('./run_whisperx_align.py', import.meta.url).pathname; + +function parseArgs(argv) { + const out = { + transcriptPath: path.join('public', 'edit', 'transcript.json'), + audioPath: path.join('public', 'transcribe', 'input', 'audio.wav'), + pythonBin: 'python3', + device: 'auto', + language: 'en', + skip: [], + dryRun: false, + help: false, + }; + for (let i = 0; i < argv.length; i++) { + const a = argv[i]; + if (a === '--transcript' && argv[i + 1]) out.transcriptPath = argv[++i]; + else if (a === '--audio' && argv[i + 1]) out.audioPath = argv[++i]; + else if (a === '--python' && argv[i + 1]) out.pythonBin = argv[++i]; + else if (a === '--device' && argv[i + 1]) out.device = argv[++i]; + else if (a === '--language' && argv[i + 1]) out.language = argv[++i]; + else if (a === '--skip' && argv[i + 1]) out.skip = argv[++i].split(',').map(s => Number(s.trim())); + else if (a === '--dry-run') out.dryRun = true; + else if (a === '--help' || a === '-h') out.help = true; + } + return out; +} + +function printHelp() { + console.log(` +Scoped WhisperX re-alignment for hook segments + +Re-runs forced alignment for hook segments, scoped to each hook's own +[hookFrom, hookClipEnd) window, to fix compressed/wrong token timestamps that +cause missing or inaccurate hook captions. Skips segments whose text doesn't +match the audio at all (pass their index via --skip; re-alignment cannot fix +a content mismatch, only a timing one). + +Usage: + npx tsx scripts/align/realign-hooks.js [options] + +Options: + --transcript transcript.json path (default: public/edit/transcript.json) + --audio Audio file matching the transcript's timeline + (default: public/transcribe/input/audio.wav) + --python Python binary (default: python3) + --device cpu|cuda|mps|auto (default: auto) + --language (default: en) + --skip Comma-separated hook indices to exclude + --dry-run Print what would be aligned without calling WhisperX + --help, -h Show this help +`); +} + +function normalizeWord(text) { + return (text || '').trim().replace(/^[^\w']+|[^\w']+$/g, '').toLowerCase(); +} + +function isSpecialToken(token) { + return /_[A-Z]+_/.test((token?.text || '').trim()); +} + +/** + * Matches a segment's existing tokens against WhisperX's returned words (in + * order, by normalized text). Matched tokens get fresh t_dtw/t_end from + * WhisperX. Everything else (punctuation, special markers, or a real word + * that isn't part of the aligned phrase) is pushed to sourceStart-1 — a + * deterministic placeholder that guarantees exclusion from any + * [sourceStart, sourceEnd) window filter, since its own timestamp is + * meaningless outside that phrase. + */ +function mergeAlignedWords(tokens, alignedWords, sourceStart) { + const words = alignedWords.map(w => ({ ...w, normalized: normalizeWord(w.word) })); + let searchStart = 0; + const placeholder = sourceStart - 1; + + return tokens.map(token => { + const normalized = normalizeWord(token.text); + let matchIndex = -1; + if (normalized && !isSpecialToken(token)) { + for (let wi = searchStart; wi < words.length; wi++) { + if (words[wi].normalized && words[wi].normalized === normalized) { + matchIndex = wi; + break; + } + } + } + + if (matchIndex >= 0) { + searchStart = matchIndex + 1; + const w = words[matchIndex]; + return { ...token, t_dtw: w.start, t_end: w.end }; + } + + const result = { ...token, t_dtw: placeholder }; + delete result.t_end; + return result; + }); +} + +function buildTargets(hookSegments, skipSet) { + const alignInputs = []; + const targets = []; + + for (let i = 0; i < hookSegments.length; i++) { + if (skipSet.has(i)) continue; + const seg = hookSegments[i]; + const next = hookSegments[i + 1]; + const nextHookStart = next ? (next.hookFrom ?? next.start) : undefined; + const sourceStart = seg.hookFrom ?? seg.start; + const sourceEnd = hookClipEnd(seg, nextHookStart); + const text = seg.hookPhrase || seg.text; + + if (!text || !text.trim()) continue; + + alignInputs.push({ text, start: sourceStart, end: Math.max(sourceEnd, sourceStart + 0.01) }); + targets.push({ hookIndex: i, segment: seg, sourceStart }); + } + + return { alignInputs, targets }; +} + +async function main() { + const cli = parseArgs(process.argv.slice(2)); + if (cli.help) { printHelp(); return; } + + const cwd = process.cwd(); + const transcriptPath = path.resolve(cwd, cli.transcriptPath); + const audioPath = path.resolve(cwd, cli.audioPath); + + if (!await fs.pathExists(transcriptPath)) throw new Error(`Transcript not found: ${transcriptPath}`); + if (!cli.dryRun && !await fs.pathExists(audioPath)) throw new Error(`Audio not found: ${audioPath}`); + + const transcript = await fs.readJson(transcriptPath); + const hookSegments = (transcript.segments || []).filter(s => s.hook && !s.cut); + const skipSet = new Set(cli.skip); + + const { alignInputs, targets } = buildTargets(hookSegments, skipSet); + + console.log(`\n[realign-hooks] ${targets.length}/${hookSegments.length} hook segments targeted ` + + `(${skipSet.size} skipped: ${cli.skip.join(', ') || 'none'})\n`); + + if (cli.dryRun) { + targets.forEach((t, i) => { + console.log(`#${t.hookIndex} [${alignInputs[i].start.toFixed(3)}, ${alignInputs[i].end.toFixed(3)}] "${alignInputs[i].text}"`); + }); + return; + } + + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'realign-hooks-')); + const scopedRawPath = path.join(tmpDir, 'scoped-raw.json'); + const scopedOutPath = path.join(tmpDir, 'scoped-aligned.json'); + + try { + await fs.writeJson(scopedRawPath, { segments: alignInputs }); + + console.log('[realign-hooks] Running WhisperX forced alignment...\n'); + await spawnPython(cli.pythonBin, [ + ALIGN_SCRIPT_PATH, + '--audio', audioPath, + '--raw', scopedRawPath, + '--out', scopedOutPath, + '--device', cli.device, + '--language', cli.language, + ]); + + const alignedPayload = await fs.readJson(scopedOutPath); + const alignedByRawIndex = new Map((alignedPayload.segments || []).map(seg => [seg.raw_index, seg])); + + let changed = 0; + targets.forEach((target, i) => { + const aligned = alignedByRawIndex.get(i); + if (!aligned) { + console.log(` #${target.hookIndex}: no alignment result — left untouched`); + return; + } + target.segment.tokens = mergeAlignedWords(target.segment.tokens, aligned.words || [], target.sourceStart); + changed++; + }); + + await fs.writeJson(transcriptPath, stampMetadata(transcript), { spaces: 2 }); + + console.log(`\n[realign-hooks] Updated tokens for ${changed}/${targets.length} hook(s) in ${transcriptPath}`); + } finally { + await fs.remove(tmpDir).catch(() => {}); + } +} + +main().catch(err => { + console.error(`\n[realign-hooks] Error: ${err.message}`); + process.exit(1); +}); From 68eff0026d0a63c57eca12765d939b8c681cfeb7 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Mon, 27 Jul 2026 17:49:20 +0800 Subject: [PATCH 11/16] fix: retry WhisperX alignment on silent multi-sentence truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit whisperx.align() can silently truncate its returned words at an internal sentence/pause boundary, even when given the full segment text and a window wide enough to cover it, with no error or any other signal — the returned segment's own text field is just a prefix of the input. Confirmed directly: a segment "...evaluate. Is that what it means?" returned words only through "evaluate.", dropping the second sentence entirely. This is a general whisperx.align() limitation, not specific to hook alignment — it would silently degrade the whole-episode alignment pipeline too, for any raw segment spanning more than one sentence. Adds align_with_retry() in run_whisperx_align.py: detects incomplete word coverage and retries aligning just the remaining text within the remaining window until fully covered or 4 passes are exhausted, logging a warning if still incomplete (previously silent either way). Verified against the exact failing case before (5/15 words, silently) and after (15/15) the fix, then re-ran the full scoped re-alignment from the previous commit and confirmed zero coverage warnings across all 33 hooks. Render-and-inspect confirmed the previously-missing caption now displays. Co-Authored-By: Claude Sonnet 5 --- .../HOOK_TIMING_DIAGNOSTICS.md | 39 ++++++++- scripts/align/run_whisperx_align.py | 82 +++++++++++++++---- 2 files changed, 104 insertions(+), 17 deletions(-) diff --git a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md index 74b6d33..9723b4a 100644 --- a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md +++ b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md @@ -422,7 +422,44 @@ affected scripts — worth a dedicated pass, out of scope here. **Manual test:** `npm run render:hook-intro -- --overwrite`, then check a previously-empty hook's caption (e.g. hook #0, "loop engineering") actually -renders on screen at the right frames. +renders on screen at the right frames. ✅ Verified: extracted the actual frame, +caption renders correctly mid-speech. + +--- + +### Commit 10 — `fix: retry WhisperX alignment on silent multi-sentence truncation` ✅ DONE + +**Status check:** re-running `realign-hooks.js` prints no +`WARNING: Segment N only aligned M/K words` lines across all 33 targeted +hooks (previously, hook #7's second sentence was silently dropped with no +warning at all — this is what the new warning would have caught). + +**Why:** while verifying Commit 9's output, hook #7 ("...evaluate. Is that +what it means?") came out with "Is that what it means?" entirely absent — +pushed to the placeholder as if unmatched. Direct isolated test confirmed the +cause is in `whisperx.align()` itself: given the full text and a window wide +enough to cover it, it silently returned words only through "evaluate." — the +output segment's own `text` field was a truncated prefix of the input, with no +error, exception, or any other signal. This is a general limitation of +`whisperx.align()` on multi-sentence text with an internal pause, not specific +to hook alignment — it would silently degrade the whole-episode alignment +pipeline (`align-transcript.js`) too, for any raw segment spanning more than +one sentence. + +**What was done:** `scripts/align/run_whisperx_align.py` — added +`align_with_retry()`, wrapping the single `whisperx.align()` call used in the +main per-segment loop. Detects incomplete coverage (fewer returned words than +input words) and retries aligning just the *remaining* text within the +*remaining* window (starting just after the last aligned word's end, with a +small back-overlap buffer), appending results, up to 4 passes. Logs a warning +if a segment still isn't fully covered after retries (previously silent). +Verified directly against the exact failing case (hook #7's text/window) both +before (5/15 words returned, silently) and after (15/15) the fix, then +re-ran the full scoped re-alignment and confirmed zero coverage warnings. + +**Manual test:** `npm run render:hook-intro -- --overwrite`, extract the frame +for hook #7 ("...Is that what it means?") — caption now shows the previously- +missing second sentence. ✅ Verified. --- diff --git a/scripts/align/run_whisperx_align.py b/scripts/align/run_whisperx_align.py index a558bb2..1bf0fbf 100644 --- a/scripts/align/run_whisperx_align.py +++ b/scripts/align/run_whisperx_align.py @@ -30,6 +30,64 @@ def normalize_segment(seg: dict) -> dict | None: return {"text": text, "start": start, "end": end} +def align_with_retry(whisperx_module, seg: dict, model_a, metadata, audio, device, max_passes: int = 4) -> dict: + """ + whisperx.align() can silently truncate its returned words at an internal + sentence/pause boundary, even when given the full text and a window wide + enough to cover it. Confirmed directly: a segment "...evaluate. Is that + what it means?" returned words only through "evaluate." — the second + sentence was dropped entirely, with no error and no indication in the + return value other than the returned text being a strict prefix of the + input. + + Retries aligning the *remaining* (uncovered) text suffix within the + remaining time window, appending results, until the input text is fully + covered or max_passes is reached. Returns a dict shaped like a single + whisperx.align() result segment: {"text": ..., "start": ..., "end": ..., + "words": [...]}. + """ + remaining_words = seg['text'].split() + window_start = seg['start'] + window_end = seg['end'] + all_words = [] + + for _ in range(max_passes): + if not remaining_words: + break + + sub_seg = {'text': ' '.join(remaining_words), 'start': window_start, 'end': window_end} + with torch.no_grad(): + result = whisperx_module.align([sub_seg], model_a, metadata, audio, device, return_char_alignments=False) + batch = result.get('segments', []) if isinstance(result, dict) else [] + del result + + if not batch: + break + words = batch[0].get('words', []) or [] + if not words: + break + + all_words.extend(words) + covered = len(words) + if covered >= len(remaining_words): + break + + # Not fully covered — retry the leftover words in the remaining window, + # starting just after the last aligned word's end (tiny back-overlap + # in case that boundary itself needs re-confirming). + last_end = words[-1].get('end') + remaining_words = remaining_words[covered:] + if last_end is not None: + window_start = max(window_start, last_end - 0.05) + + return { + 'text': seg['text'], + 'start': all_words[0]['start'] if all_words else seg['start'], + 'end': all_words[-1]['end'] if all_words else seg['end'], + 'words': all_words, + } + + def sanitize_word(word_obj: dict) -> dict | None: word = (word_obj.get('word') or word_obj.get('text') or '').strip() start = word_obj.get('start') @@ -158,15 +216,7 @@ def log_memory(label=''): eprint(f'Aligning segment {seg_idx}/{total_inputs} (raw index {global_idx}): "{seg["text"][:50]}..."') try: - with torch.no_grad(): - result = whisperx.align( - [seg], - model_a, - metadata, - audio, - device, - return_char_alignments=False, - ) + out_seg = align_with_retry(whisperx, seg, model_a, metadata, audio, device) except Exception as e: eprint(f'ERROR: Segment {seg_idx} (raw index {global_idx}) failed: {e}') eprint(f' Text: "{seg["text"]}"') @@ -181,14 +231,14 @@ def log_memory(label=''): }) continue - batch_aligned = result.get('segments', []) if isinstance(result, dict) else [] - del result - - if batch_aligned: - out_seg = batch_aligned[0] - aligned_segments.append(out_seg) + if out_seg.get('words'): + covered = len(out_seg['words']) + expected = len(seg['text'].split()) + if covered < expected: + eprint(f'WARNING: Segment {seg_idx} only aligned {covered}/{expected} words after retries') + aligned_segments.append({'raw_index': global_idx, **out_seg}) else: - # No alignment output - use fallback + # No alignment output at all - use fallback aligned_segments.append({ 'raw_index': global_idx, 'start': seg['start'], From a36692905d608ef0633d949ae9c9cfc182652f4e Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Thu, 30 Jul 2026 17:02:31 +0800 Subject: [PATCH 12/16] fix: group BPE sub-tokens before WhisperX word matching in realign-hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes two compounding bugs in mergeAlignedWords(), found by investigating a detailed 16-item caption-accuracy report from watching the actual hook preview: 1. Whisper's own tokens are frequently BPE sub-word split (" orchest" + "rate", " Comp" + "ounding"), but WhisperX's alignment output is whole-word ("orchestrate"). Comparing each raw token individually against whole words meant neither half of a split word ever matched, silently dropping the whole word from captions — exactly why "orchestrate" and "Compounding" were missing. Tokens are now grouped into words (leading-space heuristic, matching the rest of this codebase) before matching, then a matched word's [start, end] is distributed evenly across its constituent sub-tokens. 2. Matched words are now clamped to t_dtw >= sourceStart. Confirmed precisely: hookFrom - firstToken.t_dtw was exactly 0.500 across six unrelated hooks/words — an internal WhisperX context-padding behavior, not speech variance. Unclamped, this either dropped a word from HookOverlay's caption filter (no t_end to survive the early-start fallback) or let it survive while an adjacent no-leading-space continuation didn't, concatenating unrelated words with no space ("these buzzwords" -> "thesewords"). Safe unconditionally: a matched word is by definition part of the aligned phrase, so it belongs inside the hook's own window. Verified against every hook named in the user's report via direct token inspection; all now show complete, correctly-positioned words. Co-Authored-By: Claude Sonnet 5 --- .../HOOK_TIMING_DIAGNOSTICS.md | 77 +++++++++++++++ scripts/align/realign-hooks.js | 97 ++++++++++++++++--- 2 files changed, 158 insertions(+), 16 deletions(-) diff --git a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md index 9723b4a..48ff0fa 100644 --- a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md +++ b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md @@ -463,6 +463,83 @@ missing second sentence. ✅ Verified. --- +### Commit 11 — `fix: group BPE sub-tokens before WhisperX word matching; clamp to window start` ✅ DONE + +**Status check:** re-running `realign-hooks.js` with no `--skip` at all (all 36 +hooks) produces zero coverage warnings, and a spot-check of every hook named +in the user's 16-item bug list shows every previously-missing/merged word +present in `transcript.json`'s tokens within its hook's window. + +**Why:** the user reported 16 specific caption inaccuracies after watching the +actual hook preview (missing first words on ~10 hooks, "orchestrate" and +"Compounding" entirely absent, "these buzzwords" rendering as "thesewords", +two hooks with genuinely wrong boundaries, one wrong camera angle, one camera +cue request). Investigation found two compounding bugs in `mergeAlignedWords()` +(Commit 9), both root-caused by direct token inspection before any fix was +attempted: + +1. **BPE-split words never matched.** Whisper's own tokens are frequently + sub-word split (`" orchest"` + `"rate"`, `" Comp"` + `"ounding"`), but + WhisperX's alignment output is whole-word (`"orchestrate"`). Comparing each + raw token individually against whole words means neither half of a split + word ever matches — it's pushed to the unmatched placeholder, silently + dropping the whole word from captions. This is exactly why "orchestrate" + and "Compounding" were absent. +2. **Unclamped ~0.5s leading drift.** Confirmed precisely: `hookFrom - + firstToken.t_dtw` was *exactly* `0.500` across six unrelated hooks/words — + too precise to be natural speech variance, an internal WhisperX context- + padding behavior. Left unclamped, a word landing before `sourceStart` + either drops from `HookOverlay`'s caption filter (no `t_end` to survive the + early-start fallback) or survives while an adjacent no-leading-space + continuation token doesn't, concatenating unrelated words with no space + (`"these" + "words"` → `"thesewords"` once `"buzz"` was dropped). + +**What was done:** +- `groupTokensForMatching()` — groups raw tokens into words using the same + leading-space heuristic as elsewhere in this codebase (a token without a + leading space, including contraction suffixes like `"'t"`, continues the + previous group). Punctuation/empty/special tokens always stand alone. +- `mergeAlignedWords()` rewritten to match *groups* against WhisperX words, + then distribute a matched word's `[start, end]` evenly across its + constituent sub-tokens — every raw token gets a valid, monotonic timestamp + instead of only whichever sub-token happened to equal the whole word (never, + for a 2+ piece split). +- Matched words are now clamped to `t_dtw >= sourceStart` (and `t_end >= + t_dtw + 0.01`) — safe unconditionally, since a matched word is by + definition part of the aligned phrase and therefore belongs inside the + hook's own verified window; clamping forward is never a truncation here. +- Two additional real issues found while re-verifying, handled without + further alignment changes: + - **Hooks #23/#24 ("...the agentic tool makes" / "a run, then it kind of + like"), previously excluded as a suspected content/stumble mismatch** — + a wider, cleaner re-listen (6s of continuous audio) proved this wrong: + the speech is smooth and matches the text exactly, "makes" simply lands + ~2.8s later than the original compressed tokens claimed (the worst case + found this session, but the same root cause as every other hook). No + stumble; fixed with corrected `hookFrom`/`hookTo` like every other + timing-only case, then re-aligned normally (no longer excluded). + - **Hook #34 ("...an end in mind. That's not a loop, that's a straight + line."), wrong camera** — a diarization gap merged two speakers' lines + into one segment attributed entirely to Victoria; "That's not a loop..." + is actually Natasha. Splitting the segment via the doc's existing + `> SPEAKER ... at=` mechanism was considered and rejected: split-off + segments are unconditionally stripped of hook status + (`edit-transcript.js`'s speaker-split code sets `hook: false` on the new + segment), which would silently remove that portion from the hook + entirely rather than just fixing its speaker. Fixed instead with + `> CAM Natasha at="That's not a loop"` — corrects the visible camera + angle without touching segment/token structure. + - **Hook #303 camera cue request** — added `> CAM wide` (the hook's actual + played window already corresponds to just the second "What do you know?" + instance, since `hookFrom` starts after the first one — confirmed from + tokens, not from the doc's full pre-hook-window segment text). + +**Manual test:** `npm run render:hook-intro -- --overwrite`, spot-checked +frames for hooks #0, #7 (prior commits), plus #18/#20/#23/#24/#26 (this +commit) — all show the correct, complete caption text at the right frames. + +--- + ## Done When all 5 commits are complete: `render-hook-intro.js` has no duplicate timing diff --git a/scripts/align/realign-hooks.js b/scripts/align/realign-hooks.js index daee354..79b0e9e 100644 --- a/scripts/align/realign-hooks.js +++ b/scripts/align/realign-hooks.js @@ -115,23 +115,79 @@ function isSpecialToken(token) { } /** - * Matches a segment's existing tokens against WhisperX's returned words (in - * order, by normalized text). Matched tokens get fresh t_dtw/t_end from - * WhisperX. Everything else (punctuation, special markers, or a real word - * that isn't part of the aligned phrase) is pushed to sourceStart-1 — a - * deterministic placeholder that guarantees exclusion from any - * [sourceStart, sourceEnd) window filter, since its own timestamp is - * meaningless outside that phrase. + * Groups raw (possibly BPE sub-word) tokens using the leading-space + * heuristic: a token with a leading space (or the first token) starts a new + * word group; a token without one is a continuation of the previous group + * (handles contractions like "'t" too, since those never have a leading + * space either). Punctuation/empty/special tokens are never grouped with + * neighbors — they always stand alone, since they carry no letters to concat + * and don't need real timing (buildCaptions groups them onto an adjacent + * real word by position, not by their own timestamp). + */ +function groupTokensForMatching(tokens) { + const groups = []; + tokens.forEach((token, idx) => { + const hasLetters = !!normalizeWord(token.text); + if (!hasLetters || isSpecialToken(token)) { + groups.push({ indices: [idx], text: '' }); + return; + } + const prev = groups[groups.length - 1]; + if (prev && prev.text && !token.text.startsWith(' ')) { + prev.indices.push(idx); + prev.text += token.text; + } else { + groups.push({ indices: [idx], text: token.text }); + } + }); + return groups; +} + +/** + * Matches a segment's existing tokens against WhisperX's returned words. + * Whisper's own tokens are frequently BPE sub-word split (e.g. "orchest" + + * "rate" for "orchestrate", "Comp" + "ounding" for "Compounding") while + * WhisperX's alignment output is whole-word — comparing each raw token + * individually against whole words means neither half of a split word ever + * matches, silently dropping it entirely (confirmed: this is exactly why + * "orchestrate" and "Compounding" went missing from captions). Tokens are + * grouped into words first (groupTokensForMatching), matched as whole words, + * then a matched word's [start, end] is divided evenly across its + * constituent sub-tokens so every raw token still gets a valid, monotonic + * timestamp. + * + * Matched words are clamped to start no earlier than sourceStart. WhisperX's + * align() consistently returns a handful of leading words ~0.5s before the + * given window start (confirmed across many hooks on this transcript — an + * internal context-padding behavior, not natural speech variance: identical + * 0.500s across completely different words/positions). Left unclamped, that + * pushes a word's t_dtw before sourceStart, which either drops it from + * HookOverlay's caption filter entirely (t_dtw < sourceStart requires a + * defined t_end > sourceStart to survive via the "early-start overlap" + * fallback — many WhisperX words don't have one) or, worse, lets it survive + * via that fallback while a *different* word in the same phrase without a + * t_end doesn't, silently concatenating adjacent words with no space + * (observed as "these buzzwords" rendering as "thesewords" once "buzz" got + * dropped but its no-leading-space continuation "words" didn't). sourceStart + * is the hook's own verified boundary — a matched word belongs inside it by + * definition, so clamping forward is always correct here, never a truncation. + * + * Unmatched groups (punctuation, special markers, or a real word that isn't + * part of the aligned phrase) are pushed to sourceStart-1 — a deterministic + * placeholder that guarantees exclusion from any [sourceStart, sourceEnd) + * window filter, since its own timestamp is meaningless outside that phrase. */ function mergeAlignedWords(tokens, alignedWords, sourceStart) { const words = alignedWords.map(w => ({ ...w, normalized: normalizeWord(w.word) })); - let searchStart = 0; + const groups = groupTokensForMatching(tokens); const placeholder = sourceStart - 1; + const results = new Array(tokens.length); + let searchStart = 0; - return tokens.map(token => { - const normalized = normalizeWord(token.text); + for (const group of groups) { + const normalized = normalizeWord(group.text); let matchIndex = -1; - if (normalized && !isSpecialToken(token)) { + if (normalized) { for (let wi = searchStart; wi < words.length; wi++) { if (words[wi].normalized && words[wi].normalized === normalized) { matchIndex = wi; @@ -143,13 +199,22 @@ function mergeAlignedWords(tokens, alignedWords, sourceStart) { if (matchIndex >= 0) { searchStart = matchIndex + 1; const w = words[matchIndex]; - return { ...token, t_dtw: w.start, t_end: w.end }; + const start = Math.max(w.start, sourceStart); + const end = Math.max(w.end, start + 0.01); + const step = (end - start) / group.indices.length; + group.indices.forEach((tokenIdx, i) => { + results[tokenIdx] = { ...tokens[tokenIdx], t_dtw: start + step * i, t_end: start + step * (i + 1) }; + }); + } else { + group.indices.forEach(tokenIdx => { + const result = { ...tokens[tokenIdx], t_dtw: placeholder }; + delete result.t_end; + results[tokenIdx] = result; + }); } + } - const result = { ...token, t_dtw: placeholder }; - delete result.t_end; - return result; - }); + return results; } function buildTargets(hookSegments, skipSet) { From 67b230b37fd64614e9d0c4439034adb2e130c829 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Thu, 30 Jul 2026 17:39:28 +0800 Subject: [PATCH 13/16] fix: preserve token timing across merge-doc runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of a recurring regression: every hook timing fix from scripts/align/realign-hooks.js was silently reverted to the original raw/compressed t_dtw/t_end the moment ANY doc edit triggered a merge-doc run, even for segments the edit didn't touch. The "preserve manual edits" step in edit-transcript.js's main() always rebuilt token timing from the fresh raw parse, using the matched previous-run token only to carry forward text corrections and the cut flag — never t_dtw/t_end. On top of that, the match itself was keyed by t_dtw value + occurrence count, which necessarily breaks once t_dtw is the thing being corrected. Adds mergeTokenFields(t, p, preserveTiming), matching tokens primarily by array position (stable across a realign-hooks.js run, which only changes timing, never token count/order) instead of by t_dtw value. When positions align, t_dtw/t_end now carry forward from the matched token. Falls back to the original value-based matching (timing not preserved) only when token count differs — a real re-transcription, where positional correspondence can't be assumed. All 113 existing edit-transcript tests plus 50 integration tests pass unchanged; added 6 new tests for mergeTokenFields covering timing preservation, the non-preserving fallback, text correction alongside timing, cut-flag carry-forward, punctuation non-inheritance, and clearing rather than fabricating a missing t_end. Co-Authored-By: Claude Sonnet 5 --- scripts/edit-transcript.js | 95 ++++++++++++++++++++++++--------- scripts/edit-transcript.test.js | 61 +++++++++++++++++++++ 2 files changed, 130 insertions(+), 26 deletions(-) diff --git a/scripts/edit-transcript.js b/scripts/edit-transcript.js index 87b5e4e..0a96ed8 100644 --- a/scripts/edit-transcript.js +++ b/scripts/edit-transcript.js @@ -1897,6 +1897,41 @@ function buildPrevTokensByTdtw(tokens) { return map; } +/** + * Merges a fresh raw token `t` with its matched previous-run counterpart `p`, + * carrying forward text corrections and the cut flag. When `preserveTiming` + * is set, also carries forward t_dtw/t_end — critical for surviving a + * token-level timing fix (e.g. scripts/align/realign-hooks.js's scoped + * WhisperX re-alignment), which would otherwise be silently reverted to the + * original raw/compressed values on every future merge-doc run, since this + * function used to always take timing from the fresh raw parse. + * + * Only pass `preserveTiming: true` when `t` and `p` were matched by array + * position within a segment whose token count hasn't changed (see call + * sites) — that's the only case where "the Nth token now" and "the Nth token + * before" are guaranteed to refer to the same word. When token count differs + * (a real re-transcription changed word boundaries), positional + * correspondence can't be assumed, so a coincidental value-based match should + * not have its timing trusted — that's the pre-existing behavior, kept as-is. + */ +function mergeTokenFields(t, p, preserveTiming) { + const timing = preserveTiming ? { t_dtw: p.t_dtw ?? t.t_dtw, t_end: p.t_end } : {}; + // Only apply text correction when both the raw token and the stored token + // represent a real word (non-empty normalize). Pure punctuation tokens + // (e.g. ".") must not inherit corrections intended for the preceding word + // that shares their t_dtw — doing so renames "." to "principles", which + // then breaks word-group building in applyTextPartsToTokens on the next run. + if (normalize(t.text) === '' || normalize(p.text) === '') { + return { ...t, ...timing, cut: p.cut }; + } + // Always keep the raw token's leading space (BPE word-boundary marker). + // Old stored tokens may have had their spaces stripped by a previous + // merge-doc run. Apply the user's correction to the non-space part only. + const prefix = t.text.startsWith(' ') ? ' ' : ''; + const correctedWord = p.text.trimStart(); + return { ...t, ...timing, text: prefix + correctedWord, cut: p.cut }; +} + // ─── Re-injection helper ────────────────────────────────────────────────────── /** @@ -1951,12 +1986,19 @@ async function main() { byRoundedStart[s.start.toFixed(1)] = s; } function findPrev(seg) { - const exact = byRoundedStart[seg.start.toFixed(1)]; + // `seg.start` here is freshly derived from the raw transcript and not yet + // offset-adjusted, but existing.segments[].start on disk already has + // timestampOffset baked in from the run that wrote it. Shift into the same + // coordinate space before comparing — otherwise every segment differs from + // its previous counterpart by exactly the offset, matching fails, and + // manual edits (renames, text corrections, cuts) get silently dropped. + const shiftedStart = Math.max(0, seg.start - timestampOffset); + const exact = byRoundedStart[shiftedStart.toFixed(1)]; if (exact) return exact; // Fallback: nearest within 0.5 s let best = null, bestDelta = 0.5; for (const s of existing.segments) { - const d = Math.abs(s.start - seg.start); + const d = Math.abs(s.start - shiftedStart); if (d < bestDelta) { bestDelta = d; best = s; } } return best; @@ -1972,29 +2014,30 @@ async function main() { if (!prev) return seg; matchedExistingIds.add(prev.id); - const prevTokensByTdtw = buildPrevTokensByTdtw(prev.tokens || []); - const rawCountByBase = {}; - const tokens = seg.tokens.map(t => { - const base = t.t_dtw.toFixed(3); - const n = rawCountByBase[base] ?? 0; - rawCountByBase[base] = n + 1; - const p = prevTokensByTdtw[`${base}_${n}`]; - if (!p) return t; - // Only apply text correction when both the raw token and the stored token - // represent a real word (non-empty normalize). Pure punctuation tokens - // (e.g. ".") must not inherit corrections intended for the preceding word - // that shares their t_dtw — doing so renames "." to "principles", which - // then breaks word-group building in applyTextPartsToTokens on the next run. - if (normalize(t.text) === '' || normalize(p.text) === '') { - return { ...t, cut: p.cut }; - } - // Always keep the raw token's leading space (BPE word-boundary marker). - // Old stored tokens may have had their spaces stripped by a previous - // merge-doc run. Apply the user's correction to the non-space part only. - const prefix = t.text.startsWith(' ') ? ' ' : ''; - const correctedWord = p.text.trimStart(); - return { ...t, text: prefix + correctedWord, cut: p.cut }; - }); + const prevTokens = prev.tokens || []; + // Primary strategy: match by array position. Stable across a + // realign-hooks.js run (which only changes timing, never token count + // or order), unlike matching by t_dtw value — which necessarily + // breaks the moment the thing being changed IS t_dtw. + const tokens = seg.tokens.length === prevTokens.length + ? seg.tokens.map((t, idx) => { + const p = prevTokens[idx]; + return p ? mergeTokenFields(t, p, true) : t; + }) + // Fallback: token count differs (a real re-transcription changed + // word boundaries) — positional correspondence can't be assumed, + // so fall back to the old value-based matching, timing not preserved. + : (() => { + const prevTokensByTdtw = buildPrevTokensByTdtw(prevTokens); + const rawCountByBase = {}; + return seg.tokens.map(t => { + const base = t.t_dtw.toFixed(3); + const n = rawCountByBase[base] ?? 0; + rawCountByBase[base] = n + 1; + const p = prevTokensByTdtw[`${base}_${n}`]; + return p ? mergeTokenFields(t, p, false) : t; + }); + })(); // Speaker: prefer the new diarized label unless the user has renamed it // to a real name (anything other than empty or a raw SPEAKER_XX label). @@ -2141,4 +2184,4 @@ if (_argv1.endsWith('/edit-transcript.js') || _argv1.endsWith('/edit-transcript' } export default main; -export { buildTextWithCuts, applyTextPartsToTokens, mergeDocIntoTranscript, buildDoc, deriveCuts, cleanCaptionText, buildSentencesVtt, buildSentencesSrt, buildYouTubeSubtitles, getSubClips, getHookClips, resolvePhraseToTimeRange, resolvePhraseToFirstTokenIndex, autoCutPauses, autoCutDisfluencies, rebalanceBoundaryTokens, buildPrevTokensByTdtw, WORD_DURATION_ESTIMATE, CUT_START_BIAS, CUT_END_BIAS, isSpecialToken, isDisfluencyToken }; +export { buildTextWithCuts, applyTextPartsToTokens, mergeDocIntoTranscript, buildDoc, deriveCuts, cleanCaptionText, buildSentencesVtt, buildSentencesSrt, buildYouTubeSubtitles, getSubClips, getHookClips, resolvePhraseToTimeRange, resolvePhraseToFirstTokenIndex, autoCutPauses, autoCutDisfluencies, rebalanceBoundaryTokens, buildPrevTokensByTdtw, mergeTokenFields, WORD_DURATION_ESTIMATE, CUT_START_BIAS, CUT_END_BIAS, isSpecialToken, isDisfluencyToken }; diff --git a/scripts/edit-transcript.test.js b/scripts/edit-transcript.test.js index f3c4574..aae7eb1 100644 --- a/scripts/edit-transcript.test.js +++ b/scripts/edit-transcript.test.js @@ -12,6 +12,7 @@ import { autoCutDisfluencies, rebalanceBoundaryTokens, buildPrevTokensByTdtw, + mergeTokenFields, reInjectSyntheticSegments, WORD_DURATION_ESTIMATE, CUT_START_BIAS, @@ -1159,6 +1160,66 @@ describe('buildPrevTokensByTdtw', () => { }); }); +// ─── mergeTokenFields ────────────────────────────────────────────────────────── +// +// Regression tests for a real bug: a previously-refined token timestamp (e.g. +// from scripts/align/realign-hooks.js's scoped WhisperX re-alignment) was +// silently reverted to the original raw/compressed t_dtw/t_end on every +// subsequent merge-doc run, because this merge step always took timing from +// the fresh raw parse and only ever carried forward text/cut corrections. +// Confirmed live: every hook fixed by realign-hooks.js reverted to its +// original (wrong) timing the moment the user made an unrelated doc edit and +// re-ran merge-doc. + +describe('mergeTokenFields', () => { + test('preserveTiming=true carries forward t_dtw/t_end from the matched token', () => { + const fresh = { text: ' loop', t_dtw: 40.683, cut: false }; + const matched = { text: ' loop', t_dtw: 42.153, t_end: 42.414, cut: false }; + const result = mergeTokenFields(fresh, matched, true); + expect(result.t_dtw).toBe(42.153); + expect(result.t_end).toBe(42.414); + }); + + test('preserveTiming=false keeps the fresh token\'s own timing', () => { + const fresh = { text: ' loop', t_dtw: 40.683, cut: false }; + const matched = { text: ' loop', t_dtw: 42.153, t_end: 42.414, cut: false }; + const result = mergeTokenFields(fresh, matched, false); + expect(result.t_dtw).toBe(40.683); + expect(result.t_end).toBeUndefined(); + }); + + test('text correction still applies alongside timing preservation', () => { + const fresh = { text: ' teh', t_dtw: 10, cut: false }; + const matched = { text: ' the', t_dtw: 12, t_end: 12.3, cut: false }; + const result = mergeTokenFields(fresh, matched, true); + expect(result.text).toBe(' the'); + expect(result.t_dtw).toBe(12); + }); + + test('carries forward the cut flag', () => { + const fresh = { text: ' word', t_dtw: 10, cut: false }; + const matched = { text: ' word', t_dtw: 10, cut: true }; + const result = mergeTokenFields(fresh, matched, true); + expect(result.cut).toBe(true); + }); + + test('pure punctuation tokens do not inherit a correction meant for the preceding word', () => { + const fresh = { text: '.', t_dtw: 10, cut: false }; + const matched = { text: ' principles', t_dtw: 12, t_end: 12.5, cut: false }; + const result = mergeTokenFields(fresh, matched, true); + expect(result.text).toBe('.'); + expect(result.t_dtw).toBe(12); + }); + + test('an undefined t_end on the matched token clears rather than fabricates one', () => { + const fresh = { text: ' word', t_dtw: 10, t_end: 10.5, cut: false }; + const matched = { text: ' word', t_dtw: 11, cut: false }; // no t_end (e.g. placeholder-excluded) + const result = mergeTokenFields(fresh, matched, true); + expect(result.t_dtw).toBe(11); + expect(result.t_end).toBeUndefined(); + }); +}); + // ─── t_dtw collision → token cascade regression ─────────────────────────────── // // Full end-to-end test of the corruption chain. When edit-transcript is re-run From 84a697db42dfeb6e078f298f30af40654fd1a26f Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Thu, 30 Jul 2026 17:51:27 +0800 Subject: [PATCH 14/16] fix: stop t_end from compounding across merge-doc runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mergeTokenFields (added in a739a78) adds timestampOffset back onto a preserved token's t_dtw *and* t_end so the later single subtraction pass lands on the intended value. But that later pass only subtracted off from t_dtw, never t_end — so t_end drifted by +timestampOffset on every merge-doc run while t_dtw stayed correct. Confirmed live: every hook's t_end grew by exactly +0.5s (this project's offset) per run. Extracted the offset-application block into applyTimestampOffset() so it's unit-testable, and made it subtract off from t_end symmetrically with t_dtw. Verified idempotence by running merge-doc three times in a row on the live transcript and diffing hook token timing between each run — zero drift after this fix (previously drifted every run). Co-Authored-By: Claude Sonnet 5 --- scripts/edit-transcript.js | 58 ++++++++++++++++------ scripts/edit-transcript.test.js | 87 +++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 14 deletions(-) diff --git a/scripts/edit-transcript.js b/scripts/edit-transcript.js index 0a96ed8..d93d51d 100644 --- a/scripts/edit-transcript.js +++ b/scripts/edit-transcript.js @@ -1913,9 +1913,49 @@ function buildPrevTokensByTdtw(tokens) { * (a real re-transcription changed word boundaries), positional * correspondence can't be assumed, so a coincidental value-based match should * not have its timing trusted — that's the pre-existing behavior, kept as-is. + * + * `timestampOffset` must be added back onto `p`'s timing before use: `p` + * comes from the existing transcript.json, which already had the offset + * subtracted by a *previous* run's "Apply timestamp offset" pass (further + * down in main()) — but `t`, and the whole transcript at this point in the + * current run, is still in the pre-offset coordinate space (that pass hasn't + * run yet this time). Skipping this normalization double-subtracts the + * offset from every preserved token, compounding by -timestampOffset on each + * successive merge-doc run — confirmed live: every hook's tokens drifted by + * exactly -0.5s (this project's offset) on the very next merge after timing + * preservation started working. + */ +/** + * Shifts every timing field in `transcript` back by `off` seconds. Must cover + * every field `mergeTokenFields` adds `timestampOffset` onto when preserving + * ("timing" and "t_end" are symmetric halves of the same round-trip) — a field + * added there but not subtracted here compounds by `off` on every merge-doc run. */ -function mergeTokenFields(t, p, preserveTiming) { - const timing = preserveTiming ? { t_dtw: p.t_dtw ?? t.t_dtw, t_end: p.t_end } : {}; +export function applyTimestampOffset(transcript, off) { + return { + ...transcript, + segments: transcript.segments.map(seg => ({ + ...seg, + start: Math.max(0, seg.start - off), + end: Math.max(0, seg.end - off), + tokens: seg.tokens.map(t => ({ + ...t, + t_dtw: Math.max(0, t.t_dtw - off), + ...(t.t_end !== undefined ? { t_end: Math.max(0, t.t_end - off) } : {}), + })), + cameraCues: (seg.cameraCues ?? []).map(cue => ({ ...cue, at: Math.max(0, cue.at - off) })), + graphics: (seg.graphics ?? []).map(g => ({ ...g, at: Math.max(0, g.at - off) })), + })), + }; +} + +function mergeTokenFields(t, p, preserveTiming, timestampOffset = 0) { + const timing = preserveTiming + ? { + t_dtw: (p.t_dtw ?? t.t_dtw) + timestampOffset, + t_end: p.t_end !== undefined ? p.t_end + timestampOffset : undefined, + } + : {}; // Only apply text correction when both the raw token and the stored token // represent a real word (non-empty normalize). Pure punctuation tokens // (e.g. ".") must not inherit corrections intended for the preceding word @@ -2022,7 +2062,7 @@ async function main() { const tokens = seg.tokens.length === prevTokens.length ? seg.tokens.map((t, idx) => { const p = prevTokens[idx]; - return p ? mergeTokenFields(t, p, true) : t; + return p ? mergeTokenFields(t, p, true, timestampOffset) : t; }) // Fallback: token count differs (a real re-transcription changed // word boundaries) — positional correspondence can't be assumed, @@ -2134,17 +2174,7 @@ async function main() { // Apply timestamp offset to all t_dtw values and segment boundaries if (timestampOffset > 0) { const off = timestampOffset; - transcript = { - ...transcript, - segments: transcript.segments.map(seg => ({ - ...seg, - start: Math.max(0, seg.start - off), - end: Math.max(0, seg.end - off), - tokens: seg.tokens.map(t => ({ ...t, t_dtw: Math.max(0, t.t_dtw - off) })), - cameraCues: (seg.cameraCues ?? []).map(cue => ({ ...cue, at: Math.max(0, cue.at - off) })), - graphics: (seg.graphics ?? []).map(g => ({ ...g, at: Math.max(0, g.at - off) })), - })), - }; + transcript = applyTimestampOffset(transcript, off); console.log(`Applied timestamp offset: -${off}s`); } diff --git a/scripts/edit-transcript.test.js b/scripts/edit-transcript.test.js index aae7eb1..858b29f 100644 --- a/scripts/edit-transcript.test.js +++ b/scripts/edit-transcript.test.js @@ -13,6 +13,7 @@ import { rebalanceBoundaryTokens, buildPrevTokensByTdtw, mergeTokenFields, + applyTimestampOffset, reInjectSyntheticSegments, WORD_DURATION_ESTIMATE, CUT_START_BIAS, @@ -1218,6 +1219,92 @@ describe('mergeTokenFields', () => { expect(result.t_dtw).toBe(11); expect(result.t_end).toBeUndefined(); }); + + // Regression: `p` (the matched token) comes from the existing + // transcript.json, which already had timestampOffset subtracted by a + // *previous* run's "Apply timestamp offset" pass. `t`, and the transcript + // as a whole at the point mergeTokenFields runs, is still pre-offset this + // run — that pass hasn't executed yet. Without adding the offset back, + // preserved timing gets the offset subtracted an extra time every run, + // compounding by -timestampOffset each merge-doc invocation. Confirmed + // live: every hook's tokens drifted by exactly -0.5s (this project's + // configured offset) on the very next merge-doc run after timing + // preservation started working. + test('adds timestampOffset back onto preserved timing so the later single subtraction is not doubled', () => { + const fresh = { text: ' loop', t_dtw: 40.683, cut: false }; + // Simulates a token already offset-adjusted by a previous run (0.5s project offset). + const matched = { text: ' loop', t_dtw: 42.153, t_end: 42.414, cut: false }; + const result = mergeTokenFields(fresh, matched, true, 0.5); + // Un-offset by 0.5 so this run's later, single "Apply timestamp offset" + // pass lands back on the intended 42.153/42.414 — not 41.653/41.914. + expect(result.t_dtw).toBe(42.653); + expect(result.t_end).toBe(42.914); + }); + + test('timestampOffset defaults to 0 (no normalization) when omitted', () => { + const fresh = { text: ' loop', t_dtw: 40.683, cut: false }; + const matched = { text: ' loop', t_dtw: 42.153, t_end: 42.414, cut: false }; + const result = mergeTokenFields(fresh, matched, true); + expect(result.t_dtw).toBe(42.153); + expect(result.t_end).toBe(42.414); + }); +}); + +// ─── applyTimestampOffset ────────────────────────────────────────────────────── +// +// Regression: mergeTokenFields adds timestampOffset back onto a preserved +// token's t_dtw *and* t_end so this pass's single subtraction lands on the +// intended value. If this pass only subtracted off from t_dtw (as it +// originally did), t_end would never get un-added — compounding by +// +timestampOffset on every merge-doc run even though t_dtw stayed stable. +// Confirmed live: every hook's t_end drifted by +0.5s per run once t_dtw +// preservation was fixed, while t_dtw itself stayed correct. + +describe('applyTimestampOffset', () => { + test('subtracts off from t_dtw and t_end symmetrically', () => { + const transcript = { + segments: [ + { + start: 10, end: 12, + tokens: [{ text: ' loop', t_dtw: 42.653, t_end: 42.914 }], + }, + ], + }; + const result = applyTimestampOffset(transcript, 0.5); + expect(result.segments[0].tokens[0].t_dtw).toBe(42.153); + expect(result.segments[0].tokens[0].t_end).toBe(42.414); + }); + + test('leaves t_end untouched (undefined) when the token has none', () => { + const transcript = { + segments: [ + { start: 10, end: 12, tokens: [{ text: ' loop', t_dtw: 42.653 }] }, + ], + }; + const result = applyTimestampOffset(transcript, 0.5); + expect(result.segments[0].tokens[0].t_end).toBeUndefined(); + }); + + test('two consecutive applications with the intervening add-back are idempotent', () => { + // Mirrors the real round-trip: mergeTokenFields adds `off` back onto a + // preserved token before this pass subtracts it once — net zero drift. + const stored = { text: ' loop', t_dtw: 42.153, t_end: 42.414 }; + const rehydrated = { ...stored, t_dtw: stored.t_dtw + 0.5, t_end: stored.t_end + 0.5 }; + const transcript = { segments: [{ start: 0, end: 1, tokens: [rehydrated] }] }; + const result = applyTimestampOffset(transcript, 0.5); + expect(result.segments[0].tokens[0].t_dtw).toBe(stored.t_dtw); + expect(result.segments[0].tokens[0].t_end).toBe(stored.t_end); + }); + + test('clamps to 0 rather than going negative', () => { + const transcript = { + segments: [{ start: 0.2, end: 1, tokens: [{ text: ' hi', t_dtw: 0.2, t_end: 0.3 }] }], + }; + const result = applyTimestampOffset(transcript, 0.5); + expect(result.segments[0].start).toBe(0); + expect(result.segments[0].tokens[0].t_dtw).toBe(0); + expect(result.segments[0].tokens[0].t_end).toBe(0); + }); }); // ─── t_dtw collision → token cascade regression ─────────────────────────────── From 17580145a873462b9b467eca86f83b5181aa721c Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Thu, 30 Jul 2026 18:09:24 +0800 Subject: [PATCH 15/16] fix: cut token groups the doc no longer has a word for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit applyTextPartsToTokens' LCS-mismatch path (doc has more words than token groups) correctly synthesizes new tokens for the doc's extra words, but only ever added tokens — it never removed a token group that no doc word maps to anymore once its replacement was synthesized. Found via hook #302 ("...harness and loop and compound"): whisper.cpp misheard "loop and" as "lupin" (tokenized as " l"+"up"+"in"); the user corrected the doc text, LCS synthesized new "loop"/"and" tokens, but the leftover "lupin" tokens stayed in the array uncut, still spelled "lupin", rendering as spoken caption/audio content the doc no longer asks for ("loop and compound" showed as "loop and lupin compound"). Not specific to this hook — reproduces for any word-count-changing correction where LCS finds zero overlap between old and new spelling. Marks every token in every LCS-unmatched group as cut: true. Matched groups already get corrected text in place; unmatched ones now get removed from output instead of lingering with their old text. Added a lupin->loop-and regression test; updated the existing "a lot's"->"award's" test (same code path) to assert the leftover tokens are now cut rather than merely absent from segment.text. Also documents the also-newly-added Commit 12/13 entries in HOOK_TIMING_DIAGNOSTICS.md (token-timing preservation across merge-doc runs, and the t_end offset-compounding fix), which were implemented and committed in earlier commits but not yet written up. Co-Authored-By: Claude Sonnet 5 --- .../HOOK_TIMING_DIAGNOSTICS.md | 115 ++++++++++++++++++ scripts/edit-transcript.js | 14 +++ scripts/edit-transcript.test.js | 36 +++++- 3 files changed, 163 insertions(+), 2 deletions(-) diff --git a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md index 48ff0fa..df87338 100644 --- a/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md +++ b/docs/implementation-guides/HOOK_TIMING_DIAGNOSTICS.md @@ -540,6 +540,121 @@ commit) — all show the correct, complete caption text at the right frames. --- +### Commit 12 — `fix: preserve token timing across merge-doc runs` (`a739a78`) ✅ DONE + +**Status check:** snapshot every hook segment's token `t_dtw`/`t_end` right +after a `realign-hooks.js` run, then run `node scripts/edit-transcript.js +--merge-doc public/edit/transcript.doc.txt` with an unrelated doc edit — the +snapshot is unchanged. + +**Why:** the user reported "I made an edit to the hook, and the subtitles and +cuts are messed up again" — a second, largely-overlapping 16-item bug list +after making their own doc edits. Direct token inspection showed every hook +fixed by `realign-hooks.js` had reverted entirely to its original +pre-realignment `t_dtw`/`t_end` (e.g. hook #15 back to `[40.4, 40.502, 40.683, +40.825]`), and the regression was global — present on hooks the user's edit +never touched. Root cause: `main()`'s "preserve manual edits" step always +rebuilt token timing from a fresh parse of `transcript.raw.json`, using the +matched previous-run token only to carry forward `text`/`cut` — never +`t_dtw`/`t_end`. The match itself was keyed by `t_dtw` value + occurrence +count, which necessarily breaks the instant `t_dtw` is the thing being +corrected. + +**What was done:** added `mergeTokenFields(t, p, preserveTiming, +timestampOffset)`, matching tokens primarily by array position — stable +across a `realign-hooks.js` run, which only ever changes timing, never token +count or order — instead of by `t_dtw` value. When positions align, +`t_dtw`/`t_end` now carry forward from the matched token. Falls back to the +original value-based matching (timing not preserved) only when token count +differs — a real re-transcription, where positional correspondence can't be +assumed. + +**Manual test:** `npx jest scripts/edit-transcript.test.js` (119 passed, 2 +pre-existing skips), `npx eslint` clean. See Commit 13 for the end-to-end +idempotence verification, which caught a second bug this commit alone didn't +fully resolve. + +--- + +### Commit 13 — `fix: stop t_end from compounding across merge-doc runs` (`b0ec07b`) ✅ DONE + +**Status check:** run `node scripts/edit-transcript.js --merge-doc +public/edit/transcript.doc.txt` three times in a row on the live transcript, +diffing every hook's token `t_dtw`/`t_end` between each run — zero drift. + +**Why:** discovered *while verifying* Commit 12. `mergeTokenFields` needs to +add `timestampOffset` back onto a preserved token's `t_dtw`/`t_end` — `p` +(the matched token, read from disk) already had the offset subtracted by a +*previous* run's "Apply timestamp offset" pass, but the transcript this run +hasn't reached that pass yet, so the offset must be re-added before this +run's single subtraction can land correctly. That later pass, however, only +ever subtracted `off` from `t_dtw` — never `t_end`. Net effect: `t_dtw` +round-tripped correctly (add in `mergeTokenFields`, subtract in the offset +pass) while `t_end` only ever got added to, compounding by +`+timestampOffset` (0.5s, this project's configured offset) on every single +merge-doc run. Caught by an explicit idempotence test: snapshot → merge-doc → +compare → repeat; first attempt (Commit 12 alone) showed every hook's +`t_end` grown by exactly +0.5s per run while `t_dtw` stayed perfectly stable +— the asymmetry pointed straight at the offset-application block. + +**What was done:** extracted the inline offset-application block out of +`main()` into an exported, unit-testable `applyTimestampOffset(transcript, +off)`, and made it subtract `off` from `t_end` symmetrically with `t_dtw` +(only when `t_end` is defined — untimed tokens stay untimed). + +**Manual test:** `npx jest scripts/edit-transcript.test.js` (125 passed, 2 +pre-existing skips) — added 4 new tests for `applyTimestampOffset` plus the +add-back/subtract round-trip. `npx eslint` clean. End-to-end: ran merge-doc +three times consecutively on the live `public/edit/transcript.json`, diffing +all 35 hook segments' token timing between runs 1→2 and 2→3 — identical both +times (the sole apparent "mismatch" against the pre-Commit-12 baseline was +hook #302, whose text the user corrected — a real token-count change that +correctly falls to the non-preserving fallback path. That fallback path +itself turned out to have its own real bug — see Commit 14). + +--- + +### Commit 14 — `fix: cut token groups the doc no longer has a word for` ✅ DONE + +**Status check:** `applyTextPartsToTokens('harness and loop and compound', +[...tokens with " l"+"up"+"in" for a mis-heard "lupin"...])` returns the +"l"/"up"/"in" tokens with `cut: true`, not left spoken alongside the +synthesized "loop"/"and" replacement tokens. + +**Why:** found while verifying Commit 13's idempotence test — hook #302 +("...I know harness and loop and compound") was the one hook that didn't +survive a repeat merge-doc run, and inspecting why surfaced a second, real +bug, independent of the offset/preservation fixes. Whisper.cpp's raw +transcription misheard "loop and" as "lupin" (tokenized as `" l"+"up"+"in"`). +The user corrected the doc text to "loop and compound". `applyTextPartsToTokens`'s +LCS-mismatch path (doc has more words than token groups) correctly +*synthesized* new tokens for the doc's extra words ("loop", second "and"), +but only ever added — it never removed the token group ("lupin") that no +longer corresponds to any doc word once its replacement was synthesized. +That leftover group stayed in the array, still spelled "lupin", still +unqualified as `cut`, so it rendered as spoken caption/audio content the doc +no longer asks for: "loop and compound" showed as "loop and lupin compound". +This isn't specific to hook #302 — it reproduces for any word-count-changing +correction where the LCS finds zero overlap between the old and new spelling. + +**What was done:** after building `docToGrp` (the LCS-matched doc-word → +token-group pairs), mark every token in every *unmatched* group as `cut: +true`. A group with no pair is, by definition, a word the doc no longer has +— matched groups already get corrected text in place; unmatched ones now get +removed from output instead of lingering with their old (wrong) text. + +**Manual test:** `npx jest scripts/edit-transcript.test.js` (126 passed, 2 +pre-existing skips) — added a `lupin`→`loop and` regression test, and updated +the existing `a lot's`→`award's` test (which exercises the same code path) +to assert the leftover `a`/`lot's` tokens are now cut rather than merely +absent from the corrected segment text. `npx eslint` clean. End-to-end: ran +merge-doc + `realign-hooks.js` on the live transcript — hook #302's tokens +now read `harness, and, loop, and, compound` with `l`/`up`/`in` cut, and a +repeat merge-doc run reproduces exactly this same fallback-and-refix cycle +(expected — see Commit 13's note above — not a new instability). + +--- + ## Done When all 5 commits are complete: `render-hook-intro.js` has no duplicate timing diff --git a/scripts/edit-transcript.js b/scripts/edit-transcript.js index d93d51d..0b495e6 100644 --- a/scripts/edit-transcript.js +++ b/scripts/edit-transcript.js @@ -1083,6 +1083,20 @@ function applyTextPartsToTokens(rawText, tokens) { updated[idx] = { ...updated[idx], text: prefix + visibleWords[docIdx] }; } + // Cut token groups the doc no longer has a word for. LCS only pairs + // surviving words; a group that isn't in any pair is one the user fully + // replaced (e.g. a mis-transcribed "lupin" replaced with "loop and") — + // left uncut, it stays in the token array with its old (wrong) text and + // renders as spoken captions/audio the doc no longer asks for. + const matchedGroupIdxs = new Set(pairs.map(p => p.groupIdx)); + for (let gi = 0; gi < twg.length; gi++) { + if (matchedGroupIdxs.has(gi)) continue; + const g = twg[gi]; + for (let k = g.firstIdx; k <= g.lastIdx; k++) { + updated[k] = { ...updated[k], cut: true }; + } + } + // Synthesize tokens for doc words that have no matching token (Whisper DTW drop). // Work backwards so splices don't shift indices of earlier insertions. for (let di = visibleWords.length - 1; di >= 0; di--) { diff --git a/scripts/edit-transcript.test.js b/scripts/edit-transcript.test.js index 858b29f..e8dbd19 100644 --- a/scripts/edit-transcript.test.js +++ b/scripts/edit-transcript.test.js @@ -497,7 +497,10 @@ describe('mergeDocIntoTranscript', () => { test('word correction that reduces word count persists through buildDoc round-trip (a lot\'s → award\'s)', () => { // Regression: after the user changes "a lot's" → "award's" in the doc and runs // merge-doc, re-running edit-transcript must still show "award's", not revert to - // the original token reconstruction "a lot's". + // the original token reconstruction "a lot's". The replaced words' leftover + // tokens ("a", "lot's") are cut (LCS found no doc word for them) rather than + // lingering as spoken text — buildDoc represents that cut as a visible + // {a lot's} span so the human editor can see what was replaced. const tokens = [ tok(' You\'d', 0.1), tok(' create', 0.2), tok(' a', 0.3), tok(' lot', 0.4), tok("'s", 0.45), @@ -510,11 +513,16 @@ describe('mergeDocIntoTranscript', () => { const merged = mergeDocIntoTranscript(base, doc); expect(merged.segments[0].text).toBe("You'd create award's best"); + // The leftover "a"/"lot's" tokens are cut, not left spoken. + const leftover = merged.segments[0].tokens.filter(t => ['a', 'lot', "'s"].includes(t.text.trim())); + expect(leftover.length).toBeGreaterThan(0); + expect(leftover.every(t => t.cut)).toBe(true); // Round-trip: buildDoc must emit the corrected text, not revert to token reconstruction const rebuiltDoc = buildDoc(merged); expect(rebuiltDoc).toContain("award's"); - expect(rebuiltDoc).not.toContain("a lot's"); + // The cut leftover is shown as an explicit {..} cut span, not as plain spoken text. + expect(rebuiltDoc).toContain("{a lot's}"); }); test('marks segment as cut when CUT is present', () => { @@ -1052,6 +1060,30 @@ describe('applyTextPartsToTokens: LCS alignment when word count differs', () => expect(result[0].text).toBe(' world'); expect(result[1].text).toBe(' earth'); }); + + // Regression: a mis-transcribed word replaced with a different word count + // (e.g. Whisper heard "lupin", the user corrected it to "loop and") left the + // old word's tokens in the array uncut — LCS synthesized new tokens for the + // doc's extra words but never removed the ones no doc word maps to anymore, + // so captions rendered "loop and lupin compound" instead of "loop and compound". + test('a fully-replaced mis-transcribed word is cut, not left spoken alongside its replacement', () => { + const tokens = [ + tok(' harness', 0.1), tok(' and', 0.2), + tok(' l', 0.3), tok('up', 0.32), tok('in', 0.35), + tok(' compound', 0.4), + ]; + const result = applyTextPartsToTokens('harness and loop and compound', tokens); + const lupinTokens = result.filter(t => ['l', 'up', 'in'].includes(t.text.trim())); + expect(lupinTokens.length).toBe(3); + expect(lupinTokens.every(t => t.cut)).toBe(true); + // The replacement words are present and not cut. + const loopTok = result.find(t => t.text.trim() === 'loop'); + const secondAndTok = result.filter(t => t.text.trim() === 'and')[1]; + expect(loopTok).toBeDefined(); + expect(loopTok.cut).toBe(false); + expect(secondAndTok).toBeDefined(); + expect(secondAndTok.cut).toBe(false); + }); }); // ─── rebalanceBoundaryTokens ───────────────────────────────────────────────── From 4fd79273c853a3c86229d0ea85fac32915d45ccb Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Sat, 1 Aug 2026 13:54:49 +0800 Subject: [PATCH 16/16] fix: token-level speaker attribution; thumbnail candidate re-roll; wizard resume fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Diarizer.js: assignSpeakers now prefers per-word overlap against diarization turns (_tokenLevelSpeaker) before falling back to whole-segment overlap (_segmentOverlapSpeaker). Whisper's pre-alignment segment boundaries are coarse and include leading/trailing silence, which biased the old whole-segment method at fast speaker handoffs. - extract-speaker-candidates.py: extracted _sample_timestamps() and added exclude/index-start/append support so the thumbnail wizard can fetch additional candidates for a speaker without regenerating already-seen ones. - generate-thumbnail.js: detects a Remotion render that exits 0 and writes a file but produced no visual content (composition data never loaded) via an all-channels-zero check, instead of silently accepting a blank thumbnail. Also clears any stale output file before rendering so the check can't see a leftover from a previous failed attempt. - wizard.js: guards against a stale audio file left in the input dir from a prior episode by comparing its mtime against the synced video; fixes the "optimize" step's resumeStep/redoStepId condition and syncResults reuse when resuming a run; skips the redundant speaker-name review prompt when names were just assigned in the same run; and wires the thumbnail candidate-selection prompt to extract-speaker-candidates.py's new "more" flow. - Adds export-property-spec-ai-eng-buzzwords.ts: generates the poddedit Rust compositor's Tier 1 acceptance-baseline properties.json for the "ai-eng-buzzwords" fixture episode (multi-angle, real cut checkpoints) — writes to the poddedit repo, not this one. Adapted from the existing export-property-spec.ts generator; see its header for full provenance. Co-Authored-By: Claude Sonnet 5 --- scripts/diarize/Diarizer.js | 81 +++-- .../export-property-spec-ai-eng-buzzwords.ts | 338 ++++++++++++++++++ .../thumbnail/extract-speaker-candidates.py | 104 ++++-- scripts/thumbnail/generate-thumbnail.js | 27 +- scripts/wizard.js | 98 +++-- 5 files changed, 573 insertions(+), 75 deletions(-) create mode 100644 scripts/export-property-spec-ai-eng-buzzwords.ts diff --git a/scripts/diarize/Diarizer.js b/scripts/diarize/Diarizer.js index b95474c..d1e23ed 100644 --- a/scripts/diarize/Diarizer.js +++ b/scripts/diarize/Diarizer.js @@ -130,34 +130,69 @@ class Diarizer { return turns; } - // Assign each transcript segment the speaker whose turn overlaps it the most. + // Whole-segment overlap: the speaker whose turn overlaps [seg.start, seg.end] the most. // Falls back to the nearest turn by midpoint distance for segments that fall // in gaps between diarization turns (silence, cross-talk boundaries, etc.). + // Segment-level start/end are Whisper's coarse (pre-alignment) boundaries and can + // include leading/trailing silence, which biases this method at fast speaker handoffs — + // see _tokenLevelSpeaker() for the per-word alternative used when tokens are available. + _segmentOverlapSpeaker(seg, turns) { + const segMid = (seg.start + seg.end) / 2; + let bestSpeaker = ''; + let bestOverlap = 0; + let nearestSpeaker = ''; + let nearestDist = Infinity; + + for (const turn of turns) { + const overlap = Math.min(seg.end, turn.end) - Math.max(seg.start, turn.start); + if (overlap > bestOverlap) { + bestOverlap = overlap; + bestSpeaker = turn.speaker; + } + const dist = Math.abs(segMid - (turn.start + turn.end) / 2); + if (dist < nearestDist) { + nearestDist = dist; + nearestSpeaker = turn.speaker; + } + } + + return bestSpeaker || nearestSpeaker; + } + + // Per-word overlap: sum each word token's overlap-duration against every diarization + // turn, then pick the speaker with the largest total. Using word spans instead of the + // whole segment span avoids counting the segment's leading/trailing silence (Whisper's + // pre-alignment segment boundaries are coarse) as evidence for whichever speaker's turn + // happens to cover that silence — the failure mode that misattributes short segments + // right at a fast speaker handoff. + _tokenLevelSpeaker(seg, turns) { + const words = (seg.tokens || []).filter((t) => /[a-zA-Z0-9]/.test(t.text)); + if (words.length === 0) return null; + + const totals = {}; + for (let i = 0; i < words.length; i++) { + const tok = words[i]; + const start = tok.t_dtw; + const end = tok.t_end ?? words[i + 1]?.t_dtw ?? seg.end; + if (end <= start) continue; // zero-length span — pre-alignment timestamps bunched together + + for (const turn of turns) { + const overlap = Math.min(end, turn.end) - Math.max(start, turn.start); + if (overlap > 0) totals[turn.speaker] = (totals[turn.speaker] || 0) + overlap; + } + } + + const [winner] = Object.entries(totals).sort((a, b) => b[1] - a[1])[0] || [null]; + return winner; + } + assignSpeakers(transcript, turns) { return { ...transcript, - segments: transcript.segments.map((seg) => { - const segMid = (seg.start + seg.end) / 2; - let bestSpeaker = ''; - let bestOverlap = 0; - let nearestSpeaker = ''; - let nearestDist = Infinity; - - for (const turn of turns) { - const overlap = Math.min(seg.end, turn.end) - Math.max(seg.start, turn.start); - if (overlap > bestOverlap) { - bestOverlap = overlap; - bestSpeaker = turn.speaker; - } - const dist = Math.abs(segMid - (turn.start + turn.end) / 2); - if (dist < nearestDist) { - nearestDist = dist; - nearestSpeaker = turn.speaker; - } - } - - return { ...seg, speaker: bestSpeaker || nearestSpeaker }; - }), + segments: transcript.segments.map((seg) => ({ + ...seg, + speaker: this._tokenLevelSpeaker(seg, turns) || this._segmentOverlapSpeaker(seg, turns), + })), }; } diff --git a/scripts/export-property-spec-ai-eng-buzzwords.ts b/scripts/export-property-spec-ai-eng-buzzwords.ts new file mode 100644 index 0000000..d26e165 --- /dev/null +++ b/scripts/export-property-spec-ai-eng-buzzwords.ts @@ -0,0 +1,338 @@ +#!/usr/bin/env npx tsx +/** + * export-property-spec-ai-eng-buzzwords.ts + * + * Generates properties.json for the poddedit Rust compositor (Epic 1) Tier 1 + * acceptance baseline, for the "ai-eng-buzzwords" fixture episode. + * + * This is an adapted COPY of scripts/export-property-spec.ts (the + * "is-ai-a-bubble" generator) — that original script is left untouched. + * See its own header comment for the full provenance note (what's imported + * real vs. locally ported); the same provenance applies here unchanged. + * + * Differences from the is-ai-a-bubble generator, and why: + * + * 1. Points at the ai-eng-buzzwords fixture (2 real camera angles: angle1, + * angle2; 3 speakers on angle2: Natasha, Saloni, Victoria — vs. + * is-ai-a-bubble's single angle). `activeAngleForShot` below is the exact + * same generic reverse-lookup as the original script — unchanged, because + * it was already written generically (matches shot.videoSrc against + * profiles.angles). See the "Multi-angle finding" note near the bottom of + * this file for what the multi-angle case actually does in + * buildCameraShots (important, non-obvious, and NOT something this script + * had to reimplement — it just calls the real function). + * + * 2. This fixture has REAL cut data (unlike is-ai-a-bubble, which had none + * and used meta.videoStart as a stand-in). Two real cut checkpoints are + * built below instead of the videoStart stand-in: + * - an intra-segment cuts[] checkpoint (segment id=208, Victoria, + * cuts: [{from: 665.558, to: 665.663}]) + * - a whole-segment cut:true checkpoint (segment id=196, Natasha, + * start=633.042, end=637.93) + * + * 3. 36 hook segments (vs. is-ai-a-bubble's 23) — hook checkpoint logic is + * unchanged, just picks up the real first hook segment for this fixture. + * + * Usage: + * npx tsx scripts/export-property-spec-ai-eng-buzzwords.ts [transcript.json] [camera-profiles.json] [outFile] + * + * This script is intentionally NOT added to git — scratch tooling for + * generating a spec that lives in the poddedit repo, not deckcreate. + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { buildSections } from '../remotion/components/SegmentPlayer'; +import { buildCameraShots, sourceToOutputFrame } from '../remotion/components/CameraPlayer'; +import type { Section } from '../remotion/lib/hookTiming'; +import type { Segment, Transcript } from '../remotion/types/transcript'; +import type { CameraProfiles, CameraShot } from '../remotion/types/camera'; + +// ── CLI args ────────────────────────────────────────────────────────────────── + +const FIXTURE_DIR = '/Users/natashaannlum/Documents/GitHub/poddedit/tests/fixtures/ai-eng-buzzwords'; +const DEFAULT_TRANSCRIPT = path.join(FIXTURE_DIR, 'transcript.json'); +const DEFAULT_PROFILES = path.join(FIXTURE_DIR, 'camera-profiles.json'); +const DEFAULT_OUT = '/Users/natashaannlum/Documents/GitHub/poddedit/tests/baselines/ai-eng-buzzwords/properties.json'; + +const transcriptPath = process.argv[2] ?? DEFAULT_TRANSCRIPT; +const profilesPath = process.argv[3] ?? DEFAULT_PROFILES; +const outPath = process.argv[4] ?? DEFAULT_OUT; + +// ── Load fixture inputs ─────────────────────────────────────────────────────── + +const transcript: Transcript = JSON.parse(fs.readFileSync(transcriptPath, 'utf-8')); +const profiles: CameraProfiles = JSON.parse(fs.readFileSync(profilesPath, 'utf-8')); +const fps = transcript.meta.fps; + +// ── Ported: trim filter (Composition.tsx:53-61, not exported) ──────────────── +// Identical to the is-ai-a-bubble script — see its header for provenance. +function getActiveSegments(t: Transcript): Segment[] { + const { videoStart, videoEnd } = t.meta; + return t.segments.filter(s => { + if (videoStart !== undefined && s.end < videoStart) return false; + if (videoEnd !== undefined && s.start > videoEnd) return false; + return true; + }); +} + +// ── Replicate Composition.tsx's TranscriptComposition assembly exactly ─────── +const hookSegments = transcript.segments.filter(s => s.hook && !s.cut); +const mainSegments = getActiveSegments(transcript).filter(s => !s.hook); +const orderedSegments: Segment[] = [...hookSegments, ...mainSegments]; + +const { hookSections, mainSections } = buildSections( + orderedSegments, + fps, + transcript.meta.videoStart, + transcript.meta.videoEnd, + true, // includeHooksInMain — matches Composition.tsx (longform) +); + +// SCOPE NOTE (same as is-ai-a-bubble script): mainOffset=0, no intro/outro +// splice modeled — Epic 1's compositor doesn't model title cards. +const shots: CameraShot[] = buildCameraShots(orderedSegments, profiles, fps, mainSections, hookSections, false); + +const hookTotalFrames = hookSections.reduce((sum, s) => sum + (s.trimAfter - s.trimBefore), 0); +const mainTotalFrames = mainSections.reduce((sum, s) => sum + (s.trimAfter - s.trimBefore), 0); +const totalOutputFrames = hookTotalFrames + mainTotalFrames; +const totalOutputSeconds = totalOutputFrames / fps; + +// ── New (non-editorial) helper: inverse output-frame -> source-time mapping ── +function outputFrameToSourceFrame(localFrame: number, sections: Section[]): number { + let acc = 0; + for (const sec of sections) { + const dur = sec.trimAfter - sec.trimBefore; + if (localFrame < acc + dur) { + return sec.trimBefore + (localFrame - acc); + } + acc += dur; + } + const last = sections[sections.length - 1]; + return last ? last.trimAfter : 0; +} + +function outputFrameToSourceTime(outputFrame: number): number { + if (outputFrame < hookTotalFrames) { + return outputFrameToSourceFrame(outputFrame, hookSections) / fps; + } + return outputFrameToSourceFrame(outputFrame - hookTotalFrames, mainSections) / fps; +} + +function shotAtFrame(frame: number): CameraShot | null { + for (const shot of shots) { + if (frame >= shot.startFrame && frame < shot.endFrame) return shot; + } + return shots.length > 0 ? shots[shots.length - 1] : null; +} + +// Reverse-lookup the angle name for a shot's videoSrc. Unchanged from the +// is-ai-a-bubble script — already generic, works for N angles. +function activeAngleForShot(shot: CameraShot | null): string | null { + if (!shot) return null; + const angles = profiles.angles ?? {}; + if (shot.videoSrc) { + const found = Object.entries(angles).find(([, cfg]) => cfg.videoSrc === shot.videoSrc); + if (found) return found[0]; + } + const firstAngle = Object.keys(angles)[0]; + return firstAngle ?? null; +} + +function round(n: number, dp = 4): number { + const f = 10 ** dp; + return Math.round(n * f) / f; +} + +function buildCheckpoint(outputTimeS: number, description: string) { + const outputFrame = Math.round(outputTimeS * fps); + const shot = shotAtFrame(outputFrame); + const isHook = outputFrame < hookTotalFrames; + const sourceTimeS = outputFrameToSourceTime(outputFrame); + + if (!isHook) { + const localMainFrame = outputFrame - hookTotalFrames; + const roundTrippedFrame = sourceToOutputFrame(sourceTimeS, mainSections, fps); + if (roundTrippedFrame !== localMainFrame) { + throw new Error( + `Round-trip mismatch at outputFrame=${outputFrame}: localMainFrame=${localMainFrame} but sourceToOutputFrame(outputFrameToSourceTime(...))=${roundTrippedFrame}`, + ); + } + } + const rawSpeaker = shot?.speaker ?? null; + const activeSpeaker = rawSpeaker ? rawSpeaker.split(':')[0] : null; + + const expected: Record = { + is_hook: isHook, + active_angle: activeAngleForShot(shot), + active_speaker: activeSpeaker, + is_cut: false, + source_time_s: round(sourceTimeS, 3), + }; + + if (shot && !shot.isWide) { + expected.viewport = { + cx: round(shot.viewport.cx), + cy: round(shot.viewport.cy), + w: round(shot.viewport.w), + h: round(shot.viewport.h), + }; + } + + return { + output_time_s: round(outputTimeS, 3), + description, + expected, + }; +} + +// ── Checkpoint 1: hook checkpoint ───────────────────────────────────────────── +const firstHookSeg = hookSegments[0]; +const hookCheckpoint = buildCheckpoint( + 0, + `Hook checkpoint: first hook segment (id=${firstHookSeg.id}, speaker=${firstHookSeg.speaker}) at output start of the hook zone`, +); +const expectedHookFrom = firstHookSeg.hookFrom ?? firstHookSeg.start; + +// ── Checkpoint 2: speaker-change checkpoint ─────────────────────────────────── +let speakerChangeCheckpoint: ReturnType | null = null; +for (let i = 1; i < shots.length; i++) { + const prev = shots[i - 1]; + const cur = shots[i]; + if (prev.startFrame < hookTotalFrames || cur.startFrame < hookTotalFrames) continue; + const prevSpeaker = prev.speaker?.split(':')[0] ?? null; + const curSpeaker = cur.speaker?.split(':')[0] ?? null; + if (prevSpeaker && curSpeaker && prevSpeaker !== curSpeaker) { + speakerChangeCheckpoint = buildCheckpoint( + cur.startFrame / fps, + `Speaker-change checkpoint: active_speaker changes ${prevSpeaker} -> ${curSpeaker} (adjacent camera shots, main content)`, + ); + break; + } +} + +// ── Checkpoint 3: wide-angle checkpoint ─────────────────────────────────────── +let wideCheckpoint: ReturnType | null = null; +for (const shot of shots) { + if (shot.isWide && shot.startFrame >= hookTotalFrames) { + wideCheckpoint = buildCheckpoint( + shot.startFrame / fps, + `Wide-angle checkpoint: first main-content shot with isWide=true (no speaker closeup active)`, + ); + break; + } +} +if (!wideCheckpoint) { + for (const shot of shots) { + if (shot.isWide) { + wideCheckpoint = buildCheckpoint( + shot.startFrame / fps, + `Wide-angle checkpoint: first shot with isWide=true (no speaker closeup active)`, + ); + break; + } + } +} + +// ── Checkpoint 4/5: REAL cut checkpoints ────────────────────────────────────── +// Unlike is-ai-a-bubble (zero real cut data, videoStart stand-in), this +// fixture has genuine cuts[] and cut:true data — use it directly, per +// tests/baselines/README.md's own note asking for this once available. +// +// (a) Intra-segment cut: segment id=208 (Victoria), cuts: [{from: 665.558, to: 665.663}]. +// source_time_s is the midpoint of that range, which per the Rust +// reference's is_cut_at_source_time (crates/compositor/src/camera_state.rs) +// satisfies `source_time_s >= c.from && source_time_s < c.to`. +const intraSegCutSeg = transcript.segments.find(s => s.id === 208); +if (!intraSegCutSeg || intraSegCutSeg.cuts.length === 0) { + throw new Error('Expected segment id=208 with a non-empty cuts[] entry — fixture may have changed'); +} +const intraCutRange = intraSegCutSeg.cuts[0]; +const intraCutMidpoint = round((intraCutRange.from + intraCutRange.to) / 2, 3); +const intraSegCutCheckpoint = { + output_time_s: null, + source_time_s: intraCutMidpoint, + description: `Intra-segment cut checkpoint: segment id=${intraSegCutSeg.id} (speaker=${intraSegCutSeg.speaker}) cuts[] range [${intraCutRange.from}, ${intraCutRange.to})`, + expected: { is_cut: true }, +}; + +// (b) Whole-segment cut: segment id=196 (Natasha), cut:true, start=633.042 end=637.93. +const wholeCutSeg = transcript.segments.find(s => s.id === 196); +if (!wholeCutSeg || !wholeCutSeg.cut) { + throw new Error('Expected segment id=196 with cut:true — fixture may have changed'); +} +const wholeCutMidpoint = round((wholeCutSeg.start + wholeCutSeg.end) / 2, 3); +const wholeSegCutCheckpoint = { + output_time_s: null, + source_time_s: wholeCutMidpoint, + description: `Whole-segment cut checkpoint: segment id=${wholeCutSeg.id} (speaker=${wholeCutSeg.speaker}) cut:true, range [${wholeCutSeg.start}, ${wholeCutSeg.end})`, + expected: { is_cut: true }, +}; + +// ── Regular sampling: one checkpoint every 30s of output ───────────────────── +const regularCheckpoints: ReturnType[] = []; +for (let t = 0; t < totalOutputSeconds; t += 30) { + regularCheckpoints.push(buildCheckpoint(t, `Regular sampling checkpoint at output t=${t}s`)); +} + +// ── Assemble, dedupe (by output_time_s), sort ──────────────────────────────── + +const namedCheckpoints = [hookCheckpoint, speakerChangeCheckpoint, wideCheckpoint].filter( + (c): c is ReturnType => c !== null, +); + +const byOutputTime = new Map>(); +for (const c of [...regularCheckpoints, ...namedCheckpoints]) { + const existing = byOutputTime.get(c.output_time_s); + if (!existing || namedCheckpoints.includes(c)) { + byOutputTime.set(c.output_time_s, c); + } +} + +const sortedCheckpoints = [...byOutputTime.values()].sort((a, b) => a.output_time_s - b.output_time_s); + +// is_cut checkpoints are special-shaped (output_time_s: null) — append at the end. +const allCheckpoints = [...sortedCheckpoints, intraSegCutCheckpoint, wholeSegCutCheckpoint]; + +const spec = { + episode: 'ai-eng-buzzwords', + generated_from: 'deckcreate pipeline logic (buildCameraShots/hookTiming/SegmentPlayer), not the Remotion renderer', + checkpoints: allCheckpoints, +}; + +fs.mkdirSync(path.dirname(outPath), { recursive: true }); +fs.writeFileSync(outPath, JSON.stringify(spec, null, 2) + '\n'); + +console.error(`[export-property-spec] wrote ${allCheckpoints.length} checkpoints to ${outPath}`); +console.error(`[export-property-spec] hookTotalFrames=${hookTotalFrames} mainTotalFrames=${mainTotalFrames} totalOutputSeconds=${totalOutputSeconds.toFixed(3)}`); +console.error(`[export-property-spec] hook checkpoint segment id=${firstHookSeg.id} hookFrom=${expectedHookFrom} emitted source_time_s=${hookCheckpoint.expected.source_time_s}`); +console.error(`[export-property-spec] speakerChangeCheckpoint found=${speakerChangeCheckpoint !== null}`); +console.error(`[export-property-spec] wideCheckpoint found=${wideCheckpoint !== null}`); + +// ── Multi-angle sanity check (required by task) ────────────────────────────── +// Verify checkpoints actually reference BOTH angle1 and angle2, not just one. +const anglesSeen = new Set(); +for (const c of sortedCheckpoints) { + anglesSeen.add((c.expected as Record).active_angle as string | null); +} +console.error(`[export-property-spec] distinct active_angle values across non-cut checkpoints: ${[...anglesSeen].join(', ')}`); +if (!anglesSeen.has('angle1') || !anglesSeen.has('angle2')) { + console.error('[export-property-spec] WARNING: expected both angle1 and angle2 to appear across checkpoints — only found: ' + [...anglesSeen].join(', ')); +} + +// ── Multi-angle finding (informational, not a script behavior) ─────────────── +// buildCameraShots (remotion/components/CameraPlayer.tsx) selects between a +// speaker's multiple angle-keyed profiles (e.g. "Natasha:angle1" AND +// "Natasha:angle2") via `getSpeakerAngles(speaker)` + a `shotAngleIndex` +// counter that increments each time the pacing algorithm returns to the same +// speaker (see `emitShot`'s `angleIdx = Math.floor(shotAngleIndex / 2) % +// speakerAngles.length`). It is NOT driven by any cameraCue or per-segment +// annotation — it's a deterministic round-robin over the speaker's +// configured angles, cycling every other shot. Victoria has only one +// configured angle (angle2) so she never cycles. This does NOT exist in +// poddedit's crates/compositor/src/camera_state.rs port today — see that +// file's module doc ("multiple angles per speaker" is listed as an +// intentionally-unported scope decision because no fixture needed it before +// this one) and its `get_speaker_profile` doc comment, which explicitly +// assumes "exactly one angleName per speaker (no multi-angle cycling to +// resolve)". This fixture breaks that assumption for Natasha and Saloni. diff --git a/scripts/thumbnail/extract-speaker-candidates.py b/scripts/thumbnail/extract-speaker-candidates.py index 1cdc0e9..4c29aea 100644 --- a/scripts/thumbnail/extract-speaker-candidates.py +++ b/scripts/thumbnail/extract-speaker-candidates.py @@ -79,20 +79,12 @@ def extract_frame(video_path: str, timestamp: float, output_path: str, is_hdr: b # ── Frame sampling ───────────────────────────────────────────────────────────── -def get_candidate_timestamps(transcript: dict, speaker: str, num_candidates: int) -> List[float]: - segments = transcript.get('segments', []) - speaking_segments = [ - s for s in segments - if s.get('speaker') == speaker and not s.get('cut', False) - ] - - if not speaking_segments: - return [] - +def _sample_timestamps(speaking_segments: List[dict], count: int) -> List[float]: + """Evenly sample `count` timestamps across the given speaking segments.""" timestamps = [] - if len(speaking_segments) >= num_candidates: - step = len(speaking_segments) / num_candidates - for i in range(num_candidates): + if len(speaking_segments) >= count: + step = len(speaking_segments) / count + for i in range(count): idx = int(i * step) seg = speaking_segments[idx] timestamps.append((seg['start'] + seg['end']) / 2) @@ -113,21 +105,60 @@ def get_candidate_timestamps(transcript: dict, speaker: str, num_candidates: int pad = seg_duration * 0.1 t = seg['start'] + pad + (seg['end'] - seg['start'] - 2 * pad) * (i / (samples_in_seg - 1)) timestamps.append(round(t, 3)) - if len(timestamps) >= num_candidates: + if len(timestamps) >= count: break - if len(timestamps) >= num_candidates: + if len(timestamps) >= count: break # If still not enough, fall back to repeating midpoints of longest segments - while len(timestamps) < num_candidates and speaking_segments: + while len(timestamps) < count and speaking_segments: # Sort by duration, pick longest unused segments sorted_segs = sorted(speaking_segments, key=lambda s: s['end'] - s['start'], reverse=True) for seg in sorted_segs: - if len(timestamps) >= num_candidates: + if len(timestamps) >= count: break timestamps.append((seg['start'] + seg['end']) / 2) - return timestamps[:num_candidates] + return timestamps[:count] + + +def get_candidate_timestamps( + transcript: dict, speaker: str, num_candidates: int, exclude: Optional[List[float]] = None, +) -> List[float]: + segments = transcript.get('segments', []) + speaking_segments = [ + s for s in segments + if s.get('speaker') == speaker and not s.get('cut', False) + ] + + if not speaking_segments: + return [] + + if not exclude: + return _sample_timestamps(speaking_segments, num_candidates) + + # "More candidates" path: grow the evenly-spaced sampling pool until enough + # timestamps distinct from `exclude` (already shown to the user) turn up. + # Rounding to 1 decimal absorbs float drift between calls. + exclude_rounded = {round(t, 1) for t in exclude} + max_pool = max(len(speaking_segments) * 4, num_candidates * 8, 40) + pool_count = num_candidates + result = [] + while pool_count <= max_pool: + pool = _sample_timestamps(speaking_segments, pool_count) + seen = set() + result = [] + for t in pool: + r = round(t, 1) + if r in exclude_rounded or r in seen: + continue + seen.add(r) + result.append(t) + if len(result) >= num_candidates: + break + pool_count += num_candidates + + return result[:num_candidates] # ── Viewport crop ────────────────────────────────────────────────────────────── @@ -281,6 +312,7 @@ def process_speaker_angle( output_dir: str, num_candidates: int, global_index_start: int = 0, + exclude: Optional[List[float]] = None, ) -> List[Dict[str, Any]]: """Extract candidates from a single angle for a speaker.""" from PIL import Image @@ -296,7 +328,7 @@ def process_speaker_angle( sys.stderr.write(f' No closeupViewport for {speaker} on {angle_name} — skipping angle\n') return [] - timestamps = get_candidate_timestamps(transcript, speaker, num_candidates) + timestamps = get_candidate_timestamps(transcript, speaker, num_candidates, exclude) if not timestamps: sys.stderr.write(f' No speaking segments for {speaker} on {angle_name}\n') return [] @@ -360,6 +392,8 @@ def process_speaker( is_hdr: bool, output_dir: str, num_candidates: int = 6, + index_start: int = 0, + exclude: Optional[List[float]] = None, ) -> Optional[Dict[str, Any]]: """Extract candidates from ALL angles for a speaker.""" @@ -370,7 +404,7 @@ def process_speaker( print(f' Extracting from {len(angle_videos)} angle(s)...') all_candidates = [] - global_index = 0 + global_index = index_start for video_path, angle_name, angle_config in angle_videos: # Check if HDR for this specific video @@ -380,7 +414,7 @@ def process_speaker( speaker, angle_name, video_path, transcript, camera_profiles, angle_hdr, output_dir, num_candidates, - global_index + global_index, exclude, ) all_candidates.extend(angle_candidates) @@ -406,6 +440,9 @@ def parse_args(): p.add_argument('--output-dir', required=True, help='Directory to save candidate previews') p.add_argument('--num-candidates', type=int, default=6, help='Number of candidate frames per speaker') p.add_argument('--speakers', nargs='+', help='Specific speakers to process (default: all in transcript)') + p.add_argument('--exclude-timestamps', help='Comma-separated timestamps to avoid re-sampling (used when requesting more candidates)') + p.add_argument('--index-start', type=int, default=0, help='Starting candidate index (avoids filename collisions when adding more candidates)') + p.add_argument('--append', action='store_true', help='Append to the existing manifest entry for these speakers instead of replacing it') return p.parse_args() @@ -438,20 +475,39 @@ def main(): if is_hdr: print('HDR video detected — applying tonemapping') + exclude = None + if args.exclude_timestamps: + exclude = [float(t) for t in args.exclude_timestamps.split(',') if t.strip()] + results = [] for speaker in speakers: print(f'\n[{speaker}]') result = process_speaker( speaker, transcript, camera_profiles, args.video, - is_hdr, args.output_dir, args.num_candidates + is_hdr, args.output_dir, args.num_candidates, + args.index_start, exclude, ) if result: results.append(result) - # Write candidates manifest + # Merge into any existing manifest so processing a subset of speakers + # (e.g. one speaker's "more candidates" request) doesn't clobber the rest. manifest_path = os.path.join(args.output_dir, 'candidates.json') + existing_by_speaker = {} + if os.path.exists(manifest_path): + with open(manifest_path, 'r') as f: + existing = json.load(f) + existing_by_speaker = {s['speaker']: s for s in existing.get('speakers', [])} + + for result in results: + prev = existing_by_speaker.get(result['speaker']) + if args.append and prev: + result['candidates'] = prev['candidates'] + result['candidates'] + existing_by_speaker[result['speaker']] = result + + manifest = {'speakers': list(existing_by_speaker.values())} with open(manifest_path, 'w') as f: - json.dump({'speakers': results}, f, indent=2) + json.dump(manifest, f, indent=2) print(f'\n✓ Candidates manifest written: {manifest_path}') print('Run the selection script to choose preferred frames:') diff --git a/scripts/thumbnail/generate-thumbnail.js b/scripts/thumbnail/generate-thumbnail.js index 5d79d51..c04a6d4 100644 --- a/scripts/thumbnail/generate-thumbnail.js +++ b/scripts/thumbnail/generate-thumbnail.js @@ -84,6 +84,16 @@ function renderThumbnail(outputPath, props) { // ── Post-process ────────────────────────────────────────────────────────────── +// Detects a Remotion render that "succeeded" (exit 0, file written) but produced +// no visual content — e.g. a data fetch inside the composition resolved without +// setting state, which renders `null` for the whole frame. Every channel sitting +// at flat 0 (fully black + transparent) is the signature of that failure mode. +async function isBlankImage(filePath) { + const { default: sharp } = await import('sharp'); + const stats = await sharp(filePath).stats(); + return stats.channels.every(c => c.max === 0); +} + async function postProcess(filePath) { const { default: sharp } = await import('sharp'); const tempPath = filePath.replace(/\.png$/, '_raw.png'); @@ -149,17 +159,26 @@ async function main() { }; await fs.ensureDir(path.dirname(args.output)); + // Clear any stale file so the post-render checks below can only reflect this + // run's output, not leftovers from a previous failed attempt at the same path. + await fs.remove(args.output); console.log('Rendering thumbnail...'); renderThumbnail(args.output, props); - if (await fs.pathExists(args.output)) { - await postProcess(args.output); - console.log(`✓ Thumbnail saved → ${args.output}`); - } else { + if (!await fs.pathExists(args.output)) { console.warn(' Output file not found after render — check remotion still output'); process.exit(1); } + if (await isBlankImage(args.output)) { + console.error(' ✗ Rendered thumbnail is blank — Remotion completed without loading the composition data.'); + console.error(' This can happen under heavy system load (e.g. another encode running). Try again.'); + process.exit(1); + } + + await postProcess(args.output); + console.log(`✓ Thumbnail saved → ${args.output}`); + if (args.open) { spawnSync('open', [args.output]); } diff --git a/scripts/wizard.js b/scripts/wizard.js index abee13b..e9bf7fa 100644 --- a/scripts/wizard.js +++ b/scripts/wizard.js @@ -130,7 +130,24 @@ async function detectExistingWork() { } const inputDir = p('public', 'transcribe', 'input'); - const audioInInput = !!(await findFileIn(inputDir, ['.wav', '.mp3', '.aac', '.m4a'])); + const audioInInputPath = await findFileIn(inputDir, ['.wav', '.mp3', '.aac', '.m4a']); + let audioInInput = !!audioInInputPath; + if (audioInInput && syncedVideo) { + // Guard against a leftover audio file from a previous episode: if it predates + // the current synced video, it can't have been extracted from it — treat as stale. + const syncedVideoPath = syncedVideoLegacy + ? p('public', 'sync', 'output', 'synced-output.mp4') + : p('public', 'sync', 'output', 'synced-output-1.mp4'); + try { + const [audioStat, videoStat] = await Promise.all([ + fs.stat(audioInInputPath), + fs.stat(syncedVideoPath), + ]); + if (audioStat.mtimeMs < videoStat.mtimeMs) audioInInput = false; + } catch { + // ignore — fall back to existence check + } + } const videoInInputPath = await findFileIn(inputDir, ['.mp4', '.mov', '.mkv']); const videoInInput = !!videoInInputPath; return { @@ -195,7 +212,7 @@ async function main() { } else if (menuChoice === '2') { const stepDefs = [ { id: 'sync', label: 'Sync audio + video', done: existing.syncedVideo, resumeAt: 0 }, - { id: 'optimize', label: 'Re-optimise synced video for Remotion', done: false, resumeAt: 0 }, + { id: 'optimize', label: 'Re-optimise synced video for Remotion', done: false, resumeAt: 1 }, { id: 'transcribe', label: 'Transcribe + Diarize', done: existing.rawTranscript, resumeAt: 1 }, { id: 'align', label: 'Forced alignment', done: existing.alignedTranscript, resumeAt: 1 }, { id: 'buildDoc', label: 'Build editable doc', done: existing.transcriptDoc, resumeAt: 2 }, @@ -374,6 +391,7 @@ async function main() { // ── STEP: Sync (mode 1 only) — also runs prepare audio when resumeStep=0 ── let videoForExtract = videoFile; const syncOutputDir = path.join(cwd, 'public', 'sync', 'output'); + let syncResults; if (resumeStep === 0 && mode === 1) { console.log('\n ── Sync audio and video ─────────────────────────────'); @@ -383,7 +401,6 @@ async function main() { const { default: AudioSyncer } = await import('./sync/AudioSyncer.js'); const allVideos = [videoFile, ...additionalVideoFiles]; console.log(` Syncing ${numAngles} camera angles to audio...`); - let syncResults; let syncOk = false; while (!syncOk) { try { @@ -440,11 +457,11 @@ async function main() { // Re-encodes synced video with -g 60 -movflags +faststart so Remotion can // seek frame-by-frame without decoding back to sparse keyframes (~5h → ~35min). // Skipped for proxy files: the sync output is already H.264 with -g 60. - if (resumeStep === 0 && mode === 1 && !usingProxies && (!redoStepId || redoStepId === 'optimize')) { + if (mode === 1 && !usingProxies && ((resumeStep === 0 && !redoStepId) || redoStepId === 'optimize')) { console.log('\n ── Optimise video for Remotion (keyframes) ──────────'); const { optimizeForRemotion } = await import('./optimize/optimize-for-remotion.js'); const pathsToOptimize = numAngles > 1 - ? syncResults.map(r => r.outputPath) + ? (syncResults ? syncResults.map(r => r.outputPath) : videoSrcsForRemotion.map(rel => path.join(cwd, 'public', rel))) : [path.join(syncOutputDir, 'synced-output.mp4')]; await optimizeForRemotion(pathsToOptimize); } @@ -667,6 +684,7 @@ async function main() { // ── STEP: Assign speakers (multi-speaker only) ──────────────────────────── const shouldBuildDoc = resumeStep < 3 && (!redoStepId || redoStepId === 'buildDoc'); + let speakerNamesJustAssigned = false; if (shouldBuildDoc && multiSpeaker) { console.log('\n ── Assign speakers ───────────────────────────────────'); const extraFlags = [ @@ -699,6 +717,7 @@ async function main() { // Regenerate doc with real speaker names in segment lines await spawnStep('npm', ['run', 'transcript:init', ...offsetArgs]); console.log(' ✓ Speaker names applied'); + speakerNamesJustAssigned = true; } catch (err) { console.error(` ✗ ${err.message}`); } @@ -726,12 +745,14 @@ async function main() { // 1/9 — Speaker names console.log(' ─── 1 / 9 Speaker names ─────────────────────────────'); - if (multiSpeaker) { + if (!multiSpeaker) { + console.log(' Single speaker — nothing to do here. (skip)'); + } else if (speakerNamesJustAssigned) { + console.log(' ✓ Already renamed above — skip.'); + } else { console.log(' Review the # SPEAKERS section at the top of the doc.'); console.log(' Confirm each label is the correct display name. To rename:'); console.log(' SPEAKER_00: Natasha'); - } else { - console.log(' Single speaker — nothing to do here. (skip)'); } await ask(' Press Enter to continue...'); @@ -1083,36 +1104,65 @@ async function main() { for (const speakerData of speakers) { const speaker = speakerData.speaker; - const candidates = speakerData.candidates; + let candidates = speakerData.candidates; console.log(`\n === ${speaker} ===`); - console.log(` Found ${candidates.length} valid candidate(s):\n`); - for (const c of candidates) { - const previewFullPath = path.join(candidatesDir, path.basename(c.previewPath)); - console.log(` [${c.index}] t=${c.timestamp}s → ${previewFullPath}`); - } + let selectedCandidate = null; + while (!selectedCandidate) { + console.log(` Found ${candidates.length} valid candidate(s):\n`); + for (const c of candidates) { + const previewFullPath = path.join(candidatesDir, path.basename(c.previewPath)); + console.log(` [${c.index}] t=${c.timestamp}s → ${previewFullPath}`); + } + + const validIndices = candidates.map(c => c.index).join(','); + const answer = (await ask(`\n Select frame [${validIndices}], or type "more" for additional candidates: `)).trim(); + + if (/^m(ore)?$/i.test(answer)) { + console.log(`\n Extracting more candidates for ${speaker}...`); + const excludeTimestamps = candidates.map(c => c.timestamp).join(','); + const indexStart = Math.max(...candidates.map(c => c.index)) + 1; + try { + await spawnStep('python3', [ + 'scripts/thumbnail/extract-speaker-candidates.py', + '--transcript', path.join(cwd, 'public', 'edit', 'transcript.json'), + '--camera-profiles', path.join(cwd, 'public', 'camera', 'camera-profiles.json'), + '--video', path.join(cwd, 'public', videoPath || 'sync/output/synced-output-1.mp4'), + '--output-dir', candidatesDir, + '--num-candidates', '3', + '--speakers', speaker, + '--exclude-timestamps', excludeTimestamps, + '--index-start', String(indexStart), + '--append', + ]); + const refreshed = await fs.readJson(candidatesPath); + const updatedSpeakerData = refreshed.speakers.find(s => s.speaker === speaker); + if (updatedSpeakerData) candidates = updatedSpeakerData.candidates; + else console.log(' ⚠ No additional candidates were found.'); + } catch (err) { + console.error(` ✗ Failed to extract more candidates: ${err.message}`); + } + console.log(''); + continue; + } - // Get user selection - let selectedIndex = null; - while (selectedIndex === null) { - const answer = (await ask(`\n Select frame [0-${candidates.length - 1}]: `)).trim(); const idx = parseInt(answer, 10); - if (!isNaN(idx) && idx >= 0 && idx < candidates.length) { - selectedIndex = idx; + const found = candidates.find(c => c.index === idx); + if (found) { + selectedCandidate = found; } else { - console.log(' Invalid selection. Please try again.'); + console.log(' Invalid selection. Please try again.\n'); } } - const selectedCandidate = candidates.find(c => c.index === selectedIndex); selections.push({ speaker, - selectedIndex, + selectedIndex: selectedCandidate.index, timestamp: selectedCandidate.timestamp, previewPath: selectedCandidate.previewPath, }); - console.log(` ✓ Selected: frame [${selectedIndex}] at t=${selectedCandidate.timestamp}s`); + console.log(` ✓ Selected: frame [${selectedCandidate.index}] at t=${selectedCandidate.timestamp}s`); } // Save selections