perf(desktop): channel-switch tracing + high-membership perf harness

[Maxwellimus]

Channel switching on large communities felt 1–2s slow with no way to attribute the time. This PR adds the measurement layer used to diagnose and verify the fixes in the follow-up PRs; it changes no product behavior.

Tracing (channelSwitchPerf.ts): every channel navigation records click → route commit → settled paint, with the two relay fetches that can sit on that path (message window, member roster) attributed only when they complete inside the switch window:

[switch-perf] channel=dc3f12db total=739ms commit=+397ms window=89 events in 307ms members=51 members in 273ms
[switch-perf] channel=29414326 total=24ms  commit=+19ms window=cache members=cache

Each switch also emits User Timing marks (buzz:channel-switch:*) and appends a JSONL record via a new Tauri command to {app_log_dir}/switch-perf.jsonl, stamped with the build's git revision (baked by build.rs) and an optional BUZZ_PERF_LOG_LABEL run label — so before/after sessions are attributable offline. Settle waits (bounded) for the timeline's deferred commit so render-heavy switches aren't underreported; forum surfaces abandon the trace rather than underreport; a settle for a different channel never clobbers a newer switch's trace; the active trace resets on community switch.

Harness: an inflateChannelMembers mock-bridge knob (channel name → member count, synthetic members appended on first read) and member-heavy-switch.perf.ts, measuring warm-switch wall time + longtasks for channel↔channel and channel↔Projects at baseline / 2k / 10k members with fixed message volume.

Baselines this instrument established (pre-fix)

measurement	value
mock harness, warm channel↔channel wall (median, 4× throttle)	394ms baseline → 574ms at 10k members
mock harness, longtask total per switch at 10k members	364ms
live community, click→settled paint (median, n=26)	310ms; p90 994ms
live community, roster fetch frequency	nearly every switch (30s staleness)

The follow-up PRs cut the live median to 201ms (−35%) and warm switches to 148ms (−43%), verified with this same instrument.

[Maxwellimus]

🤖 Comment from Maxwell's AI agent.

Adversarial-review findings (5 P2s), all addressed in the latest push:

• Fetch attribution start-guard: fetches attribute to a switch trace only when they started after the switch began (shouldAttributeFetch, unit-tested) — a stale A→B→A first-leg response can no longer claim the trace slot and block the real fetch's attribution.
• Trace stays attributable through the deferred-commit wait: fetches finishing inside the measured window now land in the record; a newer switch closes the previous record immediately, and a community reset drops it.
• goChannel boundary: re-selecting the active channel no longer opens a trace that could only time out (history back/forward remains deliberately untraced — documented).
• Perf sink off the measured path: append_switch_perf_log is now async + spawn_blocking, so filesystem writes never stall the main thread they're measuring.
• Log rotation: switch-perf.jsonl rotates at 10 MB keeping one prior generation (unit-tested) instead of growing unbounded.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Requesting changes at exact head a5e8e530b9be277691589b616a1264953cc79799 against base d97780b4777f2fe3430b4e30a7d47fc6837ee059.

P1 — Do not record a replaced switch from the replacement trace’s clock.

After switch A reaches settleChannelSwitchTrace() and begins waiting for its deferred render, starting switch B replaces activeTrace. A’s pending callback then calls record() whenever the replacement is non-null, and samples settledAt only at that later callback. A rapid A → B sequence can therefore charge B’s click/main-thread delay to A and emit an A “settled” mark that was never anchored to A’s painted result. This can manufacture the regression the tracer is supposed to diagnose.

Please close or cancel A at replacement time, or retain an A-specific paint completion signal, and add a rapid A → B lifecycle regression for this branch.

P1 — Serialize append and rotation per log path.

Every append runs in an independent spawn_blocking, but metadata → rename → open → append has no per-path serialization. At the 10 MiB boundary, concurrent writers can both decide to rotate; after one renames the file, the other rename fails and its frontend caller silently swallows the error, dropping a trace. Interleavings can also place records into the wrong generation.

Please serialize the complete append-and-rotation transaction and add a concurrent boundary-write test.

[Maxwellimus]

🤖 Comment from Maxwell's AI agent.

Both P1s fixed in the latest push:

P1 — replaced switch recorded from the replacement's clock. settleChannelSwitchTrace now drops the pending record whenever the active trace is no longer the settling trace — whether replaced by a rapid follow-up switch or nulled by a community reset — instead of sampling settledAt from the replacement's timeline. Better no measurement than a fabricated one; recording A against B's clock could manufacture exactly the regression this tracer exists to diagnose. Covered by three new lifecycle tests driving the rAF chain with a stubbed frame queue (channelSwitchPerf.test.mjs): rapid A→B drops A and still records B; reset drops the record; an undisturbed settle records exactly once.

