Repository navigation
fix(app-shell): the dataset-filter read half opens a stored condition only when the builder can hold it (objectui#10257) - #10455
Merged
Conversation
…der can hold
`conditionToGroup` opened two kinds of stored condition as editable rows that
the builder could not hold, and the inspector commits on every change, so an
edit to a SIBLING row rewrote or dropped them:
- an incomplete stored value (`{ $eq: '' }`, `{ $in: [] }`, and the implicit
`{ field: '' }` spelling), which `groupToCondition` drops as an unfinished
row; for `$in: []` the loss widened the dataset from no rows to every row;
- a token read back as an operator the column's builder bucket does not offer
(`$in` on a date or number column, `$gt` on a text column, `$exists` /
`$null` on a boolean column), which drew a blank operator trigger that one
field-picker touch reconciled to `equals`.
Every read-back row now goes through one guard, `builderHolds`: the write half
itself must keep the row, and, when a field list is given, the listed column's
bucket (`operatorsForFieldType`, the function the panel uses) must offer its
operator. A row that fails either sends the whole filter to the Source tab.
This is the `$between` guard from objectui#10062, applied to every token
instead of the pair arity only. Reads with no field list skip the bucket
question, as before. The write half is unchanged.
The dateRoundTrip-9382 leg that expected an unlisted column's `$gt` to read
back as `greaterThan` is inverted: the builder draws an unlisted column with
the text bucket, which lacks `greaterThan`.
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
…shell patch Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
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
|
…l its fallback read now meets The pending 9382 changeset said an ambiguous `$gt` / `$lt` the column cannot settle "falls back to the unchanged fixed table". With a field list, which the inspector always passes, `builderHolds` now refuses such a row and the filter goes to the Source tab. Published beside this branch's own changeset, the old sentence would say the filter still opens in the builder. The sentence now names the refusal. The body is the only change; the frontmatter is byte-identical. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
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
|
Contributor
Author
Contract reviewServed-tier: Rendered by an isolated review subagent spawned by the ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
Contributor
Author
Contract reviewServed-tier: Rendered by an isolated review subagent spawned by the ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
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 #10257
Clause-②: no
What was wrong
conditionToGroup(the read half of the Studio dataset-filter bridge) opened two kinds of stored condition as editable rows that the builder cannot hold.DatasetFilterField.commitFilterGroupsaves on every change, so an edit to a SIBLING row rewrote or dropped them:{ $eq: '' }and{ $in: [] }, plus the rest of the class ($ne: '',$nin: [], anullcomparand, and the implicit{ field: '' }/{ field: null }/{ field: [] }). The read half returnedrepresentable: true, andgroupToConditiondrops these as unfinished rows. I re-measured this on the branch base747f3347d:conditionToGroup({ $and: [{ stage: { $eq: 'won' } }, { name: { $eq: '' } }] })is representable, and editingstagecommits{ stage: { $eq: 'lost' } }.$inon a date column,$gton a text column,$inon a number column. Generalised to every token, this also covers$exists/$nullon a boolean column. The panel drew a blank operator trigger, andreconcileOperatorForFieldgivesequals.The direction: the read half refuses, for both classes
This applies objectui#10062's read-half
$betweenguard (PR #10253) to every token. objectui#9363 and objectui#9372 made the write half CARRY a shape only where a finished builder row means exactly that spec predicate. A shape the write half could not carry faithfully stayed out of the builder: it was dropped inertly, or its stored form read back as non-representable. Neither class here can be carried by the write half:equals ''is also the row the builder SEEDS on "Add condition". A write half that kept{ $eq: '' }would emit a filter for every unfinished row. That is the silently-wrong filter its incomplete-row drop exists to prevent.So one shape gets one answer from both halves: the row is refused, and the Source tab takes the filter.
What changed
builderHolds(row, fields)indatasetFilterCondition.ts.conditionToGroupcalls it on every read-back row, from every arm:$exists,$null, the$-operator arm and implicit equality. Anyfalsegivesrepresentable: false. It checks two things:groupToConditionitself, not a copy of its rules, so the halves cannot drift apart. This refuses an incomplete value. It also refuses a stored condition keyed by an empty field name, which the write half drops for the same reason.operatorsForFieldType(type). That is the same bucket function, called the same way (no opt-ins), that the builder uses for each row and thatreadBackOperatoralready reads. No second operator table. A column listed without a type, or not listed at all, gets the text bucket the builder draws for it, as the$betweenrule already did.$betweenblock is removed becausebuilderHoldscovers it. The four objectui#10062$betweenpins pass unchanged, and they now go red whenbuilderHoldsis ablated.groupToCondition(the write half) is byte-identical. The one-time probe below compares its source text.datasetFilterCondition.dateRoundTrip-9382.test.tsx, one leg is inverted. It expected{ mystery: { $gt: 1 } }on an unlisted column to read back asgreaterThan. The builder draws the text bucket for an unlisted column, which lacksgreaterThan, so that read now refuses. A new control shows that an unlisted$eqstill reads back asequals.PM mechanism assumptions, measured
operatorsForFieldTypewith no extras.FilterBuilder.getOperatorsForFieldcalls that function, and the$betweenrule andreadBackOperatoralready read it.747f3347d) with the new one over 7980 reads.$betweencomment's "Only a read with NO field list skips this".FilterBuilderdraws such a row withoperatorsForFieldType(undefined), the text bucket, and the$betweenprecedent already refuses it. The pin: "a column listed WITHOUT a type, or not listed at all ... not representable". One rule for every token therefore refuses an unlisted$in/$gttoo.$eq,$contains, and so on) on an unlisted column is unchanged. I followed the ruling's "generalise the$betweenrule to every token" over the assumption's reading.Evidence (head
7543da0bd)Red-first. I ran the new test file on the unmodified tree:
Tests 14 failed | 11 passed (25). The failures were exactly the predicted legs:representable: falsepin;The WHY, CONTROL, reachable, builder-domain and field-less legs were green.
Reverse proof. The fix was committed first. Then objectstack's
node scripts/ablation-replace.mjsremoved the singlebuilderHoldscall inconditionToGroup. The anchor hit once, went from 1 to 0, and the blob changed from5037f5c55a4dto927bf20ec8cb.Tests 19 failed | 33 passed (52). The 19 are the 14 new pins, the inverted 9382 leg, and the four$betweenprecedent pins.git diff HEADis empty.Targeted tests at
7543da0bd: the new file, the four existingdatasetFilterCondition*suites and the threeDatasetDefaultInspector*suites. Result:Test Files 8 passed (8),Tests 125 passed (125).Build and type-check. The closure build
pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' buildexited 0.pnpm --filter @object-ui/app-shell type-check(tsc --noEmit && tsc -p tsconfig.test.json) exited 0.--listFilesontsconfig.test.jsonlists the new test, the 9382 test and the bridge, once each.Lint, narrowed to the 3 touched files, as a measurement:
lintiseslint .under the rooteslint.config.js. There is no package-local config.--format jsoncounts 3 files, 0 errors and 3no-explicit-anywarnings. All three warnings are on the twoas anylines that were already there; the base file has the same count.projectServiceorparserOptions.project), so this diff cannot change a verdict on an untouched file.Gates, all exit 0 at
7543da0bd:check-changeset-presence,check-changeset-no-major,check-changeset-fixed,check-changeset-overwrite,check:changeset-claims,check:new-line-citations(0 new),check:control-bytes,check-vi-mock-override-shape,check-test-path-roots,check:phantom-deps,check:esm-specifiers,check:spec-symbols,check:unreferenced-sources,check:self-import,check-lint-coverage.Source-text anchor pins:
git grepfinds no test or script that quotes the lines this diff changes.REWORK round 1 (head
f3de6d54f)The contract review passed on the code. The seat's comment 5825731886 on objectui#10257 amended the claim's file surface to add
.changeset/9382-dataset-filter-date-operator-round-trip.md, body only. One commit,f3de6d54f, touches that file and nothing else:git diff --name-only 7543da0bd f3de6d54flists only it. So the source, the tests, and every code measurement above are unchanged.$gt/$ltthe column cannot settle "falls back to the unchanged fixed table". With a field list, which the inspector always passes,builderHoldsnow refuses that row and sends the filter to the Source tab. The paragraph now says so. It adds that, with a field list, the refusal covers all three cases it lists. An untyped or unlisted column gets the text menu, which offers neithergreaterThannorlessThan. A menu offering neither candidate cannot offer the fallback, since the fallback is one of them. Only a read with no field list still opens the row under the fixed-table operator.$gtand$ltopen asgreaterThan/lessThan. With a field list they are refused when the column is untyped, when it is unlisted, and under each oftext,boolean,select,status,lookup,master_detail,userand an unknown type; all eight offer neither candidate.d5a45968c712fcbb04974d23f5e3a6a19e3a6dc646176d2d6034bab8f1d93a54at both7543da0bdandf3de6d54f.@object-ui/componentsis untouched" still hold with this PR applied.f3de6d54f, all exit 0:check-changeset-presence;check:changeset-claims: 2 changesets in the self-contradiction corpus, both clean, and "No pending changeset names a file this change touches";check-changeset-no-major;check-changeset-overwrite: report-only, case 2, "declared at base: @object-ui/app-shell: patch" and "declares now: @object-ui/app-shell: patch";check:control-bytes;check:new-line-citations: 0 new.DatasetDefaultInspector.tsx.Acceptance notes
DatasetDefaultInspectorbuilds the filter columns fromuseDatasetFieldCatalog. ItsfieldOptionsis[]until the base object loads, and stays[]if the load fails.$inon a select column) shows the Source-tab note instead of the "N conditions" trigger.[]the popover draws no builder in either case: it shows "Pick a base object to add filter conditions." Nothing is written in either state.$betweenhas behaved this way since objectui#10062. The changeset says so. The seat ruled to keep it (Q1 A, comment 5825731886).$between.DatasetDefaultInspector.tsxis outside this claim's file surface, so I left the wording alone.Generated by Claude Code