From bba1bd554e07c12d0c2fdb170e5b6e8bf2f0c95f Mon Sep 17 00:00:00 2001 From: sudacode Date: Wed, 11 Feb 2026 00:39:43 -0800 Subject: [PATCH] update backlog --- .../m-0 - codebase-clarity-&-composability.md | 8 +++ ...e-naming-conventions-and-barrel-exports.md | 40 ++++++++++++++ ...titleLayoutFromMpvMetrics-mega-function.md | 38 +++++++++++++ ...undling-for-multi-file-renderer-support.md | 41 ++++++++++++++ ...ermaid-diagrams-in-docs-for-readability.md | 30 ++++++++++ ...cation-between-renderer.ts-and-types.ts.md | 37 +++++++++++++ ... Split-renderer.ts-into-focused-modules.md | 55 +++++++++++++++++++ ...global-state-into-an-AppState-container.md | 35 ++++++++++++ ...eparate-protocol-from-application-logic.md | 38 +++++++++++++ ...-trivial-wrapper-functions-from-main.ts.md | 47 ++++++++++++++++ 10 files changed, 369 insertions(+) create mode 100644 backlog/milestones/m-0 - codebase-clarity-&-composability.md create mode 100644 backlog/tasks/task-10 - Consolidate-service-naming-conventions-and-barrel-exports.md create mode 100644 backlog/tasks/task-11 - Break-up-the-applyInvisibleSubtitleLayoutFromMpvMetrics-mega-function.md create mode 100644 backlog/tasks/task-12 - Add-renderer-module-bundling-for-multi-file-renderer-support.md create mode 100644 backlog/tasks/task-4 - Improve-Mermaid-diagrams-in-docs-for-readability.md create mode 100644 backlog/tasks/task-5 - Eliminate-type-duplication-between-renderer.ts-and-types.ts.md create mode 100644 backlog/tasks/task-6 - Split-renderer.ts-into-focused-modules.md create mode 100644 backlog/tasks/task-7 - Extract-main.ts-global-state-into-an-AppState-container.md create mode 100644 backlog/tasks/task-8 - Reduce-MpvIpcClient-deps-interface-and-separate-protocol-from-application-logic.md create mode 100644 backlog/tasks/task-9 - Remove-trivial-wrapper-functions-from-main.ts.md diff --git a/backlog/milestones/m-0 - codebase-clarity-&-composability.md b/backlog/milestones/m-0 - codebase-clarity-&-composability.md new file mode 100644 index 0000000..bfb2d13 --- /dev/null +++ b/backlog/milestones/m-0 - codebase-clarity-&-composability.md @@ -0,0 +1,8 @@ +--- +id: m-0 +title: "Codebase Clarity & Composability" +--- + +## Description + +Improvements to code clarity, simplicity, and composability identified during the Feb 2026 codebase review. Focus on reducing monolithic files, eliminating duplication, and improving architectural boundaries. diff --git a/backlog/tasks/task-10 - Consolidate-service-naming-conventions-and-barrel-exports.md b/backlog/tasks/task-10 - Consolidate-service-naming-conventions-and-barrel-exports.md new file mode 100644 index 0000000..aa2b7e6 --- /dev/null +++ b/backlog/tasks/task-10 - Consolidate-service-naming-conventions-and-barrel-exports.md @@ -0,0 +1,40 @@ +--- +id: TASK-10 +title: Consolidate service naming conventions and barrel exports +status: To Do +assignee: [] +created_date: '2026-02-11 08:21' +labels: + - refactor + - services + - naming +milestone: Codebase Clarity & Composability +dependencies: [] +references: + - src/core/services/index.ts +priority: low +--- + +## Description + + +The service layer has inconsistent naming: +- Some functions end in `Service`: `handleCliCommandService`, `loadSubtitlePositionService` +- Some end in `RuntimeService`: `replayCurrentSubtitleRuntimeService`, `sendMpvCommandRuntimeService` +- Some are plain: `shortcutMatchesInputForLocalFallback` +- Factory functions mix `create*DepsRuntimeService` with `create*Service` + +The barrel export (src/core/services/index.ts) re-exports 79 symbols from 28 files through a single surface, which obscures dependency boundaries. Consumers import everything from `./core/services` and can't tell which service file they actually depend on. + +Establish consistent naming: +- Exported service functions: `verbNounService` (e.g., `handleCliCommand`) +- Deps factory functions: `create*Deps` +- Consider whether the barrel re-export is still the right pattern vs direct imports from individual files. + + +## Acceptance Criteria + +- [ ] #1 All service functions follow a consistent naming convention +- [ ] #2 Decision documented on barrel export vs direct imports +- [ ] #3 No functional changes + diff --git a/backlog/tasks/task-11 - Break-up-the-applyInvisibleSubtitleLayoutFromMpvMetrics-mega-function.md b/backlog/tasks/task-11 - Break-up-the-applyInvisibleSubtitleLayoutFromMpvMetrics-mega-function.md new file mode 100644 index 0000000..9979863 --- /dev/null +++ b/backlog/tasks/task-11 - Break-up-the-applyInvisibleSubtitleLayoutFromMpvMetrics-mega-function.md @@ -0,0 +1,38 @@ +--- +id: TASK-11 +title: Break up the applyInvisibleSubtitleLayoutFromMpvMetrics mega function +status: To Do +assignee: [] +created_date: '2026-02-11 08:21' +labels: + - refactor + - renderer + - complexity +milestone: Codebase Clarity & Composability +dependencies: [] +references: + - src/renderer/renderer.ts +priority: medium +--- + +## Description + + +In renderer.ts (around lines 865-1075), `applyInvisibleSubtitleLayoutFromMpvMetrics` is a 211-line function with up to 5 levels of nesting. It handles OSD scaling calculations, platform-specific font compensation (macOS vs Linux), DPR calculations, ASS alignment tag interpretation (\an tags), baseline compensation, line-height fixes, font property application, and transform origin — all interleaved. + +Extract into focused helpers: +- `calculateOsdScale(metrics, renderAreaHeight)` — pure scaling math +- `calculateSubtitlePosition(metrics, scale, alignment)` — ASS \an tag interpretation + positioning +- `applyPlatformFontCompensation(style, platform)` — macOS kerning/size adjustments +- `applySubtitleStyle(element, computedStyle)` — DOM style application + +This can be done independently of or as part of TASK-6 (renderer split). + + +## Acceptance Criteria + +- [ ] #1 No single function exceeds ~50 lines in the positioning logic +- [ ] #2 Helper functions are pure where possible (take inputs, return outputs) +- [ ] #3 Platform-specific branches isolated into dedicated helpers +- [ ] #4 Invisible overlay positioning still works correctly on Linux and macOS + diff --git a/backlog/tasks/task-12 - Add-renderer-module-bundling-for-multi-file-renderer-support.md b/backlog/tasks/task-12 - Add-renderer-module-bundling-for-multi-file-renderer-support.md new file mode 100644 index 0000000..c80cd53 --- /dev/null +++ b/backlog/tasks/task-12 - Add-renderer-module-bundling-for-multi-file-renderer-support.md @@ -0,0 +1,41 @@ +--- +id: TASK-12 +title: Add renderer module bundling for multi-file renderer support +status: To Do +assignee: [] +created_date: '2026-02-11 08:21' +labels: + - infrastructure + - renderer + - build +milestone: Codebase Clarity & Composability +dependencies: + - TASK-5 +references: + - src/renderer/renderer.ts + - src/renderer/index.html + - package.json + - tsconfig.json +priority: medium +--- + +## Description + + +Currently renderer.ts is a single file loaded directly by Electron's renderer process via a script tag in index.html. To split it into modules (TASK-6), we need a bundling step since Electron renderer's default context doesn't support bare ES module imports without additional configuration. + +Options: +1. **esbuild** — fast, minimal config, already used in many Electron projects +2. **Electron's native ESM support** — requires `"type": "module"` and sandbox configuration +3. **TypeScript compiler output** — if targeting a single concatenated bundle + +The build pipeline already compiles TypeScript and copies renderer assets. Adding a bundling step for the renderer would slot into the existing `npm run build` script. + + +## Acceptance Criteria + +- [ ] #1 Renderer code can be split across multiple .ts files with imports +- [ ] #2 Build pipeline bundles renderer modules into a single output for Electron +- [ ] #3 Existing `make build` still works end-to-end +- [ ] #4 No runtime errors in renderer process + diff --git a/backlog/tasks/task-4 - Improve-Mermaid-diagrams-in-docs-for-readability.md b/backlog/tasks/task-4 - Improve-Mermaid-diagrams-in-docs-for-readability.md new file mode 100644 index 0000000..ffdfc43 --- /dev/null +++ b/backlog/tasks/task-4 - Improve-Mermaid-diagrams-in-docs-for-readability.md @@ -0,0 +1,30 @@ +--- +id: TASK-4 +title: Improve Mermaid diagrams in docs for readability +status: Done +assignee: [] +created_date: '2026-02-11 07:11' +updated_date: '2026-02-11 07:11' +labels: [] +dependencies: [] +priority: medium +--- + +## Description + + +Refine Mermaid charts in documentation (primarily architecture docs) to improve readability, grouping, and label clarity without changing system behavior. + + +## Acceptance Criteria + +- [x] #1 Mermaid diagrams render successfully in VitePress docs build +- [x] #2 Diagrams have clearer grouping, edge labels, and flow direction +- [x] #3 No broken markdown or Mermaid syntax in updated docs + + +## Final Summary + + +Improved Mermaid diagrams in docs/architecture.md by redesigning both flowcharts with clearer subgraphs, labeled edges, and consistent lifecycle/runtime separation. Verified successful rendering via `pnpm run docs:build` with no chunk-size warning regressions. + diff --git a/backlog/tasks/task-5 - Eliminate-type-duplication-between-renderer.ts-and-types.ts.md b/backlog/tasks/task-5 - Eliminate-type-duplication-between-renderer.ts-and-types.ts.md new file mode 100644 index 0000000..e5db9cc --- /dev/null +++ b/backlog/tasks/task-5 - Eliminate-type-duplication-between-renderer.ts-and-types.ts.md @@ -0,0 +1,37 @@ +--- +id: TASK-5 +title: Eliminate type duplication between renderer.ts and types.ts +status: To Do +assignee: [] +created_date: '2026-02-11 08:20' +labels: + - refactor + - types + - renderer +milestone: Codebase Clarity & Composability +dependencies: [] +references: + - src/renderer/renderer.ts + - src/types.ts + - src/main.ts +priority: high +--- + +## Description + + +renderer.ts locally redefines 20+ interfaces/types that already exist in types.ts: MergedToken, SubtitleData, MpvSubtitleRenderMetrics, Keybinding, SubtitlePosition, SecondarySubMode, all Jimaku/Kiku types, RuntimeOption types, SubsyncSourceTrack, SubsyncManualPayload, etc. + +This creates divergence risk — changes in types.ts don't automatically propagate to the renderer's local copies. + +Additionally, `DEFAULT_MPV_SUBTITLE_RENDER_METRICS` and `sanitizeMpvSubtitleRenderMetrics()` exist only in renderer.ts despite being shared concerns (main.ts also has DEFAULT_MPV_SUBTITLE_RENDER_METRICS). + + +## Acceptance Criteria + +- [ ] #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 + diff --git a/backlog/tasks/task-6 - Split-renderer.ts-into-focused-modules.md b/backlog/tasks/task-6 - Split-renderer.ts-into-focused-modules.md new file mode 100644 index 0000000..da0b56c --- /dev/null +++ b/backlog/tasks/task-6 - Split-renderer.ts-into-focused-modules.md @@ -0,0 +1,55 @@ +--- +id: TASK-6 +title: Split renderer.ts into focused modules +status: To Do +assignee: [] +created_date: '2026-02-11 08:20' +labels: + - refactor + - renderer + - architecture +milestone: Codebase Clarity & Composability +dependencies: + - TASK-5 +references: + - src/renderer/renderer.ts +priority: high +--- + +## Description + + +renderer.ts is 2,754 lines with 94 functions handling 6+ distinct concerns: subtitle rendering, invisible overlay positioning, 4 modal UIs (Jimaku, Kiku, RuntimeOptions, Subsync), event handlers, keyboard chord system, and platform-specific layout. 16+ module-level state variables track overlapping modal states. + +Proposed structure: +``` +src/renderer/ +├── renderer.ts (entry point, initialization, IPC listeners) +├── state.ts (centralized state object replacing 16+ scattered lets) +├── subtitle-render.ts (renderSubtitle, renderTokenized, renderCharLevel, renderPlain) +├── positioning.ts (applyInvisibleSubtitleLayoutFromMpvMetrics + helpers) +├── modals/ +│ ├── jimaku.ts (Jimaku download modal - lines 1097-1518) +│ ├── kiku.ts (Kiku field grouping modal - lines 1519-1702) +│ ├── runtime-options.ts (Runtime options modal - lines 1247-1364) +│ └── subsync.ts (Subsync modal - lines 1387-1466) +├── handlers/ +│ ├── keyboard.ts (keydown handlers, chord system) +│ └── mouse.ts (drag, hover, click handlers) +└── utils/ + ├── dom.ts (DOM element access with validation) + └── platform.ts (isLinux/isMacOS detection, platform-specific helpers) +``` + +Note: The renderer runs in Electron's renderer process, so module bundling considerations (esbuild/webpack or Electron's native ESM) need to be evaluated. + + +## Acceptance Criteria + +- [ ] #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 + diff --git a/backlog/tasks/task-7 - Extract-main.ts-global-state-into-an-AppState-container.md b/backlog/tasks/task-7 - Extract-main.ts-global-state-into-an-AppState-container.md new file mode 100644 index 0000000..a3b544b --- /dev/null +++ b/backlog/tasks/task-7 - Extract-main.ts-global-state-into-an-AppState-container.md @@ -0,0 +1,35 @@ +--- +id: TASK-7 +title: Extract main.ts global state into an AppState container +status: To Do +assignee: [] +created_date: '2026-02-11 08:20' +labels: + - refactor + - main + - architecture +milestone: Codebase Clarity & Composability +dependencies: [] +references: + - src/main.ts +priority: high +--- + +## Description + + +main.ts has 30+ module-level `let` declarations holding all application state: mpvClient, yomitanExt, yomitanSettingsWindow, yomitanParserWindow, reconnectTimer, currentSubText, currentSubAssText, subtitlePosition, currentMediaPath, mecabTokenizer, keybindings, ankiIntegration, secondarySubMode, runtimeOptionsManager, overlayRuntimeInitialized, subsyncInProgress, shortcutsRegistered, etc. + +These are mutated through closures from many different functions, making state changes hard to trace and impossible to test. + +Consolidate into a typed AppState object (or small set of domain-specific state containers) that provides controlled access to state. This also enables passing state to services without building 50-property dependency objects. + + +## Acceptance Criteria + +- [ ] #1 All mutable state consolidated into typed container(s) +- [ ] #2 No bare `let` declarations at module scope for application state +- [ ] #3 State access goes through the container rather than closures +- [ ] #4 Dependency objects for services shrink significantly (reference the container instead) +- [ ] #5 TypeScript compiles cleanly + diff --git a/backlog/tasks/task-8 - Reduce-MpvIpcClient-deps-interface-and-separate-protocol-from-application-logic.md b/backlog/tasks/task-8 - Reduce-MpvIpcClient-deps-interface-and-separate-protocol-from-application-logic.md new file mode 100644 index 0000000..eb84644 --- /dev/null +++ b/backlog/tasks/task-8 - Reduce-MpvIpcClient-deps-interface-and-separate-protocol-from-application-logic.md @@ -0,0 +1,38 @@ +--- +id: TASK-8 +title: >- + Reduce MpvIpcClient deps interface and separate protocol from application + logic +status: To Do +assignee: [] +created_date: '2026-02-11 08:20' +labels: + - refactor + - mpv + - architecture +milestone: Codebase Clarity & Composability +dependencies: [] +references: + - src/core/services/mpv-service.ts +priority: medium +--- + +## Description + + +MpvIpcClient (761 lines) in src/core/services/mpv-service.ts has a 22-property `MpvIpcClientDeps` interface that reaches back into main.ts state for application-level concerns (overlay visibility, subtitle timing, media path updates, OSD display). + +The class mixes two responsibilities: +1. **IPC Protocol**: Socket connection, JSON message framing, reconnection, property observation +2. **Application Integration**: Subtitle text broadcasting, overlay visibility sync, timing tracking + +Separating these would let the protocol layer be simpler and testable, while application-level reactions to mpv events could be handled by listeners/callbacks registered externally. + + +## Acceptance Criteria + +- [ ] #1 MpvIpcClient deps interface reduced to protocol-level concerns only +- [ ] #2 Application-level reactions (subtitle broadcast, overlay sync, timing) handled via event emitter or external listeners +- [ ] #3 MpvIpcClient is testable without mocking 22 callbacks +- [ ] #4 Existing behavior preserved + diff --git a/backlog/tasks/task-9 - Remove-trivial-wrapper-functions-from-main.ts.md b/backlog/tasks/task-9 - Remove-trivial-wrapper-functions-from-main.ts.md new file mode 100644 index 0000000..868abdc --- /dev/null +++ b/backlog/tasks/task-9 - Remove-trivial-wrapper-functions-from-main.ts.md @@ -0,0 +1,47 @@ +--- +id: TASK-9 +title: Remove trivial wrapper functions from main.ts +status: To Do +assignee: [] +created_date: '2026-02-11 08:21' +labels: + - refactor + - main + - simplicity +milestone: Codebase Clarity & Composability +dependencies: + - TASK-7 +references: + - src/main.ts +priority: medium +--- + +## Description + + +main.ts contains many trivial single-line wrapper functions that add indirection without value: + +```typescript +function getOverlayWindows(): BrowserWindow[] { + return overlayManager.getOverlayWindows(); +} +function updateOverlayBounds(geometry: WindowGeometry): void { + updateOverlayBoundsService(geometry, () => getOverlayWindows()); +} +function ensureOverlayWindowLevel(window: BrowserWindow): void { + ensureOverlayWindowLevelService(window); +} +``` + +Similarly, config accessor wrappers like `getJimakuLanguagePreference()`, `getJimakuMaxEntryResults()`, `resolveJimakuApiKey()` are pure boilerplate. + +After TASK-7 (AppState container), many of these can be eliminated by having services access the state container directly, or by using the service functions directly at call sites without local wrappers. + + +## Acceptance Criteria + +- [ ] #1 Trivial pass-through wrappers eliminated (call service/manager directly) +- [ ] #2 Config accessor wrappers replaced with direct calls or a config accessor helper +- [ ] #3 main.ts line count reduced +- [ ] #4 No functional changes +