Preserve playlist browser selection across refreshes

- keep modal selection stable when playlist snapshots mutate
- align test clock and timestamp fixtures with db helpers
- add regression coverage for selection and time parsing
This commit is contained in:
2026-03-30 22:16:21 -07:00
parent e16250dedc
commit 13680af3f6
6 changed files with 293 additions and 24 deletions

View File

@@ -5,7 +5,7 @@ status: In Progress
assignee: assignee:
- codex - codex
created_date: '2026-03-30 05:46' created_date: '2026-03-30 05:46'
updated_date: '2026-03-31 01:42' updated_date: '2026-03-31 04:57'
labels: labels:
- feature - feature
- overlay - overlay
@@ -26,8 +26,8 @@ Add an in-session overlay modal that opens from a keybinding during active playb
- [ ] #2 The modal shows video files from the current media file's parent directory in best-effort episode order and highlights the current file when present. - [ ] #2 The modal shows video files from the current media file's parent directory in best-effort episode order and highlights the current file when present.
- [ ] #3 The modal shows the active mpv playlist/queue with enough metadata to identify the current item and queued order. - [ ] #3 The modal shows the active mpv playlist/queue with enough metadata to identify the current item and queued order.
- [ ] #4 The user can add a directory file to the mpv playlist, remove playlist items, and reorder playlist items from the modal using both mouse and keyboard interactions. - [ ] #4 The user can add a directory file to the mpv playlist, remove playlist items, and reorder playlist items from the modal using both mouse and keyboard interactions.
- [ ] #5 Modal state stays in sync after playlist mutations so the rendered queue reflects mpv's current playlist order. - [x] #5 Modal state stays in sync after playlist mutations so the rendered queue reflects mpv's current playlist order.
- [ ] #6 Feature coverage includes automated tests for ordering/playlist behavior and docs or shortcut/help updates for the new modal. - [x] #6 Feature coverage includes automated tests for ordering/playlist behavior and docs or shortcut/help updates for the new modal.
<!-- AC:END --> <!-- AC:END -->
## Implementation Plan ## Implementation Plan
@@ -42,6 +42,8 @@ Add an in-session overlay modal that opens from a keybinding during active playb
7. Run targeted test lanes first, then the maintained verification gate relevant to the touched surfaces; update task notes/criteria as checks pass. 7. Run targeted test lanes first, then the maintained verification gate relevant to the touched surfaces; update task notes/criteria as checks pass.
2026-03-30 CodeRabbit follow-up: 1) add failing runtime coverage for unreadable playlist-browser file stat failures, 2) add failing renderer coverage for stale snapshot UI reset on refresh failure/close, 3) add failing renderer coverage to block playlist-browser open when another modal already owns the overlay, 4) implement minimal fixes, 5) rerun targeted tests plus typecheck for touched surfaces. 2026-03-30 CodeRabbit follow-up: 1) add failing runtime coverage for unreadable playlist-browser file stat failures, 2) add failing renderer coverage for stale snapshot UI reset on refresh failure/close, 3) add failing renderer coverage to block playlist-browser open when another modal already owns the overlay, 4) implement minimal fixes, 5) rerun targeted tests plus typecheck for touched surfaces.
2026-03-30 current CodeRabbit round: verify 4 unresolved threads, ignore already-fixed outdated dblclick thread if current code matches, add failing-first coverage for selection preservation / timestamp fixture consistency / string test-clock alignment, implement minimal fixes, rerun targeted tests plus typecheck.
<!-- SECTION:PLAN:END --> <!-- SECTION:PLAN:END -->
## Implementation Notes ## Implementation Notes
@@ -70,4 +72,8 @@ Addressed CodeRabbit follow-ups on the playlist browser PR: clamped stale playin
Additional follow-up: moved playlist-browser keydown handling ahead of keyboard-driven lookup controls so KeyH/ArrowLeft/ArrowRight and related chords are routed to the modal first. Verification refreshed with `bun test src/main/runtime/playlist-browser-runtime.test.ts src/renderer/modals/playlist-browser.test.ts src/renderer/handlers/keyboard.test.ts`, `bun run typecheck`, and `bun run build`. Additional follow-up: moved playlist-browser keydown handling ahead of keyboard-driven lookup controls so KeyH/ArrowLeft/ArrowRight and related chords are routed to the modal first. Verification refreshed with `bun test src/main/runtime/playlist-browser-runtime.test.ts src/renderer/modals/playlist-browser.test.ts src/renderer/handlers/keyboard.test.ts`, `bun run typecheck`, and `bun run build`.
Split playlist-browser UI row rendering into `src/renderer/modals/playlist-browser-renderer.ts` and left `src/renderer/modals/playlist-browser.ts` as the controller/wiring layer. Moved playlist-browser IPC/runtime wiring into `src/main/runtime/playlist-browser-ipc.ts` and collapsed the `src/main.ts` registration block to use that helper. Verification after refactor: `bun run typecheck`, `bun run build`, `bun test src/main/runtime/playlist-browser-runtime.test.ts src/renderer/modals/playlist-browser.test.ts src/renderer/handlers/keyboard.test.ts`. Split playlist-browser UI row rendering into `src/renderer/modals/playlist-browser-renderer.ts` and left `src/renderer/modals/playlist-browser.ts` as the controller/wiring layer. Moved playlist-browser IPC/runtime wiring into `src/main/runtime/playlist-browser-ipc.ts` and collapsed the `src/main.ts` registration block to use that helper. Verification after refactor: `bun run typecheck`, `bun run build`, `bun test src/main/runtime/playlist-browser-runtime.test.ts src/renderer/modals/playlist-browser.test.ts src/renderer/handlers/keyboard.test.ts`.
2026-03-30 PR #37 unresolved CodeRabbit threads currently reduce to three likely-actionable items plus one outdated renderer dblclick thread to verify against HEAD before touching code.
2026-03-30 Addressed latest unresolved CodeRabbit items on PR #37: preserved playlist-browser selection across mutation snapshots, taught nowMs() to honor string-backed test clocks so it stays aligned with currentDbTimestamp(), and normalized maintenance test timestamp fixtures to toDbTimestamp(). The older playlist-browser dblclick thread remains unresolved in GitHub state but current HEAD already contains that fix in playlist-browser-renderer.ts.
<!-- SECTION:NOTES:END --> <!-- SECTION:NOTES:END -->

