From a355a0b1d3bdc32929a57629c9d2fd006eaf7d6f Mon Sep 17 00:00:00 2001 From: Max Lampert Date: Tue, 25 Aug 2026 03:35:15 -0700 Subject: [PATCH] fix(desktop): anchor the switch trace at the input event, not handler dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blind-review round 17 (Fable high) finding: startedAt sampled performance.now() at click-handler dispatch, silently excluding input delay — a click queued behind a long task under-reported by the whole queueing time, unbounded, in exactly the contention regime the tracer exists to expose. begin() now anchors at the dispatching event's timeStamp when one is present (window.event is set only during synchronous dispatch, so stale timestamps cannot leak in from async continuations; min() guards skewed clocks). Co-Authored-By: Claude Fable 5 Signed-off-by: Max Lampert --- .../src/shared/lib/channelSwitchPerf.test.mjs | 26 +++++++++++++++++++ desktop/src/shared/lib/channelSwitchPerf.ts | 14 +++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/desktop/src/shared/lib/channelSwitchPerf.test.mjs b/desktop/src/shared/lib/channelSwitchPerf.test.mjs index b38ee5bad8..b5a3872880 100644 --- a/desktop/src/shared/lib/channelSwitchPerf.test.mjs +++ b/desktop/src/shared/lib/channelSwitchPerf.test.mjs @@ -458,6 +458,32 @@ test("a scheduled route-exit abandon canceled in the same task keeps the trace", ); }); +test("the trace anchors at the input event, not handler dispatch", async () => { + await withSettleHarness(async ({ begin, settle, flush }) => { + // Real-clock gap: earlier tests fired visibilitychange listeners, and a + // back-dated anchor overlapping those timestamps is (correctly) dropped + // by the hidden-window guard. Let them age out first. + await new Promise((resolve) => setTimeout(resolve, 600)); + // A click can sit queued behind a long task before its handler runs; + // that input delay is felt switch latency and must be inside totalMs. + // window.event is set only during synchronous dispatch, so this anchor + // can never leak in from async continuations. + globalThis.window.event = { timeStamp: performance.now() - 550 }; + begin("aaaa1111aaaa1111"); + delete globalThis.window.event; + settle("aaaa1111aaaa1111"); + flush(); + const measure = performance + .getEntriesByName("buzz:channel-switch:click-to-settled") + .at(-1); + assert.ok(measure, "measure recorded"); + assert.ok( + measure.duration >= 550, + `input delay must be inside the measure (got ${measure.duration})`, + ); + }); +}); + test("beginning a switch revokes a pending route-exit abandon for that channel", async () => { await withSettleHarness( async ({ begin, scheduleAbandon, settle, flush, measures }) => { diff --git a/desktop/src/shared/lib/channelSwitchPerf.ts b/desktop/src/shared/lib/channelSwitchPerf.ts index e1a09ea51b..9307daf19a 100644 --- a/desktop/src/shared/lib/channelSwitchPerf.ts +++ b/desktop/src/shared/lib/channelSwitchPerf.ts @@ -252,9 +252,21 @@ export function beginChannelSwitchTrace(channelId: string): void { // cleanup's scheduled abandon pending; it must not kill the fresh trace // when its microtask drains. cancelRouteExitAbandon(channelId); + // Anchor at the triggering input event when one is dispatching: a click + // can sit queued behind a long task before its handler runs, and that + // input delay is felt switch latency. window.event is set only during + // synchronous dispatch, so a stale timestamp can never leak in from async + // continuations; min() guards against skewed event clocks. + const now = performance.now(); + const dispatchingEvent = + typeof window === "undefined" ? undefined : window.event; + const startedAt = + dispatchingEvent && typeof dispatchingEvent.timeStamp === "number" + ? Math.min(dispatchingEvent.timeStamp, now) + : now; activeTrace = { channelId, - startedAt: performance.now(), + startedAt, routeCommitAt: null, windowFetch: null, membersFetch: null,