1.5.18: Remember a track with no text and read the video id from the cached track list

[samson-art]

Closes #60

What and why

The review of #55 found two extra requests on the caption path. Caption requests use a quota per outbound IP, and every extra request can make a rate limit last longer (ADR 002, ADR 003).

1. A track that came back with no text was not cached. downloadSubtitles returned null, the caller got the list answer, and the server stored nothing. The list answer is a NotFoundError with the track lists and one next step. The same call again asked for the same track again. This was true for auto-discovery (a call without lang, ADR 006) and for a request by name (type and lang).
2. Off YouTube, the URL does not carry the video id. After a list answer, the caller names a track. handleExplicitRequestFlow downloaded the track and then called loadVideoJson only to get the id. That is one more metadata run (one yt-dlp run that reads the video info and the track list). The list answer cached that track list, with the id, one call before.

Three test cases showed both faults before the fix (see Verified).

The fix is in src/validation.ts:

• A new helper, downloadTrack, is now the only path to downloadSubtitles. Both flows call it. Before a request, it reads the mark. The mark is a cache entry sub:{url}:{type}:{lang}:{format}:empty for a track that brought no text. If the mark is there, downloadTrack returns null and makes no request. If a request brings no text, it writes the mark with the metadata TTL (CACHE_TTL_METADATA_SECONDS, 1 hour by default). The canary passes skipCache, so it neither reads nor writes the mark.
• A new helper, readCachedAvail, holds the cache read of loadAvailableSubtitles, with the same hit and miss counts. videoIdFor calls it before loadVideoJson. For the canary (skipCache), videoIdFor does not change.

The mark and the list read work only with CACHE_MODE=redis. With the cache off (the default), get returns nothing and set stores nothing, so both paths behave as before. The tool contract, the caller texts, the metric names and labels, and the per-call log line do not change. This PR adds no request and no retry.

Commits: f8036a1 (the fix, the tests and the docs), 7fdb9d3 (review 1, text only).

Decisions

The maintainer took these decisions on 2026-09-26, in the brief:

• Remember an empty track for the metadata TTL. This is the TTL that cached metadata and track lists already use. No new env var. This answers the open question of the issue: the metadata TTL, not a shorter TTL of its own.
• The CHANGELOG names the new cache key shape, because cache key shapes are an external contract.
• For the follow-up call by name off YouTube, videoIdFor reads the cached track list before it starts a metadata run.
• ADR 002, 003 and 006 stay true. ADR 006 changes where it describes an empty track or the follow-up call.
• No extra requests of any kind on the caption path.
• The branch is fix/1.5.18-caption-leftovers. The entries go under ## [Unreleased].

Acceptance criteria

• AC 1: a second identical call after an empty-track answer makes no caption request. Test in src/validation.test.ts: remembers a track that brought no text, so the same call again asks for nothing, in two cases, without lang and by name. An in-memory map backs the mocked cache get and set. Two identical calls give the same NotFoundError text, and downloadSubtitles runs once. The mark is stored with TTL 3600, the metadata TTL in the mock.
• AC 2: a call by name after a list answer off YouTube makes no metadata run while the list is cached. Test in src/validation.test.ts: names a track off YouTube after a list answer without another metadata run. A Vimeo URL with two official tracks and no reported language gives the list answer. Then a call with type: 'official' and lang: 'de' returns the track with the videoId from the cached list. fetchYtDlpJson runs once in total.
• Brief: the canary does not use the mark. The existing test skips the cache when asked, so the canary always exercises yt-dlp now also gives an empty probe. It expects no :empty key in the calls to get and set. The TTL is the metadata TTL, no env var is added, and the CHANGELOG names the key shape.

Plan

Files that change

• src/validation.ts: the new helpers downloadTrack and readCachedAvail. Auto-discovery (downloadWithAutoDiscover) and handleExplicitRequestFlow call downloadTrack. loadAvailableSubtitles calls readCachedAvail. videoIdFor reads the cached list before loadVideoJson.
• src/validation.test.ts: two new tests (one of them in two cases), a small in-memory cache helper, and the extended canary test.
• docs/adr/006-original-language-without-lang.md: "The track" describes the mark. Consequences says that the second call off YouTube makes no metadata run. A new "Don't" line says not to keep the mark longer than a track list is kept. Sources names issue Two extra caption-path requests remain after the original-language change #60.
• CHANGELOG.md: two entries under [Unreleased], Fixed. The first names the key shape. The second says that the list read counts in cache_hits_total{kind="avail"} or cache_misses_total{kind="avail"}.
• .env.example: the comment for CACHE_TTL_METADATA_SECONDS names the mark.

