fix(core): lower the TRUE-identity combinators to "no constraint" instead of handing back the caller's object - #9019
Conversation
…tead of the caller's object
`convertFiltersToAST({ $and: [] })`, `{ $or: [{}] }` and `{ $and: [{}] }` returned the
input object unchanged. `lowerLogicalGroup` answers `undefined` for a group that
reduces to the TRUE identity — deliberately, so no childless `['and']` is emitted —
but when such a group was the only thing in the filter that `undefined` fell through
to the general tail (`if (conditions.length === 0) return filter`), so the group
reappeared one level up in the `$` dialect, in the slot the AST occupies. The same
function already lowered the fourth identity, `{ $or: [] }`, correctly.
Measured against @objectstack/spec 17.4.0 and @objectstack/client 17.4.0: the returned
object is not sent as a filter and refused — `client.data.find()` spreads a plain
object's entries as query parameters, so `{ $and: [] }` left as `?$and=` with no
`filter` parameter, and the server answered 400 UNSUPPORTED_QUERY_PARAM. A filter whose
ruled answer is every row was a failed list, so nothing depended on the refusal to
scope data.
Scoped to a filter whose every key is such a group: the same tail also serves `{}`, an
all-null filter and an empty operator map, and those keep the object they returned — a
null-valued key is this converter's own tolerance rather than a ruled identity, and it
reaches the server as a real `a IS NULL` predicate on the `$expand` route.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
… fold Both ablation legs leave it green — objectui#8513 already made the matcher answer the object dialect correctly, so the unlowered filter and the absent one select the same rows. It pins the producer/consumer alignment direction, not this card's change, and now says so. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
|
One more gate result, arrived after the body was written — the three consumers the card names on the lowering chain (
Also re-derived after the fact: Out of scope and filed separately rather than repaired here: #9020, a null-valued filter key getting two different row sets on the two Generated by Claude Code |
✅ 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
|
In-seat review of record — Clause-② (contract delta),
|
| site | absorbs undefined? |
how |
|---|---|---|
:627, inside translateFilterToAST |
already | that function's own declared return type is unknown | undefined and its docblock already reads "Returns undefined when the input is empty/unrecognized so callers can skip emitting ?filter= entirely." The new answer is inside a type and a meaning it already had. |
:4613 → :4622 |
this PR | const lowered = …; if (lowered !== undefined) options.filters = lowered; |
And translateFilterToAST has two callers, not the one the report names — :4459 (list GET) and :4541 (exportRecords). I checked both: each is const translated = …; if (translated !== undefined) { queryParams.set('filter', …) }. So the widening is absorbed at every sink, and the report undercounted in the safe direction.
The four direct toFilterNode sinks outside core — RelatedList.tsx:477, LineItemsPanel.tsx:151, ObjectGrid.tsx:1479 and :1853 — are unaffected at the type level, because toFilterNode's signature already carried | undefined and is unchanged here. At runtime they now receive undefined where they received an object; that direction cannot regress, since the object they used to receive is the one that left as ?$and= and drew 400 UNSUPPORTED_QUERY_PARAM.
The fence, and why the scoping is a count and not a flag
trueIdentityGroups > 0 && trueIdentityGroups === Object.keys(filter).length is the right shape. The return filter tail also serves {}, an all-null filter and an empty operator map, and those are not the ruled family: an all-null filter travels the $expand route as filter={"a":null} and the server reads a real a IS NULL predicate, so folding it would return more rows on a path objectstack#5322 never ruled on. The PR keeps that boundary and pins it in section 6. { $or: [] } stays FALSE with its ['$or','=',[]] leaf, carried as a live control through every section — a change that flattened the identities into one arm takes the control with it. That is the correct control design for this defect class.
On the evidence, including the part that is honestly labelled as not evidence
Section 3 is declared in its own docblock to not witness this fix — both ablation legs leave it green, because objectui#8513 already taught ValueDataSource's matcher to answer the object dialect. Saying so in the file, rather than letting the next reader count it, is the standard this lane wants. The witnesses are sections 1, 2, 4, 5 and 6.
The edit to #8513's existing pin is legitimate and I checked it specifically, because editing another card's pin is where a ruling quietly dies. It does not weaken #8513: that card's ruling is what this adapter answers for the object dialect, and the assertion still exercises exactly that — moved from selectedIds(toFilterNode({ $and: [] })) to selectedIds({ $and: [] }), i.e. from the literals rather than through a producer that no longer hands them down. A pin was added (toFilterNode({ $and: [] }) is now asserted toBeUndefined), not removed, so a silent revert of this PR reddens there too.
The changeset is minor with the break and a Migration section spelled out, which is the correct handling given this repo forbids major; Changeset Bump Policy is green. The new JSDoc @example compiles under Doc Snippet Type Check (green on this head) with no ledger row added, so it is a real example and not a declared failure.
Observation — the fence is one shape wider than its own rationale. Follow-up card, not a change here.
Object.keys(filter).length counts keys that the loop itself deliberately skipped: the first statement in the loop body is if (value === null || value === undefined) continue;. For null the PR has a stated and correct reason to keep the old answer. For an undefined-valued key that reason does not hold — JSON.stringify drops the key entirely, so { $and: [], b: undefined } reaches the $expand route as filter={"$and":[]}, which is already the ruled TRUE answer, and reaches the raw-GET route as the same ?$and= failure this card exists to end. So { $and: [], b: undefined } is in the ruled family and is not folded.
Explicitly not a blocker and not a regression: the behaviour is byte-identical to before this PR, so nothing that worked stops working. It is a missed member, not a broken one, and widening a PR that is already green under review to chase it is the wrong trade. I will file it as its own card against this tail once this lands, so the ruled family closes on the record rather than in a review comment.
Gates
34-check roster read on this head; the five that were still running at review time are the test shards and Type Check. Landing on every-check-green, not on this comment.
Generated by Claude Code
2 participants
Fixes #8770
convertFiltersToAST({ $and: [] }),{ $or: [{}] }and{ $and: [{}] }returned the input object unchanged.lowerLogicalGroupanswersundefinedfor a group that reduces to the TRUE identity — deliberately, so no childless['and']is emitted — but when such a group was the only thing in the filter thatundefinedfell through to the general tail (if (conditions.length === 0) return filter), so the group reappeared one level up, in the$dialect, in the slot the AST occupies. The same function already lowered the fourth identity correctly, which is what made this an internal inconsistency rather than an open question.Direction per triage (comment 5619918295): the producer aligns to the consumer that objectui#8513 already settled.
1. Re-measurement of all four identity groups
Taken myself, against
@objectstack/spec17.4.0 (the card measured 17.3.0) and this branch's base2596b1b85, before any edit:isFilterAST{ $and: [] }{"$and":[]}— same object backundefined{ $or: [{}] }{"$or":[{}]}— same object backundefined{ $and: [{}] }{"$and":[{}]}— same object backundefined{ $or: [] }["$or","=",[]]The PM's "exactly three wrong, one right" assumption holds on 17.4.0. Two extra readings worth recording, because they show the defect is a family and the three shapes are representatives, not the whole set:
{ $or: [{}, { s: 1 }] }and{ $and: [{}, {}] }also come back unlowered — anything whose combinators all absorb or drop.{ $and: [], s: 1 }was already correct (["s","=",1]): the defect only appears when the identity group is the only producer of conditions. That sibling-dependence is the same hazard objectui#8555 called out on this file.2. The p2 fence — what the refusal currently does
Answer: a HARD ERROR (
400 UNSUPPORTED_QUERY_PARAM) on the defaultprovider: 'object'list path. Not a silent empty. Nothing can be relying on it to scope data, so there is no caller to stop for.How I established it — three links, each measured or pinned rather than assumed:
The producer (measured, above): the object comes back unlowered.
The client (measured, real
@objectstack/client17.4.0 with a stub fetch, observing the URL it builds):client.data.find()tests the value withisFilterAST, and its else branch spreads a plain object's entries as query parameters. So the request is?$and=for{ $and: [] }?$or=[object Object]for{ $or: [{}] }?$and=[object Object]for{ $and: [{}] }with no
filterparameter at all, while the control{ $or: [] }correctly leaves as?filter=["$or","=",[]].The server (read + pinned in its own repo): the shared normalizer rejects any unknown
$-prefixed query parameter —Unsupported query parameter(s),status: 400,code: 'UNSUPPORTED_QUERY_PARAM'— and that is pinned bypackages/objectql/src/protocol-data.test.ts("rejects an unknown $-prefixed query param with 400 UNSUPPORTED_QUERY_PARAM").$and/$orare not in the supported set ($top, $skip, $orderby, $select, $count, $search, $searchFields, $filter, $expand) and are not reserved names either.isFilterAST, so the filter is refused". The outcome is right but the mechanism is not, and the difference is the whole fence: the object is never sent as a filter and judged. It is shredded into junk query parameters, and the 400 is about the junk. The list therefore fails loudly — anINVALID_FILTER-class "the filter is malformed" outcome, never an empty page that could be mistaken for "no records".I also traced the other wire route (
rawFindWithPopulate, taken when$expandor$searchis present, plus the export route). There the same object travels verbatim asfilter={"$and":[]}, and the server's normalizer accepts a plain object as aFilterCondition(options.where = parsedFilter), so that route already answers every row today. ⇒ the twofind()routes disagreed about one filter; after this change they agree, and the widening direction the fence warns about is not new on either — one route was loudly broken, the other already wide.No caller depends on the refusal. In-repo, the only pin that recorded the old behaviour is objectui#8513's conformance file, and it recorded it explicitly as a symptom ("Not asserting this is right — asserting it is what happens"); it is updated here.
3. Which layer, and why
⛔ Not picked in passing. Three candidates, and the tail is the one that is actually wrong:
toFilterNode— rejected. It does not serve the consumer this card is about:@object-ui/data-objectstackcallsconvertFiltersToASTdirectly on both of its routes, so a fold there would leave the wire exactly as broken as it is.$not,$regex, a bare array, an exotic comparand, an unknown or retired operator). What reachesconditions.length === 0is only: a TRUE-identity combinator, a null/undefined-valued key, and an empty operator map — all of them "no constraint". There is no second meaning to distinguish, so a new sentinel would be vocabulary without a referent.convertFiltersToAST's tail — chosen.undefinedis not a new vocabulary either: it is already whattoFilterNode,mergeFilterNodesanddata-objectstack'stranslateFilterToASTmean by "no filter, skip the slot", and all four call sites already act on it correctly —lowerLogicalGrouptestsArray.isArray, the other three test forundefinedor falsiness. The declared return type gains| undefined(a published-API change, called out in the changeset with migration text).And it is deliberately scoped to a filter whose EVERY key is such a group — that is why the implementation counts the collapsed groups and compares the count with the key count, instead of using a flag. The tail also serves inputs that are not combinators at all, and folding those in would have been a real widening on a path nobody ruled about:
{ $and: [], a: null }travels the$expandroute asfilter={"$and":[],"a":null}, which the server reads as a genuinea IS NULLpredicate. Those keep the object they have always returned, and section 6 of the new test file pins that boundary.4. Tests — one assertion per group, with the already-correct one as control
New:
packages/core/src/utils/__tests__/filter-true-identity-8770.test.ts(24 cases), driven by the publishedFILTER_LOGIC_ROWSfixture the identity ruling is held to.{ $or: [] }keeps its['$or','=',[]]leaf and still answers zero rows;isFilterAST-readable;mergeFilterNodes(identity, ['a','=','x'])is now the plain node, where it used to be['and', { $and: [] }, ['a','=','x']], an object in AST child position;{ $and: [{ $and: [] }] };{ $and: [], a: null },{ $and: [], a: 'x' }.5. Ablation (run, not reasoned about)
Fix committed first, then the fold removed on disk, then restored. No
dist/is involved: both test files import the module by relative source path, so vitest compiles the mutated source directly.1 -> 0, injected marker0 -> 1, blob hashf6b0632 -> ae00617(a hash change was asserted, and an empty hash treated as failure);f6b0632= theHEADblob, andgit diff HEADon the path is empty;trap restore EXIT INT TERMwith absolute paths, and restores withgit checkout HEAD -- path(never baregit checkout --, which would take the mutation back out of the index).HEAD)HEADThe 12 are exactly the predicted ones: the 3 shape cases, the 3
isFilterAST-invariant cases, the 3 merge cases, TRUE-and-FALSE, the nested case, and objectui#8513's updated chain pin. Predicted 13 before running; the single difference was my own miscount (two assertions living in oneit), not a case that failed to witness.6. Gates
turbo run type-check(repo-wide, 81 packages)vitest run packages/core/ packages/data-objectstack/turbo run lint(@object-ui/core,@object-ui/data-objectstack)no-explicit-anyeslint --format jsonon the 4 changed source filescheck-control-bytes.mjscheck-changeset-presence.mjscheck-changeset-no-major.mjsminorcheck-governed-queue-guard.mjs --test(5 paths)check-new-cross-file-line-citations.mjscheck:doc-typescheck:doc-snippetscheck:doc-examplesRepo-wide
pnpm lintand the full-farm suites are left to CI. Every exit code above was captured to a file before being read, never through a pipe. The two heavy runs went through the shared verify lock.7. Assumptions I was asked to falsify
@objectstack/spec17.4.0, re-measured after feat(ci): differential gate for newly added cross-file line citations, and retire two stored line-number ledgers (objectui#8875) #8974 / test(scripts): census the$-dialect lowercase aliases before objectui#8568 is ruled #8977 / fix(core)!: retire the four lowercase$-dialect aliases fromconvertFiltersToAST(objectui#8568) #8988 / fix(core): lower$icontains, the canonical operatorconvertFiltersToASTrefused (objectui#8976) #8996 landed. Refined: the broken family is larger than three inputs (any filter whose combinators all reduce to TRUE), and the three named shapes are its minimal representatives.lowerLogicalGroup'sundefinedis deliberate; the defect is the general tail" — HOLDS, chain verified end to end.convertFiltersToASTis exported, its tail also serves non-combinators, so the obvious shape is not obviously safe" — HOLDS, and it changed the design: the fold is key-count-scoped for exactly this reason, and the boundary is pinned.isFilterAST" — FALSIFIED. The refusal is real but arrives as400 UNSUPPORTED_QUERY_PARAMfor shredded query parameters, one layer earlier than the card says. Section 2.packages/core— fix(plugin-grid): one owner for ObjectGrid's cell-renderer resolution, so aformat-hinted column keeps its renderer on every path #9010 (packages/plugin-grid), feat(scripts): gate documented SchemaExample ids against the catalog registry #9009 (scripts/), fix(create-plugin): stop stamping an "ObjectStack Inc." copyright header on the author's own source #9008 (packages/create-plugin), fix(ci): give the eager-closure sensitivity check a lower bound #9007 (ci), docs(guide): point ci-cd-pipeline.md's populations at what derives them, instead of counting them #9005 (docs), fix(types)!: retire DetailViewSchema.related — author record:related_list (objectui#7997, Route C) #8984 (packages/types+ app-shell), chore(deps): lucide-react 1.31.0 -> 1.43.0, with the one retired spelling repaired #8941 (deps). objectui#8983 is real and its branch is pushed with no PR; it touchespackages/app-shellandpackages/plugin-detail, so it does not overlap either. The only other branches whose names suggested this neighbourhood (claude/issue-7971-...,claude/issue-8600-...) point at commits that are alreadymainhistory, i.e. stale, not in flight. objectui#8976 has landed (PR fix(core): lower$icontains, the canonical operatorconvertFiltersToASTrefused (objectui#8976) #8996) and nothing it changed is revisited here.🤖 Generated with Claude Code
https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
Generated by Claude Code