Repository navigation
fix(core): execute the declared $ operators in ValueDataSource, refuse the rest - #8512
Merged
os-justin merged 2 commits intoSep 8, 2026
Merged
Conversation
…use the rest `matchesFilter` ended its operator switch with `default: break`, which adds no constraint — so an unrecognised operator matched EVERY row, silently. The asymmetry sat inside one `if` in `find()`: an array `$filter` goes to `matchesASTFilter`, which refuses an unknown node, excludes the row and logs it; an object `$filter` went to `matchesFilter`, which admitted it. The object dialect now carries one arm per member of the spec's `FILTER_OPERATORS`, each answering the same question its AST twin answers — `$eq`, `$nin`, `$between`, `$notContains`, `$startsWith`, `$endsWith` and `$null` join the eight already implemented. Everything else is refused through the same `refuseFilterNode` the AST arm uses: excluded, and logged once per distinct refusal per `find()`, with a prescription naming the spelling that works. `$exists` prescribes `$null`; `$regex` / `$options` print the spec's own `RETIRED_FILTER_OPERATORS` guidance verbatim; `$like` / `$ilike` say why they are staged out; `$and` / `$or` / `$not` say that a group belongs in the AST array dialect. Both arms of `find()` now drain ONE refusal collector, which is what makes the two dialects symmetrical rather than merely both non-empty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
github-actions
Bot
added
documentation
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. Which half objected:
📦 Bundle Size Report
Size Limits
|
This was referenced Sep 8, 2026
…oses Contract review on objectui#8447 flipped the one ruling this PR carried against its own census. `$exists` is emitted by two producers in this repo (`FilterConditionField`'s `exists` / `notExists`, kept reachable on purpose by objectui#4736, and `datasetFilterCondition`'s `isEmpty` / `isNotEmpty`) and its semantics are settled: `convertFiltersToAST` lowers `$exists: true` to `is_not_null` and `$exists: false` to `is_null`, three lines from where it lowers `$null` the other way round. Refusing it would have turned "every row" into "no rows" on a filter that looks like it works. The arm is the smaller half. With nothing the spec declares refused by name, `REFUSED_OPERATORS` in the companion test is EMPTY, so the parity guard asserts the case table equals `FILTER_OPERATORS` exactly — a spec release that adds a `$` operator now reddens this file instead of letting the new spelling reach the refusal arm unannounced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
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. Which half objected:
📦 Bundle Size Report
Size Limits
|
os-justin
marked this pull request as ready for review
September 8, 2026 05:15
os-justin
enabled auto-merge
September 8, 2026 05:15
os-justin
deleted the
claude/issue-8447-matchesfilter-unknown-operators
branch
September 8, 2026 05:44
This was referenced Sep 8, 2026
This was referenced Sep 8, 2026
os-justin
pushed a commit
that referenced
this pull request
Sep 8, 2026
…ertFiltersToAST and pin it The README's "Supported Filter Operators" table was a hand-written mirror of convertOperatorToAST's operatorMap and had drifted from the code three ways: `$nin` / `$notin` documented as lowering to `notin` (the map says `nin`, and the worked example's output was a node the server refuses); `$regex` documented as lowering to `contains` (it has been refused with a FilterOperatorError since PR #8512); only the non-spec `$startswith` spelling listed. Re-deriving every row found four supported operators with no row at all — `$notContains`, `$endsWith`, `$null`, `$exists`, the ones the code's own "Supported operators" message enumerates — and no mention of `$and` / `$or` or the `$not` refusal. Every row is corrected against the file as it stands after PRs #8512, #8529 and #8551, the missing rows are added, and two small tables document the combinators and the refusals. A new pin, src/readme-filter-operator-table.test.ts, reconciles the tables against the code on every run: each worked example is executed through convertFiltersToAST and must produce exactly its documented output (or throw the INVALID_FILTER / 400 envelope the refused table promises), each spelling must lower to the operator its row names, and the supported table must carry a row for every key of the operator map (read from source, aliases included) and for every operator the unknown-operator error calls supported. The package tsconfig names `types: ["node"]` so the pin's node:fs / node:url reads type-check in the program that already compiles this package's tests (measured: TypeScript 6.0.3 does not see the root @types/node from here without it). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
This was referenced Sep 8, 2026
os-justin
pushed a commit
that referenced
this pull request
Sep 8, 2026
…g raise could not (objectui#8542) `fa9e76ccd` raised `PER_CHUNK_GZIP_CEILINGS.framework` to 100,000 and recorded, honestly, that it could not say what the bytes bought: "the commits in that window have never been bisected — this checker is a two-build predicate over them and no one has run it." That is the one sentence this change replaces. The bisect has now been run — one `apps/console` build per point, each from the repo ROOT, `framework` read out of the `apps/console/dist/eager-closure.json` the build itself writes: | build | landed by | `framework` gzip | moved by | | `40a7c538a` | #8503 | 70,999 | last GREEN, 1 byte under | | `512c84b16` | #8519 | 70,999 | 0 | | `f76f43628` | #8512 | 71,261 | +262 | | `e76634cc8` | #8529 | 72,245 | +984 | | `e411c3e58` | #8562 | 72,248 | +3 | Two commits own the overage, not one, and the LARGER one is outside the window every card bounded its reasoning to: `e76634cc8` landed after the red had already started and carries 984 of the 1,246 bytes the pair added. A repair scoped to that window would have left `main` red. `512c84b16` emitted a byte-identical chunk to `40a7c538a` — the same `framework-nDOJv2Ij.js` content hash — so the three commits between them moved this chunk by zero, and `270f2825b` touches no file under `packages/(core|react|types)` at all. What the bytes buy is therefore sayable, which is what the gate's own failure message asks of a raise: both contributors are silent-wrong-answer fixes on one file's filter path. `f76f43628` stopped an unrecognised `$` operator adding no constraint and matching every row; `e76634cc8` stopped an array comparand and a `{ $field }` reference being compared by reference, which selected every row under `$ne` and no rows under `$eq` without saying so. No constant moves. The prose that records what the raise COSTS is kept intact, including that the attribution arrived after the ceiling did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Sep 9, 2026
…ertFiltersToAST and pin it (objectstack-ai#8565) The README's "Supported Filter Operators" table was a hand-written mirror of convertOperatorToAST's operatorMap and had drifted from the code three ways: `$nin` / `$notin` documented as lowering to `notin` (the map says `nin`, and the worked example's output was a node the server refuses); `$regex` documented as lowering to `contains` (it has been refused with a FilterOperatorError since PR objectstack-ai#8512); only the non-spec `$startswith` spelling listed. Re-deriving every row found four supported operators with no row at all — `$notContains`, `$endsWith`, `$null`, `$exists`, the ones the code's own "Supported operators" message enumerates — and no mention of `$and` / `$or` or the `$not` refusal. Every row is corrected against the file as it stands after PRs objectstack-ai#8512, objectstack-ai#8529 and objectstack-ai#8551, the missing rows are added, and two small tables document the combinators and the refusals. A new pin, src/readme-filter-operator-table.test.ts, reconciles the tables against the code on every run: each worked example is executed through convertFiltersToAST and must produce exactly its documented output (or throw the INVALID_FILTER / 400 envelope the refused table promises), each spelling must lower to the operator its row names, and the supported table must carry a row for every key of the operator map (read from source, aliases included) and for every operator the unknown-operator error calls supported. The package tsconfig names `types: ["node"]` so the pin's node:fs / node:url reads type-check in the program that already compiles this package's tests (measured: TypeScript 6.0.3 does not see the root @types/node from here without it). Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S Co-authored-by: Claude <noreply@anthropic.com>
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Sep 9, 2026
…g raise could not (objectui#8542) (objectstack-ai#8579) `fa9e76ccd` raised `PER_CHUNK_GZIP_CEILINGS.framework` to 100,000 and recorded, honestly, that it could not say what the bytes bought: "the commits in that window have never been bisected — this checker is a two-build predicate over them and no one has run it." That is the one sentence this change replaces. The bisect has now been run — one `apps/console` build per point, each from the repo ROOT, `framework` read out of the `apps/console/dist/eager-closure.json` the build itself writes: | build | landed by | `framework` gzip | moved by | | `40a7c538a` | objectstack-ai#8503 | 70,999 | last GREEN, 1 byte under | | `512c84b16` | objectstack-ai#8519 | 70,999 | 0 | | `f76f43628` | objectstack-ai#8512 | 71,261 | +262 | | `e76634cc8` | objectstack-ai#8529 | 72,245 | +984 | | `e411c3e58` | objectstack-ai#8562 | 72,248 | +3 | Two commits own the overage, not one, and the LARGER one is outside the window every card bounded its reasoning to: `e76634cc8` landed after the red had already started and carries 984 of the 1,246 bytes the pair added. A repair scoped to that window would have left `main` red. `512c84b16` emitted a byte-identical chunk to `40a7c538a` — the same `framework-nDOJv2Ij.js` content hash — so the three commits between them moved this chunk by zero, and `270f2825b` touches no file under `packages/(core|react|types)` at all. What the bytes buy is therefore sayable, which is what the gate's own failure message asks of a raise: both contributors are silent-wrong-answer fixes on one file's filter path. `f76f43628` stopped an unrecognised `$` operator adding no constraint and matching every row; `e76634cc8` stopped an array comparand and a `{ $field }` reference being compared by reference, which selected every row under `$ne` and no rows under `$eq` without saying so. No constant moves. The prose that records what the raise COSTS is kept intact, including that the attribution arrived after the ceiling did. Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S Co-authored-by: Claude <noreply@anthropic.com>
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 #8447
The defect, in its strongest form
find()picks a matcher on nothing but the shape of$filter— two arms of oneif:$filtershapematchesASTFilterconsole.warns once per distinct refusalmatchesFilterdefault: break— no constraint, so every row passed, silentlySame file, opposite defaults, and nothing tells an author which dialect their filter took.
$eqis the sharpest case:{ age: { $eq: 26 } }selected every row while the plain{ age: 26 }on the sibling equality arm was correct all along.The census that decided refuse-vs-implement
Re-derived on
origin/mainwith'packages/*/src/*'pathspecs and$gtas a lit control (the'packages/*/src'form is a whole-path glob and returns a silent zero for every operator). Producers of an object-shaped$-dialect criteria in this repo:$eqdatasetFilterCondition.ts,ObjectFieldInspector.tsx$ninFilterConditionField.tsx(x2),RecordPickerDialog.tsx,datasetFilterCondition.ts,ObjectFieldInspector.tsx$notContainsFilterConditionField.tsx$startsWith/$endsWithFilterConditionField.tsx$nullFilterConditionField.tsx(both directions)$existsFilterConditionField.tsx(exists/notExists, kept reachable on purpose by objectui#4736),datasetFilterCondition.ts(isEmpty/isNotEmpty)$between$-dialect producer; accepted byconvertOperatorToASTand executed by the AST twin in this same file$like/$ilikeFILTER_OPERATORSon purpose$regex/$optionsfilter-converter.tsalready throws on$regex)$betweenis the one place the strict census reading ("a spelling nothing emits, refuse-and-warn") was widened, deliberately and declared:matchesComparisonNodeimplementsbetween130 lines above in the same file, so refusing it in the object dialect would re-create this card's own asymmetry one level down.What changed
matchesFilternow carries one arm per member of the spec'sFILTER_OPERATORS— all sixteen, a one-to-one pairing with the AST arms that already existed:$existsis the exact inverse of$null, and that is the platform's own reading rather than one invented here:convertFiltersToASTlowers$exists: truetois_not_nullthree lines below where it lowers$null: truetois_null.The closed vocabulary is worth more than any one arm
Because nothing the spec declares is refused by name,
REFUSED_OPERATORSin the companion test is empty, so the parity guard can assert the case table equalsFILTER_OPERATORSexactly. A future spec release that adds a$operator now turns this file red instead of letting the new spelling reach the refusal arm unannounced. An operator parked on a refused list would have been a hole that guard could not see into.What is still refused — everything outside the declared vocabulary
Through the same
refuseFilterNodethe AST arm uses — excluded, and logged once per distinct refusal perfind()— with a message that names the spelling that does work:$like/$ilike, declared byStringOperatorSchemabut deliberately staged out ofFILTER_OPERATORS(objectstack#7536); this matcher has no pattern engine, and the spec's own note says naming them before an arm exists turns a loud refusal into a dropped predicate;$regex/$optionsprintRETIRED_FILTER_OPERATORS' prescription verbatim, the way the five driver-side refusal sites do, so six faces say one thing;$startswith,$notcontains,$notin) and the off-spec$ncontainsare refused rather than aliased — growing alias arms in the renderer is exactly the second de-facto contract AGENTS.md #0.1 forbids;{ profile: { verified: true } }) is refused by the key it could not read; this matcher does not descend into relations.Both arms of
find()now drain one refusal collector. That is what makes the two dialects symmetrical, rather than merely both non-empty.Combinators — stated as their own case, not swept in
$and/$orcarry an array, so they fell to the simple-equality branch and were already excluding every row, silently. The rows do not move here; only the silence does.$notcarries an object (FilterConditionSchema, not an array), so it entered the operator branch, its inner field names were read as operator names, and each hitdefault: break. It matched every row and now matches none.None of the three is implemented: executing a group in this dialect has its own semantics to settle (the empty-group identities, objectstack#5322, and
$not's NULL-safe rule, objectstack#5146). The refusal names the AST array$filteras the door that works today, and there is a test proving that door is open.Pin discipline — both ablations were RUN, and RE-RUN after the flip
Restored by state each time (
git diff HEADempty andgit hash-object PATHequal togit rev-parse HEAD:PATH), from a committed implementation, with the mutation proven on disk by a marker count before and after plus a blob-hash inequality.ValueDataSource.tsrestored to its pre-card bytesmatchesFilterreturnsfalseunconditionally (an empty result set, strictly worse than the bug)The survivor list was re-derived, not carried across the
$existsflip. Exactly one case moved, and it is the one the review predicted:`$exists false` selects neither rowwas RED under the caricature before the flip and is GREEN after it — it discriminated only because it asserted the refusal was logged, and executing the operator removed the log. Nothing else moved; the red count is 54 in both runs.The eight survivors are named rather than counted: the two
{}cases and the AST-group case never reachmatchesFilterat all; theFILTER_OPERATORSparity guard reads no rows; and the remaining four are the card's own$nin/$startsWith/$null/$existsexamples — the quotable ones, and none of them can tell this fix from an empty result set. They are kept in the test file's section 5 and labelled a scope declaration, not proof. Section 5's last case is the exception that keeps the block from being decorative: it asserts the complement of all four, which is four non-empty answers.The weight is carried by the non-regression axis: every executed operator asserts a non-empty proper subset of the three rows, so the full set (the bug) and the empty set (the caricature) are both red; the plain-equality arm and a genuinely matching row have their own controls; every operator is additionally asserted to answer the same rows as its AST twin; and
$exists/$nullare pinned as one predicate read from opposite ends, including anot.toEqualso an arm with the boolean inverted cannot pass.Verification
Every row below was run on this branch's final commit,
09374d601.pnpm exec vitest runon the new fileValueDataSource.dollarFilterVocabulary.test.tspnpm exec vitest run packages/core/pnpm exec vitest runover data-objectstack + fields + components tests + plugin-list + app-shell metadata-admin inspectors — every package or directory in the repo that mentions$existspnpm --filter @object-ui/core run type-check(both programs:tsc --noEmitandtsc -p tsconfig.test.json)pnpm --filter @object-ui/core run lint(eslint ., the unitturbo run lintruns)node scripts/check-changeset-presence.mjscheck:control-bytes·check:spec-symbols·check:doc-fences·check:vi-mock-specifiers·check:vi-mock-inherit·check:unreferenced-sourcescheck:readme-exportspnpm buildfirst ("type entry./dist/index.d.tsis not on disk") and reports 532 unjudged self-imports across every package, none of them this diff's. Declared to CI.On the first commit of this branch the wider consumer set was also green — core + fields + data-objectstack + types + plugin-designer + plugin-map + plugin-list + plugin-gantt + examples/schema-catalog (669 files, 12322 passed) and react + app-shell + core/adapters (731 files, 7393 passed, 1 skipped). The second commit's only behavioural delta is the
$existsarm, and no file outsidepackages/coreboth constructs a value data source and mentions$exists(measured: the two matches are a CHANGELOG and a comment infilter-entry-translation.test.ts, which constructs none). The whole-repopnpm test(4 shards) andturbo run lintare CI's runs.Lint narrowing is a measurement, not an omission: the population is
packages/core's owneslint .(the exact unit CI'sturbo run lintruns for the only package this diff touches), the count is eslint's own (0 errors / 526 warnings), and type-aware linting is not enabled —eslint.config.jsusestseslint.configs.recommendedwith noparserOptions.projectorprojectService— so this diff cannot move the verdict of any file it does not touch.Contract review — two corrections against the dispatch brief, both accepted
Recorded because the reasoning is load-bearing for the census, not to relitigate.
$notdoes not fall to the equality arm. The brief said$and/$or/$notall carry an array and therefore all fail closed.FilterConditionSchemadeclares$not: FilterConditionSchema— an object — so$notreacheddefault: breakand matched every row. It was in this card's defect class after all. Fixed here as a refusal, with its own case. Accepted.$existswas refused on a falsified ground, and the ruling is now flipped. The brief refused$existsbecausefilter-converter.test.ts:31pinsconvertOperatorToAST('$exists')tonull. That map contains neither$nullnor$exists; both are special-cased before it is consulted (:283and:287), so the pin distinguished nothing between the operator to refuse and the operator to implement. The first commit of this branch shipped the ruling as written and flagged the flip; the second commit implements$exists, per the review.Out of scope — found here, filed by the PM
Both dedupe channels were down for this seat (
search_issues:API rate limit already exceeded; repo-scoped REST: 403), so these were handed over rather than filed blind:filterGroupToMongoinFilterConditionField.tsxemits{ $or: [...] }/{ $and: [...] }, so a value-provider list given that criteria returns zero rows. Pre-existing; this PR makes it explicit instead of silent.{ tags: ['a','b'] }compares by reference, so it excludes every row even when the stored array is deep-equal. Silent and fail-closed; untouched here because it is the equality branch, not thedefaultarm.{ $field: path }comparand. Declared by the spec and produced by@objectstack/formula; both matchers compare the reference object itself. Symmetric across the two arms, so not this card's asymmetry — but after this PR the$side excludes rather than passes, which makes it visible.Drafted by the
domain:uidev seat, sessionhttps://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S. (Written into the prose deliberately: aPATCHdowngrades a session-URL attribution footer to the bare form, so a PR body that is edited after creation loses the reference unless it is carried as a code span.)