fix: avoid merged dictionary rebuilds on revisits

backlog/tasks/task-153 - Fix-character-dictionary-MRU-eviction-after-revisits.md

@@ -0,0 +1,68 @@
+---
+id: TASK-153
+title: Fix character dictionary MRU eviction after revisits
+status: Done
+assignee:
+  - '@codex'
+created_date: '2026-03-10 07:56'
+updated_date: '2026-03-10 08:48'
+labels:
+  - character-dictionary
+  - yomitan
+  - anilist
+dependencies: []
+references:
+  - >-
+    /home/sudacode/projects/japanese/SubMiner/src/main/runtime/character-dictionary-auto-sync.ts
+  - >-
+    /home/sudacode/projects/japanese/SubMiner/src/main/runtime/character-dictionary-auto-sync.test.ts
+  - >-
+    /home/sudacode/projects/japanese/SubMiner/src/main/character-dictionary-runtime.ts
+documentation:
+  - /home/sudacode/projects/japanese/subminer-docs/development.md
+  - /home/sudacode/projects/japanese/subminer-docs/architecture.md
+priority: high
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+Keep merged character dictionary retention truly MRU-based when a currently retained anime is revisited before opening a new title, so the oldest retained title is evicted instead of the revisited one.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 Revisiting an anime already retained in the merged character dictionary updates MRU ordering before the next new title is added.
+- [x] #2 When retention exceeds maxLoaded after that revisit-plus-new-title sequence, the least recently used retained anime is evicted rather than the revisited title.
+- [x] #3 Auto-sync rebuilds or reloads the merged dictionary so an evicted anime becomes available again when reopened.
+- [x] #4 Regression tests cover the revisit-before-eviction flow.
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+1. Reproduce the revisit-before-eviction scenario with a focused regression test in `src/main/runtime/character-dictionary-auto-sync.test.ts` using a retained set that revisits an existing anime before introducing a new anime.
+2. Run the focused test to confirm current behavior fails for the expected MRU ordering / eviction case.
+3. Adjust `src/main/runtime/character-dictionary-auto-sync.ts` so retained ordering stays MRU-correct across revisits and subsequent additions without regressing unchanged revisit behavior.
+4. Re-run the focused auto-sync suite, then run the relevant broader checks required for handoff.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Added a focused regression in `src/main/runtime/character-dictionary-auto-sync.test.ts` for the `1,2,3,1,4,1` revisit-before-eviction flow. The current MRU auto-sync logic passed: merged builds were `[1]`, `[2,1]`, `[3,2,1]`, `[1,3,2]`, `[4,1,3]`, `[1,4,3]`, so I have not reproduced the reported eviction bug in the in-process auto-sync service yet.
+
+Inspected local cache under `~/.config/SubMiner/character-dictionaries/` and found Little Witch Academia snapshot `anilist-21858.json` already present, so the reported regeneration path was not a snapshot-cache miss. The on-disk `merged.zip` revision (`9251ae23e136`) also differed from `auto-sync-state.json` (`bc16c5b5af17`), indicating a rebuilt merged dictionary artifact could land without the MRU state being persisted.
+
+Fixed `src/main/runtime/character-dictionary-auto-sync.ts` to persist the rebuilt retained-set state immediately after a merged dictionary revision/title is known, before later Yomitan mutation steps. This keeps MRU eviction state aligned even if import/settings work fails after the rebuild artifact is written.
+
+Added regression coverage in `src/main/runtime/character-dictionary-auto-sync.test.ts` for the revisit-before-eviction sequence and for post-build import failure preserving the rebuilt MRU state. Verified with `bun test src/main/runtime/character-dictionary-auto-sync.test.ts src/main/character-dictionary-runtime.test.ts`, `bun run typecheck`, `bun run changelog:lint`, and `bun run test:fast`.
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Persisted merged character-dictionary MRU state as soon as a rebuilt retained set is known in `src/main/runtime/character-dictionary-auto-sync.ts`, so revisits are not lost if later Yomitan import/settings work fails after `merged.zip` has already been rewritten. Added regression coverage for revisit-before-eviction ordering and import-failure state preservation in `src/main/runtime/character-dictionary-auto-sync.test.ts`, plus a changelog fragment in `changes/character-dictionary-mru-state-recovery.md`.
+
+Validation: `bun test src/main/runtime/character-dictionary-auto-sync.test.ts src/main/character-dictionary-runtime.test.ts`, `bun run typecheck`, `bun run changelog:lint`, `bun run test:fast`.
+<!-- SECTION:FINAL_SUMMARY:END -->

