diff --git a/apps/bench/src/__tests__/bench-app.test.tsx b/apps/bench/src/__tests__/bench-app.test.tsx index 950568eeb..426a8adcd 100644 --- a/apps/bench/src/__tests__/bench-app.test.tsx +++ b/apps/bench/src/__tests__/bench-app.test.tsx @@ -15,6 +15,10 @@ describe("BenchApp", () => { afterEach(() => { cleanup(); vi.restoreAllMocks(); + // The published result is a global and nothing else clears it, so a test + // that waits for one can match the PREVIOUS test's result and assert + // against it — passing or failing on a run that never happened. + delete window[BENCH_RESULT_KEY]; }); test("renders selected scenario metadata and publishes a terminal result", async () => { @@ -443,6 +447,79 @@ describe("BenchApp", () => { expect(groupRowsAtCallTime).toBeGreaterThan(0); }, 20_000); + test("declines to measure group-expand when the grouping never paints", async () => { + // The CI failure this pins: the row MODEL settles several frames before + // React commits the paint, so gating the wait on the model alone opened the + // measurement window with zero group rows on screen — folding the grouping + // render into a number that is supposed to measure only the toggle. + // + // Withholding the paint FOREVER is what makes this discriminate. Gating on + // the model (the old behaviour) ignores the DOM entirely and still reports + // `completed`; gating on the paint runs out its frame budget and reports + // `partial`, which is the honest answer when the precondition never held. + const realQuery = Element.prototype.querySelectorAll; + // Both prototypes: the app queries `viewportRef.current ?? document`, and + // Document does not inherit Element's method — stubbing only Element left + // the fallback path live, which is why this first passed alone and failed + // in the suite. + const realDocumentQuery = Document.prototype.querySelectorAll; + const hide = (selector: string) => + selector === "[data-pretable-group-row]" + ? "[data-nonexistent-so-this-is-empty]" + : selector; + + vi.spyOn(Element.prototype, "querySelectorAll").mockImplementation( + function (this: Element, selector: string) { + return realQuery.call(this, hide(selector)); + } as typeof Element.prototype.querySelectorAll, + ); + vi.spyOn(Document.prototype, "querySelectorAll").mockImplementation( + function (this: Document, selector: string) { + return realDocumentQuery.call(this, hide(selector)); + } as typeof Document.prototype.querySelectorAll, + ); + + const measureSpy = vi + .spyOn(benchRuntime, "measureBenchInteractionRun") + .mockResolvedValue({ + status: "completed", + notes: ["interaction mode: group-expand"], + metrics: { + interaction_latency_ms: 9, + settle_duration_ms: 8, + post_interaction_blank_gap_frames: 0, + post_interaction_anchor_shift_px: 0, + post_interaction_row_height_error_p95_px: 0, + post_interaction_row_height_error_measurable_rows: 11, + result_row_count: 40, + selected_row_preserved: 1, + focused_row_preserved: 1, + dom_nodes_peak: 400, + rendered_rows_peak: 11, + rendered_cells_peak: 440, + }, + }); + + render( + , + ); + + await waitFor( + () => { + expect(window[BENCH_RESULT_KEY]).toMatchObject({ + adapterId: "pretable", + }); + }, + { timeout: 20_000 }, + ); + + expect(window[BENCH_RESULT_KEY]).toMatchObject({ status: "partial" }); + expect(measureSpy).not.toHaveBeenCalled(); + }, 30_000); + test("runs the group script through the interaction probe with the grouping applied by the trigger", async () => { const interactionSpy = vi .spyOn(benchRuntime, "measureBenchInteractionRun") diff --git a/apps/bench/src/bench-app.tsx b/apps/bench/src/bench-app.tsx index 420225c4c..d1fa644a0 100644 --- a/apps/bench/src/bench-app.tsx +++ b/apps/bench/src/bench-app.tsx @@ -231,6 +231,19 @@ export function BenchApp({ search, browserVersion }: BenchAppProps) { continue; } + // The model is grouped, but React may not have committed the paint yet, + // and the invariant this wait exists to hold is about the SCREEN: if the + // group rows render inside the measurement window, that render is what + // gets measured instead of the toggle. Under CI load the model settles + // several frames before the commit lands, so gating on the model alone + // opens the window early and silently folds grouping cost into the + // group-expand number. + if (countPaintedGroupRows() === 0) { + previousRowCount = -1; + stableFrames = 0; + continue; + } + if (snapshot.visibleRowCount === previousRowCount) { stableFrames += 1; @@ -243,8 +256,11 @@ export function BenchApp({ search, browserVersion }: BenchAppProps) { } } + // Budget exhausted. Only report a grouped model if it actually reached the + // screen — an unpainted one would reopen the hole above, and a `partial` + // run that says so beats a completed run measuring the wrong thing. const snapshot = pretableGridRef.current?.rowModel.getState().snapshot; - if (!snapshot) return null; + if (!snapshot || countPaintedGroupRows() === 0) return null; for (let index = 0; index < snapshot.visibleRowCount; index += 1) { const row = snapshot.rowAt(index); if (row?.kind === "group") return row; @@ -252,6 +268,13 @@ export function BenchApp({ search, browserVersion }: BenchAppProps) { return null; } + /** Group rows actually committed to the DOM, which is what the measurement + * window's precondition is about — `countGroupRows` reads the model. */ + function countPaintedGroupRows() { + const scope: ParentNode = viewportRef.current ?? document; + return scope.querySelectorAll("[data-pretable-group-row]").length; + } + /** * Setup barrier for the row-set change scripts: hand over only once the resident * window is mounted and the surface has stopped moving.