Repository navigation
fix(core): a non-string stored value satisfies not_contains instead of failing the operator and its negation both - #8599
Merged
Conversation
…than failing it and its negation both `ValueDataSource`'s `not_contains` and `$notContains` arms were written as `typeof value === 'string' && !value.includes(target)` — a TYPE test standing in for the predicate. A row whose column held the number 5 failed `contains '5'` (correct: a number cannot contain a substring) and also failed `not_contains '5'` (wrong: for that same reason it does not contain it), so the row was excluded from both halves of a partition. No filter answer included it, and the opposite filter — the one thing a user has to debug a missing row with — was silent too. Both arms are now the exact complement of `contains` / `$contains`: `!(typeof value === 'string' && value.includes(String(target)))`. The positive text operators are unchanged and keep their type gate; that is the other half of the same ruling, not a leftover. objectstack#14079, maintainer ruling 2026-09-05, option A, as carried by `filter-text-conformance.ts` in `@objectstack/spec`: "a stored value that is not a string never satisfies a positive text operator (`$contains` / `$startsWith` / `$endsWith` / `$icontains` / `$like` / `$ilike`) and satisfies `$notContains` — complementarity holds, on every face." Measured before the change, over a fixture of eight rows mixing numeric, string, boolean, null and absent values: `['score','contains','5']` answered `['s5']` and `['score','not_contains','5']` answered `['sx']` — six of eight rows appeared in neither half. After: the same two filters partition all eight rows exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…t-contains-non-string
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. 📦 Bundle Size Report
Size Limits
|
os-justin
marked this pull request as ready for review
September 8, 2026 15:17
This was referenced Sep 8, 2026
This was referenced Sep 9, 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 #8452
What was wrong
ValueDataSource's two negation arms —not_contains(AST / infix dialect,matchesComparisonNode) and$notContains($dialect,matchesDollarOperator) — were both written as:The leading
typeofis a type test standing in for the predicate. A row whose column holds the number5failscontains '5'(correct — a number cannot contain a substring) and also failsnot_contains '5'(wrong — for that same reason it does not contain it). The row is excluded from both halves of a partition: it appears in no filter answer at all, and the opposite filter, the one thing a user has to debug a missing row with, is silent too.Measured on
21529629cover an eight-row fixture mixing numeric, string, boolean,nulland absent values:['score','contains','5']['s5']['s5']— unchanged['score','not_contains','5']['sx']['n5','n50','n0','sx','bool','nul','gone']Six of the eight rows were in neither half.
The direction is ruled upstream, not decided here
objectstack#14079, maintainer ruling 2026-09-05, option A. Quoted verbatim from the contract that carries it —
packages/spec/src/data/filter-text-conformance.tsin@objectstack/spec, section "A stored value that is not a string":FILTER_TEXT_CASESpins it as five rows over a numericscorecolumn, anddriver-memory's reference matcher — the face the defect was originally measured on — now answers the predicate:This adapter's two arms are that same expression, negated the same way:
No second dialect was needed and none was invented. The only other statement about this operator in the package is
filter-converter.ts's$notrefusal, which says the negated operators "follow each operator's own answer for a missing value" — this change is what supplies that answer here, so the two files agree.The same axis on the other text operators — checked, already conformant
The card asked for the whole class, not one spelling. Both dialects were swept; the vocabulary has exactly one negated text operator.
contains/$containsicontains/$icontainsstarts_with/$startsWithends_with/$endsWithnot_contains/$notContainslike/ilikeVALID_AST_OPERATORSandALL_OPERATORS(read from the installed@objectstack/spec) carry no other negated text spelling — nonot_starts_with, nonot_icontains— so the class is shut by these two arms. The type gate on the positive family is deliberately kept: it is the other half of the same ruling, not a leftover to clean up, and the code comment says so.Evidence — four legs, each mutation proven on disk in both directions
Harness:
pnpm exec vitest run packages/core/src/adapters/__tests__/ValueDataSource.nonStringStoredValue.test.ts, from the repo root, paths not behind a bare separator. Every leg restores by state:git hash-object PATHcompared againstgit rev-parse HEAD:PATH, plusgit diff HEADempty; the script traps onEXIT INT TERMwith absolute paths.AssertionErrorlinestypeofspellingreturn trueunconditionallyLEG C is the caricature the card named — "
not_containsreturns true for every value" restores the partition and makes the operator mean nothing. It is not predicted here, it was executed. Three cases go red in C and not in A, and they are precisely the string-side, load-bearing ones:`not_contains` still EXCLUDES a real string that contains the comparand`$notContains` still withholds the string that contains the comparandthe inclusion half is non-empty and the exclusion half is not everythingOne case goes red in A and not in C —
a null and an absent key both satisfy not_contains— because the caricature happens to admit those rows. So every discriminating assertion was observed to fail in the leg it was written for; none of them passed by comparing one absent thing to another.LEG D is the harness-death control, and it corrected the instrument rather than confirming it. The alternation borrowed from objectui#8582 (
Cannot read properties of null,Unable to find,TypeError,is not a function) matched 0 lines on this repo's empty-file mode: vitest here reportsFailed Suites/Error: No test suite found in file/Tests no tests, none of which is a TypeError. Counted with the narrow alternation alone, leg D would have read as "a clean run with zero assertion failures" — the exact misreading the leg exists to prevent. The widened alternation (addingNo test suite found,Failed Suites,Tests no tests) reads 3 on D and 0 on B, A and C, so it separates the two failure modes without firing on a real red.Nothing in this change refuses anything, so objectui#8530's envelope-only-refusal hazard has no surface here: there is no new error code, status or message to pin.
Other checks
Run from the repo root, exit codes captured before any pipe.
pnpm exec vitest run packages/core/— 132 files, 2788 tests, all pass (re-run after merging currentmainin).pnpm exec vitest runover the eight packages that consumeValueDataSource(react,types,data-objectstack,plugin-designer,plugin-map,plugin-gantt,plugin-list,fields) — 592 files, 8505 tests, all pass.pnpm --filter '@object-ui/core^...' buildthenpnpm --filter @object-ui/core type-check— exit 0, both programs (tsc --noEmitandtsc -p tsconfig.test.json). Verified withtsc -p tsconfig.test.json --listFilesthat the new test file really is in the test program: 1 hit, and the control (the siblingtextOperatorCasefile) also 1.eslint .over the whole ofpackages/coreatc3fbc7b14— 231 files in eslint's own selection, 0 errors, 532 warnings, all pre-existingno-explicit-any. The new test file contributes exactly 1 warning, identical to the siblingValueDataSource.textOperatorCase.test.tsbaseline.check:control-bytesOK (6789 files);check:doc-fencesOK;check-changeset-presence/check-changeset-fixed/check-changeset-no-major/check-changeset-overwriteOK;check:vi-mock-specifiers/check:vi-mock-inherit/check:unreferenced-sourcesOK.check:control-bytesdoes not cover zero-width spaces inside string literals, which is why the separate scan exists.node scripts/check-governed-queue-guard.mjs --testover the four changed paths: NOT GOVERNED — an ordinary pull request.Declared narrowing: the repo-wide
turbo run lintand the fullpnpm testfarm are left to CI. The diff touches one package, andeslint.config.jsconfigures no type-aware linting (0 hits forprojectService/parserOptions/project:; lit controlrulesreads 11), so this diff cannot move the verdict on any file it does not edit.History cited by sha
Verified with
git log -1andgit show --statin a non-shallow checkout rather than taken on trust:f76f43628—$-operator vocabulary and therefuseFilterNodeidiom,ValueDataSource.tse76634cc8— comparand shapes (array by reference,{ $field })f5cfbbd81— both filter-converter arms gate the comparand617707a48—$and/$orlowered to real AST group nodesrefuseFilterNodeis defined atValueDataSource.ts:65;FilterOperatorErroratfilter-converter.ts:53. Different files, different conventions — this change touches neither idiom.Base
Branched from
21529629c; currentmain(70c452369) was brought in by merge, not rebase, and the branch has never been force-pushed. The merge touched nothing this change depends on, andpackages/corewas re-verified afterwards.Not addressed here, deliberately
ValueDataSourceinFILTER_TEXT_CASESitself. That table's own rule 2 is that a face's rows join it in the pull request that shuts the gap, and enrolling means answering all of it, including the folding and comparand-literalness rows. Out of scope for this card; recorded for the PM instead.$dialect refuses what it does not recognise, so no unimplemented arm returns every row underneath this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code