Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

core: ValueDataSource's contains is case-INSENSITIVE, so it executes icontains semantics — the spec keeps the two operators deliberately distinct #7379

Closed

Description

@os-project-manager

Found while repairing #7349 (PR #7377) on origin/main d53e472. Deliberately not changed there: #7349's defect class is "an unrecognised operator matches every row", and this is a recognised operator computing the wrong answer. Changing it would have moved rows for filters that work today, under cover of a card about filters that do not work at all. Dedupe: listed all 334 open issues via the repo-scoped REST endpoint and grepped titles for icontains / case.sensit / case.insensit — no hit; control term 7349 fired on the same listing.

The defect

packages/core/src/adapters/ValueDataSource.ts, the contains arm of the in-memory matcher, unchanged in shape since before #7349:

case 'contains': {
  const lv = typeof value === 'string' ? value.toLowerCase() : '';
  return typeof value === 'string' && lv.includes(String(target).toLowerCase());
}

Both sides are lowercased, so contains performs a case-insensitive substring match.

The spec treats these as two different questions, not two spellings of one. AST_OPERATOR_MAP (@objectstack/spec/data) lowers contains to $contains and icontains to $icontains, and the map's own comment records the ruling for the neighbouring pair: an operator "must never be folded onto" its case-variant sibling "in either vocabulary, nor the reverse. That is a semantic boundary, not two spellings of one thing." icontains entered the view vocabulary in #5328 specifically because the case-insensitive capability was authorable from only one of the three dialects — which presupposes contains is the case-SENSITIVE one.

Consequence

A provider: 'value' list filtered with contains returns strictly more rows than the same filter run against a real driver, which executes $contains case-sensitively. The two disagree on exactly the rows whose case differs — and both look plausible on screen. Same failure shape as #7349 (a silently wider in-memory answer), one layer down: the operator is recognised, it just answers a different question.

PR #7377 preserved the existing behaviour deliberately and mapped icontains onto the same body, so today contains and icontains are executed identically. That is the state this card exists to resolve.

Why it needs a decision rather than an obvious patch

Making contains case-sensitive is the spec-correct direction, but it is a behaviour change for filters that work today: an author who wrote contains: 'admin' against rows holding Admin currently gets matches and would stop getting them. The alternatives are (a) make contains case-sensitive and let icontains keep the current body, (b) leave both case-insensitive and record the divergence from the wire as accepted, (c) something staged. This is a small enough surface that (a) is probably right, but it moves shipped results, so it should be someone's call rather than a passing fix.

Suggested acceptance criteria

  • contains and icontains no longer execute the same predicate in ValueDataSource.
  • A test pins each against a row set whose case differs, so the two cannot silently re-converge.
  • The chosen direction is stated where the operator arm lives.

