fix: pace AniList character dictionary requests

backlog/tasks/task-85.1 - Address-PR-14-character-dictionary-review-follow-ups.md

@@ -0,0 +1,64 @@
+---
+id: TASK-85.1
+title: 'Address PR #14 character dictionary review follow-ups'
+status: Done
+assignee:
+  - codex
+created_date: '2026-03-06 07:48'
+updated_date: '2026-03-06 07:56'
+labels: []
+dependencies: []
+references:
+  - >-
+    /home/sudacode/projects/japanese/SubMiner/launcher/commands/dictionary-command.ts
+  - >-
+    /home/sudacode/projects/japanese/SubMiner/src/main/character-dictionary-runtime.ts
+  - /home/sudacode/projects/japanese/SubMiner/launcher/types.ts
+documentation:
+  - 'https://docs.anilist.co/guide/rate-limiting'
+parent_task_id: TASK-85
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+Apply the accepted follow-up fixes from Claude's PR review for the AniList character dictionary work: remove dead launcher code, deduplicate video extension handling where practical, and add explicit pacing for AniList character-page requests / character image downloads so the integration stays within AniList rate-limiting expectations.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 Launcher dictionary command no longer contains unreachable dead code after the app handoff.
+- [x] #2 Character dictionary runtime no longer maintains a separate ad hoc video extension list when existing shared extension data can be reused safely.
+- [x] #3 Character dictionary generation spaces outbound AniList-related requests with explicit named delays, and tests cover the pacing behavior and unchanged command forwarding behavior.
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+1. Add failing tests for dictionary command handoff semantics and dictionary runtime request pacing.
+2. Remove unreachable boolean return path from the launcher dictionary command while preserving call sites.
+3. Reuse the shared launcher video extension set inside the character dictionary runtime with extname normalization, then add named AniList pacing constants for page fetches and character image downloads.
+4. Run targeted tests, then broader relevant test slices, and update acceptance criteria / notes with the validated result.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Added a shared `src/shared/video-extensions.ts` source and rewired both launcher/runtime consumers to remove the duplicated runtime extension list.
+
+Replaced the hardcoded AniList page sleep with a per-generation AniList request pacer (2000ms between API requests) plus 250ms spacing between character image download attempts, including failed image fetches.
+
+Hardened `runDictionaryCommand` so an unexpected return from the `never`-typed app handoff throws immediately instead of silently falling through.
+
+Validated with targeted and adjacent test slices plus `bun run tsc --noEmit`.
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Removed the dead post-handoff return from the launcher dictionary command and replaced it with an explicit invariant error if the `never`-typed app handoff ever returns unexpectedly. Extracted video extension data into `src/shared/video-extensions.ts` so the launcher and character dictionary runtime share one source of truth.
+
+Adjusted character dictionary generation to use a per-run AniList request pacer with a conservative 2000ms delay between AniList API calls, and added 250ms spacing between character image download attempts so repeated image fetches are not bursty even when an image URL fails. Added regression coverage for the pacing behavior and the launcher handoff invariant.
+
+Validation: `bun test src/main/character-dictionary-runtime.test.ts`, `bun test launcher/commands/command-modules.test.ts`, `bun test launcher/main.test.ts launcher/parse-args.test.ts src/cli/args.test.ts src/core/services/cli-command.test.ts src/main/runtime/character-dictionary-auto-sync.test.ts`, `bun run tsc --noEmit`.
+<!-- SECTION:FINAL_SUMMARY:END -->

launcher/commands/command-modules.test.ts

@@ -115,3 +115,16 @@ test('dictionary command forwards --dictionary and target path to app binary', (
 
   assert.deepEqual(forwarded, [['--dictionary', '--dictionary-target', '/tmp/anime']]);
 });
+
+test('dictionary command throws if app handoff unexpectedly returns', () => {
+  const context = createContext();
+  context.args.dictionary = true;
+
+  assert.throws(
+    () =>
+      runDictionaryCommand(context, {
+        runAppCommandWithInherit: () => undefined as never,
+      }),
+    /unexpectedly returned/,
+  );
+});

