From ae6ff7803f419e233498ea22afa8528c9b9e8108 Mon Sep 17 00:00:00 2001 From: Jonathan Locala Date: Thu, 23 Jul 2026 09:48:25 -0400 Subject: [PATCH 1/8] fix(media): isSilenceType sweep + filler double-count (review findings) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a shared isSilenceType() helper and sweeps every site that compared against 'silence' alone — silence-newline entries were counted as spoken words in stats, unsplittable, given the wrong editor modal, polluting filler counts, and copied into the clipboard as [Ns] chips. Also collapses the filler matchCounts/durations double-count: DEFAULT_FILLER_WORDS entries were incremented once in a defaults loop and again in the custom-word line, doubling both occurrence counts and time-saved estimates. Fixes findings 3-7 and 16 of mieweb/ui#323. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/components/MediaEditor/MediaEditor.tsx | 7 ++- .../MediaEditor/WordEditorModal.tsx | 3 +- src/components/TranscriptView/transcript.ts | 8 +++ src/hooks/useTranscriptEdits.ts | 53 ++++++------------- 4 files changed, 30 insertions(+), 41 deletions(-) diff --git a/src/components/MediaEditor/MediaEditor.tsx b/src/components/MediaEditor/MediaEditor.tsx index 1082d3e4..133434c1 100644 --- a/src/components/MediaEditor/MediaEditor.tsx +++ b/src/components/MediaEditor/MediaEditor.tsx @@ -22,7 +22,10 @@ import type { PlaybackSegment, PlaybackSpeed, } from '../TranscriptView/transcript'; -import { PLAYBACK_SPEEDS } from '../TranscriptView/transcript'; +import { + PLAYBACK_SPEEDS, + isSilenceType, +} from '../TranscriptView/transcript'; import { useTranscriptEdits } from '../../hooks/useTranscriptEdits'; import { MediaPlayer, @@ -580,7 +583,7 @@ export const MediaEditor = React.forwardRef( .filter((ew): ew is EditableWord => ew !== undefined && !ew.deleted); if (selectedWords.length === 0) return; const selectedText = selectedWords - .filter((ew) => ew.word.wordType !== 'silence') + .filter((ew) => !isSilenceType(ew.word.wordType)) .map((ew) => ew.word.text) .join(' '); e.preventDefault(); diff --git a/src/components/MediaEditor/WordEditorModal.tsx b/src/components/MediaEditor/WordEditorModal.tsx index 77e50038..3eff62b3 100644 --- a/src/components/MediaEditor/WordEditorModal.tsx +++ b/src/components/MediaEditor/WordEditorModal.tsx @@ -17,6 +17,7 @@ import { } from '../Modal'; import { Button } from '../Button'; import { Input } from '../Input'; +import { isSilenceType } from '../TranscriptView/transcript'; import type { EditableWord } from '../TranscriptView/transcript'; export interface WordEditorModalProps { @@ -53,7 +54,7 @@ export const WordEditorModal: React.FC = ({ const [error, setError] = React.useState(null); const inputRef = React.useRef(null); - const isSilence = editableWord?.word.wordType === 'silence'; + const isSilence = isSilenceType(editableWord?.word.wordType); const durationMs = editableWord ? editableWord.word.endMs - editableWord.word.startMs : 0; diff --git a/src/components/TranscriptView/transcript.ts b/src/components/TranscriptView/transcript.ts index e1924f96..6dd559f3 100644 --- a/src/components/TranscriptView/transcript.ts +++ b/src/components/TranscriptView/transcript.ts @@ -11,6 +11,14 @@ /** Type of transcript word - 'word' for spoken content, 'silence' for detected gaps, 'silence-newline' for longer pauses */ export type WordType = 'word' | 'silence' | 'silence-newline'; +/** True for BOTH silence pseudo-word types ('silence' and 'silence-newline'). + * Use this instead of comparing against 'silence' directly — checking only one + * of the two types is the root cause of a whole family of review findings + * (stats, splitting, filler analysis, clipboard copy). */ +export function isSilenceType(wordType: WordType | undefined): boolean { + return wordType === 'silence' || wordType === 'silence-newline'; +} + export interface TranscriptWord { text: string; startMs: number; diff --git a/src/hooks/useTranscriptEdits.ts b/src/hooks/useTranscriptEdits.ts index 78c5d621..1307fc79 100644 --- a/src/hooks/useTranscriptEdits.ts +++ b/src/hooks/useTranscriptEdits.ts @@ -20,6 +20,7 @@ import type { PlaybackSpeed, SpeedMarker, } from '../components/TranscriptView/transcript'; +import { isSilenceType } from '../components/TranscriptView/transcript'; /** Default filler words offered for removal */ export const DEFAULT_FILLER_WORDS = [ @@ -565,7 +566,7 @@ export function useTranscriptEdits( const splitSilence = useCallback( (index: number, durationsSec: number[]) => { const ew = editedWords[index]; - if (!ew || ew.word.wordType !== 'silence') return; + if (!ew || !isSilenceType(ew.word.wordType)) return; pushUndo(); @@ -632,11 +633,7 @@ export function useTranscriptEdits( if (ew.deleted) return ew; // Check if it's a silence that should be removed - if ( - (ew.word.wordType === 'silence' || - ew.word.wordType === 'silence-newline') && - silenceThresholdMs !== null - ) { + if (isSilenceType(ew.word.wordType) && silenceThresholdMs !== null) { const durationMs = ew.word.endMs - ew.word.startMs; if (durationMs > silenceThresholdMs) { return { ...ew, deleted: true }; @@ -645,10 +642,7 @@ export function useTranscriptEdits( } // Check if it's a filler word that should be removed - if ( - ew.word.wordType !== 'silence' && - ew.word.wordType !== 'silence-newline' - ) { + if (!isSilenceType(ew.word.wordType)) { const wordText = normalizeWordText(ew.word.text); if (fillerSet.has(wordText)) { return { ...ew, deleted: true }; @@ -779,16 +773,16 @@ export function useTranscriptEdits( const stats = useMemo(() => { // Count active (non-deleted) words (excluding silences for word count) const activeWordCount = editedWords.filter( - (ew) => !ew.deleted && ew.word.wordType !== 'silence' + (ew) => !ew.deleted && !isSilenceType(ew.word.wordType) ).length; const activeSilenceCount = editedWords.filter( - (ew) => !ew.deleted && ew.word.wordType === 'silence' + (ew) => !ew.deleted && isSilenceType(ew.word.wordType) ).length; const deletedWordCount = editedWords.filter( - (ew) => ew.deleted && ew.word.wordType !== 'silence' + (ew) => ew.deleted && !isSilenceType(ew.word.wordType) ).length; const deletedSilenceCount = editedWords.filter( - (ew) => ew.deleted && ew.word.wordType === 'silence' + (ew) => ew.deleted && isSilenceType(ew.word.wordType) ).length; // Calculate edited duration (sum of non-deleted words/silences, adjusted for speed markers) @@ -813,34 +807,21 @@ export function useTranscriptEdits( // Occurrence counts per filler (and every active word, for custom fillers) const matchCounts = new Map(); for (const ew of editedWords) { - if (ew.deleted || ew.word.wordType === 'silence') continue; + if (ew.deleted || isSilenceType(ew.word.wordType)) continue; const wordText = normalizeWordText(ew.word.text); - for (const filler of DEFAULT_FILLER_WORDS) { - if (wordText === filler) { - matchCounts.set(filler, (matchCounts.get(filler) || 0) + 1); - } - } - // Also check for the exact word in case it's a custom filler + // One increment per occurrence — DEFAULT_FILLER_WORDS entries were + // previously counted twice (once in a defaults loop, once here). matchCounts.set(wordText, (matchCounts.get(wordText) || 0) + 1); } // Total duration per filler word type const durations = new Map(); for (const ew of editedWords) { - if ( - ew.deleted || - ew.word.wordType === 'silence' || - ew.word.wordType === 'silence-newline' - ) - continue; + if (ew.deleted || isSilenceType(ew.word.wordType)) continue; const wordText = normalizeWordText(ew.word.text); const wordDuration = ew.word.endMs - ew.word.startMs; - for (const filler of DEFAULT_FILLER_WORDS) { - if (wordText === filler) { - durations.set(filler, (durations.get(filler) || 0) + wordDuration); - } - } - // Also track duration for the exact word in case it's a custom filler + // One accumulation per occurrence — DEFAULT_FILLER_WORDS entries were + // previously accumulated twice, inflating time-saved estimates 2x. durations.set(wordText, (durations.get(wordText) || 0) + wordDuration); } @@ -851,11 +832,7 @@ export function useTranscriptEdits( let durationMs = 0; for (const ew of editedWords) { if (ew.deleted) continue; - if ( - ew.word.wordType !== 'silence' && - ew.word.wordType !== 'silence-newline' - ) - continue; + if (!isSilenceType(ew.word.wordType)) continue; const silenceDuration = ew.word.endMs - ew.word.startMs; if (silenceDuration > thresholdMs) { count++; From b5b547134a8fe012b9a0d4230d236ebc1dae9408 Mon Sep 17 00:00:00 2001 From: Jonathan Locala Date: Thu, 23 Jul 2026 09:49:23 -0400 Subject: [PATCH 2/8] =?UTF-8?q?fix(media):=20edit-state=20correctness=20?= =?UTF-8?q?=E2=80=94=20threshold=20keyspace,=20undo=20baseline,=20hasEdits?= =?UTF-8?q?=20init?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on the edit-state lifecycle: - setSilenceThresholds saved deletions keyed by position in the old silence-inserted array but restored by word ordinal — a keyspace mismatch that applied deletions to the WRONG words after changing thresholds. Both sides now use the word ordinal. - undo's isOriginal check compared against raw transcript.words; the baseline is the silence-inserted timeline, so with any detected silence hasEdits stayed true after undoing everything. Now compares against the derived baseline (including text equality). - hasEdits initialized from deleted/inserted flags only, so saved text-only edits read as unedited and suppressed onChange. Now compares the saved state against the derived baseline. Fixes findings 1, 2, and 13 of mieweb/ui#323. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/hooks/useTranscriptEdits.ts | 62 +++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/src/hooks/useTranscriptEdits.ts b/src/hooks/useTranscriptEdits.ts index 1307fc79..226b87ca 100644 --- a/src/hooks/useTranscriptEdits.ts +++ b/src/hooks/useTranscriptEdits.ts @@ -384,9 +384,25 @@ export function useTranscriptEdits( ); const [clipboard, setClipboard] = useState(null); const [hasEdits, setHasEdits] = useState(() => { - // Check if initial state has edits + // Compare saved state against the derived baseline: deletions, insertions, + // length changes, AND text edits (text-only edits previously initialized + // hasEdits=false, which also suppressed onChange persistence). if (initialEditedWords) { - return initialEditedWords.some((ew) => ew.deleted || ew.inserted); + const baseline = initEditableWords( + transcript, + initialMinSilenceMs, + initialNlSilenceMs + ); + return ( + initialEditedWords.length !== baseline.length || + initialEditedWords.some( + (ew, i) => + ew.deleted || + ew.inserted || + ew.originalIndex !== i || + ew.word.text !== baseline[i].word.text + ) + ); } return false; }); @@ -439,11 +455,21 @@ export function useTranscriptEdits( const previousState = undoStack[undoStack.length - 1]; setUndoStack((prev) => prev.slice(0, -1)); setEditedWords(previousState); + // Compare against the silence-inserted baseline (what the editor actually + // starts from), not raw transcript.words — with any detected silence the + // lengths never matched and hasEdits stayed true after undoing everything. + const baseline = initEditableWords(transcript, minSilenceMs, nlSilenceMs); const isOriginal = - previousState.every((ew, i) => ew.originalIndex === i && !ew.deleted) && - previousState.length === transcript.words.length; + previousState.length === baseline.length && + previousState.every( + (ew, i) => + ew.originalIndex === i && + !ew.deleted && + !ew.inserted && + ew.word.text === baseline[i].word.text + ); setHasEdits(!isOriginal); - }, [undoStack, transcript.words.length]); + }, [undoStack, transcript, minSilenceMs, nlSilenceMs]); // Toggle deleted state on a single word const toggleWordDeleted = useCallback( @@ -670,17 +696,18 @@ export function useTranscriptEdits( (newMinSilenceMs: number, newNlSilenceMs: number) => { setMinSilenceMs(newMinSilenceMs); setNlSilenceMs(newNlSilenceMs); - // Rebuild the editable words with new silence detection - // Preserve deleted status for actual words (not silences) + // Rebuild the editable words with new silence detection. + // Preserve deleted status for actual words (not silences), keyed by the + // word's ORDINAL among non-silence words — the same key the restore loop + // below uses. (Keying by originalIndex — a position in the old + // silence-inserted array — mismatched the restore counter and applied + // deletions to the wrong words after a threshold change.) const wordDeletedStatus = new Map(); + let saveIdx = 0; editedWords.forEach((ew) => { - if ( - ew.word.wordType !== 'silence' && - ew.word.wordType !== 'silence-newline' && - ew.originalIndex >= 0 - ) { - // Map original word index to deleted status - wordDeletedStatus.set(ew.originalIndex, ew.deleted); + if (!isSilenceType(ew.word.wordType) && ew.originalIndex >= 0) { + wordDeletedStatus.set(saveIdx, ew.deleted); + saveIdx++; } }); @@ -691,13 +718,10 @@ export function useTranscriptEdits( newNlSilenceMs ); - // Restore deleted status for words + // Restore deleted status for words (same ordinal keyspace as the save) let wordIdx = 0; const restoredWords = newEditedWords.map((ew) => { - if ( - ew.word.wordType !== 'silence' && - ew.word.wordType !== 'silence-newline' - ) { + if (!isSilenceType(ew.word.wordType)) { const wasDeleted = wordDeletedStatus.get(wordIdx) ?? false; wordIdx++; return { ...ew, deleted: wasDeleted }; From 1ecd1e7442b8de740eb76131166f059a02140e22 Mon Sep 17 00:00:00 2001 From: Jonathan Locala Date: Thu, 23 Jul 2026 09:53:33 -0400 Subject: [PATCH 3/8] fix(media): sticky error, stale playerRef, perf, docs, a11y, safelist, ScriptPanel validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remaining review findings: - MediaPlayer: clear error state when src changes (a failed load trapped the UI even after the host supplied a good URL). - MediaEditor: external playerRef is now a stable delegating proxy — the [] snapshot captured the first-commit handle and could leave hosts holding a stale ref with a null mediaElement. - getSpeedAtIndex: drop the per-call copy+sort (markers are kept sorted at insert; stats calls this per word). - transcript.ts: correct the originalIndex doc — it indexes the silence-inserted timeline, not transcript.words. - TranscriptView segment rows: native button + custom Enter/Space handlers double-invoked; native activation only now. - ScriptPanel: whitelist wordType values on parse instead of casting arbitrary strings into the union. - tailwind-preset: add the 22 media-stack class variants missing from miewebUISafelist (TW3 safelist consumers). Fixes findings 8, 10, 12, 14, 15, 17, 18 (+9) of mieweb/ui#323. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/components/MediaEditor/MediaEditor.tsx | 24 ++++++++++++++----- src/components/MediaEditor/ScriptPanel.tsx | 10 ++++++++ src/components/MediaPlayer/MediaPlayer.tsx | 7 ++++++ .../TranscriptView/TranscriptView.tsx | 2 +- src/components/TranscriptView/transcript.ts | 5 +++- src/hooks/useTranscriptEdits.ts | 10 +++----- src/tailwind-preset.ts | 24 +++++++++++++++++++ 7 files changed, 67 insertions(+), 15 deletions(-) diff --git a/src/components/MediaEditor/MediaEditor.tsx b/src/components/MediaEditor/MediaEditor.tsx index 133434c1..c954b86d 100644 --- a/src/components/MediaEditor/MediaEditor.tsx +++ b/src/components/MediaEditor/MediaEditor.tsx @@ -22,10 +22,7 @@ import type { PlaybackSegment, PlaybackSpeed, } from '../TranscriptView/transcript'; -import { - PLAYBACK_SPEEDS, - isSilenceType, -} from '../TranscriptView/transcript'; +import { PLAYBACK_SPEEDS, isSilenceType } from '../TranscriptView/transcript'; import { useTranscriptEdits } from '../../hooks/useTranscriptEdits'; import { MediaPlayer, @@ -318,10 +315,25 @@ export const MediaEditor = React.forwardRef( // -- Refs -- const playerRef = React.useRef(null); - // Mirror the internal player handle to the host-provided ref, if any + // Mirror the internal player handle to the host-provided ref via a STABLE + // delegating proxy: every call forwards to playerRef.current at call time. + // (Snapshotting playerRef.current with [] deps captured the first-commit + // handle — ref assignment doesn't re-render, so hosts could hold a stale + // handle whose mediaElement stayed null forever.) React.useImperativeHandle( externalPlayerRef, - () => playerRef.current as MediaPlayerRef, + (): MediaPlayerRef => ({ + seekToMs: (timeMs) => playerRef.current?.seekToMs(timeMs), + play: () => playerRef.current?.play(), + pause: () => playerRef.current?.pause(), + getCurrentTimeMs: () => playerRef.current?.getCurrentTimeMs() ?? 0, + getDurationMs: () => playerRef.current?.getDurationMs() ?? 0, + isPaused: () => playerRef.current?.isPaused() ?? true, + setPlaybackRate: (rate) => playerRef.current?.setPlaybackRate(rate), + get mediaElement() { + return playerRef.current?.mediaElement ?? null; + }, + }), [] ); const contentRef = React.useRef(null); diff --git a/src/components/MediaEditor/ScriptPanel.tsx b/src/components/MediaEditor/ScriptPanel.tsx index a7176591..9f176e5f 100644 --- a/src/components/MediaEditor/ScriptPanel.tsx +++ b/src/components/MediaEditor/ScriptPanel.tsx @@ -74,6 +74,16 @@ async function parseScript( `Entry ${i + 1}: each entry needs word.text, word.startMs and word.endMs.` ); } + if ( + w.wordType !== undefined && + w.wordType !== 'word' && + w.wordType !== 'silence' && + w.wordType !== 'silence-newline' + ) { + throw new Error( + `Entry ${i + 1}: wordType must be 'word', 'silence' or 'silence-newline'.` + ); + } const word: TranscriptWord = { text: w.text, startMs: w.startMs, diff --git a/src/components/MediaPlayer/MediaPlayer.tsx b/src/components/MediaPlayer/MediaPlayer.tsx index f88235ab..a4d2cb15 100644 --- a/src/components/MediaPlayer/MediaPlayer.tsx +++ b/src/components/MediaPlayer/MediaPlayer.tsx @@ -134,6 +134,13 @@ export const MediaPlayer = React.forwardRef( ref ) => { const [error, setError] = React.useState(null); + + // A failed load must not outlive its src: clearing on change lets a host + // swap in a corrected URL without being trapped on the error surface + // (which unmounts the media element, so the new src would never load). + React.useEffect(() => { + setError(null); + }, [src]); const resolvedKind = kind ?? inferMediaKind(src); const transport = useMediaTransport({ diff --git a/src/components/TranscriptView/TranscriptView.tsx b/src/components/TranscriptView/TranscriptView.tsx index 11e42814..d23e4cda 100644 --- a/src/components/TranscriptView/TranscriptView.tsx +++ b/src/components/TranscriptView/TranscriptView.tsx @@ -298,7 +298,7 @@ export const TranscriptView = React.forwardRef< className={`hover:bg-muted focus-visible:ring-ring flex w-full cursor-pointer gap-3 rounded px-2 py-1 text-left text-sm transition-colors focus-visible:ring-2 focus-visible:outline-none ${ isActive ? 'bg-primary-500/20' : '' }`} - {...activationProps(() => seekTo(row.startMs))} + onClick={() => seekTo(row.startMs)} > {showTimestamps && ( diff --git a/src/components/TranscriptView/transcript.ts b/src/components/TranscriptView/transcript.ts index 6dd559f3..ee1680ba 100644 --- a/src/components/TranscriptView/transcript.ts +++ b/src/components/TranscriptView/transcript.ts @@ -58,7 +58,10 @@ export interface Transcript { * Used in the edited timeline to track deletions and reordering. */ export interface EditableWord { - /** Reference to original word in transcript.words */ + /** Index into the SILENCE-INSERTED baseline timeline (transcript.words + * with detected silence pseudo-words interleaved) — NOT into + * transcript.words. -1 for split/inserted entries with no baseline slot. + * Consumers persisting edits should key by this timeline. */ originalIndex: number; /** The word data (from original transcript) */ word: TranscriptWord; diff --git a/src/hooks/useTranscriptEdits.ts b/src/hooks/useTranscriptEdits.ts index 226b87ca..da6084b4 100644 --- a/src/hooks/useTranscriptEdits.ts +++ b/src/hooks/useTranscriptEdits.ts @@ -138,14 +138,10 @@ export function getSpeedAtIndex( speedMarkers: SpeedMarker[], defaultSpeed: PlaybackSpeed ): PlaybackSpeed { - // Sort markers by word index (should already be sorted, but be safe) - const sortedMarkers = [...speedMarkers].sort( - (a, b) => a.wordIndex - b.wordIndex - ); - - // Find the last marker at or before this word index + // Markers are kept sorted at insert (toggleSpeedMarker) — scanning without + // a per-call copy+sort matters because stats calls this once per word. let effectiveSpeed = defaultSpeed; - for (const marker of sortedMarkers) { + for (const marker of speedMarkers) { if (marker.wordIndex <= wordIndex) { effectiveSpeed = marker.speed; } else { diff --git a/src/tailwind-preset.ts b/src/tailwind-preset.ts index e494e4bb..122e1d2c 100644 --- a/src/tailwind-preset.ts +++ b/src/tailwind-preset.ts @@ -39,6 +39,30 @@ * @deprecated For Tailwind CSS 4 users — use the `@source` directive instead. */ export const miewebUISafelist = [ + // Media stack (MediaEditor / TranscriptView / MediaPlayer) — opacity-modifier + // and state variants introduced with the media components (ui#323 finding): + 'bg-muted/20', + 'bg-primary-50', + 'bg-primary-500/20', + 'bg-primary-500/40', + 'bg-success/15', + 'bg-warning/20', + 'bg-warning/40', + 'border-l-warning', + 'border-primary-600', + 'border-warning/50', + 'dark:bg-primary-950', + 'hover:bg-destructive/20', + 'hover:bg-muted', + 'hover:bg-muted/70', + 'hover:bg-primary-500/15', + 'hover:bg-primary-500/30', + 'hover:bg-success/25', + 'hover:text-destructive', + 'outline-destructive', + 'outline-primary-500', + 'outline-warning', + 'text-primary-500', // Semantic colors 'border-border', 'border-input', From c411f244520ed70e84aba27144ed4ea5201b4bbb Mon Sep 17 00:00:00 2001 From: Jonathan Locala Date: Thu, 23 Jul 2026 09:55:15 -0400 Subject: [PATCH 4/8] test(media): regression tests for the ui#323 findings Six tests encoding the verified failure modes: silence-newline stats, filler single-counting, splitting newline silences, deletion identity across threshold rebuilds, undo-to-baseline clearing hasEdits, and hasEdits initialization on text-only saved edits. The pre-fix code fails all six. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/hooks/useTranscriptEdits.test.ts | 93 ++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/src/hooks/useTranscriptEdits.test.ts b/src/hooks/useTranscriptEdits.test.ts index 24c5fd37..7ade60a7 100644 --- a/src/hooks/useTranscriptEdits.test.ts +++ b/src/hooks/useTranscriptEdits.test.ts @@ -233,3 +233,96 @@ describe('useTranscriptEdits', () => { expect((words as EditableWord[])[0].deleted).toBe(true); }); }); + +// ============================================================================ +// Regression tests for the ui#323 review findings +// ============================================================================ + +// A transcript with a leading pause and an inter-word gap, so the derived +// timeline interleaves silence pseudo-words: [sil, A, sil-nl, B] +const gappy: Transcript = { + durationMs: 4600, + words: [ + { text: 'Alpha', startMs: 600, endMs: 1000 }, + { text: 'Beta', startMs: 3000, endMs: 3500 }, + ], +}; + +describe('ui#323 regressions', () => { + it('counts silence-newline entries as silences, not words (finding 3)', () => { + const { result } = renderHook(() => + useTranscriptEdits({ transcript: gappy }) + ); + // Timeline: [silence 0-600][Alpha][silence-newline 1000-3000][Beta] + expect(result.current.stats.activeWordCount).toBe(2); + expect(result.current.stats.activeSilenceCount).toBe(2); + }); + + it('counts default filler words exactly once (findings 4/5)', () => { + const { result } = renderHook(() => + useTranscriptEdits({ transcript: packed }) + ); + expect(result.current.fillerAnalysis.matchCounts.get('um')).toBe(1); + expect(result.current.fillerAnalysis.durations.get('um')).toBe(300); + }); + + it('can split a silence-newline entry (finding 6)', () => { + const { result } = renderHook(() => + useTranscriptEdits({ transcript: gappy }) + ); + const nlIndex = result.current.editedWords.findIndex( + (ew) => ew.word.wordType === 'silence-newline' + ); + expect(nlIndex).toBeGreaterThan(-1); + act(() => result.current.splitSilence(nlIndex, [1, 1])); + // The 2s newline silence is replaced by two 1s segments + expect(result.current.editedWords.filter((ew) => ew.inserted)).toHaveLength( + 2 + ); + }); + + it('keeps deletions on the SAME words across a threshold rebuild (finding 2)', () => { + const { result } = renderHook(() => + useTranscriptEdits({ transcript: gappy }) + ); + // Delete 'Beta' (the second non-silence word) + const betaIdx = result.current.editedWords.findIndex( + (ew) => ew.word.text === 'Beta' + ); + act(() => result.current.toggleWordDeleted(betaIdx)); + // Change thresholds so the silence layout changes entirely + act(() => result.current.setSilenceThresholds(100, 5000)); + const beta = result.current.editedWords.find( + (ew) => ew.word.text === 'Beta' + ); + const alpha = result.current.editedWords.find( + (ew) => ew.word.text === 'Alpha' + ); + expect(beta?.deleted).toBe(true); + expect(alpha?.deleted).toBe(false); + }); + + it('clears hasEdits after undoing back to baseline with silences present (finding 1)', () => { + const { result } = renderHook(() => + useTranscriptEdits({ transcript: gappy }) + ); + act(() => result.current.toggleWordDeleted(1)); + expect(result.current.hasEdits).toBe(true); + act(() => result.current.undo()); + expect(result.current.hasEdits).toBe(false); + }); + + it('initializes hasEdits=true for saved text-only edits (finding 13)', () => { + const baseline = initEditableWords(packed); + const withTextEdit: EditableWord[] = baseline.map((ew, i) => + i === 0 ? { ...ew, word: { ...ew.word, text: 'Howdy' } } : ew + ); + const { result } = renderHook(() => + useTranscriptEdits({ + transcript: packed, + initialEditedWords: withTextEdit, + }) + ); + expect(result.current.hasEdits).toBe(true); + }); +}); From 072775e7849304e7e44382f3c65a947850144675 Mon Sep 17 00:00:00 2001 From: Jonathan Locala Date: Thu, 23 Jul 2026 10:29:36 -0400 Subject: [PATCH 5/8] fix(media): clear undo stack on silence-threshold rebuild MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Undo snapshots are captured against the current silence layout; after setSilenceThresholds rebuilds the timeline, restoring one resurrects silence chips that no longer match the active thresholds. Threshold changes are documented as not undoable — drop the stale history. Found by Copilot review on #327; regression test fails pre-fix. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/hooks/useTranscriptEdits.test.ts | 18 ++++++++++++++++++ src/hooks/useTranscriptEdits.ts | 8 +++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/hooks/useTranscriptEdits.test.ts b/src/hooks/useTranscriptEdits.test.ts index 7ade60a7..963b5ee2 100644 --- a/src/hooks/useTranscriptEdits.test.ts +++ b/src/hooks/useTranscriptEdits.test.ts @@ -312,6 +312,24 @@ describe('ui#323 regressions', () => { expect(result.current.hasEdits).toBe(false); }); + it('drops stale undo snapshots on a threshold rebuild (#327 review)', () => { + const { result } = renderHook(() => + useTranscriptEdits({ transcript: gappy }) + ); + const betaIdx = result.current.editedWords.findIndex( + (ew) => ew.word.text === 'Beta' + ); + act(() => result.current.toggleWordDeleted(betaIdx)); + expect(result.current.undoStack).toHaveLength(1); + // The snapshot in the stack was captured against the old silence layout + act(() => result.current.setSilenceThresholds(100, 5000)); + expect(result.current.undoStack).toHaveLength(0); + // undo() is a no-op instead of restoring a stale-layout timeline + const before = result.current.editedWords; + act(() => result.current.undo()); + expect(result.current.editedWords).toBe(before); + }); + it('initializes hasEdits=true for saved text-only edits (finding 13)', () => { const baseline = initEditableWords(packed); const withTextEdit: EditableWord[] = baseline.map((ew, i) => diff --git a/src/hooks/useTranscriptEdits.ts b/src/hooks/useTranscriptEdits.ts index da6084b4..1af7421c 100644 --- a/src/hooks/useTranscriptEdits.ts +++ b/src/hooks/useTranscriptEdits.ts @@ -317,7 +317,8 @@ export interface UseTranscriptEditsResult { ) => void; // -- Silence thresholds -- - /** Rebuild silence detection with new thresholds, preserving word deletions */ + /** Rebuild silence detection with new thresholds, preserving word deletions. + * Not undoable — clears the undo stack (prior snapshots reference the old silence layout). */ setSilenceThresholds: (minMs: number, nlMs: number) => void; /** Current silence detection thresholds (ms) */ silenceThresholds: { minSilenceMs: number; nlSilenceMs: number }; @@ -726,6 +727,11 @@ export function useTranscriptEdits( }); setEditedWords(restoredWords); + // Undo snapshots were captured against the previous silence layout; + // restoring one after a rebuild would resurrect silence chips that no + // longer match the current thresholds. Threshold changes are not + // undoable, so drop the stale history rather than restore it. + setUndoStack([]); }, [editedWords, transcript] ); From 3fe2912f47c14c3a66f4ba2bab89e83fda1d04e1 Mon Sep 17 00:00:00 2001 From: Jonathan Locala Date: Thu, 23 Jul 2026 17:48:15 -0400 Subject: [PATCH 6/8] fix(media): keep getSpeedAtIndex order-independent getSpeedAtIndex is exported publicly (src/hooks/index.ts). The perf fix replaced its defensive copy+sort with an early-breaking scan, which silently returns the wrong speed if a caller passes markers out of order. Use a single O(n) pass that takes the nearest marker at or before the index instead: same per-call cost as the scan (no allocation, no sort), but correct for arbitrary order. Found by Copilot review on #327; regression test fails on the previous implementation. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/hooks/useTranscriptEdits.test.ts | 13 +++++++++++++ src/hooks/useTranscriptEdits.ts | 12 +++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/hooks/useTranscriptEdits.test.ts b/src/hooks/useTranscriptEdits.test.ts index 963b5ee2..313c8294 100644 --- a/src/hooks/useTranscriptEdits.test.ts +++ b/src/hooks/useTranscriptEdits.test.ts @@ -117,6 +117,19 @@ describe('getSpeedAtIndex', () => { expect(getSpeedAtIndex(2, markers, 1)).toBe(1.5); expect(getSpeedAtIndex(3, markers, 1)).toBe(2); }); + + it('is order-independent for external callers (#327 review)', () => { + // Public helper: callers may pass markers in arbitrary order. + const unsorted: SpeedMarker[] = [ + { wordIndex: 5, speed: 2 }, + { wordIndex: 1, speed: 1.5 }, + { wordIndex: 3, speed: 0.5 }, + ]; + expect(getSpeedAtIndex(0, unsorted, 1)).toBe(1); + expect(getSpeedAtIndex(1, unsorted, 1)).toBe(1.5); + expect(getSpeedAtIndex(4, unsorted, 1)).toBe(0.5); + expect(getSpeedAtIndex(9, unsorted, 1)).toBe(2); + }); }); // ============================================================================ diff --git a/src/hooks/useTranscriptEdits.ts b/src/hooks/useTranscriptEdits.ts index 1af7421c..ed15b4b7 100644 --- a/src/hooks/useTranscriptEdits.ts +++ b/src/hooks/useTranscriptEdits.ts @@ -138,14 +138,16 @@ export function getSpeedAtIndex( speedMarkers: SpeedMarker[], defaultSpeed: PlaybackSpeed ): PlaybackSpeed { - // Markers are kept sorted at insert (toggleSpeedMarker) — scanning without - // a per-call copy+sort matters because stats calls this once per word. + // Single O(n) pass that takes the marker with the greatest wordIndex at or + // before the target. This avoids the per-call copy+sort (stats calls this + // once per word) without assuming callers pass sorted markers — it is a + // public helper, so arbitrary order must still yield the right answer. let effectiveSpeed = defaultSpeed; + let nearest = -1; for (const marker of speedMarkers) { - if (marker.wordIndex <= wordIndex) { + if (marker.wordIndex <= wordIndex && marker.wordIndex > nearest) { + nearest = marker.wordIndex; effectiveSpeed = marker.speed; - } else { - break; } } return effectiveSpeed; From acda054ed4370eb5d33f6ee770fda097fb35a44b Mon Sep 17 00:00:00 2001 From: Jonathan Locala Date: Fri, 24 Jul 2026 19:52:23 -0400 Subject: [PATCH 7/8] fix(media): exclude pasted words from the threshold-rebuild deletion keyspace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setSilenceThresholds() saves deletion state keyed by ordinal among non-silence words, but counted pasted words too (they keep their source originalIndex). The rebuild re-inits from the original transcript with no inserts, so a pasted word shifted every later ordinal and deletions landed on the wrong words. Save now skips inserted words, matching the restore keyspace. Regression test follows the review repro: copy A, paste before C, delete C, change thresholds — C stays deleted. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/hooks/useTranscriptEdits.test.ts | 25 +++++++++++++++++++++++++ src/hooks/useTranscriptEdits.ts | 8 +++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/hooks/useTranscriptEdits.test.ts b/src/hooks/useTranscriptEdits.test.ts index 313c8294..ab170caf 100644 --- a/src/hooks/useTranscriptEdits.test.ts +++ b/src/hooks/useTranscriptEdits.test.ts @@ -343,6 +343,31 @@ describe('ui#323 regressions', () => { expect(result.current.editedWords).toBe(before); }); + it('keeps deletions on the right words across a threshold rebuild with pasted words (#327 review)', () => { + const { result } = renderHook(() => + useTranscriptEdits({ transcript: packed }) + ); + // Copy 'Hello' and paste the copy before 'world'. The pasted word keeps + // its source originalIndex, but the rebuild re-inits from the original + // transcript (dropping inserts) — so it must not occupy a slot in the + // deletion keyspace, or every word after it shifts. + act(() => result.current.copy([0])); + act(() => result.current.paste(2, true)); + const worldIdx = result.current.editedWords.findIndex( + (ew) => ew.word.text === 'world' + ); + act(() => result.current.toggleWordDeleted(worldIdx)); + + act(() => result.current.setSilenceThresholds(100, 5000)); + + // The rebuild drops the pasted copy; 'world' must still be the deleted one + expect(result.current.editedWords).toHaveLength(3); + const [hello, um, world] = result.current.editedWords; + expect(world.deleted).toBe(true); + expect(hello.deleted).toBe(false); + expect(um.deleted).toBe(false); + }); + it('initializes hasEdits=true for saved text-only edits (finding 13)', () => { const baseline = initEditableWords(packed); const withTextEdit: EditableWord[] = baseline.map((ew, i) => diff --git a/src/hooks/useTranscriptEdits.ts b/src/hooks/useTranscriptEdits.ts index ed15b4b7..648ab0ef 100644 --- a/src/hooks/useTranscriptEdits.ts +++ b/src/hooks/useTranscriptEdits.ts @@ -701,10 +701,16 @@ export function useTranscriptEdits( // below uses. (Keying by originalIndex — a position in the old // silence-inserted array — mismatched the restore counter and applied // deletions to the wrong words after a threshold change.) + // Pasted words keep their source originalIndex but are dropped by the + // rebuild, so they must not occupy a slot in this keyspace either. const wordDeletedStatus = new Map(); let saveIdx = 0; editedWords.forEach((ew) => { - if (!isSilenceType(ew.word.wordType) && ew.originalIndex >= 0) { + if ( + !isSilenceType(ew.word.wordType) && + ew.originalIndex >= 0 && + !ew.inserted + ) { wordDeletedStatus.set(saveIdx, ew.deleted); saveIdx++; } From 218be38b071b2a3750a0ae5758a544ecfca1b802 Mon Sep 17 00:00:00 2001 From: Jonathan Locala Date: Fri, 24 Jul 2026 19:59:01 -0400 Subject: [PATCH 8/8] test(media): name the regression suite for both its sources The block collected ui#323 findings first and #327 review findings since; the name now says so. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/hooks/useTranscriptEdits.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useTranscriptEdits.test.ts b/src/hooks/useTranscriptEdits.test.ts index ab170caf..9626afc2 100644 --- a/src/hooks/useTranscriptEdits.test.ts +++ b/src/hooks/useTranscriptEdits.test.ts @@ -261,7 +261,7 @@ const gappy: Transcript = { ], }; -describe('ui#323 regressions', () => { +describe('ui#323 / #327-review regressions', () => { it('counts silence-newline entries as silences, not words (finding 3)', () => { const { result } = renderHook(() => useTranscriptEdits({ transcript: gappy })