sudacode/SubMiner
1
0
Fork 0
mirror of https://github.com/ksyasuda/SubMiner.git synced 2026-02-27 18:22:41 -08:00
Code Issues Packages Projects Releases Wiki Activity

Fix renderer overlay loading and modularize renderer

Browse Source
This commit is contained in:
Kyle Yasuda
2026-02-11 18:27:29 -08:00
parent ee21c77fd0
commit 8a82a1b5f9
29 changed files with 3150 additions and 2741 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
backlog/tasks/task-13 - Fix-second-instance-start-when-texthooker-only-instance-is-running.md Normal file
View File

@@ -0,0 +1,51 @@
---
id: TASK-13
title: Fix second-instance --start when texthooker-only instance is running
status: Done
assignee: []
created_date: '2026-02-11 23:47'
updated_date: '2026-02-11 23:47'
labels:
- bugfix
- cli
- overlay
dependencies: []
priority: high
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
When SubMiner is already running in texthooker-only mode, a subsequent `--start` command from a second instance is currently ignored. This can leave users without an initialized overlay runtime even though startup commands were issued. Adjust CLI command handling so `--start` on second-instance initializes overlay runtime when it is not yet initialized, while preserving current ignore behavior when overlay runtime is already active.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 Second-instance `--start` initializes overlay runtime when current instance has deferred/not-initialized overlay runtime.
- [x] #2 Second-instance `--start` remains ignored (existing behavior) when overlay runtime is already initialized.
- [x] #3 CLI command service tests cover both behaviors and pass.
<!-- AC:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
Patched CLI second-instance `--start` handling in `src/core/services/cli-command-service.ts` to initialize overlay runtime when deferred.
Added regression test for deferred-runtime start path and updated initialized-runtime second-instance tests in `src/core/services/cli-command-service.test.ts`.
<!-- SECTION:NOTES:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Fixed overlay startup regression path where a second-instance `--start` could be ignored even when the primary instance was running in texthooker-only/deferred overlay mode.
Changes:
- Updated `handleCliCommandService` logic so `ignoreStart` applies only when source is second-instance, `--start` is present, and overlay runtime is already initialized.
- Added explicit overlay-runtime initialization path for second-instance `--start` when runtime is not initialized.
- Kept existing behavior for already-initialized runtime (still logs and ignores redundant `--start`).
- Added and updated tests in `cli-command-service.test.ts` to validate both deferred and initialized second-instance startup behaviors.
Validation:
- `pnpm run build` succeeded.
- `node dist/core/services/cli-command-service.test.js` passed (11/11).
<!-- SECTION:FINAL_SUMMARY:END -->

43
backlog/tasks/task-14 - Ensure-subminer-launcher-shows-visible-overlay-on-startup.md Normal file
View File

@@ -0,0 +1,43 @@
---
id: TASK-14
title: Ensure subminer launcher shows visible overlay on startup
status: Done
assignee: []
created_date: '2026-02-12 00:22'
updated_date: '2026-02-12 00:23'
labels:
- bugfix
- launcher
- overlay
dependencies: []
priority: high
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
The `subminer` launcher starts SubMiner with `--start` but can leave the visible overlay hidden when runtime config defers auto-show (`auto_start_overlay=false`). Update launcher command args to explicitly request visible overlay at startup so script-mode behavior matches user expectations.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 Running `subminer <video>` starts SubMiner with startup args that include visible-overlay show intent.
- [x] #2 Launcher startup remains compatible with texthooker-enabled startup and backend/socket args.
- [x] #3 No regressions in existing startup argument construction for texthooker-only mode.
<!-- AC:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Updated `subminer` launcher startup args in `startOverlay()` to include `--show-visible-overlay` alongside `--start`.
This makes script-mode startup idempotently request visible overlay presentation instead of depending on runtime config auto-start visibility flags, while preserving existing backend/socket and optional texthooker args.
Scope:
- `subminer` script only.
- No changes to AppImage internal CLI parsing or runtime services.
Validation:
- Verified argument block in `startOverlay()` now includes `--show-visible-overlay` and preserves existing flags.
- Confirmed texthooker-only path (`launchTexthookerOnly`) is unchanged.
<!-- SECTION:FINAL_SUMMARY:END -->

34
backlog/tasks/task-15 - Fix-renderer-module-loading-regression-after-task-6-split.md Normal file
View File

@@ -0,0 +1,34 @@
---
id: TASK-15
title: Fix renderer module loading regression after task 6 split
status: Done
assignee: []
created_date: '2026-02-12 00:45'
updated_date: '2026-02-12 00:46'
labels:
- regression
- overlay
- renderer
dependencies: []
priority: high
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Overlay renderer stopped initializing after renderer.ts was split into modules. The emitted JS now uses CommonJS require/exports in a browser context (nodeIntegration disabled), causing script load failure and a blank transparent overlay with missing subtitle interactions.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 Renderer script loads successfully in overlay BrowserWindow without nodeIntegration.
- [x] #2 Visible overlay displays subtitles again on initial launch.
- [x] #3 Overlay keyboard/mouse interactions are functional again.
- [x] #4 Build output remains compatible with Electron main/preload while renderer runs as browser modules.
<!-- AC:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Fixed a renderer module-loading regression introduced by renderer modularization. Added a dedicated renderer TypeScript build target (`tsconfig.renderer.json`) that emits browser-compatible ES modules, updated build script to compile renderer with that config, switched overlay HTML to load `renderer.js` as a module, and updated renderer runtime imports to `.js` module specifiers. Verified that built renderer output no longer contains CommonJS `require(...)` and that core test suite passes (`pnpm run test:core`).
<!-- SECTION:FINAL_SUMMARY:END -->

34
backlog/tasks/task-16 - Revert-overlay-startup-experiment-changes-and-keep-renderer-fix.md Normal file
View File

@@ -0,0 +1,34 @@
---
id: TASK-16
title: Revert overlay startup experiment changes and keep renderer fix
status: Done
assignee: []
created_date: '2026-02-12 01:45'
updated_date: '2026-02-12 01:46'
labels:
- regression
- overlay
- launcher
dependencies: []
priority: high
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
User confirmed renderer module-loading fix resolved the broken overlay, but startup experiment changes introduced side effects (e.g., y-s start path re-launch behavior). Revert non-essential auto-start/debugging changes in launcher/plugin/CLI startup flow while preserving renderer ESM fix.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 Remove wrapper/plugin auto-start experiment changes that were added during debugging.
- [x] #2 Restore previous y-s start behavior without relaunching a new overlay session from wrapper-managed startup side effects.
- [x] #3 Keep renderer ESM/module-loading fix intact.
- [x] #4 Build and core tests pass after reversion.
<!-- AC:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Reverted startup experiment changes while preserving the renderer ESM fix. Removed wrapper-forced visible overlay startup and wrapper-managed mpv script opts from `subminer`, restored plugin defaults/behavior (`auto_start=true`) and removed `wrapper_managed` handling from `plugin/subminer.lua` + `plugin/subminer.conf`, and reverted CLI/bootstrap debug-path changes in `src/core/services/cli-command-service.ts` and `src/core/services/startup-service.ts` with matching test updates. Verified `pnpm run build` and full `pnpm run test:core` pass.
<!-- SECTION:FINAL_SUMMARY:END -->

76
backlog/tasks/task-5 - Eliminate-type-duplication-between-renderer.ts-and-types.ts.md
View File

