mirror of
https://github.com/block/buzz.git
synced 2026-10-30 05:51:46 +00:00
fix(desktop): guard-gated traces, on-disk log cap, honest projects benchmark
Address the three findings from the 2026-08-24 re-review of #6455, plus one finding from a follow-up adversarial review: - The switch trace now opens inside commitGuardedNavigation, only after the navigation guard accepts — a refused click can no longer leave an orphan trace that a later history navigation would settle with inflated time. The commit flow is extracted and dependency-injected; new unit tests pin guard ordering and the refusal→history-back case. - The perf-log 4 KiB cap now bounds bytes on disk: one byte is reserved for the newline writeln! appends. New boundary test measures the file. - The channel↔Projects benchmark gates readiness on a new data-projects-hydrating marker (driven by the surface's query fan via isLoading, so disabled queries never wedge it) instead of the shell header, verifies member inflation on both inflated channels, uses a prefixed ready-selector for general, and reports each switch direction as its own median so a one-leg regression cannot hide. - A settle wait that hits its 5s deadline while the render is still pending now records with settleWaitTruncated instead of posing as an honest settled paint — the >deadline tail is what the tracer exists to expose. Also removes the dead ChannelSwitchFetchTrace type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Max Lampert <maxwell@squareup.com>
This commit is contained in:
@@ -75,8 +75,9 @@ fn shape_perf_log_line(
|
||||
}
|
||||
let line = serde_json::to_string(&value).map_err(|e| e.to_string())?;
|
||||
// The cap must hold for what actually reaches the disk: gitSha and label
|
||||
// are folded in after the record-size check above.
|
||||
if line.len() > MAX_RECORD_BYTES {
|
||||
// are folded in after the record-size check above, and writeln! appends
|
||||
// a newline terminator — reserve one byte for it.
|
||||
if line.len() + 1 > MAX_RECORD_BYTES {
|
||||
return Err("perf log line too large".to_string());
|
||||
}
|
||||
Ok(line)
|
||||
@@ -217,6 +218,37 @@ mod tests {
|
||||
assert!(shape_perf_log_line(&record, Some(&"s".repeat(64)), None).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_cap_bounds_bytes_on_disk_including_the_newline() {
|
||||
// Shaped line = {"pad":"…","gitSha":null} → pad length + 24 bytes.
|
||||
// The largest accepted line is MAX_RECORD_BYTES - 1: writeln! appends
|
||||
// a newline, and the cap bounds what reaches the disk.
|
||||
let fits = format!(r#"{{"pad":"{}"}}"#, "x".repeat(MAX_RECORD_BYTES - 25));
|
||||
let line = shape_perf_log_line(&fits, None, None).expect("one byte reserved for newline");
|
||||
assert_eq!(line.len(), MAX_RECORD_BYTES - 1);
|
||||
|
||||
let dir = std::env::temp_dir().join(format!(
|
||||
"perf-log-newline-{}-{:?}",
|
||||
std::process::id(),
|
||||
std::thread::current().id()
|
||||
));
|
||||
std::fs::create_dir_all(&dir).expect("tempdir");
|
||||
let path = dir.join("switch-perf.jsonl");
|
||||
let _ = std::fs::remove_file(&path);
|
||||
append_line_rotating(&path, &line, MAX_LOG_BYTES).expect("append");
|
||||
assert_eq!(
|
||||
std::fs::metadata(&path).expect("metadata").len(),
|
||||
MAX_RECORD_BYTES as u64,
|
||||
"on-disk record must not exceed the cap"
|
||||
);
|
||||
std::fs::remove_dir_all(&dir).ok();
|
||||
|
||||
// One pad byte more serializes to exactly MAX_RECORD_BYTES, which
|
||||
// would write MAX_RECORD_BYTES + 1 bytes — rejected.
|
||||
let over = format!(r#"{{"pad":"{}"}}"#, "x".repeat(MAX_RECORD_BYTES - 24));
|
||||
assert!(shape_perf_log_line(&over, None, None).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_rotates_once_over_the_cap_and_keeps_one_generation() {
|
||||
let dir = std::env::temp_dir().join(format!("perf-log-test-{}", std::process::id()));
|
||||
|
||||
169
desktop/src/app/navigation/commitGuardedNavigation.test.mjs
Normal file
169
desktop/src/app/navigation/commitGuardedNavigation.test.mjs
Normal file
@@ -0,0 +1,169 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { commitGuardedNavigation } from "./commitGuardedNavigation.ts";
|
||||
import { registerNavigationGuard } from "./navigationGuard.ts";
|
||||
import {
|
||||
CHANNEL_SWITCH_MEASURE,
|
||||
resetChannelSwitchTrace,
|
||||
settleChannelSwitchTrace,
|
||||
} from "../../shared/lib/channelSwitchPerf.ts";
|
||||
|
||||
const route = (href) => ({ kind: "route", href });
|
||||
|
||||
test("a refused navigation opens no trace; a later history settle records nothing", async () => {
|
||||
// Frame-queue stub so the settle's rAF chain can be drained synchronously.
|
||||
const frames = [];
|
||||
const originalWindow = globalThis.window;
|
||||
const originalDocument = globalThis.document;
|
||||
globalThis.window = {
|
||||
requestAnimationFrame: (cb) => frames.push(cb) && frames.length,
|
||||
cancelAnimationFrame: () => {},
|
||||
};
|
||||
globalThis.document = { querySelector: () => null };
|
||||
performance.clearMeasures?.(CHANNEL_SWITCH_MEASURE);
|
||||
const unregister = registerNavigationGuard(() => false);
|
||||
try {
|
||||
let navigated = false;
|
||||
const committed = await commitGuardedNavigation({
|
||||
currentHref: "/channels/aaaa",
|
||||
nextHref: "/channels/bbbb",
|
||||
guardedTarget: route("/channels/bbbb"),
|
||||
traceChannelId: "bbbb",
|
||||
navigate: async () => {
|
||||
navigated = true;
|
||||
},
|
||||
});
|
||||
assert.equal(committed, false);
|
||||
assert.equal(navigated, false);
|
||||
|
||||
// Browser Back into the refused channel (history navigation is
|
||||
// deliberately untraced): its mount settles, and must find NO orphan
|
||||
// trace from the refused click — otherwise the measure would span the
|
||||
// refusal and everything the user did in between.
|
||||
settleChannelSwitchTrace("bbbb");
|
||||
for (let i = 0; i < 20 && frames.length > 0; i += 1) {
|
||||
for (const cb of frames.splice(0, frames.length)) cb();
|
||||
}
|
||||
assert.deepEqual(
|
||||
performance
|
||||
.getEntriesByName(CHANNEL_SWITCH_MEASURE)
|
||||
.map((entry) => entry.detail?.channelId),
|
||||
[],
|
||||
);
|
||||
} finally {
|
||||
unregister();
|
||||
resetChannelSwitchTrace();
|
||||
performance.clearMeasures?.(CHANNEL_SWITCH_MEASURE);
|
||||
if (originalWindow === undefined) delete globalThis.window;
|
||||
else globalThis.window = originalWindow;
|
||||
if (originalDocument === undefined) delete globalThis.document;
|
||||
else globalThis.document = originalDocument;
|
||||
}
|
||||
});
|
||||
|
||||
test("an accepted navigation opens the trace after the guard, before navigate", async () => {
|
||||
const order = [];
|
||||
const committed = await commitGuardedNavigation(
|
||||
{
|
||||
currentHref: "/channels/aaaa",
|
||||
nextHref: "/channels/bbbb",
|
||||
guardedTarget: route("/channels/bbbb"),
|
||||
traceChannelId: "bbbb",
|
||||
navigate: async () => {
|
||||
order.push("navigate");
|
||||
},
|
||||
},
|
||||
{
|
||||
allow: () => {
|
||||
order.push("guard");
|
||||
return true;
|
||||
},
|
||||
beginTrace: (channelId) => {
|
||||
order.push(`begin:${channelId}`);
|
||||
},
|
||||
},
|
||||
);
|
||||
assert.equal(committed, true);
|
||||
assert.deepEqual(order, ["guard", "begin:bbbb", "navigate"]);
|
||||
});
|
||||
|
||||
test("a same-destination no-op consults neither the guard nor the trace", async () => {
|
||||
const order = [];
|
||||
const committed = await commitGuardedNavigation(
|
||||
{
|
||||
currentHref: "/channels/aaaa",
|
||||
nextHref: "/channels/aaaa",
|
||||
guardedTarget: route("/channels/aaaa"),
|
||||
traceChannelId: "aaaa",
|
||||
navigate: async () => {
|
||||
order.push("navigate");
|
||||
},
|
||||
},
|
||||
{
|
||||
allow: () => {
|
||||
order.push("guard");
|
||||
return true;
|
||||
},
|
||||
beginTrace: () => {
|
||||
order.push("begin");
|
||||
},
|
||||
},
|
||||
);
|
||||
assert.equal(committed, false);
|
||||
assert.deepEqual(order, []);
|
||||
});
|
||||
|
||||
test("force overrides the same-destination no-op but still runs the guard first", async () => {
|
||||
const order = [];
|
||||
const committed = await commitGuardedNavigation(
|
||||
{
|
||||
currentHref: "/channels/aaaa",
|
||||
nextHref: "/channels/aaaa",
|
||||
force: true,
|
||||
guardedTarget: route("/channels/aaaa"),
|
||||
navigate: async () => {
|
||||
order.push("navigate");
|
||||
},
|
||||
},
|
||||
{
|
||||
allow: () => {
|
||||
order.push("guard");
|
||||
return true;
|
||||
},
|
||||
beginTrace: () => {
|
||||
order.push("begin");
|
||||
},
|
||||
},
|
||||
);
|
||||
assert.equal(committed, true);
|
||||
// No traceChannelId: forced re-selection of the active channel stays
|
||||
// untraced (nothing would settle it).
|
||||
assert.deepEqual(order, ["guard", "navigate"]);
|
||||
});
|
||||
|
||||
test("a same-destination navigation carrying router state still commits", async () => {
|
||||
const order = [];
|
||||
const committed = await commitGuardedNavigation(
|
||||
{
|
||||
currentHref: "/channels/aaaa",
|
||||
nextHref: "/channels/aaaa",
|
||||
guardedTarget: route("/channels/aaaa"),
|
||||
hasStateUpdate: true,
|
||||
navigate: async () => {
|
||||
order.push("navigate");
|
||||
},
|
||||
},
|
||||
{
|
||||
allow: () => {
|
||||
order.push("guard");
|
||||
return true;
|
||||
},
|
||||
beginTrace: () => {
|
||||
order.push("begin");
|
||||
},
|
||||
},
|
||||
);
|
||||
assert.equal(committed, true);
|
||||
assert.deepEqual(order, ["guard", "navigate"]);
|
||||
});
|
||||
51
desktop/src/app/navigation/commitGuardedNavigation.ts
Normal file
51
desktop/src/app/navigation/commitGuardedNavigation.ts
Normal file
@@ -0,0 +1,51 @@
|
||||
import {
|
||||
allowNavigation,
|
||||
type GuardedNavigation,
|
||||
} from "@/app/navigation/navigationGuard";
|
||||
import { beginChannelSwitchTrace } from "@/shared/lib/channelSwitchPerf";
|
||||
|
||||
/**
|
||||
* commitGuardedNavigation runs the shared commit flow for app navigations:
|
||||
* skip same-destination no-ops (unless forced or carrying a router-state
|
||||
* update), consult the navigation guard, then navigate. A same-href
|
||||
* navigation that writes state — e.g. setting or clearing the search
|
||||
* highlight — must still commit, or the state never lands. When
|
||||
* `traceChannelId` is set, the channel-switch
|
||||
* trace opens only after the guard accepts — a refused click must not leave
|
||||
* an orphan trace that a later history navigation (deliberately untraced)
|
||||
* would settle with the refused click's inflated wall time. Returns whether
|
||||
* the navigation was performed. `deps` exists for unit tests.
|
||||
*/
|
||||
export async function commitGuardedNavigation(
|
||||
input: {
|
||||
currentHref: string;
|
||||
nextHref: string;
|
||||
force?: boolean;
|
||||
guardedTarget: GuardedNavigation;
|
||||
hasStateUpdate?: boolean;
|
||||
traceChannelId?: string;
|
||||
navigate: () => Promise<unknown>;
|
||||
},
|
||||
deps: {
|
||||
allow?: typeof allowNavigation;
|
||||
beginTrace?: typeof beginChannelSwitchTrace;
|
||||
} = {},
|
||||
): Promise<boolean> {
|
||||
const allow = deps.allow ?? allowNavigation;
|
||||
const beginTrace = deps.beginTrace ?? beginChannelSwitchTrace;
|
||||
if (
|
||||
input.currentHref === input.nextHref &&
|
||||
!input.force &&
|
||||
!input.hasStateUpdate
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
if (!allow(input.guardedTarget)) {
|
||||
return false;
|
||||
}
|
||||
if (input.traceChannelId !== undefined) {
|
||||
beginTrace(input.traceChannelId);
|
||||
}
|
||||
await input.navigate();
|
||||
return true;
|
||||
}
|
||||
@@ -6,15 +6,14 @@ import {
|
||||
useRouter,
|
||||
} from "@tanstack/react-router";
|
||||
|
||||
import { commitGuardedNavigation } from "@/app/navigation/commitGuardedNavigation";
|
||||
import type { SearchHighlightNavigation } from "@/app/navigation/searchHighlightNavigation";
|
||||
import { openSearchHitWithNavigation } from "@/app/navigation/searchHitNavigation";
|
||||
import {
|
||||
allowNavigation,
|
||||
type GuardedNavigation,
|
||||
traverseHistory,
|
||||
} from "@/app/navigation/navigationGuard";
|
||||
import type { SearchHit } from "@/shared/api/types";
|
||||
import { beginChannelSwitchTrace } from "@/shared/lib/channelSwitchPerf";
|
||||
|
||||
type NavigationBehavior = {
|
||||
force?: boolean;
|
||||
@@ -42,32 +41,26 @@ export function useAppNavigation() {
|
||||
},
|
||||
behavior: NavigationBehavior = {},
|
||||
guardedTarget?: GuardedNavigation,
|
||||
traceChannelId?: string,
|
||||
) => {
|
||||
const nextLocation = router.buildLocation(next as never);
|
||||
const hasStateUpdate = next.state !== undefined;
|
||||
|
||||
if (
|
||||
location.href === nextLocation.href &&
|
||||
!behavior.force &&
|
||||
!hasStateUpdate
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (
|
||||
!allowNavigation(
|
||||
guardedTarget ?? { kind: "route", href: nextLocation.href },
|
||||
)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
await navigate({
|
||||
...next,
|
||||
replace: behavior.replace,
|
||||
resetScroll: behavior.resetScroll,
|
||||
} as never);
|
||||
return true;
|
||||
return commitGuardedNavigation({
|
||||
currentHref: location.href,
|
||||
force: behavior.force,
|
||||
guardedTarget: guardedTarget ?? {
|
||||
kind: "route",
|
||||
href: nextLocation.href,
|
||||
},
|
||||
hasStateUpdate: next.state !== undefined,
|
||||
navigate: () =>
|
||||
navigate({
|
||||
...next,
|
||||
replace: behavior.replace,
|
||||
resetScroll: behavior.resetScroll,
|
||||
} as never),
|
||||
nextHref: nextLocation.href,
|
||||
traceChannelId,
|
||||
});
|
||||
},
|
||||
[location.href, navigate, router],
|
||||
);
|
||||
@@ -287,16 +280,6 @@ export function useAppNavigation() {
|
||||
threadRootId?: string | null;
|
||||
},
|
||||
) => {
|
||||
// Every channel navigation entry point funnels through here, so this
|
||||
// is the single click-time anchor for the switch trace. Re-selecting
|
||||
// the already-active channel is a no-op navigation: the channel's
|
||||
// effects never rerun, nothing would settle the trace, and it would
|
||||
// squat on the singleton until timeout — so don't open one. (History
|
||||
// back/forward bypasses goChannel entirely and is deliberately
|
||||
// untraced.)
|
||||
if (!location.pathname.endsWith(`/channels/${channelId}`)) {
|
||||
beginChannelSwitchTrace(channelId);
|
||||
}
|
||||
return commitNavigation(
|
||||
{
|
||||
to: "/channels/$channelId",
|
||||
@@ -336,6 +319,17 @@ export function useAppNavigation() {
|
||||
threadRootId: options.threadRootId ?? null,
|
||||
}
|
||||
: undefined,
|
||||
// Every channel navigation entry point funnels through here, so this
|
||||
// is the single click-time anchor for the switch trace; it opens
|
||||
// inside commitGuardedNavigation only after the navigation guard
|
||||
// accepts. Re-selecting the already-active channel is a no-op
|
||||
// navigation: the channel's effects never rerun, nothing would
|
||||
// settle the trace, and it would squat on the singleton until
|
||||
// timeout — so don't open one. (History back/forward bypasses
|
||||
// goChannel entirely and is deliberately untraced.)
|
||||
location.pathname.endsWith(`/channels/${channelId}`)
|
||||
? undefined
|
||||
: channelId,
|
||||
);
|
||||
},
|
||||
[commitNavigation, location.pathname],
|
||||
|
||||
@@ -172,6 +172,15 @@ export function ProjectsView() {
|
||||
snapshotProjects,
|
||||
activeCommunity?.reposDir,
|
||||
);
|
||||
// Hydration marker for the switch-perf harness: the shell commits long
|
||||
// before the query fan loads. isLoading so disabled queries never wedge it.
|
||||
const projectsHydrating =
|
||||
projectsQuery.isLoading ||
|
||||
projectsWorkItemsQuery.isLoading ||
|
||||
repoSnapshotsQuery.isLoading ||
|
||||
activitySummariesQuery.isLoading ||
|
||||
repositoryActivitySummariesQuery.isLoading ||
|
||||
localRepositoriesQuery.isLoading;
|
||||
const memberChannelIds = useMemberChannelIds();
|
||||
const repositoryUnavailableReasonFor = useRepositoryUnavailableReasonFor(
|
||||
repoSnapshotsQuery.data?.unavailable,
|
||||
@@ -722,6 +731,7 @@ export function ProjectsView() {
|
||||
data-project-context-detached={
|
||||
isNarrowProjectsLayout ? undefined : "true"
|
||||
}
|
||||
data-projects-hydrating={projectsHydrating ? "true" : undefined}
|
||||
data-testid="projects-overview-layout"
|
||||
>
|
||||
<ProjectsWorkspaceChrome
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
shouldAttributeFetch,
|
||||
buildSwitchPerfLogRecord,
|
||||
resolveSettleAction,
|
||||
resolveSettleWait,
|
||||
summarizeChannelSwitchTrace,
|
||||
} from "./channelSwitchPerf.ts";
|
||||
|
||||
@@ -104,6 +105,32 @@ test("settle drops a trace that has timed out", () => {
|
||||
);
|
||||
});
|
||||
|
||||
test("the settle wait records truncated — never as an honest settle — at deadline", () => {
|
||||
// Still pending, before the deadline: keep waiting.
|
||||
assert.equal(resolveSettleWait(4_999, 5_000, true), "wait");
|
||||
// Render caught up: record cleanly.
|
||||
assert.deepEqual(resolveSettleWait(1_000, 5_000, false), {
|
||||
settleWaitTruncated: false,
|
||||
});
|
||||
// Deadline expired while still pending: the record must say so — a >5s
|
||||
// switch reported as an ordinary settle would hide exactly the tail this
|
||||
// tracer exists to expose.
|
||||
assert.deepEqual(resolveSettleWait(5_000, 5_000, true), {
|
||||
settleWaitTruncated: true,
|
||||
});
|
||||
});
|
||||
|
||||
test("a truncated settle is flagged in the summary and the log record", () => {
|
||||
const summary = summarizeChannelSwitchTrace(trace(), 1_412, true);
|
||||
assert.ok(summary.endsWith(" settle=truncated"), summary);
|
||||
const record = buildSwitchPerfLogRecord(trace(), 1_412, true);
|
||||
assert.equal(record.settleWaitTruncated, true);
|
||||
// Clean settles keep the field out of the line entirely.
|
||||
assert.ok(
|
||||
!("settleWaitTruncated" in buildSwitchPerfLogRecord(trace(), 1_412)),
|
||||
);
|
||||
});
|
||||
|
||||
test("fetches attribute only when started after the switch began", () => {
|
||||
const active = trace({ channelId: "abcdef1234567890", startedAt: 1_000 });
|
||||
// Started before the switch (stale A→B→A leg): not attributable.
|
||||
|
||||
@@ -19,12 +19,6 @@
|
||||
* `members=cache` — by design, not omission.
|
||||
*/
|
||||
|
||||
export type ChannelSwitchFetchTrace = {
|
||||
durationMs: number;
|
||||
eventCount?: number;
|
||||
memberCount?: number;
|
||||
};
|
||||
|
||||
import { invoke, isTauri } from "@tauri-apps/api/core";
|
||||
|
||||
export type ChannelSwitchTrace = {
|
||||
@@ -48,6 +42,7 @@ let activeTrace: ChannelSwitchTrace | null = null;
|
||||
export function summarizeChannelSwitchTrace(
|
||||
trace: ChannelSwitchTrace,
|
||||
settledAt: number,
|
||||
settleWaitTruncated = false,
|
||||
): string {
|
||||
const total = Math.round(settledAt - trace.startedAt);
|
||||
const commit =
|
||||
@@ -64,7 +59,8 @@ export function summarizeChannelSwitchTrace(
|
||||
: `${trace.membersFetch.memberCount} members in ${Math.round(trace.membersFetch.durationMs)}ms`;
|
||||
return (
|
||||
`[switch-perf] channel=${trace.channelId.slice(0, 8)} total=${total}ms ` +
|
||||
`commit=${commit} window=${window} members=${members}`
|
||||
`commit=${commit} window=${window} members=${members}` +
|
||||
(settleWaitTruncated ? " settle=truncated" : "")
|
||||
);
|
||||
}
|
||||
|
||||
@@ -76,6 +72,7 @@ export function summarizeChannelSwitchTrace(
|
||||
export function buildSwitchPerfLogRecord(
|
||||
trace: ChannelSwitchTrace,
|
||||
settledAt: number,
|
||||
settleWaitTruncated = false,
|
||||
): {
|
||||
ts: string;
|
||||
channelId: string;
|
||||
@@ -83,8 +80,10 @@ export function buildSwitchPerfLogRecord(
|
||||
commitOffsetMs: number | null;
|
||||
windowFetch: { durationMs: number; eventCount: number } | null;
|
||||
membersFetch: { durationMs: number; memberCount: number } | null;
|
||||
settleWaitTruncated?: true;
|
||||
} {
|
||||
return {
|
||||
...(settleWaitTruncated ? { settleWaitTruncated: true as const } : {}),
|
||||
ts: new Date().toISOString(),
|
||||
channelId: trace.channelId,
|
||||
totalMs: Math.round(settledAt - trace.startedAt),
|
||||
@@ -258,6 +257,23 @@ export function resetChannelSwitchTrace(): void {
|
||||
/** Bound on waiting for the deferred timeline commit before recording. */
|
||||
const SETTLE_RENDER_WAIT_MS = 5_000;
|
||||
|
||||
/**
|
||||
* Per-frame decision for the bounded settle wait: keep waiting only while
|
||||
* the deferred render is still pending AND the deadline hasn't passed. When
|
||||
* the wait ends with the render still pending, the record must say so — the
|
||||
* >deadline tail is exactly what this tracer exists to expose, so the
|
||||
* measurement is kept but flagged rather than posing as an honest settled
|
||||
* paint. Pure for unit testing.
|
||||
*/
|
||||
export function resolveSettleWait(
|
||||
now: number,
|
||||
waitDeadline: number,
|
||||
renderPending: boolean,
|
||||
): "wait" | { settleWaitTruncated: boolean } {
|
||||
if (renderPending && now < waitDeadline) return "wait";
|
||||
return { settleWaitTruncated: renderPending };
|
||||
}
|
||||
|
||||
/**
|
||||
* Closes the active trace once the settled frame has painted. The timeline
|
||||
* renders rows through a deferred snapshot that exposes
|
||||
@@ -287,7 +303,7 @@ export function settleChannelSwitchTrace(channelId: string): void {
|
||||
// finish inside the measured window still attribute to it. It is released
|
||||
// when the record lands; a newer switch's begin() simply replaces it.
|
||||
const waitDeadline = performance.now() + SETTLE_RENDER_WAIT_MS;
|
||||
const record = () => {
|
||||
const record = (settleWaitTruncated: boolean) => {
|
||||
const settledAt = performance.now();
|
||||
if (activeTrace === trace) activeTrace = null;
|
||||
performance.mark(CHANNEL_SWITCH_SETTLED_MARK, {
|
||||
@@ -299,12 +315,17 @@ export function settleChannelSwitchTrace(channelId: string): void {
|
||||
routeCommitAt: trace.routeCommitAt,
|
||||
windowFetch: trace.windowFetch,
|
||||
membersFetch: trace.membersFetch,
|
||||
...(settleWaitTruncated ? { settleWaitTruncated: true } : {}),
|
||||
},
|
||||
start: trace.startedAt,
|
||||
end: settledAt,
|
||||
});
|
||||
console.info(summarizeChannelSwitchTrace(trace, settledAt));
|
||||
appendSwitchPerfLogRecord(buildSwitchPerfLogRecord(trace, settledAt));
|
||||
console.info(
|
||||
summarizeChannelSwitchTrace(trace, settledAt, settleWaitTruncated),
|
||||
);
|
||||
appendSwitchPerfLogRecord(
|
||||
buildSwitchPerfLogRecord(trace, settledAt, settleWaitTruncated),
|
||||
);
|
||||
};
|
||||
const awaitDeferredCommit = () => {
|
||||
if (activeTrace !== trace) {
|
||||
@@ -315,16 +336,18 @@ export function settleChannelSwitchTrace(channelId: string): void {
|
||||
// to diagnose. Better no measurement than a fabricated one.
|
||||
return;
|
||||
}
|
||||
if (
|
||||
performance.now() < waitDeadline &&
|
||||
document.querySelector('[data-render-pending="true"]') !== null
|
||||
) {
|
||||
const decision = resolveSettleWait(
|
||||
performance.now(),
|
||||
waitDeadline,
|
||||
document.querySelector('[data-render-pending="true"]') !== null,
|
||||
);
|
||||
if (decision === "wait") {
|
||||
window.requestAnimationFrame(awaitDeferredCommit);
|
||||
return;
|
||||
}
|
||||
window.requestAnimationFrame(() => {
|
||||
if (activeTrace !== trace) return;
|
||||
record();
|
||||
record(decision.settleWaitTruncated);
|
||||
});
|
||||
};
|
||||
window.requestAnimationFrame(awaitDeferredCommit);
|
||||
|
||||
@@ -21,13 +21,20 @@ import { installMockBridge } from "../helpers/bridge";
|
||||
* (project enumeration, work items, repo snapshots,
|
||||
* activity summaries) on top of the shell, so this
|
||||
* axis captures the cross-surface switch the felt
|
||||
* 1-2s report singled out.
|
||||
* 1-2s report singled out. Readiness gates on the
|
||||
* surface's data-projects-hydrating marker, not just
|
||||
* the shell header — a sample whose query fan is
|
||||
* still loading must not count as settled. After the
|
||||
* untimed warmup both surfaces are cache-warm, so
|
||||
* measured samples are warm-switch commits.
|
||||
*
|
||||
* Method mirrors warm-switch-markdown.perf.ts: in-page click + rAF polling
|
||||
* (CDP latency never pollutes samples), longtask capture per switch, 4x CPU
|
||||
* throttle, medians over repeated switches, untimed warmup round-trip first.
|
||||
* `deep-history` is pinned to 150 rows so the message-mount cost is fixed
|
||||
* and comparable across member counts.
|
||||
* and comparable across member counts. Each direction of a round-trip is
|
||||
* reported as its own median — the two legs mount different surfaces, and a
|
||||
* combined median could represent neither and hide a one-leg regression.
|
||||
*
|
||||
* Run it (from desktop/):
|
||||
* pnpm build:e2e
|
||||
@@ -42,6 +49,11 @@ const MEASURED_SWITCHES = 8;
|
||||
const THROTTLE_RATE = 4;
|
||||
const DEEP_HISTORY_ROWS = 150;
|
||||
const MEMBER_COUNTS = [0, 2_000, 10_000] as const;
|
||||
/** general + deep-history — the two channels inflateChannelMembers targets. */
|
||||
const INFLATED_CHANNEL_IDS = [
|
||||
"9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50",
|
||||
"feedf00d-0000-4000-8000-000000000007",
|
||||
];
|
||||
|
||||
type SwitchSample = {
|
||||
ms: number;
|
||||
@@ -69,6 +81,8 @@ async function measureSwitch(
|
||||
targetTitle: string | null;
|
||||
/** Selector that must be present before the switch counts. */
|
||||
readySelector: string;
|
||||
/** Selector that must be ABSENT before the switch counts. */
|
||||
pendingSelector: string | null;
|
||||
},
|
||||
): Promise<SwitchSample> {
|
||||
return page.evaluate(async (args) => {
|
||||
@@ -92,6 +106,8 @@ async function measureSwitch(
|
||||
const ready =
|
||||
titleReady &&
|
||||
document.querySelector(args.readySelector) !== null &&
|
||||
(args.pendingSelector === null ||
|
||||
document.querySelector(args.pendingSelector) === null) &&
|
||||
document.querySelector('[data-render-pending="true"]') === null;
|
||||
if (ready) {
|
||||
requestAnimationFrame(() => requestAnimationFrame(() => resolve()));
|
||||
@@ -121,18 +137,23 @@ type SwitchTarget = {
|
||||
targetTestId: string;
|
||||
targetTitle: string | null;
|
||||
readySelector: string;
|
||||
pendingSelector: string | null;
|
||||
};
|
||||
|
||||
const GENERAL_TARGET: SwitchTarget = {
|
||||
targetTestId: "channel-general",
|
||||
targetTitle: "general",
|
||||
readySelector: "[data-message-id]",
|
||||
// Prefixed so the previous channel's still-mounted rows can never satisfy
|
||||
// the gate.
|
||||
readySelector: '[data-message-id^="mock-general-"]',
|
||||
pendingSelector: null,
|
||||
};
|
||||
|
||||
const DEEP_HISTORY_TARGET: SwitchTarget = {
|
||||
targetTestId: "channel-deep-history",
|
||||
targetTitle: "deep-history",
|
||||
readySelector: '[data-message-id^="mock-deep-history-"]',
|
||||
pendingSelector: null,
|
||||
};
|
||||
|
||||
const PROJECTS_TARGET: SwitchTarget = {
|
||||
@@ -140,25 +161,12 @@ const PROJECTS_TARGET: SwitchTarget = {
|
||||
targetTitle: null,
|
||||
// Rendered by every Projects view mode (Activity intro or section header).
|
||||
readySelector: '[data-testid="projects-page-header"]',
|
||||
// The header is shell — the query fan (projects, work items, repo
|
||||
// snapshots, activity summaries) must have settled too.
|
||||
pendingSelector: '[data-projects-hydrating="true"]',
|
||||
};
|
||||
|
||||
async function runScenario(
|
||||
page: import("@playwright/test").Page,
|
||||
label: string,
|
||||
target: SwitchTarget,
|
||||
back: SwitchTarget,
|
||||
): Promise<SwitchSample[]> {
|
||||
// Untimed warmup round-trip: caches both surfaces' queries and jits the
|
||||
// switch code paths.
|
||||
await measureSwitch(page, target);
|
||||
await measureSwitch(page, back);
|
||||
|
||||
const samples: SwitchSample[] = [];
|
||||
for (let run = 0; run < MEASURED_SWITCHES; run += 1) {
|
||||
samples.push(await measureSwitch(page, target));
|
||||
samples.push(await measureSwitch(page, back));
|
||||
}
|
||||
|
||||
function reportDirection(label: string, samples: SwitchSample[]): void {
|
||||
const times = samples.map((sample) => sample.ms);
|
||||
const longtaskTotals = samples.map((sample) => sample.longtaskTotal);
|
||||
/* eslint-disable no-console */
|
||||
@@ -178,7 +186,37 @@ async function runScenario(
|
||||
`worst single longtask: ${Math.max(...samples.map((sample) => sample.longtaskMax)).toFixed(1)}ms`,
|
||||
);
|
||||
/* eslint-enable no-console */
|
||||
return samples;
|
||||
}
|
||||
|
||||
async function runScenario(
|
||||
page: import("@playwright/test").Page,
|
||||
label: string,
|
||||
target: SwitchTarget,
|
||||
back: SwitchTarget,
|
||||
): Promise<{ toTarget: SwitchSample[]; toBack: SwitchSample[] }> {
|
||||
// Untimed warmup round-trip: caches both surfaces' queries and jits the
|
||||
// switch code paths.
|
||||
await measureSwitch(page, target);
|
||||
await measureSwitch(page, back);
|
||||
|
||||
// The two legs mount different surfaces; report each as its own median so
|
||||
// a one-leg regression can never hide in a combined number.
|
||||
const toTarget: SwitchSample[] = [];
|
||||
const toBack: SwitchSample[] = [];
|
||||
for (let run = 0; run < MEASURED_SWITCHES; run += 1) {
|
||||
toTarget.push(await measureSwitch(page, target));
|
||||
toBack.push(await measureSwitch(page, back));
|
||||
}
|
||||
|
||||
reportDirection(
|
||||
`${label}, ${back.targetTestId} -> ${target.targetTestId}`,
|
||||
toTarget,
|
||||
);
|
||||
reportDirection(
|
||||
`${label}, ${target.targetTestId} -> ${back.targetTestId}`,
|
||||
toBack,
|
||||
);
|
||||
return { toTarget, toBack };
|
||||
}
|
||||
|
||||
for (const memberCount of MEMBER_COUNTS) {
|
||||
@@ -234,11 +272,13 @@ for (const memberCount of MEMBER_COUNTS) {
|
||||
),
|
||||
);
|
||||
|
||||
// Verify the inflation actually landed before measuring anything.
|
||||
// Verify the inflation actually landed — on BOTH inflated channels. The
|
||||
// bridge silently skips unknown channel names, so a rename/typo would
|
||||
// otherwise run baseline membership under a "10,000 members" label.
|
||||
if (memberCount > 0) {
|
||||
await expect
|
||||
.poll(() =>
|
||||
page.evaluate(async () => {
|
||||
page.evaluate(async (channelIds) => {
|
||||
const invoke = (
|
||||
window as unknown as {
|
||||
__TAURI_INTERNALS__: {
|
||||
@@ -249,11 +289,16 @@ for (const memberCount of MEMBER_COUNTS) {
|
||||
};
|
||||
}
|
||||
).__TAURI_INTERNALS__.invoke;
|
||||
const response = await invoke("get_channel_members", {
|
||||
channelId: "feedf00d-0000-4000-8000-000000000007",
|
||||
});
|
||||
return response.members.length;
|
||||
}),
|
||||
const counts = await Promise.all(
|
||||
channelIds.map(async (channelId) => {
|
||||
const response = await invoke("get_channel_members", {
|
||||
channelId,
|
||||
});
|
||||
return response.members.length;
|
||||
}),
|
||||
);
|
||||
return Math.min(...counts);
|
||||
}, INFLATED_CHANNEL_IDS),
|
||||
)
|
||||
.toBeGreaterThanOrEqual(memberCount);
|
||||
}
|
||||
@@ -282,9 +327,14 @@ for (const memberCount of MEMBER_COUNTS) {
|
||||
await client.send("Emulation.setCPUThrottlingRate", { rate: 1 });
|
||||
|
||||
// Instrument, not a gate: assert the harness measured real work.
|
||||
expect(channelSamples.length).toBe(MEASURED_SWITCHES * 2);
|
||||
expect(channelSamples.every((sample) => sample.ms > 0)).toBe(true);
|
||||
expect(projectsSamples.length).toBe(MEASURED_SWITCHES * 2);
|
||||
expect(projectsSamples.every((sample) => sample.ms > 0)).toBe(true);
|
||||
for (const samples of [
|
||||
channelSamples.toTarget,
|
||||
channelSamples.toBack,
|
||||
projectsSamples.toTarget,
|
||||
projectsSamples.toBack,
|
||||
]) {
|
||||
expect(samples.length).toBe(MEASURED_SWITCHES);
|
||||
expect(samples.every((sample) => sample.ms > 0)).toBe(true);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user