No env var is added, changed or removed, so the README env table does not change.

Order of work

1. Wrote the new tests and extended the canary test. Saved the red run (red-60.txt): 3 failed and 1 passed. The canary test passed, because nothing read or wrote the mark yet.
2. Added downloadTrack and readCachedAvail, and changed both flows and videoIdFor. Updated ADR 006, the CHANGELOG and .env.example. Commit f8036a1.
3. Review 1 found two minor points and no unmet criteria. Both are about the text:
   • The mark lives for the metadata TTL from the empty answer. The track list can be older, for example when get_video_info or a widget cached it first. So the mark can outlive the list by up to one TTL. In that time, a repeated auto-discovery call reads the mark but makes a new metadata run. The docs said "no request" and "as long as the track list". They now say "no caption request" and "the same time as a track list".
   • The server writes the mark for every null from downloadSubtitles. That includes the catch path for unclassified errors, for example an HTTP 5xx on the subtitle URL, a connection reset or a proxy error. The docs said "a failure about the video". They now name network errors too.
4. Fixed both in 7fdb9d3. The commit also fixes the same overclaim in a test comment that the review did not list. It changes only comments and docs.
5. After the workflow, eef41ae removes the second point in the code instead of in the docs. downloadSubtitles now returns '' for a run that went through with no text, and null for a failed run. It has one caller, downloadTrack, and only '' writes the mark. A network error or an HTTP 5xx on the track is not remembered, so the next call asks again, as before this change. Every other reader of the result checks it with !content, so '' and null act the same there.

Risks

• A run that went through with no text, for example on a video that the platform refuses without an error, answers "got no text" for up to CACHE_TTL_METADATA_SECONDS (1 hour by default) to every caller that shares the Redis. The maintainer chose the metadata TTL. A failed run is not remembered (eef41ae).
• The mark is keyed by format too, so a call in another format asks for the same track once more. This is deliberate, because a format conversion can fail on its own.
• During a hold (the process-local caption hold after a 429, ADR 002), a request by name that has a mark still answers rate_limited. assertSubtitlesNotRateLimited runs before the mark is read, because speech-to-text can follow an empty track. Auto-discovery with a cached list and a mark answers with the list and makes no run. This matches ADR 006: the server tests the hold only in front of a run.
• With speech-to-text on, a request by name that reads a mark still falls back to speech-to-text, as before. The mark saves only the caption request.
• A request by name off YouTube now counts one avail lookup: cache_hits_total{kind="avail"} or cache_misses_total{kind="avail"} grows by one. The names and labels do not change. The CHANGELOG says so.
• Two identical calls at the same time can both miss the mark and both download the track, because track downloads have no in-flight dedupe. This was true before this change.

Verified

• Red before the fix (red-60.txt, on 152a609 with the new tests):

  $ npx jest src/validation.test.ts -w 2 -t 'remembers a track that brought no text|names a track off YouTube after a list answer|skips the cache when asked'
        ✓ skips the cache when asked, so the canary always exercises yt-dlp (3 ms)
      ✕ remembers a track that brought no text, so the same call again asks for nothing (without lang) (8 ms)
      ✕ remembers a track that brought no text, so the same call again asks for nothing (by name)
      ✕ names a track off YouTube after a list answer without another metadata run (1 ms)
    ● ... › remembers a track that brought no text, so the same call again asks for nothing (without lang)
      Expected number of calls: 1
      Received number of calls: 2
      > 1710 |       expect(download).toHaveBeenCalledTimes(1);
    ● ... › remembers a track that brought no text, so the same call again asks for nothing (by name)
      Expected number of calls: 1
      Received number of calls: 2
      > 1710 |       expect(download).toHaveBeenCalledTimes(1);
    ● ... › names a track off YouTube after a list answer without another metadata run
      Expected number of calls: 1
      Received number of calls: 2
      > 1730 |     expect(metadata).toHaveBeenCalledTimes(1);
  Tests:       3 failed, 117 skipped, 1 passed, 121 total
  