View File

@@ -95,22 +95,22 @@ test('pruneRawRetention skips disabled retention windows', () => {
INSERT INTO imm_videos ( INSERT INTO imm_videos (
video_id, video_key, canonical_title, source_type, duration_ms, CREATED_DATE, LAST_UPDATE_DATE video_id, video_key, canonical_title, source_type, duration_ms, CREATED_DATE, LAST_UPDATE_DATE
) VALUES ( ) VALUES (
1, 'local:/tmp/video.mkv', 'Video', 1, 0, '${nowMs}', '${nowMs}' 1, 'local:/tmp/video.mkv', 'Video', 1, 0, '${toDbTimestamp(nowMs)}', '${toDbTimestamp(nowMs)}'
); );
INSERT INTO imm_sessions ( INSERT INTO imm_sessions (
session_id, session_uuid, video_id, started_at_ms, ended_at_ms, status, CREATED_DATE, LAST_UPDATE_DATE session_id, session_uuid, video_id, started_at_ms, ended_at_ms, status, CREATED_DATE, LAST_UPDATE_DATE
) VALUES ( ) VALUES (
1, 'session-1', 1, '${nowMs - 1_000}', '${nowMs - 500}', 2, '${nowMs}', '${nowMs}' 1, 'session-1', 1, '${toDbTimestamp(nowMs - 1_000)}', '${toDbTimestamp(nowMs - 500)}', 2, '${toDbTimestamp(nowMs)}', '${toDbTimestamp(nowMs)}'
); );
INSERT INTO imm_session_telemetry ( INSERT INTO imm_session_telemetry (
session_id, sample_ms, total_watched_ms, active_watched_ms, CREATED_DATE, LAST_UPDATE_DATE session_id, sample_ms, total_watched_ms, active_watched_ms, CREATED_DATE, LAST_UPDATE_DATE
) VALUES ( ) VALUES (
1, '${nowMs - 2_000}', 0, 0, '${nowMs}', '${nowMs}' 1, '${toDbTimestamp(nowMs - 2_000)}', 0, 0, '${toDbTimestamp(nowMs)}', '${toDbTimestamp(nowMs)}'
); );
INSERT INTO imm_session_events ( INSERT INTO imm_session_events (
session_id, event_type, ts_ms, payload_json, CREATED_DATE, LAST_UPDATE_DATE session_id, event_type, ts_ms, payload_json, CREATED_DATE, LAST_UPDATE_DATE
) VALUES ( ) VALUES (
1, 1, '${nowMs - 3_000}', '{}', '${nowMs}', '${nowMs}' 1, 1, '${toDbTimestamp(nowMs - 3_000)}', '{}', '${toDbTimestamp(nowMs)}', '${toDbTimestamp(nowMs)}'
); );
`); `);
@@ -162,17 +162,17 @@ test('raw retention keeps rollups and rollup retention prunes them separately',
INSERT INTO imm_videos ( INSERT INTO imm_videos (
video_id, video_key, canonical_title, source_type, duration_ms, CREATED_DATE, LAST_UPDATE_DATE video_id, video_key, canonical_title, source_type, duration_ms, CREATED_DATE, LAST_UPDATE_DATE
) VALUES ( ) VALUES (
1, 'local:/tmp/video.mkv', 'Video', 1, 0, '${nowMs}', '${nowMs}' 1, 'local:/tmp/video.mkv', 'Video', 1, 0, '${toDbTimestamp(nowMs)}', '${toDbTimestamp(nowMs)}'
); );
INSERT INTO imm_sessions ( INSERT INTO imm_sessions (
session_id, session_uuid, video_id, started_at_ms, ended_at_ms, status, CREATED_DATE, LAST_UPDATE_DATE session_id, session_uuid, video_id, started_at_ms, ended_at_ms, status, CREATED_DATE, LAST_UPDATE_DATE
) VALUES ( ) VALUES (
1, 'session-1', 1, '${nowMs - 200_000_000}', '${nowMs - 199_999_000}', 2, '${nowMs}', '${nowMs}' 1, 'session-1', 1, '${toDbTimestamp(nowMs - 200_000_000)}', '${toDbTimestamp(nowMs - 199_999_000)}', 2, '${toDbTimestamp(nowMs)}', '${toDbTimestamp(nowMs)}'
); );
INSERT INTO imm_session_telemetry ( INSERT INTO imm_session_telemetry (
session_id, sample_ms, total_watched_ms, active_watched_ms, CREATED_DATE, LAST_UPDATE_DATE session_id, sample_ms, total_watched_ms, active_watched_ms, CREATED_DATE, LAST_UPDATE_DATE
) VALUES ( ) VALUES (
1, '${nowMs - 200_000_000}', 0, 0, '${nowMs}', '${nowMs}' 1, '${toDbTimestamp(nowMs - 200_000_000)}', 0, 0, '${toDbTimestamp(nowMs)}', '${toDbTimestamp(nowMs)}'
); );
INSERT INTO imm_daily_rollups ( INSERT INTO imm_daily_rollups (
rollup_day, video_id, total_sessions, total_active_min, total_lines_seen, rollup_day, video_id, total_sessions, total_active_min, total_lines_seen,
@@ -184,7 +184,7 @@ test('raw retention keeps rollups and rollup retention prunes them separately',
rollup_month, video_id, total_sessions, total_active_min, total_lines_seen, rollup_month, video_id, total_sessions, total_active_min, total_lines_seen,
total_tokens_seen, total_cards, CREATED_DATE, LAST_UPDATE_DATE total_tokens_seen, total_cards, CREATED_DATE, LAST_UPDATE_DATE
) VALUES ( ) VALUES (
${oldMonth}, 1, 1, 10, 1, 1, 1, '${nowMs}', '${nowMs}' ${oldMonth}, 1, 1, 10, 1, 1, 1, '${toDbTimestamp(nowMs)}', '${toDbTimestamp(nowMs)}'
); );
`); `);

View File

@@ -5,3 +5,14 @@ import { nowMs } from './time.js';
test('nowMs returns wall-clock epoch milliseconds', () => { test('nowMs returns wall-clock epoch milliseconds', () => {
assert.ok(nowMs() > 1_600_000_000_000); assert.ok(nowMs() > 1_600_000_000_000);
}); });
test('nowMs honors string-backed test clock values', () => {
const previousNowMs = globalThis.__subminerTestNowMs;
globalThis.__subminerTestNowMs = '1700000000123.9';
try {
assert.equal(nowMs(), 1_700_000_000_123);
} finally {
globalThis.__subminerTestNowMs = previousNowMs;
}
});

View File

@@ -2,11 +2,24 @@ declare global {
var __subminerTestNowMs: number | string | undefined; var __subminerTestNowMs: number | string | undefined;
} }
export function nowMs(): number { function getMockNowMs(testNowMs: number | string | undefined): number | null {
const testNowMs = globalThis.__subminerTestNowMs;
if (typeof testNowMs === 'number' && Number.isFinite(testNowMs)) { if (typeof testNowMs === 'number' && Number.isFinite(testNowMs)) {
return Math.floor(testNowMs); return Math.floor(testNowMs);
} }
if (typeof testNowMs === 'string') {
const parsed = Number(testNowMs.trim());
if (Number.isFinite(parsed)) {
return Math.trunc(parsed);
}
}
return null;
}
export function nowMs(): number {
const mockedNowMs = getMockNowMs(globalThis.__subminerTestNowMs);
if (mockedNowMs !== null) {
return mockedNowMs;
}
const perf = globalThis.performance; const perf = globalThis.performance;
if (perf && Number.isFinite(perf.timeOrigin)) { if (perf && Number.isFinite(perf.timeOrigin)) {

View File

@@ -141,6 +141,68 @@ function createSnapshot(): PlaylistBrowserSnapshot {
}; };
} }
function createMutationSnapshot(): PlaylistBrowserSnapshot {
return {
directoryPath: '/tmp/show',
directoryAvailable: true,
directoryStatus: '/tmp/show',
currentFilePath: '/tmp/show/Show - S01E02.mkv',
playingIndex: 0,
directoryItems: [
{
path: '/tmp/show/Show - S01E01.mkv',
basename: 'Show - S01E01.mkv',
episodeLabel: 'S1E1',
isCurrentFile: false,
},
{
path: '/tmp/show/Show - S01E02.mkv',
basename: 'Show - S01E02.mkv',
episodeLabel: 'S1E2',
isCurrentFile: true,
},
{
path: '/tmp/show/Show - S01E03.mkv',
basename: 'Show - S01E03.mkv',
episodeLabel: 'S1E3',
isCurrentFile: false,
},
],
playlistItems: [
{
index: 1,
id: 2,
filename: '/tmp/show/Show - S01E02.mkv',
title: 'Episode 2',
displayLabel: 'Episode 2',
current: true,
playing: true,
path: '/tmp/show/Show - S01E02.mkv',
},
{
index: 2,
id: 3,
filename: '/tmp/show/Show - S01E03.mkv',
title: 'Episode 3',
displayLabel: 'Episode 3',
current: false,
playing: false,
path: '/tmp/show/Show - S01E03.mkv',
},
{
index: 0,
id: 1,
filename: '/tmp/show/Show - S01E01.mkv',
title: 'Episode 1',
displayLabel: 'Episode 1',
current: false,
playing: false,
path: '/tmp/show/Show - S01E01.mkv',
},
],
};
}
test('playlist browser modal opens with playlist-focused current item selection', async () => { test('playlist browser modal opens with playlist-focused current item selection', async () => {
const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown }; const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown };
const previousWindow = globals.window; const previousWindow = globals.window;
@@ -298,6 +360,111 @@ test('playlist browser modal action buttons stop double-click propagation', asyn
} }
}); });
test('playlist browser preserves prior selection across mutation snapshots', async () => {
const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown };
const previousWindow = globals.window;
const previousDocument = globals.document;
Object.defineProperty(globalThis, 'window', {
configurable: true,
value: {
electronAPI: {
getPlaylistBrowserSnapshot: async () => ({
...createSnapshot(),
directoryItems: [
...createSnapshot().directoryItems,
{
path: '/tmp/show/Show - S01E03.mkv',
basename: 'Show - S01E03.mkv',
episodeLabel: 'S1E3',
isCurrentFile: false,
},
],
playlistItems: [
...createSnapshot().playlistItems,
{
index: 2,
id: 3,
filename: '/tmp/show/Show - S01E03.mkv',
title: 'Episode 3',
displayLabel: 'Episode 3',
current: false,
playing: false,
path: '/tmp/show/Show - S01E03.mkv',
},
],
}),
notifyOverlayModalOpened: () => {},
notifyOverlayModalClosed: () => {},
focusMainWindow: async () => {},
setIgnoreMouseEvents: () => {},
appendPlaylistBrowserFile: async () => ({
ok: true,
message: 'Queued file',
snapshot: createMutationSnapshot(),
}),
playPlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
removePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
movePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
} as unknown as ElectronAPI,
focus: () => {},
},
});
Object.defineProperty(globalThis, 'document', {
configurable: true,
value: {
createElement: () => createPlaylistRow(),
},
});
try {
const state = createRendererState();
const ctx = {
state,
platform: {
shouldToggleMouseIgnore: false,
},
dom: {
overlay: {
classList: createClassList(),
focus: () => {},
},
playlistBrowserModal: createFakeElement(),
playlistBrowserTitle: createFakeElement(),
playlistBrowserStatus: createFakeElement(),
playlistBrowserDirectoryList: createListStub(),
playlistBrowserPlaylistList: createListStub(),
playlistBrowserClose: createFakeElement(),
},
};
const modal = createPlaylistBrowserModal(ctx as never, {
modalStateReader: { isAnyModalOpen: () => false },
syncSettingsModalSubtitleSuppression: () => {},
});
await modal.openPlaylistBrowserModal();
state.playlistBrowserActivePane = 'directory';
state.playlistBrowserSelectedDirectoryIndex = 2;
state.playlistBrowserSelectedPlaylistIndex = 0;
await modal.handlePlaylistBrowserKeydown({
key: 'Enter',
code: 'Enter',
preventDefault: () => {},
ctrlKey: false,
metaKey: false,
shiftKey: false,
} as never);
assert.equal(state.playlistBrowserSelectedDirectoryIndex, 2);
assert.equal(state.playlistBrowserSelectedPlaylistIndex, 2);
} finally {
Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow });
Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument });
}
});
test('playlist browser modal keydown routes append, remove, reorder, tab switch, and play', async () => { test('playlist browser modal keydown routes append, remove, reorder, tab switch, and play', async () => {
const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown }; const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown };
const previousWindow = globals.window; const previousWindow = globals.window;

View File

@@ -1,4 +1,5 @@
import type { import type {
PlaylistBrowserDirectoryItem,
PlaylistBrowserMutationResult, PlaylistBrowserMutationResult,
PlaylistBrowserQueueItem, PlaylistBrowserQueueItem,
PlaylistBrowserSnapshot, PlaylistBrowserSnapshot,
@@ -23,6 +24,68 @@ function buildDefaultStatus(snapshot: PlaylistBrowserSnapshot): string {
return `${directoryCount} sibling videos · ${playlistCount} queued`; return `${directoryCount} sibling videos · ${playlistCount} queued`;
} }
function getDefaultDirectorySelectionIndex(snapshot: PlaylistBrowserSnapshot): number {
const directoryIndex = snapshot.directoryItems.findIndex((item) => item.isCurrentFile);
return clampIndex(directoryIndex >= 0 ? directoryIndex : 0, snapshot.directoryItems.length);
}
function getDefaultPlaylistSelectionIndex(snapshot: PlaylistBrowserSnapshot): number {
const playlistIndex =
snapshot.playingIndex ?? snapshot.playlistItems.findIndex((item) => item.current || item.playing);
return clampIndex(playlistIndex >= 0 ? playlistIndex : 0, snapshot.playlistItems.length);
}
function resolvePreservedIndex<T>(
previousIndex: number,
previousItems: T[],
nextItems: T[],
matchIndex: (previousItem: T) => number,
): number {
if (nextItems.length <= 0) return 0;
if (previousItems.length <= 0) return clampIndex(previousIndex, nextItems.length);
const normalizedPreviousIndex = clampIndex(previousIndex, previousItems.length);
const previousItem = previousItems[normalizedPreviousIndex];
const matchedIndex = previousItem ? matchIndex(previousItem) : -1;
return clampIndex(matchedIndex >= 0 ? matchedIndex : normalizedPreviousIndex, nextItems.length);
}
function resolveDirectorySelectionIndex(
snapshot: PlaylistBrowserSnapshot,
previousSnapshot: PlaylistBrowserSnapshot,
previousIndex: number,
): number {
return resolvePreservedIndex(
previousIndex,
previousSnapshot.directoryItems,
snapshot.directoryItems,
(previousItem: PlaylistBrowserDirectoryItem) =>
snapshot.directoryItems.findIndex((item) => item.path === previousItem.path),
);
}
function resolvePlaylistSelectionIndex(
snapshot: PlaylistBrowserSnapshot,
previousSnapshot: PlaylistBrowserSnapshot,
previousIndex: number,
): number {
return resolvePreservedIndex(
previousIndex,
previousSnapshot.playlistItems,
snapshot.playlistItems,
(previousItem: PlaylistBrowserQueueItem) => {
if (previousItem.id !== null) {
const byId = snapshot.playlistItems.findIndex((item) => item.id === previousItem.id);
if (byId >= 0) return byId;
}
if (previousItem.path) {
return snapshot.playlistItems.findIndex((item) => item.path === previousItem.path);
}
return -1;
},
);
}
export function createPlaylistBrowserModal( export function createPlaylistBrowserModal(
ctx: RendererContext, ctx: RendererContext,
options: { options: {
@@ -52,17 +115,25 @@ export function createPlaylistBrowserModal(
ctx.dom.playlistBrowserStatus.classList.remove('error'); ctx.dom.playlistBrowserStatus.classList.remove('error');
} }
function syncSelection(snapshot: PlaylistBrowserSnapshot): void { function syncSelection(
const directoryIndex = snapshot.directoryItems.findIndex((item) => item.isCurrentFile); snapshot: PlaylistBrowserSnapshot,
const playlistIndex = previousSnapshot: PlaylistBrowserSnapshot | null,
snapshot.playingIndex ?? snapshot.playlistItems.findIndex((item) => item.current || item.playing); ): void {
ctx.state.playlistBrowserSelectedDirectoryIndex = clampIndex( if (!previousSnapshot) {
directoryIndex >= 0 ? directoryIndex : 0, ctx.state.playlistBrowserSelectedDirectoryIndex = getDefaultDirectorySelectionIndex(snapshot);
snapshot.directoryItems.length, ctx.state.playlistBrowserSelectedPlaylistIndex = getDefaultPlaylistSelectionIndex(snapshot);
return;
}
ctx.state.playlistBrowserSelectedDirectoryIndex = resolveDirectorySelectionIndex(
snapshot,
previousSnapshot,
ctx.state.playlistBrowserSelectedDirectoryIndex,
); );
ctx.state.playlistBrowserSelectedPlaylistIndex = clampIndex( ctx.state.playlistBrowserSelectedPlaylistIndex = resolvePlaylistSelectionIndex(
playlistIndex >= 0 ? playlistIndex : 0, snapshot,
snapshot.playlistItems.length, previousSnapshot,
ctx.state.playlistBrowserSelectedPlaylistIndex,
); );
} }
@@ -102,8 +173,9 @@ export function createPlaylistBrowserModal(
} }
function applySnapshot(snapshot: PlaylistBrowserSnapshot): void { function applySnapshot(snapshot: PlaylistBrowserSnapshot): void {
const previousSnapshot = ctx.state.playlistBrowserSnapshot;
ctx.state.playlistBrowserSnapshot = snapshot; ctx.state.playlistBrowserSnapshot = snapshot;
syncSelection(snapshot); syncSelection(snapshot, previousSnapshot);
render(); render();
} }