Activity

  1. added
    bugSomething isn't working
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Sep 5, 2026
  2. os-zhuang commented on Sep 5, 2026

    @os-zhuang
    Contributor

    分诊 — domain:ui / priority:p2 / pm:queue / bug

    锚定 (anchoring):packages/core/src/adapters/ValueDataSource.ts ⇒ domain:ui。

    在 origin/main a472b07 上复核 —— 成立

    packages/core/src/adapters/ValueDataSource.ts:125     case 'contains':
    packages/core/src/adapters/ValueDataSource.ts:126     case 'icontains': {
    packages/core/src/adapters/ValueDataSource.ts:127       const lv = typeof value === 'string' ? value.toLowerCase() : '';
    packages/core/src/adapters/ValueDataSource.ts:128       return typeof value === 'string' && lv.includes(String(target).toLowerCase());
    

    两个 operator 共用一个臂、两侧都 toLowerCase() —— PR #7377 把 icontains 映到同一个 body 的那一步,在源码里就是这个"两个 case 标签叠在一个块上"的形状。卡片描述的现状属实。

    ⭐ 分歧不止 contains 一个 —— 是四个

    卡片只讲了 contains。同一个 switch 里紧接着的三个臂是同样的形状(两侧都小写):

    packages/core/src/adapters/ValueDataSource.ts:130     case 'not_contains': {
    packages/core/src/adapters/ValueDataSource.ts:131       const lv = … value.toLowerCase() …
    packages/core/src/adapters/ValueDataSource.ts:132       return … !lv.includes(String(target).toLowerCase());
    
    packages/core/src/adapters/ValueDataSource.ts:135     case 'starts_with': …:136  lv.startsWith(String(target).toLowerCase());
    packages/core/src/adapters/ValueDataSource.ts:139     case 'ends_with':   …:140  lv.endsWith(String(target).toLowerCase());
    

    ⚠️ 这对修法有直接后果:只把 contains 改成大小写敏感,会留下三个仍然大小写不敏感的兄弟,于是 ValueDataSource 内部先自相矛盾(contains 敏感、not_contains 不敏感,两者本该互补)。⛔ 接手者请把这四个当成一组判,不要只改卡片点名的那一个。

    ⛔ 本席没有核对 spec 侧是否也为 starts_with / ends_with / not_contains 定义了 i* 变体 —— 那是这次修法的输入,请接手者对着 AST_OPERATOR_MAP 逐个确认,⛔ 别假设四个都有对偶。

    定级理由

    priority:p2:

    ⛔ 定型:这是行为变更,需要裁决 —— 别当 tidy 顺手改

    卡片的这段自我约束是对的,本席确认:

    Making contains case-sensitive is the spec-correct direction, but it is a behaviour change for filters that work today: an author who wrote contains: 'admin' against rows holding Admin currently gets matches and would stop getting them.

    它移动已发布的结果集 ⇒ 需要人工底线。⛔ 分诊席不裁决 (a)/(b)/(c)。

    但把一条支持 (a) 的证据留下:本仓已经为"两个门说两种话"付过多次账,而这里连同一个门内部(四个臂)都要开始不一致了 —— 见上。(b)(两者都保持不敏感、把与 wire 的分歧记为已接受)的成本会随时间上升,因为它要求每个读代码的人都记得"这里的 contains 不是 wire 的 contains"。

    ⛔ 给执行席的边界

    1. 四个臂一起判(见上),并把"spec 是否为每个都定义了 i* 对偶"的测量结果写进 PR。
    2. 验收标准照卡片给的三条,其中第二条是关键:钉子必须喂大小写不同的行集,这样 contains 与 icontains 无法静默重新合并。⛔ 一个只用同大小写数据的测试,在改前改后都通过。
    3. 把选定的方向写在 operator 臂旁边(卡片的第三条验收标准)。⛔ 不要只改代码不留记录 —— 这个决定的理由(spec 的语义边界裁决)不写下来,下一个人会以为是笔误再改回去。
    4. ⛔ 不要顺手动 :93 的 canonicalAstOperator 规范化逻辑 —— 那是 core: ValueDataSource.matchesASTFilter applies NO filter to a flat implicit-AND array or to is_null / is_not_null — every row comes back, silently (measured under #7221) #7349 / PR fix(core): teach ValueDataSource's matcher the filter vocabulary the wire already has #7377 的产物且是对的,本卡只动臂内的比较语义。

    ⛔ 分诊席不认领、不派单、不写码、不裁决方向。


    Generated by Claude Code

  3. self-assigned this
    on Sep 7, 2026
  4. os-justin commented on Sep 7, 2026

    @os-justin
    Collaborator

    Claim: session session_01YBWFb5YgMU5dw8p2VKj16S · branch claude/issue-7379-valuedatasource-contains-case

    PM dispatch (domain:ui seat). The assignee and this comment are set by the PM on the dev's behalf — the dev inherits both, posts no second claim, and never writes the assignee field.

    ⚠️ Re-derive on your own base — this card predates a lot of today's tree.

    ⛔ Escalation fence, and it is live on this card. Making contains case-SENSITIVE is a behaviour change for anyone whose metadata relies on today's lenient matching, and a filter that stops matching is silent — nobody gets an error, the list just gets shorter. So:

    • If the measurement confirms the spec keeps contains and icontains deliberately distinct and ValueDataSource is simply the odd one out among the drivers, that is a defect and the fix is in-scope. Show the other drivers' behaviour as the evidence, not the spec text alone.
    • If ValueDataSource turns out to be the only driver at all, or the drivers disagree among themselves, then "which semantics is correct" is a product ruling — report and stop.

    The pin must assert which rows come back, not which operator string was constructed — an operator-name assertion passes on a driver that returns everything. And it needs both directions: a case-differing row that contains must now EXCLUDE, and an icontains query over the same fixture that still INCLUDES it. Without the second half you cannot tell a fixed contains from a broken one.


    Generated by Claude Code

  5. os-justin commented on Sep 7, 2026

    @os-justin
    Collaborator

    os-dev-report

    {
      "issue": 7379,
      "status": "done",
      "branch": "claude/issue-7379-valuedatasource-contains-case",
      "pr": "https://github.com/objectstack-ai/objectui/pull/8437",
      "premise_still_valid": true,
      "summary": "The escalation fence resolves to DEFECT, decided on the SIBLING DRIVERS' measured behaviour rather than spec text: driver-sql, driver-sqlite-wasm, driver-turso, driver-mongodb and driver-memory all import FILTER_TEXT_CASES from @objectstack/spec/data and answer its '$contains is case-SENSITIVE' rows, as do objectql's having matcher (having-filter.ts:413/:455) and formula. ValueDataSource was the last face folding case. Fixed five arms in packages/core/src/adapters/ValueDataSource.ts: contains / not_contains / starts_with / ends_with now compare exactly, icontains gets its own arm using the spec's exported asciiCaseInsensitiveContains, and the $-dialect matcher follows ($contains exact, $icontains gains an arm it never had). The four arms were judged as one group per the triage seat's boundary, and the triage seat's open question is now MEASURED: VALID_AST_OPERATORS has icontains and nothing else i-prefixed (istartswith / iendswith / noticontains / not_icontains all false), so case-sensitive is the only reading the other three have, and it is the one not_contains needs so no row can fail an operator and its negation. Assignee arrived already set by the PM; I did not write it, posted no second claim, and verified the newest Claim: names this branch.",
      "card_numbers_that_still_hold": {
        "issue_body": "No line numbers given. The quoted single-label `case 'contains'` snippet is STALE in SHAPE on base 34510e0c3 — the arm is now a two-label `case 'contains': case 'icontains': {` block — but the card's own prose already says PR #7377 stacked icontains onto it, so the card is self-consistent. Its claim that contains and icontains execute identically today: VERIFIED on base. Its AST_OPERATOR_MAP citations: VERIFIED (contains -> $contains, icontains -> $icontains, and the 'must never be folded onto ... a semantic boundary, not two spellings of one thing' comment is verbatim, about the icontains/ilike pair).",
        "triage_comment_line_numbers_on_a472b07_rechecked_on_34510e0c3": {
          "125 case 'contains':": "HOLDS",
          "126 case 'icontains': {": "HOLDS",
          "127 / 128 (lv, return)": "HOLD",
          "130 / 131 / 132 case 'not_contains'": "HOLD",
          "135 case 'starts_with'": "SHIFTED — it is line 134 on my base; the paired :136 lv.startsWith HOLDS",
          "139 case 'ends_with'": "SHIFTED — it is line 138 on my base; the paired :140 lv.endsWith HOLDS",
          ":93 canonicalAstOperator (do not touch)": "HOLDS, and untouched"
        },
        "issue_5328_attribution": "Holds, with a repo-numbering note: the card cites objectui#5328 for icontains entering the view vocabulary; the spec-side change is objectstack#8934. Different repos' numbering for the same capability, not a contradiction."
      },
      "sibling_driver_evidence": "Measured, not quoted: (1) grep of every suite importing FILTER_TEXT_CASES returns driver-memory, driver-mongodb, driver-sql, driver-sqlite-wasm, driver-turso, objectql having, formula, service-analytics. (2) objectql having-filter.ts:413 is `case '$contains': if (typeof value !== 'string' || !value.includes(target)) return false;` — exact comparison, no fold — and :455 `$icontains` calls asciiCaseInsensitiveContains. (3) objectstack packages/spec/src/data/filter-text-conformance.ts carries the canonical row: `$contains is case-SENSITIVE — a lower-case comparand misses the upper-case row`, filter { name: { $contains: 'acme' } }, expected ['2'] over ACME Corp / acme corp. (4) EXECUTED against installed @objectstack/spec 17.3.0: asciiCaseInsensitiveContains('ACME Corp','acme') = true, asciiCaseInsensitiveContains('CAFÉ','café') = FALSE while 'CAFÉ'.toLowerCase().includes('café') = true. So it is a defect, not a product ruling.",
      "pin": {
        "file": "packages/core/src/adapters/__tests__/ValueDataSource.textOperatorCase.test.ts (14 cases, all row-SET equalities, no operator-name assertions)",
        "exclusion_half": "['name','contains','acme'] over ACME Corp / acme corp / CAFÉ / café selects ['lower'] — the case-differing row ACME Corp is EXCLUDED.",
        "inclusion_half": "['name','icontains','acme'] over the SAME fixture selects ['upper','lower'] — the row contains just dropped comes back. Mirrored in the $ dialect: { name: { $contains: 'acme' } } -> ['lower'], { name: { $icontains: 'acme' } } -> ['upper','lower'].",
        "would_a_worse_than_the_bug_implementation_pass": "NO, and it is a named case: 'a matcher answering [] for everything FAILS these cases, not just passes the exclusions'. Ablation leg 2 makes both dialects' icontains return nothing and that case goes red first. A SECOND degenerate direction the brief did not name is also closed: an inclusion assertion is equally satisfied by a matcher returning EVERYTHING, which is exactly what $icontains did before this PR (no arm, default: break, no constraint) — so the pin adds `expect(ids).not.toEqual(ALL_IDS)` for that dialect."
      },
      "tests": "All on the final commit c889b18a7, every heavy run through scripts/pm/os-verify-lock.sh, verdict lines read (never a bare $?). UNION: `pnpm exec vitest run packages/core/` + the 67 other test files that reach a value-provider datasource -> VERDICT command-exit 0, 'Test Files 190 passed (190)' / 'Tests 3364 passed (3364)'. That 67-file set is a MEASURED narrowing: the change is confined to two functions in ValueDataSource, so a test can only see it by (a) reaching that adapter and (b) passing one of six operator spellings; the union is condition (a) over the whole repo (76 files, 12 packages). turbo ls --affected names 38 packages only because @object-ui/core is a base dependency — declared to CI's four shards. TYPE-CHECK: `pnpm --filter @object-ui/core type-check` (tsc --noEmit && tsc -p tsconfig.test.json) exit 0; the new file is proven inside the program via `tsc -p tsconfig.test.json --listFiles` (1 hit) with lit control adapters/ValueDataSource.ts (1 hit) and a negative control (0 hits). LINT: package-scoped `eslint .` in packages/core, exit 0, '518 problems (0 errors, 518 warnings)', all pre-existing no-explicit-any; --no-inline-config deliberately NOT used. GATES: check-changeset-presence.mjs verdict 'OK 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/wise-poems-behave.md.'; check-changeset-no-major.mjs verdict 'OK No changeset declares a major bump.'; check-governed-queue-guard.mjs --test with the four paths: 'NOT GOVERNED — 4 path(s) checked against 5 governed surface(s); none matched.' Control-character self-scan over the touched files: zero hits. ABLATION, two legs, both from the COMMITTED implementation, both with trap '<restore>' EXIT INT TERM on absolute paths. Leg 1 (restore the case-fold at the contains READ SITE): HEAD blob 2722d1ba8dff3cc8b5d7defe81b2c556b86c93ca vs mutated on-disk git hash-object 2a25e57da0dd179b64430a25cdf5258c58b892f6 — DIFFERENT, mutation reached disk; anchor count before mutation 1, after 0, injected spelling grepped and printed. Result 5 failed | 9 passed, red BY NAME: 'a lower-case comparand: contains misses the upper-case row, icontains takes both' (message: 'contains is case-SENSITIVE — ACME Corp must NOT come back: expected [upper, lower] to deeply equal [lower]'), 'an upper-case comparand: the mirror', 'the two operators cannot silently re-converge', 'not_contains is case-sensitive, and complements contains exactly', 'both dialects answer the same rows for the same question'. Leg 2 (the strictly-worse implementation — both dialects' icontains answer nothing): blob 2722d1ba... vs mutated 4a0f83a7acf71bf175580839f56492007adaa149. Result 6 failed | 8 passed, headed by 'a matcher answering [] for everything FAILS these cases', plus 'É does not fold to é, in either direction' and the $-dialect pair. BOTH legs restored BY STATE, never by exit code: git diff HEAD empty, git status --short empty, on-disk git hash-object back to 2722d1ba...; pin re-ran 14 passed (14). No rebuild leg is owed — the pin imports '../ValueDataSource' (relative SOURCE), so no dist/ hop exists to stale it.",
      "changeset": ".changeset/wise-poems-behave.md, level `minor` for @object-ui/core. Argument: this MOVES SHIPPED RESULTS (metadata relying on the lenient matching gets fewer rows and no error), so `patch` is wrong; the repo FORBIDS `major` — every package is in one fixed group, so one major carries all of them off the pinned @objectstack major, enforced by check-changeset-no-major.mjs and stated in the presence gate's own text. `skip-changeset` correctly NOT used — it is a phantom label in this repo and this change publishes real behaviour. The changeset body names the migration: a filter meaning 'match regardless of case' is authored as icontains or $icontains, and both now execute.",
      "mcp_calls": "6 — issue_read(get_comments) x1, search_issues x2 (both returned 'API rate limit already exceeded for user ID 323634890'), create_pull_request x1, pull_request_read x1, add_issue_comment x1. Card body and the whole timeline were read through the zero-quota public payload channel; the drivers were read from the local objectstack checkout; the spec was read by EXECUTING the installed package.",
      "contradicts_the_brief": [
        "The brief (and the card) frame this as a binary — contains sensitive or insensitive — but icontains in ValueDataSource was ALSO wrong, on a different axis. It folded via String.prototype.toLowerCase(), the FULL Unicode fold, where the contract folds ASCII ONLY (objectstack#4706 Q1 = A, because three of five backends are SQLite and its lower() folds ASCII only). Measured: asciiCaseInsensitiveContains('CAFÉ','café') = false, toLowerCase() route = true. Fixing only contains would have left the very operator the card names as the correct escape hatch silently wider than every driver, and would have made acceptance criterion 3 ('state the chosen direction at the arm') a place to write a half-truth. Taken as a bounded in-place fix: same defect class, mechanical, the shape already pinned by the spec's own exported helper, no other claim on the file, no new verification surface. FLAGGING IT FOR CONTRACT REVIEW — it is a second silent narrowing (non-ASCII rows stop matching under icontains) and the PM may want it split.",
        "The brief's pin recipe is necessary but NOT sufficient here, because ValueDataSource has TWO matchers. A pin written only in the AST dialect passes green while the $-dialect $contains keeps folding. Both dialects are pinned.",
        "The brief warns that an operator-name assertion passes on a driver that returns everything. The same hole exists on the INCLUSION half and the brief does not name it: an inclusion assertion is equally satisfied by a matcher that returns EVERYTHING — which is precisely what $icontains did before this PR (no arm at all, default: break, no constraint). So 'the inclusion half rules out the [] implementation' is only half the guard; the pin adds an explicit not-everything assertion for that dialect.",
        "The fence says 'if ValueDataSource is the only driver ... report and stop'. Inside objectui it very nearly IS: data-objectstack is a TRANSLATOR (it maps spellings and ships them to the wire, executing nothing), ApiDataSource and runner's mockDataSource implement no text operator at all. The deciding siblings are all in the objectstack repo. A seat that scoped its driver survey to objectui would have found no sibling and escalated a defect as a ruling.",
        "The existing fixture packages/core/src/adapters/__tests__/ValueDataSource.test.ts pinned the BUG: `{ name: { $contains: 'ali' } }` expecting Alice, reachable only under folding — its comment ('ali is not in Charlie') shows the author's model was substring and the fold was invisible. Retriaged to 'Ali' (a spelling fix, per fixture-triage), not deleted. The card's acceptance criteria did not anticipate an existing green test that only passes because of the defect.",
        "The three VIEW_OPERATOR_CASES rows in ValueDataSource.astFilterVocabulary.test.ts (contains/'dmi', not_contains/'dmi', starts_with/'adm', ends_with/'min', icontains/'ADMI') are exactly the same-case fixture the triage seat warned about: they pass identically before and after. They were left alone — the case rule is pinned in its own file rather than smuggled into the vocabulary parity table."
      ],
      "open_questions": [],
      "out_of_scope_findings": [
        "NOT FILED — search_issues answered 'API rate limit already exceeded for user ID 323634890' on both dedupe queries and repo-scoped REST is 403 for this seat, so neither could be searched; handing them to the PM rather than filing unsearched or dropping them. FINDING A (objectui, packages/core/src/adapters/ValueDataSource.ts, matchesFilter): the $-dialect matcher waves through EVERY $ operator it does not recognise — `default: break` adds no constraint. Same defect class as objectui#7349 ('an unrecognised operator matches every row'), in the dialect #7349 did not touch. MEASURED over rows n (score: 5) and s (score: '5'): { score: { $nin: [5,'5'] } } -> ['n','s'] (must be []); { score: { $startsWith: 'zzz' } } -> ['n','s'] (must be []); { score: { $null: true } } -> ['n','s'] (neither row is null); { score: { $exists: false } } -> ['n','s']. The implemented set is $gt $gte $lt $lte $ne $in $contains plus the $icontains this PR adds; everything else is silent no-constraint. Not fixed here: closing it means refusing unknown operators the way matchesASTFilter does, which moves results for every one of those spellings — its own card and its own escalation fence.",
        "NOT FILED (same reason) — FINDING B (objectui, same file, the not_contains arm and the absent $notContains arm): a stored value that is not a string fails the operator AND its negation. MEASURED: ['score','contains','5'] -> ['s'] (correct: a number cannot contain a substring) but ['score','not_contains','5'] -> [] — row n is dropped from 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_CASES pins it on every face, and ValueDataSource has exactly the shape objectql's reference matcher had before that card. Deliberately not ridden in: different axis (stored-value type, not case folding) with its own upstream ruling and its own silent-result blast radius."
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatchedpriority:p2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions