[finding] engine.ts ADR-0061 docblock still describes search expansion as $or of $contains — the implementation and its own neighbor file say $icontains #13744

Activity

1. huangyiirene

   added

   findingRecorded observation — held, not dispatchable until the findings triage round grades itRecorded observation — held, not dispatchable until the findings triage round grades it

   on Aug 31, 2026
2. os-warren

   added

   documentationImprovements or additions to documentationImprovements or additions to documentation

   pm:queueReady for the PM dispatch loopReady for the PM dispatch loop

   domain:engineRetired 2026-10-22, alias of domain:server (engine surface); in-flight cards keep it until they landRetired 2026-10-22, alias of domain:server (engine surface); in-flight cards keep it until they land

   priority:p3

   and removed

   findingRecorded observation — held, not dispatchable until the findings triage round grades itRecorded observation — held, not dispatchable until the findings triage round grades it

   on Aug 31, 2026
3. os-warren commented on Aug 31, 2026

   

   os-warren

   on Aug 31, 2026 – with Claude

   Collaborator

   More actions

   分诊定级 → domain:engine · p3 · documentation · pm:queue。摘 finding。

   锚定。 修复落在 packages/objectql/src/engine.ts ⇒ domain:engine,与立卡席位判断一致(它明说路由归中央分诊,做得对)。

   复核结果:成立,而且假句是两处不是一处

   ⚠️ 行号漂了(卡片写 :8550),按字符串重锚,ADR-0061 那段 docblock 现在在 engine.ts:8645:

   :8645   * ADR-0061: expand `search` into a server-resolved cross-field `$or` of
   :8646   * `$contains`, AND it with any caller `where`, then strip the search keys off
   :8647   * the AST.
           …
   :8664   * allowed set, never widened. All drivers already execute `$or`/`$contains`,
   :8665   * so this needs no driver changes.
   

   ⇒ 同一段 docblock 里说了两次 $contains,卡片只点了第一处。第二处(:8664)同样为假,而且更容易被漏 —— 它读起来像一句关于驱动能力的事实陈述,而不像对展开形状的描述。⇒ 派单范围是两处,⛔ 不是一处。

   对照句(一个文件之隔,已是正确的)仍在:

   packages/objectql/src/search-filter.ts:23
     * [#7641] The case-insensitive operator is `$icontains`, NOT `$contains`.
   

   ⇒ 两个文件现在互相矛盾,而实现侧是对的那一个。

   定级 p3

   纯注释,零行为。不判更低是因为卡片的论证成立且具体:这是已发布技能 sweep 正在花钱清除的那一类幽灵(PR #13740 正在改published skills/objectstack-query 里的同一句),而它坐在一个 agent 做查询引擎工作时第一个读的文件里。⇒ 一个信任该 docblock 的 agent 会把 sweep 刚付钱退役掉的错误重新引入 —— 同一处漂移,往上游一跳。

   ⛔ 但它不该高于 p3:没有任何东西因此坏掉,$icontains 的实现一直是对的。

   派单说明

   • 改两处(:8645-8646 与 :8664),按 search-filter.ts:23 的既有措辞对齐。
   • ⚠️ 锚定字符串,不锚行号 —— 本卡的行号已经漂过一次,本轮我在 [finding] filter.zod.ts:954 still calls $exists a key-presence check — the last false site of #13539's six, and its tracking cards closed without covering it #13709、[finding] security-plugin.ts:6351-6367 still says the #9138/#9139 pair "neither has landed yet" — measured false (#9138 is live), and the paragraph's own residue claim needs re-scoping to the raw .parse() path #13722 上也各遇到一次同样的事。
   • ⛔ 不要顺手改 engine.ts 里其它 $contains。:11212–:11258 那一片讲的是多值包含($contains 在那里是正确的拼写,且明写「No public filter surface is widened」),与 ADR-0061 的搜索展开无关。这是本卡最容易做坏的地方:一次天真的全文件替换会把一段正确的散文改错。
   • 验收:改完之后 packages/objectql/src 内 $contains 与 $icontains 的分布应当与 search-filter.ts 的裁定一致 —— 搜索展开说 $icontains,多值包含说 $contains。

   Refs:#13717(flight-③ 报告,原始观察)· PR #13740(published 文本的更正)· search-filter.ts 里引的 $icontains 裁定链(#7641 / #6520 / #6682)。

   ---

   Generated by Claude Code
4. zhuangjianguo

   added

   pm:dispatchedDispatched to a dev agent by /pm-dispatchDispatched to a dev agent by /pm-dispatch

   and removed

   pm:queueReady for the PM dispatch loopReady for the PM dispatch loop

   on Aug 31, 2026
5. zhuangjianguo

   self-assigned this

   on Aug 31, 2026
6. zhuangjianguo commented on Aug 31, 2026

   

   zhuangjianguo

   on Aug 31, 2026 – with Claude

   Collaborator

   More actions

   Claim — domain:engine lane PM, session session_01F3jdziLbAPGeceVNmSox5L

   Branch: claude/issue-13744-engine-icontains-docblock. Dispatching now.

   Clause-②: no. Both limbs measured, not assumed:

   • Path limb — does not fire. The whole diff is packages/objectql/src/engine.ts; nothing under packages/spec/src/**.
   • Content limb — does not fire. The change is inside a /** … */ block. No accept/reject behaviour moves, no published surface widens — the implementation (search-filter.ts) already emits $icontains and is not touched. ⛔ If the seat finds itself editing anything outside a comment block, that reading is void and it must re-declare.

   ⚠️ Scope is the two false spellings triage measured (:8645–:8646 and :8664), anchored by string, not by line — this card's line numbers have already drifted once (:8550 → :8645).

   ⛔ The named do-not-touch: engine.ts :11212–:11258 is about multi-value containment, where $contains is the correct spelling. A whole-file replace would turn a correct paragraph false — that is the one way to fail this card.

   Region disjointness, measured before dispatch: the only other open PR touching engine.ts is #13864, whose hunks land at new-file 9489..9562 and 10534..10568 — both below this card's ~8645/~8664, so neither shifts the other's anchors. No contention.

   ---

   Generated by Claude Code
7. claude commented on Aug 31, 2026

   

   claudeBot

   on Aug 31, 2026 – with Claude

   Contributor

   More actions

   os-dev-report

   {
     "issue": 13744,
     "status": "done",
     "branch": "claude/issue-13744-engine-icontains-docblock",
     "pr": "https://github.com/objectstack-ai/objectstack/pull/13987",
     "premise_still_valid": true,
     "summary": "Comment-only, zero behaviour. Corrected BOTH false `$contains` spellings in the one ADR-0061 docblock on `ObjectQL.expandSearchOnAst` in `packages/objectql/src/engine.ts`, aligning it with the implementation in `search-filter.ts` (which already emits `$icontains` and carries the #7641 adjudication at its line 23). Diff is 2 insertions / 2 deletions, every line inside one `/** ... */` block, plus a `patch` changeset. File line count unchanged (13805 before and after), so no cited line anywhere in the repo shifted.\n\nANCHORED BEFORE/AFTER (anchored by quoted string; line numbers are the ones ACTUALLY FOUND at merge base eb649cb8bc):\n  Site 1 - engine.ts:8647 (triage said :8645-8646; drifted by 1 line again)\n    BEFORE:    * `$contains`, AND it with any caller `where`, then strip the search keys off\n    AFTER:     * `$icontains`, AND it with any caller `where`, then strip the search keys off\n  Site 2 - engine.ts:8664 (exactly where triage said)\n    BEFORE:    * allowed set, never widened. All drivers already execute `$or`/`$contains`,\n    AFTER:     * allowed set, never widened. All drivers already execute `$or`/`$icontains`,\n  Each anchor string was unique in the file (grep -c == 1) before the edit, and confirmed on disk after (old spelling count 0, new spelling count 1, both grepped, not inferred from an editor exit code).\n\nA2.1 - CONFIRMED, and it drifted AGAIN. Card said :8550, triage said :8645-8646, actual is :8647 for site 1. Site 2 was exactly :8664. The docblock had NOT moved packages or been rewritten; the quoted text matched triage verbatim, so no STOP condition fired. Anchoring was by string throughout.\n\nA2.2 - CONFIRMED. Read the WHOLE ADR-0061 docblock (engine.ts:8645-8670) end to end: it contains exactly two `$contains` occurrences, both false, both fixed. No third inside it. Neither of triage's two was already correct on current main, so no invented work and no discrepancy to report. Note engine.ts contained ZERO `$icontains` mentions before this change, so these two were its only search-expansion references.\n\nA2.3 - FALSIFIED (in the useful direction): a THIRD file carries the same false sentence. Swept every `$contains`/`$icontains` hit in packages/objectql/src/** and classified all of them:\n  SEARCH EXPANSION (must say `$icontains`):\n    - engine.ts:8647, :8664 -- FIXED HERE.\n    - search-companion.ts:6 -- '`$search` (ADR-0061 Tier 1) is a `$contains` over source columns' -- FALSE, same class, DIFFERENT FILE. NOT fixed in this PR (dispatch scoped the PR to engine.ts and asked for a separate finding). Filed unassigned as #13984. Worth noting: the ARGUMENT that sentence carries survives the correction (typing 'zhangwei' cannot hit a stored CJK value under either operator, since folding does not transliterate), which is exactly why it reads as fine and has survived.\n    - search-filter.ts:8, :10, :21, :30 -- already `$icontains`, correct, untouched.\n  MULTI-VALUE CONTAINMENT (correctly `$contains`) -- untouched:\n    - engine.ts:11212, :11217, :11220, :11247, :11250, :11251, :11258 (3 of them code, 4 prose). This is the named do-not-touch region and the diff has zero hunks near it.\n  NORMALIZED `__search` COMPANION CLAUSE (deliberately `$contains`) -- untouched:\n    - search-filter.ts:40, :137, :143(mentions `$icontains` as the contrast), :148 (the emit site)\n    - search-companion.ts:237, :286\n    search-filter.ts:137-143 states why in the imperative: the column is a normalized blob already lowercase on BOTH sides, so a case-SENSITIVE operator over two folded values is exact -- 'Do not \"align\" the two.'\n  OPERATOR IMPLEMENTATION (the HAVING evaluator defining the operators themselves) -- untouched:\n    - having-filter.ts:139, :257, :416, :417, :418, :429, :430, :443 (plus its `$icontains` arm at :444)\n  TESTS/PINS (14 files, both classes, all correct) -- untouched.\n  Result: the post-change distribution matches triage's acceptance criterion exactly, with the single documented exception of search-companion.ts:6, which is now #13984.\n\nA2.4 - CONFIRMED (no test needed), and it was measured rather than assumed. Nothing reads this docblock's PROSE. `scripts/adr-anchors/packages__objectql__src__engine.ts.json` anchors engine.ts to ADR-0057 / ADR-0067 / ADR-0119 -- NOT ADR-0061 -- and `check:adr-anchors` only requires those ADR IDs stay referenced in the file; this change touches no ADR id. `content/docs/permissions/system-context.mdx` cites engine.ts at :3406, :3574, :3584, :3611, :5639, :6337, :9588-9605, :9642, :10712, :10874, :11389, :11460, :13801 -- none in the 8645-8670 docblock, and the change inserts no lines anyway. check-system-context-census.mjs ran GREEN. So the card's shape is unchanged: a comment fix, no test owed. What I ran instead of a new test is the existing positive control that the corrected sentence is TRUE (see `tests`).\n\nCLAUSE-2 DECLARATION (mine, made against the diff as it LANDED, not as it was scoped):\n  Clause-②: no.\n  - Path limb -- does not fire. Diff is packages/objectql/src/engine.ts plus .changeset/engine-adr0061-icontains-docblock.md. Nothing under packages/spec/src/**.\n  - Content limb -- does not fire. Both hunks sit inside one `/** ... */` block; all 4 changed lines are comment lines. No accept/reject behaviour moves, no published surface widens; search-filter.ts already emitted `$icontains` and is not touched, so the runtime is byte-for-byte identical. I agree with the PM's reading and had no cause to overrule it -- the diff did not end up wider than a comment (the one thing that would have voided it, the third site in search-companion.ts, was deliberately kept OUT).\n\nCHANGESET: chose a real `patch` changeset for @objectstack/objectql over the `skip-changeset` label. Criterion applied is the repo review checklist's, which is deliberately the package's PUBLISH STATUS and explicitly not the size of the change or a 'user-visible' feel judgement: packages/objectql/package.json declares no `private` flag, so this lands in the source of a published package. All 8 changeset-triggered gate families green.\n\nHALF-STATE: none. Issue was already assigned (zhuangjianguo) and claimed by the dispatching PM; I did not re-claim or reassign. Nothing on the PR was set by another actor during the run.",
     "tests": "GATE UNION re-run at the FINAL commit 24d04ba988 (`git rev-parse --short HEAD` of the tree this PR proposes), AFTER both commits landed -- not at the earlier 917cbae75c where the first pass ran. Family derived by `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` with NO paths passed, so the script computed its own change set (stderr: \"change set derived from git - 2 path(s) vs merge base eb649cb8b\", \"--repo 'objectstack-ai/objectstack' checked against this checkout's 'origin' remote - it holds\"). Harvested with --commands so neither gate spelling could be dropped. 32 families; every exit code captured BEFORE any pipe (redirect to a log, then $?), never through `| tail`.\n\nRESULT: 30 EXIT=0, 2 EXIT=3.\n  EXIT=0: check-adr-0087-registration, check-changeset-no-major, check-ci-filter-parity, check-comment-mask-adoption, check-cross-package-test-inputs, check-empty-changeset, check-engine-split-ratio, check-keyed-text-bounds, check-plugin-teardown-shape, check-shard-attestation, check-system-context-census, check-undeclared-dep-imports, docs-audit/check-affected-docs, docs-audit/check-drift-comment, pm/check-half-states, pm/release-rehearsal-clone --self-test, check:changeset-gate-self-tests, check:cross-package-test-inputs, check:dispatcher-error-vocabulary, check:doc-authoring, check:durability-log-level, check:logger-receiver-detach, check:objectui-changeset, check:page-declaration-shape, check:pm-half-states, check:published-files, check:slot-lookup, check:stack-collection-maps, check:test-source-alias, check:type-source-resolution.\n  EXIT=3 = PREREQUISITE NOT MET = NOT MEASURED (not a pass, not a red), 2 families, both quoted from their OWN verdict text:\n    - check-test-completeness.mjs: \"Arrived here from the gate family scripts/pm/dispatch-gates.mjs derives? That list names this script with NO argument, which is this branch. There is no local log to hand it, so the local reading for this gate is NOT MEASURED.\" Covered by CI's Test Core, which tees it a real log.\n    - check:dual-build-cjs-loads: \"PREREQUISITE NOT MET - this gate reads built output, and some package has no dist/.\" It listed 54 packages this diff never touches (hono, account, setup, studio, client-react, ...). \"Run `pnpm build` first. This is NOT a pass: nothing was measured.\" A repo-wide `pnpm build` is CI's Build Core job by the local-scope rule; I did not run it. Its own --self-test passed 93 cases first.\n  DECLARED NARROWING: I did not run repo-wide `pnpm lint`. That is CI's `Lint & Repo Gates` run, and it is the only narrowing I took.\n\nKEY GATE VERDICT LINE (quoted from the gate, not from $?): check-system-context-census.mjs -- \"check-system-context-census: OK - 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read.\" The dispatch warned this census can go red if a cited file gains lines; the change is a pure spelling substitution that adds none (engine.ts is 13805 lines before AND after), so nothing needed re-pointing and `--fix` was NOT run.\n\nBUILD FIRST (both under the shared verify lock, `bash scripts/pm/os-verify-lock.sh -c ...`, slot issue-13744-dev set before the first attempt):\n  - pnpm --filter '@objectstack/objectql^...' build -- VERDICT command-exit 0 (held 176s, waited 258s)\n  - pnpm --filter '@objectstack/objectql' build -- VERDICT command-exit 0 (held 13s, waited 316s)\n  Verdicts read from the wrapper's own VERDICT line, not from a bare exit code.\n\nTYPECHECK: pnpm --filter @objectstack/objectql typecheck -- VERDICT command-exit 0 (`tsc --noEmit && tsc --noEmit -p tsconfig.scripts.json`, both clean). Guarded against the ran-but-measured-nothing case: `tsc --noEmit --listFiles` confirms engine.ts IS in the typecheck program (1 hit), so this green really covers the edited file.\n\nTESTS -- the positive control that the CORRECTED sentence is now true. Comment-only changes cannot move a test, so the useful measurement is not 'did my change break anything' but 'does the behaviour match what the docblock now claims':\n  pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/search-filter.test.ts src/global-search-palette-recall.test.ts src/search-companion.test.ts\n  (no bare `--` anywhere in that command -- vitest silently discards everything after one)\n  => Test Files 3 passed (3) | Tests 57 passed (57), VERDICT command-exit 0.\n  Re-run with --reporter=verbose to name them (2 files, 28 passed):\n    OK expandSearchToFilter > single term -> $or of $icontains across resolved fields\n    OK [#7641] textual fields compile to the case-folding operator > emits $icontains -- never the case-SENSITIVE $contains -- for text fields\n    OK [#7641] source columns still compile to $icontains, never the case-SENSITIVE $contains\n    OK [#7641] ... > leaves the `__search` companion clause on $contains (both sides already lowercase)\n  The first three prove sites 1 and 2 now say the true thing; the fourth is why the companion mentions in search-filter.ts and search-companion.ts were deliberately left on `$contains`.\n\nNO ABLATION was run, and the template's ablation field is not applicable here: there is no guard to remove and no mutation to confirm on disk -- the deliverable is a comment correction whose truth is established by the existing #7641 pins above, which are the positive control. Saying so beats fabricating a rebuild/mutation narrative for a run that had none.\n\nBYTES: grep -naP over the edited file for control characters returned exit 1 (no hits). check:nul-bytes is subsumed by the derived family run above. PR body and the finding body were both read BACK after writing and compared byte-for-byte against what was sent: both PREFIX IDENTICAL, the only difference being the platform's own appended footer -- which is the platform's, not my form downgraded, and was left alone.",
     "mcp_calls": "0 - every GitHub read AND write went through repo-scoped REST (probe: GET /repos/objectstack-ai/objectstack = HTTP 200). Zero MCP GitHub tool calls for the whole run. Channel note for the PM: the GLOBAL /search/issues path is 403 on this seat ('sessions are bound to their configured repositories'), so the dedup search was done as bounded repo-scoped label listings (labels=finding 5 open, labels=documentation 49 open, labels=domain:engine 43 open) plus local grep, with a control sample printed to prove the grep matches at all. No wide-table scan, no MCP fallback needed.",
     "open_questions": [],
     "out_of_scope_findings": [
       "filed as #13984: search-companion.ts:6 calls the ADR-0061 `$search` expansion 'a `$contains` over source columns' - same false-spelling class as this card, third file, deliberately NOT folded into this PR per the dispatch; the card also records the two mentions in that same file where `$contains` is CORRECT (:237, :286) so nobody batch-replaces it, and notes the adr-anchors entry binding that file to ADR-0015/0045/0061.",
       "recorded inside #13984 rather than as its own card (same file, same docblock family, one sentence apart): search-companion.ts:286 names the companion's reader as `buildSearchFilter`, but the exported function in search-filter.ts is `expandSearchToFilter` - `buildSearchFilter` is not an export of that module. Stale identifier, comment-only, no behaviour."
     ]
   }

   ---

   Generated by Claude Code
8. zhuangjianguo

   closed this as completedin #13987on Aug 31, 2026
9. zhuangjianguo

   removed

   pm:dispatchedDispatched to a dev agent by /pm-dispatchDispatched to a dev agent by /pm-dispatch

   on Aug 31, 2026