fix(stats): address PR 19 review follow-ups

backlog/tasks/task-191 - Assess-PR-19-CodeRabbit-review-follow-ups.md

@@ -0,0 +1,67 @@
+---
+id: TASK-191
+title: 'Assess PR #19 CodeRabbit review follow-ups'
+status: Done
+assignee:
+  - codex
+created_date: '2026-03-17 23:15'
+updated_date: '2026-03-17 23:18'
+labels:
+  - pr-review
+  - stats
+  - immersion-tracker
+milestone: m-1
+dependencies: []
+references:
+  - src/core/services/immersion-tracker-service.ts
+  - src/core/services/immersion-tracker-service.test.ts
+priority: medium
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+Validate the open CodeRabbit review comments on PR #19 against the current branch, implement only the confirmed fixes, and record which bot suggestions are stale or technically incomplete.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 Each open CodeRabbit PR #19 comment is validated against the current branch behavior
+- [x] #2 Confirmed issues are fixed with regression coverage where it fits
+- [x] #3 Non-actionable or partially-wrong bot guidance is documented explicitly
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+1. Inspect the open CodeRabbit review threads on PR #19 and restate each finding in codebase terms.
+2. Add failing regression tests for any verified bugs before changing production code.
+3. Patch the smallest safe service-layer behavior, rerun focused verification, and record which suggestions were accepted versus rejected.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Validated the two open CodeRabbit inline findings on PR #19 against the current branch. Both reported real bugs in `ImmersionTrackerService`, but the first suggestion's exact remediation was incomplete for this codebase.
+
+`reassignAnimeAnilist` did overwrite `imm_anime.description` with `NULL` when callers omitted `description`. Fixed with a presence-aware SQL update that preserves the existing description when the field is omitted while still allowing explicit `description: null` to clear the stored value. Rejected the bot's `COALESCE(?, description)` prompt because that would silently remove the explicit-clear behavior the API already supports.
+
+`ensureCoverArt` could return `true` after a fetcher reported success even when no cover-art row/blob was stored, because `undefined !== null` evaluated truthy through optional chaining. Fixed by loading the row into a local variable and requiring a non-null blob.
+
+Added regression coverage in `src/core/services/immersion-tracker-service.test.ts` for omitted-description preservation, explicit-null clearing, and the no-row `ensureCoverArt` false-positive case.
+
+Verification passed:
+- `bun test src/core/services/immersion-tracker-service.test.ts`
+- `bash .agents/skills/subminer-change-verification/scripts/classify_subminer_diff.sh src/core/services/immersion-tracker-service.ts src/core/services/immersion-tracker-service.test.ts`
+- `bash .agents/skills/subminer-change-verification/scripts/verify_subminer_change.sh --lane core src/core/services/immersion-tracker-service.ts src/core/services/immersion-tracker-service.test.ts`
+
+Verifier artifact directory: `.tmp/skill-verification/subminer-verify-20260317-231743-wHFNnN`
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Assessed the open PR #19 CodeRabbit comments and fixed the two confirmed service-layer regressions. `reassignAnimeAnilist` now preserves an existing anime description when callers omit the `description` field but still clears it on explicit `null`, and `ensureCoverArt` no longer reports success when no cover-art row/blob exists after a fetch attempt.
+
+Both comments were actionable, but one bot-proposed fix was not correct as written for this branch: replacing the description update with `COALESCE(?, description)` would have broken intentional description clearing. Added regression tests for the accepted behaviors and verified the change with the full touched service test file plus the SubMiner `core` verification lane.
+<!-- SECTION:FINAL_SUMMARY:END -->

backlog/tasks/task-192 - Assess-remaining-PR-19-review-batch.md

