Skip to content

Commit f6caa12

Browse files
fix(scripts): the where-matcher lift reads identifiers from code, never from template-literal text (#30472)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgMPwCdpvbf2imf5vac9Zoo4)_ domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF Fixes #29860 Clause-②: no — an internal instrument; no product surface moves Metadata-protocol: none ## 维护者速读 - **改了什么:** where-matcher 一致性检查在"抽出测试替身的过滤函数单独运行"时,改为只顺着代码里出现的名字去找同文件的依赖,字符串、模板文字的正文和注释里的单词不再算数;`${…}` 里的表达式仍算代码照收。 - **为什么改:** 替身的报错文案里写了 "sparse driver",检查就把旁边的 `driver` 对象一起拉进来,运行时报 `name is not defined`,一个本来合规的替身被判"无法判定",挡住了 PR #29858,作者只好改文案绕过去。门禁结论不该取决于报错文案用了哪个英文单词。 - **风险与回滚:** 只动一个仓库内部检查脚本,不发布任何包。全树 684 个过滤函数改前改后判定逐条一致,耗时持平(约 15 秒)。回滚即撤 PR。 - **先看:** `scripts/check-where-matcher-conformance.mjs` 的 `identifiersIn`(改为遍历语法树)与 `buildCallable` 里读 `sourceIdentifiers` 的三行;self-test 新电池 `#29860`。 - **两分钟验证:** `pnpm check:where-matcher`,预期 self-test 打印 OK,全树一行 `0 silently-wrong and 0 unjudged`(合 main 后共 685 个 matcher)。 - **需要你做的:** 无,席位落地。 ## What changes `identifiersIn` was a word regex over source TEXT (`/\b([A-Za-z_$][A-Za-z0-9_$]*)\b/g`), so every word in a string literal, a template literal's text, a regular expression or a comment counted as a name the lift should follow. It now walks the parsed tree and collects `Identifier` nodes only: a `${…}` hole is code and is collected, the text around it is not. Discovery computes the sets off the nodes it already holds (the candidate's `sourceIdentifiers`, and each visible declaration's `identifiers` beside its `text`), so nothing is re-parsed and no fragment goes through a second parse door. `buildCallable` follows those sets instead of re-scanning text. The header's "Extraction" paragraph says what "names" now means. Over-inclusion stays in one direction only, as before: a property name (`row.name`) is still an identifier node, and a name that resolves to no same-file declaration is skipped by the lift. **Premise note.** The card says ordinary string literals already behaved this way. Measured on `main`, they did not: the same regex read them too. This PR covers both spellings, and the self-test pins both. ## Tests New self-test battery `#29860: the lift reads identifiers from code, never from text` (floor 7; roster floor 4 → 5): - `FIXTURE_TEXT_NAMES_SIBLING`, the card's shape: a refusing matcher inside a factory whose messages say "sparse driver" (one template literal with a `${k}` hole, one plain string), beside `const driver = { name, … }` that closes over the factory parameter. Pinned: discovered; `driver` and `implemented` not collected; lifts and judges CONFORMING (refusing). - `FIXTURE_HOLE_NAMES_HELPER`, the other direction: a same-file helper `label` named only inside a `${…}` hole on the matcher's answering path. Pinned: `label` collected; the matcher lifts with it and judges CONFORMING. <details><summary>Mutation replay (one-off, restored by blob hash == HEAD)</summary> - Old semantics (word regex over the node's text): self-test exit 1 — `template-literal TEXT names no identifier: "driver" in the message is not collected`, `a matcher beside text naming a sibling binding lifts and is CONFORMING, not UNJUDGED`. - Holes skipped (`if (ts.isTemplateExpression(n)) return;`): self-test exit 1 — `the helper named only inside the hole is collected`, `so the hole-reading matcher lifts with its helper and is CONFORMING`. </details> <details><summary>Real producer replay</summary> `packages/objectql/src/write-answer-declared-columns.test.ts` read in memory with its messages put back to `sparse driver:` (the wording PR #29858 had to rename away), then `discoverInSource` + `judge`: - `main` (989ae42): `46 matches UNJUDGED could not lift: ReferenceError: name is not defined` - this branch: `46 matches CONFORMING` </details> ## Acceptance notes - Whole-tree verdicts, before (989ae42) and after: 692 structural candidates, 257 CONFORMING, 427 CONFORMING by refusing, 8 OUT_OF_SCOPE, both runs; a per-candidate diff (file, line, name, verdict, reason) is empty. No verdict changed, so no matcher newly lifts. Gate line both runs: `684 matcher(s) discovered … 0 silently-wrong and 0 unjudged`. Wall time about 15.6 s before, 14.9 s after. After merging `origin/main` (head da5a909): `685 matcher(s) discovered … 0 silently-wrong and 0 unjudged`. - `scripts/check-objectql-double-limit.mjs` carries a near-identical lifter (`identifiersIn` at line 400 is the same word regex). Same defect class, no real producer measured there; not touched (outside this card's file surface), reported to the seat. ## Verification Gate list derived by `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (31 commands), each run with its exit code recorded before any pipe and reconciled with `--ran`; `pnpm lint` run. Readings are in the os-dev-report on #29860. No changeset: the diff is one repo-root script that no released package ships (`skip-changeset`). --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CYZ6tmbWZDWzLW4Yzg)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0c7e95a commit f6caa12

1 file changed

Lines changed: 103 additions & 11 deletions

File tree

‎scripts/check-where-matcher-conformance.mjs‎

Lines changed: 103 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,9 @@
216216
// is transpiled with the same-file declarations it references (resolved from
217217
// its enclosing scopes, transitively) and evaluated. Only the matcher and the
218218
// declarations it names are executed -- never the test file, never a suite.
219+
// "Names" means its CODE names them: identifiers are read off the parsed tree,
220+
// so the words of a string, a template literal's text or a comment never pull
221+
// a sibling binding into the lift (#29860).
219222
// Anything that cannot be lifted this way is UNJUDGED and must be declared;
220223
// it is never treated as passing.
221224

@@ -316,7 +319,11 @@ function declaredName(node) {
316319
return null;
317320
}
318321

319-
/** Same-file declarations visible from `node`, innermost scope first. */
322+
/**
323+
* Same-file declarations visible from `node`, innermost scope first. Each entry
324+
* carries the declaration's TEXT (what the lift executes) and the identifiers
325+
* its CODE names (what the lift follows), read off the tree rather than the text.
326+
*/
320327
function visibleDeclarations(node, sf) {
321328
const decls = new Map();
322329
for (let cur = node.parent; cur; cur = cur.parent) {
@@ -328,22 +335,44 @@ function visibleDeclarations(node, sf) {
328335
if (ts.isVariableStatement(s)) {
329336
for (const d of s.declarationList.declarations) {
330337
if (ts.isIdentifier(d.name) && d.initializer && !decls.has(d.name.text)) {
331-
decls.set(d.name.text, `const ${d.name.text} = ${d.initializer.getText(sf)};`);
338+
decls.set(d.name.text, {
339+
text: `const ${d.name.text} = ${d.initializer.getText(sf)};`,
340+
identifiers: identifiersIn(d.initializer),
341+
});
332342
}
333343
}
334344
} else if (ts.isFunctionDeclaration(s) && s.name && !decls.has(s.name.text)) {
335-
decls.set(s.name.text, s.getText(sf));
345+
decls.set(s.name.text, { text: s.getText(sf), identifiers: identifiersIn(s) });
336346
}
337347
}
338348
}
339349
return decls;
340350
}
341351

342-
const identifiersIn = (src) => {
352+
/**
353+
* Every identifier the CODE under `node` names — and nothing its TEXT says
354+
* (#29860). Walked off the parsed tree, so a string literal, a template
355+
* literal's text between its `${…}` holes, a regular expression and a comment
356+
* contribute nothing, while an identifier inside a `${…}` hole is code and is
357+
* collected. Over-inclusive on purpose in one direction only: a property name
358+
* (`row.name`) is still an identifier node, and a name that resolves to no
359+
* declaration is skipped by the lift anyway.
360+
*
361+
* It used to be a word regex over the source text. A refusal message reading
362+
* `sparse driver: …` then pulled the sibling `const driver = { name, … }` into
363+
* the lift, `name` was a factory parameter the lift cannot supply, and a
364+
* CONFORMING matcher read UNJUDGED (`ReferenceError: name is not defined`) —
365+
* the gate's verdict hung on which English words a string contained.
366+
*/
367+
function identifiersIn(node) {
343368
const out = new Set();
344-
for (const m of src.matchAll(/\b([A-Za-z_$][A-Za-z0-9_$]*)\b/g)) out.add(m[1]);
369+
const visit = (n) => {
370+
if (ts.isIdentifier(n)) out.add(n.text);
371+
ts.forEachChild(n, visit);
372+
};
373+
visit(node);
345374
return out;
346-
};
375+
}
347376

348377
/**
349378
* Does `body` read `id` as a filter object? The two spellings are exactly the
@@ -474,6 +503,7 @@ export function discoverInSource(text, label) {
474503
line: line + 1,
475504
name: declaredName(node) ?? '(anonymous)',
476505
source: node.getText(sf),
506+
sourceIdentifiers: identifiersIn(node),
477507
declarations: declarations ?? visibleDeclarations(node, sf),
478508
capturedPath: captured ? w : null,
479509
});
@@ -498,15 +528,15 @@ function buildCallable(candidate, dropped = new Set()) {
498528
// cannot supply anyway (`query?.where ?? {}`).
499529
const skip = captured ? new Set([...dropped, captured[0]]) : dropped;
500530
const included = new Map();
501-
let frontier = identifiersIn(candidate.source);
531+
let frontier = candidate.sourceIdentifiers;
502532
for (let depth = 0; depth < 6; depth++) {
503533
const next = new Set();
504534
for (const id of frontier) {
505535
if (included.has(id) || id === self || skip.has(id)) continue;
506536
const decl = candidate.declarations.get(id);
507537
if (!decl) continue;
508-
included.set(id, decl);
509-
for (const x of identifiersIn(decl)) next.add(x);
538+
included.set(id, decl.text);
539+
for (const x of decl.identifiers) next.add(x);
510540
}
511541
if (next.size === 0) break;
512542
frontier = next;
@@ -649,7 +679,7 @@ export function measure() {
649679
const bucket = measured.get(rel);
650680
if (result.verdict === 'SILENT') bucket.silent++;
651681
else bucket.unjudged++;
652-
bucket.details.push({ ...candidate, declarations: undefined, result });
682+
bucket.details.push({ ...candidate, declarations: undefined, sourceIdentifiers: undefined, result });
653683
}
654684
}
655685
return { measured, discovered, conforming, refusing };
@@ -844,6 +874,39 @@ const FIXTURE_CAPTURED_NEGATED = `
844874
const run = (rows: any[], opts: any) =>
845875
rows.filter((r) => !Object.entries(opts.where).every(([k, v]) => r[k] === v));`;
846876

877+
// --- #29860: the lift reads identifiers from code, never from text ---------
878+
// A conforming, refusing matcher inside a factory, beside a sibling binding the
879+
// lift cannot run (`driver` closes over the factory's `name` parameter). The
880+
// matcher's refusal MESSAGE says "sparse driver", so a lift that reads words
881+
// out of template-literal text pulls `driver` in and throws `name is not
882+
// defined`. The control word is in a plain string literal too, so neither
883+
// spelling of a string may name a binding into the lift.
884+
const FIXTURE_TEXT_NAMES_SIBLING = `
885+
function makeSparseDriver(name: string) {
886+
const matches = (row: any, where: any): boolean => {
887+
for (const [k, v] of Object.entries(where ?? {})) {
888+
if (k.startsWith('$')) throw new Error(\`sparse driver: filter operator \${k} is not implemented\`);
889+
if (v !== null && typeof v === 'object') throw new Error('sparse driver: operator is not implemented');
890+
if (row[k] !== v) return false;
891+
}
892+
return true;
893+
};
894+
const driver = { name, version: '0.0.0' };
895+
return { driver, matches };
896+
}`;
897+
898+
// The other direction: an identifier inside a `\${…}` hole IS code. Here the
899+
// hole calls a same-file helper on the matcher's answering path, so a lift that
900+
// stopped reading holes would leave `label` undefined and the control probe
901+
// would throw — UNJUDGED, not CONFORMING.
902+
const FIXTURE_HOLE_NAMES_HELPER = `
903+
const label = (k: string) => k.toUpperCase();
904+
const matches = (row: any, where: any): boolean => {
905+
if (Array.isArray(where.$or) && !where.$or.some((w: any) => matches(row, w))) return false;
906+
if (Array.isArray(where.$and) && !where.$and.every((w: any) => matches(row, w))) return false;
907+
return Object.entries(where).every(([k, v]) => k.startsWith('$') || \`\${label(k)}\` === k.toUpperCase() && row[k] === v);
908+
};`;
909+
847910
function judgeFixture(src) {
848911
const found = discoverInSource(src, 'fixture.test.ts');
849912
return { found, results: found.map(judge) };
@@ -876,11 +939,12 @@ const SELF_TEST_BATTERIES = Object.freeze({
876939
'#8615: the captured-filter arm': 17,
877940
'the dispatch-gates declaration (#13163\'s landing obligation)': 6,
878941
'the ratchet is ONE-SIDED: a decrease passes and is reported (#26444)': 5,
942+
'#29860: the lift reads identifiers from code, never from text': 7,
879943
});
880944

881945
// DELETING an entry silences that battery's floor exactly as effectively as
882946
// zeroing it, so the roster's own size is pinned too.
883-
const SELF_TEST_BATTERY_FLOOR = 4;
947+
const SELF_TEST_BATTERY_FLOOR = 5;
884948

885949
// The key an assertion is filed under when no battery is open. It is not a
886950
// declared battery, so it reds by the same set difference rather than silently
@@ -991,6 +1055,33 @@ function selfTest() {
9911055
expect('a fallen count PASSES (one-sided, #26444)', reconcile(fakeMeasured, { 'a.test.ts': { silent: 2 } }).length === 0);
9921056
expect('a clean/gone entry PASSES (one-sided, #26444)', reconcile(new Map(), { 'gone.test.ts': { silent: 1 } }).length === 0);
9931057

1058+
// --- #29860: the lift reads identifiers from code, never from text -------
1059+
battery('#29860: the lift reads identifiers from code, never from text');
1060+
const textNames = judgeFixture(FIXTURE_TEXT_NAMES_SIBLING);
1061+
expect('a matcher beside text naming a sibling binding is discovered', textNames.found.length === 1);
1062+
expect(
1063+
'template-literal TEXT names no identifier: "driver" in the message is not collected',
1064+
textNames.found[0]?.sourceIdentifiers?.has('driver') === false,
1065+
);
1066+
expect(
1067+
'nor does a plain string literal: its "implemented" is not collected either',
1068+
textNames.found[0]?.sourceIdentifiers?.has('implemented') === false,
1069+
);
1070+
expect(
1071+
'a matcher beside text naming a sibling binding lifts and is CONFORMING, not UNJUDGED',
1072+
textNames.results[0]?.verdict === 'CONFORMING' && textNames.results[0]?.refused === true,
1073+
);
1074+
const holeNames = judgeFixture(FIXTURE_HOLE_NAMES_HELPER);
1075+
expect('a matcher calling a helper inside a `${…}` hole is discovered', holeNames.found.length === 1);
1076+
expect(
1077+
'the helper named only inside the hole is collected',
1078+
holeNames.found[0]?.sourceIdentifiers?.has('label') === true,
1079+
);
1080+
expect(
1081+
'so the hole-reading matcher lifts with its helper and is CONFORMING',
1082+
holeNames.results[0]?.verdict === 'CONFORMING',
1083+
);
1084+
9941085
// ── The ratchet is ONE-SIDED: a decrease passes and is reported (#26444) ──
9951086
battery('the ratchet is ONE-SIDED: a decrease passes and is reported (#26444)');
9961087
expect('a fallen count is reported once', reconcileDown(fakeMeasured, { 'a.test.ts': { silent: 2 } }).length === 1);
@@ -1110,6 +1201,7 @@ function selfTest() {
11101201
' parameter callbacks capturing their filter — by name and by access path — are\n' +
11111202
' discovered and judged, while an unresolvable root, a computed filter and an\n' +
11121203
' inverted survivor filter are each declined for their own recorded reason; the\n' +
1204+
' lift follows identifiers its code names, never the words of its text; the\n' +
11131205
' ledger reconciles one-sided (a decrease passes and is reported).',
11141206
);
11151207

0 commit comments

Comments
 (0)