fix(subtitles): release prefetch pause across all priming paths

The repeated-subtitle pause leak was only fixed for ordinary subtitle changes. Startup autoplay priming and visible-overlay priming pause the same way and also ignored whether any tokenization was scheduled, so a cache miss on text the controller already holds (mining a card while the line is on screen) left prefetching idle for the rest of the cue. Pause and release are now one operation via pausePrefetchUntilEmit, and both controller entry points report whether an emit is expected. A repeat arriving while a run is already in flight keeps the pause, since that run still emits. Also invalidate the character dictionary lookups centrally from the sync completion handler instead of at three manager call sites. Ordinary selection sync never invalidated them, so a stale non-null name candidate list could skip a newly added name for up to five seconds; a missing list falls back to the exhaustive scan, but a stale one does not. Ordering matters: the invalidation runs before the subtitle refreshes so they re-tokenize against the new dictionary content. Docs: subtitle-overlay-priming no longer claims every subtitle change calls onSeek().
2026-08-04 07:21:32 -07:00 · 2026-08-04 00:01:14 -07:00
parent f43674cc39
commit 2003efa235
8 changed files with 200 additions and 37 deletions
@@ -9,5 +9,6 @@ area: subtitles
 - Subtitle changes no longer restart the prefetch run per line (which discarded in-flight tokenization work); prefetch now only pauses for the live line and restarts on real seeks, cache invalidation, or option changes. Prefetch also stays paused across a provisional raw-subtitle emit and resumes only after the tokenized payload lands, so it never competes with the on-screen line for the parser window.
 - Added per-stage debug timings (`scanMs`, `mecabMs`, `frequencyMs`, `annotateMs`) to the subtitle tokenization pipeline log.
 - Fixed a reading that stopped covering its surface when an unmatched kana run extended the preceding token (for example a trailing る on 待ち合わせ), which silently disabled the known-word reading fallback for those tokens.
- Subtitle prefetching no longer stays paused for the rest of a cue when the same subtitle text is reported twice and there is nothing to tokenize.
+- Subtitle prefetching no longer stays paused for the rest of a cue when the same subtitle text is reported twice and there is nothing to tokenize. This covers the startup and overlay priming paths as well as ordinary subtitle changes.
+- Character name and image lookups are now refreshed centrally whenever a character dictionary sync changes its content, so a newly added name can no longer be skipped by a stale candidate list.
 - Character name annotations no longer cost a dictionary lookup at every position in a line. The scanner now knows which name forms the current title's character dictionary actually contains and only checks where one can start, which removes the whole overhead of having the character dictionary enabled (measured: 21 lookups per line down to 10, the same as with it disabled). Titles with no cached character data keep the previous exhaustive scan, so a missing snapshot costs speed rather than a missing name.
@@ -50,10 +50,16 @@ subtitles do not draw.
 7. Cache miss: call `refreshCurrentSubtitle(text)`. Normal processing emits a plain payload
   synchronously, then replaces it with the tokenized payload when ready.

-In `src/main.ts`, both `onSubtitleChange` and `refreshCurrentSubtitle` pause
-`subtitlePrefetchService`, notify it with `onSeek(lastObservedTimePos)`, and then call the matching
-`subtitleProcessingController` method. This gives the visible overlay priority over background
-prefetch work and re-centers prefetch around the live playback time.
+Both `onSubtitleChange` and `refreshCurrentSubtitle` pause `subtitlePrefetchService` and then call
+the matching `subtitleProcessingController` method, giving the visible overlay priority over
+background prefetch work. Prefetch is not re-centered here: restarting the run per line
+(`onSeek`) discarded the in-flight tokenization every time the subtitle changed, so only real
+seeks restart it (see `onTimePosUpdate` in `src/main.ts`).
+
+The pause is released by the emit that carries the tokenized payload. Both controller methods
+return whether an emit is expected, and the caller resumes immediately when it is not — otherwise
+a repeated subtitle (which schedules no work) would leave prefetching idle for the rest of the
+cue.

 ## Live Cue Delivery

