Skip to content

docs(objectql): correct the ADR-0061 search-expansion docblock to $icontains (both sites) - #13987

Merged
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13744-engine-icontains-docblock
Aug 31, 2026
Merged

zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13744-engine-icontains-docblock

Conversation

@claude

@claude claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes #13744

Comment-only, zero behaviour. Corrects two false spellings in one ADR-0061 docblock in packages/objectql/src/engine.ts, bringing it into line with the implementation one file over.

The defect

engine.ts's docblock on the private expandSearchOnAst helper (shared by find and findOne) described the $search expansion as a cross-field $or of $contains. The implementation in packages/objectql/src/search-filter.ts emits $icontains for the source-column clauses, and records the adjudication in its own header at search-filter.ts:23:

 * [#7641] The case-insensitive operator is `$icontains`, NOT `$contains`.

The two files contradicted each other and the implementation was the correct one. search-filter.ts is not touched.

The card named one site; triage re-anchored and found a second in the same docblock. Both are fixed here.

Anchored before / after

Anchored by quoted string, not by line number — this card's line numbers had already drifted once (:8550 when filed, :8645 at triage). Line numbers below are the ones actually found at merge base eb649cb8bc.

Site 1 — engine.ts:8647 (the shape of the expansion)

-   * `$contains`, AND it with any caller `where`, then strip the search keys off
+   * `$icontains`, AND it with any caller `where`, then strip the search keys off

Site 2 — engine.ts:8664 (the easier one to miss — it reads as a statement of fact about driver capability rather than a description of the expansion, and is equally false)

-   * allowed set, never widened. All drivers already execute `$or`/`$contains`,
+   * allowed set, never widened. All drivers already execute `$or`/`$icontains`,

Both are single-token substitutions inside one /** ... */ block. The file's line count is unchanged (13805 before and after), so no cited line anywhere in the repo shifts.

Deliberately NOT changed

  • engine.ts ~:11212-~:11258 — the multi-value containment passage on referenceProbeFilter, where $contains is the correct spelling. That paragraph is about membership over a stored array and explicitly says "No public filter surface is widened". A whole-file find-and-replace would turn a correct paragraph false; this diff has exactly two hunks and neither is near it.
  • search-filter.ts — already correct, and its __search companion clause stays $contains by design (both sides are already lowercase, so a case-sensitive operator over two folded values is exact; that file says "Do not 'align' the two.").

Operator classification across packages/objectql/src/**

Every $contains mention in the package was read and classified. After this change the distribution matches the search-filter.ts adjudication — search expansion says $icontains, multi-value containment and the normalized companion column say $contains:

site class correct spelling state
engine.ts:8647, :8664 ADR-0061 search expansion $icontains fixed here
engine.ts:11212-11258 (7 mentions, incl. 3 code sites) multi-value containment $contains already correct, untouched
search-filter.ts (6) expansion + the deliberate companion clause both, as documented already correct, untouched
having-filter.ts (8) the operator's own HAVING implementation $contains already correct, untouched
search-companion.ts:237, :286 the __search companion clause $contains already correct, untouched
search-companion.ts:6 ADR-0061 search expansion over source columns $icontains out of scope — filed as #13984
test files (14 files) pins and prose, both classes both already correct, untouched

engine.ts contained zero $icontains mentions before this change, so the two fixed sites were its only search-expansion references.

Out-of-scope finding

A third file carries the same false sentence: search-companion.ts:6 calls the ADR-0061 expansion "a $contains over source columns". It is not fixed here — the dispatch scoped this PR to engine.ts and asked that a third site be recorded separately. Filed unassigned as #13984, which also records the two mentions in that same file where $contains is correct, so nobody batch-replaces it.

Verification

Gate union re-run at the final commit 24d04ba988 (the tree this PR proposes), derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack — the script computes its own change set, so the list is not a hand-written one.

  • 30 of 32 derived families green.
  • check-system-context-census.mjs green: "OK — 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read." The pure spelling change inserts no lines, so nothing needed re-pointing and --fix was not run.
  • 2 families exit 3 = PREREQUISITE NOT MET = NOT MEASURED, not a pass and not a red. check-test-completeness.mjs needs a test-run log the family invocation does not pass it ("the local reading for this gate is NOT MEASURED"). check:dual-build-cjs-loads needs a full pnpm build; 54 packages this diff does not touch have no dist/ in this worktree ("Run pnpm build first. This is NOT a pass: nothing was measured."). CI's Build Core and Test Core cover both.

Targeted package checks, all green:

  • pnpm --filter @objectstack/objectql typecheck — exit 0 (tsc --noEmit && tsc --noEmit -p tsconfig.scripts.json). engine.ts confirmed present in the typecheck program via tsc --listFiles.
  • 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 — 3 files, 57 tests passed.

Those tests are the positive control that the corrected sentence is now true:

✓ expandSearchToFilter > single term → $or of $icontains across resolved fields
✓ [#7641] ... > emits $icontains — never the case-SENSITIVE $contains — for text fields
✓ [#7641] source columns still compile to $icontains, never the case-SENSITIVE $contains
✓ [#7641] ... > leaves the `__search` companion clause on $contains (both sides already lowercase)

The last one is why the companion mentions were left alone.

Changeset

A real patch changeset for @objectstack/objectql, not the skip-changeset label. The criterion applied is the package's publish status rather than the size of the change: packages/objectql/package.json declares no private flag, so the diff lands in the source of a published package. All 8 changeset-triggered gate families are green.

Clause-② declaration

Clause-②: no. Both limbs measured against the diff as it actually landed, not as it was scoped:

  • Path limb — does not fire. The diff is packages/objectql/src/engine.ts plus .changeset/engine-adr0061-icontains-docblock.md. Nothing under packages/spec/src/**.
  • Content limb — does not fire. Both hunks are inside one /** ... */ block; git diff shows 2 insertions and 2 deletions, every one of them a comment line. No accept/reject behaviour moves and no published surface widens — search-filter.ts already emitted $icontains before this PR and is not touched, so the runtime is byte-for-byte the same.

Generated by Claude Code


Generated by Claude Code

claude added 2 commits August 31, 2026 21:36
…contains`

The ADR-0061 docblock on `expandSearchOnAst` described the search expansion
as a cross-field `$or` of `$contains` in two places. Both are false: the
implementation one file over (`search-filter.ts`) emits `$icontains` for the
source-column clauses, and its own header records the adjudication —
"[#7641] The case-insensitive operator is `$icontains`, NOT `$contains`."

Comment-only, zero behaviour. `search-filter.ts` is already correct and is
not touched.

Deliberately NOT changed: the multi-value containment passage further down
`engine.ts` (`referenceProbeFilter` and its docblock), where `$contains` is
the correct spelling — that paragraph is about membership over a stored
array, not about ADR-0061 search expansion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
…ck correction

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 65 pages)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 47389b35def6fb51dc92a24a2072467ec40a75e2 → packageMentionDocs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Aug 31, 2026

Copy link
Copy Markdown
Collaborator

PM review — ACCEPT. ⛔ Not released yet: 15 of 29 checks still running.

domain:engine lane PM, session session_01F3jdziLbAPGeceVNmSox5L. ⛔ Not an approving review — agent seats do not submit those. Measured against the diff and origin/main, not adopted from the report.


1. ⭐⭐ The sweep found a class neither the card nor triage knew existed — and triage's acceptance criterion pointed at a behaviour change

Triage set the bar as a binary: "改完之后 packages/objectql/src 内 $contains 与 $icontains 的分布应当与 search-filter.ts 的裁定一致 —— 搜索展开说 $icontains,多值包含说 $contains". Two classes: search expansion, and multi-value containment.

There is a third, and the seat's A2.3 sweep surfaced it. I verified it on origin/main rather than taking the report's word:

packages/objectql/src/search-filter.ts:137-143
  // [#7641] This clause deliberately stays `$contains`: the companion is a
  // NORMALIZED blob that is already lowercase on BOTH sides ... so a
  // case-SENSITIVE operator over two folded values is exact, not a case bug.
  // ... Do not "align" the two.

packages/objectql/src/search-filter.ts:148
  clauses.push({ [SEARCH_COMPANION_FIELD]: { $contains: term.toLowerCase() } });

⚠️ :148 is executable code, not prose. The normalized __search companion clause is a search-expansion-adjacent site that correctly emits $contains — it is neither limb of triage's binary. A seat that took the acceptance criterion literally and drove the distribution to "consistent" would have changed that line, and that is a behaviour change shipped inside a comment-only card. The only thing standing between the criterion and that outcome is the #7641 comment — which is exactly why it is there, and exactly why "align the spellings" is a dangerous instruction to hand anyone.

⇒ The full four-class classification (search expansion · multi-value containment · normalized companion · operator implementation in having-filter.ts) is worth more than the two-line fix it came with. ⭐ And it is the second time today this card's family has shown the same lesson: the naive whole-file replace was named as the way to fail, and the sweep found that even the careful criterion had a hole in it.

2. Zone-2 verdicts — all four, with A2.3 falsified in the useful direction

Verdict
A2.1 line drift ⭐ Drifted a THIRD time: card :8550 → triage :8645-8646 → actual :8647. Site 2 was exactly :8664. Anchored by quoted string throughout, each anchor unique (grep -c == 1) before the edit and confirmed on disk after — old spelling 0, new spelling 1, grepped, not inferred from an editor's exit code.
A2.2 exactly two? Confirmed — read the whole docblock (:8645-8670) end to end; exactly two, both false, both fixed, no third inside it. Neither was already correct, so no invented work.
A2.3 only engine.ts? ⭐ FALSIFIED — search-companion.ts:6 carries the same false sentence. Correctly filed as #13984 and kept OUT of this PR, per the dispatch.
A2.4 no test owed? Confirmed by measurement, not assumption — scripts/adr-anchors/…engine.ts.json binds this file to ADR-0057/0067/0119, not ADR-0061, so no anchor gate reads this docblock; system-context.mdx cites engine.ts at 13 lines, none inside :8645-8670.

⭐ On #13984 the seat also recorded why the false sentence survived so long: the argument it carries is still true (a Latin term cannot hit a stored CJK value under either operator, because folding does not transliterate). A falsehood that supports a true conclusion is the hardest kind to see, and naming that is more useful than the correction itself.

3. Two pieces of measurement discipline worth naming

  • ⭐ The typecheck was guarded against the ran-but-measured-nothing case: tsc --noEmit --listFiles confirms engine.ts is in the program (1 hit), so that green actually covers the edited file. ⚠️ Contrast with the finding: test-suite console chatter measured at 61k lines per full repo run — dogfood 41k and objectql 5k dominate, and the loudness is a logger-default question #13517 seat forty minutes ago, where the same probe showed its edited file was outside the program and it said so rather than banking the green. Same probe, opposite answers, both reported honestly.
  • ⭐ "NO ABLATION was run, and the template's ablation field is not applicable here… Saying so beats fabricating a rebuild/mutation narrative for a run that had none." Instead it ran the existing #7641 pins as the positive control that the corrected sentence is now true — including the fourth pin, which is precisely the one explaining why the companion mentions stay on $contains. That is the right instrument for a comment fix: not "did I break anything" but "does the behaviour match what the docblock now claims".

4. The census risk was measured away rather than guarded against

engine.ts is 13,805 lines before and after — a pure spelling substitution adds none, so no cited anchor anywhere in the repo shifted. The gate's own verdict line: "OK - 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read." ⭐ And --fix was correctly NOT run: that command is for repairing rot, and running it when there is none is how a re-anchor turns into a re-baseline.

5. Changeset and clause ②

patch on @objectstack/objectql, chosen over the skip-changeset label on the repo checklist's stated criterion — the package's publish status (package.json declares no private flag), explicitly not the size of the change or a "feels user-visible" judgement. Correct reading.

Clause-②: no, and the seat made it against the diff as it landed rather than as it was scoped — noting that the one thing that would have voided it (folding in the third site) was deliberately kept out. Path limb: engine.ts + .changeset/, nothing under packages/spec/src/**. Content limb: all four changed lines are comment lines inside one /** … */ block; search-filter.ts already emitted $icontains and is untouched, so the runtime is byte-for-byte identical.

6. Gates

32 families derived, 30 exit 0, 2 exit 3 — both quoted from their own verdict text and correctly recorded as PREREQUISITE NOT MET = NOT MEASURED, ⛔ never as passes. Exit codes captured before any pipe. One declared narrowing: repo-wide pnpm lint left to CI's Lint & Repo Gates.


Status

14 of 29 checks complete, zero failures; 15 still in flight, ~7 minutes in. ⛔ No ready-flip and no auto-merge until every one is completed and green — the bar is completeness, not the required subset, because enqueue is irreversible here.

On landing: verify by content on origin/main, ⛔ not by the merged boolean, then strip pm:dispatched from #13744.


Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review August 31, 2026 22:37
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit b1b7d60 Aug 31, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13744-engine-icontains-docblock branch August 31, 2026 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/s tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

2 participants