mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-02-27 18:22:41 -08:00
pretty
This commit is contained in:
@@ -19,7 +19,9 @@ 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`
|
||||
@@ -28,13 +30,16 @@ The service layer has inconsistent naming:
|
||||
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`)
|
||||
|
||||
- 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
|
||||
@@ -43,11 +48,15 @@ Establish consistent naming:
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
Naming convention consolidation should be addressed as part of TASK-27.2 (split main.ts) and TASK-27.3 (anki-integration split). As modules are extracted and given clear boundaries, naming will be standardized at each boundary. No need to do a standalone naming pass — it's wasted effort if the module structure is about to change.
|
||||
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
Subsumed by TASK-27.2 and TASK-27.3. Naming conventions were standardized at module boundaries during extraction. A standalone global naming pass would be churn with no structural benefit now that modules have clear ownership boundaries.
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
@@ -20,19 +20,24 @@ 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 -->
|
||||
|
||||
- [x] #1 No single function exceeds ~50 lines in the positioning logic
|
||||
- [x] #2 Helper functions are pure where possible (take inputs, return outputs)
|
||||
- [x] #3 Platform-specific branches isolated into dedicated helpers
|
||||
@@ -42,13 +47,17 @@ This can be done independently of or as part of TASK-6 (renderer split).
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
Helpers were split so positioning math, base layout, and typography/vertical handling are no longer in one monolith; see `src/renderer/positioning/invisible-layout.ts` and peer files.
|
||||
|
||||
Applied as part of TASK-27.5 with helper extraction: moved mpv subtitle layout orchestration to `invisible-layout.ts` and extracted metric/base/style helpers into `invisible-layout-metrics.ts` and `invisible-layout-helpers.ts`.
|
||||
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
Decomposition of `applyInvisibleSubtitleLayoutFromMpvMetrics` completed as part of TASK-27.5: function body split into metric/layout/typography helpers and small coordinator preserved. Manual validation completed by user; behavior remains stable.
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
@@ -23,18 +23,23 @@ priority: high
|
||||
## 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 -->
|
||||
|
||||
- [x] #1 Renderer code can be split across multiple .ts files with imports
|
||||
- [x] #2 Build pipeline bundles renderer modules into a single output for Electron
|
||||
- [x] #3 Existing `make build` still works end-to-end
|
||||
@@ -44,13 +49,17 @@ The build pipeline already compiles TypeScript and copies renderer assets. Addin
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
Updated root npm build pipeline to use an explicit renderer bundle step via esbuild. Added `build:renderer` script to emit a single `dist/renderer/renderer.js` from `src/renderer/renderer.ts`; `build` now runs `pnpm run build:renderer` and preserves existing index/style copy and macOS helper step. Added `esbuild` to devDependencies.
|
||||
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
Implemented renderer bundling step and wired `build` to use it. This adds `pnpm run build:renderer` which bundles `src/renderer/renderer.ts` into a single `dist/renderer/renderer.js` for Electron to load. Also added `esbuild` as a dev dependency and aligned `pnpm-lock.yaml` importer metadata for dependency consistency. Kept `index.html`/`style.css` copy path unchanged, so renderer asset layout remains stable.
|
||||
|
||||
Implemented additional test-layer type fix after build breakage by correcting `makeDepsFromMecabTokenizer` and related `tokenizeWithMecab` mocks to match expected `Token` vs `MergedToken` shapes, keeping runtime behavior unchanged while satisfying TS checks.
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
@@ -14,11 +14,15 @@ priority: medium
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
Refactor overlay runtime so each overlay layer owns and applies its bounds independently. Keep tracker geometry as shared origin input only.
|
||||
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
|
||||
- [x] #1 `updateOverlayBoundsService` no longer applies the same bounds to every overlay window by default.
|
||||
- [x] #2 Main runtime/manager exposes per-layer bounds update paths for visible and invisible overlays.
|
||||
- [x] #3 Window tracker updates feed shared origin data; each layer applies its own computed bounds.
|
||||
@@ -28,6 +32,7 @@ Refactor overlay runtime so each overlay layer owns and applies its bounds indep
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
Started implementation for per-layer overlay bounds ownership refactor.
|
||||
|
||||
Implemented per-layer bounds ownership path: visible and invisible layers now update bounds independently through overlay manager/runtime plumbing, while preserving existing geometry source behavior.
|
||||
@@ -35,10 +40,13 @@ Implemented per-layer bounds ownership path: visible and invisible layers now up
|
||||
Replaced shared all-window bounds application with per-window bound application service and layer-specific runtime calls from visibility/tracker flows.
|
||||
|
||||
Archiving requested by user.
|
||||
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
Refactored overlay bounds ownership to per-layer update paths. Tracker geometry remains shared input, but visible/invisible windows apply bounds independently via explicit layer routes. Existing single-layer UX behavior is preserved.
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
@@ -18,14 +18,18 @@ priority: high
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
Create a phased backlog-backed restructuring plan that keeps current service-oriented architecture while reducing cognitive load from oversized modules and tightening module ownership boundaries.
|
||||
|
||||
This initiative should make future feature work easier by splitting high-complexity files, reducing tightly-coupled orchestration, and introducing measurable structural guardrails.
|
||||
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
- [ ] #1 A phased decomposition plan is defined in task links and references the following target files: src/main.ts, src/anki-integration.ts, src/core/services/mpv-service.ts, src/renderer/*, src/config/*, and src/core/services/*.
|
||||
|
||||
- [ ] #1 A phased decomposition plan is defined in task links and references the following target files: src/main.ts, src/anki-integration.ts, src/core/services/mpv-service.ts, src/renderer/_, src/config/_, and src/core/services/\*.
|
||||
- [ ] #2 Tasks are assigned with clear owners and include explicit dependencies so execution can proceed in parallel where safe.
|
||||
- [ ] #3 Changes are constrained to structural refactors first (no behavior changes until foundational splits are in place).
|
||||
- [ ] #4 Each subtask includes test/verification expectations (manual or automated) and a rollback-safe checkpoint.
|
||||
@@ -34,27 +38,35 @@ This initiative should make future feature work easier by splitting high-complex
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
|
||||
## Revised Execution Sequence
|
||||
|
||||
### Phase 0 — Prerequisites (outside TASK-27 tree)
|
||||
|
||||
- **TASK-7** — Extract main.ts global state into AppState container (required before TASK-27.2)
|
||||
- **TASK-9** — Remove trivial wrapper functions from main.ts (depends on TASK-7; recommended before TASK-27.2 but not blocking)
|
||||
|
||||
### Phase 1 — Lightweight Inventory
|
||||
|
||||
- **TASK-27.1** — Inventory files >400 LOC, document contracts, define smoke test checklist
|
||||
|
||||
### Phase 2 — Sequential Split Wave
|
||||
|
||||
Order matters to avoid merge conflicts:
|
||||
|
||||
1. **TASK-27.3** — anki-integration.ts split (self-contained, doesn't affect main.ts wiring until facade is stable)
|
||||
2. **TASK-27.2** — main.ts split (after TASK-7 provides AppState container and 27.3 stabilizes the Anki facade)
|
||||
3. **TASK-27.4** — mpv-service.ts split (absorbs TASK-8 scope; blocked until 27.1 is done)
|
||||
4. **TASK-27.5** — renderer positioning.ts split (downscoped; after 27.2 to avoid import-path conflicts)
|
||||
|
||||
### Phase 3 — Stabilization
|
||||
|
||||
- **TASK-27.7** — Finalization and validation cleanup
|
||||
|
||||
## Smoke Test Checklist (applies to all subtasks)
|
||||
|
||||
Every subtask must verify before merging:
|
||||
|
||||
- [ ] App starts and connects to MPV
|
||||
- [ ] Subtitle text appears in overlay
|
||||
- [ ] Card mining creates a note in Anki
|
||||
@@ -68,9 +80,11 @@ Every subtask must verify before merging:
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
## Review Findings (2026-02-13)
|
||||
|
||||
### Key changes from original plan:
|
||||
|
||||
1. **Dropped parallel execution of Phase 2** — TASK-27.2 and 27.5 share import paths; 27.2 and 27.3 share main.ts wiring. Sequential order prevents merge conflicts.
|
||||
2. **Added TASK-7 as external prerequisite** — main.ts has 30+ module-level `let` declarations. Splitting files without a state container first just scatters mutable state.
|
||||
3. **TASK-8 absorbed into TASK-27.4** — TASK-8 (separate protocol from app logic) and TASK-27.4 (physical file split) overlap significantly. TASK-27.4 now covers both.
|
||||
@@ -82,11 +96,15 @@ Every subtask must verify before merging:
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
TASK-27 completed: plan execution sequence completed through all major refactor subtasks. Done status now confirmed for 27.1 (ownership mapping), 27.2 (main.ts split), 27.3 (anki-integration service-domain extraction), 27.4 (mpv-service split), 27.5 (renderer positioning split), and 27.7 (final validation summary, build + tests). Remaining work is now outside TASK-27 scope.
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
## Definition of Done
|
||||
|
||||
<!-- DOD:BEGIN -->
|
||||
|
||||
- [ ] #1 Plan task links and ordering are recorded in backlog descriptions.
|
||||
- [ ] #2 At least 2 independent owners are assigned with explicit labels in subtasks.
|
||||
<!-- DOD:END -->
|
||||
|
||||
@@ -25,21 +25,26 @@ priority: high
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
TASK-27.3 extracted leaf clusters (duplicate-detection 102 LOC, ai-translation 158 LOC, ui-feedback 107 LOC) but the core class remains at 2935 LOC. The heavy decomposition from the original TASK-27.3 plan was never executed.
|
||||
|
||||
Remaining extractions from the original plan:
|
||||
|
||||
1. **field-grouping** (~900 LOC) — `triggerFieldGroupingForLastAddedCard`, `applyFieldGrouping`, `computeFieldGroupingMergedFields`, `buildFieldGroupingPreview`, `performFieldGroupingMerge`, `handleFieldGroupingAuto`, `handleFieldGroupingManual`, plus ~15 span/parse/normalize helpers
|
||||
2. **card-creation** (~350 LOC) — `createSentenceCard`, `setCardTypeFields`, `extractFields`, `processSentence`, field resolution helpers
|
||||
3. **polling/lifecycle** (~250 LOC) — `start`, `stop`, `poll`, `pollOnce`, `processNewCard`
|
||||
|
||||
Also consolidate the scattered extraction files into `src/anki-integration/`:
|
||||
|
||||
- `src/anki-integration-duplicate.ts` → `src/anki-integration/duplicate.ts`
|
||||
- `src/anki-integration-ui-feedback.ts` → `src/anki-integration/ui-feedback.ts`
|
||||
- `src/anki-integration/ai.ts` (already there)
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
|
||||
- [ ] #1 anki-integration.ts reduced below 800 LOC (facade + private state wiring only)
|
||||
- [ ] #2 Field-grouping cluster (~900 LOC) extracted as its own module under src/anki-integration/
|
||||
- [ ] #3 Card-creation and polling/lifecycle extracted as separate modules
|
||||
@@ -51,5 +56,7 @@ Also consolidate the scattered extraction files into `src/anki-integration/`:
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
Implemented and stabilized the anki-integration refactor + transport/protocol fixes needed to keep 27.7 moving: fixed MPV protocol sub-end timing behavior, corrected split-buffer test fixtures, added injectable mpv transport socket factory to eliminate readonly Socket monkey-patching, and resolved TypeScript strictness issues in card-creation path (typed notesInfo cast, option signature/field guards/audio stream index). Updated related tests and build outputs accordingly. Validation results: `bun run build` passes and targeted suites pass: `src/core/services/mpv-protocol.test.ts`, `src/core/services/mpv-transport.test.ts`, `src/anki-integration.test.ts` (16/16).
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
@@ -15,7 +15,9 @@ priority: low
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
The core/services directory has inconsistent naming patterns that create confusion:
|
||||
|
||||
- Some files use `*Service.ts` suffix (e.g., `mpv-service.ts`, `tokenizer-service.ts`)
|
||||
- Others use `*RuntimeService.ts` or just descriptive names (e.g., `overlay-visibility-service.ts` exports functions with 'Service' in name)
|
||||
- Some functions in files have 'Service' suffix, others don't
|
||||
@@ -23,18 +25,23 @@ The core/services directory has inconsistent naming patterns that create confusi
|
||||
This inconsistency makes it hard to predict file/function names and creates cognitive overhead.
|
||||
|
||||
Standardize on:
|
||||
|
||||
- File names: `kebab-case.ts` without 'service' suffix (e.g., `mpv.ts`, `tokenizer.ts`)
|
||||
- Function names: descriptive verbs without 'Service' suffix (e.g., `createMpvClient()`, `tokenizeSubtitle()`)
|
||||
- Barrel exports: clean, predictable names
|
||||
|
||||
Files needing audit (47 services):
|
||||
|
||||
- All files in src/core/services/ need review
|
||||
|
||||
Note: This is a large-scale refactor that should be done carefully to avoid breaking changes.
|
||||
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
|
||||
- [x] #1 Establish naming convention rules (document in code or docs)
|
||||
- [x] #2 Audit all service files for naming inconsistencies
|
||||
- [x] #3 Rename files to follow convention (kebab-case, no 'service' suffix)
|
||||
@@ -48,6 +55,7 @@ Note: This is a large-scale refactor that should be done carefully to avoid brea
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
Starting implementation. Planning and executing a mechanical refactor: file names and exported symbols with Service suffix in `src/core/services`, then cascading import updates across `src/`.
|
||||
|
||||
Implemented naming convention refactor across `src/core/services`: removed `-service` from service file names, renamed Service-suffixed exported symbols to non-Service names, and updated barrel exports in `src/core/services/index.ts`.
|
||||
@@ -55,10 +63,13 @@ Implemented naming convention refactor across `src/core/services`: removed `-ser
|
||||
Updated call sites across `src/main/**`, `src/core/services/**` tests, `scripts/**`, `package.json` test paths, and docs references (`docs/development.md`, `docs/architecture.md`, `docs/structure-roadmap.md`).
|
||||
|
||||
Validation completed: `pnpm run build` and `pnpm run test:fast` both pass after refactor.
|
||||
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
Normalized `src/core/services` naming by removing `-service` from module filenames, dropping `Service` suffixes from exported service functions, and updating `src/core/services/index.ts` barrel exports to the new names. Updated all import/call sites across `src/main/**`, service tests, scripts, and docs/package test paths to match the new module and symbol names. Verified no behavior regressions with `pnpm run build` and `pnpm run test:fast` (all passing).
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
@@ -15,28 +15,34 @@ priority: medium
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
main.ts is still 1481 lines after previous refactoring efforts. While significant progress has been made, there are still opportunities to extract runtime functions into dedicated modules to further reduce its size and improve maintainability.
|
||||
|
||||
Current opportunities:
|
||||
|
||||
1. **JLPT dictionary lookup functions** (lines 470-535) - initializeJlptDictionaryLookup, ensureJlptDictionaryLookup, getJlptDictionarySearchPaths
|
||||
2. **Media path utilities** (lines 552-590) - updateCurrentMediaPath, updateCurrentMediaTitle, resolveMediaPathForJimaku
|
||||
3. **Overlay visibility helpers** (lines 1273-1360) - updateVisibleOverlayVisibility, updateInvisibleOverlayVisibility, syncInvisibleOverlayMousePassthrough
|
||||
|
||||
These functions are largely self-contained and could be moved to:
|
||||
|
||||
- `src/main/jlpt-runtime.ts`
|
||||
- `src/main/media-runtime.ts`
|
||||
- `src/main/media-runtime.ts`
|
||||
- `src/main/overlay-visibility-runtime.ts`
|
||||
|
||||
Goal: Reduce main.ts complexity by extracting focused runtime helpers into dedicated modules
|
||||
|
||||
Benefits:
|
||||
|
||||
- Faster navigation and comprehension of main.ts
|
||||
- Easier to test extracted modules independently
|
||||
- Clearer separation of concerns
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
|
||||
- [x] #1 Extract JLPT dictionary lookup functions to dedicated module
|
||||
- [x] #2 Extract media path utilities to dedicated module
|
||||
- [x] #3 Extract overlay visibility helpers to dedicated module
|
||||
@@ -49,7 +55,9 @@ Benefits:
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
Refactor complete for targeted runtime extraction: JLPT lookup, media utilities, and overlay visibility helpers were moved into dedicated main-runtime modules and wired from main.ts. Existing behavior preserved and full typecheck + test suite passed.
|
||||
|
||||
Task intent updated to prioritize readability over strict line-count target.
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
@@ -21,17 +21,22 @@ 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
|
||||
@@ -41,5 +46,7 @@ Separating these would let the protocol layer be simpler and testable, while app
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
Superseded by TASK-27.4 which absorbed this task's full scope (protocol/application separation, deps interface reduction from 22 properties to protocol-level concerns, event-based app reactions). All acceptance criteria met by TASK-27.4.
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
@@ -20,6 +20,7 @@ priority: low
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
main.ts contains many trivial single-line wrapper functions that add indirection without value:
|
||||
|
||||
```typescript
|
||||
@@ -37,10 +38,13 @@ function ensureOverlayWindowLevel(window: BrowserWindow): void {
|
||||
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
|
||||
@@ -50,11 +54,15 @@ After TASK-7 (AppState container), many of these can be eliminated by having ser
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
Priority changed from medium to low: this work is largely subsumed by TASK-27.2 (split main.ts). When main.ts is decomposed into composition-root modules, trivial wrappers will naturally be eliminated or inlined. Recommend folding remaining wrapper cleanup into TASK-27.2 rather than tracking separately. Keep this ticket as a checklist reference but don't execute independently.
|
||||
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
Subsumed by TASK-27.2 (main.ts split). Trivial wrappers were eliminated or inlined as composition-root modules were extracted. main.ts reduced from ~2000+ LOC to 1384 with state routed through appState container. Standalone wrapper removal pass no longer needed.
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
Reference in New Issue
Block a user