test: add real subtitle verification lane

README.md

@@ -96,6 +96,11 @@ subminer --start video.mkv # optional explicit overlay start when plugin auto_st
 
 For full guides on configuration, Anki, Jellyfin, and more, see [docs.subminer.moe](https://docs.subminer.moe).
 
+## Verification
+
+- Run `bun run test:subtitle` to verify subtitle sync coverage for the maintained `alass`/`ffsubsync` test surface.
+- That lane reuses `src/core/services/subsync.test.ts` and `src/subsync/utils.test.ts`, which are also included in the broader `bun run test:core` suite.
+
 ## Acknowledgments
 
 Built on the shoulders of [GameSentenceMiner](https://github.com/bpwhelan/GameSentenceMiner), [texthooker-ui](https://github.com/Renji-XD/texthooker-ui), [mpvacious](https://github.com/Ajatt-Tools/mpvacious), [Anacreon-Script](https://github.com/friedrich-de/Anacreon-Script), and [autosubsync-mpv](https://github.com/joaquintorres/autosubsync-mpv). Subtitles powered by [Jimaku.cc](https://jimaku.cc). Dictionary lookups via [Yomitan](https://github.com/yomidevs/yomitan).

backlog/tasks/task-87.2 - Subtitle-sync-verification-replace-the-no-op-subtitle-lane-with-real-automated-coverage.md

@@ -3,10 +3,11 @@ id: TASK-87.2
 title: >-
   Subtitle sync verification: replace the no-op subtitle lane with real
   automated coverage
-status: To Do
+status: Done
-assignee: []
+assignee:
+  - Kyle Yasuda
 created_date: '2026-03-06 03:19'
-updated_date: '2026-03-06 03:21'
+updated_date: '2026-03-06 08:06'
 labels:
   - tests
   - subsync
@@ -27,27 +28,57 @@ priority: high
 ## Description
 
 <!-- SECTION:DESCRIPTION:BEGIN -->
-
 SubMiner advertises subtitle syncing with alass and ffsubsync, but the dedicated test:subtitle command currently does not run any tests. There is already lower-level coverage in src/core/services/subsync.test.ts, but the test matrix and contributor-facing commands do not reflect that reality. This task should replace the no-op lane with real verification, align scripts with the existing subsync test surface, and make the user-facing docs honest about how subtitle sync is verified.
-
 <!-- SECTION:DESCRIPTION:END -->
 
 ## Acceptance Criteria
-
 <!-- AC:BEGIN -->
-
+- [x] #1 The test:subtitle entrypoint runs real automated verification instead of echoing a placeholder message.
-- [ ] #1 The test:subtitle entrypoint runs real automated verification instead of echoing a placeholder message.
+- [x] #2 The subtitle verification lane covers both alass and ffsubsync behavior, including at least one non-happy-path scenario relevant to current functionality.
-- [ ] #2 The subtitle verification lane covers both alass and ffsubsync behavior, including at least one non-happy-path scenario relevant to current functionality.
+- [x] #3 Contributor-facing documentation points to the real subtitle verification command and no longer implies a dedicated test lane exists when it does not.
-- [ ] #3 Contributor-facing documentation points to the real subtitle verification command and no longer implies a dedicated test lane exists when it does not.
+- [x] #4 The resulting verification strategy integrates cleanly with the repository-wide test matrix without duplicating or hiding existing subsync coverage.
-- [ ] #4 The resulting verification strategy integrates cleanly with the repository-wide test matrix without duplicating or hiding existing subsync coverage.
 <!-- AC:END -->
 
 ## Implementation Plan
 
 <!-- SECTION:PLAN:BEGIN -->
+Plan of record:
 
-1. Audit the existing subtitle-sync test surface, especially src/core/services/subsync.test.ts, and decide whether test:subtitle should reuse or regroup that coverage.
+1. Replace the placeholder package-script lane with a real `test:subtitle:src` command that runs the maintained subtitle-sync tests directly (`src/core/services/subsync.test.ts` and `src/subsync/utils.test.ts`), and point `test:subtitle` at that lane instead of build+echo behavior.
-2. Replace the placeholder script with a real automated command and keep the matrix legible alongside TASK-87.1 work.
+2. Add one focused ffsubsync non-happy-path test in `src/core/services/subsync.test.ts` so the dedicated lane explicitly covers both engines plus failure propagation relevant to current functionality.
-3. Update README or related docs so the advertised subtitle verification path matches reality.
+3. Update `README.md` contributor guidance to name `bun run test:subtitle` as the subtitle verification command and explain that it reuses the maintained subsync tests already included in broader core coverage.
-4. Verify both alass and ffsubsync behavior remain covered by the resulting lane.
+4. Verify the final strategy by running `bun run test:subtitle` and `bun run test:core:src` so the dedicated lane stays aligned with the repository-wide matrix instead of creating a divergent hidden suite.
+
+Detailed execution plan saved at `docs/plans/2026-03-06-subtitle-sync-verification.md`.
 <!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Reviewed task references and current subtitle verification surface. Existing coverage already lives in `src/core/services/subsync.test.ts` and `src/subsync/utils.test.ts`; `test:subtitle` is still a placeholder build+echo wrapper. The referenced report `docs/reports/2026-02-22-task-100-dead-code-report.md` is not present in the workspace, so planning used the task body plus repository state instead.
+
+Implementation plan written and saved to `docs/plans/2026-03-06-subtitle-sync-verification.md`. Proceeding with execution per the task request.
+
+Replaced the placeholder subtitle lane with `test:subtitle:src` in `package.json`, pointing `test:subtitle` directly at `src/core/services/subsync.test.ts` and `src/subsync/utils.test.ts` instead of build+echo behavior.
+
+Added explicit ffsubsync failure-path coverage in `src/core/services/subsync.test.ts`, asserting non-zero command failures surface detailed `ffsubsync synchronization failed` messaging alongside existing alass coverage.
+
+Updated `README.md` verification guidance to point contributors at `bun run test:subtitle` and explain that the lane reuses the maintained subsync tests already included in `bun run test:core`.
+
+Verification: `bun run test:subtitle` passed (15 tests across 2 files). `bun run test:core:src` also passed (373 pass, 6 skip, 0 fail), confirming the dedicated subtitle lane stays aligned with the broader matrix.
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Implemented a real subtitle verification lane by replacing the placeholder `test:subtitle` build+echo flow with a source-level `test:subtitle:src` command that runs the maintained subtitle-sync tests directly from `src/core/services/subsync.test.ts` and `src/subsync/utils.test.ts`. This keeps subtitle verification explicit for contributors while still reusing the same maintained test surface already covered by `test:core`.
+
+Expanded subtitle-sync coverage with an explicit ffsubsync failure-path test so the dedicated lane now exercises both engines plus a user-visible non-happy path. Updated `README.md` to document `bun run test:subtitle` as the contributor-facing subtitle verification command and to explain its relationship to the broader core suite.
+
+Verification run:
+- `bun run test:subtitle`
+- `bun run test:core:src`
+
+Notes:
+- The task reference `docs/reports/2026-02-22-task-100-dead-code-report.md` was not present in the workspace during execution, so implementation used the task body and live repository state as the source of truth.
+<!-- SECTION:FINAL_SUMMARY:END -->

docs/plans/2026-03-06-subtitle-sync-verification.md

@@ -0,0 +1,121 @@
+# Subtitle Sync Verification Implementation Plan
+
+> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
+
+**Goal:** Replace the no-op `test:subtitle` lane with real automated subtitle-sync verification that reuses the maintained subsync tests and documents the real contributor workflow.
+
+**Architecture:** Repoint the subtitle verification command at the existing source-level subsync tests instead of inventing a second hidden suite. Add one focused ffsubsync failure-path test so the subtitle lane explicitly covers both engines plus a non-happy path, then update contributor docs to describe the dedicated subtitle lane and how it relates to `test:core`.
+
+**Tech Stack:** TypeScript, Bun test, Node test/assert, npm package scripts, Markdown docs.
+
+---
+
+### Task 1: Lock subtitle lane to real subsync tests
+
+**Files:**
+
+- Modify: `package.json`
+
+**Step 1: Write the failing test**
+
+Define the intended command shape first: `test:subtitle:src` should run `src/core/services/subsync.test.ts` and `src/subsync/utils.test.ts`, `test:subtitle` should invoke that real source lane, and no placeholder echo should remain.
+
+**Step 2: Run test to verify it fails**
+
+Run: `bun run test:subtitle`
+Expected: It performs a build and prints `Subtitle tests are currently not configured`, proving the lane is still a no-op.
+
+**Step 3: Write minimal implementation**
+
+Update `package.json` so:
+
+- `test:subtitle:src` runs `bun test src/core/services/subsync.test.ts src/subsync/utils.test.ts`
+- `test:subtitle` runs the new source lane directly
+- `test:subtitle:dist` is removed if it is no longer the real verification path
+
+**Step 4: Run test to verify it passes**
+
+Run: `bun run test:subtitle`
+Expected: PASS with Bun executing the real subtitle-sync test files.
+
+### Task 2: Add explicit ffsubsync non-happy-path coverage
+
+**Files:**
+
+- Modify: `src/core/services/subsync.test.ts`
+- Test: `src/core/services/subsync.test.ts`
+
+**Step 1: Write the failing test**
+
+Add a test that runs `runSubsyncManual({ engine: 'ffsubsync' })` with a stub ffsubsync executable that exits non-zero and writes stderr, then assert:
+
+- `result.ok === false`
+- `result.message` starts with `ffsubsync synchronization failed`
+- the failure message includes command details surfaced to the user
+
+**Step 2: Run test to verify it fails**
+
+Run: `bun test src/core/services/subsync.test.ts`
+Expected: FAIL because ffsubsync failure propagation is not asserted yet.
+
+**Step 3: Write minimal implementation**
+
+Keep production code unchanged unless the new test exposes a real bug. If needed, tighten failure assertions or message propagation in `src/core/services/subsync.ts` without changing successful behavior.
+
+**Step 4: Run test to verify it passes**
+
+Run: `bun test src/core/services/subsync.test.ts`
+Expected: PASS with both alass and ffsubsync paths covered, including a non-happy path.
+
+### Task 3: Make contributor docs match the real verification path
+
+**Files:**
+
+- Modify: `README.md`
+- Modify: `package.json`
+
+**Step 1: Write the failing test**
+
+Use the repository state as the failure signal: README currently advertises subtitle sync as a feature but does not tell contributors that `bun run test:subtitle` is the real verification lane.
+
+**Step 2: Run test to verify it fails**
+
+Run: `bun run test:subtitle && bun test src/subsync/utils.test.ts`
+Expected: Tests pass, but docs still do not explain the lane; this is the remaining acceptance-criteria gap.
+
+**Step 3: Write minimal implementation**
+
+Update `README.md` with a short contributor-facing verification note that:
+
+- points to `bun run test:subtitle` for subtitle-sync coverage
+- states that the lane reuses the maintained subsync tests already included in broader core coverage
+- avoids implying there is a separate hidden subtitle test harness beyond those tests
+
+**Step 4: Run test to verify it passes**
+
+Run: `bun run test:subtitle`
+Expected: PASS, with docs and scripts now aligned around the same subtitle verification strategy.
+
+### Task 4: Verify matrix integration stays clean
+
+**Files:**
+
+- Modify: `package.json` (only if Task 1/3 exposed cleanup needs)
+
+**Step 1: Write the failing test**
+
+Treat duplication as the failure condition: confirm the dedicated subtitle lane reuses the same maintained files already present in `test:core:src` rather than creating a second divergent suite.
+
+**Step 2: Run test to verify it fails**
+
+Run: `bun run test:subtitle && bun run test:core:src`
+Expected: If file lists diverge unexpectedly, this review step exposes it before handoff.
+
+**Step 3: Write minimal implementation**
+
+If needed, do the smallest script cleanup necessary so subtitle coverage remains explicit without hiding or duplicating existing core coverage.
+
+**Step 4: Run test to verify it passes**
+
+Run: `bun run test:subtitle && bun run test:core:src`
+Expected: PASS, confirming the dedicated lane and the broader core suite agree on subtitle coverage.

package.json

@@ -23,12 +23,12 @@
     "test:core:dist": "bun test dist/cli/args.test.js dist/cli/help.test.js dist/core/services/cli-command.test.js dist/core/services/ipc.test.js dist/core/services/anki-jimaku-ipc.test.js dist/core/services/field-grouping-overlay.test.js dist/core/services/numeric-shortcut-session.test.js dist/core/services/secondary-subtitle.test.js dist/core/services/mpv-render-metrics.test.js dist/core/services/overlay-content-measurement.test.js dist/core/services/mpv-control.test.js dist/core/services/mpv.test.js dist/core/services/runtime-options-ipc.test.js dist/core/services/runtime-config.test.js dist/core/services/config-hot-reload.test.js dist/core/services/discord-presence.test.js dist/core/services/tokenizer.test.js dist/core/services/tokenizer/annotation-stage.test.js dist/core/services/tokenizer/parser-selection-stage.test.js dist/core/services/tokenizer/parser-enrichment-stage.test.js dist/core/services/subsync.test.js dist/core/services/overlay-bridge.test.js dist/core/services/overlay-manager.test.js dist/core/services/overlay-shortcut-handler.test.js dist/core/services/mining.test.js dist/core/services/anki-jimaku.test.js dist/core/services/jimaku-download-path.test.js dist/core/services/jellyfin.test.js dist/core/services/jellyfin-remote.test.js dist/core/services/immersion-tracker-service.test.js dist/core/services/overlay-runtime-init.test.js dist/core/services/app-ready.test.js dist/core/services/startup-bootstrap.test.js dist/core/services/subtitle-processing-controller.test.js dist/core/services/anilist/anilist-token-store.test.js dist/core/services/anilist/anilist-update-queue.test.js dist/renderer/error-recovery.test.js dist/renderer/subtitle-render.test.js dist/renderer/handlers/mouse.test.js dist/renderer/handlers/keyboard.test.js dist/renderer/modals/jimaku.test.js dist/subsync/utils.test.js dist/main/anilist-url-guard.test.js dist/window-trackers/x11-tracker.test.js",
     "test:core:smoke:dist": "bun test dist/cli/help.test.js dist/core/services/runtime-config.test.js dist/core/services/ipc.test.js dist/core/services/overlay-manager.test.js dist/core/services/anilist/anilist-token-store.test.js dist/core/services/startup-bootstrap.test.js dist/renderer/error-recovery.test.js dist/main/anilist-url-guard.test.js dist/window-trackers/x11-tracker.test.js",
     "test:smoke:dist": "bun run test:config:smoke:dist && bun run test:core:smoke:dist",
-    "test:subtitle:dist": "echo \"Subtitle tests are currently not configured\"",
+    "test:subtitle:src": "bun test src/core/services/subsync.test.ts src/subsync/utils.test.ts",
     "test": "bun run test:config && bun run test:core",
     "test:config": "bun run test:config:src",
     "test:launcher": "bun run test:launcher:src",
     "test:core": "bun run test:core:src",
-    "test:subtitle": "bun run build && bun run test:subtitle:dist",
+    "test:subtitle": "bun run test:subtitle:src",
     "test:fast": "bun run test:config:src && bun run test:core:src",
     "generate:config-example": "bun run build && bun dist/generate-config-example.js",
     "start": "bun run build && electron . --start",

src/core/services/subsync.test.ts

@@ -276,6 +276,59 @@ test('runSubsyncManual writes deterministic _retimed filename when replace is fa
   assert.equal(outputPath, path.join(tmpDir, 'episode.ja_retimed.srt'));
 });
 
+test('runSubsyncManual reports ffsubsync command failures with details', async () => {
+  const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subsync-ffsubsync-failure-'));
+  const ffsubsyncPath = path.join(tmpDir, 'ffsubsync.sh');
+  const ffmpegPath = path.join(tmpDir, 'ffmpeg.sh');
+  const alassPath = path.join(tmpDir, 'alass.sh');
+  const videoPath = path.join(tmpDir, 'video.mkv');
+  const primaryPath = path.join(tmpDir, 'primary.srt');
+
+  fs.writeFileSync(videoPath, 'video');
+  fs.writeFileSync(primaryPath, 'sub');
+  writeExecutableScript(ffmpegPath, '#!/bin/sh\nexit 0\n');
+  writeExecutableScript(alassPath, '#!/bin/sh\nexit 0\n');
+  writeExecutableScript(ffsubsyncPath, '#!/bin/sh\necho "reference audio missing" >&2\nexit 1\n');
+
+  const deps = makeDeps({
+    getMpvClient: () => ({
+      connected: true,
+      currentAudioStreamIndex: null,
+      send: () => {},
+      requestProperty: async (name: string) => {
+        if (name === 'path') return videoPath;
+        if (name === 'sid') return 1;
+        if (name === 'secondary-sid') return null;
+        if (name === 'track-list') {
+          return [
+            {
+              id: 1,
+              type: 'sub',
+              selected: true,
+              external: true,
+              'external-filename': primaryPath,
+            },
+          ];
+        }
+        return null;
+      },
+    }),
+    getResolvedConfig: () => ({
+      defaultMode: 'manual',
+      alassPath,
+      ffsubsyncPath,
+      ffmpegPath,
+    }),
+  });
+
+  const result = await runSubsyncManual({ engine: 'ffsubsync', sourceTrackId: null }, deps);
+
+  assert.equal(result.ok, false);
+  assert.equal(result.message.startsWith('ffsubsync synchronization failed'), true);
+  assert.match(result.message, /code=1/);
+  assert.match(result.message, /reference audio missing/);
+});
+
 test('runSubsyncManual constructs alass command and returns failure on non-zero exit', async () => {
   const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subsync-alass-'));
   const alassLogPath = path.join(tmpDir, 'alass-args.log');