fix(core): teach ValueDataSource's matcher the filter vocabulary the wire already has - #7377
Merged
os-project-manager merged 2 commits intoSep 2, 2026
Merged
Conversation
…wire has
`matchesASTFilter` recognised exactly two node shapes — a logical `and` / `or`
head and a three-element comparison — and returned `true` for everything else.
A filtered in-memory list therefore came back UNFILTERED, with no error and no
console line. Three distinct ways in, all reachable from a shipped view:
- a legacy flat implicit-AND array `[[…], […]]` matched no shape, at top
level and as a nested child of `and` / `or` alike. That is what
`mergeFilterNodes` returns for a lone surviving source, i.e. the common
case, and what `ListView`'s `finalFilter` puts on `$filter`;
- the null-ness operators had no arm in the operator switch, so `is_null` /
`is_not_null` selected every row in both dialects;
- 16 of the spec's 20 canonical `VIEW_FILTER_OPERATORS` had no arm either.
`viewFilterRuleToNode` lowers a stored view's rules through the spec's
`normalizeFilterOperator`, so what arrives is the canonical VIEW spelling
(`equals`, `greater_than`, `starts_with`) — none of which the
spelling-keyed switch knew.
The matcher now reads the four shapes `FilterArraySchema` declares and
canonicalises operators through the spec's own `canonicalAstOperator`, so the
accepted vocabulary is the published one rather than a second hand-written list
that drifts from it. An operator or shape it cannot execute excludes the row and
logs once per `find()` — the measured sibling answer (`@object-ui/permissions`
returns `false` from its `default` arm) rather than the silent widening.
No producer changes: not one request byte moves.
Refs #7221, #7349
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC
This was referenced Sep 2, 2026
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
|
This was referenced Sep 2, 2026
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
|
os-project-manager
marked this pull request as ready for review
September 2, 2026 15:37
os-project-manager
deleted the
claude/issue-7349-value-matcher-vocabulary
branch
September 2, 2026 16:13
This was referenced Sep 2, 2026
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 #7349
What was wrong
matchesASTFilterinpackages/core/src/adapters/ValueDataSource.tsrecognisedexactly two node shapes — a logical
and/orhead, and a three-elementcomparison — and its operator
switchended indefault: return true. Everyshape and every operator it did not recognise therefore matched every row,
with no error and no console line: a filtered in-memory list came back
unfiltered, which is the silent wider answer an AI-authored metadata app hides
best.
Three distinct routes in, all reachable from a shipped view:
[[rule], [rule]]matched no shape, attop level and as a nested child of
and/oralike. This is whatmergeFilterNodesreturns for a lone surviving source, i.e. the common case,and what
ListView'sfinalFilterputs on$filter. A single-rule flatarray was inert too — it is the shape, not the count.
is_null/is_not_null(and theisnull/isnotnullspellings) selected every row in both dialects.VIEW_FILTER_OPERATORShad no arm either.This one is not on the card and is the reason the fix is shaped the way it
is — see below.
The measurement the card did not have
viewFilterRuleToNode(packages/core/src/utils/filter-converter.ts) lowers astored view's rules through the spec's own
normalizeFilterOperator, whichcanonicalises to
VIEW_FILTER_OPERATORS. So what actually arrives at thematcher from a saved view is the canonical view spelling —
equals,greater_than,starts_with,not_equals— and the old switch was keyed oninfix spellings (
=,!=) plus a few aliases. Of the 20 canonical viewoperators it implemented four:
contains,in,not_in,between. The othersixteen reached
default: return true.That matters for the refusal arm the card asks for. Refusing unknown operators
while implementing only the four null-ness spellings would have flipped
equals— the single most common operator, and the one in the showcase'sshipped
showcase_task.in_progressview — from "every row" to "no rows". Sothe refusal and the vocabulary are one change, not two: the card's own headline
is "teach the consumer the vocabulary the wire already has", and that is what
this does.
The fix
FilterArraySchemadeclares andisFilterASTaccepts: a logical group, a three-element comparison, atwo-element comparison, and a flat list of conditions as an implicit AND.
A flat array is unambiguous because
FilterArrayFieldSchemaforbidsand/oras field names, so a node whose head is itself an array can only be alist.
canonicalAstOperator, so this switch has one arm per operator rather thanone per spelling, and the accepted vocabulary is the published one rather
than a second hand-written list that drifts from it. No second canonical map
is introduced.
is_empty/is_not_emptyfold onto the null-ness armsbecause that is what the spec does with them.
read, so the 2-tuple and the 3-tuple-with-
nullare the same predicate.and logs once per
find()(collected per call, so a 10k-row scan is one line,not 10k). This matches the measured sibling behaviour:
evaluateConditionin@object-ui/permissionsreturnsfalsefrom itsdefaultarm, andReportViewer's formatting switch leavesmatchfalse. The wire-side sibling@object-ui/data-objectstackthrowsMalformedFilterErrorinstead, on thestated ground that dropping one entry of an
andwidens the result set — butit is deciding whether to send a query at all, while this matcher decides
about a single row.
like/ilikeare spec-valid and deliberately refused:no producer in this repo emits them and this matcher has no pattern engine.
Deliberately not changed:
'not in'(with a space) is not a member ofVALID_AST_OPERATORS— the wire would refuse it — but this matcher has alwaysimplemented it and a test pins it, so it is mapped explicitly rather than
deleted by the new refusal arm. Whether to retire that spelling is a separate
question.
No producer changes. Not one request byte moves.
mergeFilterNodesstillreturns a lone surviving source unwrapped; the out-of-scope producer-unification
question the card records on #7221 is untouched.
Red-then-green evidence
The control problem on this card is unusually sharp: because the broken matcher
returns
truefor everything it does not know, a "the filter matched" assertionpasses on both trees. Every new case is therefore written as a row-set
equality whose broken answer is the full set.
Ablation, on the final tree, restoring
d53e472'sValueDataSource.tswhilekeeping the new tests (mutation confirmed on disk by blob hash
0832e566vs HEAD's4ebeef5b, and by both-directiongrep; restoredafterwards with
git diff HEADempty and the on-disk hash equal to the HEADblob):
it.failscases named "diverges — objectui#7221" go red underablation and are converted to plain
itin this PR, retitled"repaired — objectui#7349".
['role', '=', 'admin'], an operator and shape thematcher implemented before this card — passes on both trees (verified by
a separate ablated run,
-t "live control": 2 passed). That is what provesthe 47 reds are about the vocabulary and not about the harness.
Gates, at
224725cpnpm exec vitest run packages/core/ packages/plugin-designer/pnpm --filter @object-ui/core run type-checktsc --listFiles)pnpm lint(repo-wide,eslint .)node scripts/check-changeset-presence.mjs.changeset/value-datasource-ast-filter-vocabulary.md—@object-ui/core: patch.One measurement worth keeping
The
#7221edge-row table expected theNaNrow to surviveis_not_null. Itdoes not, and not because of the operator:
ValueDataSource's constructordeep-clones with
JSON.parse(JSON.stringify(items)), andJSON.stringifywritesNaNasnull, so by the time any filter runs that row genuinely holdsnull.Invisible before this PR, because nothing was ever excluded. Recorded in the
test file rather than worked around.
Generated by Claude Code