feat(ui): a reviewable surface for the index anomalies, not a run-on toast (GH #88) - #93
Conversation
…toast (GH #88) #81 shipped the anomaly diagnostic and rendered it as one space-joined paragraph in a 4.5s toast. On a real vault (1122 notes, three unrelated duplicate b2ids) that is six file paths run together in a sentence, none of them reachable, gone before it can be read, and unrecoverable afterwards. Detection was already right; only the desktop's rendering of it changes here. The split: a toast is the right *ping* and the wrong *home*. - ui/src/anomalies.ts (new, pure + node-tested) — `anomalySummary` is the ping, one line under 80 chars naming counts and the chord, no paths; `anomalyRows` is the structured review data; `anomalyKey` fingerprints the outstanding set. - A ⚠ badge in the top bar, in the persistent shell so it keeps its identity across repaints. It carries the count and survives the toast, which is the "where did that go" answer. Cleared by the next clean pass — not dismissed, *re-derived*, so it tracks the vault rather than a notification history. - A review panel (⇧⌘A, or the badge): one card per anomaly with what happened, which files, and the fix. Every path is a control — Open for a file that still holds its row, Copy path for a shadowed copy, which has no row and therefore no file-tree entry either, so a reveal there would be a button that does nothing. That last point corrects a claim the design docs and both doc-comment mirrors made ("stays on disk and in the tree"); it never did. - auto-index-on-open now surfaces its findings too. It stays toast-free (the user didn't ask for the run), but it was dropping its report entirely — and app launch is exactly when last week's Finder duplicate first gets indexed. - The fs-watch pulse pings only when the anomaly set *changes*. Pulses fire on every external save; re-toasting an unresolved collision each time is noise the badge already carries. Nothing is stored (index-engine.md §8, S2): the panel paints the latest report, a restart shows nothing until the next pass re-derives the same anomaly, and no anomaly is ever auto-fixed (W4). K1's four obligations: the badge is a real button, ⇧⌘A is in the `?` sheet, and the panel inherits the overlay focus trap. Two defects caught by rendering the panel and looking at it: the collision sentence didn't agree in number, and `direction: rtl` on a path (the trick `.vault-root` uses to keep a tail visible) reorders neutral characters at the string edge — `__DailyNotes/x.md` displayed as `DailyNotes/x.md__`. A path that reads wrong and pastes right is worse than one that wraps. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DshRBA8wRioLafsoQszxCw
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change replaces transient anomaly notices with persistent anomaly state, structured collision and restamp rows, a warning badge, and an actionable review modal. Projection reconciliation forwards reports to state adoption, while tests and documentation cover anomaly shaping and shadowed-file behavior. ChangesAnomaly review surface
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/main.ts (1)
2873-2894: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winVault-switch race can attribute a stale vault's anomalies to the newly opened vault.
reconcileExternalChangenever captures astartedRoot, and itsonReportcallback appliesadoptAnomalies(report)unconditionally. Every other call site that awaitsapi.project()guards against a concurrent vault switch before touching state — e.g. doReindex explicitly bails with "If a switch already committed (vaultRoot changed), it owns the UI — bail. (A late-finishing project is harmless host-side..." and autoIndexOnOpen does the same at line 2063. IfswitchVault()commits while this pulse'sapi.project()is in flight, the old vault'sonReportfires afterstate.anomalieswas already reset tonullfor the new vault (line 1880), repopulatingstate.anomalies/the badge/panel with paths from the vault the user just left — andopenFromAnomaly/copyAnomalyPathwould then act on paths that aren't in the active vault.🔒 Proposed fix — guard onReport the same way doReindex/autoIndexOnOpen do
async function reconcileExternalChange(): Promise<void> { if (state.vaultRoot === null) return; + const startedRoot = state.vaultRoot; await reprojectThenList({ reindexing: state.reindexing, project: api.project, list: loadNotes, onReport: (report) => { + if (state.vaultRoot !== startedRoot) return; // a switch took over — its own pass owns anomalies now const { summary, changed } = adoptAnomalies(report as IndexAnomalies); if (summary && changed) flash(summary); }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/src/main.ts` around lines 2873 - 2894, Capture the current vault root at the start of reconcileExternalChange and guard the onReport callback with it before calling adoptAnomalies. If state.vaultRoot has changed while reprojectThenList was awaiting api.project, ignore the stale report; otherwise preserve the existing anomaly adoption and flash behavior.
🧹 Nitpick comments (1)
ui/src/main.ts (1)
299-319: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the exported
REVIEW_CHORDinstead of hardcoding "⇧⌘A".
anomalies.tsexplicitly documents the risk here: "The chord that opens the review panel — quoted in the ping, so the toast tells you how to get the detail back after it clears. Its row in the?sheet (shortcuts.ts) and its wiring (main.ts) spell the same chord; keep the three in step." The badge title still spells it out literally instead of importing the constant, defeating that safeguard.♻️ Proposed fix
-import { anomalyCount, anomalyKey, anomalySummary, type IndexAnomalies } from "./anomalies"; +import { anomalyCount, anomalyKey, anomalySummary, REVIEW_CHORD, type IndexAnomalies } from "./anomalies";- badge.title = `${what} from the last index pass — review (⇧⌘A)`; + badge.title = `${what} from the last index pass — review (${REVIEW_CHORD})`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/src/main.ts` around lines 299 - 319, Update paintAnomalyBadge to import and use the exported REVIEW_CHORD constant when constructing badge.title, replacing the hardcoded “⇧⌘A” text while preserving the existing review message and aria-label behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/src/main.ts`:
- Around line 707-711: Update openSettings so it cannot open while anomalies is
active: either guard the invocation with currentOverlay() === null or clear
state.anomaliesOpen within openSettings before rendering. Preserve the existing
Settings-open behavior when no overlay is active and ensure overlay/focus-trap
state is refreshed after the change.
In `@ui/src/render.ts`:
- Around line 1365-1368: Update the anomaly rendering around anomalyRows so
anomalies === null produces a distinct “no index pass yet” message instead of
the clean-pass message. Preserve the existing anomaly list and clean-pass empty
state for non-null anomaly results, including the current fallback for undefined
anomalies.
---
Outside diff comments:
In `@ui/src/main.ts`:
- Around line 2873-2894: Capture the current vault root at the start of
reconcileExternalChange and guard the onReport callback with it before calling
adoptAnomalies. If state.vaultRoot has changed while reprojectThenList was
awaiting api.project, ignore the stale report; otherwise preserve the existing
anomaly adoption and flash behavior.
---
Nitpick comments:
In `@ui/src/main.ts`:
- Around line 299-319: Update paintAnomalyBadge to import and use the exported
REVIEW_CHORD constant when constructing badge.title, replacing the hardcoded
“⇧⌘A” text while preserving the existing review message and aria-label behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6cad1d1e-7067-4681-a6c0-4cb20fbd7f6f
📒 Files selected for processing (13)
crates/b2-core/src/ingest.rsdocs/design/index-engine.mdui/src/anomalies.test.tsui/src/anomalies.tsui/src/main.tsui/src/reconcile.test.tsui/src/reconcile.tsui/src/render.test.tsui/src/render.tsui/src/shortcuts.tsui/src/state.tsui/src/types.tsui/style.css
… pass that never ran (PR #93 review) Two findings from CodeRabbit, both confirmed against the code. - `openAnomalies` cleared `settingsOpen` but not the reverse, so ⌘, over the panel (that handler has no `currentOverlay()` guard) left both flags set: `baseOverlay` reported "settings" while `anomaliesOpen` stayed true, and closing Settings revealed the panel again — an accidental two-layer stack, which only the `?` sheet is meant to have. `openSettings` now closes the panel, which is also what the comment on `baseOverlay` already claimed. - The panel rendered `anomalies === null` as "The latest pass found none." That is worse than the review's framing: `null` is the *ordinary* state of a healthy vault, since auto-index-on-open returns early when the index is already complete — so a session that opens a good vault and touches nothing never runs a whole-vault pass, and the panel was vouching for a vault nothing had looked at. That is the silent-shadowing failure #81 exists to end, so the two states now say different things and the un-run one names the way to run a pass. Pinned by a render test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DshRBA8wRioLafsoQszxCw
2 participants
#81 shipped the anomaly diagnostic and rendered it as one space-joined paragraph
in a 4.5s toast. On a real vault (1122 notes, three unrelated duplicate b2ids)
that is six file paths run together in a sentence, none of them reachable, gone
before it can be read, and unrecoverable afterwards. Detection was already right;
only the desktop's rendering of it changes here.
The split: a toast is the right ping and the wrong home.
anomalySummaryis the ping,one line under 80 chars naming counts and the chord, no paths;
anomalyRowsis the structured review data;
anomalyKeyfingerprints the outstanding set.across repaints. It carries the count and survives the toast, which is the
"where did that go" answer. Cleared by the next clean pass — not dismissed,
re-derived, so it tracks the vault rather than a notification history.
which files, and the fix. Every path is a control — Open for a file that still
holds its row, Copy path for a shadowed copy, which has no row and therefore
no file-tree entry either, so a reveal there would be a button that does
nothing. That last point corrects a claim the design docs and both doc-comment
mirrors made ("stays on disk and in the tree"); it never did.
user didn't ask for the run), but it was dropping its report entirely — and
app launch is exactly when last week's Finder duplicate first gets indexed.
every external save; re-toasting an unresolved collision each time is noise
the badge already carries.
Nothing is stored (index-engine.md §8, S2): the panel paints the latest report,
a restart shows nothing until the next pass re-derives the same anomaly, and no
anomaly is ever auto-fixed (W4). K1's four obligations: the badge is a real
button, ⇧⌘A is in the
?sheet, and the panel inherits the overlay focus trap.Two defects caught by rendering the panel and looking at it: the collision
sentence didn't agree in number, and
direction: rtlon a path (the trick.vault-rootuses to keep a tail visible) reorders neutral characters at thestring edge —
__DailyNotes/x.mddisplayed asDailyNotes/x.md__. A path thatreads wrong and pastes right is worse than one that wraps.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01DshRBA8wRioLafsoQszxCw
Summary by CodeRabbit
New Features
Documentation
Tests