Uh oh!
There was an error while loading. Please reload this page.
fix(components,fields,plugin-list): withhold exists/notExists from the consumers whose dialects cannot express them - #4745
Conversation
…o dialect can express them The shared FilterBuilder offered `exists` / `notExists` to every consumer, but only one of the three at-rest dialects it feeds has an existence operator. Measured on @objectstack/spec 17.0.0-rc.6: neither VIEW_FILTER_OPERATORS nor VALID_AST_OPERATORS contains one under any spelling (zero members matching /exist/); only the MongoDB-style FieldOperatorsSchema criteria carries $exists. So on the list toolbar `mapOperator`'s `default:` arm returned the id verbatim and the query went out as ['name','exists','x'], which isFilterAST() rejects — an unfiltered read or a 400. Saving the same panel as a view produced a rule ViewFilterRuleSchema refuses, and the dataset inspector dropped the condition silently. The pair moves behind the existing OPT_IN_OPERATORS gate and FilterConditionField — which writes the criteria dialect — opts in, so $exists stays authorable where it works. Mapping onto isNotNull/isNull was rejected: those already have their own rows, the round trip is lossy, and $exists = has-value is not yet settled across drivers. Closing the class: objectui's three operator-parity guards all sweep spec vocabulary to objectui, never the reverse. A new parity test forces the set the list toolbar offers to EQUAL the set its two dialects can express, in both directions, so neither an unexpressible offering nor a stale withdrawal can survive. 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. |
yinlianghui
commented
Aug 15, 2026
PM review — ACCEPT (reviewer of record, session Independently verified, not taken from the report:
Two seat-side notes: my dispatch comment cited a wrong comment id (it pointed at another card's ACCEPT) — the dev read both PM comments and flagged rather than guessing, which is correct handling of a PM slip, acknowledged here. The commit-trailer conflict (repo convention vs harness default) was also flagged rather than silently resolved — the repo-convention choice honoring the no-model-identifier rule is the right one. Body: Generated by Claude Code |
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#4736
Verified at
9847519e3(the head commit of this branch; every number below was taken at that sha with a clean working tree).Route taken: B (withhold), after A was measured and found unavailable
The card adjudicated A-then-B: express the operator if the AST vocabulary genuinely can, withhold it only if it cannot. A is not available, and the measurement is unambiguous rather than borderline.
Measured on
@objectstack/spec17.0.0-rc.6, the three at-rest dialects this one dropdown feeds:VALID_AST_OPERATORS(52 members)ListViewlive grid, viamapOperator/exist/VIEW_FILTER_OPERATORS(19 members)foldFilterGroupToSpecRules/exist/FILTER_OPERATORS(Mongo criteria)FilterConditionFieldsharing-rule criteria$existsThe AST set's null-family members are
is_null/is_not_null/is_empty/is_not_emptyand their squashed spellings — there is no existence token to map onto under any spelling. So "add themapOperatorrows" has no target that is not a semantic claim.This is not the "expressible on some paths only" case that would have been a
needs_decision: the list consumer's dialects express it on no path. The divergence I did find (below) is in the third dialect, which belongs to a different consumer that is out of scope and works correctly today.What broke, on three surfaces
Reproduced by the new test, which drives the real production function:
mapOperator'sdefault:arm returned the id verbatim, soconvertFilterGroupToASTemitted["some_field","exists","x"]andisFilterAST()returnsfalse. Perfilter-operator-ast-parity.test.ts's own header that costs an unfiltered read or a 400 — "Both are a broken query."normalizeFilterOperator('exists')returns'exists'unchanged andViewFilterRuleSchemarefuses it withinvalid_value, so the panel's filter cannot be persisted at all.groupToCondition'sOP_TO_MONGOhas no row either, and itsif (!mop) continuedrops the condition silently.Reachable through pure UI interaction: the operator dropdown's change handler updates
operatoralone and preservesvalue, soname equals foo→ switch to Is set yields exactly the emitted triple above.The change
@object-ui/components—exists/notExistsjoincontainsCaseInsensitiveinOPT_IN_OPERATORS. They stay indefaultOperators(drawable, translated, counted byFILTER_BUILDER_OPERATORS); they are simply no longer offered to a consumer that did not ask.@object-ui/fields—FILTER_CONDITION_EXTRA_OPERATORSnames both, so the sharing-rule criteria builder keeps them. That opt-in is now the only thing keeping$existsauthorable, which the fields test pins.@object-ui/plugin-list—ListViewpasses an explicit, exportedLIST_VIEW_EXTRA_OPERATORS(deliberately empty) rather than omitting the prop, so the parity test has a real handle on what this toolbar offers and a future inlineextraOperatorscannot widen the dropdown unnoticed.@object-ui/app-shell— comment only.viewFilterFold's note explaining whyexists/notExistsare inVALUELESS_FILTER_OPERATORSdescribed a rationale this PR changes; the entries stay (the builder still draws them value-less for the criteria widget) but for a different reason now.Why not map onto
isNotNull/isNullThree independent reasons, all recorded in the code:
isNull/isNotNullas their own rows, so the collapse would put two labels on one wire predicate.existsreads back throughspecToBuilderOperator('is_not_null')asisNotNull, rewriting the author's choice on reopen. This is the same refusalview-operator-builder-parity.test.tsalready records foris_nullmapped ontoisEmpty.@objectstack/spec's owndata/index.d.tsrecords$exists= has-value (!= null) as settled and shipped onformulaanddriver-memory's reference matcher, whiledriver-memory's live mingo path anddriver-mongodbstill read key-presence, both frozen by objectstack#5499 — which is why upstream cannot enrol a$existsconformance row yet either.The structural requirement: forced by an assertion, not by discipline
objectui has three operator-parity guards and all three sweep spec vocabulary → objectui. None asked whether an id the dropdown draws is an id the consumer can persist, which is the direction that broke.
New:
packages/plugin-list/src/__tests__/list-offered-operator-expressible-parity.test.ts. Its spine is an equality, not a subset check — a subset check is satisfied by a toolbar that offers nothing, and it would let the opposite drift through silently:OPT_IN_OPERATORS";That second direction is what makes the
OPT_IN_OPERATORSentry self-retiring: the day the view and AST vocabularies gain an existence operator, this test goes red and says to offer it again. Two anti-tautology guards back it up — the emitted AST node must be non-empty (a row dropped as incomplete yields[], which would otherwise sail throughisFilterAST()), and a totality ratchet fails if a new field-type bucket appears that the probe list does not reach.Verification
All at
9847519e3, working tree clean.OPT_IN_OPERATORSon the committed fix, re-ran: 8 failures, exactly the predicted set, the live-grid leg reporting the real emission["some_field","exists","x"]. Restored withgit checkoutfrom the branch and re-ran: 96/96 green. Direction was red, as predicted before running.plugin-list,components/__tests__, the fields operator test,plugin-view/config/__tests__,data-objectstack's AST parity, and the three app-shell fold tests.components+fields+plugin-list+plugin-view+data-objectstack— 316 files, 3831 tests passed. app-shell filter surface — 57 files, 643 passed / 1 skipped.components,fields,plugin-list,app-shell— all clean (dependency closures built first, per the fresh-worktree requirement).check-changeset-presence,check-changeset-no-major,check:control-bytes,check:spec-symbols,check:phantom-deps,check:i18n-keys,check:i18n-drift— all exit 0. ThefilterBuilder.operators.exists/.notExistslocale keys stay live (the operators are still drawn, just opt-in) and do not appear in the dead-keys report.Changeset:
patchfor the four packages, with the user-visible before/after stated plainly.Out of scope, filed separately
#4744 —
convertFilterGroupToASTexempts onlyisEmpty/isNotEmptyfrom its incomplete-row drop, while the builder renders no value input for six operators. A fresh Is null row therefore carriesvalue: '', is judged unfinished, and is dropped: measuredisNull→[]versusisEmpty→["name","=",null]. That is a value-completeness defect, not a vocabulary one, andisNull/isNotNullstay offered here precisely because both dialects do express them. Not fixed in this PR.Generated by Claude Code