launcher/commands/dictionary-command.ts

@@ -27,5 +27,5 @@ export function runDictionaryCommand(
   }
 
   deps.runAppCommandWithInherit(appPath, forwarded);
-  return true;
+  throw new Error('Dictionary command app handoff unexpectedly returned.');
 }

launcher/types.ts

@@ -1,18 +1,6 @@
 import path from 'node:path';
 import os from 'node:os';
-
+export { VIDEO_EXTENSIONS } from '../src/shared/video-extensions.js';
-export const VIDEO_EXTENSIONS = new Set([
-  'mkv',
-  'mp4',
-  'avi',
-  'webm',
-  'mov',
-  'flv',
-  'wmv',
-  'm4v',
-  'ts',
-  'm2ts',
-]);
 
 export const ROFI_THEME_FILE = 'subminer.rasi';
 export const DEFAULT_SOCKET_PATH = '/tmp/subminer-socket';

src/main/character-dictionary-runtime.test.ts

@@ -344,3 +344,143 @@ test('generateForCurrentMedia regenerates dictionary when cached format version
     globalThis.fetch = originalFetch;
   }
 });
+
+test('generateForCurrentMedia paces AniList requests and character image downloads', async () => {
+  const userDataPath = makeTempDir();
+  const originalFetch = globalThis.fetch;
+  const sleepCalls: number[] = [];
+  const imageRequests: string[] = [];
+
+  globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => {
+    const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url;
+    if (url === GRAPHQL_URL) {
+      const body = JSON.parse(String(init?.body ?? '{}')) as {
+        query?: string;
+      };
+
+      if (body.query?.includes('Page(perPage: 10)')) {
+        return new Response(
+          JSON.stringify({
+            data: {
+              Page: {
+                media: [
+                  {
+                    id: 130298,
+                    episodes: 20,
+                    title: {
+                      romaji: 'Kage no Jitsuryokusha ni Naritakute!',
+                      english: 'The Eminence in Shadow',
+                      native: '陰の実力者になりたくて！',
+                    },
+                  },
+                ],
+              },
+            },
+          }),
+          {
+            status: 200,
+            headers: { 'content-type': 'application/json' },
+          },
+        );
+      }
+
+      if (body.query?.includes('characters(page: $page')) {
+        return new Response(
+          JSON.stringify({
+            data: {
+              Media: {
+                title: {
+                  romaji: 'Kage no Jitsuryokusha ni Naritakute!',
+                  english: 'The Eminence in Shadow',
+                  native: '陰の実力者になりたくて！',
+                },
+                characters: {
+                  pageInfo: { hasNextPage: false },
+                  edges: [
+                    {
+                      role: 'MAIN',
+                      node: {
+                        id: 111,
+                        description: 'First character.',
+                        image: {
+                          large: 'https://example.com/alpha.png',
+                          medium: null,
+                        },
+                        name: {
+                          full: 'Alpha',
+                          native: 'アルファ',
+                        },
+                      },
+                    },
+                    {
+                      role: 'SUPPORTING',
+                      node: {
+                        id: 222,
+                        description: 'Second character.',
+                        image: {
+                          large: 'https://example.com/beta.png',
+                          medium: null,
+                        },
+                        name: {
+                          full: 'Beta',
+                          native: 'ベータ',
+                        },
+                      },
+                    },
+                  ],
+                },
+              },
+            },
+          }),
+          {
+            status: 200,
+            headers: { 'content-type': 'application/json' },
+          },
+        );
+      }
+    }
+
+    if (url === 'https://example.com/alpha.png') {
+      imageRequests.push(url);
+      return new Response('missing', {
+        status: 404,
+        headers: { 'content-type': 'text/plain' },
+      });
+    }
+
+    if (url === 'https://example.com/beta.png') {
+      imageRequests.push(url);
+      return new Response(PNG_1X1, {
+        status: 200,
+        headers: { 'content-type': 'image/png' },
+      });
+    }
+
+    throw new Error(`Unexpected fetch URL: ${url}`);
+  }) as typeof globalThis.fetch;
+
+  try {
+    const runtime = createCharacterDictionaryRuntimeService({
+      userDataPath,
+      getCurrentMediaPath: () => '/tmp/eminence-s01e05.mkv',
+      getCurrentMediaTitle: () => 'The Eminence in Shadow - S01E05',
+      resolveMediaPathForJimaku: (mediaPath) => mediaPath,
+      guessAnilistMediaInfo: async () => ({
+        title: 'The Eminence in Shadow',
+        episode: 5,
+        source: 'fallback',
+      }),
+      now: () => 1_700_000_000_000,
+      sleep: async (ms) => {
+        sleepCalls.push(ms);
+      },
+    });
+
+    await runtime.generateForCurrentMedia();
+
+    assert.deepEqual(sleepCalls, [2000, 250]);
+    assert.deepEqual(imageRequests, ['https://example.com/alpha.png', 'https://example.com/beta.png']);
+  } finally {
+    globalThis.fetch = originalFetch;
+  }
+});

