From a8ba0503353436cd20632ef140962179828f45db Mon Sep 17 00:00:00 2001 From: Joe Hachem Date: Sun, 28 Jun 2026 18:03:30 +0300 Subject: [PATCH] fix(captions): address caption-editor review feedback --- electron/ipc/captions/generate.ts | 8 ++-- electron/ipc/captions/segment.test.ts | 24 ++++++++++ electron/ipc/captions/segment.ts | 14 ++++-- .../video-editor/CaptionListPanel.tsx | 26 ++++++++--- src/components/video-editor/SettingsPanel.tsx | 4 ++ src/components/video-editor/VideoEditor.tsx | 6 +++ .../video-editor/captionOps.test.ts | 10 +++++ src/components/video-editor/captionOps.ts | 30 +++++++------ .../video-editor/timeline/TimelineEditor.tsx | 5 ++- .../components/viewport/TimelineCanvas.tsx | 39 +++++++++++++--- .../actions/useTimelineCaptionActions.ts | 44 ++++++++++++++----- .../timeline/hooks/useTimelineDndBindings.ts | 10 +++-- .../hooks/useTimelineEditorRuntime.ts | 14 +++--- .../timeline/hooks/useTimelineSelection.ts | 17 ++++++- src/i18n/locales/en/settings.json | 10 ++++- src/i18n/locales/zh-CN/settings.json | 2 +- src/i18n/locales/zh-TW/settings.json | 2 +- 17 files changed, 205 insertions(+), 60 deletions(-) diff --git a/electron/ipc/captions/generate.ts b/electron/ipc/captions/generate.ts index 038d1f87..2a3f49cd 100644 --- a/electron/ipc/captions/generate.ts +++ b/electron/ipc/captions/generate.ts @@ -280,10 +280,10 @@ export async function generateAutoCaptionsFromVideo(options: { let cuesToReturn = cues; try { const silences = await detectSilenceIntervals({ ffmpegPath, wavPath }); - const resegmented = segmentCuesIntoPhrases(cues, silences); - if (resegmented.length > 0) { - cuesToReturn = resegmented; - } + // An empty result is a valid resegmentation (e.g. every transcribed word fell + // inside a long detected silence and was dropped as a hallucination), so take it + // as-is. Only a thrown exception should fall back to the raw cues. + cuesToReturn = segmentCuesIntoPhrases(cues, silences); } catch (error) { console.warn( "[auto-captions] Silence-aware re-segmentation failed, using raw cues:", diff --git a/electron/ipc/captions/segment.test.ts b/electron/ipc/captions/segment.test.ts index 94344d59..e6ebaf00 100644 --- a/electron/ipc/captions/segment.test.ts +++ b/electron/ipc/captions/segment.test.ts @@ -104,6 +104,30 @@ describe("segmentCuesIntoPhrases", () => { expect(result[0].text).toBe("Okay. Great."); }); + it("does not merge two short captions across a real pause just above the merge gap", () => { + // Two short, sentence-ended phrases separated by a 450ms gap (just above the 400ms + // mergeGapMs). Edge padding shrinks the apparent gap, so if merge eligibility ran on + // padded timings it would wrongly fuse them. Merge must run on the true speech gap. + const cues: CaptionCuePayload[] = [ + { + id: "caption-1", + startMs: 0, + endMs: 1_250, + text: "Okay. Great.", + words: [ + { text: "Okay.", startMs: 0, endMs: 400 }, + { text: "Great.", startMs: 850, endMs: 1_250, leadingSpace: true }, + ], + }, + ]; + const result = segmentCuesIntoPhrases(cues, []); + expect(result).toHaveLength(2); + expect(result[0].text).toBe("Okay."); + expect(result[1].text).toBe("Great."); + // Padding still applies, but cues stay separate and non-overlapping. + expect(result[0].endMs).toBeLessThanOrEqual(result[1].startMs); + }); + it("does not merge a short sentence into a full-length one", () => { const cues: CaptionCuePayload[] = [ { diff --git a/electron/ipc/captions/segment.ts b/electron/ipc/captions/segment.ts index 912f089c..60407a00 100644 --- a/electron/ipc/captions/segment.ts +++ b/electron/ipc/captions/segment.ts @@ -409,7 +409,10 @@ export function segmentCuesIntoPhrases( const shouldBreak = endsSentence(word.text) || gapMs >= pauseMs || - silenceInGap(word.endMs, next.startMs, silences, splitSilenceMs) || + // Only consult acoustic silence when there's a real gap between the words. When + // consecutive words overlap or abut (gapMs <= 0) a silence interval spanning that + // region must not manufacture a bogus split. + (gapMs > 0 && silenceInGap(word.endMs, next.startMs, silences, splitSilenceMs)) || phraseDurationMs >= maxPhraseMs; if (shouldBreak) { @@ -438,7 +441,6 @@ export function segmentCuesIntoPhrases( } pieces.sort((left, right) => left.startMs - right.startMs || left.endMs - right.endMs); - padSpans(pieces, edgePadMs); const sentenceCues: CaptionCuePayload[] = pieces.map((piece) => ({ id: "", @@ -447,5 +449,11 @@ export function segmentCuesIntoPhrases( text: piece.text, ...(piece.words.length > 0 ? { words: piece.words } : {}), })); - return renumberCues(mergeShortAdjacentCaptions(sentenceCues, mergeOptions)); + // Merge rapid-fire short captions BEFORE padding so merge eligibility sees the true + // speech gaps. Padding pulls cue edges toward each other, which would shrink the + // apparent gap and could merge two captions across a real pause sitting just above + // mergeGapMs. Pad the survivors afterward so envelopes still get their edge padding. + const merged = mergeShortAdjacentCaptions(sentenceCues, mergeOptions); + padSpans(merged, edgePadMs); + return renumberCues(merged); } diff --git a/src/components/video-editor/CaptionListPanel.tsx b/src/components/video-editor/CaptionListPanel.tsx index 91898c50..ef04e5e7 100644 --- a/src/components/video-editor/CaptionListPanel.tsx +++ b/src/components/video-editor/CaptionListPanel.tsx @@ -1,6 +1,7 @@ import { ArrowsMerge, Scissors, Trash } from "@phosphor-icons/react"; -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { Button } from "@/components/ui/button"; +import { useScopedT } from "@/contexts/I18nContext"; import type { CaptionRetimeSpan } from "./captionOps"; import type { CaptionCue } from "./types"; @@ -62,9 +63,14 @@ function CaptionEditor({ onMerge, onDelete, }: CaptionEditorProps) { + const t = useScopedT("settings"); const [draftText, setDraftText] = useState(cue.text); const [startValue, setStartValue] = useState(formatTimecode(cue.startMs)); const [endValue, setEndValue] = useState(formatTimecode(cue.endMs)); + // Escape resets the draft and blurs, but `setDraftText` is batched so the blur-driven + // `commitText` would still see the stale (edited) draft and save it. This flag lets the + // cancel path tell the next blur to discard instead of commit. + const cancelNextCommitRef = useRef(false); useEffect(() => { setDraftText(cue.text); @@ -73,6 +79,11 @@ function CaptionEditor({ }, [cue.text, cue.startMs, cue.endMs]); const commitText = useCallback(() => { + if (cancelNextCommitRef.current) { + cancelNextCommitRef.current = false; + setDraftText(cue.text); + return; + } const normalized = draftText.trim(); if (normalized && normalized !== cue.text) { onTextEdit(cue.id, normalized); @@ -98,7 +109,7 @@ function CaptionEditor({