• Green after the fix, on f8036a1:

  • npx jest src/validation.test.ts -w 2: 121 of 121. It ran twice, before and after a lint fix to the test.
  • npx prettier --write, npx eslint --fix and npx tsc --noEmit -p . on the changed files: no problems.

• On 7fdb9d3: npx jest src/validation.test.ts -w 2 -t 'remembers a track that brought no text|names a track off YouTube after a list answer|skips the cache when asked': 4 passed. prettier and eslint made no changes.

• make check-no-smoke (format-check, lint, typecheck, full Jest, build) passed in the pre-commit hook on f8036a1 and on 7fdb9d3. The test count of the gate was not kept. The gate also runs src/mcp-core.test.ts and src/canary.test.ts, which mock validation.js.

• Mutation drill on f8036a1: 6 of 6 mutations fail a named test. M3 ran in two parts, and both parts failed. After each mutation, npx jest src/validation.test.ts -w 2 ran, and the source was restored.

  Mutation	Tests that failed
  M1: remove the read of the :empty entry in downloadTrack.	remembers a track that brought no text, so the same call again asks for nothing (without lang) and (by name).
  M2: write the entry with ttlSubtitlesSeconds instead of ttlMetadataSeconds.	remembers a track that brought no text, so the same call again asks for nothing (without lang) and (by name).
  M3a: drop the !skipCache guard on the read, so the canary reads the entry.	skips the cache when asked, so the canary always exercises yt-dlp.
  M3b: drop the !skipCache guard on the write, so the canary writes the entry.	skips the cache when asked, so the canary always exercises yt-dlp.
  M4: videoIdFor skips readCachedAvail and calls loadVideoJson directly.	names a track off YouTube after a list answer without another metadata run.
  M5: the request by name calls downloadSubtitles directly instead of downloadTrack.	remembers a track that brought no text, so the same call again asks for nothing (by name).
  M6: auto-discovery calls downloadSubtitles directly instead of downloadTrack.	remembers a track that brought no text, so the same call again asks for nothing (without lang).

• 7fdb9d3 changes only comments and docs, so it has no mutation of its own.

• eef41ae: npx jest src/validation.test.ts src/youtube.test.ts -w 2: 248 of 248. Mutation drill, 2 of 2:

  Mutation	Tests that failed
  M7: downloadTrack writes the mark on any falsy result (!content), not only on ''.	does not remember a failed track run, so the next call asks again.
  M8: a run that went through with no text returns null again, not ''.	should return '' when the run brings no subtitle file, should return '' when the subtitle file is empty, keeps the limit when yt-dlp answers without a track.

  The pre-commit gate passed on eef41ae.

Not verified

• make check, smoke, e2e and load tests: not run. They call real YouTube from this IP, and the brief forbids them.
• A real Redis: not used. The tests use an in-memory map behind the mocked cache get and set. They assert the key shape and the TTL on the mock calls.
• A real track with no text on a platform: not tried.
• The full Jest suite by hand: not run. It ran only inside the pre-commit gate, because one heavy Jest run at a time keeps the machine in memory.
• The mutation drill after review 1: not run again. The review fix changes only text.

Not in scope

• An in-flight dedupe for track downloads.
• A change to the TTLs. The issue puts it out of scope.

After merge

After the deploy, on the server that serves real calls:

1. Find out whether the deployment sets CACHE_MODE=redis. If it does not, the mark and the list read do nothing there. The only change is one more cache_misses_total{kind="avail"} for each request by name off YouTube.
2. When a call answers with "got no text", make sure that the Redis has a key that matches sub:*:empty (redis-cli --scan --pattern 'sub:*:empty'). redis-cli TTL <key> must be at most CACHE_TTL_METADATA_SECONDS (3600 by default), not near the subtitles TTL (604800 by default).
3. Send that same call again within the TTL. It gets the same answer text. subtitle_requests_total does not grow, and the server log has no new line Downloading <type> subtitles in language <lang> for it.
4. Off YouTube, after a list answer, name one of the listed tracks. cache_hits_total{kind="avail"} grows by one for that call, and the call returns the track with a real video id, not unknown.
5. Make sure that transcriptor_canary_ok stays 1, and that Redis has no :empty key for the canary URL. The canary URL is CANARY_URL, or the default in src/canary.ts.
6. Watch the log line Error downloading subtitles. Such a failed run writes no mark (eef41ae), so the same call again asks for the track again.

🤖 Generated with Claude Code