fix(core): refuse a bare array comparand in convertFiltersToAST - #8551
Merged
os-justin merged 2 commits intoSep 8, 2026
Conversation
`{ tags: ['a', 'b'] }` used to reach the simple-equality arm and lower to
`['tags', '=', ['a', 'b']]` — an array in a scalar-equality slot that the
spec's doors pass through unjudged, driver-sql refuses with 400
INVALID_FILTER, and every in-memory matcher answers with an empty list.
The author learned nothing at lowering time.
It is now refused HERE with a FilterOperatorError (INVALID_FILTER / 400)
that names the field, prints the comparand, says it is deliberately not
read as membership, and prescribes `{ $in: [...] }` / `{ $nin: [...] }` /
`{ $between: [min, max] }` — the third member of the family `$regex` and
`$not` already belong to. `$in` / `$nin` / `$between` members, `$and` /
`$or` groups and stored ViewFilterRule values keep lowering untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
… shape PR #8529's own header records that a document store reads the array comparand natively; "no backend honoured" overstated it. The changeset now says no RULED contract ever answered the shape. 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 10:57
This was referenced Sep 8, 2026
os-justin
deleted the
claude/issue-8530-filter-converter-array-comparand
branch
September 8, 2026 11:14
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
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
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 #8530
What changed
convertFiltersToAST(packages/core/src/utils/filter-converter.ts) now refuses a bare ARRAY in comparand position —{ tags: ['a', 'b'] }— with aFilterOperatorError(INVALID_FILTER/ 400) instead of lowering it to['tags', '=', ['a', 'b']]. The message names the field, prints the comparand, says the value is deliberately NOT read as membership (and why), and prescribes{ $in: [...] }/{ $nin: [...] }/{ $between: [min, max] }. It is the third member of the family this file already refuses ($regex,$not), per the ruling on the card (comment 5583351371): not lowered toin.$in/$nin/$betweenmembers,$and/$orgroups and storedViewFilterRulevalues (in/between) lower exactly as before — the non-regression section of the pin file is what carries the weight (see the caricature leg below).Why the producer, measured
Against
@objectstack/spec17.3.0 (pinned in the test): the spec's own doors pass the old node through unjudged —isFilterAST(['tags', '=', ['a', 'b']])istrueandparseFilterASThands back{ tags: ['a', 'b'] }— becauseassertListComparandShapesrules only on$in/$nin/$between. So the refusal arrived two layers later (driver-sql's 400 from the wire, or an empty list from the in-memory matchers) and the author learned nothing at lowering time. Contract-first (AGENTS.md #0.1): the producer says it.No shipped producer emits the shape: every multi-value comparand under
packages/*/srcandapps/*/srcis spelled{ $in: [...] }(FilterConditionField.condToMongo,dashboard-filters.buildFilterCondition, theLookupField/PeoplePicker/RecordPickerDialogid restrictions), and no test fixture feeds a bare-array object literal toconvertFiltersToAST/toFilterNode/mergeFilterNodes— ripgrep overpackages/**/src,apps/**/src,examples/**/src: 0 hits; lit control (same regex without the array constraint): 32 hits.Files
packages/core/src/utils/filter-converter.ts— the arm and its@throwsline. Thetypeof value === 'object' && !Array.isArray(value)condition is kept as-is on purpose: with the arm ahead of it, an arm deleted later degrades to the honest pre-fix shape rather than to a nonsense "Unknown filter operator '0'" diagnostic (see the first ablation attempt below).packages/core/src/utils/__tests__/filter-array-comparand-8530.test.ts— 15 pins: 7 refusal pins (envelope on a CAPTURED error, message quality, comparand print, empty array, placement beside siblings and inside$and/$or, sink delegation) and 8 non-regression pins on PRODUCED nodes put throughisFilterAST/parseFilterAST, none of them "does not throw".packages/data-objectstack/README.md— one paragraph under the operator table that documents this lowering. Two rows of that table are stale independently of this card ($nindocumented asnotin,$regexdocumented ascontains); not touched here, reported to the PM..changeset/8530-filter-converter-array-comparand.md—@object-ui/core: patch.Verification
Code head
3fff2fd7e; the only later commit (e8d5804f7) rewords the changeset, and the ratchet gates were re-run on it.pnpm exec vitest run --maxWorkers=2 packages/core/ packages/data-objectstack/→Test Files 186 passed (186),Tests 3514 passed (3514); lock verdictVERDICT command-exit 0.git grep, lit control 12 hits in the sink's own test), includingfields/.../FilterConditionField.operators.test.ts,data-objectstack/src/filter-dialect-wire-7221.test.tsandfilter-entry-translation.test.ts→Test Files 9 passed (9),Tests 250 passed (250).pnpm --filter @object-ui/core type-check(build and test programs) exit 0;pnpm --filter @object-ui/core lint0 errors (531 pre-existingno-explicit-anywarnings, none in the touched files).e8d5804f7:check-changeset-presence✅ (2 source files of 1 released package, 1 changeset declared),check-changeset-fixed✅,check-changeset-no-major✅,check:control-bytes✅ (6758 files),check:doc-fences✅. On3fff2fd7e:check:shell-escape-residue✅,check:unreferenced-sourcesOK,check:vi-mock-specifiers✅,check:doc-example-readersOK.check:readme-exports: NOT MEASURED locally. It exits 1 with "465 self-import(s) could not be judged …./dist/index.d.tsis not on disk -- runpnpm buildfirst" — the whole-repo unbuilt-tree precondition (packages/data-objectstack/README.mdis not among its complaints, and the added paragraph carries no import or export name). CI owns the built run.turbo ls --affectedlists every package (core is a root dependency), and the fullpnpm testis CI's four-shard run. Local scope: core and data-objectstack in full plus every sink-calling test file, with the two ripgrep controls above as the checkable reason nothing outside that scope can change verdict.Ablation and caricature — run from the committed head, not predicted
Both legs mutate the committed file, prove the mutation on disk in both directions, run the pin file, and restore by STATE (
git checkout HEAD -- path, thengit diff HEADempty andgit hash-objectequal to theHEAD:blob92cd70dc3), with a trap onEXIT INT TERMusing absolute paths. The pin file imports../filter-converterrelatively, so nodist/sits in the resolution path and no rebuild is needed between mutation and observation.git diff --stat40 deletions):Tests 6 failed | 9 passed (15). Red: all six refusal pins. Green: the spec-door pin (it pins the spec, not the fix) and all eight non-regression pins.Array.isArray(operatorValue)0 → 1, arm at line 254 before the combinator arm at 277):Tests 7 failed | 8 passed (15). Red: the six non-regression pins that pass through the object arm ($in,$nin,$between, members inside$or,$and/$orgroups, merge with$in) plus "refuses the array wherever it sits" — the caricature refuses{ $or: [...] }on field$orbefore reachingtags. Green under the caricature, i.e. NOT discriminating it: envelope, message quality, comparand print, empty array, sink delegation, spec-door pin, scalar equality — and the stored-ViewFilterRulein/betweenpin, which lowers throughviewFilterRuleToNoderather than the object arm and therefore discriminates a different caricature (one placed in that function), not this one.typeof value === 'object'condition, so arrays fell into the operator loop and were refused as "Unknown filter operator '0'" —Tests 3 failed | 12 passed, with the envelope pin passing on a tree refusing for the wrong reason. The condition was restored and both legs re-run from the amended commit.Related, left open
{ $field }comparands instead of comparing them by reference #8529 — the consumer half (ValueDataSource); this PR is the producer half the card asked for. Refs only.$and/$or/$notarms this one sits beside. Refs only.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code