P1 — unserialized append/rotation. append_line_rotating now takes a global mutex around the whole metadata→rename→append transaction (one lock is sufficient: the app writes a single log path — noted in the doc comment). New concurrent boundary test: 8 writers × 4 lines against a cap sized to cross the rotation boundary exactly once; every line must survive into the live file or the single rotated generation. Red before the mutex, green after.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Re-reviewing exact head 99b0a590035a89f72a1942a48c32b0f4279cc996 against target main@f99532585a0715bac73b4a6361a9b4966bdb5095 (PR-recorded base 0e48ff26915aa32d5f05208847b9aba75f4f19cd; merge base d97780b4777f2fe3430b4e30a7d47fc6837ee059). The previous rapid-switch clock-theft and same-process append race are fixed. The new lifecycle tests and global lock cover those cases. The remaining blockers are:

P1: replace the retained generation portably before rotating. desktop/src-tauri/src/commands/perf_log.rs:70-75 renames the live log directly onto an existing .1. On Windows, rename does not replace the destination. After the first retained generation exists, the next rollover errors before opening/appending the live file, and channelSwitchPerf.ts:112-125 intentionally swallows that error, so subsequent traces disappear without a signal. This repository already documents and tests the same platform rule in managed_agents/storage.rs:663-684 and storage_tests.rs:751-778. Remove the old .1 while holding PERF_LOG_LOCK, then rename, and add a regression that seeds both current and .1 before rotation.

P1: do not settle while the initial deferred timeline is still unpainted. MessageTimeline.tsx:265-305 knows when the deferred snapshot is stale, but its data-render-pending marker exists only inside the message-list branches (:754-760, :845-855). During the initial empty/channel-changed to loaded transition, the skeleton renders and there is no marker. channelSwitchPerf.ts:284-293 therefore sees “not pending” and records after the next frame even though the heavy deferred list may not have committed or painted. Put the readiness signal on an always-mounted channel-scoped wrapper (or expose an explicit commit signal), and cover a real empty-to-loaded browser switch that asserts the emitted CHANNEL_SWITCH_MEASURE lands after the deferred list.

P1: cancel a channel trace when navigation leaves the channel surface. Only another goChannel call replaces the singleton. goProjects, goHome, and the other non-channel navigations do not clear it, and useChannelSwitchTraceMarks.ts has no route-exit cleanup. Repro: begin A, leave for Projects before A settles, then use history back within 30 seconds. History bypasses goChannel, the returning A screen matches the stale trace, and time spent on Projects is recorded as A’s switch latency. Add route-exit cancellation plus a channel → non-channel → history-back regression.

P2: reset before asynchronous community teardown, and attribute only accepted fetches. resetCommunityState() does not clear the trace until after await resetNavigationDeepLinkDrain() (useCommunityInit.ts:59-83), so queued frame callbacks can record during teardown; rejection skips the reset entirely. Clear it before the first await. Separately, messages/hooks.ts:275-289 attributes the window fetch before reconcileFetchedChannelWindow() executes signal.throwIfAborted(). A canceled request can win the singleton windowFetch ??= slot and block the accepted replacement. Attribute only after the abort gate/reconciliation succeeds, with canceled-first/accepted-second coverage.

The PR is also currently conflicted with main, specifically the post-#6456 roster-freshness work in hooks.ts, and GitHub exposes only DCO for this head. Rebase, preserve the 5-minute roster policy while retaining attribution, rerun the perf harness against current main, and run the normal checks after resolving the code findings above.

[Maxwellimus]

🤖 Comment from Maxwell's AI agent.

All five findings from the re-review addressed in the latest push:

P1 — portable rotation over an existing .1: the retained generation is now removed (under PERF_LOG_LOCK) before the rename, matching the platform rule documented in managed_agents::storage::start_install_log_session. New regression seeds both the live file and .1 before forcing a rollover.

P1 — settle while the deferred timeline is unpainted: the data-render-pending marker moved to the timeline's always-mounted wrapper (the message-list branches' own markers are removed as redundant), so the tracer's settle-wait observes the skeleton→loaded transition too. New browser regression (switch-settle-after-paint.spec.ts): a real cold empty→loaded switch into a 600-message channel polls for CHANNEL_SWITCH_MEASURE inside the page and — in the same evaluation turn — asserts rows are painted and no deferred commit is pending at the moment the measure exists.

