From 2aa56bb2a316a11ecdb7868f356106062e9897d7 Mon Sep 17 00:00:00 2001 From: clockblocker Date: Thu, 19 Feb 2026 20:25:15 +0100 Subject: [PATCH] docs: add 12 improvement ideas from automated codebase audit Appends ideas #18-#29 to existing ideas-to-consider.md as Tier 5 (Automated audit 2026-02-19). Covers: stateless helper tests (P1), OverlayManager coverage (P1), magic number extraction (P2), API error handling (P2), threshold TODOs (P2), error handling docs (P2), retry profiles (P2), teardown verification (P3), suffix consolidation (P3), prefix matching optimization (P3), and path aliases (P3). Docs-only change: 1 file, 118 insertions, 0 source code modifications. Nightshift-Task: idea-generator Nightshift-Ref: https://github.com/marcus/nightshift Co-Authored-By: Claude Opus 4.6 --- documentation/ideas-to-consider.md | 118 +++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/documentation/ideas-to-consider.md b/documentation/ideas-to-consider.md index 25b566d4e..ce44af96f 100644 --- a/documentation/ideas-to-consider.md +++ b/documentation/ideas-to-consider.md @@ -144,3 +144,121 @@ What: this.app.metadataCache.off("resolved", () => null) passes anonymous fn that never matches. Listener never removed. Check if still present in src/main.ts. Descision: Accepted. Add to the book of work + + + Tier 5: Automated audit (2026-02-19) + + #: 18 + Item: Unit tests for stateless helpers (offset-mapper, block-id, markdown-strip) + Source: Automated audit + What: Pure utility modules in src/stateless-helpers/ have no dedicated test files. offset-mapper.ts + has complex offset remapping logic, block-id.ts has parsing, markdown-strip.ts does markdown + cleaning. All are pure functions — trivial to test with no Obsidian runtime deps. + Complements #15 (unit test coverage expansion) with specific targets. + Priority: P1 + + ──────────────────────────────────────── + #: 19 + Item: OverlayManager test coverage + Source: Automated audit + What: src/managers/overlay-manager/ has zero test files despite being a complex orchestrator + (selection toolbar, bottom toolbar, context menu, edge zones, action click dispatch). Add + smoke tests for init/teardown lifecycle, workspace event handling, and action click routing. + Priority: P1 + + ──────────────────────────────────────── + #: 20 + Item: Extract magic numbers to named constants in event coalescing + Source: Automated audit + What: Hardcoded 250ms quiet window and 2000ms max window in bulk-event-emmiter.ts, 5000ms TTL + in self-event-tracker.ts, 500ms navigation timeout in cd.ts. Extract to named constants with + comments explaining the rationale for each value. + Priority: P2 + + ──────────────────────────────────────── + #: 21 + Item: Wrap undocumented Obsidian API accesses in cd.ts + Source: Automated audit + What: cd.ts uses multiple `as unknown` casts to access undocumented APIs (leftSplit.collapsed, + app.commands.executeCommandById). Wrap these in a helper with try-catch and version comments + so Obsidian API drift surfaces cleanly instead of crashing. + Priority: P2 + + ──────────────────────────────────────── + #: 22 + Item: API error catch block improvement in api-service.ts + Source: Automated audit + What: Line ~199 in api-service.ts uses empty catch with `___errors` — error info is discarded. + Also, Promise.race() with timeout at line ~187 has non-obvious semantics (rejection from + postGoogleApi propagates through race). Consider capturing error details in the catch and + clarifying the timeout race pattern. + Priority: P2 + + ──────────────────────────────────────── + #: 23 + Item: Resolve calibration TODOs for numeric thresholds + Source: Automated audit + What: Several unresolved TODOs around tunable thresholds: + - multi-span.ts:74 "search radius is a tunable parameter" + - note-adapter.ts:134 "calibrate sampling cache size" + - propagate-morphemes.ts:72 "calibrate this threshold" + Either document current values with rationale or extract to a TUNABLE_PARAMS object. + Priority: P2 + + ──────────────────────────────────────── + #: 24 + Item: Document error handling decision tree + Source: Automated audit + What: Inconsistent patterns across codebase — some code uses logError() (auto-notice + log), + some uses raw logger.error(), some catch blocks swallow silently. Add a brief section to + CLAUDE.md or a docs file with when to use each: + - Recoverable → neverthrow Result + - Unrecoverable → logError() with notice + - Expected → logger.warn() + - Debug → logger.info() + Priority: P2 + + ──────────────────────────────────────── + #: 25 + Item: Retry config per-API profile + Source: Automated audit + What: retry.ts has a single DEFAULT_CONFIG (1s base, 3 attempts, 2x multiplier). API service + has 45s timeout but uses generic retry. Allow passing custom retry profiles from ApiService + for different call types (e.g., generate vs cache creation). + Priority: P2 + + ──────────────────────────────────────── + #: 26 + Item: OverlayManager teardown verification + Source: Automated audit + What: overlay-manager.ts stores selectionHandlerTeardown as a closure. If plugin is disabled + without calling unload(), the handler reference persists in userEventInterceptor. Add explicit + verification that all teardowns fire during unload(), or log a warning if they don't. + Priority: P3 + + ──────────────────────────────────────── + #: 27 + Item: Suffix extraction consolidation + Source: Automated audit + What: Suffix parsing logic appears in multiple codec files (split-path-with-separated-suffix, + segment-id/parse.ts, wikilink.ts) with similar regex patterns. Consider extracting shared + extractSuffix()/splitNameAndSuffix() helpers to reduce maintenance burden. + Priority: P3 + + ──────────────────────────────────────── + #: 28 + Item: Self-event tracker prefix matching optimization + Source: Automated audit + What: self-event-tracker.ts iterates all tracked prefixes for every incoming event (O(n) per + event). For large vault operations with many folder renames, this could be slow. Consider a + trie or sorted-prefix binary search if profiling shows this as a bottleneck. + Priority: P3 + + ──────────────────────────────────────── + #: 29 + Item: Path aliases in tsconfig.json + Source: Automated audit + What: 15+ files have deep relative imports (4-5 levels: ../../../managers/obsidian/...). + Adding tsconfig path aliases (@managers/*, @commanders/*, @helpers/*) would improve + readability. Lower priority since it works fine as-is and has refactoring risk. + Priority: P3