Repository navigation
test(react): eviction-* flake cluster — poll the controller settle, not a 20ms sleep (#548) - #567
Merged
Merged
Conversation
The three eviction-* files settled a window slide with a fixed 20ms `setTimeout` inside `act`. `setRows` lands synchronously, but the row layout controller draws the new rows across scheduler hops (MessageChannel macrotasks), and under CPU starvation those hops outlast the sleep: the assertion then reads a DOM that is one window BEHIND the props — `row-1` still rendered, already announced at the new window's `aria-rowindex`. Reproduced 2/12 loaded full-suite runs (1-min load 138-270) and, without load, 5/11 tests by injecting a 30ms scheduler hop at the seam. Every one of the five failures had that shape; none was a wrong paint. The product's ordering is sound: grid-core skips a controller observation whose revision no longer matches the model (`create-grid-ui-core.ts`, `capturedRevision !== revision`), so a lagging revision is never reconciled against a newer window, and the transient paint of new rows under the pre-reconcile selection fails closed on `datasetTotal`. Poll for what the controller drew: the exact row ids (and, in the gate-blip file, the leading spacer the plan placed them under, which is the only observable difference between a shut and an open gate). Once the ids match, the same commit has run `observeRowModelRevision`, so the paint read next is the settled one. The gate-blip "gate restored" step gets no wait at all, because it never reached the engine: identical rows are not an effective `setRows` (revision unchanged) and the controller deliberately does not replan on a resultMeta-only change. Mutation-checked with the poll in place: failing `spanReadableInWindow` open on the population size paints `new-1..row-0` selected (negative test red); reading a shut gate as local mode destroys the selection (both blip tests red, CONTROL green); refusing remembered spans breaks the extend-from-evicted-anchor test. Refs #548 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…gine The restore step's comment claimed the revision never moves. True for the estimated-total blip (identical rows are not an effective setRows), not for the engine-sort blip, whose processing change recompiles the query as its own revision. Either way it draws the same rows under the same spacer and the assertions read the return slide, which is what the step now says. Refs #548 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Vercel preview readyPreview: https://pretable-7b00l7r9f-cacheplane.vercel.app Updated automatically by the |
…cted-rows settle, shared window-settle helpers Three review items on the #548 fix, all test-side. 1. eviction-gate-blip: the "gate restored" step was unawaited for every blip, and its comment claimed the revision never moves. True for the total blip (identical rows are not an effective setRows, and the controller does not replan on a resultMeta-only change) but not for the processing blip: restoring the sort authority recompiles the query as a revision, the controller redraws rows 10..19 under the reopened gate's 10-row spacer, and grid-core reconciles the still-evicted selection under the open gate. Unawaited, whether that reconcile ran or was skipped as stale behind the return slide was timing-dependent. It is now awaited (`restoreSpacerRows: 10`) so the observed path is exercised every run; the total blip and the control keep `null` — nothing reaches the controller there. 2. rejected-rows-data-honesty: the same `act(setTimeout(20))` settle, with a WRONG-VERDICT failure mode rather than lag. The step under test lands asynchronously (setQuery transition, controller redraw, then observeRowModelRevision in that commit) and grid-core skips an observation the model has moved past; if the slide back landed first, the step never reconciled, both unwindowed legs read the full selection, and the differential passed vacuously (or the control failed with the wrong verdict). Settles are now per leg, by what each can observe: - keep-window leg: nothing observable (same rows, same spacer, retaining reconcile) — said at the site, not awaited; - control leg: the leading spacer collapsing to zero rows, the plan the controller draws once `resultMeta.window` is gone; - refused leg: no DOM observable — a rejected write RETAINS the window coherent with the rows on screen, so the spacer stays at ten rows — so it polls the engine's verdict through telemetry `selectedRowId`, matched to the control leg's settled verdict (pinned null) so the differential stays answer-agnostic. Mutation-checked: forcing the reconcile to `windowed: true` turns the control leg red at `toEqual([])` with the full selection received. 3. `settledRows`/`settledWindow`/`windowIds`/`renderedRowIds` hoisted into `__tests__/window-settle.ts`, following the directory's flat-module convention (`rejected-write-harness.ts`), and used by all four files. Refs #548 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
Review fixes —
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #548
Test-only. No product code changes; no changeset needed.
Mechanism (one per assertion, all the same class)
Every
eviction-*file settled a window slide withact(() => setTimeout(20)).setRowslands synchronously in the layout effect, but the DOM rows come from the renderer-dom row layout controller, which draws a replacement across scheduler hops (MessageChannelmacrotasks,row-layout-controller.tsfallbackScheduler). Under CPU starvation those hops outlast a 20ms timer, and the assertion reads a DOM that is one window behind the props.Captured under load (run 4-1, load 241–270):
row-1is still rendered from the previous window, but already announced at the NEW window'saria-rowindex(10 + 1 + 2) — the surface has published the new offset while the controller has not yet published the new rows. This is the "one render where a pager swap has moved the window butsetRowshas not yet settled" skew documented atpretable-surface.tsx(selectionWindowcomment), caught mid-flight.:180row-1 nullaria-rowindex=13):229paint reportrow-10..row-19(previous window), nothing selected:179after[]— DOM still at rows 10..19:216cursor[]["row-8","name"]— row-8 not yet removed from DOM:173row-1 nullaria-rowindex=8)No real race. Traced the eviction → selection → paint flow to rule out a product bug:
create-grid-ui-core.tsobserveRowModelRevision: a controller observation whoserevisionno longer matches the model's current snapshot is skipped (capturedRevision !== revision), so a lagging intermediate revision is never reconciled against a newer window (the "chimera" thegetWindowingcomment warns about cannot happen through this path).indexedRangeContainsCell→rangeDatasetSpan→spanReadableInWindowrefuses a span whosedatasetTotaldiffers from the window's, so a prepended population paints nothing even for one frame.Deterministic reproduction without load
Temporarily replacing the controller's scheduler hop with
setTimeout(task, 30)(not committed): 5 of 11 eviction tests failed, including all three of the issue's cluster members, with identical DOM-lag shapes. With the poll in place, 11/11 under the same injected delay.Fix
Poll for what the controller drew (
waitFor, 15s, per-test 20s timeouts like the siblingindexed-renderingfile):settledRows(container, ids)— exact row ids in DOM order. Once they match, the same commit has already runobserveRowModelRevision, so the paint read next is the settled one.settledWindow(container, ids, spacerRows)— ids plus the leading spacer the plan placed the first row under (rows[].topis global). A shut gate draws 0 spacer rows, an open onewindowStart— this is the only observable difference between the two gates, and it pins that a shut gate really draws no spacer.setRows(create-local-row-model.tsbumps the revision only whendrafted.effective) and the controller deliberately does not replan on aresultMeta-only change (pretable-surface.tsx,windowGapcomment). The old 20ms sleep there was waiting for nothing; the test's real reconciles are the blip revision and the return slide, both polled.No claim was weakened: the negative "paints nothing" assertion is read only after the positive ids poll settles, so it cannot pass on a stale DOM.
Mutation results (poll in place, each reverted)
spanReadableInWindowfails open ondatasetTotalpaints nothing rather than rows the reader never selectedRED —new-1..new-4, row-0painted selectedobserveRowModelRevisionreads a shut gate as local mode (windowed: selectionWindow !== null)after: []), CONTROL + LOCAL MODE greenspanReadableInWindowalways falsesurvives an incremental slide… then extendsRED (extended spanundefined), plus two other span readersLoaded-run tallies (full
@pretable/reactsuite, 3 concurrent + 10yesspinners, 10-core Mac)eviction-population-change(run 2-2still paints the selection back…, run 4-1paints nothing…). 1778 tests/run.Also under load, no file other than
eviction-*failed in either series.Residual (not changed here)
rejected-rows-data-honesty.test.tsx:597uses the sameact(setTimeout(20))settle. It did not fail in the issue's runs or in these 24, so it is left alone; it is the same class if it ever does.🤖 Generated with Claude Code