@@ -0,0 +1,76 @@
+---
+id: TASK-192
+title: 'Assess remaining PR #19 review batch'
+status: Done
+assignee:
+  - codex
+created_date: '2026-03-17 23:24'
+updated_date: '2026-03-17 23:42'
+labels:
+  - pr-review
+  - stats
+  - docs
+milestone: m-1
+dependencies: []
+references:
+  - docs/superpowers/plans/2026-03-12-immersion-stats-page.md
+  - src/core/services/immersion-tracker/__tests__/query.test.ts
+  - src/core/services/ipc.ts
+  - src/core/services/stats-server.ts
+  - src/main.ts
+  - src/renderer/handlers/keyboard.ts
+  - stats/src
+priority: medium
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+Validate the remaining PR #19 automated review findings against the current branch, implement only the technically correct fixes, and document which comments are stale, already addressed, or not warranted.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 Each remaining review comment is classified as actionable, already fixed, stale, or not warranted
+- [x] #2 Confirmed bugs or correctness issues are fixed with focused regression coverage where it fits
+- [x] #3 Final notes record which comments were intentionally not applied and why
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+1. Inspect the referenced files in batches and compare each comment against current branch behavior.
+2. Separate correctness/security regressions from stylistic nitpicks and already-fixed items.
+3. Add tests first for confirmed behavior bugs where practical, apply the smallest safe fixes, and rerun targeted verification.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Swept the pasted PR #19 review batch against the current branch.
+
+Classification:
+- Already fixed on current branch: `src/core/services/immersion-tracker/__tests__/query.test.ts` cleanup rethrow, `src/core/services/ipc.ts` limit validation, `src/core/services/stats-server.ts` max-limit parsing and CORS removal, `src/main.ts` quit-path TDZ issue, `src/renderer/handlers/keyboard.ts` stats-toggle shortcut ordering/config usage, `stats/src/components/vocabulary/WordList.tsx`, `stats/src/hooks/useSessions.ts`, `stats/src/hooks/useTrends.ts` stale-error reset, `src/core/services/__tests__/stats-server.test.ts` kanji endpoint/readability notes, `src/core/services/stats-window.ts`, `stats/src/App.tsx`, `stats/src/components/layout/TabBar.tsx`, `stats/src/components/overview/QuickStats.tsx`, `stats/src/components/overview/WatchTimeChart.tsx`, `stats/src/components/sessions/SessionDetail.tsx`, `stats/src/components/sessions/SessionRow.tsx`, `stats/src/components/trends/DateRangeSelector.tsx`, `stats/src/components/vocabulary/KanjiBreakdown.tsx`, `stats/src/components/vocabulary/VocabularyTab.tsx`, `stats/src/hooks/useVocabulary.ts`, `stats/src/lib/api-client.ts`, `stats/src/types/stats.ts`.
+- Stale / obsolete against current architecture: `docs/superpowers/plans/2026-03-12-immersion-stats-page.md` path does not exist on this branch; `stats/src/components/trends/TrendsTab.tsx` / monthly-range comments describe older client-side aggregation code that is no longer present because trends now come from `getTrendsDashboard`.
+- Not warranted as written: `stats/src/lib/formatters.ts` no longer emits negative `Xd ago`; current code short-circuits future timestamps to `just now`, so the reported bug condition is gone even though the suggested wording differs.
+- Actionable and fixed now: `src/core/services/ipc.ts` no-tracker `statsGetOverview` fallback omitted required hint fields (`totalLookupCount`, `totalLookupHits`, `newWordsToday`, `newWordsThisWeek`). Added the missing fields in the fallback object and updated IPC tests to assert the full shape.
+
+Verification:
+- `bun test src/core/services/ipc.test.ts`
+- `bun test src/core/services/ipc.test.ts --test-name-pattern "empty stats overview shape without a tracker|validates and clamps stats request limits"`
+- `bash .agents/skills/subminer-change-verification/scripts/classify_subminer_diff.sh src/core/services/ipc.ts src/core/services/ipc.test.ts`
+
+Repo verifier note:
+- `bash .agents/skills/subminer-change-verification/scripts/verify_subminer_change.sh --lane core src/core/services/ipc.ts src/core/services/ipc.test.ts`
+- That verifier run captured a temporary `bun run typecheck` failure in `src/anki-integration.test.ts` and `src/core/services/__tests__/stats-server.test.ts`, but a fresh rerun after the follow-up validation no longer reproduces those diagnostics.
+- Fresh verification: `bun run typecheck` passes locally.
+- artifact dir from the earlier failed verifier snapshot: `.tmp/skill-verification/subminer-verify-20260317-234027-i6QJ3n`
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+The larger pasted PR #19 review batch was not mostly new work on the current branch. After verifying each item against the live code, almost all were already fixed or stale. One additional item was still actionable: the no-tracker fallback returned by `statsGetOverview` in `src/core/services/ipc.ts` omitted required hint fields, which made the fallback shape inconsistent with the normal overview payload. That fallback is now fixed and covered by IPC tests.
+
+Count-wise: the earlier open CodeRabbit service comments contributed 2 actionable fixes, and this larger pasted batch contributed 1 additional actionable fix on top of those.
+<!-- SECTION:FINAL_SUMMARY:END -->

