fix: address coderabbit feedback

backlog/tasks/task-331 - Address-unresolved-CodeRabbit-comments-on-PR-57.md

@@ -0,0 +1,37 @@
+---
+id: TASK-331
+title: Address unresolved CodeRabbit comments on PR 57
+status: Done
+assignee:
+  - codex
+created_date: '2026-05-04 03:21'
+updated_date: '2026-05-04 03:27'
+labels:
+  - pr-feedback
+  - coderabbit
+dependencies: []
+references:
+  - 'https://github.com/ksyasuda/SubMiner/pull/57'
+priority: medium
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+Assess and fix unresolved CodeRabbit review comments on PR #57 after rebasing tokenizer-updates. Scope includes manual clipboard SentenceAudio guard, tokenizer standalone particle blacklist, AniList guessit fallback confidence, startup gate duplicate auto-start, and small regression-test hardening where applicable.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 Each unresolved CodeRabbit comment is either fixed or explicitly assessed as not applicable against current code.
+- [x] #2 Regression tests cover behavior changes where practical.
+- [x] #3 Relevant focused tests and typecheck pass.
+<!-- AC:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Fixed all verified actionable CodeRabbit comments from PR #57: manual clipboard updates no longer fall back to ExpressionAudio when SentenceAudio is absent, connective particle phrases no longer suppress lexical verb readings like 立って, guessit output only borrows parser season/episode from non-low-confidence parses, duplicate auto-start no longer releases an active pause-until-ready gate, JLPT CSS tests block text-decoration shorthand underlines, post-watch update rejection logging is covered, and duplicate quit-on-disconnect predicate code is shared.
+
+Verification: bun test src/anki-integration/card-creation-manual-update.test.ts src/core/services/tokenizer/annotation-stage.test.ts src/core/services/anilist/anilist-updater.test.ts src/main/runtime/mpv-main-event-actions.test.ts src/renderer/subtitle-render.test.ts; lua scripts/test-plugin-start-gate.lua; bun run typecheck; bun run test:fast.
+<!-- SECTION:NOTES:END -->

plugin/subminer/lifecycle.lua

@@ -83,11 +83,17 @@ function M.create(ctx)
 			return
 		end
 
-		aniskip.clear_aniskip_state()
-		process.disarm_auto_play_ready_gate()
-		local has_matching_socket = rearm_managed_subtitle_defaults()
-
 		local should_auto_start = resolve_auto_start_enabled()
+		local has_matching_socket = process.has_matching_mpv_ipc_socket(opts.socket_path)
+		local preserve_active_auto_start_gate = (
+			state.overlay_running and state.auto_play_ready_gate_armed and should_auto_start and has_matching_socket
+		)
+		aniskip.clear_aniskip_state()
+		if not preserve_active_auto_start_gate then
+			process.disarm_auto_play_ready_gate()
+		end
+		has_matching_socket = rearm_managed_subtitle_defaults()
+
 		if should_auto_start then
 			if not has_matching_socket then
 				subminer_log(

plugin/subminer/process.lua

@@ -299,7 +299,9 @@ function M.create(ctx)
 			if overrides.auto_start_trigger == true then
 				subminer_log("debug", "process", "Auto-start ignored because overlay is already running")
 				local socket_path = overrides.socket_path or opts.socket_path
+				if not state.auto_play_ready_gate_armed then
 					disarm_auto_play_ready_gate()
+				end
 				local visibility_action = resolve_visible_overlay_startup()
 						and "show-visible-overlay"
 					or "hide-visible-overlay"

scripts/test-plugin-start-gate.lua

@@ -559,6 +559,49 @@ do
 	)
 end
 