backlog/tasks/task-154 - Avoid-merged-dictionary-rebuilds-on-MRU-reorder-only-revisits.md

@@ -0,0 +1,68 @@
+---
+id: TASK-154
+title: Avoid merged dictionary rebuilds on MRU reorder-only revisits
+status: Done
+assignee:
+  - '@codex'
+created_date: '2026-03-10 09:16'
+updated_date: '2026-03-10 09:22'
+labels:
+  - character-dictionary
+  - yomitan
+  - anilist
+dependencies: []
+references:
+  - >-
+    /home/sudacode/projects/japanese/SubMiner/src/main/runtime/character-dictionary-auto-sync.ts
+  - >-
+    /home/sudacode/projects/japanese/SubMiner/src/main/runtime/character-dictionary-auto-sync.test.ts
+  - >-
+    /home/sudacode/projects/japanese/SubMiner/src/main/character-dictionary-runtime.ts
+  - >-
+    /home/sudacode/projects/japanese/SubMiner/src/main/character-dictionary-runtime.test.ts
+documentation:
+  - /home/sudacode/projects/japanese/subminer-docs/development.md
+  - /home/sudacode/projects/japanese/subminer-docs/architecture.md
+priority: high
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+Keep per-title MRU retention ordering for eviction decisions, but do not rebuild or reimport the merged character dictionary when the retained set membership is unchanged and only the MRU order changes.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 Revisiting an anime already inside the retained set updates MRU ordering used for later eviction decisions without rebuilding or reimporting the merged dictionary when retained membership is unchanged.
+- [x] #2 Merged dictionary revisions and contents are stable for the same retained membership regardless of MRU ordering.
+- [x] #3 Adding a new anime that changes retained membership still rebuilds and imports the merged dictionary with the correct eviction behavior.
+- [x] #4 Regression tests cover reorder-only revisits and stable merged revisions for equivalent retained sets.
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+1. Add a failing auto-sync regression showing that a revisit which only reorders MRU state should update `activeMediaIds` but skip merged rebuild/import.
+2. Add a runtime-level regression showing `buildMergedDictionary` produces a stable revision for equivalent retained memberships regardless of input order.
+3. Update merged-dictionary build normalization and auto-sync rebuild gating so rebuilds are driven by retained membership changes (or snapshot changes), not MRU reordering alone.
+4. Re-run focused dictionary tests plus the local verification lanes impacted by the change.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Changed `src/main/runtime/character-dictionary-auto-sync.ts` so MRU order still updates in `activeMediaIds`, but merged rebuild/import now keys off retained membership changes rather than order-only reordering. Order-only revisits no longer force a merged ZIP rewrite or Yomitan reimport.
+
+Changed `src/main/character-dictionary-runtime.ts` to canonicalize merged dictionary media ids before building, which makes merged revisions stable for equivalent retained memberships regardless of MRU ordering.
+
+Updated regression coverage in `src/main/runtime/character-dictionary-auto-sync.test.ts` for reorder-only revisits and membership-change rebuilds, and extended `src/main/character-dictionary-runtime.test.ts` to assert stable merged revisions for reordered inputs. Verified with `bun test src/main/runtime/character-dictionary-auto-sync.test.ts src/main/character-dictionary-runtime.test.ts`, `bun run typecheck`, `bun run changelog:lint`, and `bun run test:fast`.
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Updated merged character-dictionary syncing so MRU ordering is still tracked for eviction in `src/main/runtime/character-dictionary-auto-sync.ts`, but reorder-only revisits no longer rebuild or reimport the merged dictionary unless retained membership or snapshot data changes. Canonicalized merged build input ordering in `src/main/character-dictionary-runtime.ts` so revisions remain stable for the same retained set regardless of MRU order, and added regressions in `src/main/runtime/character-dictionary-auto-sync.test.ts` plus `src/main/character-dictionary-runtime.test.ts`. Also updated `changes/character-dictionary-mru-state-recovery.md` to cover the new behavior.
+
+Validation: `bun test src/main/runtime/character-dictionary-auto-sync.test.ts src/main/character-dictionary-runtime.test.ts`, `bun run typecheck`, `bun run changelog:lint`, `bun run test:fast`.
+<!-- SECTION:FINAL_SUMMARY:END -->