src/core/services/immersion-tracker-service.test.ts

@@ -2089,6 +2089,165 @@ test('reassignAnimeAnilist deduplicates cover blobs and getCoverArt remains comp
   }
 });
 
+test('reassignAnimeAnilist preserves existing description when description is omitted', async () => {
+  const dbPath = makeDbPath();
+  let tracker: ImmersionTrackerService | null = null;
+
+  try {
+    const Ctor = await loadTrackerCtor();
+    tracker = new Ctor({ dbPath });
+    const privateApi = tracker as unknown as { db: DatabaseSync };
+
+    privateApi.db.exec(`
+      INSERT INTO imm_anime (
+        anime_id,
+        normalized_title_key,
+        canonical_title,
+        description,
+        CREATED_DATE,
+        LAST_UPDATE_DATE
+      ) VALUES (
+        1,
+        'little witch academia',
+        'Little Witch Academia',
+        'Original description',
+        1000,
+        1000
+      );
+    `);
+
+    await tracker.reassignAnimeAnilist(1, {
+      anilistId: 33489,
+      titleRomaji: 'Little Witch Academia',
+    });
+
+    const row = privateApi.db
+      .prepare(
+        'SELECT anilist_id AS anilistId, description FROM imm_anime WHERE anime_id = ?',
+      )
+      .get(1) as { anilistId: number | null; description: string | null } | null;
+
+    assert.equal(row?.anilistId, 33489);
+    assert.equal(row?.description, 'Original description');
+  } finally {
+    tracker?.destroy();
+    cleanupDbPath(dbPath);
+  }
+});
+
+test('reassignAnimeAnilist clears description when description is explicitly null', async () => {
+  const dbPath = makeDbPath();
+  let tracker: ImmersionTrackerService | null = null;
+
+  try {
+    const Ctor = await loadTrackerCtor();
+    tracker = new Ctor({ dbPath });
+    const privateApi = tracker as unknown as { db: DatabaseSync };
+
+    privateApi.db.exec(`
+      INSERT INTO imm_anime (
+        anime_id,
+        normalized_title_key,
+        canonical_title,
+        description,
+        CREATED_DATE,
+        LAST_UPDATE_DATE
+      ) VALUES (
+        1,
+        'little witch academia',
+        'Little Witch Academia',
+        'Original description',
+        1000,
+        1000
+      );
+    `);
+
+    await tracker.reassignAnimeAnilist(1, {
+      anilistId: 33489,
+      description: null,
+    });
+
+    const row = privateApi.db
+      .prepare('SELECT description FROM imm_anime WHERE anime_id = ?')
+      .get(1) as { description: string | null } | null;
+
+    assert.equal(row?.description, null);
+  } finally {
+    tracker?.destroy();
+    cleanupDbPath(dbPath);
+  }
+});
+
+test('ensureCoverArt returns false when fetcher reports success without storing art', async () => {
+  const dbPath = makeDbPath();
+  let tracker: ImmersionTrackerService | null = null;
+  let fetchCalls = 0;
+
+  try {
+    const Ctor = await loadTrackerCtor();
+    tracker = new Ctor({ dbPath });
+    const privateApi = tracker as unknown as { db: DatabaseSync };
+
+    privateApi.db.exec(`
+      INSERT INTO imm_videos (
+        video_id,
+        video_key,
+        canonical_title,
+        source_type,
+        duration_ms,
+        CREATED_DATE,
+        LAST_UPDATE_DATE
+      ) VALUES (
+        1,
+        'local:/tmp/lwa-1.mkv',
+        'Little Witch Academia S01E01',
+        1,
+        0,
+        1000,
+        1000
+      );
+      INSERT INTO imm_lifetime_media (
+        video_id,
+        total_sessions,
+        total_active_ms,
+        total_cards,
+        total_tokens_seen,
+        total_lines_seen,
+        CREATED_DATE,
+        LAST_UPDATE_DATE
+      ) VALUES (
+        1,
+        0,
+        0,
+        0,
+        0,
+        0,
+        1000,
+        1000
+      );
+    `);
+
+    tracker.setCoverArtFetcher({
+      fetchIfMissing: async () => {
+        fetchCalls += 1;
+        return true;
+      },
+    });
+
+    const storedBefore = await tracker.getCoverArt(1);
+    assert.equal(storedBefore?.coverBlob ?? null, null);
+
+    const result = await tracker.ensureCoverArt(1);
+
+    assert.equal(fetchCalls, 1);
+    assert.equal(result, false);
+    assert.equal((await tracker.getCoverArt(1))?.coverBlob ?? null, null);
+  } finally {
+    tracker?.destroy();
+    cleanupDbPath(dbPath);
+  }
+});
+
 test('markActiveVideoWatched marks current session video as watched', async () => {
   const dbPath = makeDbPath();
   let tracker: ImmersionTrackerService | null = null;

src/core/services/immersion-tracker-service.ts

@@ -540,7 +540,7 @@ export class ImmersionTrackerService {
           title_english = COALESCE(?, title_english),
           title_native = COALESCE(?, title_native),
           episodes_total = COALESCE(?, episodes_total),
-          description = ?,
+          description = CASE WHEN ? = 1 THEN ? ELSE description END,
           LAST_UPDATE_DATE = ?
       WHERE anime_id = ?
     `,
@@ -551,6 +551,7 @@ export class ImmersionTrackerService {
         info.titleEnglish ?? null,
         info.titleNative ?? null,
         info.episodesTotal ?? null,
+        info.description !== undefined ? 1 : 0,
         info.description ?? null,
         Date.now(),
         animeId,
@@ -658,7 +659,8 @@ export class ImmersionTrackerService {
       if (!fetched) {
         return false;
       }
-      return (await this.getCoverArt(videoId))?.coverBlob !== null;
+      const cover = await this.getCoverArt(videoId);
+      return cover?.coverBlob != null;
     })();
 
     this.pendingCoverFetches.set(videoId, fetchPromise);

src/core/services/ipc.test.ts

@@ -140,6 +140,10 @@ function createFakeImmersionTracker(
       activeDays: 0,
       totalEpisodesWatched: 0,
       totalAnimeCompleted: 0,
+      totalLookupCount: 0,
+      totalLookupHits: 0,
+      newWordsToday: 0,
+      newWordsThisWeek: 0,
     }),
     getSessionTimeline: async () => [],
     getSessionEvents: async () => [],
@@ -355,6 +359,10 @@ test('registerIpcHandlers returns empty stats overview shape without a tracker',
       activeDays: 0,
       totalEpisodesWatched: 0,
       totalAnimeCompleted: 0,
+      totalLookupCount: 0,
+      totalLookupHits: 0,
+      newWordsToday: 0,
+      newWordsThisWeek: 0,
     },
   });
 });
@@ -389,6 +397,10 @@ test('registerIpcHandlers validates and clamps stats request limits', async () =
           activeDays: 0,
           totalEpisodesWatched: 0,
           totalAnimeCompleted: 0,
+          totalLookupCount: 0,
+          totalLookupHits: 0,
+          newWordsToday: 0,
+          newWordsThisWeek: 0,
         }),
         getSessionTimeline: async (sessionId: number, limit = 0) => {
           calls.push(['timeline', limit, sessionId]);

src/core/services/ipc.ts

@@ -486,6 +486,10 @@ export function registerIpcHandlers(deps: IpcServiceDeps, ipc: IpcMainRegistrar
           activeDays: 0,
           totalEpisodesWatched: 0,
           totalAnimeCompleted: 0,
+          totalLookupCount: 0,
+          totalLookupHits: 0,
+          newWordsToday: 0,
+          newWordsThisWeek: 0,
         },
       };
     }