+do
+	local recorded, err = run_plugin_scenario({
+		process_list = "",
+		option_overrides = {
+			binary_path = binary_path,
+			auto_start = "yes",
+			auto_start_visible_overlay = "yes",
+			auto_start_pause_until_ready = "yes",
+			socket_path = "/tmp/subminer-socket",
+		},
+		input_ipc_server = "/tmp/subminer-socket",
+		media_title = "Random Movie",
+		files = {
+			[binary_path] = true,
+		},
+	})
+	assert_true(recorded ~= nil, "plugin failed to load for pre-ready duplicate auto-start scenario: " .. tostring(err))
+	fire_event(recorded, "file-loaded")
+	fire_event(recorded, "file-loaded")
+	assert_true(recorded.script_messages["subminer-autoplay-ready"] ~= nil, "subminer-autoplay-ready script message not registered")
+	assert_true(
+		count_start_calls(recorded.async_calls) == 1,
+		"pre-ready duplicate auto-start should not issue duplicate --start commands"
+	)
+	assert_true(
+		count_property_set(recorded.property_sets, "pause", true) == 1,
+		"pre-ready duplicate auto-start should not repeat the pause gate"
+	)
+	assert_true(
+		count_property_set(recorded.property_sets, "pause", false) == 0,
+		"pre-ready duplicate auto-start should not resume playback before tokenization is ready"
+	)
+	assert_true(
+		count_osd_message(recorded.osd, "SubMiner: Loading subtitle tokenization...") == 1,
+		"pre-ready duplicate auto-start should not repeat the loading OSD"
+	)
+	recorded.script_messages["subminer-autoplay-ready"]()
+	assert_true(
+		count_property_set(recorded.property_sets, "pause", false) == 1,
+		"autoplay-ready should resume the original pre-ready gate"
+	)
+end
+
 do
 	local recorded, err = run_plugin_scenario({
 		process_list = "",

src/anki-integration/card-creation-manual-update.test.ts

@@ -141,3 +141,37 @@ test('manual clipboard subtitle update replaces sentence audio without touching
     [true],
   );
 });
+
+test('manual clipboard subtitle update skips audio when sentence audio field is missing', async () => {
+  const { service, updatedFields, mergeCalls, storedMedia } = createManualUpdateService({
+    client: {
+      addNote: async () => 0,
+      addTags: async () => undefined,
+      notesInfo: async () => [
+        {
+          noteId: 42,
+          fields: {
+            Expression: { value: '単語' },
+            Sentence: { value: '' },
+            ExpressionAudio: { value: '[sound:auto-expression.mp3]' },
+          },
+        },
+      ],
+      updateNoteFields: async (_noteId, fields) => {
+        updatedFields.push(fields);
+      },
+      storeMediaFile: async (filename) => {
+        storedMedia.push(filename);
+      },
+      findNotes: async () => [42],
+      retrieveMediaFile: async () => '',
+    },
+  });
+
+  await service.updateLastAddedFromClipboard('字幕');
+
+  assert.equal(storedMedia.length, 1);
+  assert.equal(updatedFields.length, 1);
+  assert.deepEqual(updatedFields[0], { Sentence: '字幕' });
+  assert.equal(mergeCalls.length, 0);
+});

src/anki-integration/card-creation.ts

@@ -218,7 +218,7 @@ export class CardCreationService {
           fields,
           this.deps.getConfig(),
         );
-        const sentenceAudioField = this.getResolvedSentenceAudioFieldName(noteInfo);
+        const sentenceAudioField = this.getResolvedSentenceOnlyAudioFieldName(noteInfo);
         const sentenceField = this.deps.getEffectiveSentenceCardConfig().sentenceField;
 
         const sentence = blocks.join(' ');
@@ -721,6 +721,13 @@ export class CardCreationService {
     );
   }
 
+  private getResolvedSentenceOnlyAudioFieldName(noteInfo: CardCreationNoteInfo): string | null {
+    return this.deps.resolveNoteFieldName(
+      noteInfo,
+      this.deps.getEffectiveSentenceCardConfig().audioField || 'SentenceAudio',
+    );
+  }
+
   private createPendingNoteInfo(fields: Record<string, string>): CardCreationNoteInfo {
     return {
       noteId: -1,

src/core/services/anilist/anilist-updater.test.ts

@@ -34,6 +34,18 @@ test('guessAnilistMediaInfo fills missing guessit episode from filename parser',
   });
 });
 
