13 KiB
SubMiner Refactoring Plan
Goals
- Eliminate unnecessary abstraction layers and thin wrapper services
- Consolidate related services into cohesive domain modules
- Fix known bugs and code quality issues
- Add test coverage for critical untested services
- Keep main.ts lean without pushing complexity into pointless indirection
Guiding Principles
- Verify after every phase:
pnpm run build && pnpm run test:config && pnpm run test:coremust pass - One concern per commit: each commit should be a single logical change (can be multiple files as long as it makes sense logically)
- Inline first, restructure second: delete the wrapper, verify nothing breaks, then clean up
- Don't create new abstractions: the goal is fewer files, not different files
Phase 1: Delete Thin Wrappers (9 files + 7 test files)
These services wrap a single function call or trivial operation. Inline their logic into callers (mostly main.ts or the service that calls them) and delete both the service file and its test file.
1.1 Inline config-warning-runtime-service.ts (14 lines)
Two pure string-formatting functions. Inline the format string directly where
logConfigWarningRuntimeService is called (should be in main.ts startup or
app-logging-runtime-service.ts). Delete both files.
- Delete:
config-warning-runtime-service.ts,config-warning-runtime-service.test.ts
1.2 Inline overlay-modal-restore-service.ts (18 lines)
Wraps Set.add() and a conditional Set.delete(). Inline the Set operations at
call sites in main.ts.
- Delete:
overlay-modal-restore-service.ts,overlay-modal-restore-service.test.ts
1.3 Inline runtime-options-manager-runtime-service.ts (17 lines)
Wraps new RuntimeOptionsManager(...). Call the constructor directly.
- Delete:
runtime-options-manager-runtime-service.ts,runtime-options-manager-runtime-service.test.ts
1.4 Inline app-logging-runtime-service.ts (28 lines)
Creates an object with two methods. After inlining config-warning (1.1), this becomes a trivial object literal. Inline where the logging runtime is created.
- Delete:
app-logging-runtime-service.ts,app-logging-runtime-service.test.ts
1.5 Inline overlay-send-service.ts (26 lines)
Wraps window.webContents.send() with a null/destroyed guard. This is a
one-liner pattern. Inline at call sites.
- Delete:
overlay-send-service.ts(no test file)
1.6 Inline startup-resource-runtime-service.ts (26 lines)
Two functions that call a constructor and a check method. Inline into the app-ready startup flow.
- Delete:
startup-resource-runtime-service.ts,startup-resource-runtime-service.test.ts
1.7 Inline config-generation-runtime-service.ts (26 lines)
Simple conditional (if args say generate config, call generator, quit). Inline into the startup bootstrap flow.
- Delete:
config-generation-runtime-service.ts,config-generation-runtime-service.test.ts
1.8 Inline app-shutdown-runtime-service.ts (27 lines)
Calls 10 cleanup functions in sequence. Inline into the willQuit handler in main.ts.
- Delete:
app-shutdown-runtime-service.ts,app-shutdown-runtime-service.test.ts
1.9 Inline shortcut-ui-deps-runtime-service.ts (24 lines)
Single wrapper that unwraps 4 getters and calls runOverlayShortcutLocalFallback.
Inline at call site.
- Delete:
shortcut-ui-deps-runtime-service.ts,shortcut-ui-deps-runtime-service.test.ts
Phase 1 verification
pnpm run build && pnpm run test:core
Expected result: 16 files deleted, ~260 lines of service code removed,
~350 lines of test code for trivial wrappers removed. index.ts barrel export
shrinks by ~10 entries.
Phase 2: Consolidate DI Adapter Services (4 files)
These are 50-130 line files that map one interface shape to another with minimal logic. They exist because the service they adapt has a different interface than what main.ts provides. The fix is to align the interfaces so the adapter isn't needed, or absorb the adapter into the service it feeds.
2.1 Merge cli-command-deps-runtime-service.ts into cli-command-service.ts
The deps adapter (132 lines) maps main.ts state into CliCommandServiceDeps.
Instead: make handleCliCommandService accept the same shape main.ts naturally
provides, or accept a smaller interface with the actual values rather than
getter/setter pairs. Move any null-guarding logic into the command handlers
themselves.
- Delete:
cli-command-deps-runtime-service.ts,cli-command-deps-runtime-service.test.ts - Modify:
cli-command-service.tsto accept deps directly
2.2 Merge ipc-deps-runtime-service.ts into ipc-service.ts
Same pattern as 2.1. The deps adapter (100 lines) maps main.ts state for IPC handlers. Merge the defensive null checks and window guards into the IPC handlers that need them.
- Delete:
ipc-deps-runtime-service.ts,ipc-deps-runtime-service.test.ts - Modify:
ipc-service.ts
2.3 Merge tokenizer-deps-runtime-service.ts into tokenizer-service.ts
The adapter (45 lines) has one non-trivial function (tokenizeWithMecab with null
checks and token merging). Move that logic into tokenizer-service.ts.
- Delete:
tokenizer-deps-runtime-service.ts,tokenizer-deps-runtime-service.test.ts - Modify:
tokenizer-service.ts
2.4 Merge app-lifecycle-deps-runtime-service.ts into app-lifecycle-service.ts
The adapter (57 lines) wraps Electron app events. Merge event binding into the lifecycle service itself since it already knows about Electron's app lifecycle.
- Delete:
app-lifecycle-deps-runtime-service.ts,app-lifecycle-deps-runtime-service.test.ts - Modify:
app-lifecycle-service.ts
Phase 2 verification
pnpm run build && pnpm run test:core
Expected result: 8 more files deleted. DI adapters absorbed into the services
they feed. index.ts shrinks further.
Phase 3: Consolidate Related Services
Merge services that are split across multiple files but represent a single concern.
3.1 Merge overlay visibility files
overlay-visibility-runtime-service.ts (46 lines) is a thin orchestration layer
over overlay-visibility-service.ts (183 lines). Merge into one file:
overlay-visibility-service.ts.
- Delete:
overlay-visibility-runtime-service.ts - Modify:
overlay-visibility-service.ts— absorb the 3 exported functions
3.2 Merge overlay broadcast files
overlay-broadcast-runtime-service.ts (45 lines) contains utility functions for
window filtering and broadcasting. These are closely related to
overlay-manager-service.ts (49 lines) which manages the window references.
Merge broadcast functions into the overlay manager since it already owns the
window state.
- Delete:
overlay-broadcast-runtime-service.ts,overlay-broadcast-runtime-service.test.ts - Modify:
overlay-manager-service.ts— add broadcast methods
3.3 Merge overlay shortcut files
There are 4 shortcut-related files:
overlay-shortcut-service.ts(169 lines) — registrationoverlay-shortcut-runtime-service.ts(105 lines) — runtime handlersoverlay-shortcut-lifecycle-service.ts(52 lines) — sync/refresh/unregisteroverlay-shortcut-fallback-runner.ts(114 lines) — local fallback execution
Consolidate into 2 files:
-
overlay-shortcut-service.ts— registration + lifecycle (absorb lifecycle-service) -
overlay-shortcut-handler.ts— runtime handlers + fallback runner -
Delete:
overlay-shortcut-lifecycle-service.ts,overlay-shortcut-fallback-runner.ts
3.4 Merge numeric shortcut files
numeric-shortcut-runtime-service.ts (37 lines) and
numeric-shortcut-session-service.ts (99 lines) are two halves of one feature.
Merge into numeric-shortcut-service.ts.
- Delete:
numeric-shortcut-runtime-service.ts,numeric-shortcut-runtime-service.test.ts - Modify:
numeric-shortcut-session-service.ts→ rename tonumeric-shortcut-service.ts
3.5 Merge startup/bootstrap files
startup-bootstrap-runtime-service.ts (53 lines) and
app-ready-runtime-service.ts (77 lines) are both startup orchestration.
Merge into a single startup-service.ts.
- Delete:
startup-bootstrap-runtime-service.ts,startup-bootstrap-runtime-service.test.ts - Modify:
app-ready-runtime-service.ts→ rename tostartup-service.ts, absorb bootstrap
Phase 3 verification
pnpm run build && pnpm run test:core
Expected result: ~10 more files deleted. Related services consolidated into single cohesive modules.
Phase 4: Fix Bugs and Code Quality
4.1 Remove debug console.log statements
overlay-visibility-service.ts has 7+ raw console.log/console.warn calls
used for debugging. Remove them or replace with the app logger if the messages
have ongoing diagnostic value.
4.2 Fix as never type cast
tokenizer-deps-runtime-service.ts (or its successor after Phase 2) uses
return mergeTokens(rawTokens as never). Investigate the actual type mismatch
and fix it properly.
4.3 Guard fieldGroupingResolver against race conditions
In main.ts, the fieldGroupingResolver is a single global variable. If two
field grouping requests arrive concurrently, the second overwrites the first's
resolver. Add a request ID or sequence number so stale resolutions are ignored.
4.4 Audit async callbacks in CLI command handlers
Verify that async functions passed as callbacks in the CLI command and IPC handler
wiring (main.ts lines ~697-707, ~1347, ~1360) are properly awaited or have
.catch() handlers so rejections aren't silently swallowed.
4.5 Drop the -runtime-service naming convention
After phases 1-3, rename remaining files to just *-service.ts. The "runtime"
prefix adds no meaning. Do this as a batch rename commit with no logic changes.
Phase 4 verification
pnpm run build && pnpm run test:core
Manually smoke test: launch SubMiner, verify overlay renders, mine a card, toggle field grouping.
Phase 5: Add Tests for Critical Untested Services
These are the highest-risk untested modules. Add focused tests that verify real behavior, not just that mocks were called.
5.1 mpv-service.ts (761 lines, untested)
Test the IPC protocol layer:
- Socket message parsing (JSON line protocol)
- Property change event dispatch
- Request/response matching via request IDs
- Reconnection behavior on socket close
- Subtitle text extraction from property changes
5.2 subsync-service.ts (427 lines, untested)
Test:
- Config resolution (alass vs ffsubsync path selection)
- Command construction for each sync engine
- Timeout and error handling for child processes
- Result parsing
5.3 tokenizer-service.ts (305 lines, untested)
Test:
- Yomitan parser initialization and readiness
- Token extraction from parsed results
- Fallback behavior when parser unavailable
- Edge cases: empty text, CJK-only, mixed content
5.4 cli-command-service.ts (204 lines, partially tested)
Expand existing tests to cover:
- All CLI command dispatch paths
- Error propagation from async handlers
- Second-instance argument forwarding
Phase 5 verification
pnpm run test:core
All new tests pass. Aim for the 4 services above to each have 5-10 meaningful test cases.
Phase 6 (Optional): Domain-Based Directory Structure
After phases 1-5, the service count should be roughly 20-25 files down from 47. If that still feels too flat, group by domain:
src/core/
mpv/
mpv-service.ts
mpv-render-metrics-service.ts
overlay/
overlay-manager-service.ts
overlay-visibility-service.ts
overlay-window-service.ts
overlay-shortcut-service.ts
overlay-shortcut-handler.ts
mining/
mining-runtime-service.ts
field-grouping-service.ts
field-grouping-overlay-service.ts
startup/
startup-service.ts
app-lifecycle-service.ts
ipc/
ipc-service.ts
ipc-command-service.ts
shortcuts/
shortcut-service.ts
numeric-shortcut-service.ts
services/
tokenizer-service.ts
subtitle-position-service.ts
subtitle-ws-service.ts
texthooker-service.ts
yomitan-extension-loader-service.ts
yomitan-settings-service.ts
secondary-subtitle-service.ts
runtime-config-service.ts
runtime-options-runtime-service.ts
cli-command-service.ts
Only do this if the flat directory still feels unwieldy after consolidation. This is cosmetic and low-priority relative to phases 1-5.
Summary
| Phase | Files Deleted | Files Modified | Risk | Effort |
|---|---|---|---|---|
| 1. Delete thin wrappers | 16 (9 svc + 7 test) | main.ts, index.ts | Low | Small |
| 2. Consolidate DI adapters | 8 (4 svc + 4 test) | 4 services | Medium | Medium |
| 3. Merge related services | ~10 | ~5 services | Medium | Medium |
| 4. Fix bugs & rename | 0 | ~6 files | Low | Small |
| 5. Add critical tests | 0 | 4 new test files | Low | Medium |
| 6. Directory restructure | 0 | All imports | Low | Small |
Net result: ~34 files deleted, service count from 47 → ~22, index.ts from 92 exports → ~45, and the remaining services each have a clear reason to exist.