fix(core): ValueDataSource's contains is case-sensitive and icontains is its ASCII-folding twin - #8437
Merged
Conversation
…ins` is its ASCII-folding twin The in-memory matcher lower-cased both sides of `contains`, `not_contains`, `starts_with`, `ends_with` — and `icontains`, whose arm was stacked onto the `contains` one. So `contains` executed `icontains`, the two spellings named one predicate, and a `provider: 'value'` list filtered with `contains` returned strictly more rows than the same filter run against a real driver. Nothing errored; the list was just longer. `$contains` is contractually case-SENSITIVE (objectstack#4706 Q2 = A) and `$icontains` folds ASCII ONLY (Q1 = A). All five backends plus objectql's `having` matcher import `FILTER_TEXT_CASES` and answer those rows; this adapter was the last face that did not. - The four AST arms compare exactly. `VALID_AST_OPERATORS` has `icontains` and no other `i`-prefixed spelling, so case-sensitive is the only reading the other three have — and the one `not_contains` needs so no row can fail an operator and its negation. - `icontains` takes the spec's `asciiCaseInsensitiveContains`: `toLowerCase()` is the full Unicode fold, a promise the SQLite-backed drivers cannot keep. - The `$` dialect follows: `$contains` compares exactly and `$icontains` gains an arm — it had none, and an unrecognised `$` operator in that switch adds no constraint, so it used to select every row. The pin asserts row SETS in pairs over one fixture: the case-differing row `contains` must exclude, and the `icontains` query that must still include it. 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. 📦 Bundle Size Report
Size Limits
|
os-justin
marked this pull request as ready for review
September 7, 2026 23:24
os-justin
enabled auto-merge
September 7, 2026 23:24
Collaborator
Author
|
PM contract review — accepted, flipped to ready, auto-merge armed. The one thing you flagged for me: keep
|
os-justin
deleted the
claude/issue-7379-valuedatasource-contains-case
branch
September 7, 2026 23:55
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 #7379
The escalation fence resolves to DEFECT, and here is the measurement that decides it
The card's fence: if the spec keeps
containsandicontainsdeliberately distinct andValueDataSourceis the odd one out among the drivers, it is a defect; if it is the only driver, or the drivers disagree, it is a product ruling. It is the first. The sibling drivers' behaviour, not the spec text:$containscase$icontainsdriver-sqlFILTER_TEXT_CASES(sql-driver-text-case-conformance.test.ts,sql-driver-icontains-and-retired-operators.test.ts)driver-sqlite-wasmsqlite-wasm-icontains-and-retired-operators.test.ts)driver-tursoturso-local-remote-text-parity.test.ts)driver-mongodbmongodb-filter-text-conformance.test.ts)driver-memorymemory-filter-text-conformance.test.ts)havingasciiCaseInsensitiveContainshaving-filter.ts:413/:455, pinned byhaving-filter-text-conformance.test.tsformulamatches-filter-icontains.test.ts@object-ui/coreValueDataSourcecontainsThe canonical rows those eight faces answer are
FILTER_TEXT_CASESin@objectstack/spec/data, whose$contains is case-SENSITIVEcase reads:{ name: { $contains: 'acme' } }overACME Corp/acme corpselects row 2 only. So the disagreement is with the other implementations, not with a document.Two things I measured rather than assumed, both by executing the installed
@objectstack/spec17.3.0:asciiCaseInsensitiveContains('CAFÉ', 'café')isfalse, while'CAFÉ'.toLowerCase().includes('café')istrue. The fold is ASCII-only by ruling (objectstack#4706 Q1 = A) because three of five backends are SQLite underneath and itslower()folds ASCII only.itwin exists for the other three operators.VALID_AST_OPERATORS.has(...)answered:icontainstrue;istartswith,iendswith,noticontains,not_icontainsall false. The triage seat asked for exactly this and told me not to assume duals — there are none, so case-sensitive is the only readingstarts_with/ends_with/not_containshave.What changed
Five arms in
packages/core/src/adapters/ValueDataSource.ts, all of which lower-cased both sides:contains,not_contains,starts_with,ends_withcompare exactly. Judged as one group per the triage seat's boundary — a case-exactcontainsbeside a foldingnot_containsletsACME Corpfail an operator and its negation.icontainsgets its own arm and takes the spec's exportedasciiCaseInsensitiveContains. Before this it shared thecontainsbody, so PR fix(core): teach ValueDataSource's matcher the filter vocabulary the wire already has #7377's mapping made the two identical; now it is the one case-insensitive member, folding ASCII on both sides.$-dialect matcher (matchesFilter) follows:$containscompares exactly, and$icontainsgains an arm. It had none — an unrecognised$operator in that switch reachesdefault: break, which adds no constraint, so{ name: { $icontains: 'acme' } }used to select every row. Making$containsexact without adding its twin would have left that dialect with no working case-insensitive door.The direction is written at the arm (the card's third acceptance criterion), naming the ruling and the drivers, so the next reader does not take it for a typo.
Deliberately not touched:
canonicalAstOperator(#7349 / PR #7377's work, and correct), and the$searchpath'stoLowerCase()— full-text search is case-insensitive by ruling (objectstack#7641), a different question from a filter operator.The pin
packages/core/src/adapters/__tests__/ValueDataSource.textOperatorCase.test.ts, 14 cases. Every assertion is a row-set equality, never an operator-name check: a matcher that returns every row constructs exactly the right operator string.Both halves over one fixture (
ACME Corp/acme corp/CAFÉ/café):['name','contains','acme']selects['lower'];ACME Corpmust not come back.['name','icontains','acme']over the same fixture selects['upper','lower'], i.e. the rowcontainsjust dropped.Would an implementation strictly worse than the bug pass it? No, and that is its own named case —
a matcher answering [] for everything FAILS these cases, not just passes the exclusions. Ablation leg 2 below proves it fires.Ablation — two legs, both from the committed implementation
Leg 1 — restore the case-fold at the
containsREAD SITE. On-disk proof:HEAD:...ValueDataSource.tsblob2722d1ba8dff3cc8b5d7defe81b2c556b86c93ca, mutated on-diskgit hash-object2a25e57da0dd179b64430a25cdf5258c58b892f6— different, so the mutation reached disk. Result5 failed | 9 passed, red by name:Leg 2 — the strictly-worse implementation: both dialects'
icontainsanswer nothing. Blob2722d1ba…vs mutated4a0f83a7acf71bf175580839f56492007adaa149. Result6 failed | 8 passed, headed by:Both legs restored by state, not by exit code:
git diff HEADempty,git status --shortempty, and on-diskgit hash-objectback to2722d1ba…. The pin re-ran14 passed (14)afterwards. No rebuild step is involved: the pin imports../ValueDataSource(relative source), so there is nodist/hop that could stale the reading.Verification, on the final commit
c889b18a7pnpm exec vitest run packages/core/plus the 67 other test files that reach a value-provider datasource:Test Files 190 passed (190)·Tests 3364 passed (3364).ValueDataSource, so a test can only observe it by (a) reaching that adapter and (b) passing one of the six operator spellings. The union above is condition (a) over the whole repo — 76 files across 12 packages, every one run.turbo ls --affectednames 38 packages because@object-ui/coreis a base dependency; those are declared to CI's four shards.pnpm --filter @object-ui/core type-check(tsc --noEmit && tsc -p tsconfig.test.json): exit 0. The new file is in the program —tsc -p tsconfig.test.json --listFileslists it (1 hit), lit controladapters/ValueDataSource.ts1 hit, negative control 0 hits.eslint .inpackages/core:518 problems (0 errors, 518 warnings), all pre-existingno-explicit-any.node scripts/check-changeset-presence.mjs:✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/wise-poems-behave.md.node scripts/check-changeset-no-major.mjs:✅ No changeset declares a 'major' bump.Changeset level:
minorScored
minorbecause this moves shipped results — metadata relying on the lenient matching gets fewer rows and no error — and the repo forbidsmajor(every package is in one fixed group, so one major carries all of them off the pinned@objectstackmajor;check-changeset-no-major.mjsenforces it).patchwould be wrong: this is not a fix that leaves behaviour where it was. The changeset body names the migration explicitly — a filter that means "match regardless of case" is authored asicontainsor$icontains, and both now execute.Dependency range: checked, no bump needed
packages/coredeclares"@objectstack/spec": "^17.2.0".asciiCaseInsensitiveContainsis present in 17.2.0's publishedapi-surface/data.jsonand itsdist/data/index.d.ts(read from the 17.2.0 tarball; positive controlcanonicalAstOperatorpresent, negative control absent). So the declared range is honest and stays.Two out-of-scope findings, both MEASURED, neither fixed here
Reported for filing —
search_issuesreturnedAPI rate limit already exceeded for user ID 323634890on both dedupe queries, and repo-scoped REST is 403 for this seat, so I could not search and therefore did not file.A.
matchesFilterwaves through every$operator it does not recognise. Same defect class as objectui#7349, in the dialect #7349 did not touch. Measured over rowsn(score: 5) ands(score: '5'):The implemented set is
$gt $gte $lt $lte $ne $in $containsplus the$icontainsthis PR adds; everything else is silently no-constraint. Not fixed here: closing it means refusing unknown operators the waymatchesASTFilterdoes, which moves results for every one of those spellings — its own card, its own escalation.B.
not_containsanswers NO for a stored value that is not a string, so a row fails the operator AND its negation. Measured:['score','contains','5']selects['s'](correct — a number cannot contain a substring) but['score','not_contains','5']selects[], dropping rownfrom the negation too. The platform ruled this cell on 2026-09-05 (objectstack#14079, option A): a non-string never satisfies a positive text operator and does satisfy$notContains.FILTER_TEXT_CASESpins it on every face;ValueDataSourcehas exactly the shape the reference matcher had before that card. Different axis from this one (stored-value type, not case folding), so it is not ridden in here.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code