fix(plugin-dashboard): bind drill-title drawer assertions to content, not a cumulative render count - #4708
Merged
Conversation
… not a cumulative render count (#4706) DatasetWidget.drillTitleLabel.test.tsx pinned `drawerProps.length` to exactly 1 after a single drill click. That count only ever grows (the recording array is never trimmed), and a late, unrelated `setMeta` re-render from `useDatasetDimensionMeta`'s own dimension-metadata fetch — which resolves independently of the click, and can land its commit strictly after the click's own commit under load — pushes the same drawer content a second time. Once the count overshoots 1, `waitFor` can never recover, since nothing about it shrinks. Observed on an unrelated PR's shard 2 (run 31885228851): `expected 2 to be 1`. Fixed by waiting for the drawer to have opened AT LEAST once (`toBeGreaterThan(0)`) instead of pinning the exact count, and keeping the existing content assertions (`lastDrawer().title` / `.filter`) as the substantive check — which is what each case's own name says it is testing. No invariant is weakened: an extra identical render was never something these cases claimed to guard, and a drawer that never opens still times out the same as before. Reproduced deterministically pre-fix (all 5 sites, gating the metadata fetch and forcing its resolution into a separate React commit after the click's own commit, via `act()`) and re-verified positive: with the fix in place but the repro's forced overshoot still active, all 7 cases pass, and with objectui#4682's `seriesLabel ?? series` fix temporarily reverted the two identity-collision cases correctly go red on the wrong title — proving the rebound assertions still catch the defect the file exists to pin. All temporary repro/positive-control changes were reverted before this commit; this diff is the fix alone. Sites fixed, all in DatasetWidget.drillTitleLabel.test.tsx (line numbers as committed): - :190 (the issue's named site — "titles an identity-keyed group…") - :222 ("titles the OTHER colliding group…") - :253 (BOUNDARY — an ordinary group's title) - :278 (BOUNDARY — a click carrying no label) - :323 (BOUNDARY — a NON-pivoted widget) Test-only; empty-frontmatter changeset. Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQ Co-authored-by: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
Review verdict: ACCEPT (reviewer of record, PM session Verified against this PR, not the report: exactly two changed files (the test file and an empty-frontmatter changeset); body's only closing keyword is 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 13:47
This was referenced Aug 15, 2026
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Aug 17, 2026
…not a cumulative render count (objectstack-ai#4720) Fixes objectstack-ai#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 objectstack-ai#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. Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQ Co-authored-by: Claude <noreply@anthropic.com>
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
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 #4706
What was wrong
DatasetWidget.drillTitleLabel.test.tsxpinneddrawerProps.lengthto exactly1after a single drill click, in five places.
drawerPropsis a recording array of themocked
DrillDownDrawer's render props — it only ever grows.useDatasetDimensionMetaissues its own dimension-metadata fetch independently of the click; when that fetch's
setMetalands its React commit strictly AFTER the click's own commit (a realpossibility under a saturated event loop, since the two updates are only incidentally
ordered),
DrillDownDrawerrenders a second time with the SAME title/filter, anddrawerProps.lengthovershoots to2. Once it overshoots,waitForcan never recover— nothing shrinks the array. This is what CI observed on an unrelated PR's shard 2
(run 31885228851):
expected 2 to be 1.The fix
Each site now waits for the drawer to have opened AT LEAST once
(
toBeGreaterThan(0)) instead of pinning the exact cumulative count. The contentassertions immediately after (
lastDrawer().title/.filter) are unchanged andremain the substantive check — each case's own name states what it verifies (which
title the drawer shows), never 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.
Sites fixed (all in
DatasetWidget.drillTitleLabel.test.tsx, same isomorphicawait waitFor(() => expect(drawerProps.length).toBe(1))idiom immediately followed bycontent assertions):
:193— "titles an identity-keyed group…" (the issue's named site):223— "titles the OTHER colliding group…":253— BOUNDARY "an ordinary group's title is byte-identical to before":280— BOUNDARY "a click carrying no label still titles from the key":324— BOUNDARY "a NON-pivoted widget still omits the series from its title"All five share byte-identical exposure (confirmed empirically, not just by inspection —
see Verification below), so all five were fixed the same way.
Verification
Deterministic reproduction (pre-fix, all 5 sites red). Temporarily gated the
dimension-metadata
fetchmock behind a manually-releasable promise, wrapped the drillclick in its own
act()so its render fully commits first, then released the gate andflushed inside a second
act()— forcing the click's commit and the metadata commitinto two SEPARATE React passes instead of letting React 18 automatic batching coalesce
them into one (which is why a naive forced-delay attempt without the
act()boundarydid not reproduce it: both updates landed in the same commit). With the OLD assertion,
all 5 sites failed with the exact CI failure text —
expected 2 to be 1.Fix re-verified under the same forced race. With the fix applied and the forced
overshoot still active, all 7 tests in the file pass.
Positive control (assertion bite-check). With the fix in place, temporarily reverted
objectui#4682's production fix (
ev?.seriesLabel ?? ev?.series→ev?.series) inDatasetWidget.tsx'shandleChartDrill. The two identity-collision cases correctlywent red on the WRONG title (
Backlog / [null]/Backlog / ["(None)"]instead ofBacklog / (None)), while the three ordinary-group boundary cases correctly stayedgreen (label equals key there, so the reverted line can't move their title) —
confirming the rebound assertions still catch the real defect the file exists to pin.
All temporary repro/gate/positive-control changes were reverted before the commit in
this PR —
git diffagainstorigin/mainis the fix alone (test file + changeset).Gates run (final HEAD
83da6212d)pnpm --filter '@object-ui/plugin-dashboard' lint— 0 errors (334 pre-existingany/fast-refresh warnings, none introduced by this change; the changed file's ownwarnings are all pre-existing, outside the edited lines)
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— OKChangeset
Test-only change → empty-frontmatter changeset
(
.changeset/dataset-widget-drill-title-render-count.md). No published behaviourchanges.
Not in scope
#4705 and #4682 are unrelated to this change (#4705 only triggered the shard-2 red that
surfaced this issue; #4682 is the drill-title feature these tests pin, untouched here
beyond the temporary, reverted positive control). #4694 is also unrelated. None of them
are addressed or closed by this PR.
Generated by Claude Code