mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-02-28 06:22:45 -08:00
update backlog
This commit is contained in:
@@ -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.
|
||||||
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [ ] #1 All service functions follow a consistent naming convention
|
||||||
|
- [ ] #2 Decision documented on barrel export vs direct imports
|
||||||
|
- [ ] #3 No functional changes
|
||||||
|
<!-- AC:END -->
|
||||||
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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).
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [ ] #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
|
||||||
|
<!-- AC:END -->
|
||||||
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [ ] #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
|
||||||
|
<!-- AC:END -->
|
||||||
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
Refine Mermaid charts in documentation (primarily architecture docs) to improve readability, grouping, and label clarity without changing system behavior.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [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
|
||||||
|
<!-- AC:END -->
|
||||||
|
|
||||||
|
## Final Summary
|
||||||
|
|
||||||
|
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||||
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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).
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## 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
|
||||||
|
<!-- AC:END -->
|
||||||
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## 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
|
||||||
|
<!-- AC:END -->
|
||||||
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [ ] #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
|
||||||
|
<!-- AC:END -->
|
||||||
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [ ] #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
|
||||||
|
<!-- AC:END -->
|
||||||
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [ ] #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
|
||||||
|
<!-- AC:END -->
|
||||||
Reference in New Issue
Block a user