From de111dbf8d948c90580731078286a3d82a31fb07 Mon Sep 17 00:00:00 2001
From: sudacode <suda@sudacode.com>
Date: Sat, 21 Mar 2026 10:15:06 -0700
Subject: [PATCH] fix(subtitle-sidebar): address CodeRabbit review

---
 src/main.ts                                   | 47 ++++++++++---------
 .../runtime/subtitle-prefetch-source.test.ts  | 18 +++++++
 src/main/runtime/subtitle-prefetch-source.ts  |  7 ++-
 src/renderer/style.css                        |  1 -
 4 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/src/main.ts b/src/main.ts
index 2b3105d..be30496 100644
--- a/src/main.ts
+++ b/src/main.ts
@@ -4056,7 +4056,7 @@ async function extractInternalSubtitleTrackToTempFile(
   videoPath: string,
   track: MpvSubtitleTrackLike,
 ): Promise<{ path: string; cleanup: () => Promise<void> } | null> {
-  const ffIndex = typeof track['ff-index'] === 'number' ? track['ff-index'] : null;
+  const ffIndex = parseTrackId(track['ff-index']);
   const codec = typeof track.codec === 'string' ? track.codec : null;
   const extension = codecToExtension(codec ?? undefined);
   if (ffIndex === null || extension === null) {
@@ -4066,23 +4066,31 @@ async function extractInternalSubtitleTrackToTempFile(
   const tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'subminer-sidebar-'));
   const outputPath = path.join(tempDir, `track_${ffIndex}.${extension}`);
 
-  await new Promise<void>((resolve, reject) => {
-    const child = spawn(ffmpegPath, buildFfmpegSubtitleExtractionArgs(videoPath, ffIndex, outputPath));
-    let stderr = '';
-    child.stderr.on('data', (chunk: Buffer) => {
-      stderr += chunk.toString();
+  try {
+    await new Promise<void>((resolve, reject) => {
+      const child = spawn(
+        ffmpegPath,
+        buildFfmpegSubtitleExtractionArgs(videoPath, ffIndex, outputPath),
+      );
+      let stderr = '';
+      child.stderr.on('data', (chunk: Buffer) => {
+        stderr += chunk.toString();
+      });
+      child.on('error', (error) => {
+        reject(error);
+      });
+      child.on('close', (code) => {
+        if (code === 0) {
+          resolve();
+          return;
+        }
+        reject(new Error(stderr.trim() || `ffmpeg exited with code ${code ?? 'unknown'}`));
+      });
     });
-    child.on('error', (error) => {
-      reject(error);
-    });
-    child.on('close', (code) => {
-      if (code === 0) {
-        resolve();
-        return;
-      }
-      reject(new Error(stderr.trim() || `ffmpeg exited with code ${code ?? 'unknown'}`));
-    });
-  });
+  } catch (error) {
+    await fs.promises.rm(tempDir, { recursive: true, force: true }).catch(() => undefined);
+    throw error;
+  }
 
   return {
     path: outputPath,
@@ -4245,10 +4253,7 @@ const { registerIpcRuntimeHandlers } = composeIpcRuntimeHandlers({
             };
           }
 
-          if (
-            appState.activeParsedSubtitleCues.length > 0 &&
-            appState.activeParsedSubtitleSource === resolvedSource.sourceKey
-          ) {
+          if (appState.activeParsedSubtitleSource === resolvedSource.sourceKey) {
             return {
               cues: appState.activeParsedSubtitleCues,
               currentTimeSec,
diff --git a/src/main/runtime/subtitle-prefetch-source.test.ts b/src/main/runtime/subtitle-prefetch-source.test.ts
index c66362a..6bb8183 100644
--- a/src/main/runtime/subtitle-prefetch-source.test.ts
+++ b/src/main/runtime/subtitle-prefetch-source.test.ts
@@ -18,6 +18,15 @@ test('getActiveExternalSubtitleSource returns the active external subtitle path'
   assert.equal(source, 'https://host/subs.ass');
 });
 
+test('getActiveExternalSubtitleSource normalizes integer-like string track ids', () => {
+  const source = getActiveExternalSubtitleSource(
+    [{ type: 'sub', id: '2', external: true, 'external-filename': ' /tmp/subs.ass ' }],
+    '2',
+  );
+
+  assert.equal(source, '/tmp/subs.ass');
+});
+
 test('getActiveExternalSubtitleSource returns null when the selected track is not external', () => {
   const source = getActiveExternalSubtitleSource(
     [{ type: 'sub', id: 2, external: false, 'external-filename': '/tmp/subs.ass' }],
@@ -70,6 +79,15 @@ test('buildSubtitleSidebarSourceKey uses a stable identifier for internal subtit
   assert.equal(firstKey, 'internal:/media/episode01.mkv:track:3:ff:7');
 });
 
+test('buildSubtitleSidebarSourceKey normalizes integer-like string track metadata', () => {
+  const key = buildSubtitleSidebarSourceKey('/media/episode01.mkv', {
+    id: '3',
+    'ff-index': '7',
+  });
+
+  assert.equal(key, 'internal:/media/episode01.mkv:track:3:ff:7');
+});
+
 test('buildSubtitleSidebarSourceKey falls back to source path when no track metadata is available', () => {
   const key = buildSubtitleSidebarSourceKey('/media/episode01.mkv', null, '/tmp/subtitle.ass');
 
diff --git a/src/main/runtime/subtitle-prefetch-source.ts b/src/main/runtime/subtitle-prefetch-source.ts
index a7b4ede..688ad71 100644
--- a/src/main/runtime/subtitle-prefetch-source.ts
+++ b/src/main/runtime/subtitle-prefetch-source.ts
@@ -19,9 +19,8 @@ export function getActiveExternalSubtitleSource(
     return null;
   }
 
-  const sid =
-    typeof sidRaw === 'number' ? sidRaw : typeof sidRaw === 'string' ? Number(sidRaw) : null;
-  if (sid == null || !Number.isFinite(sid)) {
+  const sid = parseTrackId(sidRaw);
+  if (sid === null) {
     return null;
   }
 
@@ -30,7 +29,7 @@ export function getActiveExternalSubtitleSource(
       return false;
     }
     const track = entry as Record<string, unknown>;
-    return track.type === 'sub' && track.id === sid;
+    return track.type === 'sub' && parseTrackId(track.id) === sid && track.external === true;
   }) as Record<string, unknown> | undefined;
 
   const externalFilename =
diff --git a/src/renderer/style.css b/src/renderer/style.css
index fe3f237..699f0ac 100644
--- a/src/renderer/style.css
+++ b/src/renderer/style.css
@@ -726,7 +726,6 @@ body.subtitle-sidebar-embedded-open #secondarySubContainer {
   position: absolute;
   top: 40px;
   left: 50%;
-  transform: translateX(-50%);
   max-width: min(80%, calc(100vw - var(--subtitle-sidebar-reserved-width) - 24px));
   padding: 10px 18px;
   background: var(--secondary-sub-background-color, transparent);