Skip to content

Commit 62b488a

Browse files
authored
fix(transcription): always transcribe the currently playing episode (#182)
* fix(transcription): always transcribe the currently playing episode Transcription resolved its audio through downloadEpisode(), which derived an on-disk path from the user's download-path template and reused any file already at that path without confirming it belonged to the episode. When the template was not per-episode unique (the default empty path -> ".mp3", or any path lacking {{title}}), different episodes mapped to the same file, so the transcriber read a different episode's audio and saved it under the current episode's note. Replace that path-keyed reuse with getEpisodeAudioBuffer(episode), which always yields the episode's own audio: a resolved local file, an already-downloaded copy confirmed by the downloaded-episodes registry (keyed by the episode, not a collidable path), or an in-memory fetch of the episode's stream URL. Nothing is written to the vault during transcription, so the audio can no longer collide with another episode. A non-audio response (e.g. an expired private feed) now fails clearly instead of producing a placeholder-only transcript. Note: transcription no longer writes audio to the vault or marks an episode as downloaded; it reuses an already-downloaded copy when one exists. * fix(transcription): require stream-URL match before reusing a downloaded copy The downloaded-episodes registry is keyed by podcastName+title, which two distinct episodes can share (re-releases, placeholder titles). Reusing a registry-confirmed file on title alone could still transcribe the wrong episode's audio, so also require the stored stream URL to match; otherwise fetch the episode's own audio fresh. Addresses Codex review feedback on #182. * refactor(transcription): match audio source by origin+path; flag unused downloadEpisode - Reuse a registry-confirmed download when the stream URL's origin+path match, so a rotated signed-CDN query token no longer forces a full re-download of an already-cached episode (still safe: a different episode has a different path). - Add a warning banner to the now-unused downloadEpisode noting it carries the #107 collision behavior and must not be used for transcription. Addresses adversarial review feedback on #182.
1 parent 8ad7aa5 commit 62b488a

5 files changed

Lines changed: 350 additions & 19 deletions

File tree

docs/docs/transcripts.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ To create a transcript:
1818

1919
1. Start playing the podcast episode you want to transcribe.
2020
2. Use the "Transcribe current episode" command in Obsidian.
21-
3. PodNotes will download the episode (if it hasn't been downloaded already), split it into chunks, and send these chunks to OpenAI for transcription.
21+
3. PodNotes will fetch the audio for the episode you are playing (reusing an already-downloaded copy when one exists), split it into chunks, and send these chunks to OpenAI for transcription. The transcription always uses the currently playing episode's own audio, regardless of your download path settings.
2222
4. Once the transcription is complete, a new file will be created at the specified location with the transcribed content.
2323

2424
## Transcript Template

src/TemplateEngine.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ export function TranscriptTemplateEngine(
242242
return replacer(template);
243243
}
244244

245-
function replaceIllegalFileNameCharactersInString(string: string) {
245+
export function replaceIllegalFileNameCharactersInString(string: string) {
246246
return (
247247
string
248248
// Strip characters that are illegal in file names on major platforms,

src/downloadEpisode.test.ts

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import { requestUrl, TFile } from "obsidian";
44
import {
55
detectAudioFileExtension,
66
downloadEpisode,
7+
getEpisodeAudioBuffer,
78
} from "./downloadEpisode";
89
import { downloadedEpisodes } from "./store";
910
import type { Episode } from "./types/Episode";
11+
import type { LocalEpisode } from "./types/LocalEpisode";
1012

1113
vi.mock("obsidian", async (importOriginal) => {
1214
const actual = await importOriginal<typeof import("obsidian")>();
@@ -127,3 +129,220 @@ describe("downloadEpisode (API path)", () => {
127129
expect(createBinary).not.toHaveBeenCalled();
128130
});
129131
});
132+
133+
describe("getEpisodeAudioBuffer (issue #107)", () => {
134+
const files = new Map<string, ArrayBuffer>();
135+
136+
function makeTFile(path: string): TFile {
137+
const file = new TFile();
138+
const dot = path.lastIndexOf(".");
139+
const slash = path.lastIndexOf("/");
140+
(file as { path: string }).path = path;
141+
(file as { extension: string }).extension =
142+
dot > slash ? path.slice(dot + 1) : "";
143+
(file as { basename: string }).basename = path.slice(
144+
slash + 1,
145+
dot > slash ? dot : undefined,
146+
);
147+
return file;
148+
}
149+
150+
function seedFile(path: string, text: string): void {
151+
files.set(path, new TextEncoder().encode(text).buffer);
152+
}
153+
154+
function decode(buffer: ArrayBuffer): string {
155+
return new TextDecoder().decode(new Uint8Array(buffer));
156+
}
157+
158+
function episode(overrides: Partial<Episode>): Episode {
159+
return {
160+
title: "Title",
161+
streamUrl: "https://example.com/audio.mp3",
162+
url: "https://example.com/audio",
163+
description: "",
164+
content: "",
165+
podcastName: "Pod",
166+
feedUrl: "https://example.com/feed.xml",
167+
episodeDate: undefined,
168+
...overrides,
169+
} as unknown as Episode;
170+
}
171+
172+
beforeEach(() => {
173+
files.clear();
174+
(globalThis as { app?: unknown }).app = {
175+
vault: {
176+
getAbstractFileByPath: (path: string) =>
177+
files.has(path) ? makeTFile(path) : null,
178+
readBinary: async (file: TFile) => files.get(file.path),
179+
},
180+
};
181+
requestUrlMock.mockImplementation((req: unknown) => {
182+
const url = typeof req === "string" ? req : (req as { url: string }).url;
183+
const text = url.includes("spanish") ? "SPANISH-AUDIO" : "ENGLISH-AUDIO";
184+
return Promise.resolve({
185+
status: 200,
186+
headers: { "content-type": "audio/mpeg" },
187+
arrayBuffer: new TextEncoder().encode(text).buffer,
188+
}) as unknown as ReturnType<typeof requestUrl>;
189+
});
190+
});
191+
192+
it("fetches each episode's own audio from its stream URL — never another episode's", async () => {
193+
const spanish = episode({
194+
title: "Hola soy Sere",
195+
streamUrl: "https://example.com/spanish.mp3",
196+
podcastName: "Otro Podcast",
197+
});
198+
const english = episode({
199+
title: "107. How could a PBS of the Internet...",
200+
streamUrl: "https://example.com/english.mp3",
201+
podcastName: "Reimagining the Internet",
202+
});
203+
204+
const a = await getEpisodeAudioBuffer(spanish);
205+
const b = await getEpisodeAudioBuffer(english);
206+
207+
// Each episode gets ITS OWN bytes — the wrong-episode collision is gone.
208+
expect(decode(a.buffer)).toBe("SPANISH-AUDIO");
209+
expect(decode(b.buffer)).toBe("ENGLISH-AUDIO");
210+
expect(a.extension).toBe("mp3");
211+
expect(b.extension).toBe("mp3");
212+
expect(requestUrlMock).toHaveBeenCalledWith(
213+
expect.objectContaining({ url: "https://example.com/spanish.mp3" }),
214+
);
215+
expect(requestUrlMock).toHaveBeenCalledWith(
216+
expect.objectContaining({ url: "https://example.com/english.mp3" }),
217+
);
218+
});
219+
220+
it("reuses a registry-confirmed downloaded copy without re-fetching", async () => {
221+
const ep = episode({
222+
title: "Cached Episode",
223+
streamUrl: "https://example.com/english.mp3",
224+
});
225+
seedFile("Podcasts/cached.mp3", "CACHED-CORRECT-AUDIO");
226+
downloadedEpisodes.addEpisode(ep, "Podcasts/cached.mp3", 20);
227+
228+
const result = await getEpisodeAudioBuffer(ep);
229+
230+
expect(decode(result.buffer)).toBe("CACHED-CORRECT-AUDIO");
231+
expect(result.extension).toBe("mp3");
232+
expect(result.basename).toBe("cached");
233+
expect(requestUrlMock).not.toHaveBeenCalled();
234+
});
235+
236+
it("reuses the cached file when only the signed-CDN query token changed", async () => {
237+
// Same audio file, rotated token: origin+path match, so the cache should
238+
// still be used rather than re-downloading the (large) episode.
239+
const downloaded = episode({
240+
title: "Token Episode",
241+
streamUrl: "https://cdn.example.com/ep.mp3?token=OLD&exp=1",
242+
});
243+
seedFile("Podcasts/token.mp3", "CACHED-CORRECT-AUDIO");
244+
downloadedEpisodes.addEpisode(downloaded, "Podcasts/token.mp3", 20);
245+
246+
const current = episode({
247+
title: "Token Episode",
248+
streamUrl: "https://cdn.example.com/ep.mp3?token=NEW&exp=2",
249+
});
250+
const result = await getEpisodeAudioBuffer(current);
251+
252+
expect(decode(result.buffer)).toBe("CACHED-CORRECT-AUDIO");
253+
expect(requestUrlMock).not.toHaveBeenCalled();
254+
});
255+
256+
it("does not reuse a same-title sibling's download; fetches this episode's own audio", async () => {
257+
// Same podcast + same title, different episode (different stream URL). The
258+
// registry is keyed by podcastName+title, so getEpisode() returns the
259+
// sibling's entry — reusing its file would transcribe the wrong audio.
260+
const downloaded = episode({
261+
title: "Episode",
262+
streamUrl: "https://example.com/spanish.mp3",
263+
});
264+
seedFile("Podcasts/sibling.mp3", "SPANISH-AUDIO");
265+
downloadedEpisodes.addEpisode(downloaded, "Podcasts/sibling.mp3", 13);
266+
267+
const current = episode({
268+
title: "Episode",
269+
streamUrl: "https://example.com/english.mp3",
270+
});
271+
const result = await getEpisodeAudioBuffer(current);
272+
273+
expect(decode(result.buffer)).toBe("ENGLISH-AUDIO");
274+
expect(requestUrlMock).toHaveBeenCalledWith(
275+
expect.objectContaining({ url: "https://example.com/english.mp3" }),
276+
);
277+
});
278+
279+
it("ignores a stale registry entry whose file no longer exists and fetches fresh", async () => {
280+
const ep = episode({
281+
title: "Stale Episode",
282+
streamUrl: "https://example.com/english.mp3",
283+
});
284+
// Registry points at a file that is not in the vault.
285+
downloadedEpisodes.addEpisode(ep, "Podcasts/missing.mp3", 20);
286+
287+
const result = await getEpisodeAudioBuffer(ep);
288+
289+
expect(decode(result.buffer)).toBe("ENGLISH-AUDIO");
290+
expect(requestUrlMock).toHaveBeenCalledTimes(1);
291+
});
292+
293+
it("ignores a wrong-episode file at a colliding path and fetches the right audio", async () => {
294+
// Direct reproduction of issue #107: a different episode's audio already
295+
// lives at the old collidable download-path location, and this episode has
296+
// no registry entry. The old code returned the colliding file's bytes; the
297+
// new code must fetch this episode's own audio instead.
298+
seedFile("Downloads.mp3", "WRONG-EPISODE-AUDIO");
299+
const ep = episode({
300+
title: "Right Episode",
301+
streamUrl: "https://example.com/english.mp3",
302+
});
303+
304+
const result = await getEpisodeAudioBuffer(ep);
305+
306+
expect(decode(result.buffer)).toBe("ENGLISH-AUDIO");
307+
expect(decode(result.buffer)).not.toBe("WRONG-EPISODE-AUDIO");
308+
expect(requestUrlMock).toHaveBeenCalledTimes(1);
309+
});
310+
311+
it("throws a clear error when the stream returns non-audio content", async () => {
312+
requestUrlMock.mockImplementation(
313+
() =>
314+
Promise.resolve({
315+
status: 200,
316+
headers: { "content-type": "text/html" },
317+
arrayBuffer: new TextEncoder().encode("<html>login required</html>")
318+
.buffer,
319+
}) as unknown as ReturnType<typeof requestUrl>,
320+
);
321+
const ep = episode({
322+
title: "Private Episode",
323+
streamUrl: "https://example.com/private-feed-token",
324+
});
325+
326+
await expect(getEpisodeAudioBuffer(ep)).rejects.toThrow(/not audio/i);
327+
});
328+
329+
it("reads local-file episodes from disk by their resolved path", async () => {
330+
const local = episode({
331+
title: "Local Recording",
332+
podcastName: "local file",
333+
description: "",
334+
content: "",
335+
streamUrl: "",
336+
url: "Local/recording.m4a",
337+
filePath: "Local/recording.m4a",
338+
} as Partial<LocalEpisode>) as LocalEpisode;
339+
seedFile("Local/recording.m4a", "LOCAL-AUDIO");
340+
341+
const result = await getEpisodeAudioBuffer(local);
342+
343+
expect(decode(result.buffer)).toBe("LOCAL-AUDIO");
344+
expect(result.extension).toBe("m4a");
345+
expect(result.basename).toBe("recording");
346+
expect(requestUrlMock).not.toHaveBeenCalled();
347+
});
348+
});

src/downloadEpisode.ts

Lines changed: 121 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { Notice, TFile, requestUrl } from "obsidian";
22
import { downloadedEpisodes } from "./store";
3-
import { DownloadPathTemplateEngine } from "./TemplateEngine";
3+
import {
4+
DownloadPathTemplateEngine,
5+
replaceIllegalFileNameCharactersInString,
6+
} from "./TemplateEngine";
47
import type { Episode } from "./types/Episode";
58
import type { LocalEpisode } from "./types/LocalEpisode";
69
import { encodeUrlForRequest } from "./utility/encodeUrlForRequest";
@@ -265,6 +268,14 @@ function inferFileExtensionFromDownload(
265268
return getExtensionFromContentType(contentType);
266269
}
267270

271+
/**
272+
* UNUSED IN PRODUCTION — kept only for the #178 tests. Do NOT use this for
273+
* transcription: it derives the on-disk path from the download-path template and
274+
* reuses any file already there without confirming episode identity, which is
275+
* the exact collision behind issue #107. Use {@link getEpisodeAudioBuffer} (for
276+
* transcription) or {@link downloadEpisodeWithNotice} (for the Download command)
277+
* instead. Candidate for removal in a follow-up.
278+
*/
268279
export async function downloadEpisode(
269280
episode: Episode,
270281
downloadPathTemplate: string,
@@ -334,6 +345,115 @@ async function getFileExtension(url: string): Promise<string> {
334345
return "mp3";
335346
}
336347

348+
/**
349+
* Resolves the audio bytes for an episode for transcription.
350+
*
351+
* The returned bytes always belong to the given episode, regardless of the
352+
* user's download-path template. This is the fix for issue #107: transcription
353+
* previously went through downloadEpisode(), which derived an on-disk path from
354+
* the download-path template and reused whatever file already lived there, so
355+
* episodes that mapped to the same (non-unique) path — e.g. the default empty
356+
* path, or any path without `{{title}}` — were transcribed using a different
357+
* episode's audio.
358+
*
359+
* Resolution order:
360+
* 1. Local files already resolve to a unique, episode-specific path on disk.
361+
* 2. An already-downloaded copy is reused only when the downloaded-episodes
362+
* registry (keyed by the episode, not a collidable path) confirms the file
363+
* belongs to THIS episode — preserving the cache without the collision risk.
364+
* 3. Otherwise the episode's own stream URL is fetched into memory. Nothing is
365+
* written to the vault, so the audio can never collide with another episode.
366+
*/
367+
export async function getEpisodeAudioBuffer(
368+
episode: Episode,
369+
): Promise<{ buffer: ArrayBuffer; extension: string; basename: string }> {
370+
if (isLocalFile(episode)) {
371+
const localFilePath = resolveLocalEpisodeFilePath(episode);
372+
if (!localFilePath) {
373+
throw new Error(
374+
`Unable to locate the local audio file for "${episode.title}". Try playing the file again.`,
375+
);
376+
}
377+
378+
return readVaultAudio(localFilePath);
379+
}
380+
381+
// Reuse a previously downloaded file only when the registry entry is the SAME
382+
// episode. The registry is keyed by podcastName+title, which two distinct
383+
// episodes can share (re-releases, placeholder titles), so also require the
384+
// stream source to match before trusting the cached bytes. Comparison is by
385+
// origin+path so a rotated signed-CDN query token (?token=...) still hits the
386+
// cache; a genuinely different episode has a different path and falls through
387+
// to a fresh fetch — correct, just not cached.
388+
const registered = downloadedEpisodes.getEpisode(episode);
389+
if (
390+
registered?.filePath &&
391+
isSameAudioSource(registered.streamUrl, episode.streamUrl)
392+
) {
393+
const existingFile = app.vault.getAbstractFileByPath(registered.filePath);
394+
if (existingFile instanceof TFile) {
395+
return readVaultAudio(registered.filePath);
396+
}
397+
}
398+
399+
try {
400+
const { data, contentType } = await downloadFile(episode.streamUrl);
401+
const inferredExtension = inferFileExtensionFromDownload(
402+
episode,
403+
data,
404+
contentType,
405+
);
406+
const normalizedType = contentType.toLowerCase();
407+
const typeAppearsAudio =
408+
normalizedType === "" || normalizedType.includes("audio");
409+
if (!typeAppearsAudio && !inferredExtension) {
410+
throw new Error(
411+
`The downloaded file is not audio (received "${contentType}"). The episode may be unavailable or require re-authentication.`,
412+
);
413+
}
414+
415+
return {
416+
buffer: data,
417+
extension: inferredExtension ?? "mp3",
418+
basename:
419+
replaceIllegalFileNameCharactersInString(episode.title) || "episode",
420+
};
421+
} catch (error: unknown) {
422+
throw new Error(
423+
`Failed to fetch ${episode.title}: ${getErrorMessage(error)}`,
424+
);
425+
}
426+
}
427+
428+
/**
429+
* Whether two stream URLs point at the same audio file. Compares origin+path so
430+
* rotating query strings (signed-CDN tokens, cache-busters) don't count as a
431+
* different source. Falls back to exact equality for non-absolute/odd URLs.
432+
*/
433+
function isSameAudioSource(a: string, b: string): boolean {
434+
if (a === b) return true;
435+
if (!a || !b) return false;
436+
try {
437+
const ua = new URL(a);
438+
const ub = new URL(b);
439+
return ua.origin === ub.origin && ua.pathname === ub.pathname;
440+
} catch {
441+
return false;
442+
}
443+
}
444+
445+
async function readVaultAudio(
446+
filePath: string,
447+
): Promise<{ buffer: ArrayBuffer; extension: string; basename: string }> {
448+
const file = app.vault.getAbstractFileByPath(filePath);
449+
if (!(file instanceof TFile)) {
450+
throw new Error(`Unable to read the audio file at "${filePath}".`);
451+
}
452+
453+
const buffer = await app.vault.readBinary(file);
454+
return { buffer, extension: file.extension, basename: file.basename };
455+
}
456+
337457
interface AudioSignature {
338458
signature: number[];
339459
mask?: number[];

0 commit comments

Comments
 (0)