diff --git a/src/services/TranscriptionService.test.ts b/src/services/TranscriptionService.test.ts index 8b873fa7..7fe40dab 100644 --- a/src/services/TranscriptionService.test.ts +++ b/src/services/TranscriptionService.test.ts @@ -193,7 +193,20 @@ describe("TranscriptionService", () => { }); describe("buildTranscriptBody (TR-01)", () => { - const buildBody = (plugin: PodNotes) => { + const buildBody = ( + plugin: PodNotes, + audio: { + buffer: ArrayBuffer; + mimeType: string; + extension: string; + basename: string; + } = { + buffer: new ArrayBuffer(1024), + mimeType: "audio/mpeg", + extension: "mp3", + basename: "episode", + }, + ) => { const service = new TranscriptionService(plugin); return ( service as unknown as { @@ -205,17 +218,9 @@ describe("TranscriptionService", () => { basename: string; }, update: (message: string) => void, - ) => Promise; + ) => Promise<{ body: string; warning?: string }>; } - ).buildTranscriptBody( - { - buffer: new ArrayBuffer(1024), - mimeType: "audio/mpeg", - extension: "mp3", - basename: "episode", - }, - () => {}, - ); + ).buildTranscriptBody(audio, () => {}); }; test("throws when the trimmed Whisper body is empty", async () => { @@ -231,9 +236,133 @@ describe("TranscriptionService", () => { text: "One. Two.", }); - await expect(buildBody(createMockPlugin())).resolves.toBe( - "One.\n\nTwo.", + await expect(buildBody(createMockPlugin())).resolves.toEqual({ + body: "One.\n\nTwo.", + warning: undefined, + }); + }); + }); + + describe("failed chunks are not saved as a completed transcript (other-silent-failure)", () => { + const buildBodyDirect = ( + plugin: PodNotes, + audio: { + buffer: ArrayBuffer; + mimeType: string; + extension: string; + basename: string; + }, + ) => { + const service = new TranscriptionService(plugin); + return ( + service as unknown as { + buildTranscriptBody: ( + a: typeof audio, + update: (message: string) => void, + ) => Promise<{ body: string; warning?: string }>; + } + ).buildTranscriptBody(audio, () => {}); + }; + + const mp3Audio = (byteLength: number) => ({ + buffer: new ArrayBuffer(byteLength), + mimeType: "audio/mp3", + extension: "mp3", + basename: "episode", + }); + + test("throws (no file) when the single chunk fails every retry", async () => { + transcriptionsCreateMock.mockRejectedValue(new Error("boom")); + vi.useFakeTimers(); + try { + const promise = buildBodyDirect(createMockPlugin(), mp3Audio(1024)); + const assertion = expect(promise).rejects.toThrow( + "Transcription failed: all 1 audio chunk(s) failed or returned no text.", + ); + await vi.runAllTimersAsync(); + await assertion; + } finally { + vi.useRealTimers(); + } + }); + + test("throws when failed chunks plus empty successes leave no real text", async () => { + // >20 MB mp3 → two chunks. chunk 0 fails every retry; chunk 1 "succeeds" + // but returns empty text. The body is then only an error marker, which + // must NOT be saved as a completed transcript. + transcriptionsCreateMock.mockImplementation( + async ({ file }: { file: File }) => { + if (file.name.includes("part0")) { + throw new Error("boom"); + } + return { text: " " }; + }, ); + + vi.useFakeTimers(); + try { + const promise = buildBodyDirect( + createMockPlugin(), + mp3Audio(20 * 1024 * 1024 + 1024), + ); + const assertion = expect(promise).rejects.toThrow( + "Transcription failed: all 2 audio chunk(s) failed or returned no text.", + ); + await vi.runAllTimersAsync(); + await assertion; + } finally { + vi.useRealTimers(); + } + }); + + test("does not write a file when transcription fails completely", async () => { + transcriptionsCreateMock.mockRejectedValue(new Error("boom")); + const plugin = createMockPlugin(); + const service = new TranscriptionService(plugin); + + vi.useFakeTimers(); + try { + const promise = ( + service as unknown as { + transcribeEpisode: (episode: Episode) => Promise; + } + ).transcribeEpisode(mockEpisode); + // One chunk, MAX_RETRIES=3 → backoff 1000ms + 2000ms before it gives up. + await vi.advanceTimersByTimeAsync(3500); + await promise; + } finally { + vi.useRealTimers(); + } + + expect(plugin.app.vault.create).not.toHaveBeenCalled(); + }); + + test("keeps an otherwise-good transcript but warns when only some chunks fail", async () => { + // A >20 MB mp3 byte-splits into two chunks; fail the second one. + transcriptionsCreateMock.mockImplementation( + async ({ file }: { file: File }) => { + if (file.name.includes("part1")) { + throw new Error("boom"); + } + return { text: "Good chunk." }; + }, + ); + + vi.useFakeTimers(); + try { + const promise = buildBodyDirect( + createMockPlugin(), + mp3Audio(20 * 1024 * 1024 + 1024), + ); + await vi.runAllTimersAsync(); + const result = await promise; + + expect(result.body).toContain("Good chunk."); + expect(result.body).toContain("[Error transcribing chunk 1]"); + expect(result.warning).toContain("1 of 2 chunk(s) failed"); + } finally { + vi.useRealTimers(); + } }); }); }); diff --git a/src/services/TranscriptionService.ts b/src/services/TranscriptionService.ts index aadef589..111ba198 100644 --- a/src/services/TranscriptionService.ts +++ b/src/services/TranscriptionService.ts @@ -57,6 +57,14 @@ function formatTime(ms: number): string { return `${hours.toString().padStart(2, "0")}:${(minutes % 60).toString().padStart(2, "0")}:${(seconds % 60).toString().padStart(2, "0")}`; } +// A chunk that exhausts its retries leaves this placeholder in its transcript +// slot; the pattern matches it so buildTranscriptBody can tell real speech apart +// from a body made only of error markers. Keep the builder and matcher in sync. +function chunkErrorPlaceholder(index: number): string { + return `[Error transcribing chunk ${index}]`; +} +const CHUNK_ERROR_PLACEHOLDER_PATTERN = /\[Error transcribing chunk \d+\]/g; + export class TranscriptionService { private plugin: PodNotes; private client: OpenAI | null = null; @@ -168,7 +176,7 @@ export class TranscriptionService { } = await getEpisodeAudioBuffer(episode); const mimeType = getMimeType(fileExtension); - const transcriptBody = await this.buildTranscriptBody( + const { body: transcriptBody, warning } = await this.buildTranscriptBody( { buffer: fileBuffer, mimeType, @@ -182,7 +190,7 @@ export class TranscriptionService { await this.saveTranscription(episode, transcriptBody); notice.stop(); - notice.update("Transcription completed and saved."); + notice.update(warning ?? "Transcription completed and saved."); } catch (error) { console.error("Transcription error:", error); const message = error instanceof Error ? error.message : String(error); @@ -201,11 +209,20 @@ export class TranscriptionService { * separated into paragraphs, so it is rendered as-is. Either path yielding no * speech is treated as a failure rather than writing an empty transcript (empty * Whisper chunks join to whitespace, so the body is trimmed before the check). + * + * A chunk that exhausts its retries leaves an `[Error transcribing chunk N]` + * placeholder in its slot. If stripping those placeholders leaves no real text - + * every chunk failed, or the only successes were empty - we throw, so no file is + * written and the episode stays retryable instead of saving a "transcript" made + * only of error markers that the existence check would then refuse to re-run + * (mirrors the diarization provider's all-chunks-failed contract). When some + * chunks fail but real speech remains we keep the otherwise-good transcript and + * return a `warning` so the run is reported as partial, not a clean success. */ private async buildTranscriptBody( audio: DiarizationAudio, updateNotice: (message: string) => void, - ): Promise { + ): Promise<{ body: string; warning?: string }> { const diarization = this.plugin.settings.transcript.diarization; if (diarization?.enabled) { @@ -217,19 +234,41 @@ export class TranscriptionService { if (segments.length === 0) { throw new Error("Diarization returned no speech segments."); } - return renderDiarizedTranscript(segments, diarization.speakerTemplate); + return { + body: renderDiarizedTranscript(segments, diarization.speakerTemplate), + }; } updateNotice("Creating audio chunks..."); const files = await createChunkFiles(audio); updateNotice("Starting transcription..."); - const transcription = await this.transcribeChunks(files, updateNotice); - // Empty chunks join to " " (not ""), so trim before deciding it is empty. - const body = transcription.trim().replace(/\.\s+/g, ".\n\n"); - if (body.length === 0) { - throw new Error("Transcription returned no text."); + const { text, failedChunks } = await this.transcribeChunks( + files, + updateNotice, + ); + + // Strip the error placeholders (and trim) to see whether ANY real speech was + // transcribed. Nothing real means every chunk failed or the only successes + // were empty - either way there is no usable transcript, so throw instead of + // saving a body of pure error markers (empty chunks join to " ", not ""). + const realText = text.replace(CHUNK_ERROR_PLACEHOLDER_PATTERN, " ").trim(); + if (realText.length === 0) { + throw new Error( + failedChunks > 0 + ? `Transcription failed: all ${files.length} audio chunk(s) failed or returned no text.` + : "Transcription returned no text.", + ); } - return body; + + // Reflow the full body (placeholders kept inline so the user can see which + // chunks failed) after sentence periods for readability. + const body = text.trim().replace(/\.\s+/g, ".\n\n"); + + const warning = + failedChunks > 0 + ? `Transcription saved, but ${failedChunks} of ${files.length} chunk(s) failed - look for [Error transcribing chunk N] markers and re-run after deleting the note to retry.` + : undefined; + return { body, warning }; } /** Route the episode audio to the configured diarization provider (#168). */ @@ -269,10 +308,11 @@ export class TranscriptionService { private async transcribeChunks( files: File[], updateNotice: (message: string) => void, - ): Promise { + ): Promise<{ text: string; failedChunks: number }> { const client = await this.getClient(); const transcriptions: string[] = new Array(files.length); let completedChunks = 0; + let failedChunks = 0; let nextIndex = 0; const updateProgress = () => { @@ -308,7 +348,8 @@ export class TranscriptionService { `Failed to transcribe chunk ${index} after ${this.MAX_RETRIES} attempts:`, error, ); - transcriptions[index] = `[Error transcribing chunk ${index}]`; + transcriptions[index] = chunkErrorPlaceholder(index); + failedChunks++; completedChunks++; updateProgress(); } else { @@ -329,7 +370,7 @@ export class TranscriptionService { await Promise.all(workers); - return transcriptions.join(" "); + return { text: transcriptions.join(" "), failedChunks }; } /** diff --git a/src/services/audioChunker.test.ts b/src/services/audioChunker.test.ts index 57e395de..19133cf6 100644 --- a/src/services/audioChunker.test.ts +++ b/src/services/audioChunker.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "vitest"; +import { afterEach, describe, expect, test, vi } from "vitest"; import { CHUNK_SIZE_BYTES, createBinaryChunkFiles, @@ -22,11 +22,25 @@ describe("getMimeType", () => { }); describe("shouldConvertToWav", () => { - test("returns true for m4a files", () => { + test("only frame-synced mp3 is byte-splittable; everything else needs WAV", () => { + // MP3 resyncs at the next frame header, so it can be sliced raw. + expect(shouldConvertToWav("mp3", "audio/mp3")).toBe(false); + expect(shouldConvertToWav("MP3", "audio/mp3")).toBe(false); + // An mp3 extension wins even if the mime is mislabeled. + expect(shouldConvertToWav("mp3", "audio/mp4")).toBe(false); + + // Container/header-dependent formats can't be byte-split standalone. expect(shouldConvertToWav("m4a", "audio/mp4")).toBe(true); expect(shouldConvertToWav("M4A", "audio/mp4")).toBe(true); - expect(shouldConvertToWav("mp3", "audio/mp4")).toBe(true); - expect(shouldConvertToWav("mp3", "audio/mpeg")).toBe(false); + expect(shouldConvertToWav("ogg", "audio/ogg")).toBe(true); + expect(shouldConvertToWav("webm", "audio/webm")).toBe(true); + expect(shouldConvertToWav("flac", "audio/flac")).toBe(true); + expect(shouldConvertToWav("wav", "audio/wav")).toBe(true); + + // Unknown formats default to the safe WAV path; the catch-all audio/mpeg + // mime is deliberately not treated as splittable. + expect(shouldConvertToWav("opus", "audio/mpeg")).toBe(true); + expect(shouldConvertToWav("", "audio/mpeg")).toBe(true); }); }); @@ -136,4 +150,124 @@ describe("createChunkFiles small-file fast path (#168 / PR #204 review)", () => expect(files[0].name).toBe("episode.mp3"); expect(files[0].size).toBe(2048); }); + + test("sends a small ogg/flac as a single original file (no byte-split)", async () => { + for (const [extension, mimeType] of [ + ["ogg", "audio/ogg"], + ["flac", "audio/flac"], + ]) { + const files = await createChunkFiles({ + buffer: new ArrayBuffer(4096), + basename: "episode", + extension, + mimeType, + }); + + expect(files).toHaveLength(1); + expect(files[0].name).toBe(`episode.${extension}`); + } + }); +}); + +describe("createChunkFiles large-file routing (other-logic-bug)", () => { + // A minimal AudioBuffer/AudioContext so the decode+WAV path can run under + // jsdom, which ships no Web Audio implementation. decodeAudioData returns a + // buffer long enough to force several WAV chunks. + class FakeAudioBuffer { + numberOfChannels = 1; + sampleRate = 44100; + length: number; + private channel: Float32Array; + constructor(length: number) { + this.length = length; + this.channel = new Float32Array(length); + } + getChannelData(): Float32Array { + return this.channel; + } + } + + function stubAudioContext(decode: (buffer: ArrayBuffer) => unknown): void { + vi.stubGlobal( + "AudioContext", + class { + async decodeAudioData(buffer: ArrayBuffer): Promise { + return decode(buffer); + } + async close(): Promise {} + }, + ); + } + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + test("byte-splits a large mp3 into raw .mp3 parts (frame-synced, splittable)", async () => { + const buffer = new ArrayBuffer(CHUNK_SIZE_BYTES + 1024); + const files = await createChunkFiles({ + buffer, + basename: "episode", + extension: "mp3", + mimeType: "audio/mp3", + }); + + expect(files).toHaveLength(2); + expect(files[0].name).toBe("episode.part0.mp3"); + expect(files[1].name).toBe("episode.part1.mp3"); + for (const file of files) { + expect(file.type).toBe("audio/mp3"); + expect(file.name).not.toContain(".wav"); + } + }); + + test("decodes a large ogg to standalone WAV chunks instead of byte-splitting", async () => { + // Without the fix this returned raw .ogg byte slices that lack stream + // headers and can't be decoded standalone. Now it must produce .wav chunks. + stubAudioContext( + (buffer) => new FakeAudioBuffer(Math.ceil(buffer.byteLength / 2)), + ); + + const files = await createChunkFiles({ + buffer: new ArrayBuffer(CHUNK_SIZE_BYTES + 1024), + basename: "episode", + extension: "ogg", + mimeType: "audio/ogg", + }); + + expect(files.length).toBeGreaterThan(1); + for (const file of files) { + expect(file.type).toBe("audio/wav"); + expect(file.name).toMatch(/^episode\.part\d+\.wav$/); + expect(file.name).not.toContain(".ogg"); + } + }); + + test("throws rather than byte-splitting a large flac when no decoder is available", async () => { + // jsdom has no AudioContext, so convertToWavChunks yields nothing. The old + // code fell through to a corrupt raw byte-split; now it must throw. + await expect( + createChunkFiles({ + buffer: new ArrayBuffer(CHUNK_SIZE_BYTES + 1024), + basename: "episode", + extension: "flac", + mimeType: "audio/flac", + }), + ).rejects.toThrow(/Could not split flac audio/); + }); + + test("throws when the decoder cannot decode a large webm chunk", async () => { + stubAudioContext(() => { + throw new Error("Unsupported codec"); + }); + + await expect( + createChunkFiles({ + buffer: new ArrayBuffer(CHUNK_SIZE_BYTES + 1024), + basename: "episode", + extension: "webm", + mimeType: "audio/webm", + }), + ).rejects.toThrow(/Could not split webm audio/); + }); }); diff --git a/src/services/audioChunker.ts b/src/services/audioChunker.ts index 3c75d799..bb6485a1 100644 --- a/src/services/audioChunker.ts +++ b/src/services/audioChunker.ts @@ -1,8 +1,8 @@ /** * Pure audio chunking + WAV encoding for transcription uploads. Splits an episode - * audio buffer into request-sized files, decoding and re-encoding m4a to WAV only - * when it must be split (m4a can't be byte-split). Kept free of any - * TranscriptionService/plugin state so it can be unit-tested directly — the + * audio buffer into request-sized files, decoding and re-encoding to WAV any + * format that can't be byte-split (everything except frame-synced MP3). Kept free + * of any TranscriptionService/plugin state so it can be unit-tested directly — the * service just orchestrates it. */ @@ -31,9 +31,26 @@ export function getMimeType(fileExtension: string): string { } } +/** + * Whether a buffer too large for one request must be decoded and re-encoded to + * standalone WAV chunks instead of sliced at raw byte boundaries. + * + * Only MP3 tolerates raw byte-splitting: its frames are self-synchronizing, so a + * decoder resyncs at the next frame header even when a slice starts mid-stream. + * Every other podcast format is container/header-dependent - m4a/mp4 (ISO-BMFF), + * Ogg, WebM/Matroska (EBML) and FLAC all carry codec setup data only at the + * stream start, and WAV is raw PCM behind a leading 44-byte header - so a + * non-first arbitrary byte slice lacks the headers needed to decode standalone + * and the transcription API rejects or garbles it. Treat anything that isn't + * unambiguously MP3 as needing the WAV path (the safe default for unknown + * formats); `getMimeType` only emits `audio/mp3` for an mp3 extension, while the + * catch-all `audio/mpeg` is deliberately NOT treated as splittable. + */ export function shouldConvertToWav(extension: string, mimeType: string): boolean { const normalizedExtension = extension.toLowerCase(); - return normalizedExtension === "m4a" || mimeType === "audio/mp4"; + const isMp3 = + normalizedExtension === "mp3" || mimeType.toLowerCase() === "audio/mp3"; + return !isMp3; } export async function createChunkFiles({ @@ -58,11 +75,20 @@ export async function createChunkFiles({ return [new File([buffer], `${basename}.${extension}`, { type: mimeType })]; } + // Container/header-dependent formats must be decoded and re-encoded to WAV; + // only frame-synced MP3 falls through to raw byte-splitting. if (shouldConvertToWav(extension, mimeType)) { const wavChunks = await convertToWavChunks(buffer, basename); if (wavChunks.length > 0) { return wavChunks; } + // WAV conversion produced nothing - the platform has no Web Audio decoder + // or couldn't decode this codec. Byte-splitting the raw container here would + // hand the API undecodable chunks (garbled or rejected output saved as a + // transcript), so fail loudly and keep the run retryable instead. + throw new Error( + `Could not split ${extension || "audio"} audio for transcription. This format must be decoded to WAV before splitting, but the audio decoder is unavailable or could not decode it.`, + ); } return createBinaryChunkFiles(buffer, basename, extension, mimeType); diff --git a/src/services/diarization/segments.test.ts b/src/services/diarization/segments.test.ts index fd989b87..02e8ac9b 100644 --- a/src/services/diarization/segments.test.ts +++ b/src/services/diarization/segments.test.ts @@ -132,6 +132,16 @@ describe("formatSpeakerLabel (#168)", () => { it("prefixes the label when the template has no token", () => { expect(formatSpeakerLabel("> ", "A")).toBe("A: > "); }); + + it("inserts a label containing $-replacement patterns verbatim", () => { + // `$&`, `$\``, `$'`, `$$` are special in a String.replace replacement string; + // the label must be inserted literally rather than expanded to matched text. + const speaker = "$`$'$&$$"; + expect(formatSpeakerLabel("**{{speaker}}:** ", speaker)).toBe( + `**${speaker}:** `, + ); + expect(formatSpeakerLabel("{{speaker}} - ", speaker)).toBe(`${speaker} - `); + }); }); describe("renderDiarizedTranscript (#168)", () => { diff --git a/src/services/diarization/segments.ts b/src/services/diarization/segments.ts index 1992f522..a009908f 100644 --- a/src/services/diarization/segments.ts +++ b/src/services/diarization/segments.ts @@ -153,7 +153,10 @@ export function mergeAdjacentSpeakers( export function formatSpeakerLabel(template: string, speaker: string): string { if (SPEAKER_TOKEN.test(template)) { SPEAKER_TOKEN.lastIndex = 0; - return template.replace(SPEAKER_TOKEN, speaker); + // Pass a replacer function so the label is inserted verbatim: String.replace + // interprets `$$`, `$&`, `$\``, `$'`, `$n` in a replacement *string*, which + // would mangle a label that happened to contain one of those sequences. + return template.replace(SPEAKER_TOKEN, () => speaker); } return `${speaker}: ${template}`; }