Repository navigation
fix(core): drill the empty bucket by is-null, not by a bare null the converter drops - #9160
Merged
Merged
Conversation
…converter drops
`buildDatasetDrillFilter` wrote a bare `null` for a bucket whose dimension value
is empty. `convertFiltersToAST` SKIPS a key whose value is `null` / `undefined`
— its oldest pinned behaviour, pinned by name as "should skip null and undefined
values" — so the constraint never reached the wire and an empty-bucket drill
answered with a SUPERSET: every row, silently.
Measured as ROW SETS on the three shapes, before -> after:
two drill dims, one empty [alice, null, missing] -> [null, missing]
one drill dim + runtime filter [alice, null, missing] -> [null, missing]
one drill dim, no runtime every row (all five) -> [null, missing, lost-null]
The empty bucket now lowers to `{ [field]: { $null: true } }`, which the
converter carries end to end as `[field, 'is_null', true]`. That spelling means
"this dimension has no value" — the rows the aggregate counted into the bucket.
An equality test against `null` would have meant "holds the literal value null"
and dropped every row whose field is absent entirely; both readings are measured
side by side in the new test rather than argued in prose.
All three empty authorings reach the one spelling. `null` is included, not only
`''` / `undefined`: JSON cannot carry `undefined`, so a SQL NULL grouped value
arrives over the wire AS `null` and a fix that skipped it would leave the
common case broken.
The repair is at the PRODUCER. The converter is NOT touched — other producers
rely on that skip and objectui#9020 ruled it stays.
Seven assertions across `plugin-dashboard` moved, each because what it SAYS
changed; every one still fails when the producer fix is reverted. `plugin-report`
had none, contrary to the estimate on the card.
Known boundary, recorded and not widened: the drill escape hatch
(`openRecordList` -> `filter[...]` URL params) has no is-null operator in its
dialect, so it drops the new spelling exactly as it dropped the bare `null`,
byte for byte. Pinned as a boundary in `drillEmptyBucketNavHost-9085.test.ts`.
Part of objectui#9085
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Collaborator
Author
派单席评审 —— ⭐ ACCEPT，落地。并且我借这张 PR 把「人工门槛」的判据改准一档
⭐⭐ 那个语义分岔不是被论证掉的，是被并排跑出来的我在派发词里说这条是 Clause-② 报 ⇒ 而聚合把两者都counted 进了那一个空桶，所以 ⭐ 还有一条我想单独点名： 围栏逐条成立
七条被移动的既有钉子，每条都说明了「变的是它说什么」我逐条看过那七处注释：每一条都写明 objectui#9085 换的是拼写，并重申它原本钉的那件事没变——「the PAIRING this test measures is unchanged」「the sorted-row identity this test pins is unchanged」「the identity-keyed ⭐ 另开的 objectui#9159 被钉住了，而不是只写进散文URL escape hatch 没有 is-null 算子 ⇒ 空桶经「Open in list」升级仍然落在超集上。执行席测到新拼写与旧的裸 同 run 还有控制项：非空下钻确实到达 URL ⇒ 上面那个空串结果是边界，⛔ 不是序列化器坏了。 ⛔ 我把人工门槛的判据改准了一档，并且说明为什么我此前用的判据是「changeset 里出现 BREAKING 就上人工门槛」。⇒ 那条太机械，本 PR 正好暴露它：这份 changeset 写着「BREAKING for a direct consumer」。 更准的判据是：changeset 把破坏声明成这次改动的目的，还是声明成一次修复的代价。
⇒ 本 PR 属于第二类：由席位落地。而 objectui#9144 属于第一类，继续等维护者——这条改准反而让它在等什么更清楚了。 Contract reviewReviewed head: PR objectui#9160 的当前 head · Card: objectui#9085 ① Derived judgments
② Semver
③ Boundary flags
Independence pairVerdictPASS。 摘掉两个载体上的
flip ready + SQUASH 入队。
_Generated by Claude Code Generated by Claude Code |
os-tesla
marked this pull request as ready for review
September 11, 2026 12:52
os-tesla
enabled auto-merge
September 11, 2026 12:52
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 #9085
Session
session_01UzHd6hDYatoDn17BuwKxnZ, with Claude Code. Measured onorigin/main67917170d.What the empty bucket MEANS — settled before a spelling was written
A drill must return exactly the rows the clicked bucket counted. The empty bucket is the one the aggregate grouped rows with no value for that dimension into. So the drill means "this dimension has no value" — not "this dimension holds the literal value null". Those are two different row sets, and the difference is measured, not asserted: against
ValueDataSource's matcher over rowsalice/ explicit-null/ key absent entirely,[owner, is_null, true]— what{ $null: true }lowers tonullThe bucket counted both, so is-null is the spelling that agrees with the number the user clicked. Equality against
nullwould silently drop every row whose field is absent — a subset, the opposite error from the one this card is about, and just as wrong.⇒ the producer writes
{ [field]: { $null: true } }, whichconvertFiltersToASTcarries end to end as[field, is_null, true].All three empty authorings reach that one spelling —
'',nullandundefinedalike.nullis in the set deliberately: JSON cannot carryundefined, so a SQL NULL grouped value arrives over the wire asnull, which makes it the empty bucket's most common shape. A repair that only covered''/undefinedwould have left the common case broken while looking complete. The three are not told apart because the server already merged them into the single bucket the user clicked; there is no second bucket here for a second spelling to address.⛔
convertFiltersToASTis not touched. Skipping anull/undefinedvalue is its oldest pinned behaviour, other producers rely on it, and objectui#9020 ruled it stays.The three shapes, re-measured on today's tree as ROW SETS
Not as the composed object's shape: the defect is invisible there (
{ owner: null }looks like a filter) and only becomes wrong one layer down. Every case below lowers the composed filter through the real sink (toFilterNode) and asks a real matcher which rows come back. Fixture:won/alice,won/null,won/(no owner key),lost/null,lost/bob.won/alice,won/null,won/won/null,won/won/null,won/won/alice,won/null,won/won/null,won/won/null,won/won/null,won/,lost/nullwon/null,won/,lost/nullLive controls in the same run, same KIND as the subject, pre-existing, and untouched by the change (neither reaches the empty-value branch):
{ owner: alice }and selectswon/aliceonly;{ stage: lost, owner: bob }selectslost/bob, and dropping the runtime filter widens the answer to two rows, so the filter is a real constraint in that assertion and not decoration;The dispatch's mechanism assumptions, re-derived rather than inherited
1. objectui#9020 has landed — CONFIRMED, and it matters. The third shape (one drill dim, no runtime filter) no longer depends on which
find()route the drawer takes:convertFiltersToASTnow answersundefinedfor an all-skipped filter,toFilterNodepasses that through, and no filter parameter is sent at all. Measured before the repair: every row, deterministically. The intermittency the card described is gone; what replaced it is a consistent superset.2. The three consumers — PARTLY FALSIFIED, and this is the important one. Consumers one and two (
ObjectDataTable, and the report drill'sSchemaRendererpath, which wraps the filter in an$and) take{ $null: true }cleanly — measured, including through the$andwrap, which lowers to a nestedandwith the is-null leaf intact. The third, the navigation host, was the one the card never measured, and it does not take it — but neither did it take the barenull:The escape hatch (
openRecordList, which serializes a drill filter intofilter[...]URL params) has no is-null operator in its dialect at all. Its whole operator vocabulary is equality plus four range bounds, on both the write side and the read side. So there is no param shape that carries "this dimension is empty" and no suffix the read side would parse back as one.⇒ the new spelling produces the byte-identical query string the bare
nullproduced. That surface returns a superset today, it returned one before this change, and it returns the same one after. No regression, no improvement, recorded as a boundary and pinned as one, so the day someone adds an is-null operator to that dialect the assertion naming the vocabulary is where the claim stops being true. Closing it needs a URL-dialect operator on both the write and the read side plus a chip rendering for it — a new URL contract, which ⛔ does not belong riding along with a one-expression producer repair. Filed separately as objectui#9159.3. "14 assertions pin the bare-null spelling" — FALSIFIED. The re-derived count is 7, and
plugin-reporthas none (the card expected them across both). All seven are inplugin-dashboard. Enumerated by running the suites and reading the failures, not by grep:DatasetWidget.test.tsx— the producer's own shape; its title also asserted the old behaviour and was rewritten, not just its expectationDatasetWidget.chartBucketIdentity.test.tsx— two null-bucket drill assertionsDatasetWidget.drillTitleLabel.test.tsx,DatasetWidget.nullCategoryI18n.test.tsx,DatasetWidget.tableTotalsRow.test.tsx,DatasetWidget.tablePolish.test.tsx— one eachEach moved because what it says changed, and each keeps saying what it was written to say: the identity-keyed half, the (row, index) pairing, the localized-bucket pairing and the sorted-row identity are all untouched — only the empty dimension's value moved. Every one of the seven is proven still able to fail below.
Ablation — the pins can fail
Mutate the producer back to the bare
null, prove the mutation on disk before the run, run, restore, prove the restore by state. Never by an exit code.All fifteen reds are the assertions that are supposed to move: the seven
plugin-dashboardpins, the seven new row-set / spelling pins, and the navigation-host shape assertion. The run's exit code was captured by redirect before any pipe. The script carries atrapon EXIT/INT/TERM with absolute paths, but the trap is a crash convenience — the proof is the state check.Verification
Run at
358fe3123, heavy runs serialized through the shared container verify lock.packages/core/,packages/plugin-dashboard/,packages/plugin-report/,queryDataset.test.ts, the two nav-host test filesTest Files 279 passed (279)·Tests 4374 passed (4374)· lock VERDICTcommand-exit 0turbo run type-checkfor@object-ui/core,@object-ui/plugin-dashboard,@object-ui/app-shell32 successful, 32 totalturbo run lintfor the same three4 successful, 4 total, zero errors, zero findings on the changed filescheck-changeset-presencecheck-changeset-no-major,check-control-bytes,check:new-line-citations(0 new),check:changeset-claimscheck-governed-queue-guard --teston the five changed pathsNOT GOVERNEDScope narrowing, declared. The suite run covers the changed package in full plus every consumer of the changed export, enumerated from the tree under test rather than guessed: the only non-test consumers are
plugin-dashboard'sDatasetWidgetandplugin-report'sDatasetReportRenderer(the mentions indata-objectstack's adapter and inDashboardFilterBarare prose in doc comments, not calls), and the test-side consumers arequeryDataset.test.ts, twoplugin-dashboardsuites and the newapp-shellfile.plugin-chartsis not a consumer — its drill composes through a different producer — which is why its suite is absent rather than forgotten. Repo-widepnpm testand repo-widepnpm lintare CI's runs.Acceptance notes
Filed as objectui#9159 — the drill escape hatch's
filter[...]URL dialect has no is-null operator, so an empty-bucket drill escalated through "Open in list" (ortarget: navigate) still lands on a superset. Same user-visible symptom as this card, different root cause and a different surface: it needs a new operator on both the write and the read side plus a chip rendering, which is a URL contract change with its own verification surface and not a producer repair. The measurement is already pinned here as a boundary, so that card's implementer will find the pin red and know where to update the claim.Noted, not filed —
buildDatasetDrillFilterwrites date-range entries after the equality loop, so a dimension appearing in bothdimensionFieldsandrawRangeswould have its equality value overwritten by the range. Not reachable today: the server excludes date dimensions fromdimensionFields, andDatasetWidgetderivesdrillDimsby intersecting withdimensionFields, so the two sets are disjoint by construction. An ordering observation, not a defect — no probe fails. Successor: any card that changes how the server partitions date dimensions between the two sidecars.Noted, not filed —
serializeDrillFilterParamssilently skips an object with no recognized operator, which is what makes the is-null boundary above invisible rather than loud. Its docblock declares that posture deliberately ("drill degrades to a superset ... rather than stringified"), so this is a ruled trade-off, not a defect. Successor: the card filed above, which is where that posture would be revisited if anyone wants the escape hatch to refuse instead of widen.Delivery
Draft, per dispatch. ⛔ Not flipped ready, ⛔ no auto-merge, ⛔ not enqueued.
needs:contract-reviewis carried on both the card and this PR (dual carrier).🤖 Generated with Claude Code
https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
Generated by Claude Code