+test('guessAnilistMediaInfo ignores low-confidence parser details when guessit omits them', async () => {
+  const result = await guessAnilistMediaInfo('/tmp/Season 2/Guessit Title.mkv', null, {
+    runGuessit: async () => JSON.stringify({ title: 'Guessit Title' }),
+  });
+  assert.deepEqual(result, {
+    title: 'Guessit Title',
+    season: null,
+    episode: null,
+    source: 'guessit',
+  });
+});
+
 test('guessAnilistMediaInfo parses Little Witch Academia release filename', async () => {
   const filename =
     '/tmp/Little Witch Academia (2017) - S01E02 - 002 - Papiliodia [Bluray-1080p][10bit][h265][AC3 2.0][JA].mkv';

src/core/services/anilist/anilist-updater.ts

@@ -237,12 +237,13 @@ export async function guessAnilistMediaInfo(
       const year = firstYear(parsed.year);
       if (title) {
         const fallback = parseMediaInfo(target);
+        const canUseFallbackDetails = fallback.confidence !== 'low';
         return {
           title: buildGuessitTitle(title, alternativeTitle),
           ...(alternativeTitle ? { alternativeTitle } : {}),
           ...(year ? { year } : {}),
-          season: season ?? fallback.season,
+          season: season ?? (canUseFallbackDetails ? fallback.season : null),
-          episode: episode ?? fallback.episode,
+          episode: episode ?? (canUseFallbackDetails ? fallback.episode : null),
           source: 'guessit',
         };
       }

src/core/services/tokenizer/annotation-stage.test.ts

@@ -567,6 +567,19 @@ test('shouldExcludeTokenFromSubtitleAnnotations excludes standalone connective p
   assert.equal(shouldExcludeTokenFromSubtitleAnnotations(token), true);
 });
 
+test('shouldExcludeTokenFromSubtitleAnnotations keeps lexical verbs whose reading matches connective particles', () => {
+  const token = makeToken({
+    surface: '立って',
+    headword: '立つ',
+    reading: 'タッテ',
+    partOfSpeech: PartOfSpeech.verb,
+    pos1: '動詞',
+    pos2: '自立',
+  });
+
+  assert.equal(shouldExcludeTokenFromSubtitleAnnotations(token), false);
+});
+
 test('shouldExcludeTokenFromSubtitleAnnotations excludes rhetorical もんか grammar particle phrases', () => {
   for (const surface of ['もんか', 'ものか']) {
     const token = makeToken({

src/core/services/tokenizer/subtitle-annotation-filter.ts

@@ -57,7 +57,6 @@ export const SUBTITLE_ANNOTATION_EXCLUDED_TERMS = new Set([
   '貴方',
   'もんか',
   'ものか',
-  ...STANDALONE_GRAMMAR_PARTICLE_PHRASES,
 ]);
 const SUBTITLE_ANNOTATION_EXCLUDED_TRAILING_PARTICLE_SUFFIXES = new Set([
   'って',

src/main/runtime/mpv-main-event-actions.test.ts

@@ -223,6 +223,41 @@ test('time-pos and pause handlers report progress with correct urgency', () => {
   ]);
 });
 
+test('time-pos handler logs post-watch update rejection without blocking later handlers', async () => {
+  const calls: string[] = [];
+  const timeHandler = createHandleMpvTimePosChangeHandler({
+    recordPlaybackPosition: (time) => calls.push(`time:${time}`),
+    reportJellyfinRemoteProgress: (force) => calls.push(`progress:${force ? 'force' : 'normal'}`),
+    refreshDiscordPresence: () => calls.push('presence'),
+    maybeRunAnilistPostWatchUpdate: async () => {
+      calls.push('post-watch');
+      throw new Error('boom');
+    },
+    logError: (message, error) => calls.push(`error:${message}:${(error as Error).message}`),
+  });
+  const pauseHandler = createHandleMpvPauseChangeHandler({
+    recordPauseState: (paused) => calls.push(`pause:${paused ? 'yes' : 'no'}`),
+    reportJellyfinRemoteProgress: (force) => calls.push(`progress:${force ? 'force' : 'normal'}`),
+    refreshDiscordPresence: () => calls.push('presence'),
+  });
+
+  timeHandler({ time: 12.5 });
+  pauseHandler({ paused: true });
+  await Promise.resolve();
+  await Promise.resolve();
+
+  assert.deepEqual(calls, [
+    'time:12.5',
+    'progress:normal',
+    'presence',
+    'post-watch',
+    'pause:yes',
+    'progress:force',
+    'presence',
+    'error:AniList post-watch update failed unexpectedly:boom',
+  ]);
+});
+
 test('subtitle metrics change handler forwards patch payload', () => {
   let received: Record<string, unknown> | null = null;
   const handler = createHandleMpvSubtitleMetricsChangeHandler({

src/main/runtime/mpv-main-event-main-deps.ts

@@ -78,23 +78,19 @@ export function createBuildBindMpvMainEventHandlersMainDepsHandler(deps: {
     deps.ensureImmersionTrackerInitialized();
     deps.appState.immersionTracker?.recordPlaybackPosition?.(normalizedTimeSec);
   };
+  const hasInitialPlaybackQuitOnDisconnectArg = (): boolean =>
+    Boolean(
+      deps.appState.initialArgs?.managedPlayback ||
+      deps.appState.initialArgs?.jellyfinPlay ||
+      deps.appState.initialArgs?.youtubePlay,
+    );
 
   return () => ({
     reportJellyfinRemoteStopped: () => deps.reportJellyfinRemoteStopped(),
     syncOverlayMpvSubtitleSuppression: () => deps.syncOverlayMpvSubtitleSuppression(),
-    hasInitialPlaybackQuitOnDisconnectArg: () =>
+    hasInitialPlaybackQuitOnDisconnectArg,
-      Boolean(
-        deps.appState.initialArgs?.managedPlayback ||
-        deps.appState.initialArgs?.jellyfinPlay ||
-        deps.appState.initialArgs?.youtubePlay,
-      ),
     isOverlayRuntimeInitialized: () => deps.appState.overlayRuntimeInitialized,
-    shouldQuitOnDisconnectWhenOverlayRuntimeInitialized: () =>
+    shouldQuitOnDisconnectWhenOverlayRuntimeInitialized: hasInitialPlaybackQuitOnDisconnectArg,
-      Boolean(
-        deps.appState.initialArgs?.managedPlayback ||
-        deps.appState.initialArgs?.jellyfinPlay ||
-        deps.appState.initialArgs?.youtubePlay,
-      ),
     isQuitOnDisconnectArmed: () => deps.getQuitOnDisconnectArmed(),
     scheduleQuitCheck: (callback: () => void) => deps.scheduleQuitCheck(callback),
     isMpvConnected: () => Boolean(deps.appState.mpvClient?.connected),

src/renderer/subtitle-render.test.ts

@@ -909,6 +909,7 @@ test('subtitle annotation CSS underlines JLPT tokens without changing token colo
     // popup/selection state.
     assert.doesNotMatch(plainJlptBlock, /(?:^|\n)\s*color\s*:/m);
     assert.doesNotMatch(plainJlptBlock, /text-decoration-line:\s*underline;/);
+    assert.doesNotMatch(plainJlptBlock, /text-decoration\s*:[^;]*\bunderline\b/i);
     assert.match(
       plainJlptBlock,
       new RegExp(`border-bottom:\\s*2px\\s+solid\\s+var\\(--subtitle-jlpt-n${level}-color,`),