changes/character-dictionary-mru-state-recovery.md

@@ -0,0 +1,4 @@
+type: fixed
+area: dictionary
+
+- Persist merged character-dictionary MRU state as soon as a new retained set is built so revisits do not get dropped if later Yomitan import work fails, and skip merged dictionary rebuilds for reorder-only revisits when the retained anime set itself has not changed.

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

@@ -2206,6 +2206,7 @@ test('buildMergedDictionary combines stored snapshots into one stable dictionary
     await runtime.getOrCreateCurrentSnapshot();
 
     const merged = await runtime.buildMergedDictionary([21, 130298]);
+    const mergedReordered = await runtime.buildMergedDictionary([130298, 21]);
     const index = JSON.parse(readStoredZipEntry(merged.zipPath, 'index.json').toString('utf8')) as {
       title: string;
     };
@@ -2228,6 +2229,7 @@ test('buildMergedDictionary combines stored snapshots into one stable dictionary
 
     assert.equal(index.title, 'SubMiner Character Dictionary');
     assert.equal(merged.entryCount >= 2, true);
+    assert.equal(merged.revision, mergedReordered.revision);
     assert.ok(frieren);
     assert.ok(alpha);
     assert.equal((frieren[5][0] as { type?: string }).type, 'structured-content');

src/main/character-dictionary-runtime.ts

@@ -1980,6 +1980,16 @@ function buildMergedRevision(mediaIds: number[], snapshots: CharacterDictionaryS
   return hash.digest('hex').slice(0, 12);
 }
 
+function normalizeMergedMediaIds(mediaIds: number[]): number[] {
+  return [
+    ...new Set(
+      mediaIds
+        .filter((mediaId) => Number.isFinite(mediaId) && mediaId > 0)
+        .map((mediaId) => Math.floor(mediaId)),
+    ),
+  ].sort((left, right) => left - right);
+}
+
 export function createCharacterDictionaryRuntimeService(deps: CharacterDictionaryRuntimeDeps): {
   getOrCreateCurrentSnapshot: (
     targetPath?: string,
@@ -2154,9 +2164,7 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar
       );
     },
     buildMergedDictionary: async (mediaIds: number[]) => {
-      const normalizedMediaIds = mediaIds
-        .filter((mediaId) => Number.isFinite(mediaId) && mediaId > 0)
-        .map((mediaId) => Math.floor(mediaId));
+      const normalizedMediaIds = normalizeMergedMediaIds(mediaIds);
       const snapshotResults = await Promise.all(
         normalizedMediaIds.map((mediaId) => getOrCreateSnapshot(mediaId)),
       );

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

@@ -150,7 +150,7 @@ test('auto sync skips rebuild/import on unchanged revisit when merged dictionary
   assert.deepEqual(imports, ['/tmp/merged.zip']);
 });
 
-test('auto sync rebuilds merged dictionary when MRU order changes', async () => {
+test('auto sync updates MRU order without rebuilding merged dictionary when membership is unchanged', async () => {
   const userDataPath = makeTempDir();
   const sequence = [1, 2, 1];
   const mergedBuilds: number[][] = [];
@@ -207,8 +207,14 @@ test('auto sync rebuilds merged dictionary when MRU order changes', async () =>
   await runtime.runSyncNow();
   await runtime.runSyncNow();
 
-  assert.deepEqual(mergedBuilds, [[1], [2, 1], [1, 2]]);
-  assert.ok(deleted.length >= 2);
+  assert.deepEqual(mergedBuilds, [[1], [2, 1]]);
+  assert.equal(deleted.length, 1);
+
+  const statePath = path.join(userDataPath, 'character-dictionaries', 'auto-sync-state.json');
+  const state = JSON.parse(fs.readFileSync(statePath, 'utf8')) as {
+    activeMediaIds: number[];
+  };
+  assert.deepEqual(state.activeMediaIds, [1, 2]);
 });
 
 test('auto sync evicts least recently used media from merged set', async () => {
@@ -276,6 +282,137 @@ test('auto sync evicts least recently used media from merged set', async () => {
   assert.deepEqual(state.activeMediaIds, [4, 3, 2]);
 });
 
+test('auto sync keeps revisited media retained when a new title is added afterward', async () => {
+  const userDataPath = makeTempDir();
+  const sequence = [1, 2, 3, 1, 4, 1];
+  const mergedBuilds: number[][] = [];
+  let runIndex = 0;
+  let importedRevision: string | null = null;
+
+  const runtime = createCharacterDictionaryAutoSyncRuntimeService({
+    userDataPath,
+    getConfig: () => ({
+      enabled: true,
+      maxLoaded: 3,
+      profileScope: 'all',
+    }),
+    getOrCreateCurrentSnapshot: async () => {
+      const mediaId = sequence[Math.min(runIndex, sequence.length - 1)]!;
+      runIndex += 1;
+      return {
+        mediaId,
+        mediaTitle: `Title ${mediaId}`,
+        entryCount: 10,
+        fromCache: true,
+        updatedAt: mediaId,
+      };
+    },
+    buildMergedDictionary: async (mediaIds) => {
+      mergedBuilds.push([...mediaIds]);
+      const revision = `rev-${mediaIds.join('-')}`;
+      return {
+        zipPath: `/tmp/${revision}.zip`,
+        revision,
+        dictionaryTitle: 'SubMiner Character Dictionary',
+        entryCount: mediaIds.length * 10,
+      };
+    },
+    getYomitanDictionaryInfo: async () =>
+      importedRevision
+        ? [{ title: 'SubMiner Character Dictionary', revision: importedRevision }]
+        : [],
+    importYomitanDictionary: async (zipPath) => {
+      importedRevision = path.basename(zipPath, '.zip');
+      return true;
+    },
+    deleteYomitanDictionary: async () => {
+      importedRevision = null;
+      return true;
+    },
+    upsertYomitanDictionarySettings: async () => true,
+    now: () => Date.now(),
+  });
+
+  await runtime.runSyncNow();
+  await runtime.runSyncNow();
+  await runtime.runSyncNow();
+  await runtime.runSyncNow();
+  await runtime.runSyncNow();
+  await runtime.runSyncNow();
+
+  assert.deepEqual(mergedBuilds, [[1], [2, 1], [3, 2, 1], [4, 1, 3]]);
+
+  const statePath = path.join(userDataPath, 'character-dictionaries', 'auto-sync-state.json');
+  const state = JSON.parse(fs.readFileSync(statePath, 'utf8')) as {
+    activeMediaIds: number[];
+  };
+  assert.deepEqual(state.activeMediaIds, [1, 4, 3]);
+});
+
+test('auto sync persists rebuilt MRU state even if Yomitan import fails afterward', async () => {
+  const userDataPath = makeTempDir();
+  const dictionariesDir = path.join(userDataPath, 'character-dictionaries');
+  fs.mkdirSync(dictionariesDir, { recursive: true });
+  fs.writeFileSync(
+    path.join(dictionariesDir, 'auto-sync-state.json'),
+    JSON.stringify(
+      {
+        activeMediaIds: [2, 3, 4],
+        mergedRevision: 'rev-2-3-4',
+        mergedDictionaryTitle: 'SubMiner Character Dictionary',
+      },
+      null,
+      2,
+    ),
+  );
+
+  const runtime = createCharacterDictionaryAutoSyncRuntimeService({
+    userDataPath,
+    getConfig: () => ({
+      enabled: true,
+      maxLoaded: 3,
+      profileScope: 'all',
+    }),
+    getOrCreateCurrentSnapshot: async () => ({
+      mediaId: 1,
+      mediaTitle: 'Title 1',
+      entryCount: 10,
+      fromCache: true,
+      updatedAt: 1,
+    }),
+    buildMergedDictionary: async (mediaIds) => {
+      assert.deepEqual(mediaIds, [1, 2, 3]);
+      return {
+        zipPath: '/tmp/rev-1-2-3.zip',
+        revision: 'rev-1-2-3',
+        dictionaryTitle: 'SubMiner Character Dictionary',
+        entryCount: 30,
+      };
+    },
+    waitForYomitanMutationReady: async () => undefined,
+    getYomitanDictionaryInfo: async () => [],
+    importYomitanDictionary: async () => {
+      throw new Error('import failed');
+    },
+    deleteYomitanDictionary: async () => true,
+    upsertYomitanDictionarySettings: async () => true,
+    now: () => 1000,
+  });
+
+  await assert.rejects(runtime.runSyncNow(), /import failed/);
+
+  const state = JSON.parse(
+    fs.readFileSync(path.join(dictionariesDir, 'auto-sync-state.json'), 'utf8'),
+  ) as {
+    activeMediaIds: number[];
+    mergedRevision: string | null;
+    mergedDictionaryTitle: string | null;
+  };
+  assert.deepEqual(state.activeMediaIds, [1, 2, 3]);
+  assert.equal(state.mergedRevision, 'rev-1-2-3');
+  assert.equal(state.mergedDictionaryTitle, 'SubMiner Character Dictionary');
+});
+
 test('auto sync invokes completion callback after successful sync', async () => {
   const userDataPath = makeTempDir();
   const completions: Array<{ mediaId: number; mediaTitle: string; changed: boolean }> = [];

src/main/runtime/character-dictionary-auto-sync.ts

@@ -107,6 +107,13 @@ function arraysEqual(left: number[], right: number[]): boolean {
   return true;
 }
 
+function sameMembership(left: number[], right: number[]): boolean {
+  if (left.length !== right.length) return false;
+  const leftSorted = [...left].sort((a, b) => a - b);
+  const rightSorted = [...right].sort((a, b) => a - b);
+  return arraysEqual(leftSorted, rightSorted);
+}
+
 function buildSyncingMessage(mediaTitle: string): string {
   return `Updating character dictionary for ${mediaTitle}...`;
 }
@@ -223,10 +230,11 @@ export function createCharacterDictionaryAutoSyncRuntimeService(
         `[dictionary:auto-sync] active AniList media set: ${nextActiveMediaIds.join(', ')}`,
       );
 
-      const retainedChanged = !arraysEqual(nextActiveMediaIds, state.activeMediaIds);
+      const retainedOrderChanged = !arraysEqual(nextActiveMediaIds, state.activeMediaIds);
+      const retainedMembershipChanged = !sameMembership(nextActiveMediaIds, state.activeMediaIds);
       let merged: MergedCharacterDictionaryBuildResult | null = null;
       if (
-        retainedChanged ||
+        retainedMembershipChanged ||
         !state.mergedRevision ||
         !state.mergedDictionaryTitle ||
         !snapshot.fromCache
@@ -247,6 +255,12 @@ export function createCharacterDictionaryAutoSyncRuntimeService(
         throw new Error('Merged character dictionary state is incomplete.');
       }
 
+      writeAutoSyncState(statePath, {
+        activeMediaIds: nextActiveMediaIds,
+        mergedRevision: merged?.revision ?? revision,
+        mergedDictionaryTitle: merged?.dictionaryTitle ?? dictionaryTitle,
+      });
+
       await deps.waitForYomitanMutationReady?.();
 
       const dictionaryInfo = await withOperationTimeout(
@@ -263,7 +277,7 @@ export function createCharacterDictionaryAutoSyncRuntimeService(
         existing === null ||
         existingRevision === null ||
         existingRevision !== revision;
-      let changed = merged !== null;
+      let changed = merged !== null || retainedOrderChanged;
 
       if (shouldImport) {
         deps.onSyncStatus?.({
@@ -299,11 +313,6 @@ export function createCharacterDictionaryAutoSyncRuntimeService(
       );
       changed = changed || settingsUpdated === true;
 
-      writeAutoSyncState(statePath, {
-        activeMediaIds: nextActiveMediaIds,
-        mergedRevision: merged?.revision ?? revision,
-        mergedDictionaryTitle: merged?.dictionaryTitle ?? dictionaryTitle,
-      });
       deps.logInfo?.(
         `[dictionary:auto-sync] synced AniList ${snapshot.mediaId}: ${dictionaryTitle} (${snapshot.entryCount} entries)`,
       );