P1 — trace canceled on leaving the channel surface: useChannelSwitchTraceMarks adds a channel-id-keyed unmount cleanup that abandons the trace, covering goProjects/goHome/every non-goChannel exit. On A→B switches the cleanup runs with A's id after B's begin, so it only ever abandons its own trace. New lifecycle regression: begin → route-exit abandon → history-back settle records nothing.

P2 — reset ordering: resetChannelSwitchTrace() now runs before the first await in resetCommunityState(), so queued frame callbacks can't record during async teardown and a rejected drain can't skip the reset.

P2 — attribute only accepted fetches: the window fetch is attributed after reconcileFetchedChannelWindow() (whose abort gate throws first); duration still measures the fetch alone. New canceled-first/accepted-second regression asserts the accepted fetch owns the windowFetch slot in the emitted measure.

The conflict noted in the review was already resolved (branch rebased onto current main, preserving the post-#6456 roster-freshness work — CHANNEL_MEMBERS_STALE_TIME_MS intact with trace attribution layered on top). Full suite green: 5,410 unit / Rust 6 perf-log tests / smoke including the two new regressions.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Requesting changes at exact head 2a4d128a40b41c804f3c13966e60a08c4f0a8cad for three remaining instrumentation-correctness issues:

1. [P1] Do not settle while the lazy channel UI is still suspended. ChannelScreen drives settleChannelSwitchTrace from query readiness, but ChannelPane may still be behind ChannelScreenLoadingFallback. The pending marker exists only inside MessageTimeline, so the tracer interprets an absent marker as ready. In an adversarial browser run that delayed the real ChannelPane chunk, a deep-history measure landed at 331.7 ms with zero rows while the fallback was still visible; the rows mounted only after releasing the chunk. Make readiness explicit from the mounted timeline or mark the outer fallback pending, and add delayed-chunk coverage. The checked-in localhost smoke test does not exercise this interval.

2. [P1] Preserve traces through React StrictMode's effect replay. The new route-exit cleanup in useChannelSwitchTraceMarks abandons the just-opened trace during StrictMode's development-only mount cleanup, then the effect remount does not recreate it. This breaks the PR's stated dev-build/Performance-panel workflow. Reproduced against the real Vite dev runtime: one buzz:channel-switch:start mark, zero click-to-settled measures after rows painted, and switch-settle-after-paint.spec.ts timed out waiting for a settle. The production-style E2E build passes because it does not run StrictMode effect replay. Route-exit cancellation needs to distinguish an actual route/channel exit from effect replay, with dev-runtime coverage.

3. [P2] Enforce the 4 KiB cap after backend metadata is added. shape_perf_log_line checks only record_json.len(), then appends the unbounded BUZZ_PERF_LOG_LABEL. A 13-byte record plus a 1 MiB label serializes to a 1,048,615-byte JSONL line, defeating the documented defensive cap and inflating always-on telemetry I/O and retention. Bound/reject/truncate the label or validate the final serialized line, and cover it.

The prior Windows rotation, concurrent rotation, rapid replacement, deferred timeline marker, route history, pre-await teardown, and canceled-fetch attribution findings are otherwise fixed. Focused JS tests passed (23/23), git diff --check passed, and exact-head CI is green. Local Rust tests could not start because this machine lacks cmake; exact-head CI includes successful Rust and Windows jobs.

Non-code merge note: GitHub reports the PR conflicted with current main. git merge-tree finds one keep-both conflict in projectChannelWindow.test.mjs; preserve both independently appended regression tests when rebasing.

[Maxwellimus]

🤖 Comment from Maxwell's AI agent.

All three findings from the latest review addressed in e50edba, and the branch is rebased onto current main — both independently added projectChannelWindow.test.mjs regression tests preserved; the PR is mergeable again.

P1 — settle while the lazy channel UI is suspended: the ChannelPane Suspense fallback (ChannelScreenLoadingFallback) now carries data-render-pending="true" (via a layout-transparent contents wrapper), so the tracer's settle-wait observes the suspended interval the same way it observes the deferred timeline commit. New regression in switch-settle-after-paint.spec.ts intercepts and holds the ChannelPane-*.js chunk: no measure may land while it is held, and after release the settle must land with rows painted. Red before the marker, green after.

P1 — StrictMode effect replay kills the trace: route-exit abandonment is now scheduled one microtask out (scheduleRouteExitAbandon) and canceled by the effect re-setup (cancelRouteExitAbandon). StrictMode's replay runs cleanup + re-setup synchronously within one commit, so the re-setup cancels the abandon before its microtask fires; a real route exit has no re-setup, so the abandon still runs — and before any frame callback, so a queued settle can't record in the gap. New useChannelSwitchTraceMarks.test.mjs renders the hook under the real react-dom dev build in StrictMode (trace survives replay and settles exactly once; a real unmount still abandons; an A→B switch's deferred abandon of A never kills B's trace) — the first test fails against the old synchronous abandon. Also verified live: the settle spec passes against the Vite dev-server runtime, the environment the review reproduced the timeout in.

P2 — 4 KiB cap defeated by metadata: BUZZ_PERF_LOG_LABEL is truncated to 128 bytes at a char boundary (truncation rather than rejection, so a fat-fingered label can't silently drop every record for the whole run), and the cap is re-enforced on the final serialized line after the gitSha/label fold-in. Three new unit tests cover the 1 MiB label, multibyte boundary truncation, and a record that outgrows the cap only after metadata.

Housekeeping: the timeline-loading latch + trace marks moved verbatim from ChannelScreen into a new useChannelTimelineLoading hook — the rebase left ChannelScreen.tsx over the repo file-size ratchet, which forbids growth.

Validation: Rust perf_log tests 9/9 (fmt + clippy clean); desktop unit suite 5450/5450; settle spec 2/2 on the production e2e build and passing on the dev runtime; the data-render-pending-sensitive smoke specs (mentions, send-channel-binding, markdown-parse-cache) 76/76.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Requesting changes at exact head e50edba1021f202df5e5feb052afd468b3056918 against base f6e6617a9dcc2308d5039f8afaab974b49fb9577 for three remaining instrumentation-correctness issues:

1. [P2] Do not leave a trace when channel navigation is refused. goChannel calls beginChannelSwitchTrace(channelId) before commitNavigation, but commitNavigation may return early when allowNavigation rejects the switch. That leaves the target trace active even though navigation never occurred. Repro: visit B, return to A, begin editing a thread reply, click B and reject navigation, then resolve the edit and use browser Back to B within 30 seconds. History navigation is deliberately untraced, yet B's mount settles the orphan and emits an inflated measurement spanning the refused click and edit. Start the trace only after the guard accepts, before navigate, and cover refused click followed by history navigation.

2. [P2] Include the JSONL terminator in the final 4 KiB record cap. shape_perf_log_line accepts a serialized line whose length is exactly 4096 bytes, then append_line_rotating writes it with writeln!, producing a 4097-byte on-disk record. Reserve one byte for \n and add an exact-boundary test that measures the bytes written, not only the pre-write string length.

3. [P2] Make the channel↔Projects benchmark measure and report the claimed workload. The harness declares Projects ready when projects-page-header appears, while repository snapshots and work items can still be loading. Its warmup also populates both surfaces, and runScenario combines channel→Projects and Projects→channel samples into one median, which can represent neither direction and hide a one-leg regression. Gate readiness on the query-dependent Projects surface or explicitly relabel this as shell-only warm navigation, and report each direction separately.

Validation: focused JS lifecycle/fetch tests passed 29/29; git diff --check and applicable GitHub checks are green. The Rust boundary test could not run locally because the required Tauri sidecar binary is absent; exact-head Rust and Windows CI passed.

[Maxwellimus]

🤖 Comment from Maxwell's AI agent.

All three findings from the latest review addressed in 41b8159, then the full cumulative diff was re-reviewed by two independent reviewers with no authoring context (details at the end).

P2 — trace left behind by a guard-refused navigation: the trace now opens inside the shared commit flow (commitGuardedNavigation.ts), strictly after allowNavigation accepts and before navigate. The flow is extracted and dependency-injected; new unit tests pin the ordering (guard → begin → navigate), the same-destination no-op, and the requested regression: a guard-refused click followed by a history-navigation settle records nothing.

P2 — 4 KiB cap vs the newline terminator: shape_perf_log_line now reserves one byte for the \n that writeln! appends (line.len() + 1 > MAX_RECORD_BYTES). The new boundary test accepts a line at exactly 4095 bytes, appends it, asserts the file is exactly 4096 bytes on disk, and rejects one byte more.

P2 — channel↔Projects benchmark honesty: the Projects surface now exposes data-projects-hydrating on its layout root, driven by the overview's query fan (projects, work items, repo snapshots, project + repository activity summaries, local repositories — via isLoading so disabled queries can never wedge it), and the harness treats a present marker as not-ready instead of stopping at the shell header. runScenario now reports each direction as its own median — the first baseline run already justified the split (general→deep-history 363.7ms vs deep-history→general 162.7ms, previously blended). Also hardened while there: member inflation is verified on both inflated channels (the bridge silently skips unknown channel names), and general's ready-selector is prefixed (mock-general-) so the previous channel's rows can't satisfy it.

Fresh-eyes review round: after the fixes, a blind adversarial reviewer (no authoring context, prompted to refute) and codex review both re-reviewed the full base→tip diff. Codex: zero findings. The blind reviewer confirmed all three prior findings genuinely fixed and raised one new P2, also fixed in this push: a settle wait that hits its 5s deadline while the render is still pending now records with settleWaitTruncated: true (measure detail, console line, JSONL) instead of posing as an ordinary settle — keeping the >5s tail visible rather than silently truncating it. Its dead-code nit (ChannelSwitchFetchTrace) is removed.

Deferred (P3, tracked here deliberately): (a) the settle wait polls the global [data-render-pending="true"] selector, so a thread panel opened mid-switch can extend the wait (bounded by the 5s deadline, now flagged as truncated when hit) — scoping it to the channel content container is follow-up work; (b) a double-click on the same sidebar item within one render can restart the trace or open one that only times out — both err toward dropped/shortened measurements, never inflated; (c) the Projects marker omits the profile-batch query (declared below the marker; affects only the untimed warmup leg); (d) build.rs reruns when .git/logs/HEAD is absent, and gitSha holds a git describe string when tags exist — attribution-only cosmetics.

Validation: desktop unit suite 5,456/5,456 (incl. 4 new commitGuardedNavigation tests and 2 new truncation tests); Rust perf_log 10/10 with fmt + clippy clean; settle + navigation smoke specs 20/20; member-heavy perf baseline green with per-direction medians; file-size, px-text, and whitespace gates clean.

[Maxwellimus]

🤖 Comment from Maxwell's AI agent.

Since the last review response, this branch went through a multi-round adversarial review loop: 23 completed fresh-eyes reviews rotating across three models and three reasoning-effort levels, each blind to authorship, each prompted to refute, looping until three consecutive reviews found nothing. The last three rounds returned zero findings. Every fix below is its own commit with a red-first test where practical; head is 843e667, CI green.

Measurement-honesty fixes (tracer): frame-starvation guards on every phase of the settle wait (not-pending arrivals past the deadline, per-frame gap ceiling seeded at settle entry, 35s age bound); visibility accounting spanning the whole trace via one module-level visibilitychange watcher (hidden at any point between click and record drops the trace); traces dropped at the navigation layer on any committed non-channel navigation or history traversal — covering traces whose channel screen never mounted; begin() revokes pending same-channel route-exit abandons; the trace (and its start mark, via startTime) anchors at the dispatching input event's timeStamp, so input delay behind a long task is inside totalMs; User Timing buffer bounded to the latest switch across all trace outcomes.

Attribution fixes: the members roster gate is a per-channel supersession token rather than the query AbortSignal — an earlier abort-gate attempt was itself caught by a later round as a regression (reading context.signal flips React Query to cancel-and-revert on last-observer unsubscribe, discarding warm rosters on interrupted switches) and reverted.

Sink fixes (Rust): rotation is best-effort (a transiently locked rotated generation degrades to an unrotated append instead of dropping every record, with a stderr line); a permanently dead sink warns once in the console instead of being indistinguishable from "no switches traced"; the rotate-once contract is asserted unconditionally in the concurrency test; the degradation test blocks rotation with a non-empty directory so it holds under root and on Windows.

Harness fixes: stale longtask deliveries drained at sample start; settle specs assert the clean-measure contract before their own timing-budget guard and fail truncated-only runs with an explicit harness-timing message (null-safe).

Deferred by design (accepted, not missed — full 32-item ledger travels with the review harness): drop-not-fabricate biases (dropped traces emit no drop record; >3s single-frame renders drop rather than record), population scoping (deep-link and search-hit navigations anchor at goChannel; Pulse startDm untraced), eventCount = raw wire events, no-signal process suspensions bounded by the 30s entry timeout, and the cross-process rotation race mooted by tauri-plugin-single-instance.

Validation on head: desktop unit suite 5,471/5,471; Rust perf_log 11/11 (fmt + clippy clean); settle + navigation smoke specs green; member-heavy perf baseline green with per-direction medians; file-size/px/whitespace gates clean; CI 24 checks passing.