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

docs(objectql): correct the ADR-0061 search-expansion docblock to $icontains (both sites)#13987
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13744-engine-icontains-docblock

Conversation

@claude

@claudeclaudeBot 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:

siteclasscorrect spellingstate
engine.ts:8647, :8664ADR-0061 search expansion$icontainsfixed here
engine.ts:11212-11258 (7 mentions, incl. 3 code sites)multi-value containment$containsalready correct, untouched
search-filter.ts (6)expansion + the deliberate companion clauseboth, as documentedalready correct, untouched
having-filter.ts (8)the operator's own HAVING implementation$containsalready correct, untouched
search-companion.ts:237, :286the __search companion clause$containsalready correct, untouched
search-companion.ts:6ADR-0061 search expansion over source columns$icontainsout of scope — filed as #13984
test files (14 files)pins and prose, both classesbothalready 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.ts3 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

…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 47389b35def6fb51dc92a24a2072467ec40a75e2packageMentionDocs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tooling labels Aug 31, 2026
@zhuangjianguoClaude

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 driftDrifted 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?FALSIFIEDsearch-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 assumptionscripts/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.tsis 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 queueAug 31, 2026
Merged via the queue into main with commit b1b7d60Aug 31, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13744-engine-icontains-docblock branch August 31, 2026 22:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
docs(objectql): correct the ADR-0061 search-expansion docblock to `$icontains` (both sites) by claude[bot] · Pull Request #13987 · objectstack-ai/objectstack · GitHub
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

docs(objectql): correct the ADR-0061 search-expansion docblock to $icontains (both sites)#13987
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13744-engine-icontains-docblock

Conversation

@claude

@claudeclaudeBot 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:

siteclasscorrect spellingstate
engine.ts:8647, :8664ADR-0061 search expansion$icontainsfixed here
engine.ts:11212-11258 (7 mentions, incl. 3 code sites)multi-value containment$containsalready correct, untouched
search-filter.ts (6)expansion + the deliberate companion clauseboth, as documentedalready correct, untouched
having-filter.ts (8)the operator's own HAVING implementation$containsalready correct, untouched
search-companion.ts:237, :286the __search companion clause$containsalready correct, untouched
search-companion.ts:6ADR-0061 search expansion over source columns$icontainsout of scope — filed as #13984
test files (14 files)pins and prose, both classesbothalready 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.ts3 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

…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 47389b35def6fb51dc92a24a2072467ec40a75e2packageMentionDocs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tooling labels Aug 31, 2026
@zhuangjianguoClaude

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 driftDrifted 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?FALSIFIEDsearch-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 assumptionscripts/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.tsis 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 queueAug 31, 2026
Merged via the queue into main with commit b1b7d60Aug 31, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13744-engine-icontains-docblock branch August 31, 2026 22:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' docs(objectql): correct the ADR-0061 search-expansion docblock to `$icontains` (both sites) by claude[bot] · Pull Request #13987 · objectstack-ai/objectstack · GitHub
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

docs(objectql): correct the ADR-0061 search-expansion docblock to $icontains (both sites)#13987
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13744-engine-icontains-docblock

Conversation

@claude

@claudeclaudeBot 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:

siteclasscorrect spellingstate
engine.ts:8647, :8664ADR-0061 search expansion$icontainsfixed here
engine.ts:11212-11258 (7 mentions, incl. 3 code sites)multi-value containment$containsalready correct, untouched
search-filter.ts (6)expansion + the deliberate companion clauseboth, as documentedalready correct, untouched
having-filter.ts (8)the operator's own HAVING implementation$containsalready correct, untouched
search-companion.ts:237, :286the __search companion clause$containsalready correct, untouched
search-companion.ts:6ADR-0061 search expansion over source columns$icontainsout of scope — filed as #13984
test files (14 files)pins and prose, both classesbothalready 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.ts3 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

