diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index e9de895..e601118 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -2,6 +2,17 @@ Notable changes to `@crafter/vcut`. Entries say what changed and, where it is not obvious, what measurement led to it. +## 0.26.0 + +### Added + +- **`verify --windows` now catches a restart whose attempts diverge at word three (#72).** A speaker who backs up mid-clause and starts the phrase over with a different destination leaves a shared prefix and a divergence: "para simplemente cifrar, para simplemente detectar". Six of one recording's annotated defects have this shape and every `RUN_LENGTH` scan in the codebase is blind to it, not because the width is wrong but because normalised, the only runs occurring twice are the **bigrams**. No trigram repeats at all, so `findRepeatedPhrases` returns zero findings on the real text. Dropping to a bare 2-word run is not the fix, and the reason is measurable on the issue's own named strings: the real restart prefixes score 0, 1, 1, 1, 1, 2, 2 content words and the false positives it names score 0, 1, 1, so the two distributions **overlap completely** and no content-word floor separates them. Scoring the grown span or the prefix plus both destinations was measured too and separates them no better (`voy a mostrar / cerrar` scores 3, higher than two of the real restarts). So `findRestarts` takes the escape `findStackedOpeners` already took for the identical problem, structure instead of a lexicon: the prefix must repeat and then diverge (`growMatch` already computes that divergence index as its loop's exit condition and discarded it; for this class that index is the answer), the abandoned attempt must be shorter than the prefix the two attempts share, and the prefix must carry a content word. No constant is picked: the bound on the abandoned attempt is the repeat's own width. Measured against the issue's cases, it fires on "la ia tendria" and "para simplemente" and stays silent on all three connectives it names (`para que`, `voy a`, `de normal`) and on the `le damos clic a Create / a Sign in` reuse `auto-cut.ts` forbids cutting. **Stated limit:** it does not fire on "abrimos la guia para vincular / abrimos la opcion para vincular", whose abandoned attempt runs three words against a two-word shared prefix. Catching that one needs a wider bound, and the corpus that measured the 8-to-64 false-positive figure is not in this repo, so the bound would be fitted to fixtures rather than derived. The honest boundary is documented at `RESTART_PROBE_LENGTH` rather than papered over. + +### Fixed + +- **A repetition cut spanned both occurrences, so removing it ate the words between them (#73).** The span ran from the first occurrence's first word to the SECOND occurrence's first word, which removes occurrence 1 plus everything spoken in between. That is wrong whenever the speaker got further into the first attempt than the repeated phrase itself, because what sits between the two readings is not only the trailing-off: it is the continuation the surviving sentence was going to complete. Measured while building a corroborated detection pipeline over one master, **5 of 5 repetition cuts ate real words**, one of them removing "cual es la diferencia" outright. The cut now ends at the **seam**, the widest measured gap between two consecutive words in that stretch, which is the micro-pause a speaker leaves when they stop one attempt and start another. No threshold is introduced: the seam is the largest gap present rather than the first gap over some minimum, so this asks where the break is rather than whether a break is big enough, and two readings butting straight up against each other fall back to the retake's own start, which is where the old span already ended and is correct when there is nothing in between to preserve. Verified by transcribing 4s around every junction in source and render: 9 junctions identical, 6 differing only by ASR variance (`click`/`clic`, `aprobar`/`a probar`), **0 words lost**. This shipped no damage yet because `MAX_FUMBLE_WORDS` is 0 and the pass declines nearly everything, which is exactly why the geometry is corrected now: #71 and #72 both raise the sensitivity, and whoever raises it further inherits a cutter that eats speech. +- **`repeatedPhrases` required a repetition to straddle two transcript lines, and found 0 of 12 known repetitions in production (#71).** `semantic.ts` skipped any run it had already seen inside the current line, on the reading that a stutter the transcript kept is one line saying a thing once, badly. That reading assumed a repetition spans lines, and whisper does not transcribe that way: measured across 639 windows over the full 417s of a real recording at seven widths from 4s to 32s, it returns **one cue per window 83% of the time** at the shipped 16s sweep width, so most repeats live inside a single line and were structurally unreachable. The detector found **0 of 12** known repetitions at every width, and the cause was never the window size. A real 16s cue it discarded, verbatim: "Podemos comenzar a probar el MCP en normal. Entonces, para usarlo, simplemente lo taggeamos, arroba normal. Entonces, para usarlo, lo taggeamos." Occurrences inside a line are now counted, which takes recall to **10 of 12** with false positives going 0 to 8, every one of them carrying 2 or more content words, so `MIN_CONTENT_WORDS` is still doing its own job and no threshold moved. The narrower guard the old rule was actually reaching for is kept: occurrences are counted non-overlapping, so "eh eh eh eh" is one hesitation seen through two sliding probes rather than two sayings of it. `count` now reports occurrences rather than lines, which is also what `survivingRepeats` needs, since it compares that number against occurrences in the rendered master's own text. `verify.ts`'s own `findRepeatedPhrases` never had this defect: it scans a window's raw text and already counts within it. + ## 0.25.0 ### Added diff --git a/packages/cli/package.json b/packages/cli/package.json index a8be6f6..8c326c2 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@crafter/vcut", - "version": "0.25.0", + "version": "0.26.0", "description": "Cut dead air out of a recording, reproducibly. Agent-first CLI over ffmpeg.", "type": "module", "license": "MIT", diff --git a/packages/cli/src/auto-cut.ts b/packages/cli/src/auto-cut.ts index 2f06b68..30c6134 100644 --- a/packages/cli/src/auto-cut.ts +++ b/packages/cli/src/auto-cut.ts @@ -62,8 +62,11 @@ * own annotation of that entry writes the fuller phrasing first and the tighter one second, and * every repetition entry on the list reads the same way: the first pass is the one that trails * off. Removing the later occurrence would keep the abandoned attempt and delete the landing, - * which destroys the good take. So the span cut runs from the first occurrence's first word to - * the second occurrence's first word, leaving the second reading whole. + * which destroys the good take. So the span cut runs from the first occurrence's first word to the + * SEAM between the abandoned attempt and the retake (#73, `seamBetween`), leaving the second + * reading whole along with anything the speaker said after abandoning: running it to the second + * occurrence's own start instead removed those words too, and measured over one master that ate + * real speech in 5 of 5 repetition cuts. * * Legitimate reuse is excluded by three tests, and the measurement said all three were needed. * #57's content-word floor is re-applied here rather than inherited (`mergeRepeats` folds in @@ -629,14 +632,77 @@ export const continuesTheSame = ( return between.length <= MAX_FUMBLE_WORDS } +/** + * The seam between an abandoned attempt and the retake that replaces it: the point the repetition + * cut ends at (#73). + * + * The cut used to run from the first occurrence's first word to the SECOND occurrence's first + * word, which removes occurrence 1 plus every word spoken between the two readings. That is wrong + * whenever the speaker got further into the first attempt than the repeated phrase itself, because + * what sits between the readings is not only the trailing-off: it is the continuation the surviving + * sentence was going to complete. Measured while building a corroborated detection pipeline over + * one master, **5 of 5 repetition cuts ate real words**, one of them removing "cual es la + * diferencia" outright. + * + * The seam is the widest measured gap between two consecutive words in that stretch, which is the + * micro-pause a speaker leaves when they stop one attempt and start another. Ending the cut there + * removes the abandoned attempt and leaves everything after it standing, so the retake arrives with + * the words it needs rather than with a hole in front of it. + * + * No threshold is introduced. The seam is chosen as the largest gap present rather than the first + * gap over some minimum duration, so this asks "where is the break" rather than "is this break big + * enough", and a stretch with no gap at all (two readings butting up against each other, the only + * shape `MAX_FUMBLE_WORDS = 0` admits today) falls back to the retake's own start, which is where + * the old span already ended and is correct when there is nothing in between to preserve. + * + * Verified by transcribing 4s around every junction in source and render after the change: 9 + * junctions identical, 6 differing only by ASR variance (`click`/`clic`, `aprobar`/`a probar`), + * 0 words lost. + */ +export const seamBetween = ( + words: Word[], + first: { startMs: number; endMs: number }, + second: { startMs: number; endMs: number }, +): number => { + // Everything spoken after the first reading and before the retake begins. With + // MAX_FUMBLE_WORDS at 0 this is empty today, which is exactly why #73 is latent rather than + // shipping: raising the sensitivity is what exposes the geometry. + const between = words + .filter((word) => word.startMs >= first.endMs && word.startMs < second.startMs) + .sort((left, right) => left.startMs - right.startMs) + if (between.length === 0) { + return second.startMs + } + // Walk the boundaries from the end of the abandoned attempt through to the retake, and take the + // widest one. The candidates include the gap before the first intervening word and the gap after + // the last, so a speaker who pauses immediately after abandoning is handled the same way as one + // who trails off through a word or two first. + let seamMs = second.startMs + let widestMs = -1 + let previousEndMs = first.endMs + for (const word of between) { + const gapMs = word.startMs - previousEndMs + if (gapMs > widestMs) { + widestMs = gapMs + seamMs = word.startMs + } + previousEndMs = Math.max(previousEndMs, word.endMs) + } + const trailingGapMs = second.startMs - previousEndMs + if (trailingGapMs > widestMs) { + seamMs = second.startMs + } + return seamMs +} + /** * Exact repeated runs, cutting the EARLIER occurrence and keeping the LATER one. * * See the header for why that direction and not the other: the second attempt is the speaker's - * completed one, and reversing this deletes the landing and keeps the fumble. The removed span - * runs from the first occurrence's first word to the second occurrence's first word, so everything - * between the two readings (the trailing-off, the "eh", the breath) goes with it and the second - * reading survives whole. + * completed one, and reversing this deletes the landing and keeps the fumble. The removed span runs + * from the first occurrence's first word to the SEAM between the abandoned attempt and the retake + * (#73, see `seamBetween`), not to the second occurrence's own start: ending it there removes the + * words the speaker said in the failed attempt that belong to the sentence that survives. * * Only `repeatedPhrases` is read. `discountedRepeats` is #57's content-word floor already having * decided a finding is connective tissue, and legitimate reuse spread across a render is excluded @@ -689,10 +755,12 @@ export const repetitionCuts = ( if (!continuesTheSame(finding.words, first, second)) { continue } - const measurement = `"${finding.phrase}" x${finding.count}, first reading ${(first.startMs / 1000).toFixed(2)}-${(first.endMs / 1000).toFixed(2)}s, second at ${(second.startMs / 1000).toFixed(2)}s, ${(gapMs / 1000).toFixed(2)}s apart; earlier removed, later kept` + // #73: the seam, not the retake's start. Everything after the abandoned attempt survives. + const seamMs = seamBetween(finding.words, first, second) + const measurement = `"${finding.phrase}" x${finding.count}, first reading ${(first.startMs / 1000).toFixed(2)}-${(first.endMs / 1000).toFixed(2)}s, second at ${(second.startMs / 1000).toFixed(2)}s, ${(gapMs / 1000).toFixed(2)}s apart; earlier removed to the seam at ${(seamMs / 1000).toFixed(2)}s, later kept` cuts.push({ startMs: first.startMs, - endMs: second.startMs, + endMs: seamMs, kind: 'repetition', instrument: 'repetition', measurement, diff --git a/packages/cli/src/commit.ts b/packages/cli/src/commit.ts index 0271d34..77cc99b 100644 --- a/packages/cli/src/commit.ts +++ b/packages/cli/src/commit.ts @@ -975,7 +975,7 @@ export const commitCommand = async (argv: string[]): Promise => { autoCutSummary = rebuilt.summary writeFileSync(args.edlPath, `${JSON.stringify(finalEdl, null, 2)}\n`) writeRound(roundDir, finalEdl, autoCutSummary) - finalRender = await runRender(finalEdl as Edl, renderOptions) + finalRender = await runRender(finalEdl as Edl, { ...renderOptions, allowExisting: true }) } } diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index b0241f5..02edfe3 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -121,6 +121,33 @@ export const parseFields = (args: string[]): string[] | null => { .filter((path) => path.length > 0) } +// Every global flag a command inherits without declaring it: the four output flags plus the +// two a command answers before parsing anything of its own. +const GLOBAL_FLAGS = ['--json', '--human', '--fields', '--jq', '--help', '--version'] + +// A flag a command does not read is a question it did not answer. `silences --threshold -80` +// ran, exited 0, and reported `thresholdDb: -30` back — the default, formatted exactly like a +// measurement, with the real flag named --noise. Nothing downstream could tell the difference +// between a threshold that was honoured and one that was never seen (#67). +// +// This is the same silent-acceptance bug requireJson and requireRawOutput each close for one +// flag, widened to the flags a command declares itself. Values are skipped rather than +// validated: a command's own parser owns what its flags mean, and this only asks whether the +// flag exists at all. A bare `-` or a negative number reads as a value, never as a flag, so +// `--noise -30` does not report -30 as unknown. +export const rejectUnknownFlags = (args: string[], known: string[], commandLabel: string): void => { + const allowed = new Set([...known, ...GLOBAL_FLAGS]) + const unknown = args.filter( + (arg) => arg.startsWith('--') && !allowed.has(arg.split('=')[0] as string), + ) + if (unknown.length > 0) { + const named = unknown.join(', ') + throw new UsageError( + `${commandLabel} does not take ${named}; see \`vcut ${commandLabel} --help\``, + ) + } +} + // --jq alongside --jq=, the same two forms --fields reads. Only the last flag // wins when repeated, matching how `value()` helpers across every command read a flag today. export const parseJqExpr = (args: string[]): string | null => { diff --git a/packages/cli/src/render-edl.ts b/packages/cli/src/render-edl.ts index 2015c18..d6985b8 100644 --- a/packages/cli/src/render-edl.ts +++ b/packages/cli/src/render-edl.ts @@ -1009,6 +1009,12 @@ export type RenderOptions = { // this CLI prints; a test swaps this in to capture the lines instead of the process's own // stderr. onProgressLine?: (line: string) => void + // A caller that renders the same round twice on purpose — commit folds its deterministic cuts + // into the EDL and renders again over the same path — is not the accident the existing-output + // guard protects against. The render still writes to a temp sibling and still has to pass + // probeOutput, so the file already on disk survives a failed second attempt: only the final + // rename replaces it. + allowExisting?: boolean } export type RenderResult = { @@ -1074,7 +1080,7 @@ export const runRender = async (edl: Edl, options: RenderOptions): Promise 0) { diff --git a/packages/cli/src/semantic.ts b/packages/cli/src/semantic.ts index 7598472..9f53e7d 100644 --- a/packages/cli/src/semantic.ts +++ b/packages/cli/src/semantic.ts @@ -731,6 +731,53 @@ export type DiscountedRepeat = { reason: string } +// The words of one line, normalised the way a repeat has to be compared: case and punctuation are +// delivery, not words. Same shape verify.ts's own `normalise` produces for the identical question. +const lineWords = (text: string): string[] => + text + .toLowerCase() + .replace(/[^\p{L}\p{N}\s]/gu, ' ') + .split(/\s+/) + .filter((word) => word.length > 0) + +// How many times a run occurs in one line's words, counting non-overlapping occurrences only. +// Overlap matters for runs built from repeated tokens: "a a a a" contains the run "a a a" at index +// 0 and again at index 1, which is one stutter seen through two probes rather than two sayings of +// it. verify.ts's `countNonOverlapping` already settled this for its own window scan and this is +// the same rule applied to a line. +const runOccurrences = (words: string[], start: number, runLength: number): number => { + const run = words.slice(start, start + runLength) + let count = 0 + let index = 0 + while (index + runLength <= words.length) { + if (run.every((word, offset) => words[index + offset] === word)) { + count += 1 + index += runLength + continue + } + index += 1 + } + return count +} + +// #71: a repetition used to have to straddle two transcript lines to be counted, because a run +// already seen inside the current line was skipped. Measured across 639 windows over 417s of real +// material at seven widths from 4s to 32s, that found 0 of 12 known repetitions at every width, and +// the cause was not the window: whisper returns ONE cue per window 83% of the time at the shipped +// 16s width, so most repeats live inside a single line and were structurally invisible. A real cue +// the detector discarded, verbatim: "Podemos comenzar a probar el MCP en normal. Entonces, para +// usarlo, simplemente lo taggeamos, arroba normal. Entonces, para usarlo, lo taggeamos." +// +// Counting occurrences rather than lines takes that to 10 of 12, with false positives going 0 to 8, +// every one of them at 2+ content words, so MIN_CONTENT_WORDS is still doing its own job. The old +// rule was written against a different failure ("a stutter the transcript already collapsed"), and +// non-overlapping counting is the narrower guard that actually addresses it: a stutter seen through +// two overlapping probes counts once, while a phrase genuinely said twice counts twice. +// +// `count` is therefore occurrences, not lines, which is also what `survivingRepeats` needs: it +// compares this number against occurrences of the phrase in the rendered master's own text, and the +// old line-count made a phrase said twice inside one line read as answered by a render that still +// said it twice. export const repeatedPhrases = ( lines: Line[], lang?: string, @@ -739,47 +786,44 @@ export const repeatedPhrases = ( discounted: DiscountedRepeat[] } => { const stopwords = stopwordsFor(lang) - const seen = new Map() + const seen = new Map() lines.forEach((line, index) => { - const words = line.text - .toLowerCase() - .replace(/[^\p{L}\p{N}\s]/gu, ' ') - .split(/\s+/) - .filter((word) => word.length > 0) + const words = lineWords(line.text) const local = new Set() for (let start = 0; start + RUN_LENGTH <= words.length; start += 1) { const run = words.slice(start, start + RUN_LENGTH).join(' ') - // A run repeating inside one line still counts once for that line, so a line is never - // reported as repeating itself through a stutter the transcript already collapsed. + // Once per run per line: the occurrences inside this line are counted in one pass rather than + // re-counted by every probe that slides over them. if (local.has(run)) { continue } local.add(run) - const where = seen.get(run) ?? [] - where.push(index) - seen.set(run, where) + const entry = seen.get(run) ?? { count: 0, lineIndexes: [] } + entry.count += runOccurrences(words, start, RUN_LENGTH) + entry.lineIndexes.push(index) + seen.set(run, entry) } }) const entries = [...seen.entries()] - .filter(([, where]) => where.length > 1) + .filter(([, entry]) => entry.count > 1) .sort( ([leftPhrase, left], [rightPhrase, right]) => - right.length - left.length || leftPhrase.localeCompare(rightPhrase), + right.count - left.count || leftPhrase.localeCompare(rightPhrase), ) const repeated: Array<{ phrase: string; count: number; lineIndexes: number[] }> = [] const discounted: DiscountedRepeat[] = [] - for (const [phrase, where] of entries) { + for (const [phrase, { count, lineIndexes }] of entries) { const contentWords = contentWordCount(phrase, stopwords) if (contentWords < MIN_CONTENT_WORDS) { discounted.push({ phrase, - count: where.length, - lineIndexes: where, + count, + lineIndexes, reason: `${contentWords} content word${contentWords === 1 ? '' : 's'}, below the ${MIN_CONTENT_WORDS} needed to read as a candidate retake rather than connective tissue`, }) continue } - repeated.push({ phrase, count: where.length, lineIndexes: where }) + repeated.push({ phrase, count, lineIndexes }) } return { repeated, discounted } } diff --git a/packages/cli/src/silences.ts b/packages/cli/src/silences.ts index 589ed88..57e5a2d 100644 --- a/packages/cli/src/silences.ts +++ b/packages/cli/src/silences.ts @@ -23,7 +23,16 @@ import { resolve } from 'node:path' import { parseSilenceLog, probeDurationMs, type SilenceCandidate } from './detect.ts' import { run } from './exec.ts' -import { duration, emitJson, heading, line, type Mode, resolveMode, UsageError } from './output.ts' +import { + duration, + emitJson, + heading, + line, + type Mode, + rejectUnknownFlags, + resolveMode, + UsageError, +} from './output.ts' const HELP = `vcut silences - speech and silence blocks over a range, at a resolution you choose @@ -168,6 +177,7 @@ const parseCli = (args: string[]): CliOptions => { } return parsed } + rejectUnknownFlags(args, ['--from', '--to', '--noise', '--min'], 'silences') return { input: resolve(input), fromS: numeric('--from', null), diff --git a/packages/cli/src/verify.ts b/packages/cli/src/verify.ts index 0ae9bd3..f8dad98 100644 --- a/packages/cli/src/verify.ts +++ b/packages/cli/src/verify.ts @@ -371,6 +371,51 @@ export const findRepeatedPhrases = ( // is not this shape, because OPENERS compares only sentences that are adjacent. const OPENER_LENGTH = 2 +/** + * The restart defect (#72): a speaker who backs up mid-clause and starts the phrase over with a + * different destination. "abrimos la guia para vincular, abrimos la opcion para vincular" at ~91.5s + * of one recording is the shape exactly, and six of that material's annotated defects share it. + * + * It is invisible to every RUN_LENGTH scan in this codebase, and not because the width is wrong. + * Normalised, the only runs occurring twice there are the BIGRAMS `abrimos la` and `para vincular`; + * no trigram repeats at all, so `findRepeatedPhrases` returns zero findings on the real text. + * + * Why this is a separate structural check and not `RUN_LENGTH = 2`. Dropping the width wholesale + * takes false positives from 8 to 64, nearly all of them connectives, and the content-word floor + * that rescues the 3-word scan cannot rescue this one. Measured on the issue's OWN named strings: + * the real restart prefixes score 0, 1, 1, 1, 1, 2, 2 content words (`es muy` 0, `abrimos la` 1, + * `para vincular` 1, `la ia` 1, `para simplemente` 1, `ia tendria` 2, `voy a iterar` 2) and the + * false positives it names score 0, 1, 1 (`para que` 0, `voy a` 1, `de normal` 1). The two + * distributions overlap completely, so no floor on a 2-word run separates them. Scoring the grown + * span or the shared prefix plus both destinations was measured too and does not separate them + * either: `voy a mostrar / cerrar` scores 3, higher than two of the real restarts. + * + * So this takes the same escape `findStackedOpeners` already took for the identical problem (a + * 2-word unit that is noise anywhere, made safe by structure rather than by a lexicon): not "these + * two words recur somewhere", but "the speaker said them, went somewhere, and said them again + * before getting as far as they had gone the first time". Three requirements, all structural: + * + * 1. The shared prefix repeats and then DIVERGES. `growMatch` already computes that divergence + * index as its own loop exit condition and discards it; for this class that index is the answer. + * Two attempts that never diverge are one phrase said twice, which `findRepeatedPhrases` owns. + * 2. The abandoned attempt is shorter than the prefix the two attempts share. This is the + * "backed up" part and it is what excludes reuse: a speaker who completes a clause and reuses + * its opening later leaves the whole completed clause in between. No constant is picked here, + * the bound is the repeat's own width, the same self-referential shape `MAX_RETAKE_GAP_MS` + * derives from the sweep's own window rather than inventing a number. + * 3. The prefix carries at least one content word, so a pair of bare connectives repeating + * ("para que ... para que") is not a restart on structure alone. + * + * Measured against the issue's own cases: fires on "la ia tendria / la ia tendria acceso" and + * "para simplemente cifrar / para simplemente detectar", stays silent on all three false positives + * it names (`para que`, `voy a`, `de normal`) and on the `le damos clic a Create / a Sign in` + * reuse `auto-cut.ts` explicitly forbids cutting. It does NOT fire on "abrimos la guia para + * vincular / abrimos la opcion para vincular", whose abandoned attempt (3 words) runs longer than + * its shared prefix (2 words); catching that one needs a bound this evidence cannot justify, and + * the honest boundary is stated here rather than fitted. See the changelog entry for #72. + */ +const RESTART_PROBE_LENGTH = 2 + /** * The stacked-filler defect: two consecutive sentences inside one window that open on the exact * same short run of words. "Y bueno, eso es todo. Y bueno, ya para cerrar" is this shape @@ -412,6 +457,75 @@ export const findStackedOpeners = (windows: Window[]): RepeatedPhrase[] => { return result.sort((left, right) => left.windowStartMs - right.windowStartMs) } +/** + * Restarts found by scanning each window's own text (#72). See RESTART_PROBE_LENGTH for the + * measurement that decided the three requirements and for what this deliberately does not catch. + * + * Reported as a `RepeatedPhrase` carrying the shared prefix, so it merges through `mergeRepeats` + * with the other two detectors and reaches the gate and `commit` by the path that already exists, + * rather than as a fourth list every consumer has to learn about. + */ +export const findRestarts = (windows: Window[], lang?: string): RepeatedPhrase[] => { + const stopwords = stopwordsFor(lang) + const found: RepeatedPhrase[] = [] + for (const window of windows) { + const words = normalise(window.text).split(' ').filter(Boolean) + const positions = new Map() + for (let start = 0; start + RESTART_PROBE_LENGTH <= words.length; start += 1) { + const run = words.slice(start, start + RESTART_PROBE_LENGTH).join(' ') + const where = positions.get(run) ?? [] + where.push(start) + positions.set(run, where) + } + const seen = new Set() + for (const where of positions.values()) { + for (let index = 0; index < where.length - 1; index += 1) { + const left = where[index] + const right = where[index + 1] + if (left === undefined || right === undefined) { + continue + } + // Grow forward only, to the divergence index. Growing left as well would fold this into + // the span `findRepeatedPhrases` already reports; what identifies a restart is where the + // two attempts PART, which is exactly where this loop stops. + let end = left + RESTART_PROBE_LENGTH + let matchEnd = right + RESTART_PROBE_LENGTH + while (end < right && matchEnd < words.length && words[end] === words[matchEnd]) { + end += 1 + matchEnd += 1 + } + const diverges = end < right && matchEnd < words.length && words[end] !== words[matchEnd] + if (!diverges) { + continue + } + const prefix = words.slice(left, end) + // The abandoned attempt: what the speaker got through before backing up. Shorter than the + // prefix means they never got as far as they had already agreed on, which is a restart; + // longer means the first attempt completed something, which is reuse. + const abandoned = right - end + if (abandoned >= prefix.length) { + continue + } + if (!prefix.some((word) => contentWordCount(word, stopwords) > 0)) { + continue + } + const phrase = prefix.join(' ') + if (seen.has(phrase)) { + continue + } + seen.add(phrase) + found.push({ + phrase, + count: 2, + windowStartMs: window.startMs, + windowEndMs: window.endMs, + }) + } + } + } + return found.sort((left, right) => left.windowStartMs - right.windowStartMs) +} + // A stretched vowel or an unfinished word at the very edge of what a window heard: the last // token butts against the boundary with no closing punctuation and no gap, which is what a // clause cut off mid-word looks like in prose. Conservative on purpose (this reports a @@ -692,7 +806,11 @@ export const runVerifyWindows = async ( ? undefined : parseSrt(readFileSync(cachedTranscriptPath, 'utf8')).words - const mergedRepeats = mergeRepeats(phrases.repeated, findStackedOpeners(windows)) + const mergedRepeats = mergeRepeats( + phrases.repeated, + findStackedOpeners(windows), + findRestarts(windows, lang), + ) const repeated = cachedWords === undefined ? mergedRepeats : corroborateRepeats(mergedRepeats, cachedWords) const truncated = findTruncatedEdges(windows) diff --git a/packages/cli/tests/auto-cut.test.ts b/packages/cli/tests/auto-cut.test.ts index b69e73e..83b916c 100644 --- a/packages/cli/tests/auto-cut.test.ts +++ b/packages/cli/tests/auto-cut.test.ts @@ -9,6 +9,7 @@ import { nonspeechCuts, planAutoCuts, repetitionCuts, + seamBetween, silenceCuts, snapToWords, } from '../src/auto-cut.ts' @@ -173,6 +174,79 @@ describe('continuesTheSame', () => { }) }) +// #73: the repetition cut used to run to the SECOND occurrence's start, which removes every word +// spoken between the two readings. Those words are not only the trailing-off: when the speaker got +// further into the first attempt than the repeated phrase itself, they are the continuation the +// surviving sentence was going to complete. Measured over one master, 5 of 5 repetition cuts ate +// real words, one removing "cual es la diferencia" outright. +// +// These test seamBetween directly because MAX_FUMBLE_WORDS is 0, so repetitionCuts declines every +// finding with anything between the readings. That is precisely why #73 is latent rather than +// shipping, and why the geometry has to be right BEFORE #71 and #72 raise the sensitivity. +describe('seamBetween', () => { + test('ends at the micro-pause, so the words after the abandoned attempt survive (#73)', () => { + // The defect verbatim from the issue: a cut that removed "cual es la diferencia". The speaker + // abandons after "y bueno", pauses, then says the words that belong to the surviving sentence, + // then retakes. Ending at the retake's own start (the old geometry) deletes all four. + const words = [ + word('y', 0, 100), + word('bueno', 100, 300), + word('cual', 900, 1100), + word('es', 1110, 1200), + word('la', 1210, 1290), + word('diferencia', 1300, 1600), + word('y', 1620, 1720), + word('bueno', 1720, 1900), + ] + const seam = seamBetween(words, { startMs: 0, endMs: 300 }, { startMs: 1620, endMs: 1900 }) + // The widest boundary is 300 -> 900, so the cut ends at 900 rather than at the retake's 1620. + expect(seam).toBe(900) + // Which is the whole point: the four words the old span ate are on the keeping side now. + expect(words.filter((entry) => entry.startMs >= seam).map((entry) => entry.text)).toEqual([ + 'cual', + 'es', + 'la', + 'diferencia', + 'y', + 'bueno', + ]) + }) + + test('ends before the intervening words when the pause precedes them', () => { + // The speaker stops dead after the abandoned attempt, then says the retake's own run-up. + // The widest boundary is 300 -> 1200, so only the abandoned attempt goes. + const words = [ + word('y', 0, 100), + word('bueno', 100, 300), + word('cual', 1200, 1400), + word('es', 1410, 1500), + word('y', 1520, 1620), + word('bueno', 1620, 1800), + ] + const seam = seamBetween(words, { startMs: 0, endMs: 300 }, { startMs: 1520, endMs: 1800 }) + expect(seam).toBe(1200) + // The words after the seam are kept, which is the whole point of #73. + expect(words.filter((entry) => entry.startMs >= seam).map((entry) => entry.text)).toEqual([ + 'cual', + 'es', + 'y', + 'bueno', + ]) + }) + + test('falls back to the retake start when the readings are adjacent', () => { + // Nothing between them, so there is nothing to preserve and the old span was already right. + const words = [word('si', 0, 100), word('si', 800, 900)] + expect(seamBetween(words, { startMs: 0, endMs: 100 }, { startMs: 800, endMs: 900 })).toBe(800) + }) + + test('never returns a seam past the retake', () => { + const words = [word('a', 0, 100), word('b', 200, 300), word('a', 400, 500)] + const seam = seamBetween(words, { startMs: 0, endMs: 100 }, { startMs: 400, endMs: 500 }) + expect(seam).toBeLessThanOrEqual(400) + }) +}) + describe('repetitionCuts', () => { const retake = [ word('si', 0, 100), @@ -204,10 +278,11 @@ describe('repetitionCuts', () => { ) expect(cuts).toHaveLength(1) expect(cuts[0]?.startMs).toBe(0) - // Ends where the second reading begins, so the second reading survives whole. + // Nothing is spoken between the two readings here, so the seam IS the second reading's start + // and the span is unchanged by #73. The second reading survives whole either way. expect(cuts[0]?.endMs).toBe(800) expect(cuts[0]?.kind).toBe('repetition') - expect(cuts[0]?.measurement).toContain('earlier removed, later kept') + expect(cuts[0]?.measurement).toContain('later kept') }) test('declines a repeat below the content-word floor (#57)', () => { diff --git a/packages/cli/tests/output.test.ts b/packages/cli/tests/output.test.ts index e03a7d4..5494ffd 100644 --- a/packages/cli/tests/output.test.ts +++ b/packages/cli/tests/output.test.ts @@ -13,6 +13,7 @@ import { parseFields, parseJqExpr, projectFields, + rejectUnknownFlags, requireHumanOnly, requireJson, requireRawOutput, @@ -470,3 +471,57 @@ describe('classifierStatus', () => { expect(classifierStatus([false, false]).detail).toContain('optional') }) }) + +// Issue #67: `silences --threshold -80` ran, exited 0, and reported thresholdDb -30 — the +// default, shaped exactly like a measurement. The real flag is --noise. +describe('rejectUnknownFlags', () => { + const known = ['--from', '--to', '--noise', '--min'] + + test('rejects a flag the command does not read, naming it and the help', () => { + expect(() => rejectUnknownFlags(['--threshold', '-80'], known, 'silences')).toThrow( + 'silences does not take --threshold', + ) + }) + + test('names every unknown flag at once rather than one per run', () => { + try { + rejectUnknownFlags(['--threshold', '-80', '--min-silence', '800'], known, 'silences') + throw new Error('expected a UsageError') + } catch (error) { + expect((error as Error).message).toContain('--threshold, --min-silence') + } + }) + + test('accepts the flags the command declares', () => { + expect(() => + rejectUnknownFlags(['--from', '0', '--noise', '-80'], known, 'silences'), + ).not.toThrow() + }) + + // The trap this helper must not fall into: a negative dB value reads as a token starting + // with '-', not '--', so --noise -30 must not report -30 as an unknown flag. + test('reads a negative value as a value, not as a flag', () => { + expect(() => rejectUnknownFlags(['--noise', '-30'], known, 'silences')).not.toThrow() + }) + + test('accepts the global flags a command inherits without declaring them', () => { + expect(() => + rejectUnknownFlags( + ['--json', '--fields', 'blocks', '--jq', '.', '--help'], + known, + 'silences', + ), + ).not.toThrow() + }) + + test('reads --flag=value, the form parseFields and parseJqExpr already accept', () => { + expect(() => rejectUnknownFlags(['--noise=-80'], known, 'silences')).not.toThrow() + expect(() => rejectUnknownFlags(['--threshold=-80'], known, 'silences')).toThrow('--threshold') + }) + + test('leaves the positional media path alone', () => { + expect(() => + rejectUnknownFlags(['/tmp/master.wav', '--noise', '-80'], known, 'silences'), + ).not.toThrow() + }) +}) diff --git a/packages/cli/tests/render-edl.test.ts b/packages/cli/tests/render-edl.test.ts index c93c32d..2859adb 100644 --- a/packages/cli/tests/render-edl.test.ts +++ b/packages/cli/tests/render-edl.test.ts @@ -1,6 +1,14 @@ import { afterAll, beforeAll, describe, expect, test } from 'bun:test' import { createHash } from 'node:crypto' -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { + existsSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + statSync, + writeFileSync, +} from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { run } from '../src/exec.ts' @@ -828,6 +836,49 @@ describe.if(hasFfmpeg)('runRender progress streaming', () => { expect(result.status).toBe('rendered') expect(lines).toHaveLength(0) }) + + // Issue #66: commit renders a round, folds its deterministic cuts into the EDL, and renders + // again over the same path. The existing-output guard refused the second render, so master.wav + // never carried the auto-cuts while edl.json said it did. The guard stays for every other + // caller; only a caller that means it opts out. + test('a second render over the same path is refused by default', async () => { + const outputPath = join(workDir, 'twice-guarded.wav') + const options = { mode: 'preview' as const, dryRun: false, audioOnly: true, quiet: true } + const first = await runRender(fixtureEdl(outputPath), { ...options, outputPath }) + expect(existsSync(first.outputPath)).toBe(true) + expect(runRender(fixtureEdl(outputPath), { ...options, outputPath })).rejects.toThrow( + 'output already exists', + ) + }) + + test('allowExisting lets the same round render again over its own output', async () => { + const outputPath = join(workDir, 'twice-allowed.wav') + const options = { mode: 'preview' as const, dryRun: false, audioOnly: true, quiet: true } + await runRender(fixtureEdl(outputPath), { ...options, outputPath }) + const second = await runRender(fixtureEdl(outputPath), { + ...options, + outputPath, + allowExisting: true, + }) + expect(second.status).toBe('rendered') + expect(existsSync(outputPath)).toBe(true) + }) + + // The reason allowExisting is safe: ffmpeg writes to a temp sibling and only a render that + // passes probeOutput is renamed into place. A second render that fails leaves the first one + // standing rather than a hole where the good file was. + test('a failed second render leaves the first file intact', async () => { + const outputPath = join(workDir, 'twice-failed.wav') + const options = { mode: 'preview' as const, dryRun: false, audioOnly: true, quiet: true } + await runRender(fixtureEdl(outputPath), { ...options, outputPath }) + const before = statSync(outputPath) + const broken = fixtureEdl(outputPath) + broken.sources[0].path = join(workDir, 'this-source-does-not-exist.mp4') + expect(runRender(broken, { ...options, outputPath, allowExisting: true })).rejects.toThrow() + const after = statSync(outputPath) + expect(after.size).toBe(before.size) + expect(readdirSync(workDir).filter((name) => name.includes('.partial-'))).toHaveLength(0) + }) }) // Issue #31: render never called resolveMode and always emitted JSON, so --human on render was diff --git a/packages/cli/tests/semantic.test.ts b/packages/cli/tests/semantic.test.ts index e7300ed..5f9313f 100644 --- a/packages/cli/tests/semantic.test.ts +++ b/packages/cli/tests/semantic.test.ts @@ -474,10 +474,41 @@ describe('repeatedPhrases', () => { expect(repeated.map((entry) => entry.phrase)).toContain('un honor grande') }) - // A stutter the transcript kept is one line saying a thing once, badly. Counting it as a - // repetition would put an entry in front of a reader for every hesitation in the file. - test('does not report a line as repeating itself', () => { - expect(repeatedPhrases([line(0, 'un honor grande un honor grande')]).repeated).toEqual([]) + // #71 reversed this. It used to assert that a line never repeats itself, on the reading that a + // stutter the transcript kept is one line saying a thing once, badly. Measured on real material + // that assertion cost the detector everything it was for: whisper returns one cue per window 83% + // of the time at the shipped 16s sweep width, so a repetition mostly lives INSIDE one line, and + // requiring it to straddle two found 0 of 12 known repetitions across 639 windows at every width + // from 4s to 32s. A line saying "un honor grande" twice is the defect, not a hesitation. + test('reports a phrase a single line says twice (#71)', () => { + const { repeated } = repeatedPhrases([line(0, 'un honor grande un honor grande')]) + const entry = repeated.find((item) => item.phrase === 'un honor grande') + expect(entry?.count).toBe(2) + expect(entry?.lineIndexes).toEqual([0]) + }) + + // The issue's own evidence, verbatim from a 16s cue whisper transcribed correctly and the + // detector discarded: one cue, one line, the restart plainly in it. + test('finds the repeat in the single 16s cue #71 measured', () => { + const { repeated } = repeatedPhrases([ + line( + 0, + 'Podemos comenzar a probar el MCP en normal. Entonces, para usarlo, simplemente lo taggeamos, arroba normal. Entonces, para usarlo, lo taggeamos.', + ), + ]) + expect(repeated.map((entry) => entry.phrase)).toContain('entonces para usarlo') + }) + + // What the old rule was actually right about, kept by counting non-overlapping occurrences + // instead of by refusing intra-line repeats wholesale: "eh eh eh eh" contains the run "eh eh eh" + // at index 0 and again at index 1, which is one hesitation seen through two sliding probes, not + // two sayings of it. + test('counts a run built from repeated tokens once per non-overlapping occurrence', () => { + const { repeated, discounted } = repeatedPhrases([line(0, 'bueno eh eh eh eh bueno')]) + // One occurrence, so it never becomes a finding at all: nothing to report and nothing to + // discount, which is the hesitation case the old rule was reaching for. + expect(repeated).toEqual([]) + expect(discounted.map((entry) => entry.phrase)).not.toContain('eh eh eh') }) }) diff --git a/packages/cli/tests/verify.test.ts b/packages/cli/tests/verify.test.ts index da475ff..4f4fe0b 100644 --- a/packages/cli/tests/verify.test.ts +++ b/packages/cli/tests/verify.test.ts @@ -9,6 +9,7 @@ import { defaultConcurrency, findAnomalies, findRepeatedPhrases, + findRestarts, findStackedOpeners, findTruncatedEdges, mergeRepeats, @@ -291,6 +292,65 @@ describe('findStackedOpeners', () => { }) }) +// #72: a speaker who backs up mid-clause and restarts with a different destination. The 3-word +// scan cannot see it, and not because the width is wrong: normalised, the only runs occurring +// twice in this shape are bigrams, so no trigram repeats at all. +describe('findRestarts', () => { + test('catches a restart whose attempts diverge at word three', () => { + const windows = [ + window(0, 16_000, 'para simplemente cifrar, para simplemente detectar con asteriscos'), + ] + // The measurement that makes this a separate detector: the shipped 3-word scan returns + // nothing at all on this text, so the finding is new rather than a duplicate reported twice. + expect(findRepeatedPhrases(windows, 'es').repeated).toEqual([]) + const restarts = findRestarts(windows, 'es') + expect(restarts).toHaveLength(1) + expect(restarts[0]?.phrase).toBe('para simplemente') + }) + + test('catches the "la ia tendria" restart from the annotated list', () => { + const restarts = findRestarts([window(0, 16_000, 'la ia tendria, la ia tendria acceso')], 'es') + expect(restarts.map((entry) => entry.phrase)).toContain('ia tendria') + }) + + // The three connectives #72 names as the bulk of the 8-to-64 false-positive explosion a bare + // RUN_LENGTH=2 produces. Structure refuses all three; a content-word floor cannot, since + // "voy a" and "de normal" score exactly what several of the real restarts score. + test('stays silent on the connectives a bare 2-word run would flood with', () => { + for (const text of [ + 'no se si para que sirve esto pero bueno para que veas', + 'voy a mostrar una cosa y despues voy a cerrar', + 'el mcp de normal responde bien y el modo de normal tambien', + ]) { + expect(findRestarts([window(0, 16_000, text)], 'es')).toEqual([]) + } + }) + + // The class #63 explicitly forbids cutting: an opening reused for a different button, with the + // first clause COMPLETED in between. The abandoned attempt runs longer than the shared prefix, + // which is what tells reuse from a restart without any picked constant. + test('refuses legitimate reuse that completes its first clause', () => { + const windows = [ + window(0, 16_000, 'le damos clic aqui a open chatgpt, eso nos lleva, le damos clic a plus'), + ] + expect(findRestarts(windows, 'es')).toEqual([]) + }) + + // Two attempts that never part are one phrase said twice, which findRepeatedPhrases owns. + // Requiring divergence is what keeps the two detectors from reporting the same finding. + test('does not fire when the two readings never diverge', () => { + const windows = [window(0, 16_000, 'reciben un poema mio reciben un poema mio')] + expect(findRestarts(windows, 'es')).toEqual([]) + expect(findRepeatedPhrases(windows, 'es').repeated.length).toBeGreaterThan(0) + }) + + test('needs a content word in the shared prefix', () => { + // "de la" is two stopwords, so the shared prefix carries nothing to restart, whatever the + // two attempts diverge into. + expect(findRestarts([window(0, 16_000, 'de la casa de la mesa')], 'es')).toEqual([]) + }) +}) + describe('mergeRepeats', () => { const entry = (phrase: string, windowStartMs: number) => ({ phrase,