mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-04-01 06:12:07 -07:00
Add playlist browser overlay modal (#37)
This commit is contained in:
GitHub
@@ -1,9 +1,11 @@
|
||||
---
|
||||
id: TASK-255
|
||||
title: Add overlay playlist browser modal for sibling video files and mpv queue
|
||||
status: To Do
|
||||
assignee: []
|
||||
status: In Progress
|
||||
assignee:
|
||||
- '@codex'
|
||||
created_date: '2026-03-30 05:46'
|
||||
updated_date: '2026-03-31 05:59'
|
||||
labels:
|
||||
- feature
|
||||
- overlay
|
||||
@@ -24,6 +26,62 @@ 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.
|
||||
- [ ] #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.
|
||||
- [ ] #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] #5 Modal state stays in sync after playlist mutations so the rendered queue reflects mpv's current playlist order.
|
||||
- [x] #6 Feature coverage includes automated tests for ordering/playlist behavior and docs or shortcut/help updates for the new modal.
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
1. Add playlist-browser domain types, IPC channels, overlay modal registration, special command, and default keybinding for Ctrl+Alt+P.
|
||||
2. Write failing tests for best-effort episode sorting and main playlist-browser runtime snapshot/mutation behavior.
|
||||
3. Implement playlist-browser main/runtime helpers for local sibling video discovery, mpv playlist normalization, and append/play/remove/move operations with refreshed snapshots.
|
||||
4. Wire preload and main-process IPC handlers that expose snapshot and mutation methods to the renderer.
|
||||
5. Write failing renderer and keyboard tests for modal open/close, split-pane interaction, keyboard controls, and degraded states.
|
||||
6. Implement playlist-browser modal markup, DOM/state, renderer composition, keyboard routing, and session-help labeling.
|
||||
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 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.
|
||||
|
||||
2026-03-30 latest CodeRabbit round on PR #37: 1) add failing coverage for negative fractional numeric __subminerTestNowMs input so nowMs() matches the string-backed path, 2) add failing coverage that playlist-browser modal tests restore absent window/document globals without leaving undefined-valued properties behind, 3) refactor repeated playlist-browser modal test harness into a shared setup/teardown fixture while preserving assertions, 4) implement minimal fixes, 5) rerun touched tests plus typecheck.
|
||||
|
||||
2026-03-30 latest CodeRabbit follow-up after ff760ea: tighten the new cleanup regression so env.restore() always runs under assertion failure, and make the keydown test's append mock return a post-append mutated snapshot before exercising Ctrl+ArrowDown. Re-run targeted playlist-browser tests plus typecheck.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
Implemented overlay playlist browser modal with split directory/playlist panes, Ctrl+Alt+P keybinding, main/preload IPC, mpv queue mutations, and best-effort sibling episode sorting.
|
||||
|
||||
Added tests for sort/runtime logic, IPC wiring, keyboard routing, and playlist-browser modal behavior.
|
||||
|
||||
Verification: `bun run typecheck` passed; targeted playlist-browser and IPC tests passed; `bun run build` passed; `bun run test:smoke:dist` passed.
|
||||
|
||||
Repo gate blockers outside this feature: `bun run test:fast` hits existing Bun `node:test` NotImplementedError cases plus unrelated immersion-tracker failures; `bun run test:env` fails in existing immersion-tracker sqlite tests.
|
||||
|
||||
2026-03-30: Fixed playlist-browser local playback regression where subtitle track IDs leaked across episode jumps. `playPlaylistBrowserIndexRuntime` now reapplies local subtitle auto-selection defaults (`sub-auto=fuzzy`, `sid=auto`, `secondary-sid=auto`) before `playlist-play-index` for local filesystem targets only; remote playlist entries remain untouched. Added runtime regression tests for both paths.
|
||||
|
||||
2026-03-30: Follow-up subtitle regression fix. Pre-jump `sid=auto` was ineffective because mpv resolved it against the current episode before `playlist-play-index`. Local playlist jumps now set `sub-auto=fuzzy`, switch episodes, then schedule a delayed rearm of `sid=auto` and `secondary-sid=auto` so selection happens against the new file's tracks. Added failing-first runtime coverage for delayed local rearm and remote no-op behavior.
|
||||
|
||||
2026-03-30: Cleaned up playlist-browser runtime local-play subtitle-rearm flow by extracting focused helpers without changing behavior. Added public docs/readme coverage for the default `Ctrl+Alt+P` playlist browser keybinding and modal, plus changelog fragment `changes/260-playlist-browser.md`. Verification: `bun test src/main/runtime/playlist-browser-runtime.test.ts`, `bun run typecheck`, `bun run docs:test`, `bun run docs:build`, `bun run changelog:lint`, `bun run build`.
|
||||
|
||||
2026-03-30: Pulled unresolved CodeRabbit review threads for PR #37. Actionable set is three items: unreadable-file stat error handling in playlist-browser runtime, stale playlist-browser DOM after failed refresh/close, and missing modal-ownership guard before opening the playlist-browser overlay. Proceeding test-first for each.
|
||||
|
||||
2026-03-30: Addressed current CodeRabbit follow-up findings for PR #37. Fixed playlist-browser unreadable-file stat handling, stale playlist-browser DOM reset on refresh failure/close, modal-ownership guard before opening the playlist-browser overlay, async rejection surfacing for PLAYLIST_BROWSER_OPEN IPC commands, overlay bootstrap before playlist-browser open dispatch, texthooker option normalization in the mpv plugin, and superseded local subtitle-rearm suppression. Added targeted regressions plus new playlist-browser-open helper coverage. Verification: `bun test src/main/runtime/playlist-browser-runtime.test.ts src/main/runtime/playlist-browser-open.test.ts src/core/services/ipc-command.test.ts src/renderer/modals/playlist-browser.test.ts`, `lua scripts/test-plugin-start-gate.lua`, `bun run typecheck`, `bun run build`.
|
||||
|
||||
Addressed CodeRabbit follow-ups on the playlist browser PR: clamped stale playingIndex values, failed mutation paths when MPV rejects send(), added temp-dir cleanup in runtime tests, and blocked action-button dblclick bubbling in the renderer. Verification: `bun run typecheck`, `bun run build`, `bun test src/main/runtime/playlist-browser-runtime.test.ts src/renderer/modals/playlist-browser.test.ts`.
|
||||
|
||||
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`.
|
||||
|
||||
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.
|
||||
|
||||
2026-03-30 latest CodeRabbit remediation on PR #37: switched nowMs() numeric test-clock branch from Math.floor() to Math.trunc() so numeric and string-backed mock clocks agree for negative fractional values. Refactored playlist-browser modal tests onto a shared setup/teardown fixture that restores global window/document descriptors correctly, and added regression coverage that injected globals are deleted when originally absent. Verification: `bun test src/core/services/immersion-tracker/time.test.ts src/renderer/modals/playlist-browser.test.ts`, `bun run typecheck`.
|
||||
|
||||
2026-03-30 CodeRabbit follow-up: wrapped the injected-globals cleanup regression in try/finally so restore always runs, and changed the keydown test append mock to return createMutationSnapshot() before exercising Ctrl+ArrowDown. Verified with `bun test src/renderer/modals/playlist-browser.test.ts` and `bun run typecheck`.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
---
|
||||
id: TASK-260
|
||||
title: >-
|
||||
Fix macOS overlay subtitle sidebar passthrough without requiring a subtitle
|
||||
hover cycle
|
||||
status: Done
|
||||
assignee:
|
||||
- '@codex'
|
||||
created_date: '2026-03-31 00:58'
|
||||
updated_date: '2026-03-31 01:01'
|
||||
labels:
|
||||
- bug
|
||||
- macos
|
||||
- overlay
|
||||
- subtitle-sidebar
|
||||
- passthrough
|
||||
dependencies: []
|
||||
references:
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/renderer/modals/subtitle-sidebar.ts
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/renderer/overlay-mouse-ignore.ts
|
||||
- /Users/sudacode/projects/japanese/SubMiner/src/renderer/handlers/mouse.ts
|
||||
- /Users/sudacode/projects/japanese/SubMiner/src/main/overlay-runtime.ts
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/core/services/overlay-visibility.ts
|
||||
documentation:
|
||||
- docs/workflow/verification.md
|
||||
priority: high
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
On macOS, opening the overlay-layout subtitle sidebar should allow click-through outside the sidebar immediately. Users should not need to first hover subtitle content before passthrough/click-through starts working, including when no subtitle line is currently visible.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- [x] #1 With the overlay-layout subtitle sidebar open on macOS, areas outside the sidebar pass clicks through immediately after open without requiring a prior subtitle hover.
|
||||
- [x] #2 When no subtitle line is currently visible, opening the subtitle sidebar still leaves non-sidebar overlay regions click-through on macOS.
|
||||
- [x] #3 Regression coverage exercises the first-open/idle passthrough path so overlay interactivity does not depend on a later hover cycle.
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
1. Add/adjust focused overlay visibility regressions for the tracked macOS visible overlay so the default idle state stays click-through instead of forcing mouse interaction.
|
||||
2. Update main-process visible overlay visibility sync to keep the tracked macOS overlay passive by default and let renderer hover/sidebar state opt into interaction.
|
||||
3. Run focused verification for overlay visibility and any dependent runtime tests, then update task notes/criteria/final summary with the confirmed outcome.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
Investigation points to a main-process override on macOS: renderer sidebar open path already requests mouse passthrough outside the panel, but visible-overlay visibility sync still hard-sets the tracked overlay window interactive on macOS (`mouse-ignore:false`). Window-tracker focus/visibility resync can therefore undo renderer passthrough until a later hover cycle re-applies it.
|
||||
|
||||
Added a failing regression in `src/core/services/overlay-visibility.test.ts` showing the tracked macOS visible overlay was still forced interactive by main-process visibility sync (`mouse-ignore:false`) instead of staying forwarded click-through.
|
||||
|
||||
Updated `src/core/services/overlay-visibility.ts` so tracked macOS visible overlays now default to `setIgnoreMouseEvents(true, { forward: true })`, matching the renderer-side passthrough model and preventing window-tracker/focus resync from undoing idle sidebar clickthrough.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
Fixed the macOS subtitle-sidebar passthrough regression by changing tracked visible-overlay startup/visibility sync to stay click-through by default in the main process. Previously `updateVisibleOverlayVisibility` forced the macOS overlay window interactive, which could override renderer sidebar passthrough until a later hover cycle repaired it. Added a regression in `src/core/services/overlay-visibility.test.ts` and verified with `bun test src/core/services/overlay-visibility.test.ts`, `bun test src/renderer/modals/subtitle-sidebar.test.ts src/renderer/handlers/mouse.test.ts`, and `bun run typecheck`.
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
@@ -0,0 +1,29 @@
|
||||
---
|
||||
id: TASK-261
|
||||
title: Fix immersion tracker SQLite timestamp truncation
|
||||
status: In Progress
|
||||
assignee: []
|
||||
created_date: '2026-03-31 01:45'
|
||||
labels:
|
||||
- immersion-tracker
|
||||
- sqlite
|
||||
- bug
|
||||
dependencies: []
|
||||
references:
|
||||
- src/core/services/immersion-tracker
|
||||
priority: medium
|
||||
ordinal: 1200
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
Current-epoch millisecond values are being truncated by the libsql driver when bound as numeric parameters, which corrupts session, telemetry, lifetime, and rollup timestamps.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- [ ] #1 Current-epoch millisecond timestamps persist correctly in session, telemetry, lifetime, and rollup tables
|
||||
- [ ] #2 Startup backfill and destroy/finalize flows keep retained sessions and lifetime summaries consistent
|
||||
- [ ] #3 Regression tests cover the destroyed-session, startup backfill, and distinct-day/distinct-video lifetime semantics
|
||||
<!-- AC:END -->
|
||||
@@ -0,0 +1,50 @@
|
||||
---
|
||||
id: TASK-262
|
||||
title: Fix duplicate AniList post-watch updates for watched episodes
|
||||
status: Done
|
||||
assignee:
|
||||
- codex
|
||||
created_date: '2026-03-31 19:03'
|
||||
updated_date: '2026-03-31 19:05'
|
||||
labels:
|
||||
- bug
|
||||
- anilist
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
Watching an episode can currently produce two AniList activity updates for the same episode. The duplicate happens when the post-watch flow drains a queued retry for the current episode and then proceeds to run the live post-watch update for that same media/episode in the same pass. User report says this reproduces both when crossing the watched threshold naturally and when using the mark-watched keybinding. Fix the duplicate so one successful watch produces at most one AniList progress update for a given mediaKey/episode pair.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- [x] #1 A watched episode triggers at most one AniList post-watch progress update for a given media key and episode during a single post-watch pass, even if that episode already exists in the retry queue.
|
||||
- [x] #2 Both watched-threshold and manual mark-watched flows are protected by regression coverage for the duplicate-update case.
|
||||
- [x] #3 Relevant user-visible change note is added if required by repo policy.
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
1. Reproduce the duplicate in a unit test around `createMaybeRunAnilistPostWatchUpdateHandler` by simulating a ready retry for the same `mediaKey::episode` the live path would also submit.
|
||||
2. Fix the handler so that after processing a queued retry, it does not perform a second live update when the retry already satisfied the current attempt key.
|
||||
3. Run focused AniList runtime tests and adjacent immersion tests to confirm both threshold-driven and manual mark-watched entry points stay covered through the shared post-watch path.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
Added a regression in `src/main/runtime/anilist-post-watch.test.ts` for the case where `processNextAnilistRetryUpdate()` already satisfies the current `mediaKey::episode` before the live path runs.
|
||||
|
||||
Updated `createMaybeRunAnilistPostWatchUpdateHandler` to re-check `hasAttemptedUpdateKey(attemptKey)` immediately after draining the retry queue and short-circuit before a second live AniList submission.
|
||||
|
||||
Verification: `bun test src/main/runtime/anilist-post-watch.test.ts src/main/runtime/anilist-post-watch-main-deps.test.ts`; `bun test src/core/services/immersion-tracker-service.test.ts --test-name-pattern 'recordPlaybackPosition marks watched at 85% completion|markActiveVideoWatched'`; `bun run typecheck`; `bun run changelog:lint`.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
Fixed duplicate AniList post-watch submissions by short-circuiting the live update path when a ready retry item already handled the current `mediaKey::episode` in the same pass. Added a focused regression test for the retry-plus-live duplicate scenario and a changelog fragment documenting the fix.
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
Reference in New Issue
Block a user