Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-dashboard): bind chart-bucket drill assertion to content, not a cumulative render count - #4720
Merged
Conversation
…not a cumulative render count Fixes#4718 DatasetWidget.chartBucketIdentity.test.tsx pinned `drillFilters.length` to exactly 1 after a single bar click. `drillFilters` is a recording array of the mocked DrillDownDrawer's render props — it only ever grows. useDatasetDimensionMeta issues its own dimension-metadata fetch independently of the click; when that fetch's setMeta lands its React commit strictly AFTER the click's own commit, DrillDownDrawer renders a second time with the SAME filter and drillFilters.length overshoots to 2, which waitFor can never recover from. Same defect class PR #4708 fixed in drillTitleLabel.test.tsx (objectui#4706), second file. The one site now waits for the drill to have opened AT LEAST once (toBeGreaterThan(0)) instead of pinning the exact cumulative count. The content assertion immediately after (lastFilter()) is unchanged and remains the substantive check. Family-wide sweep of packages/plugin-dashboard/src/__tests__/ found no other site needing this treatment: the sibling `drillFilters` sites in nullCategoryI18n/optionLabelI18n/localSelectI18n.test.tsx already use toBeGreaterThan(0); every other `.length).toBe(N)` waitFor pin in the family reads a capturedChartProps-style snapshot (overwritten on each render, not pushed) or a live DOM/mock-call count with no useDatasetDimensionMeta-driven race, so those stay exact-count by construction. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQ
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
commented
Aug 15, 2026
CollaboratorAuthor
Review verdict: ACCEPT (reviewer of record, PM session Verified against this PR, not the report: two changed files (+20/−1), One process note: the dev self-corrected a transient Will flip ready + enable auto-merge once Lint, Type Check, and Test shards 1–4 conclude Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 15, 2026 16:34
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes#4718
What was wrong
DatasetWidget.chartBucketIdentity.test.tsxpinneddrillFilters.lengthto exactly1after a single bar click.
drillFiltersis a recording array of the mockedDrillDownDrawer's render props — it only ever grows.useDatasetDimensionMetaissuesits own dimension-metadata fetch independently of the click; when that fetch's
setMetalands its React commit strictly AFTER the click's own commit (a real possibility under a
saturated event loop, since the two updates are only incidentally ordered),
DrillDownDrawerrenders a second time with the SAME filter, anddrillFilters.lengthovershoots to
2. Once it overshoots,waitForcan never recover — nothing shrinks thearray.
This is the same defect class PR #4708 fixed in
drillTitleLabel.test.tsxfor cardobjectui#4706, and card #4718 reports it observed on a shard 2 red against an unrelated
PR (data-objectstack test-only change), same causality argument as the earlier sibling.
The fix
The one site (
:151, the issue's named site) now waits for the drill to have opened ATLEAST once (
toBeGreaterThan(0)) instead of pinning the exact cumulative count. Thecontent assertion immediately after (
lastFilter()) is unchanged and remains thesubstantive check — the case is about which records the drawer filters on, not how many
times React chose to render it. A drawer that never opens still times out exactly as
before; only the overshoot-vulnerable exact-count pin is relaxed.
Family-wide sweep (
packages/plugin-dashboard/src/__tests__/, all files)Per the card's direction, every
waitFor(() => expect(recordingArray.length).toBe(N))-shaped site in the directory was surveyed, not just the two files already known. Grepped
for
.length).toBe(,toHaveLength(, and everyvi.mock('../DrillDownDrawer', …)/.push(usage across the whole directory.Relaxed (this PR):
DatasetWidget.chartBucketIdentity.test.tsx:151—drillFilters.length).toBe(1)→toBeGreaterThan(0). Same overshoot mechanism as objectui#4706:drillFiltersis apush-based recording array (mocked
DrillDownDrawer), and this file's widget usesuseDatasetDimensionMeta, which is the only hook in this package whose independently-resolving fetch can land a late, unrelated re-render.
Already relaxed (pre-existing, untouched by this PR):
DatasetWidget.nullCategoryI18n.test.tsx:190,215—drillFilters.lengthalreadytoBeGreaterThan(0).DatasetWidget.optionLabelI18n.test.tsx:302— same, already relaxed.DatasetWidget.localSelectI18n.test.tsx:342— same, already relaxed.Kept exact-count, with a stated causal reason (untouched):
DatasetWidget.chartBucketIdentity.test.tsx:145,173,199,218,243—chartRows().length).toBe(2).chartRows()readscapturedChartProps, which themocked chart component OVERWRITES (
capturedChartProps = props) on every render, notpushes. It is a snapshot of the latest render, not a cumulative recording — a late,
unrelated re-render re-assigns the same reference and cannot grow the array beyond the
actual row count the chart was given. No overshoot is structurally possible.
DatasetWidget.drillTitleLabel.test.tsx:172,218,244,270,314—chartSeries()/chartData(), samecapturedChartProps-snapshot idiom as above. AlsoDatasetWidget.nullCategoryI18n.test.tsx:158,169,202,227,236(categories(), sameidiom). All snapshot reads, not recording arrays — safe to keep exact.
DatasetWidget.test.tsx:93—screen.getAllByText('100.0%').length).toBe(2). A livetesting-library DOM query re-evaluated by
waitFor, not a recording array — cannotaccumulate past the actual DOM's current match count.
DatasetWidget.test.tsx:794,DatasetWidget.dottedDimensionTable.test.tsx:352,DatasetWidget.compareTo.test.tsx:460,DatasetWidget.localSelectI18n.test.tsx:375—blobs/callsrecording arrays from a stubbedURL.createObjectURL, but each ischecked SYNCHRONOUSLY immediately after a synchronous
fireEvent.click, never inside awaitForpolling loop — there is no repeated re-evaluation window in which atransient/overshot state could be caught, so this idiom's specific vulnerability (a
polling assertion that can observe a later, larger value) does not apply.
PivotTable.drill.test.tsx:32,38—queryAllByRole('button').length).toBe(0), live DOMqueries, synchronous (no
waitFor), andPivotTabletakes its data as a prop with nointernal async fetch (no
useDatasetDimensionMetausage) — no race exists to overshoot.DatasetWidget.queryOptions.test.tsx,DashboardFilterBar.options.test.tsx,DashboardFilterBar.optionsFromRawValue.test.tsx,DashboardRenderer.headerActions. test.tsx,ObjectMetricWidget.compareTo.test.tsx— variouswaitFor(() => expect(mock).toHaveBeenCalledTimes(N))sites. Different defect class:these count calls to the widget's OWN primary data-fetch mock (
queryDataset/aggregate/find/onWidgetClickhandler), driven deterministically by the test's ownactions and effect-dependency changes, not by an incidental re-render from an unrelated
hook. None of the components under test in these files use
useDatasetDimensionMeta(confirmed:
grep -rl useDatasetDimensionMeta packages/plugin-dashboard/src/*.tsxreturns only
DatasetWidget.tsx), so the specific race this card is about cannot reachthese counts.
No third file needed the treatment — the two files the card flagged as "possibly a third"
turned out to already carry the relaxed form.
Verification
Deterministic reproduction (pre-fix, red). Temporarily gated
chartBucketIdentity. test.tsx's meta-fetch mock behind a manually-releasable promise, wrapped the bar click inits own
act()so its commit lands alone, then released the gate and flushed inside asecond
act()— forcing the click's commit and the metadata commit into two separateReact passes (same technique as PR #4708's body). With the OLD assertion, this reproduced
the exact CI failure text:
AssertionError: expected 2 to be 1, withdrillFilters.lengthobserved at
2after the forced late commit.Fix re-verified under the same forced race. With the fix applied and the forced
overshoot still active, the test passes.
Positive control (assertion bite-check). With the fix in place, temporarily forced
handleChartDrill'sbucketId: ev?.categoryIdtobucketId: undefinedinDatasetWidget.tsx(simulating the identity carrying nothing, i.e. the pre-objectui#4508behavior). The touched test's second click correctly went red on the wrong records
(
{ owner_id: 'user-literal' }instead of the expected{ owner_id: 'user-absent' }—the exact defect objectui#4508 fixed), while the three BOUNDARY cases in the file stayed
green. (Note, for completeness: the file's other collision case, "an empty-string group
drills to itself," stayed green under this particular override — that collision is
evidently guarded by something beyond the
bucketIdparameter alone, so this overridedoesn't reach it; it wasn't the site this PR touches.) Confirms the relaxed count
assertion does not mask a real regression of the underlying drill-identity feature — the
content assertion right after it still carries the real check.
All temporary repro/gate/positive-control changes were reverted before the commit in this
PR (verified byte-identical to
origin/mainbefore re-applying only the real fix) —git diffagainstorigin/mainis the fix alone (test file + changeset).Gates run (final HEAD
00d868d5e)pnpm --filter '@object-ui/plugin-dashboard' lint— 0 errors (334 pre-existingany/fast-refresh warnings, none introduced by this change)pnpm --filter '@object-ui/plugin-dashboard' type-check— cleanpnpm exec vitest run packages/plugin-dashboard/— 58 files / 462 tests passednode scripts/check-control-bytes.mjs— OKnode scripts/check-changeset-presence.mjs— OK (empty-frontmatter changeset declared)node scripts/check-changeset-fixed.mjs/check-changeset-no-major.mjs— OKscripts/pm/dispatch-gates.mjsdoes not exist in this repo; the gate set above is the onethe dispatch prompt named, matching PR #4708's own gate list for the same class of change.
Changeset
Test-only change → empty-frontmatter changeset
(
.changeset/dataset-widget-chart-bucket-drill-render-count.md). No published behaviourchanges.
Not in scope
Card #4717 (the data-objectstack PR whose shard-2 red surfaced this) and card #4508 (the
chart-bucket-identity feature this file pins) are unrelated to this change beyond being
named for context above; neither is addressed or closed by this PR.
Generated by Claude Code