diff --git a/docs/superpowers/plans/2026-09-02-v0.7.0-review-fixes.md b/docs/superpowers/plans/2026-09-02-v0.7.0-review-fixes.md new file mode 100644 index 0000000..06ef2e9 --- /dev/null +++ b/docs/superpowers/plans/2026-09-02-v0.7.0-review-fixes.md @@ -0,0 +1,1243 @@ +# Vault Inspector 0.7.0 Review Fixes Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix all eight findings from the `0.6.0..0.7.0` code review and the status-badge rendering defect shown in the UI screenshot, without expanding Vault Inspector beyond its existing scanners, report, verified-fix pipeline, automatic scans, and read-only CLI. + +**Architecture:** Preserve the current scanner and report boundaries. Reference coverage must fail closed before destructive actions are offered; duplicate decisions must be revalidated against every selection field that can change deletion targets; review-required fixes must enter the existing single-item confirmation pipeline; CLI baselines must carry a complete, unfiltered identity set; automatic notices must select only new confirmed errors; empty-note structure detection must recognize Markdown links; high-degree reference aggregation must use sets internally and materialize sorted arrays only once; status badges must size to content and use theme-safe contrasting colors. + +**Tech Stack:** TypeScript, Obsidian API, Node.js, Vitest, ESLint, CSS + +Source: `0.6.0..0.7.0` code review findings and the 2026-09-02 UI screenshot. + +--- + +## Scope and acceptance matrix + +| # | Finding | Repair task | Primary regression proof | +|---|---|---|---| +| 1 | Fresh duplicate preflight can reuse a stale keep decision after reference evidence changes | Task 4 | `getFreshFixAction` returns `null` when `automaticKeepPath` or `referencedPaths` changes | +| 2 | JSON Canvas group backgrounds are absent from the shared reference index | Task 2 | A group `background` image contributes a `canvas` inbound reference | +| 3 | A Markdown file with no metadata cache entry still leaves reference coverage marked complete | Task 2 | Missing cache emits `metadata-cache-missing`, blocks orphan deletion, and creates one unverified coverage finding | +| 4 | `review-required` fixes are excluded from bulk actions and have no single-item UI route | Task 5 | Active issue Actions exposes `Review fix`; it invokes `onFixAllIssues([issue])`; blocked and ignored rows do not expose it | +| 5 | A filtered CLI JSON report loses fingerprints but can still be reused as a complete baseline | Task 6 | A filtered baseline preserves the full identity set and does not relabel hidden prior findings as new or resolved | +| 6 | Automatic scans notify for new confirmed warnings and info findings | Task 7 | Only `severity: "error"`, `classification: "confirmed"`, lifecycle `new` issues produce a notice | +| 7 | A Markdown-link-only MOC can be classified as empty | Task 8 | `[Target](target.md)` and similar Markdown links count as meaningful structures and suppress the empty-note finding | +| 8 | Reference source deduplication is quadratic for high-degree targets | Task 3 | A 50,000-source target completes inside a bounded performance test with exact, sorted output | +| 9 | Green `Eligible` and `Confirmed` badges stretch like progress bars and their text can disappear | Task 9 | Badges use content width and a background/text pairing that remains distinct under the existing CSS token contract | + +## Ground rules + +- Branch: `fix/v0.7.0-review-findings`, cut from the latest `main`. +- This plan fixes existing behavior only. Do not add scanners, new settings, automatic mutation, reference rewriting, or new CLI commands. +- Keep the CLI scan path read-only and keep fix execution behind the existing confirmation and fresh-scan pipeline. +- Treat incomplete reference coverage as a safety boundary: no trash action may be offered while any Markdown or Canvas reference source is unindexed. +- Do not weaken bulk gating. `review-required` remains excluded from one-click bulk fixes; the new route is an explicit single-item review. +- Preserve deterministic fingerprints and deterministic ordering of reference kinds, sources, coverage failures, and CLI baseline fingerprints. +- Keep `schemaVersion: 1`; the CLI change is additive. Current-format reports must carry a complete identity field, while older profile-aware reports that lack it must fail closed with a regeneration message instead of being silently treated as complete. +- Legacy reports with no `comparison` object keep their documented fingerprint-only compatibility mode and warning. +- Never `eslint-disable` an `obsidianmd/*` rule. +- Each task ends in its own conventional commit. Scanner-specific work is not bundled with report, CLI, or scheduler changes. +- Full release gate before the final handoff: `npm run lint && npm run lint:obsidian-warnings && npm run build && npm test && npm pack --dry-run`. + +## Commit map + +1. `fix: fail closed on incomplete reference coverage` +2. `perf: bound reference index aggregation` +3. `fix: revalidate duplicate keep decisions` +4. `fix: expose reviewed fix actions` +5. `fix: preserve complete CLI baseline identity` +6. `fix: notify only for new confirmed errors` +7. `fix: recognize markdown links as note structure` +8. `fix: render status badges legibly` + +--- + +### Task 1: Create the repair branch and record the starting state + +**Files:** +- No file changes + +- [ ] **Step 1: Synchronize the protected base branch** + +```bash +git switch main +git pull --ff-only +git status --short --branch +``` + +Expected: `main` is aligned with `origin/main` and the worktree is clean. Stop if unrelated local changes exist; do not discard them. + +- [ ] **Step 2: Create the implementation branch** + +```bash +git switch -c fix/v0.7.0-review-findings +``` + +- [ ] **Step 3: Pin the pre-change gate** + +```bash +npm run lint +npm run lint:obsidian-warnings +npm run build +npm test +npm pack --dry-run +``` + +Expected: all commands pass before implementation. If a command already fails on untouched `main`, record the exact failure and separate it from this repair scope. + +--- + +### Task 2: Make shared reference coverage complete and fail closed + +**Findings:** #2 Canvas group background, #3 missing Markdown metadata cache + +**Files:** +- Modify: `src/scanner/reference-index.ts` +- Modify: `src/scanner/scanners/orphan-attachments.ts` +- Modify: `src/tests/reference-index.test.ts` +- Modify: `src/tests/orphan-attachments.test.ts` + +- [ ] **Step 1: Add failing reference-index tests** + +In `src/tests/reference-index.test.ts`, add a Markdown coverage test to `describe("buildReferenceIndex markdown sources")`: + +```typescript + it("marks coverage incomplete when a Markdown metadata cache entry is missing", async () => { + const source = makeTestFile("notes/uncached.md"); + const attachment = makeTestFile("assets/maybe-used.png"); + const ctx = makeScanContext({ + files: [source, attachment], + metadataByPath: {}, + }); + + const index = await buildReferenceIndex(ctx); + + expect(index.coverageComplete).toBe(false); + expect(index.coverageFailures).toEqual([ + { path: "notes/uncached.md", reason: "metadata-cache-missing" }, + ]); + }); +``` + +Add a Canvas group-background test to `describe("buildReferenceIndex canvas sources")`: + +```typescript + it("records Canvas group background references", async () => { + const ctx = canvasContext({ + "canvas/board.canvas": JSON.stringify({ + nodes: [ + { + id: "group-1", + type: "group", + background: "assets/background.png", + }, + ], + edges: [], + }), + }); + const background = makeTestFile("assets/background.png"); + ctx.allFiles = [...ctx.allFiles, background]; + ctx.filePathIndex = new Set([...ctx.filePathIndex, background.path]); + + const index = await buildReferenceIndex(ctx); + + expect(getInboundReference(index, background.path)).toEqual({ + count: 1, + kinds: ["canvas"], + sources: ["canvas/board.canvas"], + }); + }); +``` + +- [ ] **Step 2: Add the failing orphan-safety test** + +In `src/tests/orphan-attachments.test.ts`, add a test using a `metadata-cache-missing` coverage failure: + +```typescript + it("blocks deletion when a Markdown reference source was not indexed", async () => { + const img = makeFile("assets/maybe-used.png", OLD_MTIME); + const ctx = makeCtx({ + allFiles: [img], + filePathIndex: new Set([img.path]), + referenceIndex: makeIndex({}, [ + { path: "notes/uncached.md", reason: "metadata-cache-missing" }, + ]), + }); + + const issues = await orphanAttachmentsScanner.scan(ctx); + const orphan = issues.find((issue) => issue.title === "Orphan attachment"); + const coverage = issues.find( + (issue) => issue.title === "Reference coverage incomplete", + ); + + expect(orphan?.fixAction).toBeUndefined(); + expect(orphan?.evidence.coverageComplete).toBe(false); + expect(coverage).toMatchObject({ + classification: "unverified", + primaryPath: "notes/uncached.md", + evidence: { reasons: "metadata-cache-missing" }, + }); + expect(coverage?.message).toContain("reference source"); + expect(coverage?.explanation.why).toContain("Markdown metadata"); + }); +``` + +Update the existing exact explanatory assertion from Canvas-only wording to the new general reference-source wording. + +- [ ] **Step 3: Run the focused tests and confirm failure** + +```bash +npm test -- src/tests/reference-index.test.ts src/tests/orphan-attachments.test.ts +``` + +Expected: FAIL because group backgrounds are ignored, missing Markdown cache entries do not create failures, and `ReferenceCoverageFailure.reason` does not admit `metadata-cache-missing`. + +- [ ] **Step 4: Extend the coverage failure type and record missing caches** + +In `src/scanner/reference-index.ts`, extend the reason union: + +```typescript +export type ReferenceCoverageFailure = { + path: string; + reason: + | "metadata-cache-missing" + | "malformed-json" + | "read-failed" + | "unexpected-shape"; + detail?: string; +}; +``` + +Replace the silent Markdown-cache skip with a fail-closed record: + +```typescript + for (const file of ctx.markdownFiles) { + const cache = ctx.metadataCache.getFileCache(file); + if (!cache) { + coverageFailures.push({ + path: file.path, + reason: "metadata-cache-missing", + }); + continue; + } +``` + +Do not read Markdown text or invent a fallback parser in this task. The safety contract is that missing authoritative metadata makes coverage incomplete. + +- [ ] **Step 5: Index Canvas group backgrounds** + +Extend `CanvasNode`: + +```typescript +type CanvasNode = { + type?: unknown; + file?: unknown; + background?: unknown; +}; +``` + +Replace the file-node-only branch with a target selector: + +```typescript + for (const node of nodes) { + const canvasNode = node as CanvasNode | null; + if (canvasNode === null) continue; + const target = canvasNode.type === "file" + ? canvasNode.file + : canvasNode.type === "group" + ? canvasNode.background + : undefined; + if (typeof target !== "string" || target === "") continue; + const resolved = resolveTarget(target, file.path); + if (resolved) addReference(resolved, file.path, "canvas"); + } +``` + +Retain `ReferenceSourceKind = "canvas"`; the source channel is Canvas even when the node subtype is `group`. + +- [ ] **Step 6: Generalize the coverage finding copy** + +In `src/scanner/scanners/orphan-attachments.ts`, change Canvas-only user text so mixed Markdown and Canvas failures remain truthful: + +```typescript + message: `${failedPaths.length} reference source file${failedPaths.length === 1 ? "" : "s"} could not be indexed (${reasons}); orphan results may be incomplete`, +``` + +Use this explanation: + +```typescript + ...describeFinding( + "unverified", + "Markdown metadata or Canvas reference sources could not be fully indexed, so the absence of references for some attachments is not yet trustworthy.", + "Resolve the reference coverage failures listed here, then rescan.", + ), +``` + +Also update the nearby source comment from “unresolved Canvas content” to “unindexed Markdown or Canvas sources.” Do not change the finding fingerprint shape; its sorted paths already make the failure set deterministic. + +- [ ] **Step 7: Run focused and policy tests** + +```bash +npm test -- src/tests/reference-index.test.ts src/tests/orphan-attachments.test.ts src/tests/action-policy.test.ts +``` + +Expected: PASS. Confirm that group-background attachments are not reported as orphaned and missing Markdown metadata prevents `trash-file` eligibility. + +- [ ] **Step 8: Commit the safety repair** + +```bash +git add src/scanner/reference-index.ts src/scanner/scanners/orphan-attachments.ts src/tests/reference-index.test.ts src/tests/orphan-attachments.test.ts +git commit -m "fix: fail closed on incomplete reference coverage" +``` + +--- + +### Task 3: Replace quadratic reference-source deduplication + +**Finding:** #8 reference index source dedupe is O(n²) + +**Files:** +- Modify: `src/scanner/reference-index.ts` +- Modify: `src/tests/reference-index.test.ts` + +- [ ] **Step 1: Add a failing high-degree regression test** + +At the end of `src/tests/reference-index.test.ts`, add: + +```typescript +describe("buildReferenceIndex high-degree targets", () => { + it("aggregates 50,000 unique sources within a bounded time", async () => { + const sourceCount = 50_000; + const target = makeTestFile("assets/shared.png"); + const markdownFiles = Array.from({ length: sourceCount }, (_, index) => + makeTestFile(`notes/source-${String(index).padStart(5, "0")}.md`), + ); + const metadataByPath = Object.fromEntries(markdownFiles.map((file) => [ + file.path, + { links: [mdLink(target.path)], embeds: [], frontmatterLinks: [] }, + ])); + const ctx = makeScanContext({ + files: [...markdownFiles, target], + metadataByPath, + }); + + const startedAt = performance.now(); + const index = await buildReferenceIndex(ctx); + const elapsedMs = performance.now() - startedAt; + const inbound = getInboundReference(index, target.path); + + expect(elapsedMs).toBeLessThan(5_000); + expect(inbound?.count).toBe(sourceCount); + expect(inbound?.kinds).toEqual(["note-link"]); + expect(inbound?.sources).toHaveLength(sourceCount); + expect(inbound?.sources[0]).toBe("notes/source-00000.md"); + expect(inbound?.sources.at(-1)).toBe("notes/source-49999.md"); + }, 30_000); +}); +``` + +The 30-second Vitest timeout lets the old implementation fail on the explicit 5-second bound instead of being reported as a generic test timeout. If the repository's supported Node version lacks `Array.prototype.at`, use indexed access in the assertion. + +- [ ] **Step 2: Run and confirm the performance failure** + +```bash +npm test -- src/tests/reference-index.test.ts +``` + +Expected before the refactor: the high-degree test exceeds 5 seconds because each new source scans the growing `sources` array. + +- [ ] **Step 3: Aggregate into sets and materialize the public shape once** + +In `src/scanner/reference-index.ts`, add an internal-only mutable type: + +```typescript +type MutableInboundReference = { + count: number; + kinds: Set; + sources: Set; +}; +``` + +Build into `Map`: + +```typescript + const mutableInboundByPath = new Map(); + + const addReference = ( + targetPath: string, + sourcePath: string, + kind: ReferenceSourceKind, + ): void => { + const entry = mutableInboundByPath.get(targetPath) ?? { + count: 0, + kinds: new Set(), + sources: new Set(), + }; + entry.count += 1; + entry.kinds.add(kind); + entry.sources.add(sourcePath); + mutableInboundByPath.set(targetPath, entry); + }; +``` + +After all Markdown and Canvas sources have been processed, materialize the exported arrays: + +```typescript + const inboundByPath = new Map(); + for (const [path, entry] of mutableInboundByPath) { + inboundByPath.set(path, { + count: entry.count, + kinds: [...entry.kinds].sort(), + sources: [...entry.sources].sort(), + }); + } +``` + +Remove the old `includes` checks and the final in-place sort loop. Preserve occurrence `count`: repeated links still increment the count even though `kinds` and `sources` are unique sets. + +- [ ] **Step 4: Run functional and performance tests** + +```bash +npm test -- src/tests/reference-index.test.ts src/tests/orphan-attachments.test.ts src/tests/duplicate-files.test.ts src/tests/scan-performance.test.ts +``` + +Expected: PASS with exact existing source ordering and the new 50,000-source bound. + +- [ ] **Step 5: Commit the performance repair** + +```bash +git add src/scanner/reference-index.ts src/tests/reference-index.test.ts +git commit -m "perf: bound reference index aggregation" +``` + +--- + +### Task 4: Reject stale duplicate keep decisions + +**Finding:** #1 changed reference evidence can retarget duplicate deletion after confirmation + +**Files:** +- Modify: `src/fix/fix-decisions.ts` +- Modify: `src/tests/fix-decisions.test.ts` + +- [ ] **Step 1: Make duplicate fixtures express the selected automatic keep path** + +Adjust the local `makeDuplicateIssue` helper in `src/tests/fix-decisions.test.ts` so tests can independently control `referencedPaths` and `automaticKeepPath`. Keep existing callers unchanged through defaults: + +```typescript +function makeDuplicateIssue( + fingerprint = "duplicates", + paths = ["a.md", "b.md", "c.md"], + referencedPaths: string[] = [], + automaticKeepPath = referencedPaths[0] ?? paths.slice().sort()[0], +): Issue { + return { + // retain the existing issue fields + fixAction: { + // retain kind, label, description, and targetPaths + selection: { + candidatePaths: paths, + automaticKeepPath, + referencedPaths, + requiresReview: referencedPaths.length > 1, + }, + }, + }; +} +``` + +Retain the helper's existing complete issue body; only parameterize the three selection fields. + +- [ ] **Step 2: Add failing stale-evidence tests** + +Add to `describe("getFreshFixAction", ...)`: + +```typescript + it("rejects a duplicate decision when the automatic keep path changed", () => { + const requested = makeDuplicateIssue( + "duplicates", + ["a.md", "b.md", "c.md"], + ["a.md"], + "a.md", + ); + const fresh = makeDuplicateIssue( + "duplicates", + ["a.md", "b.md", "c.md"], + ["b.md"], + "b.md", + ); + + expect(getFreshFixAction(requested, fresh, { + fingerprint: "duplicates", + keepPath: "a.md", + })).toBeNull(); + }); + + it("rejects a duplicate decision when referenced paths changed", () => { + const requested = makeDuplicateIssue( + "duplicates", + ["a.md", "b.md", "c.md"], + ["a.md"], + "a.md", + ); + const fresh = makeDuplicateIssue( + "duplicates", + ["a.md", "b.md", "c.md"], + ["a.md", "b.md"], + "a.md", + ); + + expect(getFreshFixAction(requested, fresh, { + fingerprint: "duplicates", + keepPath: "a.md", + })).toBeNull(); + }); +``` + +The second case also changes `requiresReview`; retain it because it proves the complete evidence contract. Add a third case with reordered-but-identical `referencedPaths` if needed to pin order-insensitive comparison. + +- [ ] **Step 3: Run and confirm failure** + +```bash +npm test -- src/tests/fix-decisions.test.ts +``` + +Expected: at least the automatic-keep-path case returns a non-null action under the old preflight. + +- [ ] **Step 4: Compare all selection semantics before resolving the fresh action** + +In the selection branch of `getFreshFixAction`, extend the invalidation predicate: + +```typescript + || requested.selection.automaticKeepPath + !== fresh.selection.automaticKeepPath + || !samePaths( + requested.selection.referencedPaths, + fresh.selection.referencedPaths, + ) +``` + +Keep the existing comparisons for kind, label, `requiresReview`, and `candidatePaths`. Do not recalculate or silently migrate the old decision. A changed keep recommendation or reference set requires a new confirmation modal with fresh evidence. + +- [ ] **Step 5: Run the full fix pipeline tests** + +```bash +npm test -- src/tests/fix-decisions.test.ts src/tests/fix-runner.test.ts src/tests/main.test.ts +``` + +Expected: PASS, including existing fresh-scan action checks. + +- [ ] **Step 6: Commit the stale-decision repair** + +```bash +git add src/fix/fix-decisions.ts src/tests/fix-decisions.test.ts +git commit -m "fix: revalidate duplicate keep decisions" +``` + +--- + +### Task 5: Expose an explicit single-item route for review-required fixes + +**Finding:** #4 review-required items are intentionally excluded from bulk but otherwise unreachable + +**Files:** +- Modify: `src/report/render-issues.ts` +- Modify: `src/report/InspectorView.ts` +- Modify: `src/tests/render-issue-actions.test.ts` +- Modify: `src/tests/inspector-view-filters.test.ts` + +- [ ] **Step 1: Add failing renderer tests for fix actions** + +In `src/tests/render-issue-actions.test.ts`, add: + +```typescript + it("renders an explicit review action for a review-required fix", () => { + const container = new FakeElement(); + const issue = makeFixIssueWith("review-required", "notes/file.md"); + const onFixIssue = vi.fn(); + + renderIssueList(container as any, { + issues: [issue], + scannersRun: ["broken-links"], + selectionMode: false, + selectedFingerprints: new Set(), + onOpenIssue: vi.fn(), + onToggleSelect: vi.fn(), + onFixIssue, + }); + + findByText(container, "Review fix")?.click(); + expect(onFixIssue).toHaveBeenCalledOnce(); + expect(onFixIssue).toHaveBeenCalledWith(issue); + }); + + it("does not render a fix action for blocked or non-fixable findings", () => { + for (const issue of [ + makeFixIssueWith("blocked", "blocked.md"), + makeIssue("plain.md"), + ]) { + const container = new FakeElement(); + renderIssueList(container as any, { + issues: [issue], + scannersRun: ["broken-links"], + selectionMode: false, + selectedFingerprints: new Set(), + onOpenIssue: vi.fn(), + onToggleSelect: vi.fn(), + onFixIssue: vi.fn(), + }); + expect(findByText(container, "Review fix")).toBeUndefined(); + expect(findByText(container, "Fix this issue")).toBeUndefined(); + } + }); +``` + +Also extend the existing contextual-actions test so an eligible item renders `Fix this issue`, proving the callback is a general per-item route while the label communicates whether extra review is required. + +- [ ] **Step 2: Add a failing InspectorView wiring test** + +In the active-versus-ignored callback test in `src/tests/inspector-view-filters.test.ts`: + +- capture `const onFixAllIssues = vi.fn();` +- pass it to `view.setCallbacks` +- expect the active config to contain `onFixIssue` +- expect the ignored config not to contain `onFixIssue` +- invoke `activeConfig.onFixIssue(activeIssue)` and assert `onFixAllIssues` was called with `[activeIssue]`. + +Use: + +```typescript + await activeConfig.onFixIssue(activeIssue); + expect(onFixAllIssues).toHaveBeenCalledWith([activeIssue]); +``` + +- [ ] **Step 3: Run and confirm failure** + +```bash +npm test -- src/tests/render-issue-actions.test.ts src/tests/inspector-view-filters.test.ts +``` + +Expected: FAIL because `IssueListConfig` has no `onFixIssue` callback and `InspectorView` does not wire one. + +- [ ] **Step 4: Extend the issue-list callback contract** + +In `src/report/render-issues.ts`, add: + +```typescript + onFixIssue?: (issue: Issue) => void | Promise; +``` + +In `renderIssueActions`, derive the eligibility once: + +```typescript + const eligibility = issue.fixAction ? resolveEligibility(issue) : null; + const canFixIssue = Boolean( + config.onFixIssue + && issue.fixAction + && eligibility !== "blocked", + ); +``` + +Include `!canFixIssue` in the early-return condition, then render the fix control before ignore/exclude/settings: + +```typescript + if (canFixIssue) { + createActionButton( + actions, + eligibility === "review-required" ? "Review fix" : "Fix this issue", + () => { void config.onFixIssue?.(issue); }, + ); + } +``` + +Do not change `selectBulkFixable`: review-required findings must remain excluded from one-click batch execution. + +- [ ] **Step 5: Wire only active findings to the existing confirmation pipeline** + +In the primary `renderIssueList` call in `src/report/InspectorView.ts`, add: + +```typescript + onFixIssue: (issue) => this.handleBatchAction( + this.onFixAllIssues, + [issue], + "Fixing issue", + ), +``` + +Do not add this callback to the ignored list. The existing `onFixAllIssues` implementation in `src/main.ts` already opens the confirmation modal, records explicit duplicate choices, rescans, and revalidates before execution. + +- [ ] **Step 6: Run report and integration tests** + +```bash +npm test -- src/tests/render-issue-actions.test.ts src/tests/inspector-view-filters.test.ts src/tests/confirm-modal.test.ts src/tests/main.test.ts +``` + +Expected: PASS. Verify that eligible bulk behavior is unchanged, review-required items are available only through the per-item control, and blocked/ignored rows remain non-executable. + +- [ ] **Step 7: Commit the reachable-action repair** + +```bash +git add src/report/render-issues.ts src/report/InspectorView.ts src/tests/render-issue-actions.test.ts src/tests/inspector-view-filters.test.ts +git commit -m "fix: expose reviewed fix actions" +``` + +--- + +### Task 6: Preserve a complete CLI baseline identity independent of output filters + +**Finding:** #5 filtered reports are unsound current-format baselines + +**Files:** +- Modify: `cli/cli.ts` +- Modify: `src/tests/cli.test.ts` +- Modify: `README.md` +- Modify: `skills/vault-inspector/SKILL.md` + +- [ ] **Step 1: Define the additive baseline identity contract** + +Add `fingerprints: string[]` to `CliComparison`. It is always present, sorted, unique, and computed from the full unfiltered `ScanResult` (`issues` plus `ignoredIssues`). It is baseline transport metadata, not a replacement for visible issue records. + +Current-format baseline rules after this change: + +- `comparison` absent: legacy fingerprint-only report; read only visible `issues` as today and emit the existing warning. +- `comparison` present with valid `scanProfile`, `comparisonVersion`, and `fingerprints`: current baseline; compare the complete identity set. +- `comparison` present without valid `fingerprints`: incomplete profile-aware baseline; exit `2` with no stdout and tell the user to regenerate it. Never fall back to filtered issue arrays. + +- [ ] **Step 2: Add a failing filtered-baseline regression test** + +In `src/tests/cli.test.ts`, add to the baseline comparison describe block: + +```typescript + it("preserves hidden findings when a filtered report becomes the baseline", async () => { + await withVault({ "error-source.md": "[[missing]]", "empty.md": "" }, async (vaultPath) => { + const filtered = await runCli([ + "scan", + vaultPath, + "--scanner", + "broken-links,empty-notes", + "--severity", + "error", + "--fail-on", + "none", + ]); + const filteredPayload = JSON.parse(filtered.stdout); + expect(filteredPayload.issues).toHaveLength(1); + expect(filteredPayload.comparison.fingerprints).toHaveLength(2); + + const baselinePath = join(vaultPath, "baseline.json"); + await writeFile(baselinePath, filtered.stdout, "utf8"); + const current = await runCli([ + "scan", + vaultPath, + "--scanner", + "broken-links,empty-notes", + "--baseline", + baselinePath, + "--fail-on", + "new", + ]); + const payload = JSON.parse(current.stdout); + + expect(current.exitCode).toBe(0); + expect(payload.comparison).toMatchObject({ + available: true, + mode: "profile", + newIssues: 0, + persistingIssues: 2, + resolvedIssues: 0, + }); + expect(payload.issues.every( + (issue: { isNew?: boolean }) => issue.isNew === false, + )).toBe(true); + }); + }); +``` + +Use the actual scanner severities produced by these fixtures. If broken links are not `error` in the current implementation, filter by the severity that keeps exactly one of the two findings; keep the assertion that the visible array has one record while `comparison.fingerprints` has two. + +- [ ] **Step 3: Add a failing old-current-format rejection test** + +```typescript + it("rejects a profile-aware baseline without a complete fingerprint set", async () => { + await withVault({ "empty.md": "" }, async (vaultPath) => { + const first = await runCli([ + "scan", vaultPath, "--scanner", "empty-notes", "--fail-on", "none", + ]); + const baseline = JSON.parse(first.stdout); + delete baseline.comparison.fingerprints; + const baselinePath = join(vaultPath, "baseline.json"); + await writeFile(baselinePath, JSON.stringify(baseline), "utf8"); + + const second = await runCli([ + "scan", vaultPath, "--scanner", "empty-notes", + "--baseline", baselinePath, "--fail-on", "none", + ]); + + expect(second.exitCode).toBe(2); + expect(second.stdout).toBe(""); + expect(second.stderr).toContain("complete fingerprint set"); + expect(second.stderr).toContain("Regenerate"); + }); + }); +``` + +Update every exact `payload.comparison` expectation in this file with `fingerprints: expect.any(Array)`, and add exact sorted-content checks to the no-baseline and baseline lifecycle tests. This prevents a future implementation from emitting only filtered fingerprints. + +- [ ] **Step 4: Run and confirm failure** + +```bash +npm test -- src/tests/cli.test.ts +``` + +Expected: FAIL because `comparison.fingerprints` is absent and current baselines still reconstruct their set from filtered `issues`/`ignoredIssues`. + +- [ ] **Step 5: Emit the complete identity from the unfiltered result** + +In `cli/cli.ts`, extend `CliComparison`: + +```typescript + fingerprints: string[]; +``` + +At the start of `buildCliComparison`, compute once: + +```typescript + const fingerprints = [...new Set([ + ...result.issues.map((issue) => issue.fingerprint), + ...result.ignoredIssues.map((issue) => issue.fingerprint), + ])].sort(); + const currentFingerprints = new Set(fingerprints); + const metadata = { + scanProfile, + comparisonVersion: COMPARISON_VERSION, + fingerprints, + }; +``` + +Remove the later duplicate `currentFingerprints` construction. Spread `metadata` into every comparison result, including `missing-baseline` and incompatibility results, so every newly generated JSON report is a safe future baseline regardless of output filters. + +- [ ] **Step 6: Require the identity field when reading current baselines** + +Change `isBaselineComparisonMetadata` to validate: + +```typescript +type BaselineComparisonMetadata = { + scanProfile: string; + comparisonVersion: number; + fingerprints: string[]; +}; +``` + +Validation requirements: + +- `fingerprints` is an array; +- every item is a non-empty string; +- the array is already unique (or normalize through `new Set` after validating strings); +- malformed metadata throws `Invalid baseline: comparison metadata is malformed`; +- a profile-aware object whose only missing field is `fingerprints` throws the more actionable `Invalid baseline: complete fingerprint set is missing. Regenerate the baseline with the current Vault Inspector version.` + +Build the current `BaselineReport` from `parsed.comparison.fingerprints`, not from visible issue arrays: + +```typescript + return { + kind: "current", + fingerprints: new Set(parsed.comparison.fingerprints), + scanProfile: parsed.comparison.scanProfile, + comparisonVersion: parsed.comparison.comparisonVersion, + }; +``` + +Keep the legacy branch unchanged: a report with no `comparison` object still reads only `issues`, exactly as its documented frozen behavior requires. + +- [ ] **Step 7: Document the machine-readable field and migration behavior** + +In `README.md`: + +- add `comparison.fingerprints` to the stable JSON fields; +- define it as the sorted, unique, complete unfiltered set used for later baselines; +- state that `issues` and `ignoredIssues` may be filtered for presentation but do not define current baseline completeness; +- state that profile-aware reports created before this field existed must be regenerated and exit `2` if supplied as `--baseline`; +- retain the separate legacy-mode paragraph for reports with no `comparison` object. + +In `skills/vault-inspector/SKILL.md`: + +- include `fingerprints` in the `comparison` field list; +- instruct consumers to preserve it unchanged when saving a baseline; +- prohibit rebuilding a current baseline identity from filtered visible issue arrays; +- describe the exit-2 regeneration path for incomplete profile-aware baselines. + +- [ ] **Step 8: Run CLI, package, and documentation-adjacent tests** + +```bash +npm test -- src/tests/cli.test.ts src/tests/cli-package.test.ts src/tests/version-consistency.test.ts +npm run build +node cli.js --help +npm pack --dry-run +``` + +Expected: PASS. Inspect one filtered JSON run and confirm the visible issue count can be smaller than `comparison.fingerprints.length` while baseline comparison remains correct. + +- [ ] **Step 9: Commit the baseline repair** + +```bash +git add cli/cli.ts src/tests/cli.test.ts README.md skills/vault-inspector/SKILL.md +git commit -m "fix: preserve complete CLI baseline identity" +``` + +--- + +### Task 7: Restrict automatic notices to new confirmed errors + +**Finding:** #6 warnings and info findings currently trigger automatic notices + +**Files:** +- Modify: `src/scanner/scan-scheduler.ts` +- Modify: `src/tests/scan-scheduler.test.ts` + +- [ ] **Step 1: Parameterize the scheduler issue fixture severity** + +In `src/tests/scan-scheduler.test.ts`, extend `makeIssue` without changing current callers unexpectedly: + +```typescript +function makeIssue( + fingerprint: string, + classification: Issue["classification"] = "confirmed", + severity: Issue["severity"] = "error", +): Issue { + return { + // retain all existing fields + severity, + classification, + }; +} +``` + +- [ ] **Step 2: Add a failing severity-gating test** + +Replace the first `confirmedNewIssues` test with an explicit cross-product: + +```typescript + it("returns only new confirmed errors from the active result", () => { + const error = makeIssue("error", "confirmed", "error"); + const warning = makeIssue("warning", "confirmed", "warning"); + const info = makeIssue("info", "confirmed", "info"); + const candidate = makeIssue("candidate", "candidate", "error"); + const persisting = makeIssue("persisting", "confirmed", "error"); + const result = makeResult([error, warning, info, candidate, persisting]); + + const issues = confirmedNewIssues(result, makeComparison(new Map([ + [error.fingerprint, "new"], + [warning.fingerprint, "new"], + [info.fingerprint, "new"], + [candidate.fingerprint, "new"], + [persisting.fingerprint, "persisting"], + ]))); + + expect(issues.map((issue) => issue.fingerprint)).toEqual(["error"]); + }); +``` + +Update scheduler integration fixtures that expect a notice so their selected issue has `severity: "error"`. Add an integration case with a new confirmed warning and assert `notice` is not called. + +- [ ] **Step 3: Run and confirm failure** + +```bash +npm test -- src/tests/scan-scheduler.test.ts +``` + +Expected: FAIL because the warning and info issues currently pass the filter. + +- [ ] **Step 4: Add the severity predicate** + +In `confirmedNewIssues`: + +```typescript + return result.issues.filter((issue) => + comparison.statuses.get(issue.fingerprint) === "new" + && issue.classification === "confirmed" + && issue.severity === "error"); +``` + +Update the function comment to state all three required predicates. Keep ignored issues excluded because the function reads only `result.issues`. + +- [ ] **Step 5: Run scheduler and session tests** + +```bash +npm test -- src/tests/scan-scheduler.test.ts src/tests/scan-session.test.ts +``` + +Expected: PASS. Notices remain silent when comparison is unavailable, the scan rejects, the issue is ignored, or the only new findings are warning/info/candidate/unverified. + +- [ ] **Step 6: Commit the notification repair** + +```bash +git add src/scanner/scan-scheduler.ts src/tests/scan-scheduler.test.ts +git commit -m "fix: notify only for new confirmed errors" +``` + +--- + +### Task 8: Count Markdown links as meaningful note structure + +**Finding:** #7 Markdown-link-only MOCs can be marked empty + +**Files:** +- Modify: `src/scanner/scanners/empty-notes.ts` +- Modify: `src/tests/empty-notes.test.ts` +- Modify: `src/tests/fixtures/precision-vault/notes/empty/link-only-moc.md` +- Modify: `src/tests/scanner-precision.test.ts` only if the exact expected finding set changes + +- [ ] **Step 1: Convert the precision fixture to cover both link syntaxes** + +Keep the existing wikilink-only coverage in `short-link-moc.md`. Change `src/tests/fixtures/precision-vault/notes/empty/link-only-moc.md` to a link-only Markdown MOC: + +```markdown +[Target](../target.md) +[Sibling](../hub/sibling-note.md) +``` + +This is fixture data, not product documentation. Do not add prose that would make the test pass through word count rather than structure count. + +- [ ] **Step 2: Add failing unit cases** + +In `src/tests/empty-notes.test.ts`, rename the internal-link structure test to cover wiki and Markdown links, then add: + +```typescript + expect(countMeaningfulStructures("[Target](target.md)")).toBe(1); + expect(countMeaningfulStructures("[Section](target.md#Part)")).toBe(1); + expect(countMeaningfulStructures("[External](https://example.com)")).toBe(1); + expect(countMeaningfulStructures("![alt](photo.jpg)")).toBe(1); +``` + +Add a scanner-level test using the existing context helpers: + +```typescript + it("does not report a Markdown-link-only MOC as empty", async () => { + const file = makeFile("notes/moc.md", "[Target](target.md)"); + const issues = await emptyNotesScanner.scan(makeCtx([file])); + expect(issues).toEqual([]); + }); +``` + +Adapt `makeFile`/`makeCtx` arguments to their current signatures; the assertion and link-only body are the required behavior. + +- [ ] **Step 3: Run and confirm failure** + +```bash +npm test -- src/tests/empty-notes.test.ts src/tests/scanner-precision.test.ts +``` + +Expected: FAIL because ordinary Markdown links do not increment `structureCount`, and the updated precision fixture becomes a false positive. + +- [ ] **Step 4: Count Markdown links and images exactly once** + +In `countMeaningfulStructures`, after the wikilink loop and before line-by-line block parsing, add a Markdown link loop: + +```typescript + for (const match of body.matchAll(/!?\[[^\]\r\n]*\]\(\s*(?:<[^>\r\n]+>|[^)\r\n]+)\s*\)/g)) { + void match; + count++; + } +``` + +Remove the Markdown-image half of the later line-level condition so images are not counted twice: + +```typescript + if (/ { + const css = await readFile("styles.css", "utf8"); + + expect(css).toMatch( + /\.vi-classification-badge\s*\{[^}]*align-self:\s*flex-start;/, + ); + for (const className of [ + "vi-classification-confirmed", + "vi-eligibility-eligible", + ]) { + const rule = css.match(new RegExp(`\\.${className}\\s*\\{([^}]*)\\}`))?.[1]; + expect(rule, `missing .${className}`).toBeDefined(); + expect(rule).toContain("background: var(--background-secondary)"); + expect(rule).toContain("color: var(--text-success)"); + expect(rule).toContain("border: 1px solid var(--text-success)"); + expect(rule).not.toContain("background: var(--background-modifier-success)"); + } + }); +``` + +The test intentionally avoids pixel screenshots and theme-specific computed colors. It pins the two root causes visible in the screenshot: flex-column stretching and same-family success foreground/background tokens. + +- [ ] **Step 2: Run and confirm failure** + +```bash +npm test -- src/tests/styles.test.ts +``` + +Expected: FAIL because `.vi-classification-badge` has no `align-self` and both green states use `--background-modifier-success`. + +- [ ] **Step 3: Make the status pills compact** + +Extend the existing badge declarations: + +```css +.vi-classification-badge, .vi-status-badge { + /* retain existing declarations */ +} +.vi-classification-badge { align-self: flex-start; max-width: 100%; } +``` + +`Confirmed` is a direct child of `.vi-issue-details`, whose `flex-direction: column` otherwise stretches it to the full available width. `Eligible` is already inside the target value flex row; keep its shared `flex: 0 0 auto` behavior. + +- [ ] **Step 4: Use a theme-safe success treatment** + +Replace the two affected rules: + +```css +.vi-classification-confirmed { background: var(--background-secondary); color: var(--text-success); border: 1px solid var(--text-success); } +``` + +```css +.vi-eligibility-eligible { background: var(--background-secondary); color: var(--text-success); border: 1px solid var(--text-success); } +``` + +Do not hard-code a green hex value and do not use `--background-modifier-success` with `--text-success`; some themes map those tokens to nearly identical colors. Leave candidate, unverified, review-required, blocked, and lifecycle badges unchanged unless a focused visual check proves the same defect. + +- [ ] **Step 5: Run CSS and render tests** + +```bash +npm test -- src/tests/styles.test.ts src/tests/render-evidence.test.ts src/tests/render-issue-actions.test.ts +npm run lint:obsidian-warnings +``` + +Expected: PASS. + +- [ ] **Step 6: Perform a focused Obsidian visual check** + +Using `/Users/Roger/my-vault` as the test vault: + +1. Build and load the plugin in Obsidian. +2. Open a confirmed broken-link finding with an eligible fix. +3. Confirm `Confirmed` appears as a short badge rather than a full-width bar. +4. Confirm `Eligible` is readable after the `FIX` label. +5. Repeat in the currently active theme and Obsidian's default dark theme. +6. Resize the report below 500 px and confirm neither badge overflows or obscures adjacent content. + +This is the only manual acceptance step. Do not change vault note content merely to manufacture a result; use an existing finding or a disposable test note within the authorized test vault. + +- [ ] **Step 7: Commit the UI repair** + +```bash +git add styles.css src/tests/styles.test.ts +git commit -m "fix: render status badges legibly" +``` + +--- + +### Task 10: Run the integrated regression gate and review the final diff + +**Files:** +- No planned production changes +- Modify earlier task files only if a regression test identifies a root-cause defect within this nine-item scope + +- [ ] **Step 1: Run every focused suite together** + +```bash +npm test -- src/tests/reference-index.test.ts src/tests/orphan-attachments.test.ts src/tests/action-policy.test.ts src/tests/fix-decisions.test.ts src/tests/fix-runner.test.ts src/tests/render-issue-actions.test.ts src/tests/inspector-view-filters.test.ts src/tests/cli.test.ts src/tests/scan-scheduler.test.ts src/tests/empty-notes.test.ts src/tests/scanner-precision.test.ts src/tests/scan-performance.test.ts src/tests/styles.test.ts src/tests/main.test.ts +``` + +Expected: PASS. + +- [ ] **Step 2: Run the mandatory repository gate** + +```bash +npm run lint +npm run lint:obsidian-warnings +npm run build +npm test +npm pack --dry-run +``` + +Expected: all commands exit `0`; the package contains only the documented npm assets, and Obsidian release assets remain `main.js`, `manifest.json`, and `styles.css`. + +- [ ] **Step 3: Smoke-test the shipped CLI artifact** + +```bash +node cli.js --help +node cli.js /Users/Roger/my-vault --scanner empty-notes --severity error --format json --fail-on none +``` + +Inspect the second command's JSON: + +- output is valid JSON on stdout; +- `comparison.fingerprints` is sorted and may be longer than filtered `issues` plus `ignoredIssues`; +- `comparison.available` is `false` with `reason: "missing-baseline"` when no baseline is supplied; +- no file in the vault is mutated. + +- [ ] **Step 4: Audit the diff against the nine-item matrix** + +```bash +git diff --check main...HEAD +git diff --stat main...HEAD +git log --oneline main..HEAD +git status --short +``` + +Expected: + +- no whitespace errors; +- exactly eight logical commits from the commit map; +- no version bump, release tag, new scanner, setting, or CLI command; +- no unrelated generated or vault files; +- worktree clean. + +- [ ] **Step 5: Re-read the safety invariants before PR handoff** + +Confirm from the final code and tests: + +1. no orphan trash fix exists when Markdown or Canvas reference coverage is incomplete; +2. Canvas group backgrounds contribute inbound references; +3. duplicate execution stops when automatic keep or referenced evidence changes; +4. review-required actions require an explicit single-item confirmation and fresh scan; +5. filtered presentation cannot truncate a current CLI baseline identity; +6. automatic notices contain only new confirmed errors; +7. Markdown-link-only notes are not empty; +8. reference source dedupe is set-based and output remains deterministic; +9. `Confirmed` and `Eligible` are compact, readable status badges. + +Do not merge, tag, publish, or bump the version as part of this plan unless separately requested.