fix: address coderabbit subtitle follow-ups

backlog/tasks/task-345 - Address-PR-57-latest-CodeRabbit-review-comments.md

@@ -0,0 +1,70 @@
+---
+id: TASK-345
+title: Address PR 57 latest CodeRabbit review comments
+status: Done
+assignee:
+  - codex
+created_date: '2026-05-12 06:35'
+updated_date: '2026-05-12 06:38'
+labels:
+  - pr-review
+  - coderabbit
+dependencies: []
+references:
+  - 'https://github.com/ksyasuda/SubMiner/pull/57'
+priority: medium
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+Assess the 2026-05-11 CodeRabbit review on PR #57 and address still-valid actionable comments with minimal changes. Current comments cover mpv subtitle playback unpause behavior, parser-selection empty reading handling, and overlay focus selector accessibility.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 Still-valid CodeRabbit comments from the latest PR #57 review are fixed or explicitly documented as skipped with rationale.
+- [x] #2 Regression coverage is added for behavior-affecting fixes where practical before production code changes.
+- [x] #3 Relevant targeted checks pass locally.
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+1. Inspect current code and nearby tests for the three latest CodeRabbit comments on PR #57.
+2. Add regression tests first for behavior-impacting findings: mpv playNextSubtitle unpauses when pause state is unknown, and parser selection preserves combined reading for empty segment readings.
+3. Apply minimal production fixes plus the CSS :focus-visible selector change.
+4. Run targeted test commands for touched areas and update task notes/final status.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Added red tests for the two behavior comments. `bun test src/core/services/mpv.test.ts` fails because playNextSubtitle skips unpause when pause state is null. `bun test src/core/services/tokenizer/parser-selection-stage.test.ts` fails because empty grammar-ending reading clears preceding combined reading.
+
+Implemented all three latest CodeRabbit findings. Added regressions for mpv unknown pause state and parser-selection empty reading handling; changed overlay focus selectors to :focus-visible. Also fixed existing Prettier failure in src/core/services/stats-window.ts. Verification passed: targeted tests, typecheck, test:fast, test:env, format:check:src, build, test:smoke:dist, changelog:lint, changelog:pr-check.
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Addressed the latest CodeRabbit comments on PR #57.
+
+Changes:
+- `playNextSubtitle` now sends `pause=false` unless mpv is explicitly known to be playing, covering startup/reconnect unknown pause state.
+- Parser selection no longer slices combined readings for empty grammar-ending readings, preserving preceding token readings.
+- Overlay focus suppression now targets `:focus-visible` selectors.
+- Applied Prettier to `src/core/services/stats-window.ts` to clear the existing formatting gate failure.
+
+Verification:
+- `bun test src/core/services/mpv.test.ts`
+- `bun test src/core/services/tokenizer/parser-selection-stage.test.ts`
+- `bun run typecheck`
+- `bun run test:fast`
+- `bun run test:env`
+- `bun run format:check:src`
+- `bun run build`
+- `bun run test:smoke:dist`
+- `bun run changelog:lint`
+- `bun run changelog:pr-check`
+<!-- SECTION:FINAL_SUMMARY:END -->

src/core/services/mpv.test.ts

@@ -515,6 +515,23 @@ test('MpvIpcClient playNextSubtitle starts playback from paused state and auto-p
   ]);
 });
 
+test('MpvIpcClient playNextSubtitle starts playback when pause state is unknown', () => {
+  const commands: unknown[] = [];
+  const client = new MpvIpcClient('/tmp/mpv.sock', makeDeps());
+  (client as any).send = (payload: unknown) => {
+    commands.push(payload);
+    return true;
+  };
+
+  client.playNextSubtitle();
+
+  assert.equal((client as any).pendingPauseAtSubEnd, true);
+  assert.deepEqual(commands, [
+    { command: ['sub-seek', 1] },
+    { command: ['set_property', 'pause', false] },
+  ]);
+});
+
 test('MpvIpcClient playNextSubtitle still auto-pauses at end while already playing', async () => {
   const commands: unknown[] = [];
   const client = new MpvIpcClient('/tmp/mpv.sock', makeDeps());

src/core/services/mpv.ts

@@ -525,7 +525,7 @@ export class MpvIpcClient implements MpvClient {
     this.pendingPauseAtSubEnd = true;
     this.pauseAtTime = null;
     this.send({ command: ['sub-seek', 1] });
-    if (this.playbackPaused === true) {
+    if (this.playbackPaused !== false) {
       this.send({ command: ['set_property', 'pause', false] });
     }
   }

src/core/services/stats-window.ts

@@ -51,7 +51,9 @@ function showStatsWindow(window: BrowserWindow, options: StatsWindowOptions): vo
   promoteStatsWindowLevel(window);
   window.show();
   placementBounds = syncStatsWindowBounds(window, bounds) ?? placementBounds;
-  if (!ensureHyprlandWindowFloatingByTitle({ title: STATS_WINDOW_TITLE, bounds: placementBounds })) {
+  if (
+    !ensureHyprlandWindowFloatingByTitle({ title: STATS_WINDOW_TITLE, bounds: placementBounds })
+  ) {
     placementBounds = syncStatsWindowBounds(window, bounds) ?? placementBounds;
   }
   window.focus();

src/core/services/tokenizer/parser-selection-stage.test.ts

@@ -187,6 +187,38 @@ test('splits trailing grammar endings when later segments are standalone words',
   );
 });
 
+test('keeps preceding reading when standalone grammar ending has empty reading', () => {
+  const parseResults = [
+    makeParseItem('scanning-parser', [
+      [
+        { text: '猫', reading: 'ねこ', headword: '猫' },
+        { text: 'です', reading: '', headword: 'です' },
+      ],
+    ]),
+  ];
+
+  const tokens = selectYomitanParseTokens(parseResults, () => false, 'headword');
+  assert.deepEqual(
+    tokens?.map((token) => ({
+      surface: token.surface,
+      reading: token.reading,
+      headword: token.headword,
+    })),
+    [
+      {
+        surface: '猫',
+        reading: 'ねこ',
+        headword: '猫',
+      },
+      {
+        surface: 'です',
+        reading: '',
+        headword: 'です',
+      },
+    ],
+  );
+});
+
 test('splits trailing ja-nai grammar endings from preceding content', () => {
   const parseResults = [
     makeParseItem('scanning-parser', [

src/core/services/tokenizer/parser-selection-stage.ts

@@ -270,7 +270,7 @@ export function mapYomitanParseResultItemToMergedTokens(
       const segmentHeadword = extractYomitanHeadword(segment);
       if (isStandaloneGrammarEndingSegment(segment)) {
         combinedSurface = combinedSurface.slice(0, -segmentText.length);
-        if (typeof segment.reading === 'string') {
+        if (typeof segment.reading === 'string' && segment.reading.length > 0) {
           combinedReading = combinedReading.slice(0, -segment.reading.length);
         }
         flushCombinedToken(segmentStart);

src/renderer/style.css

@@ -40,9 +40,9 @@ body {
     'Hiragino Kaku Gothic ProN', 'Yu Gothic', 'Arial Unicode MS', Arial, sans-serif;
 }
 
-html:focus,
-body:focus,
-#overlay:focus {
+html:focus-visible,
+body:focus-visible,
+#overlay:focus-visible {
   outline: none;
 }