Improvement wave 3: 27 verified fixes across sessions, presence, AI, a11y, settings, and the updater - #80
Conversation
…w switches
The `tail` block (InboxBoot, PairDeepLinkBoot, ContactImportDialog,
TopicGateModal) is defined once but rendered at a different child index in
each branch — index 1 on main, index 2 in settings/report/session. React
matches unkeyed children by position, so on main -> settings, settings ->
main, main -> session and report -> main it compared the tail fragment
against `<Settings/>` / `<Report/>`, found different types, and deleted and
rebuilt the whole subtree.
That remount ran InboxBoot's cleanup: `presence.leave()` broadcasts a
`{leaving:true}` goodbye (friends' dots blip offline and N3 fires a spurious
"came online" ping — the I49 symptom via a path the friend-list diff can't
defend), the rebuilt presence map starts empty so Invite buttons disappear
until the next sweep, and an invite landing in the inbox teardown window is
dropped outright since Nostr doesn't buffer.
`key="app-tail"` pins the slot, so the fiber is reused across every branch.
Session -> report and the in-session settings overlay were already preserved
by index realignment; those two are unaffected. Also corrects the now-stale
"view switches re-mount the boot" note in pairDeepLink.ts — `launchConsumed`
stays, it still guards the identity/onboarding mount.
Wave item: home-tail-and-invite-clock (04).
…on-empty `now` was seeded once at mount and only ever advanced by the 10s interval, which is armed by the same `pending.length` change that puts the first row on screen — so its first tick lands 10s AFTER the row is already visible. With close-to-tray on by default the app sits parked for hours, so a 5-minute envelope rendered as "Expires in 214 min" for those 10 seconds. That is precisely the window the arrival toast and OS notification summon the user to look in, and the countdown is the first thing they read on the row. Reseeding after the empty guard keeps it off the mount path and off the n -> 0 path; the accept path still re-validates expiry at click time, so this only ever affected the displayed number. The set-state-in-effect suppression follows the existing ones in this repo (Report, SessionsCategory, Onboarding): the reseed can't move into an adjust-during-render because `Date.now()` trips react-hooks/purity. Not covered by a test — vitest is node-env with no jsdom/RTL and the defect lives in the container's useState/useEffect wiring; the store test is unaffected. Storybook is unaffected too, since PendingInvitesView takes `now` as a controlled prop. Wave item: home-tail-and-invite-clock (06).
Closing the lid on a live session and ending it in the morning persisted the whole overnight span: wall-clock subtraction alone turned a 45-minute session into a ~600-minute bar, a fabricated streak day, and a report reading "Studied for 612 min". host/join now capture a monotonic origin alongside startedAt and the leave handler persists min(wall, monotonic). performance.now() advances on demand, so a hidden or throttled webview (the default close-to-tray flow) still reads the real awake span, and min() can only shrink an inflated number -- where a platform's monotonic clock does include suspend this degrades to today's value rather than under-counting real study time. The live footer clock applies the same rule from the store's startedAtMono so it does not read 612:34 on wake while the row says 45. Historical inflated rows are not corrected. The crash-orphan adoption path in the Rust writer is left alone: it derives its span from audit events, which stop while suspended. Item: session-minutes-monotonic
Improvements wave items 01 + 02 (same mechanism). When a friend clicks Leave they broadcast a signed 'left' audit event and await it before room.leave(), but nothing consumed it: the receiver armed the 20 s S1 grace window, sat on "Waiting for your friend to reconnect…" for someone who was never coming back, and then offered "Rejoin session" into a room nobody is in. In a two-person session whoever does not click Leave first hit this at the end of essentially every session. SessionView now marks the peerId when a verified 'left' lands (reliable because both messages ride the same ordered data channel), and wireSessionRoom skips the grace window only when EVERY departure since the last join was marked — a mixed "one leaves, one blips" three-way still waits, which is the safe way to be wrong. The end is staged as a new 'peer' reason so the Report suppresses Rejoin without overloading 'user'. peerJoined clears the mark, because trystero's peerId is process-stable and a re-invited peer would otherwise have their next genuine blip kill the session instantly. Crash / kill / tray-quit never emit 'left', so those keep today's grace window and the 'auto' + Rejoin path.
The N3 "friend came online" baseline was unbounded: the first time we resolved a friend ONLINE since subscribing never notified, so it could not tell "already online when we subscribed" from "offline then walked in hours later". With the app parked in the tray all day, that silently dropped the one event the feature exists for. Item 05 (friend-online-first-arrival). Keeps the baseline set (it is the only sweep guard that is independent of tick ordering) and bounds it with a per-friend watch-start map: subscribe time for the seed set, add time for anyone imported later, so a ContactCard import stays silent for its own settle window. Past ONLINE_WINDOW_MS (60s, the same window isOnline uses) a first online resolution is a real arrival. Strictly additive — everything that notified before still notifies. The decision moves to a pure shouldNotifyFriendOnline helper so it can be covered node-env.
The session hello was the one untrusted-name surface the PR-29/I37 sweep skipped: validateHelloPayload returned display_name verbatim, so bidi overrides and zero-width chars a peer pasted into their own name rendered as-is on every peer tile, audit row and note. normalizeUntrustedName now takes an optional byte bound and the hello passes HELLO_NAME_CAP = 64, mirroring the 64-char maxLength of our own name inputs rather than the 32-byte card NAME_CAP, so no legitimate name is truncated. Normalization applies strictly to the returned value; the signed canonical bytes and the sender side are untouched, so the wire format is unchanged and old peers still verify. The local self-tile stays unnormalized by design, so a user who pastes a bidi char into their own name still sees it reversed on their own tile while peers see it clean. Item 35.
Item ai-capture-error-latch. When a camera dies mid-session (unplugged,
OS-revoked, or grabbed by another app on Windows) getFaceTrack() keeps
returning the ended track, every tick throws CaptureError('track_ended'),
and SessionView's undeduped toast.error fires every 5-30 s for the rest
of the session — on top of the MediaErrorBanner that already says the
same thing, and in violation of the "callbacks fire at most once per loop
lifetime" contract the options block documents.
The face-track guard now also skips when readyState is 'ended', matching
the screen-track guard directly below it; both cases are "input absent",
so the tick reschedules without counting a sample (scoring is unchanged —
applyJudgment already sat inside the try that was throwing). onCaptureError
is latched behind state.captureErrorReported and cleared on the next
successful tick, mirroring sidecarErrorReported / batteryNoticeShown.
Follow-up worth tracking: the repeated toast was also the only signal for a
persistent *screen*-capture failure, whose handler sets no AI runtime status.
That path is now quieter than before. Fixing it properly needs a paired
recovery callback (the onBatteryPause/onBatteryResume shape), so it is left
out of this change.
… real
Items 13 + 38 — the same token surface.
The global focus ring is `focus-visible:ring-3 focus-visible:ring-accent-ring`
and, because `:focus-visible { outline: none }` removes the UA default, it is
the only focus affordance on most primitives. At 40% alpha it composited to
2.50-2.56:1 dark and 1.76-1.86:1 light — below SC 1.4.11's 3:1 floor and
effectively invisible on the light canvas. Raised to 60% dark (#F2B05A99) and
80% light (#8C5215CC), which measures 4.24/4.12/3.90/4.29 dark and
3.85/4.02/3.67/3.44 light over base/surface/raised/sunk. Both `tokens.ts` and
`index.css` are edited — they are hand-mirrored with no parity guard.
The ring's INNER edge on a `variant="default"` Button falls to 2.31:1 against
the amber fill (it was 3.83:1 at 40%). That is a deliberate trade: an amber
ring cannot win both edges, and the outer edge against the canvas is the one
carrying the identification load. The alternative, `ring-offset-*` across 16
call sites, is a layout change and out of scope.
check-contrast never saw any of this: the pairing registered the fully OPAQUE
accent as the foreground and reported 9.81:1 dark / 5.85:1 light. `evaluate()`
strips foreground alpha by design, so an alpha'd `fg` would be silently
ignored too; instead the ring now sits in the bg stack, where `compositeBg`
honors alpha, with the surface as foreground (contrast is symmetric). One
pairing becomes four, one per surface. `accent-default on bg-base` stays
covered by `text-accent-default on bg-base`, so coverage does not regress.
Both stale "≥4.7:1" comments that leaned on the vacuous number are corrected.
Item 38: `shadow.glow` said 3px in tokens.ts but 4px in index.css and
DESIGN-SYSTEM. tokens.ts is the one that was right — it matches the 15 shipped
`ring-3` sites — so the two stale copies come down to 3px rather than the
reverse. No rendered pixel changes; `--shadow-glow` has zero consumers. §11 now
names the mechanism the primitives actually use.
Not machine-walked: this box cannot run the Tauri app, and neither axe-core nor
any test evaluates focus-indicator contrast. Worth an eyeball in Storybook.
Item 14. DropdownMenuItem and DropdownMenuRadioItem declared their highlighted state as focus:bg-bg-raised on a DropdownMenuContent that is already bg-bg-raised — 1.00:1 in both themes, so the mid-session mic and speaker pickers gave a keyboard user no indication of which row was highlighted while arrowing through them, and mouse hover was equally dead. With two identically-named devices (the exact case the radio semantics were added to disambiguate) the user was arrowing blind. Items carry outline-hidden and the global :focus-visible reset removes the UA outline, so no fallback indicator existed. Replaces the dead fill with the house ring-3 at ring-inset (the content is overflow-clipped) in accent-default, which the existing "accent-default border on bg-raised" pairing already pins at 5.44:1 light / 8.21:1 dark. focus:text-text-primary stays — it is what the check-contrast IGNORED_COOCCURRENCES entry for this file is keyed on. focus:, not focus-visible:, because Radix moves DOM focus on pointer-move and this is the hover affordance too. The destructive row keeps a red ring to match its tint, mirroring button.tsx. Not eyeballed live: this box cannot run the Tauri app. Worth a human look at both device pickers in both themes to confirm ring-3 does not read heavy on 2-5 short rows; ring-2 is the fallback if it does.
Item 14's sibling instance, kept separate from the dropdown change. The `secondary` variants read `bg-bg-raised text-text-primary hover:bg-bg-raised` — the hover fill is byte-identical to the idle fill, so 20+ in-app secondary buttons (Report, Dashboard, every Settings category, SessionView, ModelPicker) had no hover affordance at all. Same string in badge.tsx for the anchor-badge case. hover:bg-bg-surface is the one-token fix and moves in both themes: surface is darker than raised in dark, lighter in light. check-contrast covers the six new token co-occurrences with existing pairings; no PAIRINGS edit needed. Hover pixels are not eyeballed live — this box cannot run the Tauri app.
Item 15. AuditLogPanel's viewport and SessionNotesPanel's capped list are
both bare scrolling divs whose only children are text rows — no button, no
link, nothing focusable. The notes form is a sibling of its list, not a
descendant. So both regions were pointer-only, and the read-back
AuditLogPanel's own doc comment advertises ("preserving scroll position on
read-back") was unavailable to a keyboard (WCAG 2.1.1). Before this,
`tabIndex` appeared exactly once in the whole frontend, and it was a -1.
Both get tabIndex={0} and the house focus ring, at ring-inset because a
normal ring is clipped by the pane's own overflow box. They land together
on purpose: fixing only the audit log puts a tab stop above the notes list
but not on it.
Scope note: this is a real macOS fix and a no-op on Windows — WebView2 has
auto-focused childless scrollers since Chromium 127, WKWebView has not.
Both panels gained an Overflowing story, because the shipped ones barely
scroll and axe's scrollable-region-focusable rule is inapplicable without
real overflow. Measured in the built Storybook under Playwright chromium:
audit 1124 vs 435, notes 291 vs 180. `npm run build-storybook &&
npm run check-a11y` was run before and after — 283 tests green at baseline
(the predicted `listitem` fallout does not reproduce) and 285 green now.
…e settings The v1.6.0 settings search shipped four keywords in the `advanced` bucket for settings that live in other panes: 'tray'/'minimize' (Notifications), 'capture displays' (AI), and 'auto-update' (About). Typing any of them — or pressing Enter, which selects filtered[0] — opened Advanced, where the setting does not exist, reading as if it were removed. Meanwhile Advanced's own rows (launch-at-login, data folder, share log, replay onboarding, clear history) contributed no keywords, so those queries returned "No settings match." Move each term to the pane that owns the concept and give Advanced keywords for its real rows. 'capture displays' routes to the AI pane, whose row renders only when AI is on — correct, and consistent with the other AI keywords behind the same gate. Removing 'auto-update' from Advanced also fixes "update", which was Enter-routing to Advanced instead of About; 'share log' (not bare 'diagnostics') keeps the shared "diagnostics" query on Network as before. Adds a compile-time `Record<SettingsCategoryId, readonly string[]>` guard in Settings.tsx over strings.settings.searchKeywords so a future pane without a keyword bucket fails tsc, instead of importing the id union into the leaf strings module (which would invert the existing strings -> feature dependency direction). Item 21.
…lision resetShortcutsToDefaults reset ptt-friends then ptt-ai in a fixed order and awaited each. If the user had rebound PTT-AI onto the friends-default combo (reachable through the rebind UI), the first register hit an already-registered combo, global-hotkey threw AlreadyRegistered, and the second reset never ran — ShortcutsCategory swallowed the rejection into console.error and the store's `error` field is rendered nowhere, so the button was a visual no-op. Reset the action squatting on the other's default combo first so that combo is free before re-registering it, and wrap each call in try/catch so a refusal on one binding still lets the other reset; keep and rethrow the last error. ShortcutsCategory now surfaces it via toast.error with new copy in strings.ts (settings.shortcuts.reset.resetError) — the residual fully-swapped case, which no reset order can break, is a real reject rather than a silent nothing. Adds a stateful keybindings.test.ts fake that rejects a register whose combo the other action currently holds: asserts the reorder lands both bindings on their defaults in the single-collision case, and that the second setter still runs when the first rejects. Per the item spec the Rust register-side skip is deliberately dropped — it would re-open the #47 B5 tray-idle combo swallow. Item 22.
…e banner Item 25. Settings → About is reachable mid-session (#47 B2), but its "Restart now" and "Check now" affordances had no session check, unlike UpdateReadyBanner which suppresses itself while a session is live. Restart mid-session is an unconfirmed quit that bypasses leaveBeforeQuit and the CloseRequested confirm, silently losing the session; a mid-session check pulls an installer onto the live WebRTC mesh. Enforce in the store where the node-env test harness reaches it: installAndRestart bails false when isSessionActive, and checkNow drops the now-false userInitiated exemption so every check defers during a session. AboutCategory disables both buttons when a session is active and states the reason in the help line (no info by color alone), following the shipped ModelPicker actionsLocked idiom. Flips the updater-store test that pinned the old exemption and adds an installAndRestart session-block case.
…heck ran Item 26. AboutCategory's help text used a bare else that caught both idle (the store's initial value, held for the first ~20s of every launch and after a session-deferral reset) and a background-check-failure error state, asserting "You're on X, the latest." with no check behind it — worst exactly when the update channel has silently died. Makes the help state-driven with upToDate as an explicit positive branch: idle and background failures now read step-agnostic unknown / retry copy instead of a false currency claim. The store's tested background-silence contract is untouched — this is a UI-local ternary plus three strings on a pull surface. Also relabels the status row so it no longer duplicates the read-only Version row.
classifyMnemonic collapsed a misread word and a checksum-only slip into one "Those 24 words don't add up" message, pointing at all 24 words equally on the highest-stakes screen in the app. Split the 'invalid' case into its actionable half: a module-level BIP39 Set in recoverLogic.ts (not lib/crypto, whose cross-version contract stays untouched; not features/friends' isBip39Word, which would invert an existing feature edge into a cycle) fills unknownWords on the 24-word path. The 'invalid' error now names up to 3 non-wordlist tokens (plural-aware, capped, reads correctly at the 24-token comma-paste case) and falls back to the old copy for a pure checksum failure. Words are held in Recover state beside error and cleared on edit so a later checksum-only failure never renders stale words. Adds a UnknownWord story for axe coverage and extends recoverLogic tests. Item: bip39-unknown-words.
Item 18. The trend line plots one dot per AI-scored session on a bare 1,2,3… ordinal axis and its tooltip showed only "73% focused", so a dip was un-anchorable to a day while the study-minutes bar chart stacked directly above it carries dates. Carry startedAt into the datum and add a day line to TrendTooltip, using the shared dayKey formatter so the string is byte-identical to the bar chart's. Axis stays dataKey="index" — the help copy already says the x-axis is session order.
Item 20. The Stats pane leads with the current streak and average score, but the CSV export wrote only the trailing-30-day daily minutes and per-partner counts — neither headline number, and the streak is not derivable from daily totals (it needs a single >=25-min session per day). Prepend a `summary` section (total sessions, streak, average score, scored sessions) to buildStatsCsvModel; a null average writes '' to keep the "AI off" vs "scored 0" distinction. No header or signature change, so the section-filtered assertions still pass; added a summary-section test.
Item 10. open_log_file ran after spawn_llama, so a failed log open (disk full, an AV/indexer holding the file, a permissions change on the data dir) returned Err while the freshly spawned multi-GB CommandChild was dropped without kill() and before guard.child was set. CommandChild does not kill on drop and kill_blocking then finds no child, so the orphan outlives the app. Reordering rotate -> open -> spawn front-loads the last fallible step so no failure can sit between the spawn and guard.child = Some(child).
Item 12. The watcher's budget was a 30s sliding window keyed on crash timestamps, so any death more than RESTART_WINDOW after the previous one reset the counter to 1. A model that reliably OOMs ~20s into loading therefore respawned forever: `errored` was never set, so AiCategory's "AI model crashed / Restart" row never appeared and the user got a silently dead AI. Switch the discriminator to child uptime: consecutive respawns that each died before MIN_HEALTHY_UPTIME (120s) accumulate toward RESTART_BUDGET, while a child that ran at least that long resets the streak, so a sidecar that self-heals after a long clean run is never starved. Extract next_attempts as a pure fn and pin its three branches, since the Tauri crate can't compile on the dev box.
…mport Item 29. After a corrupt app.db is set aside and recreated, DB_RECOVERED_BODY told the user "you'll need to pair with your friends again" — wrong, because the encrypted friends backup imports from the OS keychain identity, which the DB wipe never touches (only app.db and its -journal/-wal/-shm sidecars are removed). Name the actual control (Settings -> Identity -> Import friends) so a user who exported a .svfriends file restores the whole list in two clicks, and so the dialog stops asserting an out-of-band re-pairing flow is required.
The Stats focus-insights view only ever consumes ai_warning/ai_alert rows
carrying `reasoning`, yet the cross-session read shipped every audit row
(joined/left/pomodoro_*/topic_*/break_*) over IPC to be parsed and discarded.
Add `WHERE kind IN ('ai_warning', 'ai_alert')` so the read scales with what
the pane displays rather than lifetime history (~4x smaller IPC payload).
Rename the internal db fn `list_all` -> `list_ai_distractions_all` so the
narrowed contract is legible; the `audit_events_list_all` Tauri command name
is kept stable to leave the IPC surface untouched. A comment at the WHERE
clause links the kind list to its TS twins (isDistraction / reportData) since
a new distraction kind added in TS alone would silently vanish here.
Per the verifier-corrected item 19: query-narrowing only — no migration, no
indexes (they do nothing for the WHERE-less scan and only cost an irreversible
MAX_KNOWN_VERSION bump). Tests and ARCHITECTURE.md updated to match.
…lish it Item 27. release.yml runs both platform legs with fail-fast off, and publish is the last job, so a draft can survive with only the platform whose leg built (the other tripped on fetch-llama-server, a clippy break, a runner timeout). Publishing that half-built draft strands every friend on the missing platform: the plugin's check() rejects a latest.json whose platforms map has no matching key. Nothing else asserted the published manifest was complete. Adds a verify-updater-manifest job that fails hard only when latest.json is missing darwin-aarch64 or windows-x86_64, and stamps the draft's own title "INCOMPLETE, DO NOT PUBLISH" where the publish button is, since a red Actions run already trains no one. contents: write is scoped to the job (a draft is readable and editable only with push access, so contents: read would 404). if: !cancelled() plus a no-release branch keeps "both legs died before any draft existed" from misreporting as a missing platform; the success path clears the title back so a fixed re-run is publishable; and gh transport errors stay non-blocking so a chronically-red gate can't teach the reviewer to publish past it.
The six peer-binding topic/password derivations (inbox, pair, presence) plus buildPairAuthMessage are exercised only symmetrically by the integration suites, so an in-place change to a label, payload encoding, or joiner passes every gate yet strands peers on an older build after an auto-update. Pin the computed bytes so such an edit fails loud. sessionTopic is pinned for collision hygiene only and documented as transmitted, not a rendezvous contract. Item 33.
validateHelloPayload is the sole gate binding a trystero peerId to an Ed25519 pubkey (audit attribution, sessions.peer_pubkeys, note verification, tile names), yet every branch was exercised only indirectly. Cover the round-trip, the peer_id-mismatch impersonation gate, a foreign-pubkey claim, tampered name/joined_at, and the malformed-shape rejections, plus a pinned canonical-bytes assertion on serializeHelloForSig so a key-order edit — which would silently strand already-installed builds — fails loud. Item 32.
…t delivery The PR-18/I30 (from_ed_pubkey, nonce) dedup in subscribeToOwnInbox is load-bearing on the everyday path since #47 C1: the inbox races Nostr + MQTT and the guard is the sole idempotency mechanism absorbing one envelope delivered twice, yet nothing asserted it. The existing serialization test sends the same envelope twice but only awaits a promise (resolves once regardless), so it cannot catch a dedup regression. Add two cases to tests/integration/invite.test.ts, both delivering directly on the mocked inbox room via joinTopic so the concurrent window is real (sendInviteEnvelope serializes per inbox topic and never exercises it): - one envelope delivered twice concurrently fires onValidInvite once — catches the guard being removed AND an await slipping between the has/set pair, the exact regression the concurrent shape exists to find; - two distinct-nonce invites from the same friend both dispatch — catches an over-broad key that drops the nonce. Verified each bites by injecting the corresponding regression into inbox.ts. Covers items 07 and 34 (replay-guard ground); no ledger row — I30 is already marked fixed, this only adds its missing test.
… story-coverage claim §4's component inventory was the untouched V1 build spec and had drifted hard from the real tree; five code comments defer to it by name. Primitives table: add the two files it omitted (Checkbox, Skeleton) so it matches src/components/ui/ (23 files, 21 rows). Skeleton's row cites its existing §6/§10 "no spinners, use this instead" prose. Composed table: regenerate from `ls src/components/*.tsx` (31 files). Drop the `BenchmarkRunner` ghost (never built — grep finds it only in this doc) and fold its shipped behavior into a note that benchmark progress renders inline on the model-picker card; drop `FriendRow`, which is a file-local row renderer inside features/friends, not a src/components export. Move the five feature-owned surfaces that were mislabeled as src/components (FriendsList, AddFriendDialog, ModelPicker, FocusInsights, IdentityLoadErrorView) into one sentence pointing at src/features/*, which obeys the same Radix wall tree-wide and is exhibited in Storybook. Add the thirteen shipped components the table never listed. Rule 4 claimed "adding a component without a story fails the PR check" — no such gate exists (no check-stories script, ever). Rewrite it to describe what runs: check-a11y over the stories that exist in CI, with coverage a review responsibility. Preserves the V1-P2 provenance tag. Items 16, 40, 17 (doc half).
…ities inventory §11 still listed the retired `system_fetch_latest_version` command (removed in v1.5.0 when X6's tauri-plugin-updater subsumed it). Replace it with an annotated pointer that also records why the `version_check_enabled` store key is still read (never written) — deleting the bullet would strand that key looking like dead code and invite a real opt-out-carry-over regression. Item 36. §12 named three plugins the ACL does not grant (shell / global-shortcut / autostart) and omitted the ones it does (dialog / deep-link / updater), and the repo-layout tree showed a one-file capabilities dir. Transcribe the real default.json and ai-dialog.json inventories from the files, add the missing ai-dialog.json tree entry, and state the Rust-only-plugins rule instead of a stale three-item list. Item 37.
The release-line sentence asserted "v1.5.0 is the current release" while HEAD ships 1.6.0 — the same drift I45 fixed once at v1.3.1 and that has recurred three times, because the one-click release-prep flow only stages the five version files and never touches README. Rewrite the chain so every entry is past-tense provenance and CHANGELOG.md owns "whatever shipped most recently"; this also finally names v1.6.0's settings search and startup-perf work, which appeared nowhere in README. Naming v1.6.0 does not obligate a v1.7.0 append — the sentence no longer claims which release is current, so it cannot go stale again. Item 39 (docs-readme-versioning); durable rewrite only, no CLAUDE.md checklist change.
Three low-severity defects the review pass surfaced in this wave's own commits, all fixed with tests: - session end-at-once (606f583): the "every peer left deliberately" gate used a single `sawUnexplainedLeave` boolean, but a departure is explained per-peer via `departedPeerIds`. An intervening join by a *different* peer reset the flag, so a peer still absent from an earlier unexplained blip was forgotten and the room could end instantly with no grace/Rejoin. Track unexplained-absent peers in a Set, cleared per-peer on rejoin. - friend-online notify (df030f2): the first-arrival settle window reused the 60s presence-heartbeat window, so a friend who was online all along but whose presence handshake resolved >60s after launch pinged as an arrival — the boot-sweep false positive the baseline exists to prevent. Give the decision its own NOTIFY_SETTLE_MS (3 min), sized above realistic connection-establishment latency. - session hello name cap (c3d8b1d): HELLO_NAME_CAP was 64 *bytes* but our own inputs cap at 64 UTF-16 *units*, so a legitimate multibyte name (CJK, emoji) was silently truncated. Raise to 192 bytes — the worst case for a 64-unit input — and fix the comment that equated the two.
Record the improvement-wave-3 fixes: an Unreleased CHANGELOG section in the app's voice, and one ISSUES.md row per confirmed item (plus a note on the survey→verify→review process and the test-/doc-only outcomes that aren't ledgered).
|
Warning Review limit reached
Next review available in: 33 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 (5)
📝 WalkthroughWalkthroughThe PR adds release-manifest verification and updates session lifecycle, sidecar restart, audit querying, accessibility, friend notifications, identity recovery, settings, updater, statistics, and documentation behavior, with corresponding unit, integration, and Storybook coverage. ChangesRelease, documentation, and platform behavior
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
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: 5
🤖 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 @.github/workflows/release.yml:
- Around line 241-247: Update the release completeness check around the gh
release download flow to list release assets first and distinguish an absent
latest.json from genuine API or download failures. Fail the job and apply the
existing stamping behavior when the manifest is missing, while retaining the
non-blocking warning-and-exit path only for transport or API errors.
In `@ISSUES.md`:
- Around line 11-12: Update the Improvement wave 3 landing plan in ISSUES.md to
specify the repository-required squash merge strategy, replacing the
merge-commit objective while preserving the surrounding survey and fix history.
In `@README.md`:
- Around line 381-383: Update the v1.6.0 entry in the README release summary to
restore the missing verb before “a searchable settings rail,” while preserving
the surrounding release descriptions and formatting.
In `@src/features/session/SessionView.tsx`:
- Around line 725-732: The departure handling in SessionView’s verified left
branch incorrectly assumes the auditAction.send() completion guarantees peer
receipt before onPeerLeave. Replace this ordering dependency with a
request/response acknowledgment, or invoke markPeerDeparted(peerId) through the
same teardown path that emits the leave notification, ensuring deliberate
departures always take the immediate peer path.
In `@src/stories/AuditLogPanel.stories.tsx`:
- Line 76: Replace the hard-coded 480px height on the AuditLogPanel story’s
wrapper div with the existing layout design token or shared Storybook viewport
fixture, while preserving the current overflow-testing behavior and
flex/background classes.
🪄 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: aa88fea0-3c8b-41d9-bcf0-00a7bfb68840
📒 Files selected for processing (57)
.github/workflows/release.ymlARCHITECTURE.mdCHANGELOG.mdDESIGN-SYSTEM.mdISSUES.mdREADME.mdscripts/check-contrast.tssrc-tauri/src/commands/sessions.rssrc-tauri/src/commands/sidecar.rssrc-tauri/src/db/audit_events.rssrc-tauri/src/lib.rssrc/components/AuditLogPanel.tsxsrc/components/ui/badge.tsxsrc/components/ui/button.tsxsrc/components/ui/dropdown-menu.tsxsrc/design/index.csssrc/design/tokens.tssrc/features/ai/sampleLoop.tssrc/features/friends/InboxBoot.tsxsrc/features/friends/PendingInvites.tsxsrc/features/friends/contactCard.tssrc/features/friends/friendOnlineNotify.tssrc/features/friends/pairDeepLink.tssrc/features/identity/Recover.tsxsrc/features/identity/RecoverView.tsxsrc/features/identity/recoverLogic.tssrc/features/session/SessionNotesPanel.tsxsrc/features/session/SessionView.tsxsrc/features/session/hello.tssrc/features/session/host.tssrc/features/session/join.tssrc/features/session/lifecycle.tssrc/features/settings/Settings.tsxsrc/features/settings/categories/AboutCategory.tsxsrc/features/settings/categories/ShortcutsCategory.tsxsrc/features/stats/FocusInsights.tsxsrc/features/stats/statsData.tssrc/features/updater/updaterStore.tssrc/routes/Home.tsxsrc/stores/sessionStore.tssrc/stores/settingsStore.tssrc/stories/AuditLogPanel.stories.tsxsrc/stories/Recover.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/strings.tstests/integration/invite.test.tstests/unit/ai-sample-loop.test.tstests/unit/file-export.test.tstests/unit/friend-online-notify.test.tstests/unit/keybindings.test.tstests/unit/recoverLogic.test.tstests/unit/session-end-reason.test.tstests/unit/session-grace.test.tstests/unit/session-hello.test.tstests/unit/session-reentry-merge.test.tstests/unit/topics.test.tstests/unit/updater-store.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Frontend
- GitHub Check: Rust (Windows)
🧰 Additional context used
📓 Path-based instructions (14)
src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx,css}: Use design tokens instead of hard-coded visual values in frontend TypeScript, TSX, and CSS files.
Meet WCAG AA requirements for every text/background pairing in both themes; do not convey information by color alone.
Respect reduced-motion preferences through the global kill switch; new motion sites must be gated by default.
Files:
src/features/session/host.tssrc/features/settings/categories/ShortcutsCategory.tsxsrc/components/ui/badge.tsxsrc/features/session/join.tssrc/features/session/SessionNotesPanel.tsxsrc/stories/Recover.stories.tsxsrc/components/ui/button.tsxsrc/stories/AuditLogPanel.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/features/friends/contactCard.tssrc/features/friends/pairDeepLink.tssrc/design/tokens.tssrc/components/AuditLogPanel.tsxsrc/routes/Home.tsxsrc/features/friends/PendingInvites.tsxsrc/stores/settingsStore.tssrc/features/updater/updaterStore.tssrc/design/index.csssrc/features/settings/categories/AboutCategory.tsxsrc/features/settings/Settings.tsxsrc/features/session/hello.tssrc/components/ui/dropdown-menu.tsxsrc/features/identity/recoverLogic.tssrc/features/identity/Recover.tsxsrc/features/stats/statsData.tssrc/features/stats/FocusInsights.tsxsrc/features/identity/RecoverView.tsxsrc/features/session/SessionView.tsxsrc/stores/sessionStore.tssrc/features/session/lifecycle.tssrc/features/ai/sampleLoop.tssrc/strings.tssrc/features/friends/InboxBoot.tsxsrc/features/friends/friendOnlineNotify.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Keep JSX text andaria-labelliterals consistent with the voice defined inDESIGN-SYSTEM.md §14, and prefer centralized strings.
Treat peer wire formats and identity derivation as cross-version contracts; coordinate changes so older peers and existing stored data remain compatible.
Do not add telemetry; the application must remain local-only.
Files:
src/features/session/host.tssrc/features/settings/categories/ShortcutsCategory.tsxsrc/components/ui/badge.tsxsrc/features/session/join.tssrc/features/session/SessionNotesPanel.tsxsrc/stories/Recover.stories.tsxsrc/components/ui/button.tsxsrc/stories/AuditLogPanel.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/features/friends/contactCard.tssrc/features/friends/pairDeepLink.tssrc/design/tokens.tssrc/components/AuditLogPanel.tsxsrc/routes/Home.tsxsrc/features/friends/PendingInvites.tsxsrc/stores/settingsStore.tssrc/features/updater/updaterStore.tssrc/features/settings/categories/AboutCategory.tsxsrc/features/settings/Settings.tsxsrc/features/session/hello.tssrc/components/ui/dropdown-menu.tsxsrc/features/identity/recoverLogic.tssrc/features/identity/Recover.tsxsrc/features/stats/statsData.tssrc/features/stats/FocusInsights.tsxsrc/features/identity/RecoverView.tsxsrc/features/session/SessionView.tsxsrc/stores/sessionStore.tssrc/features/session/lifecycle.tssrc/features/ai/sampleLoop.tssrc/strings.tssrc/features/friends/InboxBoot.tsxsrc/features/friends/friendOnlineNotify.ts
src/**/*.{ts,tsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Never instruct users to paste a model file or BIP39 mnemonic into an AI service or chat.
Files:
src/features/session/host.tssrc/features/settings/categories/ShortcutsCategory.tsxsrc/components/ui/badge.tsxsrc/features/session/join.tssrc/features/session/SessionNotesPanel.tsxsrc/stories/Recover.stories.tsxsrc/components/ui/button.tsxsrc/stories/AuditLogPanel.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/features/friends/contactCard.tssrc/features/friends/pairDeepLink.tssrc/design/tokens.tssrc/components/AuditLogPanel.tsxsrc/routes/Home.tsxsrc/features/friends/PendingInvites.tsxsrc/stores/settingsStore.tssrc/features/updater/updaterStore.tssrc/features/settings/categories/AboutCategory.tsxsrc/features/settings/Settings.tsxsrc/features/session/hello.tssrc/components/ui/dropdown-menu.tsxsrc/features/identity/recoverLogic.tssrc/features/identity/Recover.tsxsrc/features/stats/statsData.tssrc/features/stats/FocusInsights.tsxsrc/features/identity/RecoverView.tsxsrc/features/session/SessionView.tsxsrc/stores/sessionStore.tssrc/features/session/lifecycle.tssrc/features/ai/sampleLoop.tssrc/strings.tssrc/features/friends/InboxBoot.tsxsrc/features/friends/friendOnlineNotify.ts
**/*.{ts,tsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,rs}: Add comments only when the reason is non-obvious; identifiers should carry meaning and code should read top-to-bottom.
Maintain scope discipline: do not refactor adjacent code during feature work or add abstractions for hypothetical future needs.
Files:
src/features/session/host.tssrc/features/settings/categories/ShortcutsCategory.tsxsrc/components/ui/badge.tsxtests/unit/friend-online-notify.test.tstests/unit/session-end-reason.test.tssrc/features/session/join.tssrc/features/session/SessionNotesPanel.tsxsrc-tauri/src/commands/sessions.rssrc/stories/Recover.stories.tsxsrc/components/ui/button.tsxsrc/stories/AuditLogPanel.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/features/friends/contactCard.tssrc/features/friends/pairDeepLink.tstests/unit/recoverLogic.test.tssrc/design/tokens.tssrc/components/AuditLogPanel.tsxsrc/routes/Home.tsxsrc-tauri/src/lib.rssrc/features/friends/PendingInvites.tsxsrc/stores/settingsStore.tstests/unit/file-export.test.tssrc/features/updater/updaterStore.tssrc/features/settings/categories/AboutCategory.tsxtests/unit/topics.test.tssrc/features/settings/Settings.tsxtests/unit/session-hello.test.tssrc/features/session/hello.tssrc/components/ui/dropdown-menu.tsxsrc/features/identity/recoverLogic.tssrc/features/identity/Recover.tsxscripts/check-contrast.tssrc/features/stats/statsData.tssrc/features/stats/FocusInsights.tsxsrc/features/identity/RecoverView.tsxtests/unit/updater-store.test.tssrc/features/session/SessionView.tsxtests/integration/invite.test.tstests/unit/session-reentry-merge.test.tssrc/stores/sessionStore.tssrc/features/session/lifecycle.tstests/unit/keybindings.test.tstests/unit/session-grace.test.tssrc/features/ai/sampleLoop.tstests/unit/ai-sample-loop.test.tssrc/strings.tssrc-tauri/src/commands/sidecar.rssrc-tauri/src/db/audit_events.rssrc/features/friends/InboxBoot.tsxsrc/features/friends/friendOnlineNotify.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript strict mode and ensure frontend code passes the TypeScript build and type checks.
Files:
src/features/session/host.tssrc/features/settings/categories/ShortcutsCategory.tsxsrc/components/ui/badge.tsxtests/unit/friend-online-notify.test.tstests/unit/session-end-reason.test.tssrc/features/session/join.tssrc/features/session/SessionNotesPanel.tsxsrc/stories/Recover.stories.tsxsrc/components/ui/button.tsxsrc/stories/AuditLogPanel.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/features/friends/contactCard.tssrc/features/friends/pairDeepLink.tstests/unit/recoverLogic.test.tssrc/design/tokens.tssrc/components/AuditLogPanel.tsxsrc/routes/Home.tsxsrc/features/friends/PendingInvites.tsxsrc/stores/settingsStore.tstests/unit/file-export.test.tssrc/features/updater/updaterStore.tssrc/features/settings/categories/AboutCategory.tsxtests/unit/topics.test.tssrc/features/settings/Settings.tsxtests/unit/session-hello.test.tssrc/features/session/hello.tssrc/components/ui/dropdown-menu.tsxsrc/features/identity/recoverLogic.tssrc/features/identity/Recover.tsxscripts/check-contrast.tssrc/features/stats/statsData.tssrc/features/stats/FocusInsights.tsxsrc/features/identity/RecoverView.tsxtests/unit/updater-store.test.tssrc/features/session/SessionView.tsxtests/integration/invite.test.tstests/unit/session-reentry-merge.test.tssrc/stores/sessionStore.tssrc/features/session/lifecycle.tstests/unit/keybindings.test.tstests/unit/session-grace.test.tssrc/features/ai/sampleLoop.tstests/unit/ai-sample-loop.test.tssrc/strings.tssrc/features/friends/InboxBoot.tsxsrc/features/friends/friendOnlineNotify.ts
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Before committing or opening a PR, run the required build, lint, test, design-token, strings, contrast, accessibility, formatting, and relevant Rust quality gates.
Use one focused change per commit with a Conventional Commit subject such asfeat:,fix:,chore:,docs:, orci:; PRs should squash-merge.
Files:
src/features/session/host.tssrc/features/settings/categories/ShortcutsCategory.tsxsrc/components/ui/badge.tsxtests/unit/friend-online-notify.test.tstests/unit/session-end-reason.test.tssrc/features/session/join.tssrc/features/session/SessionNotesPanel.tsxsrc-tauri/src/commands/sessions.rssrc/stories/Recover.stories.tsxsrc/components/ui/button.tsxsrc/stories/AuditLogPanel.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/features/friends/contactCard.tssrc/features/friends/pairDeepLink.tstests/unit/recoverLogic.test.tssrc/design/tokens.tssrc/components/AuditLogPanel.tsxsrc/routes/Home.tsxsrc-tauri/src/lib.rssrc/features/friends/PendingInvites.tsxsrc/stores/settingsStore.tstests/unit/file-export.test.tssrc/features/updater/updaterStore.tssrc/design/index.cssREADME.mdCHANGELOG.mdsrc/features/settings/categories/AboutCategory.tsxtests/unit/topics.test.tssrc/features/settings/Settings.tsxtests/unit/session-hello.test.tssrc/features/session/hello.tssrc/components/ui/dropdown-menu.tsxsrc/features/identity/recoverLogic.tssrc/features/identity/Recover.tsxscripts/check-contrast.tssrc/features/stats/statsData.tssrc/features/stats/FocusInsights.tsxsrc/features/identity/RecoverView.tsxtests/unit/updater-store.test.tssrc/features/session/SessionView.tsxISSUES.mdtests/integration/invite.test.tstests/unit/session-reentry-merge.test.tssrc/stores/sessionStore.tssrc/features/session/lifecycle.tstests/unit/keybindings.test.tstests/unit/session-grace.test.tssrc/features/ai/sampleLoop.tstests/unit/ai-sample-loop.test.tssrc/strings.tsDESIGN-SYSTEM.mdsrc-tauri/src/commands/sidecar.rssrc-tauri/src/db/audit_events.rssrc/features/friends/InboxBoot.tsxsrc/features/friends/friendOnlineNotify.tsARCHITECTURE.md
src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import Radix and shadcn primitives only from
src/components/ui/.
Files:
src/components/ui/badge.tsxsrc/components/ui/button.tsxsrc/components/ui/dropdown-menu.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Components under
src/components/should compose fromui/,src/design/, and shared utilities; reverse imports fromui/are prohibited.
Files:
src/components/ui/badge.tsxsrc/components/ui/button.tsxsrc/components/AuditLogPanel.tsxsrc/components/ui/dropdown-menu.tsx
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Write Vitest tests for applicable unit and integration behavior; component tests are not currently supported without an explicit test-harness scope expansion.
Files:
tests/unit/friend-online-notify.test.tstests/unit/session-end-reason.test.tstests/unit/recoverLogic.test.tstests/unit/file-export.test.tstests/unit/topics.test.tstests/unit/session-hello.test.tstests/unit/updater-store.test.tstests/integration/invite.test.tstests/unit/session-reentry-merge.test.tstests/unit/keybindings.test.tstests/unit/session-grace.test.tstests/unit/ai-sample-loop.test.ts
src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
src-tauri/**/*.rs: Preserve compatibility for Rust-side peer wire formats, identity derivation, and persisted data across manually installed updates.
For Rust changes, runcargo test,cargo fmt --check, andcargo clippybefore committing or opening a PR.
Files:
src-tauri/src/commands/sessions.rssrc-tauri/src/lib.rssrc-tauri/src/commands/sidecar.rssrc-tauri/src/db/audit_events.rs
src/**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.stories.{ts,tsx}: Provide Storybook coverage for every primitive and feature component.
Ensure every Storybook story passes the axe-core accessibility gate.
Files:
src/stories/Recover.stories.tsxsrc/stories/AuditLogPanel.stories.tsxsrc/stories/SessionNotesPanel.stories.tsx
src/design/tokens.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Define every design token—colors, spacing, fonts, radii, shadows, motion, and z-index values—in
src/design/tokens.ts; do not use raw hex values, arbitrarypx, or inlinecubic-beziervalues elsewhere.
Files:
src/design/tokens.ts
.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Use the documented release workflow or manual process to keep versions synchronized across
package.json,package-lock.json,src-tauri/Cargo.toml,src-tauri/Cargo.lock, andsrc-tauri/tauri.conf.json.
Files:
.github/workflows/release.yml
src/strings.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep toast and notification copy in
src/strings.ts; prefer this module for user-facing strings.
Files:
src/strings.ts
🪛 ast-grep (0.44.1)
tests/unit/file-export.test.ts
[warning] 141-141: Enforce overriding default config
Context: session({ id: 's1', score: 90, focused_pct: 0.7 })
Note: [CWE-1188] Insecure Default Initialization of Resource (default session cookie name not overridden).
(default-session-config-typescript)
[warning] 142-142: Enforce overriding default config
Context: session({ id: 's2', score: 70, focused_pct: 0.5 })
Note: [CWE-1188] Insecure Default Initialization of Resource (default session cookie name not overridden).
(default-session-config-typescript)
[warning] 143-143: Enforce overriding default config
Context: session({ id: 's3', score: null })
Note: [CWE-1188] Insecure Default Initialization of Resource (default session cookie name not overridden).
(default-session-config-typescript)
🪛 LanguageTool
CHANGELOG.md
[style] ~41-~41: The noun “invitation” is usually used instead of ‘invite’ in formal writing.
Context: ...ntom "came online" ping and dropping an invite that arrived in the gap. The connecti...
(AN_INVITE)
[style] ~48-~48: ‘in the meantime’ might be wordy. Consider a shorter alternative.
Context: ...g when another friend joins or leaves in the meantime. - **A friend's first arrival of the d...
(EN_WORDINESS_PREMIUM_IN_THE_MEANTIME)
🔇 Additional comments (62)
src/features/identity/Recover.tsx (1)
48-51: LGTM!Also applies to: 66-66, 88-92, 130-130
src/features/identity/RecoverView.tsx (1)
22-25: LGTM!Also applies to: 41-45, 55-57, 70-70, 207-207
src/features/identity/recoverLogic.ts (1)
1-20: LGTM!Also applies to: 34-53
src/stories/Recover.stories.tsx (1)
54-62: LGTM!tests/unit/recoverLogic.test.ts (1)
53-70: LGTM!.github/workflows/release.yml (1)
13-15: LGTM!Also applies to: 186-240, 249-263
ARCHITECTURE.md (1)
535-535: LGTM!Also applies to: 560-561, 615-616, 639-645, 719-743
CHANGELOG.md (1)
21-105: LGTM!DESIGN-SYSTEM.md (1)
54-54: LGTM!Also applies to: 127-127, 212-212, 277-285, 298-326, 357-357, 528-528
ISSUES.md (1)
13-84: LGTM!scripts/check-contrast.ts (1)
454-485: LGTM!Also applies to: 495-496, 628-630
src/features/stats/FocusInsights.tsx (3)
26-26: LGTM!
158-162: LGTM!
208-220: 🎯 Functional CorrectnessVerify tooltip dates use the stats timezone.
dayKeyaccepts an optionaltimeZone, but this call omits it. If the trend was bucketed using a user-selected timezone, timestamps near midnight can display the adjacent calendar date in the tooltip. Thread the stats timezone through the chart, or confirm thatstartedAtis already normalized to the runtime timezone.src/features/stats/statsData.ts (2)
237-241: LGTM!
250-253: LGTM!tests/unit/file-export.test.ts (2)
120-136: LGTM!
138-159: LGTM!src/features/ai/sampleLoop.ts (1)
384-389: LGTM!Also applies to: 431-431, 775-779, 876-876, 896-899
tests/unit/ai-sample-loop.test.ts (1)
152-157: LGTM!Also applies to: 983-1025, 1027-1054
src/features/friends/InboxBoot.tsx (1)
17-20: LGTM!Also applies to: 114-118, 129-140, 152-152, 166-181, 229-238
src/features/friends/friendOnlineNotify.ts (1)
3-8: LGTM!Also applies to: 18-63
tests/unit/friend-online-notify.test.ts (1)
1-150: LGTM!tests/integration/invite.test.ts (1)
124-136: LGTM!Also applies to: 546-617
src/features/friends/PendingInvites.tsx (1)
117-123: LGTM!src/features/friends/contactCard.ts (1)
199-205: LGTM!src/features/friends/pairDeepLink.ts (1)
14-19: LGTM!src/routes/Home.tsx (1)
10-20: LGTM!Also applies to: 342-373
src/components/AuditLogPanel.tsx (1)
93-103: LGTM!src/features/session/SessionNotesPanel.tsx (1)
60-66: LGTM!src/components/ui/badge.tsx (1)
14-14: LGTM!src/components/ui/button.tsx (1)
17-17: LGTM!src/components/ui/dropdown-menu.tsx (1)
60-65: LGTM!Also applies to: 81-81, 113-113
src/design/index.css (1)
23-23: LGTM!Also applies to: 44-44, 93-93
src/design/tokens.ts (1)
32-37: LGTM!Also applies to: 232-235
src/stories/AuditLogPanel.stories.tsx (1)
35-43: LGTM!Also applies to: 73-75, 77-81
src/stories/SessionNotesPanel.stories.tsx (1)
54-69: LGTM!src-tauri/src/lib.rs (1)
323-326: LGTM!src-tauri/src/commands/sessions.rs (1)
114-119: LGTM!src-tauri/src/db/audit_events.rs (1)
67-97: LGTM!Also applies to: 182-249
src-tauri/src/commands/sidecar.rs (1)
488-497: LGTM!Also applies to: 555-556, 665-680
tests/unit/session-hello.test.ts (1)
1-187: LGTM!tests/unit/topics.test.ts (1)
1-124: LGTM!src/features/settings/categories/ShortcutsCategory.tsx (1)
2-2: LGTM!Also applies to: 67-79
src/stores/settingsStore.ts (1)
1073-1099: LGTM!src/strings.ts (2)
229-243: LGTM!Also applies to: 850-898, 1740-1760
1120-1121: 🎯 Functional Correctness | ⚡ Quick win"Couldn't reset both shortcuts" overstates failure on partial success.
resetShortcutsToDefaults(settingsStore.ts) can fail on only one of the two setter calls while the other succeeds, yet still rethrowslastError— so this toast always says "both" failed even when one binding was actually reset to default.✏️ Suggested wording tweak
- resetError: (message: string) => - `Couldn't reset both shortcuts: ${message}`, + resetError: (message: string) => + `Couldn't finish resetting shortcuts: ${message}`,tests/unit/keybindings.test.ts (2)
382-465: LGTM!
379-381: 🎯 Functional CorrectnessNo duplicate
attemptsdeclaration is present.The current source has a single
let attemptsdeclaration here, so this does not break the TypeScript build.> Likely an incorrect or invalid review comment.tests/unit/updater-store.test.ts (1)
203-217: LGTM!Also applies to: 293-313
src/features/session/hello.ts (1)
11-26: LGTM!Also applies to: 102-109
src/stores/sessionStore.ts (1)
17-22: LGTM!Also applies to: 49-51, 77-79, 119-130, 157-157, 167-167, 176-176, 186-186, 206-206, 216-230, 250-255
src/features/session/lifecycle.ts (1)
177-180: LGTM!Also applies to: 198-201, 224-241, 347-353, 410-410, 419-434
src/features/session/SessionView.tsx (1)
162-162: LGTM!Also applies to: 1546-1546, 1731-1772
src/features/session/host.ts (1)
21-28: LGTM!src/features/session/join.ts (1)
26-33: LGTM!tests/unit/session-grace.test.ts (1)
177-286: LGTM!tests/unit/session-end-reason.test.ts (1)
41-52: LGTM!tests/unit/session-reentry-merge.test.ts (1)
61-159: LGTM!src/features/settings/Settings.tsx (1)
58-141: LGTM!src/features/settings/categories/AboutCategory.tsx (1)
18-18: LGTM!Also applies to: 38-38, 88-97, 122-145
src/features/updater/updaterStore.ts (1)
137-141: LGTM!Also applies to: 195-199
| # gh transport errors (or a wholly absent asset) must not redden the | ||
| # gate; only a genuinely missing platform key does. | ||
| if ! gh release download "$TAG" --pattern latest.json --clobber; then | ||
| echo "::warning::Could not fetch latest.json for '$TAG' (gh transport error or asset absent) — skipping the completeness check." \ | ||
| | tee -a "$GITHUB_STEP_SUMMARY" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail when latest.json is absent.
An absent manifest follows the same success path as a transient gh failure, so an incomplete draft remains publishable and unstamped—the exact release state this job is meant to prevent. List assets first; fail/stamp if latest.json is missing, and keep only genuine API/download failures non-blocking.
Proposed fix
+ assets="$(gh release view "$TAG" --json assets --jq '.assets[].name')" || {
+ echo "::warning::Could not list release assets for '$TAG' — skipping the completeness check." \
+ | tee -a "$GITHUB_STEP_SUMMARY"
+ exit 0
+ }
+ if ! grep -Fxq latest.json <<<"$assets"; then
+ stamp_and_fail "Draft release '$TAG' has no latest.json. Do NOT publish — re-run the failed build leg."
+ fi
+
if ! gh release download "$TAG" --pattern latest.json --clobber; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # gh transport errors (or a wholly absent asset) must not redden the | |
| # gate; only a genuinely missing platform key does. | |
| if ! gh release download "$TAG" --pattern latest.json --clobber; then | |
| echo "::warning::Could not fetch latest.json for '$TAG' (gh transport error or asset absent) — skipping the completeness check." \ | |
| | tee -a "$GITHUB_STEP_SUMMARY" | |
| exit 0 | |
| fi | |
| # gh transport errors (or a wholly absent asset) must not redden the | |
| # gate; only a genuinely missing platform key does. | |
| assets="$(gh release view "$TAG" --json assets --jq '.assets[].name')" || { | |
| echo "::warning::Could not list release assets for '$TAG' — skipping the completeness check." \ | |
| | tee -a "$GITHUB_STEP_SUMMARY" | |
| exit 0 | |
| } | |
| if ! grep -Fxq latest.json <<<"$assets"; then | |
| stamp_and_fail "Draft release '$TAG' has no latest.json. Do NOT publish — re-run the failed build leg." | |
| fi | |
| if ! gh release download "$TAG" --pattern latest.json --clobber; then | |
| echo "::warning::Could not fetch latest.json for '$TAG' (gh transport error or asset absent) — skipping the completeness check." \ | |
| | tee -a "$GITHUB_STEP_SUMMARY" | |
| exit 0 | |
| fi |
🤖 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 @.github/workflows/release.yml around lines 241 - 247, Update the release
completeness check around the gh release download flow to list release assets
first and distinguish an absent latest.json from genuine API or download
failures. Fail the job and apply the existing stamping behavior when the
manifest is missing, while retaining the non-blocking warning-and-exit path only
for transport or API errors.
| **Improvement wave 3 (post-v1.6.0, `feat/improvements-wave3`).** A 12-subsystem multi-agent survey with per-finding adversarial verification, then a multi-lens review of the branch (the three low-severity findings it confirmed were fixed on-branch). Rows I51+ record the confirmed fixes; each shipped as its own commit with tests where the harness allows. Test-only and doc-only outcomes (rendezvous-derivation vectors, signed-hello gate coverage, inbox replay coverage, and the DESIGN-SYSTEM §4 / ARCHITECTURE §11–§12 / README true-ups) are not ledgered separately. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the required squash merge strategy.
The PR objective requests a merge commit; this repository requires PRs to squash-merge. Update the landing plan before merge.
As per coding guidelines, “PRs should squash-merge.”
🤖 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 `@ISSUES.md` around lines 11 - 12, Update the Improvement wave 3 landing plan
in ISSUES.md to specify the repository-required squash merge strategy, replacing
the merge-commit objective while preserving the surrounding survey and fix
history.
Source: Coding guidelines
| the verified backlog; **v1.5.0** brought in-app auto-update; **v1.6.0** | ||
| a searchable settings rail and a lighter, faster startup. `CHANGELOG.md` | ||
| has the full history, including whatever shipped most recently. The version |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Restore the missing verb in the v1.6.0 entry.
The sentence currently reads “v1.6.0 a searchable settings rail,” which is grammatically incomplete.
Proposed fix
- the verified backlog; **v1.5.0** brought in-app auto-update; **v1.6.0**
- a searchable settings rail and a lighter, faster startup.
+ the verified backlog; **v1.5.0** brought in-app auto-update; **v1.6.0**
+ brought a searchable settings rail and a lighter, faster startup.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| the verified backlog; **v1.5.0** brought in-app auto-update; **v1.6.0** | |
| a searchable settings rail and a lighter, faster startup. `CHANGELOG.md` | |
| has the full history, including whatever shipped most recently. The version | |
| the verified backlog; **v1.5.0** brought in-app auto-update; **v1.6.0** | |
| brought a searchable settings rail and a lighter, faster startup. `CHANGELOG.md` | |
| has the full history, including whatever shipped most recently. The version |
🤖 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 `@README.md` around lines 381 - 383, Update the v1.6.0 entry in the README
release summary to restore the missing verb before “a searchable settings rail,”
while preserving the surrounding release descriptions and formatting.
| // This mark is what lets the lifecycle layer tell a deliberate | ||
| // departure from a WiFi blip, and it is reliable only because | ||
| // handleLeave awaits this broadcast before room.leave() and both ride | ||
| // the same single ordered data channel — the mark always lands before | ||
| // trystero's own leave notification. Don't drop that await. | ||
| if (verified.kind === 'left') { | ||
| useSessionStore.getState().markPeerDeparted(peerId) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
trystero webrtc strategy makeAction send resolve semantics vs onPeerLeave connection close order
💡 Result:
In Trystero, the semantics of action sending and their relation to peer lifecycle events (like onPeerLeave) are defined by the library's decentralized, peer-to-peer nature [1][2]. Action Send Semantics The makeAction() method creates an action object that supports two modes: message actions and request actions [3][4]. 1. Message Actions: The send() method transmits data to one or more peers [3][5]. It returns a Promise that resolves once the local transmission (the process of handing the data off to the underlying WebRTC data channel) is complete [5][6]. It does not wait for a network-level acknowledgment from the recipient [3][5]. 2. Request Actions: When configured with kind: 'request', the request() and requestMany() methods implement request/response semantics [3][5]. These return a Promise that resolves with the actual response(s) provided by the peer(s), or rejects if the request times out [3][5]. Connection Close and onPeerLeave Trystero operates on a decentralized architecture where peer connections are established individually [1][2]. There is no central server, so "leaving a room" is the aggregate result of individual peer connections closing [1][2]. Regarding the order of operations: - When a peer leaves (e.g., via room.leave() or a browser tab closing), the underlying WebRTC data channel closes [7][8]. - The onPeerLeave callback is triggered when the library detects that the connection to a specific peer has been lost [7][8]. - Because these are individual P2P connections, there is no guaranteed global order of disconnection across the room [1][2]. One peer may detect another's departure before others do [2]. If you send a message via send() just as a peer connection is closing, the Promise returned by send() may resolve successfully if the data was handed off to the browser's networking stack before the channel fully finalized its closure, but the data will not be received by the departing peer [3][5][9]. If the channel is already closed, attempts to send data will fail [9]. To handle this, rely on onPeerLeave to clean up state and prevent sending messages to disconnected peers [7][8].
Citations:
- 1: Local peer connected event dmotz/trystero#114
- 2: New members also receive onPeerJoin event dmotz/trystero#124
- 3: https://trystero.dev/docs/actions/
- 4: https://github.com/dmotz/trystero/releases/tag/0.25.0
- 5: https://github.com/dmotz/trystero?tab=readme-ov-file
- 6: https://www.npmjs.com/package/trystero
- 7: Callbacks and leave() seem unreliable? dmotz/trystero#55
- 8: https://github.com/dmotz/trystero
- 9: https://github.com/dmotz/trystero/blob/main/README.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Candidate files =="
git ls-files | rg '(^src/features/session/SessionView\.tsx$|lifecycle\.ts$|package\.json$|package-lock\.json$|pnpm-lock\.yaml$|yarn\.lock$)' || true
echo
echo "== SessionView context =="
if [ -f src/features/session/SessionView.tsx ]; then
nl -ba src/features/session/SessionView.tsx | sed -n '680,750p'
fi
echo
echo "== lifecycle onPeerLeave references =="
for f in $(git ls-files | rg 'lifecycle\.ts$'); do
echo "--- $f"
nl -ba "$f" | sed -n '1,260p'
done
echo
echo "== package trystero version references =="
for f in $(git ls-files | rg '(^|/)package(-lock)?\.json$|pnpm-lock\.yaml$|yarn\.lock$'); do
if rg -q 'trystero' "$f"; then
echo "--- $f"
rg -n '"?trystero"?|trystero' "$f" || true
fi
done
echo
echo "== send calls around verify/incoming audit ="
rg -n "send\\(|verifyIncomingAuditEvent|markPeerDeparted|departedPeerIds|onPeerLeave" src || trueRepository: scotej/studyvis
Length of output: 345
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Candidate files =="
git ls-files | rg '(^src/features/session/SessionView\.tsx$|lifecycle\.ts$|package\.json$|package-lock\.json$|pnpm-lock\.yaml$|yarn\.lock$)' || true
echo
echo "== SessionView context =="
if [ -f src/features/session/SessionView.tsx ]; then
nl -ba src/features/session/SessionView.tsx | sed -n '680,750p'
fi
echo
echo "== lifecycle onPeerLeave references =="
for f in $(git ls-files | rg 'lifecycle\.ts$'); do
echo "--- $f"
nl -ba "$f" | sed -n '1,260p'
done
echo
echo "== package trystero version references =="
for f in $(git ls-files | rg '(^|/)package(-lock)?\.json$|pnpm-lock\.yaml$|yarn\.lock$'); do
if rg -q 'trystero' "$f"; then
echo "--- $f"
rg -n '"?trystero"?|trystero' "$f" || true
fi
done
echo
echo "== send calls around verify/incoming audit =="
rg -n "send\\(|verifyIncomingAuditEvent|markPeerDeparted|departedPeerIds|onPeerLeave" src || trueRepository: scotej/studyvis
Length of output: 345
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SessionView context =="
if [ -f src/features/session/SessionView.tsx ]; then
awk '{printf "%6d\t%s\n", NR, $0}' src/features/session/SessionView.tsx | sed -n '690,740p'
fi
echo
echo "== lifecycle onPeerLeave references =="
for f in $(git ls-files | rg 'lifecycle\.ts$'); do
echo "--- $f"
awk '{printf "%6d\t%s\n", NR, $0}' "$f" | sed -n '1,260p'
done
echo
echo "== trystero version references =="
for f in package.json package-lock.json; do
if [ -f "$f" ]; then
echo "--- $f"
grep -n 'trystero' "$f" || true
fi
done
echo
echo "== focused send/incoming audit/lifecycle references =="
grep -RnE "send\\(|verifyIncomingAuditEvent|markPeerDeparted|departedPeerIds|onPeerLeave" src || trueRepository: scotej/studyvis
Length of output: 21775
🌐 Web query:
trystero 0.24.0 source get/send Promise resolves on receiveAck webRTC data channel
💡 Result:
In Trystero version 0.24.0, the send() method of a message action returns a Promise that resolves when the local sending process is complete, not when the receiver has acknowledged the message [1][2][3]. Trystero actions are built on top of WebRTC data channels [4][5]. The library abstracts communication into "Message actions" and "Request actions" [5][3]. 1. Message Actions: The send() method is designed for "fire-and-forget" events [5]. The returned Promise indicates that the data has been successfully sent from the local peer, but it does not wait for a formal application-level acknowledgment (ACK) from the receiver [1][2][3]. 2. Request Actions: If you require confirmation that a peer has received and processed a message, you should use a "Request action" [5][6]. By setting the action kind to 'request' (kind: 'request'), the request() method returns a Promise that resolves only when the target peer provides a response [5][6]. This mechanism naturally handles the acknowledgment and return of data [5][6]. In summary, the send() Promise in Trystero does not resolve on a receive-ACK; it resolves upon the completion of the local transmission [1][2][3]. For reliable receipt confirmation, use the request/response pattern instead [5][6].
Citations:
- 1: https://github.com/dmotz/trystero
- 2: https://github.com/dmotz/trystero/
- 3: https://github.com/dmotz/trystero?tab=readme-ov-file
- 4: https://trystero.dev/
- 5: https://trystero.dev/docs/actions/
- 6: 0.25.0 dmotz/trystero#177
🌐 Web query:
trystero 0.24.0 source onPeerClose onPeerLeave data channel closed reason code 4000
💡 Result:
In Trystero version 0.24.0, the "4000" reason code appearing when a data channel closes is not a specific Trystero-defined error code, but rather a standard WebRTC RTCDataChannel close code [1][2]. WebRTC data channel close codes in the 4000 range are typically used by browsers or the underlying WebRTC implementation to indicate that the channel was closed due to an application-level event or a non-standard error condition [1][2]. In the context of Trystero, which handles peer-to-peer signaling and connection management, a "4000" code often arises when a peer connection is terminated, either gracefully via a call to room.leave or due to a connection disruption (such as network loss, a peer closing their tab, or a timeout) [3][4][1]. When Trystero triggers onPeerLeave, it indicates that a peer has been removed from your local room instance [3][5]. While Trystero manages the signaling and connection lifecycle, the underlying WebSocket or RTCDataChannel may report a close event with code 4000 to the browser, signifying the end of the transport stream [1]. If you are seeing this code frequently, it usually suggests the expected lifecycle of the P2P connection—either intentional (leaving the room) or unintentional (network instability)—is being captured by the browser's RTC implementation [4][1]. Trystero 0.24.0, released in April 2026, focused on internal improvements including a new WebSocket relay package and more proactive announce cycles, but it did not introduce custom Trystero-specific error codes for data channel closures [6][7]. If you are encountering these closures unexpectedly, it is often related to the stability of the signaling strategy being used (e.g., Nostr, MQTT, or BitTorrent) or NAT traversal issues [8][9].
Citations:
- 1: Re-connect dmotz/trystero#29
- 2: 0.23.0 dmotz/trystero#157
- 3: https://github.com/dmotz/trystero?tab=readme-ov-file
- 4: Callbacks and leave() seem unreliable? dmotz/trystero#55
- 5: https://www.npmjs.com/package/trystero
- 6: dmotz/trystero@0357840
- 7: https://github.com/dmotz/trystero/releases/tag/0.24.0
- 8: https://github.com/dmotz/trystero/
- 9: https://www.npmjs.com/package/@trystero-p2p/core
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lifecycle.ts relevant handlers =="
for f in $(git ls-files | rg 'lifecycle\.ts$'); do
echo "--- $f"
awk '{printf "%6d\t%s\n", NR, $0}' "$f" | sed -n '400,440p'
done
echo
echo "== SessionView peerLeave/receive context =="
if [ -f src/features/session/SessionView.tsx ]; then
awk '{printf "%6d\t%s\n", NR, $0}' src/features/session/SessionView.tsx | sed -n '470,540p'
awk '{printf "%6d\t%s\n", NR, $0}' src/features/session/SessionView.tsx | sed -n '620,650p'
fi
echo
echo "== trystero wrapper context =="
if [ -f src/lib/trystero/index.ts ]; then
awk '{printf "%6d\t%s\n", NR, $0}' src/lib/trystero/index.ts | sed -n '130,170p'
awk '{printf "%6d\t%s\n", NR, $0}' src/lib/trystero/index.ts | sed -n '250,305p'
awk '{printf "%6d\t%s\n", NR, $0}' src/lib/trystero/index.ts | sed -n '335,365p'
fi
echo
echo "== sessionStore departed peer helpers =="
if [ -f src/stores/sessionStore.ts ]; then
awk '{printf "%6d\t%s\n", NR, $0}' src/stores/sessionStore.ts | sed -n '175,260p'
fiRepository: scotej/studyvis
Length of output: 14342
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SessionView handleLeave/buildAudit context =="
if [ -f src/features/session/SessionView.tsx ]; then
awk '{printf "%6d\t%s\n", NR, $0}' src/features/session/SessionView.tsx | sed -n '540,580p'
awk '{printf "%6d\t%s\n", NR, $0}' src/features/session/SessionView.tsx | sed -n '660,705p'
fi
echo
echo "== verifyIncomingAuditEvent shape =="
if [ -f src/stores/auditStore.ts ]; then
awk '{printf "%6d\t%s\n", NR, $0}' src/stores/auditStore.ts | sed -n '140,195p'
fi
echo
echo "== lifecycle armGrace/end peer helper context =="
if [ -f src/features/session/lifecycle.ts ]; then
awk '{printf "%6d\t%s\n", NR, $0}' src/features/session/lifecycle.ts | sed -n '258,325p'
fiRepository: scotej/studyvis
Length of output: 8769
Don’t rely on auditAction.send() resolving before onPeerLeave.
Trystero’s message-action send() resolves after local buffering/local transmission, not after peer receipt, while onPeerLeave is driven from connection/data-channel teardown. A deliberate leave can arrive on onPeerLeave before the left audit message is delivered, so markPeerDeparted(peerId) would be skipped and the session can fall through the 20s auto-end grace instead of taking the no-grace ‘peer’ path. Use a request/response acknowledgment for the departure mark, or add the mark through the same teardown path that triggers the leave notification.
🤖 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 `@src/features/session/SessionView.tsx` around lines 725 - 732, The departure
handling in SessionView’s verified left branch incorrectly assumes the
auditAction.send() completion guarantees peer receipt before onPeerLeave.
Replace this ordering dependency with a request/response acknowledgment, or
invoke markPeerDeparted(peerId) through the same teardown path that emits the
leave notification, ensuring deliberate departures always take the immediate
peer path.
| // Long enough to actually scroll — the keyboard-reachability case. | ||
| export const Overflowing: Story = { | ||
| render: () => ( | ||
| <div style={{ height: 480 }} className="flex bg-bg-base"> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a design token for the story viewport height.
Line 76 adds a hard-coded visual value in TSX. Use an existing layout token or shared Storybook fixture/decorator while preserving the overflow condition.
As per coding guidelines, frontend TSX should use design tokens instead of hard-coded visual values.
🤖 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 `@src/stories/AuditLogPanel.stories.tsx` at line 76, Replace the hard-coded
480px height on the AuditLogPanel story’s wrapper div with the existing layout
design token or shared Storybook viewport fixture, while preserving the current
overflow-testing behavior and flex/background classes.
Source: Coding guidelines
Textually clean merge; overlapping files (lib.rs, index.css, strings.ts, ARCHITECTURE.md, DESIGN-SYSTEM.md) touched disjoint hunks.
Improvement wave 3 — 27 verified fixes across sessions, presence, AI, a11y, settings, and the updater
The third improvement wave, run with the established process (survey → adversarial per-finding verification → one focused commit per item → multi-lens adversarial review of the branch). 31 commits; all local gates green (
build,lint,test826 passing,check-tokens,check-strings,check-contrast,format:check,cargo fmt --check,build-storybook,check-a11y286 axe checks).Please merge with a merge commit, not squash (PR #43 / #75 precedent) so the per-commit rationale survives.
How this was scoped
A 12-subsystem survey produced 51 candidates; adversarial verification kept 40 and refuted 11 (e.g. a DB-mutex-poisoning recovery, a presence-emit dedupe, and closing the stale
#77issue were all knocked down with cited reasoning). Of the 40, this branch lands the 27 that are correct, in-scope, and safe to verify by reading on this dev box; the rest are deferred below with cause.What landed
Correctness (Sev2)
<Fragment key>.total_minuteswas pure wall-clock, so a session slept on persisted the whole overnight span as study time (a free streak day + a ~10-hour bar). Nowmin(wall, monotonic); not retroactive.Sessions & presence (Sev3)
AI (Sev3)
Accessibility (Sev3)
bg-raisedonbg-raised, 1.00:1) (I58). The session-log and notes scrollers are keyboard-reachable (I59).Settings & updater (Sev3)
Added / quality-of-life
Rust backend (Sev3–4) — reviewed by reading; CI is the first compiler on the dev box
sidecar_startopens the log before spawning, so an open failure can't orphan a multi-GB llama-server (I66); the respawn budget counts consecutive short-lived deaths instead of a sliding window that could respawn forever (I67); the cross-session insights read is narrowed to AI-distraction rows, ~4× less IPC, with the Tauri command name unchanged (I68).Tests & CI
latest.jsonis missing a platform (I70).Docs — DESIGN-SYSTEM §4 inventory true-up, ARCHITECTURE §11/§12, README versioning.
Review pass
A 6-lens adversarial review (Rust compile-correctness, session, friends/AI, a11y/tokens, updater/settings/stats, tests/docs) confirmed 3 low-severity defects in this wave's own commits, all fixed in
e55d664with tests:NOTIFY_SETTLE_MS(3 min).The Rust, a11y, and updater/settings lenses returned clean.
Deferred, with cause (not dropped)
waitForVideoReadydegrade — needs a desktop smoke test (same reason as I50); can't run headless.Manual tests still owed (can't run headless here)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation