chore: updated refactoring plan

This commit is contained in:
2026-02-15 22:43:55 +01:00
parent e13cb209be
commit 60a291bedb
2 changed files with 289 additions and 7 deletions

3
.gitignore vendored
View File

@@ -92,6 +92,9 @@ crash-reports/
# Testing
# ===================
coverage/
.jscpd-report/
.jscpd-report-*/
.jscpd*.json
.nyc_output/
*.lcov
test-results/

View File

@@ -3,6 +3,23 @@
Date: 2026-02-15
Scope: Reduce high-impact code duplication in production and test-adjacent areas while preserving behavior.
## Current Status (after latest duplicate scan)
| Phase | Area | Status | Notes |
|---|---|---|---|
| 1 | Electron API contract | ⚠️ Partial | Shared contract exists, but overlapping renderer store data interfaces remain. |
| 2 | Tag mutation workflow | ⚠️ Partial | Helpers exist; duplicate blocks still present in task flows. |
| 3 | Post-media single/batch ops | ⚠️ Partial | Core primitives extracted; some duplicate link/unlink logic remains. |
| 4 | Project mapping + delete guards | ✅ Completed | Guard + mapper extracted and used in target methods. |
| 5 | Metadata sync loop scaffolding | ✅ Completed | `runSyncLoop` extracted and reused by both sync directions. |
| 6 | Shared color utility | ✅ Completed | Shared `getContrastColor` utility is in use. |
### Retired Phases (do not revisit unless regressions appear)
- Phase 4
- Phase 5
- Phase 6
## Objectives
- Eliminate high-risk duplication that can cause drift or inconsistent behavior.
@@ -22,6 +39,8 @@ Scope: Reduce high-impact code duplication in production and test-adjacent areas
## Phase 1 — Consolidate Electron API Contract (Highest ROI)
Status: ⚠️ Partially complete
### Problem
`electronAPI` shape is duplicated across multiple files and can drift:
- `src/main/preload.ts`
@@ -38,6 +57,10 @@ Create one canonical API contract and consume it from both preload and renderer
- Update `src/renderer/types/electron.d.ts` to reference the shared contract.
- Remove duplicate preload-local `ElectronAPI` declarations if no longer needed.
### Progress Check
- Completed: shared contract module exists and is imported by preload and renderer ambient types.
- Remaining: reduce overlapping data-shape duplication between shared API data interfaces and `src/renderer/store/appStore.ts` interfaces.
### Acceptance Criteria
- Single source of truth for `electronAPI` contract shape.
- No contract mismatch between preload and renderer types.
@@ -46,10 +69,21 @@ Create one canonical API contract and consume it from both preload and renderer
### Risks / Notes
- Ambient type import patterns in `.d.ts` files can be tricky; keep changes minimal and explicit.
### Coverage & Test Quality (fresh run: `npm run test:coverage`)
- `src/main/preload.ts`: not instrumented in coverage output (`MISSING`), so direct statement/branch confidence is low.
- `tests/engine/PreloadContract.test.ts`: 1 contract test; good smoke check, but narrow for refactor safety.
- `src/renderer/store/appStore.ts`: 63.33% statements, 53.25% functions, 56.25% branches.
- `tests/renderer/store/appStore.test.ts`: 14 tests; moderate confidence for store behavior, limited for API-contract drift.
### Impact (size/criticality)
- **Critical / Medium**: cross-process API/type drift can break renderer↔main integration and is hard to diagnose.
---
## Phase 2 — Extract Common Tag Mutation Workflow
Status: ⚠️ Partially complete
### Problem
`TagEngine` repeats similar logic in:
- `deleteTag`
@@ -70,6 +104,10 @@ Refactor shared behavior into private helpers while preserving all task/event se
- Update `deleteTag`, `mergeTags`, and `renameTag` to compose helpers.
- Expand/adjust tests in `tests/engine/TagEngine.test.ts` to protect behavior.
### Progress Check
- Completed: helper extraction has started (`queryPostsContainingTag`, `updatePostTagsAndSync`, and shared update path usage).
- Remaining: duplicate progress/task scaffolding still exists across `deleteTag` / `mergeTags` / `renameTag`.
### Acceptance Criteria
- Public method behavior and events unchanged.
- Reduced duplicate blocks in `TagEngine`.
@@ -78,10 +116,20 @@ Refactor shared behavior into private helpers while preserving all task/event se
### Risks / Notes
- Must preserve task progress percentages/messages expected by UI.
### Coverage & Test Quality (fresh run: `npm run test:coverage`)
- `src/main/engine/TagEngine.ts`: 95.98% statements, 98.00% functions, 77.89% branches.
- `tests/engine/TagEngine.test.ts`: 48 tests covering create/update/delete/merge/rename flows and edge cases.
- Quality is generally strong; remaining risk is branch-level behavior around progress/task scaffolding that still has partial gaps.
### Impact (size/criticality)
- **High / Medium-Large**: duplication sits in mutation workflows; drift can cause inconsistent tag edits across posts.
---
## Phase 3 — Unify Single/Batch Post-Media Operations
Status: ⚠️ Partially complete
### Problem
`PostMediaEngine` duplicates linking/unlinking logic across single and batch methods:
- `linkMediaToPost` vs `linkManyToPost`
@@ -98,6 +146,10 @@ Create common primitives and make single/batch methods compose them.
- Refactor single and batch methods to reuse primitives.
- Add parity tests in `tests/engine/PostMediaEngine.test.ts`.
### Progress Check
- Completed: shared primitives exist (`createPostMediaLink`, `removePostMediaLink`, sidecar helpers).
- Remaining: small duplicate segments remain between single/batch paths and can still be collapsed further.
### Acceptance Criteria
- No behavior regressions in link/unlink semantics.
- Event behavior preserved (`mediaLinked`/`mediaUnlinked` vs batch events).
@@ -106,10 +158,20 @@ Create common primitives and make single/batch methods compose them.
### Risks / Notes
- Preserve ordering and skip logic for already-linked media.
### Coverage & Test Quality (fresh run: `npm run test:coverage`)
- `src/main/engine/PostMediaEngine.ts`: 99.12% statements, 100.00% functions, 86.67% branches.
- `tests/engine/PostMediaEngine.test.ts`: 35 tests across single/batch link-unlink paths, events, ordering, and dedupe behavior.
- Very good safety net; branch coverage indicates some untested conditional paths remain.
### Impact (size/criticality)
- **High / Medium**: affects media linking integrity and batch operation consistency in editing workflows.
---
## Phase 4 — Consolidate Project Mapping and Guard Clauses
Status: ✅ Completed (retired)
### Problem
`ProjectEngine` repeats:
- default-project delete protection checks,
@@ -138,6 +200,8 @@ Centralize repetitive mapping and guard logic.
## Phase 5 — Extract Metadata Sync Loop Scaffolding
Status: ✅ Completed (retired)
### Problem
`MetadataDiffEngine` has near-identical loop scaffolding in:
- `syncDbToFile`
@@ -168,6 +232,8 @@ Abstract common iteration mechanics into one helper with operation-specific call
## Phase 6 — Deduplicate Shared Color Utility in Renderer
Status: ✅ Completed (retired)
### Problem
`getContrastColor` is duplicated in:
- `src/renderer/components/TagInput/TagInput.tsx`
@@ -192,14 +258,227 @@ Move color contrast logic into a shared renderer utility.
## Suggested Delivery Sequence
1. Phase 1 (API contract)
2. Phase 2 (TagEngine)
3. Phase 3 (PostMediaEngine)
4. Phase 4 (ProjectEngine)
5. Phase 5 (MetadataDiffEngine)
6. Phase 6 (Renderer utility)
1. Phase 1 (finish API/store type convergence)
2. Phase 2 (finish TagEngine workflow dedup)
3. Phase 3 (finish PostMedia single/batch dedup)
4. Phase 7 (WxrParser repeated parse blocks)
5. Phase 8 (MetaEngine ↔ TagEngine overlap)
6. Phase 9 (renderer tag event subscription helper)
7. Phase 10 (local UI repeated blocks in component files)
Rationale: start with highest blast-radius drift risk, then engine-level duplication, then UI utility cleanup.
Rationale: complete in-flight high-impact phases first, then address newly detected production hotspots.
---
## Newly Detected Phases (from latest scan)
## Phase 7 — Consolidate WXR Item Parse Blocks
### Problem
`WxrParser` contains repeated `pubDate` parsing + return-shape scaffolding in nearby item parse paths.
### Goal
Extract shared `parsePubDate` and/or shared item base builder helper to avoid drift.
### Tasks
- Add small private parser helpers in `src/main/engine/WxrParser.ts`.
- Replace duplicated blocks in both item parse branches.
- Preserve parse semantics with focused tests in `tests/engine/WxrParser.test.ts`.
### Acceptance Criteria
- No behavior change in parsed output.
- Duplicated `pubDate`/return scaffolding materially reduced.
### Coverage & Test Quality (fresh run: `npm run test:coverage`)
- `src/main/engine/WxrParser.ts`: 93.55% statements, 100.00% functions, 67.14% branches.
- `tests/engine/WxrParser.test.ts`: 19 tests covering parse variants, status handling, metadata extraction, and file-read paths.
- Branch coverage is the main gap; refactor should add branch-focused tests around date parsing and branch-specific fallbacks.
### Impact (size/criticality)
- **Medium / Small-Medium**: parser duplication can create subtle import differences, but blast radius is mostly import pipeline quality.
---
## Phase 8 — Merge Shared Taxonomy/Metadata Logic (MetaEngine + TagEngine)
### Problem
A medium-sized duplicate block appears across `MetaEngine` and `TagEngine` for taxonomy/tag-style handling.
### Goal
Extract a shared helper module or internal utility used by both engines.
### Tasks
- Identify exact shared operations (normalization, set/merge/update behavior).
- Add reusable utility in `src/main/engine` (or `src/main/shared` if appropriate).
- Replace both engine-local copies with utility calls.
- Add/adjust tests in `tests/engine/MetaEngine.test.ts` and `tests/engine/TagEngine.test.ts`.
### Acceptance Criteria
- Equivalent behavior in both engines.
- Single implementation for overlapping logic.
### Coverage & Test Quality (fresh run: `npm run test:coverage`)
- `src/main/engine/MetaEngine.ts`: 95.03% statements, 96.55% functions, 77.59% branches.
- `src/main/engine/TagEngine.ts`: 95.98% statements, 98.00% functions, 77.89% branches.
- Tests are extensive (`MetaEngine`: 53 tests, `TagEngine`: 48 tests), but mostly engine-local; cross-engine shared-helper contracts are not explicitly asserted yet.
### Impact (size/criticality)
- **High / Medium**: duplicate taxonomy logic across engines is a drift risk for metadata/tag consistency.
---
## Phase 9 — Renderer Tag Event Subscription Dedup
### Problem
`TagInput` and `TagsView` duplicate tag-event subscription setup/teardown scaffolding.
### Goal
Introduce a small shared renderer utility/hook for tag event subscriptions.
### Tasks
- Add a focused helper/hook under `src/renderer/utils` or `src/renderer/components`.
- Replace duplicated subscribe/unsubscribe code in both components.
- Keep event coverage differences explicit where needed (e.g., `tag:updated` in `TagsView`).
### Acceptance Criteria
- Same runtime behavior and refresh triggers.
- One canonical subscription implementation.
### Coverage & Test Quality (fresh run: `npm run test:coverage`)
- `src/renderer/components/TagInput/TagInput.tsx`: 0% statements/functions/branches.
- `src/renderer/components/TagsView/TagsView.tsx`: 0% statements/functions/branches.
- No dedicated renderer component tests found for these files; refactor safety currently relies on manual behavior checks.
### Impact (size/criticality)
- **Medium / Medium**: mostly UX/event-refresh consistency risk; low data-corruption risk but high chance of regressions in UI responsiveness.
---
## Phase 10 — Local UI Component Block Dedup (Low Risk)
### Problem
Small duplicated blocks exist within individual components (`ProjectSelector`, `CredentialsPanel`, `AISuggestionsModal`, and parts of `App.tsx`).
### Goal
Reduce repeated local code with tiny helpers while preserving readability.
### Tasks
- Consolidate repeated JSX/control-flow blocks per component.
- Keep changes local and minimal; avoid broad redesign.
- Add/adjust nearby renderer tests where behavior could regress.
### Acceptance Criteria
- Lower clone count in affected files.
- No UX or behavior changes.
### Coverage & Test Quality (fresh run: `npm run test:coverage`)
- `src/renderer/components/ProjectSelector/ProjectSelector.tsx`: 0% statements/functions/branches.
- `src/renderer/components/CredentialsPanel/CredentialsPanel.tsx`: 0% statements/functions/branches.
- `src/renderer/components/AISuggestionsModal/AISuggestionsModal.tsx`: 0% statements/functions/branches.
- `src/renderer/App.tsx`: 0% statements/functions/branches.
- No dedicated component tests found for these phase-10 targets.
### Impact (size/criticality)
- **Medium-Low / Small-Medium**: local duplications with limited architectural risk, but high regression risk without renderer tests.
---
## Test Plan (Open Phases)
Goal: provide a concrete Red-Green checklist for remaining phases so refactors are protected by behavior-first tests.
### Phase 1 — Electron API Contract
Add/extend tests in:
- `tests/engine/PreloadContract.test.ts`
- `tests/renderer/store/appStore.test.ts`
Planned test cases:
- [ ] `preload exposes shared contract keys only`: assert `contextBridge.exposeInMainWorld` receives exactly the shared API shape.
- [ ] `contract import parity`: assert preload and renderer ambient declarations both resolve to shared contract types (compile-time fixture test pattern).
- [ ] `store contract boundary`: add a test that verifies store-side API data assumptions match shared contract field names and optionality.
### Phase 2 — TagEngine Workflow Dedup
Add/extend tests in:
- `tests/engine/TagEngine.test.ts`
Planned test cases:
- [ ] `deleteTag progress sequence remains stable`: verify progress/message checkpoints across multi-post delete.
- [ ] `mergeTags progress sequence remains stable`: same assertion pattern for merge path.
- [ ] `renameTag progress sequence remains stable`: same assertion pattern for rename path.
- [ ] `shared helper parity`: same input set through delete/merge/rename keeps DB/file updates and emitted events identical to pre-refactor behavior.
- [ ] `single-write-per-post invariant`: explicit assertion that duplicated result rows never cause duplicate writes.
### Phase 3 — PostMedia Single/Batch Unification
Add/extend tests in:
- `tests/engine/PostMediaEngine.test.ts`
Planned test cases:
- [ ] `single vs batch link parity`: linking one item via single API and batch API yields identical row + sidecar state.
- [ ] `single vs batch unlink parity`: unlinking one item via both paths yields identical state.
- [ ] `already-linked/skip parity`: same skip semantics and counters between paths.
- [ ] `event contract parity`: preserve single-event vs batch-event distinctions after helper consolidation.
- [ ] `sort-order parity`: batch and single operations preserve expected incremental ordering rules.
### Phase 7 — WxrParser Parse Block Consolidation
Add/extend tests in:
- `tests/engine/WxrParser.test.ts`
Planned test cases:
- [ ] `pubDate variants`: valid RFC822, invalid date, and missing date all map to same outputs as before.
- [ ] `item branch parity`: duplicated parse branches produce identical base item fields after helper extraction.
- [ ] `fallback timestamp behavior`: explicitly verify fallback behavior for empty/invalid date nodes.
- [ ] `mixed content regression`: ensure post/page/media classification remains unchanged when helper is introduced.
### Phase 8 — MetaEngine + TagEngine Shared Taxonomy Logic
Add/extend tests in:
- `tests/engine/MetaEngine.test.ts`
- `tests/engine/TagEngine.test.ts`
Planned test cases:
- [ ] `shared normalization contract`: same tag/category normalization outputs in both engines for a shared fixture matrix.
- [ ] `cross-engine consistency`: tag/category sets derived from identical post inputs are equivalent across engines.
- [ ] `event invariants preserved`: no change to emitted event names/payloads while shared helper is adopted.
- [ ] `file-persistence invariants`: persistence behavior remains engine-specific where intended (no accidental cross-responsibility).
### Phase 9 — Renderer Tag Event Subscription Dedup
Add tests in (new files expected):
- `tests/renderer/components/TagInput.test.tsx`
- `tests/renderer/components/TagsView.test.tsx`
Planned test cases:
- [ ] `subscribe on mount / unsubscribe on unmount`: helper registers and cleans up listeners exactly once.
- [ ] `TagInput refresh triggers`: relevant tag events trigger refresh/update behavior.
- [ ] `TagsView refresh triggers`: includes `tag:updated` path specific to view behavior.
- [ ] `duplicate listener prevention`: re-render does not leak or double-register listeners.
### Phase 10 — Local UI Block Dedup
Add tests in (new files expected):
- `tests/renderer/components/ProjectSelector.test.tsx`
- `tests/renderer/components/CredentialsPanel.test.tsx`
- `tests/renderer/components/AISuggestionsModal.test.tsx`
- `tests/renderer/App.test.tsx`
Planned test cases:
- [ ] `ProjectSelector repeated-branch parity`: same render/action outcomes for duplicated conditional blocks.
- [ ] `CredentialsPanel validation/render parity`: unchanged validation and submit enablement behavior.
- [ ] `AISuggestionsModal action parity`: unchanged accept/reject/apply flows.
- [ ] `App.tsx orchestration parity`: deduped local helper paths preserve top-level view switching and wiring behavior.
### Execution Order for Tests
1. Add phase-local failing tests first.
2. Refactor only that phase until tests pass.
3. Run targeted suite for touched area.
4. Run full suite: `npm test`.
5. Run full coverage: `npm run test:coverage`.
6. Run build: `npm run build`.
---