src/main/character-dictionary-runtime.ts

@@ -2,8 +2,11 @@ import * as fs from 'fs';
 import * as os from 'os';
 import * as path from 'path';
 import type { AnilistMediaGuess } from '../core/services/anilist/anilist-updater';
+import { hasVideoExtension } from '../shared/video-extensions';
 
 const ANILIST_GRAPHQL_URL = 'https://graphql.anilist.co';
+const ANILIST_REQUEST_DELAY_MS = 2000;
+const CHARACTER_IMAGE_DOWNLOAD_DELAY_MS = 250;
 const HONORIFIC_SUFFIXES = [
   'さん',
   '様',
@@ -21,19 +24,6 @@ const HONORIFIC_SUFFIXES = [
   '社長',
   '部長',
 ] as const;
-const VIDEO_EXTENSIONS = new Set([
-  '.mkv',
-  '.mp4',
-  '.avi',
-  '.webm',
-  '.mov',
-  '.flv',
-  '.wmv',
-  '.m4v',
-  '.ts',
-  '.m2ts',
-]);
-
 type CharacterDictionaryRole = 'main' | 'primary' | 'side' | 'appears';
 
 type CharacterDictionaryCacheEntry = {
@@ -137,6 +127,7 @@ export interface CharacterDictionaryRuntimeDeps {
     mediaTitle: string | null,
   ) => Promise<AnilistMediaGuess | null>;
   now: () => number;
+  sleep?: (ms: number) => Promise<void>;
   logInfo?: (message: string) => void;
   logWarn?: (message: string) => void;
 }
@@ -325,8 +316,7 @@ function expandUserPath(input: string): string {
 }
 
 function isVideoFile(filePath: string): boolean {
-  const ext = path.extname(filePath).toLowerCase();
+  return hasVideoExtension(path.extname(filePath));
-  return VIDEO_EXTENSIONS.has(ext);
 }
 
 function findFirstVideoFileInDirectory(directoryPath: string): string | null {
@@ -604,7 +594,11 @@ function createStoredZip(files: Array<{ name: string; data: Buffer }>): Buffer {
 async function fetchAniList<T>(
   query: string,
   variables: Record<string, unknown>,
+  beforeRequest?: () => Promise<void>,
 ): Promise<T> {
+  if (beforeRequest) {
+    await beforeRequest();
+  }
   const response = await fetch(ANILIST_GRAPHQL_URL, {
     method: 'POST',
     headers: {
@@ -634,6 +628,7 @@ async function fetchAniList<T>(
 
 async function resolveAniListMediaIdFromGuess(
   guess: AnilistMediaGuess,
+  beforeRequest?: () => Promise<void>,
 ): Promise<ResolvedAniListMedia> {
   const data = await fetchAniList<AniListSearchResponse>(
     `
@@ -654,6 +649,7 @@ async function resolveAniListMediaIdFromGuess(
     {
       search: guess.title,
     },
+    beforeRequest,
   );
 
   const media = data.Page?.media ?? [];
@@ -664,7 +660,10 @@ async function resolveAniListMediaIdFromGuess(
   return resolved;
 }
 
-async function fetchCharactersForMedia(mediaId: number): Promise<{
+async function fetchCharactersForMedia(
+  mediaId: number,
+  beforeRequest?: () => Promise<void>,
+): Promise<{
   mediaTitle: string;
   characters: CharacterRecord[];
 }> {
@@ -708,6 +707,7 @@ async function fetchCharactersForMedia(mediaId: number): Promise<{
         id: mediaId,
         page,
       },
+      beforeRequest,
     );
 
     const media = data.Media;
@@ -744,7 +744,6 @@ async function fetchCharactersForMedia(mediaId: number): Promise<{
       break;
     }
     page += 1;
-    await sleep(300);
   }
 
   return {
@@ -805,12 +804,22 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar
 } {
   const outputDir = path.join(deps.userDataPath, 'character-dictionaries');
   const cachePath = path.join(outputDir, 'cache.json');
+  const sleepMs = deps.sleep ?? sleep;
 
   return {
     generateForCurrentMedia: async (
       targetPath?: string,
       options?: CharacterDictionaryGenerateOptions,
     ) => {
+      let hasAniListRequest = false;
+      const waitForAniListRequestSlot = async (): Promise<void> => {
+        if (!hasAniListRequest) {
+          hasAniListRequest = true;
+          return;
+        }
+        await sleepMs(ANILIST_REQUEST_DELAY_MS);
+      };
+
       const dictionaryTarget = targetPath?.trim() || '';
       const guessInput =
         dictionaryTarget.length > 0
@@ -826,7 +835,7 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar
         throw new Error('Unable to resolve current anime from media path/title.');
       }
 
-      const resolvedMedia = await resolveAniListMediaIdFromGuess(guessed);
+      const resolvedMedia = await resolveAniListMediaIdFromGuess(guessed, waitForAniListRequestSlot);
       const cache = readCache(cachePath);
       const cached = cache.anilistById[String(resolvedMedia.id)];
       const refreshTtlMsRaw = options?.refreshTtlMs;
@@ -859,6 +868,7 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar
 
       const { mediaTitle: fetchedMediaTitle, characters } = await fetchCharactersForMedia(
         resolvedMedia.id,
+        waitForAniListRequestSlot,
       );
       if (characters.length === 0) {
         throw new Error(`No characters returned for AniList media ${resolvedMedia.id}.`);
@@ -870,9 +880,14 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar
         [];
       const seen = new Set<string>();
 
+      let hasAttemptedCharacterImageDownload = false;
       for (const character of characters) {
         let imagePath: string | null = null;
         if (character.imageUrl) {
+          if (hasAttemptedCharacterImageDownload) {
+            await sleepMs(CHARACTER_IMAGE_DOWNLOAD_DELAY_MS);
+          }
+          hasAttemptedCharacterImageDownload = true;
           const image = await downloadCharacterImage(character.imageUrl, character.id);
           if (image) {
             imagePath = `img/${image.filename}`;

src/shared/video-extensions.ts

@@ -0,0 +1,17 @@
+export const VIDEO_EXTENSIONS = new Set([
+  'mkv',
+  'mp4',
+  'avi',
+  'webm',
+  'mov',
+  'flv',
+  'wmv',
+  'm4v',
+  'ts',
+  'm2ts',
+]);
+
+export function hasVideoExtension(value: string): boolean {
+  const normalized = value.trim().toLowerCase().replace(/^\./, '');
+  return normalized.length > 0 && VIDEO_EXTENSIONS.has(normalized);
+}