…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 47389b35def6fb51dc92a24a2072467ec40a75e2packageMentionDocs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tooling labels Aug 31, 2026
@zhuangjianguoClaude

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 driftDrifted 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?FALSIFIEDsearch-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 assumptionscripts/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.tsis 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 queueAug 31, 2026
Merged via the queue into main with commit b1b7d60Aug 31, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13744-engine-icontains-docblock branch August 31, 2026 22:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' docs(objectql): correct the ADR-0061 search-expansion docblock to `$icontains` (both sites) by claude[bot] · Pull Request #13987 · objectstack-ai/objectstack · GitHub
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

docs(objectql): correct the ADR-0061 search-expansion docblock to $icontains (both sites)#13987
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13744-engine-icontains-docblock

Conversation

@claude

@claudeclaudeBot 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:

siteclasscorrect spellingstate
engine.ts:8647, :8664ADR-0061 search expansion$icontainsfixed here
engine.ts:11212-11258 (7 mentions, incl. 3 code sites)multi-value containment$containsalready correct, untouched
search-filter.ts (6)expansion + the deliberate companion clauseboth, as documentedalready correct, untouched
having-filter.ts (8)the operator's own HAVING implementation$containsalready correct, untouched
search-companion.ts:237, :286the __search companion clause$containsalready correct, untouched
search-companion.ts:6ADR-0061 search expansion over source columns$icontainsout of scope — filed as #13984
test files (14 files)pins and prose, both classesbothalready 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.ts3 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

…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 47389b35def6fb51dc92a24a2072467ec40a75e2packageMentionDocs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tooling labels Aug 31, 2026
@zhuangjianguoClaude

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 driftDrifted 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?FALSIFIEDsearch-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 assumptionscripts/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.tsis 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 queueAug 31, 2026
Merged via the queue into main with commit b1b7d60Aug 31, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13744-engine-icontains-docblock branch August 31, 2026 22:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' docs(objectql): correct the ADR-0061 search-expansion docblock to `$icontains` (both sites) by claude[bot] · Pull Request #13987 · objectstack-ai/objectstack · GitHub
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

docs(objectql): correct the ADR-0061 search-expansion docblock to $icontains (both sites)#13987
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13744-engine-icontains-docblock

Conversation

@claude

@claudeclaudeBot 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:

siteclasscorrect spellingstate
engine.ts:8647, :8664ADR-0061 search expansion$icontainsfixed here
engine.ts:11212-11258 (7 mentions, incl. 3 code sites)multi-value containment$containsalready correct, untouched
search-filter.ts (6)expansion + the deliberate companion clauseboth, as documentedalready correct, untouched
having-filter.ts (8)the operator's own HAVING implementation$containsalready correct, untouched
search-companion.ts:237, :286the __search companion clause$containsalready correct, untouched
search-companion.ts:6ADR-0061 search expansion over source columns$icontainsout of scope — filed as #13984
test files (14 files)pins and prose, both classesbothalready 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.ts3 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

…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 47389b35def6fb51dc92a24a2072467ec40a75e2packageMentionDocs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tooling labels Aug 31, 2026
@zhuangjianguoClaude

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 driftDrifted 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?FALSIFIEDsearch-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 assumptionscripts/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.tsis 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 queueAug 31, 2026
Merged via the queue into main with commit b1b7d60Aug 31, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13744-engine-icontains-docblock branch August 31, 2026 22:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' docs(objectql): correct the ADR-0061 search-expansion docblock to `$icontains` (both sites) by claude[bot] · Pull Request #13987 · objectstack-ai/objectstack · GitHub
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

docs(objectql): correct the ADR-0061 search-expansion docblock to $icontains (both sites)#13987
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13744-engine-icontains-docblock

Conversation

@claude

@claudeclaudeBot 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:

siteclasscorrect spellingstate
engine.ts:8647, :8664ADR-0061 search expansion$icontainsfixed here
engine.ts:11212-11258 (7 mentions, incl. 3 code sites)multi-value containment$containsalready correct, untouched
search-filter.ts (6)expansion + the deliberate companion clauseboth, as documentedalready correct, untouched
having-filter.ts (8)the operator's own HAVING implementation$containsalready correct, untouched
search-companion.ts:237, :286the __search companion clause$containsalready correct, untouched
search-companion.ts:6ADR-0061 search expansion over source columns$icontainsout of scope — filed as #13984
test files (14 files)pins and prose, both classesbothalready 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.ts3 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

…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 47389b35def6fb51dc92a24a2072467ec40a75e2packageMentionDocs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tooling labels Aug 31, 2026
@zhuangjianguoClaude

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 driftDrifted 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?FALSIFIEDsearch-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 assumptionscripts/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.tsis 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 queueAug 31, 2026
Merged via the queue into main with commit b1b7d60Aug 31, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13744-engine-icontains-docblock branch August 31, 2026 22:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' docs(objectql): correct the ADR-0061 search-expansion docblock to `$icontains` (both sites) by claude[bot] · Pull Request #13987 · objectstack-ai/objectstack · GitHub
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

docs(objectql): correct the ADR-0061 search-expansion docblock to $icontains (both sites)#13987
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13744-engine-icontains-docblock

Conversation

@claude

@claudeclaudeBot 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:

siteclasscorrect spellingstate
engine.ts:8647, :8664ADR-0061 search expansion$icontainsfixed here
engine.ts:11212-11258 (7 mentions, incl. 3 code sites)multi-value containment$containsalready correct, untouched
search-filter.ts (6)expansion + the deliberate companion clauseboth, as documentedalready correct, untouched
having-filter.ts (8)the operator's own HAVING implementation$containsalready correct, untouched
search-companion.ts:237, :286the __search companion clause$containsalready correct, untouched
search-companion.ts:6ADR-0061 search expansion over source columns$icontainsout of scope — filed as #13984
test files (14 files)pins and prose, both classesbothalready 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.ts3 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

…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 47389b35def6fb51dc92a24a2072467ec40a75e2packageMentionDocs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tooling labels Aug 31, 2026
@zhuangjianguoClaude

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 driftDrifted 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?FALSIFIEDsearch-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 assumptionscripts/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.tsis 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 queueAug 31, 2026
Merged via the queue into main with commit b1b7d60Aug 31, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13744-engine-icontains-docblock branch August 31, 2026 22:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); docs(objectql): correct the ADR-0061 search-expansion docblock to `$icontains` (both sites) by claude[bot] · Pull Request #13987 · objectstack-ai/objectstack · GitHub
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

docs(objectql): correct the ADR-0061 search-expansion docblock to $icontains (both sites)#13987
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13744-engine-icontains-docblock

Conversation

@claude

@claudeclaudeBot 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:

siteclasscorrect spellingstate
engine.ts:8647, :8664ADR-0061 search expansion$icontainsfixed here
engine.ts:11212-11258 (7 mentions, incl. 3 code sites)multi-value containment$containsalready correct, untouched
search-filter.ts (6)expansion + the deliberate companion clauseboth, as documentedalready correct, untouched
having-filter.ts (8)the operator's own HAVING implementation$containsalready correct, untouched
search-companion.ts:237, :286the __search companion clause$containsalready correct, untouched
search-companion.ts:6ADR-0061 search expansion over source columns$icontainsout of scope — filed as #13984
test files (14 files)pins and prose, both classesbothalready 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.ts3 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

…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 47389b35def6fb51dc92a24a2072467ec40a75e2packageMentionDocs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tooling labels Aug 31, 2026
@zhuangjianguoClaude

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 driftDrifted 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?FALSIFIEDsearch-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 assumptionscripts/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.tsis 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 queueAug 31, 2026
Merged via the queue into main with commit b1b7d60Aug 31, 2026
34 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13744-engine-icontains-docblock branch August 31, 2026 22:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

2 participants

@zhuangjianguo@claude