@@ -1,9 +1,11 @@
---
id: TASK-5
title: Eliminate type duplication between renderer.ts and types.ts
status: To Do
assignee: []
status: Done
assignee:
- codex
created_date: '2026-02-11 08:20'
updated_date: '2026-02-11 17:46'
labels:
- refactor
- types
@@ -29,9 +31,69 @@ Additionally, `DEFAULT_MPV_SUBTITLE_RENDER_METRICS` and `sanitizeMpvSubtitleRend
## Acceptance Criteria
<!-- AC:BEGIN -->
- [ ] #1 All shared types are imported from types.ts — no local redefinitions in renderer.ts
- [ ] #2 DEFAULT_MPV_SUBTITLE_RENDER_METRICS lives in one canonical location (types.ts or a shared module)
- [ ] #3 sanitizeMpvSubtitleRenderMetrics moved to a shared module importable by both main and renderer
- [ ] #4 TypeScript compiles cleanly with no type errors
- [ ] #5 Renderer-only types (ChordAction, KikuModalStep, KikuPreviewMode) can stay local
- [x] #1 All shared types are imported from types.ts — no local redefinitions in renderer.ts
- [x] #2 DEFAULT_MPV_SUBTITLE_RENDER_METRICS lives in one canonical location (types.ts or a shared module)
- [x] #3 sanitizeMpvSubtitleRenderMetrics moved to a shared module importable by both main and renderer
- [x] #4 TypeScript compiles cleanly with no type errors
- [x] #5 Renderer-only types (ChordAction, KikuModalStep, KikuPreviewMode) can stay local
<!-- AC:END -->
## Implementation Plan
<!-- SECTION:PLAN:BEGIN -->
1) Audit `src/renderer/renderer.ts`, `src/types.ts`, and `src/main.ts` to identify every duplicated type and both subtitle-metrics helpers/constants.
2) Remove duplicated shared type declarations from `renderer.ts` and replace them with direct imports from `types.ts`; keep renderer-only types (`ChordAction`, `KikuModalStep`, `KikuPreviewMode`) local.
3) Create a single canonical home for `DEFAULT_MPV_SUBTITLE_RENDER_METRICS` in a shared location and update all call sites to import it from that canonical module.
4) Move `sanitizeMpvSubtitleRenderMetrics` to a shared module importable by both main and renderer, then switch both files to consume that shared implementation.
5) Run TypeScript compile/check, fix any fallout, and verify no local shared-type redefinitions remain in `renderer.ts`.
6) Update task notes and check off acceptance criteria as each item is validated.
<!-- SECTION:PLAN:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
Replaced renderer-local shared type declarations with `import type` from `src/types.ts`; only `KikuModalStep`, `KikuPreviewMode`, and `ChordAction` remain local in renderer.
Moved canonical MPV subtitle metrics defaults to `src/core/services/mpv-render-metrics-service.ts` as `DEFAULT_MPV_SUBTITLE_RENDER_METRICS` and switched `main.ts` to consume it.
Added shared `sanitizeMpvSubtitleRenderMetrics` export in `src/core/services/mpv-render-metrics-service.ts` and re-exported it from `src/core/services/index.ts`.
Removed renderer-local `DEFAULT_MPV_SUBTITLE_RENDER_METRICS`, `coerceFiniteNumber`, and `sanitizeMpvSubtitleRenderMetrics`; renderer now consumes full sanitized metrics from main via IPC and keeps nullable local state until startup metrics are loaded.
Validation: `pnpm run build` passed; `node --test dist/core/services/mpv-render-metrics-service.test.js` passed.
<!-- SECTION:NOTES:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Implemented TASK-5 by eliminating duplicate shared type/model definitions from the renderer and centralizing MPV subtitle render metrics primitives.
What changed:
- `src/renderer/renderer.ts`
- Removed local redefinitions of shared interfaces/types (subtitle, keybinding, Jimaku, Kiku, runtime options, subsync, render metrics).
- Added `import type` usage from `src/types.ts` for shared contracts.
- Kept renderer-only local types (`KikuModalStep`, `KikuPreviewMode`, `ChordAction`).
- Removed renderer-local metrics default/sanitization helpers and switched invisible overlay resize behavior to rely on already-synced metrics state from main.
- `src/core/services/mpv-render-metrics-service.ts`
- Added canonical `DEFAULT_MPV_SUBTITLE_RENDER_METRICS` export.
- Added shared `sanitizeMpvSubtitleRenderMetrics` export.
- `src/core/services/index.ts`
- Re-exported `DEFAULT_MPV_SUBTITLE_RENDER_METRICS` and `sanitizeMpvSubtitleRenderMetrics`.
- `src/main.ts`
- Removed local default metrics constant and imported the canonical default from core services.
- `src/core/services/mpv-render-metrics-service.test.ts`
- Updated base fixture to derive from canonical default metrics constant.
Why:
- Prevent type drift between renderer and shared contracts.
- Establish a single source of truth for MPV subtitle render metric defaults and sanitization utilities.
Validation:
- `pnpm run build`
- `node --test dist/core/services/mpv-render-metrics-service.test.js`
Result:
- Shared renderer types now come from `types.ts`.
- MPV subtitle render metrics defaults are canonicalized in one shared module.
- TypeScript compiles cleanly and relevant metrics service tests pass.
<!-- SECTION:FINAL_SUMMARY:END -->

79
backlog/tasks/task-6 - Split-renderer.ts-into-focused-modules.md
View File

@@ -1,9 +1,11 @@
---
id: TASK-6
title: Split renderer.ts into focused modules
status: To Do
assignee: []
status: Done
assignee:
- codex
created_date: '2026-02-11 08:20'
updated_date: '2026-02-11 20:45'
labels:
- refactor
- renderer
@@ -46,10 +48,71 @@ Note: The renderer runs in Electron's renderer process, so module bundling consi
## Acceptance Criteria
<!-- AC:BEGIN -->
- [ ] #1 renderer.ts reduced to <400 lines (init + IPC wiring)
- [ ] #2 Each modal UI in its own module
- [ ] #3 Positioning logic extracted with helper functions replacing the 211-line mega function
- [ ] #4 State centralized in a single object/module
- [ ] #5 Platform-specific logic isolated behind abstractions
- [ ] #6 All existing functionality preserved
- [x] #1 renderer.ts reduced to <400 lines (init + IPC wiring)
- [x] #2 Each modal UI in its own module
- [x] #3 Positioning logic extracted with helper functions replacing the 211-line mega function
- [x] #4 State centralized in a single object/module
- [x] #5 Platform-specific logic isolated behind abstractions
- [x] #6 All existing functionality preserved
<!-- AC:END -->
## Implementation Plan
<!-- SECTION:PLAN:BEGIN -->
1. Create shared renderer infrastructure modules (`state.ts`, `platform.ts`, `dom.ts`) and a typed context for cross-module dependencies.
2. Extract subtitle render and secondary subtitle logic into `subtitle-render.ts` with behavior-preserving APIs.
3. Extract invisible/visible subtitle positioning and offset edit logic into `positioning.ts`, splitting the mega layout function into helper functions.
4. Extract each modal into separate modules: `modals/jimaku.ts`, `modals/kiku.ts`, `modals/runtime-options.ts`, `modals/subsync.ts`.
5. Extract input and UI interaction logic into `handlers/keyboard.ts` and `handlers/mouse.ts`.
6. Rewrite `renderer.ts` as entrypoint/orchestrator only (<400 lines), wire IPC listeners and module composition.
7. Run `pnpm run build` and targeted tests; update task notes and acceptance checklist to reflect completion status.
<!-- SECTION:PLAN:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
Reviewed src/renderer/renderer.ts structure and build wiring (tsc CommonJS output loaded by Electron). Confirmed renderer module splitting can be done safely without introducing a new bundler in this task.
Implemented renderer modularization with a centralized `RendererState` and shared context (`src/renderer/state.ts`, `src/renderer/context.ts`).
Extracted platform and DOM abstractions into `src/renderer/utils/platform.ts` and `src/renderer/utils/dom.ts`.
Extracted subtitle render/style concerns to `src/renderer/subtitle-render.ts` and positioning/layout concerns to `src/renderer/positioning.ts`, including helperized invisible subtitle layout pipeline.
Split modal UIs into dedicated modules: `src/renderer/modals/jimaku.ts`, `src/renderer/modals/kiku.ts`, `src/renderer/modals/runtime-options.ts`, `src/renderer/modals/subsync.ts`.
Split interaction logic into `src/renderer/handlers/keyboard.ts` and `src/renderer/handlers/mouse.ts`.
Reduced `src/renderer/renderer.ts` to entrypoint/orchestration (225 lines) with IPC wiring and module composition only.
Validation: `pnpm run build` passed; `pnpm run test:core` passed (21/21).
<!-- SECTION:NOTES:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Refactored the Electron renderer implementation from a monolithic file into focused modules while preserving runtime behavior and IPC integration.
What changed:
- Replaced ad hoc renderer globals with a centralized mutable state container in `src/renderer/state.ts`, wired through a shared renderer context (`src/renderer/context.ts`).
- Isolated platform/environment detection and DOM element resolution into `src/renderer/utils/platform.ts` and `src/renderer/utils/dom.ts`.
- Extracted subtitle rendering and subtitle style/secondary subtitle behavior into `src/renderer/subtitle-render.ts`.
- Extracted subtitle positioning logic into `src/renderer/positioning.ts`, including breaking invisible subtitle layout into helper functions for scale, container layout, vertical alignment, and typography application.
- Split each modal into its own module:
- `src/renderer/modals/jimaku.ts`
- `src/renderer/modals/kiku.ts`
- `src/renderer/modals/runtime-options.ts`
- `src/renderer/modals/subsync.ts`
- Split user interaction concerns into handler modules:
- `src/renderer/handlers/keyboard.ts`
- `src/renderer/handlers/mouse.ts`
- Rewrote `src/renderer/renderer.ts` to an initialization/orchestration entrypoint (225 lines), retaining IPC listeners and module composition only.
Why:
- Addressed architectural and maintainability issues in a large mixed-concern renderer file by enforcing concern boundaries and explicit dependencies.
- Improved testability and future change safety by reducing hidden cross-function/module state coupling.
Validation:
- `pnpm run build` succeeded.
- `pnpm run test:core` succeeded (21 passing tests).
<!-- SECTION:FINAL_SUMMARY:END -->