From 2dcf21d805035c042644bca2d5db9c86e76b9863 Mon Sep 17 00:00:00 2001 From: Artem Samsonov Date: Sat, 26 Sep 2026 20:05:42 +0200 Subject: [PATCH 1/2] Fail the canary probe on an empty track instead of transcribing it With speech-to-text on, an empty caption track on the canary video got a speech-to-text answer. The canary counted that answer as a working caption path: it set transcriptor_canary_ok to 1 and could end a failure streak while captions failed. The probe now passes skipWhisper, so an empty track is a failed probe and no transcription runs. A speech-to-text answer that came after WHISPER_TIMEOUT also went into the cache for a call with skipCache. The late write now checks skipCache like the other cache writes. ADR 003 now says that the probe never falls back to speech-to-text. Fixes #59. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +++ docs/adr/003-caption-request-budget.md | 11 ++++--- src/canary.test.ts | 27 ++++++++++++++-- src/canary.ts | 4 ++- src/validation.test.ts | 44 ++++++++++++++++++++++++++ src/validation.ts | 18 +++++++---- 6 files changed, 93 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d806435..290a9a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- With speech-to-text on, the canary counted an empty caption track as a working path when speech-to-text answered the probe. It set `transcriptor_canary_ok` to 1 and could end a failure streak while captions failed. The probe now never falls back to speech-to-text. An empty track is a failed probe: the gauge goes to 0 and the streak goes on. The probe also no longer costs a transcription. Its empty track does not count in `subtitles_extraction_failures_total{reason="no_subtitles"}`, which counts a failure only when speech-to-text ran. +- A speech-to-text answer that came after `WHISPER_TIMEOUT` went into the cache, also for a call that skips the cache. The canary probe skips the cache, so its late answer could land under the key of the canary video. Real calls for that video then got speech-to-text text instead of captions until the entry expired (`CACHE_TTL_SUBTITLES_SECONDS`, 7 days by default). A call that skips the cache now writes nothing to it, also late. + ## [1.5.13] - 2026-09-26 ### Changed diff --git a/docs/adr/003-caption-request-budget.md b/docs/adr/003-caption-request-budget.md index 1830ba9..5270700 100644 --- a/docs/adr/003-caption-request-budget.md +++ b/docs/adr/003-caption-request-budget.md @@ -1,8 +1,8 @@ # 003. Auto-discovery asks for at most two ranked tracks (superseded by 006), and the canary stands down while real traffic proves the path - Status: Accepted. The ladder part is superseded by [006](006-original-language-without-lang.md): one track request, or the track list. The canary part stands. -- Date: 2026-09-22 (1.5.4), canary stand-down narrowed to real traffic 2026-09-25 -- Sources: PR #39 (2b837f0), PR #37 operator notes, PR #40 (0303698), CHANGELOG 1.5.4, issue #48 +- Date: 2026-09-22 (1.5.4), canary stand-down narrowed to real traffic 2026-09-25, probe without speech-to-text 2026-09-26 +- Sources: PR #39 (2b837f0), PR #37 operator notes, PR #40 (0303698), CHANGELOG 1.5.4, issues #48 and #59 ## Context @@ -16,7 +16,7 @@ In `src/validation.ts`: - *(Superseded by ADR 006.)* The ladder (the ordered list of track requests) asks for at most `AUTO_DISCOVERY_ATTEMPTS = 2` tracks. This is a module constant, not an env var. It alternates between the ranked official and automatic lists. When a video lists only one kind, it asks for the two best of that kind. - *(Superseded by ADR 006.)* `subtitle_tracks_untried_total{platform}` counts the tracks that the cap left unasked. It counts them only for a ladder that came back empty. -In `src/canary.ts`, a tick is skipped when a track from the canary URL's platform came back within the last `CANARY_INTERVAL_MS`. That track must be newer than the track of the last probe that returned one. After a 429 (ADR 002), no tick is skipped until a track comes back. During the hold the probe stops at the hold with no request. After the hold the probe asks the platform. A skipped tick counts as a success: it sets `transcriptor_canary_ok` to 1, ends a failure streak, and reports the recovery the same way a probe does. +In `src/canary.ts`, a tick is skipped when a track from the canary URL's platform came back within the last `CANARY_INTERVAL_MS`. That track must be newer than the track of the last probe that returned one. After a 429 (ADR 002), no tick is skipped until a track comes back. During the hold the probe stops at the hold with no request. After the hold the probe asks the platform. A skipped tick counts as a success: it sets `transcriptor_canary_ok` to 1, ends a failure streak, and reports the recovery the same way a probe does. The probe never falls back to speech-to-text. An empty track is a failed probe. The probe does not read the cache and does not write to it. ## Alternatives @@ -29,11 +29,12 @@ In `src/canary.ts`, a tick is skipped when a track from the canary URL's platfor - *(Superseded by ADR 006.)* Other tracks can be listed, but a video whose two best tracks both fail still answers "no subtitles". It can then fall back to Whisper. The "no subtitles" text states the cap. To see the cost, compare `subtitle_tracks_untried_total` with the "no subtitles" answers. `subtitles_extraction_failures_total{reason="no_subtitles"}` counts them only for a `WHISPER_MODE` other than `off`. With Whisper off (the default), use the `not_found` outcome of `get_transcript` in the per-call log line or in `mcp_tool_errors_total`. - The canary makes no caption requests while real traffic keeps returning tracks. An idle server probes once per interval: 96 times a day at the default 15 minutes, 24 at one hour. -- A track from a real call that comes back while a probe runs to success is taken for the probe's own. When Whisper answers the probe, the run includes the whole transcription. The next tick may then probe once more than it had to. A probe that fails, or stops at a busy server or a hold, hides no real track. +- A track from a real call that comes back while a probe runs to success is taken for the probe's own. The next tick may then probe once more than it had to. A probe that fails, or stops at a busy server or a hold, hides no real track. ## Do not - *(Superseded by ADR 006.)* Do not raise the cap or walk every track in answer to one "no subtitles for a video that has some" report. Look at the untried metric first. - Do not make the canary always probe "to be safe". It spends caption quota. (The rule about the ranking is superseded by ADR 006.) - Do not count the canary's own track as traffic again. It halves the probes on an idle server and delays the alert. -- Guarded by `src/canary.test.ts`. The ladder tests went with the ladder (ADR 006). +- Do not let speech-to-text answer the probe. Its answer set the gauge to 1 while captions failed (#59). +- Guarded by `src/canary.test.ts` and by the probe tests in `src/validation.test.ts`. The ladder tests went with the ladder (ADR 006). diff --git a/src/canary.test.ts b/src/canary.test.ts index 7790999..e779641 100644 --- a/src/canary.test.ts +++ b/src/canary.test.ts @@ -1,6 +1,6 @@ import * as Sentry from '@sentry/node'; -import { ServerBusyError, YtDlpError } from './errors.js'; +import { NotFoundError, ServerBusyError, YtDlpError } from './errors.js'; import { runCanary, startCanary, resetCanaryForTests } from './canary.js'; import { renderPrometheus } from './metrics.js'; import { @@ -106,7 +106,7 @@ describe('canary', () => { expect(validateAndDownloadSubtitlesMock).toHaveBeenCalled(); }); - it('probes CANARY_URL with one explicit language and skips the cache', async () => { + it('probes CANARY_URL with one explicit language and skips the cache and speech-to-text', async () => { process.env.CANARY_URL = 'https://www.youtube.com/watch?v=other123'; validateAndDownloadSubtitlesMock.mockResolvedValue({ subtitlesContent: 'hello' }); @@ -119,7 +119,28 @@ describe('canary', () => { lang: 'en', }), expect.anything(), - { skipCache: true } + { skipCache: true, skipWhisper: true } + ); + }); + + it('counts an empty track as a failure when speech-to-text is on', async () => { + // Like the real call with speech-to-text on: an empty track gets a speech-to-text answer + // unless the caller turns the fallback off. That answer says nothing about captions. + const logger = createLogger(); + validateAndDownloadSubtitlesMock.mockImplementation((_request, _log, opts) => + opts?.skipWhisper + ? Promise.reject(new NotFoundError('No official subtitles.', 'Subtitles not found')) + : Promise.resolve({ subtitlesContent: 'speech', source: 'whisper' }) + ); + + await runCanary(logger as any); + await runCanary(logger as any); + + expect(await renderPrometheus()).toMatch(/^transcriptor_canary_ok\{[^}]*\} 0$/m); + expect(captureMessageMock).toHaveBeenCalledTimes(1); + expect(captureMessageMock).toHaveBeenCalledWith( + 'canary: transcript path failing', + expect.objectContaining({ tags: { reason: 'not_found', canary: 'true' } }) ); }); diff --git a/src/canary.ts b/src/canary.ts index 97e12e2..f068b03 100644 --- a/src/canary.ts +++ b/src/canary.ts @@ -5,7 +5,8 @@ * so the failure shows up as a metric and one alert instead of user reports. * * The probe bypasses the response cache: a cached fixture would prove Redis works, - * not that yt-dlp still reaches YouTube. + * not that yt-dlp still reaches YouTube. It also skips speech-to-text: an empty track is a + * failed probe, because a transcription says nothing about the caption path (#59). */ import * as Sentry from '@sentry/node'; import type { FastifyBaseLogger } from 'fastify'; @@ -64,6 +65,7 @@ export async function runCanary(log: FastifyBaseLogger): Promise { // front of it (the YouTube URL already carries the id); auto-discovery would add one. await validateAndDownloadSubtitles({ url, type: 'official', lang: 'en' }, log, { skipCache: true, + skipWhisper: true, }); // ponytail: a real track that lands while a probe runs to success is taken for the // probe's own, so the next tick may probe once more than it had to; a per-call origin diff --git a/src/validation.test.ts b/src/validation.test.ts index a5e2326..5bf759e 100644 --- a/src/validation.test.ts +++ b/src/validation.test.ts @@ -682,6 +682,50 @@ describe('validation', () => { }); }); + it('does not write a late speech-to-text answer to the cache when skipCache is set', async () => { + (cacheSet as jest.Mock).mockClear(); + jest.spyOn(youtube, 'downloadSubtitles').mockResolvedValue(null); + (whisper.getWhisperConfig as jest.Mock).mockReturnValue({ mode: 'local', timeout: 1 }); + let lateResolve!: (v: string | null) => void; + (whisperJobs.startOrReuseWhisperJob as jest.Mock).mockReturnValue( + new Promise((resolve) => { + lateResolve = resolve; + }) + ); + + await expect( + validateAndDownloadSubtitles( + { url: 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', type: 'auto', lang: 'en' } as any, + undefined, + { skipCache: true } + ) + ).rejects.toThrow(NotFoundError); + lateResolve('1\n00:00:00,000 --> 00:00:01,000\nLate probe'); + await new Promise((resolve) => setImmediate(resolve)); + + expect(cacheSet).not.toHaveBeenCalled(); + }); + + it('does not start speech-to-text for a probe whose track is empty', async () => { + // The canary's options: an answer from speech-to-text would pass the probe while captions fail. + const probe = { skipCache: true, skipWhisper: true }; + jest.spyOn(youtube, 'downloadSubtitles').mockResolvedValue(null); + (whisper.getWhisperConfig as jest.Mock).mockReturnValue({ mode: 'local', timeout: 600_000 }); + (whisperJobs.startOrReuseWhisperJob as jest.Mock) + .mockClear() + .mockResolvedValue('1\n00:00:00,000 --> 00:00:01,000\nWhisper transcript'); + + const err = await validateAndDownloadSubtitles( + { url: 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', type: 'official', lang: 'en' } as any, + undefined, + probe + ).catch((e: unknown) => e); + + expect(err).toBeInstanceOf(NotFoundError); + expect((err as Error).message).not.toMatch(/speech-to-text/i); + expect(whisperJobs.startOrReuseWhisperJob).not.toHaveBeenCalled(); + }); + it('should throw NotFoundError when Whisper fallback is enabled but returns null', async () => { jest.spyOn(youtube, 'downloadSubtitles').mockResolvedValue(null); jest.spyOn(youtube, 'fetchYtDlpJson').mockResolvedValue({ diff --git a/src/validation.ts b/src/validation.ts index f988c4e..0da9378 100644 --- a/src/validation.ts +++ b/src/validation.ts @@ -778,7 +778,7 @@ async function handleExplicitRequestFlow( request: GetSubtitlesRequest, url: string, logger?: FastifyBaseLogger, - skipCache = false + { skipCache = false, skipWhisper = false } = {} ): Promise { const type = request.type ?? 'auto'; const format = request.format as SubtitleFormat | undefined; @@ -805,16 +805,19 @@ async function handleExplicitRequestFlow( (skipCache ? null : await loadVideoJson(url, logger))?.info.videoId ?? 'unknown'; + // The canary skips speech-to-text: its answer would pass the probe while captions fail (#59). + const whisperConfig = getWhisperConfig(); + const transcribe = !skipWhisper && whisperConfig.mode !== 'off'; if (!subtitlesContent) { - const whisperConfig = getWhisperConfig(); - if (whisperConfig.mode !== 'off') { + if (transcribe) { logger?.info({ lang: sanitizedLang }, 'Trying Whisper fallback'); const job = startOrReuseWhisperJob(url, sanitizedLang, 'srt', logger); const outcome = await raceWhisperJob(job, whisperConfig.timeout); if (outcome.kind === 'timeout') { void job.then(async (text) => { - if (!text?.trim()) { + // A call that skips the cache does not fill it later either (#59). + if (skipCache || !text?.trim()) { return; } const vid = await videoIdFor(); @@ -840,7 +843,7 @@ async function handleExplicitRequestFlow( // A lang without a type means the server substituted the type, and the caller cannot // see that unless the text says so. asked: { type, lang: sanitizedLang, defaulted: request.type === undefined }, - whisperTried: getWhisperConfig().mode !== 'off', + whisperTried: transcribe, logger, }); } @@ -868,7 +871,8 @@ async function handleExplicitRequestFlow( export async function validateAndDownloadSubtitles( request: GetSubtitlesRequest, logger?: FastifyBaseLogger, - opts?: { skipCache?: boolean } + /** The canary's options. They apply only to a request that names lang. */ + opts?: { skipCache?: boolean; skipWhisper?: boolean } ): Promise { const validated = validateVideoRequest(request.url); const { url } = validated; @@ -877,7 +881,7 @@ export async function validateAndDownloadSubtitles( if (request.lang === undefined) { return await handleAutoDiscoverFlow(request, url, logger); } - return await handleExplicitRequestFlow(request, url, logger, opts?.skipCache); + return await handleExplicitRequestFlow(request, url, logger, opts); } catch (err) { if (err instanceof YtDlpError) recordSubtitlesFailure(url, err.reason); throw err; From e19a890cb4131801972df30916eba2620013254c Mon Sep 17 00:00:00 2001 From: Artem Samsonov Date: Sat, 26 Sep 2026 20:11:26 +0200 Subject: [PATCH 2/2] Say which cache entries the canary probe leaves alone The review of #59 found two sentences that claim too much. A failed probe still fills the track-list, info and chapters entries: the "no subtitles" answer reads the track list, and a miss runs one metadata run. ADR 003 now says only that the probe does not read a cached transcript and does not store one. The CHANGELOG entry now says that the late speech-to-text answer went under the official English track of the canary video. Only calls that asked for that track by name read it: auto-discovery skips a speech-to-text answer stored under a track name. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- docs/adr/003-caption-request-budget.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 290a9a6..e6359c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - With speech-to-text on, the canary counted an empty caption track as a working path when speech-to-text answered the probe. It set `transcriptor_canary_ok` to 1 and could end a failure streak while captions failed. The probe now never falls back to speech-to-text. An empty track is a failed probe: the gauge goes to 0 and the streak goes on. The probe also no longer costs a transcription. Its empty track does not count in `subtitles_extraction_failures_total{reason="no_subtitles"}`, which counts a failure only when speech-to-text ran. -- A speech-to-text answer that came after `WHISPER_TIMEOUT` went into the cache, also for a call that skips the cache. The canary probe skips the cache, so its late answer could land under the key of the canary video. Real calls for that video then got speech-to-text text instead of captions until the entry expired (`CACHE_TTL_SUBTITLES_SECONDS`, 7 days by default). A call that skips the cache now writes nothing to it, also late. +- A speech-to-text answer that came after `WHISPER_TIMEOUT` went into the cache, also for a call that skips the cache. The canary probe skips the cache, so a late answer to it could be stored as the official English track of the canary video. Calls that asked for that track by name (`type: "official"`, `lang: "en"`) then got speech-to-text text instead of captions until the entry expired (`CACHE_TTL_SUBTITLES_SECONDS`, 7 days by default). A call that skips the cache now stores no transcript, also late. ## [1.5.13] - 2026-09-26 diff --git a/docs/adr/003-caption-request-budget.md b/docs/adr/003-caption-request-budget.md index 5270700..65e9254 100644 --- a/docs/adr/003-caption-request-budget.md +++ b/docs/adr/003-caption-request-budget.md @@ -16,7 +16,7 @@ In `src/validation.ts`: - *(Superseded by ADR 006.)* The ladder (the ordered list of track requests) asks for at most `AUTO_DISCOVERY_ATTEMPTS = 2` tracks. This is a module constant, not an env var. It alternates between the ranked official and automatic lists. When a video lists only one kind, it asks for the two best of that kind. - *(Superseded by ADR 006.)* `subtitle_tracks_untried_total{platform}` counts the tracks that the cap left unasked. It counts them only for a ladder that came back empty. -In `src/canary.ts`, a tick is skipped when a track from the canary URL's platform came back within the last `CANARY_INTERVAL_MS`. That track must be newer than the track of the last probe that returned one. After a 429 (ADR 002), no tick is skipped until a track comes back. During the hold the probe stops at the hold with no request. After the hold the probe asks the platform. A skipped tick counts as a success: it sets `transcriptor_canary_ok` to 1, ends a failure streak, and reports the recovery the same way a probe does. The probe never falls back to speech-to-text. An empty track is a failed probe. The probe does not read the cache and does not write to it. +In `src/canary.ts`, a tick is skipped when a track from the canary URL's platform came back within the last `CANARY_INTERVAL_MS`. That track must be newer than the track of the last probe that returned one. After a 429 (ADR 002), no tick is skipped until a track comes back. During the hold the probe stops at the hold with no request. After the hold the probe asks the platform. A skipped tick counts as a success: it sets `transcriptor_canary_ok` to 1, ends a failure streak, and reports the recovery the same way a probe does. The probe never falls back to speech-to-text. An empty track is a failed probe. The probe does not read a cached transcript and does not store one. ## Alternatives