@@ -23,7 +23,8 @@ export interface SubtitleProcessingController {
   * gate work on the emit (such as pausing subtitle prefetching) need to know.
   */
  onSubtitleChange: (text: string) => boolean;
-  refreshCurrentSubtitle: (textOverride?: string) => void;
+  /** Same contract as onSubtitleChange: whether an emit is expected. */
+  refreshCurrentSubtitle: (textOverride?: string) => boolean;
  invalidateTokenizationCache: () => void;
  preCacheTokenization: (text: string, data: SubtitleData) => void;
  consumeCachedSubtitle: (text: string) => SubtitleData | null;
@@ -176,7 +177,8 @@ export function createSubtitleProcessingController(
  return {
    onSubtitleChange: (text: string) => {
      if (text === latestText) {
-        return false;
+        // A run already in flight for this text will still emit for it.
+        return processing;
      }
      latestText = text;
      if (
@@ -195,15 +197,16 @@ export function createSubtitleProcessingController(
        latestText = textOverride;
      }
      if (!latestText.trim()) {
-        return;
+        return false;
      }
-      if (
-        processing ||
-        (latestText === lastEmittedText && cacheGeneration === lastEmittedGeneration)
-      ) {
-        return;
+      if (processing) {
+        return true;
+      }
+      if (latestText === lastEmittedText && cacheGeneration === lastEmittedGeneration) {
+        return false;
      }
      processLatest();
+      return true;
    },
    invalidateTokenizationCache: () => {
      tokenizationCache.clear();
@@ -2538,6 +2538,10 @@ const characterDictionaryAutoSyncRuntime = createCharacterDictionaryAutoSyncRunt
      },
      {
        hasParserWindow: () => Boolean(appState.yomitanParserWindow),
+        invalidateCharacterDictionaryLookups: () => {
+          characterDictionaryImageLookup.invalidate();
+          characterNameCandidateLookup.invalidate();
+        },
        clearParserCaches: () => {
          if (appState.yomitanParserWindow) {
            clearYomitanParserCachesForWindow(appState.yomitanParserWindow);
@@ -5692,9 +5696,6 @@ const { registerIpcRuntimeHandlers } = composeIpcRuntimeHandlers({
          if (result.ok && result.rebuildRequired) {
            try {
              await characterDictionaryAutoSyncRuntime.runSyncNow();
-              characterDictionaryImageLookup.invalidate();
-              characterNameCandidateLookup.invalidate();
-              characterNameCandidateLookup.invalidate();
            } catch (error) {
              logger.warn('Failed to rebuild character dictionary after manager override:', error);
            }
@@ -5725,8 +5726,6 @@ const { registerIpcRuntimeHandlers } = composeIpcRuntimeHandlers({
        if (result.ok && result.rebuildRequired) {
          try {
            await characterDictionaryAutoSyncRuntime.runSyncNow();
-            characterDictionaryImageLookup.invalidate();
-            characterNameCandidateLookup.invalidate();
          } catch (error) {
            logger.warn('Failed to rebuild character dictionary after manager removal:', error);
          }
@@ -5743,8 +5742,6 @@ const { registerIpcRuntimeHandlers } = composeIpcRuntimeHandlers({
        if (result.ok && result.rebuildRequired) {
          try {
            await characterDictionaryAutoSyncRuntime.runSyncNow();
-            characterDictionaryImageLookup.invalidate();
-            characterNameCandidateLookup.invalidate();
          } catch (error) {
            logger.warn('Failed to rebuild character dictionary after manager reorder:', error);
          }
@@ -42,8 +42,8 @@ test('scheduleSubtitlePrefetchRefresh logs refresh failures from timer callback'
    setActiveParsedSubtitleMediaPath: () => {},
    subtitleProcessingController: {
      consumeCachedSubtitle: () => null,
-      onSubtitleChange: () => {},
-      refreshCurrentSubtitle: () => {},
+      onSubtitleChange: () => true,
+      refreshCurrentSubtitle: () => true,
    },
    emitSubtitlePayload: () => {},
    getSubtitlePrefetchService: () => null,
@@ -93,13 +93,24 @@ test('primeCurrentSubtitleForAutoplay refreshes active subtitle cues when mpv su
    setActiveParsedSubtitleMediaPath: () => {},
    subtitleProcessingController: {
      consumeCachedSubtitle: () => null,
-      onSubtitleChange: (text) => calls.push(`change:${text}`),
-      refreshCurrentSubtitle: (text) => calls.push(`refresh:${text ?? ''}`),
+      onSubtitleChange: (text) => {
+        calls.push(`change:${text}`);
+        return true;
+      },
+      refreshCurrentSubtitle: (text) => {
+        calls.push(`refresh:${text ?? ''}`);
+        return true;
+      },
    },
    emitSubtitlePayload: (payload, options) =>
      calls.push(`emit:${payload.text}:resume=${options?.resumePrefetch !== false}`),
    getSubtitlePrefetchService: () => ({
-      pause: () => calls.push('prefetch:pause'),
+      pause: () => {
+        calls.push('prefetch:pause');
+      },
+      resume: () => {
+        calls.push('prefetch:resume');
+      },
    }),
    getLastObservedTimePos: () => 12,
    getVisibleOverlayVisible: () => true,
@@ -151,13 +162,24 @@ test('primeCurrentSubtitleForAutoplay emits raw first paint on cache miss before
    setActiveParsedSubtitleMediaPath: () => {},
    subtitleProcessingController: {
      consumeCachedSubtitle: () => null,
-      onSubtitleChange: (text) => calls.push(`change:${text}`),
-      refreshCurrentSubtitle: (text) => calls.push(`refresh:${text ?? ''}`),
+      onSubtitleChange: (text) => {
+        calls.push(`change:${text}`);
+        return true;
+      },
+      refreshCurrentSubtitle: (text) => {
+        calls.push(`refresh:${text ?? ''}`);
+        return true;
+      },
    },
    emitSubtitlePayload: (payload, options) =>
      calls.push(`emit:${payload.text}:resume=${options?.resumePrefetch !== false}`),
    getSubtitlePrefetchService: () => ({
-      pause: () => calls.push('prefetch:pause'),
+      pause: () => {
+        calls.push('prefetch:pause');
+      },
+      resume: () => {
+        calls.push('prefetch:resume');
+      },
    }),
    getLastObservedTimePos: () => 12,
    getVisibleOverlayVisible: () => true,
@@ -179,3 +201,65 @@ test('primeCurrentSubtitleForAutoplay emits raw first paint on cache miss before
    'change:起動字幕',
  ]);
 });
+
+test('primeCurrentSubtitleForAutoplay releases the prefetch pause when no tokenization is scheduled', async () => {
+  const calls: string[] = [];
+  let currentSubText = '';
+  const mediaPath = '/media/video.mkv';
+
+  const runtime = createAutoplaySubtitlePrimingRuntime({
+    getCurrentMediaPath: () => mediaPath,
+    getMpvClient: () => ({
+      connected: true,
+      currentVideoPath: mediaPath,
+      requestProperty: async (name) => {
+        if (name === 'sub-text') return '起動字幕';
+        return null;
+      },
+    }),
+    setCurrentSubText: (text) => {
+      currentSubText = text;
+    },
+    getCurrentSubText: () => currentSubText,
+    getCurrentSubtitleData: () => null,
+    getActiveParsedSubtitleCues: () => [],
+    setActiveParsedSubtitleMediaPath: () => {},
+    subtitleProcessingController: {
+      // The tokenization cache was invalidated (for example by mining a card),
+      // so the cached payload is gone...
+      consumeCachedSubtitle: () => null,
+      // ...but the controller still holds this text, so it schedules nothing
+      // and no emit will arrive to release the pause.
+      onSubtitleChange: (text) => {
+        calls.push(`change:${text}`);
+        return false;
+      },
+      refreshCurrentSubtitle: () => true,
+    },
+    emitSubtitlePayload: (payload, options) =>
+      calls.push(`emit:${payload.text}:resume=${options?.resumePrefetch !== false}`),
+    getSubtitlePrefetchService: () => ({
+      pause: () => {
+        calls.push('prefetch:pause');
+      },
+      resume: () => {
+        calls.push('prefetch:resume');
+      },
+    }),
+    getLastObservedTimePos: () => 12,
+    getVisibleOverlayVisible: () => true,
+    emitSecondarySubtitle: () => {},
+    initSubtitlePrefetch: async () => {},
+    refreshSubtitlePrefetchFromActiveTrack: async () => {},
+    logDebug: () => {},
+  });
+
+  await runtime.primeCurrentSubtitleForAutoplay(mediaPath);
+
+  assert.deepEqual(calls, [
+    'prefetch:pause',
+    'emit:起動字幕:resume=false',
+    'change:起動字幕',
+    'prefetch:resume',
+  ]);
+});
@@ -17,6 +17,7 @@ type AutoplaySubtitlePrimingMpvClient = {

 type AutoplaySubtitlePrimingPrefetchService = {
  pause: () => void;
+  resume: () => void;
 };

 export interface AutoplaySubtitlePrimingRuntimeDeps {
@@ -29,8 +30,9 @@ export interface AutoplaySubtitlePrimingRuntimeDeps {
  setActiveParsedSubtitleMediaPath: (mediaPath: string | null) => void;
  subtitleProcessingController: {
    consumeCachedSubtitle: (text: string) => SubtitleData | null;
-    onSubtitleChange: (text: string) => void;
-    refreshCurrentSubtitle: (text: string) => void;
+    // Both report whether an emit is expected; see pausePrefetchUntilEmit.
+    onSubtitleChange: (text: string) => boolean;
+    refreshCurrentSubtitle: (text: string) => boolean;
  };
  emitSubtitlePayload: (payload: SubtitleData, options?: { resumePrefetch?: boolean }) => void;
  getSubtitlePrefetchService: () => AutoplaySubtitlePrimingPrefetchService | null;
@@ -63,6 +65,18 @@ export function setMpvCurrentSecondarySubText(
 export function createAutoplaySubtitlePrimingRuntime(deps: AutoplaySubtitlePrimingRuntimeDeps) {
  const { subtitleProcessingController, emitSubtitlePayload } = deps;

+  // Prefetching is paused so the on-screen line gets the parser to itself, and
+  // the resume rides on the tokenized emit. When the controller reports that no
+  // emit is coming (repeat text with nothing scheduled), release it here or
+  // prefetching idles until some later line happens to complete.
+  function pausePrefetchUntilEmit(scheduleTokenization: () => boolean): void {
+    const prefetch = deps.getSubtitlePrefetchService();
+    prefetch?.pause();
+    if (!scheduleTokenization()) {
+      prefetch?.resume();
+    }
+  }
+
  let subtitlePrefetchRefreshTimer: ReturnType<typeof setTimeout> | null = null;
  let autoplaySubtitlePrimedMediaPath: string | null = null;
  let visibleOverlaySubtitleRefreshAfterFirstPaintTimer: ReturnType<typeof setTimeout> | null =
@@ -103,6 +117,7 @@ export function createAutoplaySubtitlePrimingRuntime(deps: AutoplaySubtitlePrimi
    const cachedPayload = subtitleProcessingController.consumeCachedSubtitle(text);
    if (cachedPayload) {
      subtitleProcessingController.onSubtitleChange(text);
+      // This emit resumes prefetching, so no pause is left outstanding.
      emitSubtitlePayload(cachedPayload);
      return true;
    }
@@ -110,7 +125,11 @@ export function createAutoplaySubtitlePrimingRuntime(deps: AutoplaySubtitlePrimi
    // Provisional raw emit: keep prefetch paused until the tokenized payload
    // for this line is delivered by the processing controller.
    emitSubtitlePayload({ text, tokens: null }, { resumePrefetch: false });
-    subtitleProcessingController.onSubtitleChange(text);
+    if (!subtitleProcessingController.onSubtitleChange(text)) {
+      // Cache miss on text the controller already holds (it was invalidated
+      // under us): nothing will be tokenized, so no emit is coming.
+      deps.getSubtitlePrefetchService()?.resume();
+    }
    return true;
  }

@@ -154,12 +173,10 @@ export function createAutoplaySubtitlePrimingRuntime(deps: AutoplaySubtitlePrimi
      getCurrentSubtitleData: () => deps.getCurrentSubtitleData(),
      consumeCachedSubtitle: (text) => subtitleProcessingController.consumeCachedSubtitle(text),
      onSubtitleChange: (text) => {
-        deps.getSubtitlePrefetchService()?.pause();
-        subtitleProcessingController.onSubtitleChange(text);
+        pausePrefetchUntilEmit(() => subtitleProcessingController.onSubtitleChange(text));
      },
      refreshCurrentSubtitle: (text) => {
-        deps.getSubtitlePrefetchService()?.pause();
-        subtitleProcessingController.refreshCurrentSubtitle(text);
+        pausePrefetchUntilEmit(() => subtitleProcessingController.refreshCurrentSubtitle(text));
      },
      deferUncachedRefresh: true,
      emitSubtitle: (payload) => emitSubtitlePayload(payload),
@@ -203,8 +220,7 @@ export function createAutoplaySubtitlePrimingRuntime(deps: AutoplaySubtitlePrimi
      if (!text.trim()) {
        return;
      }
-      deps.getSubtitlePrefetchService()?.pause();
-      subtitleProcessingController.refreshCurrentSubtitle(text);
+      pausePrefetchUntilEmit(() => subtitleProcessingController.refreshCurrentSubtitle(text));
    }, VISIBLE_OVERLAY_SUBTITLE_REFRESH_AFTER_FIRST_PAINT_DELAY_MS);
    visibleOverlaySubtitleRefreshAfterFirstPaintTimer.unref?.();
  }
@@ -53,3 +53,52 @@ test('character dictionary sync completion refreshes subtitle state when diction
    'log:[dictionary:auto-sync] refreshed current subtitle after sync (AniList 1, changed=yes, title=Frieren)',
  ]);
 });
+
+test('character dictionary sync completion drops cached dictionary reads before refreshing', () => {
+  const calls: string[] = [];
+
+  handleCharacterDictionaryAutoSyncComplete(
+    {
+      mediaId: 1,
+      mediaTitle: 'Frieren',
+      changed: true,
+    },
+    {
+      hasParserWindow: () => true,
+      invalidateCharacterDictionaryLookups: () => calls.push('invalidate-dictionary-lookups'),
+      clearParserCaches: () => calls.push('clear-parser'),
+      invalidateTokenizationCache: () => calls.push('invalidate'),
+      refreshSubtitlePrefetch: () => calls.push('prefetch'),
+      refreshCurrentSubtitle: () => calls.push('refresh-subtitle'),
+      logInfo: () => {},
+    },
+  );
+
+  // Must run before the refreshes, or they re-tokenize against the character
+  // names and images from the previous dictionary build.
+  assert.equal(calls[0], 'invalidate-dictionary-lookups');
+  assert.ok(calls.indexOf('invalidate-dictionary-lookups') < calls.indexOf('refresh-subtitle'));
+});
+
+test('character dictionary sync completion leaves cached dictionary reads alone when unchanged', () => {
+  const calls: string[] = [];
+
+  handleCharacterDictionaryAutoSyncComplete(
+    {
+      mediaId: 1,
+      mediaTitle: 'Frieren',
+      changed: false,
+    },
+    {
+      hasParserWindow: () => true,
+      invalidateCharacterDictionaryLookups: () => calls.push('invalidate-dictionary-lookups'),
+      clearParserCaches: () => calls.push('clear-parser'),
+      invalidateTokenizationCache: () => calls.push('invalidate'),
+      refreshSubtitlePrefetch: () => calls.push('prefetch'),
+      refreshCurrentSubtitle: () => calls.push('refresh-subtitle'),
+      logInfo: () => {},
+    },
+  );
+
+  assert.deepEqual(calls, []);
+});
@@ -7,6 +7,12 @@ export function handleCharacterDictionaryAutoSyncComplete(
  deps: {
    hasParserWindow: () => boolean;
    clearParserCaches: () => void;
+    /**
+     * Drops cached reads of the generated dictionary (character images, and the
+     * name candidates the scanner uses to skip lookups). Runs before the
+     * refreshes below so they re-tokenize against the new dictionary content.
+     */
+    invalidateCharacterDictionaryLookups?: () => void;
    invalidateTokenizationCache: () => void;
    refreshSubtitlePrefetch: () => void;
    refreshCurrentSubtitle: () => void;
@@ -14,6 +20,7 @@ export function handleCharacterDictionaryAutoSyncComplete(
  },
 ): void {
  if (completion.changed) {
+    deps.invalidateCharacterDictionaryLookups?.();
    if (deps.hasParserWindow()) {
      deps.clearParserCaches();
    }