Skip to content

Commit 2038c56

Browse files
test(cli): the parity scan reads a few characters before each call site, not the file up to it (#30811)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DCPdvD63feNqsZZh3NrEFsx)_ domain:server#1 · os-elon-musk · session_016WV4cytC5Tt2HZ6K6M63Ek Fixes #30785 Clause-②: no Metadata-protocol: none Claim: 6677387566 (re-post of 6676066855 with amendment 6676114947 folded in) · Branch: `claude/project-thread-96icbf` · Same defect, filed separately and left to triage: #30789 (no closing keyword here). ## 维护者速读 - 改了什么:只改一个测试文件 `packages/cli/test/validate-build-gate-parity.test.ts`。它的调用点扫描以前每碰到一个 `名字(` 都把文件从头切到这里再跑三个正则,现在只看匹配点前面那段空白和再往前 9 个字符;每个命令源文件每轮只遮罩、只扫描一次。所有断言原样保留,另加 3 条反例钉住新窗口。 - 为什么改:「the command sources use no call shape this scan cannot read」用例在 Test Core (4/8) 两次卡在 5 秒超时(main 推送一次、合并队列一次),把与 cli 无关的 PR 踢出队列。根因是扫描平方级(一次扫 `compile.ts` 要读 21.8 MB),而该用例对 143 个导入各扫一遍;遮罩器本身只占 2–3.5 ms。 - 风险与回滚:极小,纯测试代码,不发布任何文件。新扫描在 1914 个源文件、153,667 个调用点上与旧扫描逐点一致。回滚即撤 PR。 - 先看:`packages/cli/test/validate-build-gate-parity.test.ts` 的 `callSitesOf`(窗口起点 `ws - 9` 那一行和缓存),其次 `codeOnly` 的缓存,最后「a comment or a string literal is not a wiring」里新增的 3 条长注释反例。 - 两分钟验证:`pnpm --filter @objectstack/cli exec vitest run --project unit test/validate-build-gate-parity.test.ts`,预期 33/33 通过,整文件测试耗时约 70 ms(之前约 4 s),该用例约 3 ms。 - 需要你做的:无,席位落地。 ## What changes `packages/cli/test/validate-build-gate-parity.test.ts` only. Every assertion the file had is kept as it was. - **`callSitesOf` reads a few characters before each match, not the file up to it.** The three tests that decide whether a `name(` match is a call (the character before the name is an identifier character; the last non-blank character before it is a dot that is not the last of a spread's three; the last word before it is `function` or `async`) only look at the whitespace run right before the match and at most nine characters before that run (the `\b` before `function`, or the `..` before a `.`). `before` is now cut there. The run is walked, not assumed, because a blanked comment or string is part of it and can be any length. The three regexes are unchanged. - **Each source is masked and scanned once per run.** `codeOnly` and `callSitesOf` are memoized by source text (a changed source is a new key). `callSitesOf` now returns a read-only set of strings. - **The "no call shape" case scans once per file**, not once per value import inside its `filter`. - **Three new negative controls** in "a comment or a string literal is not a wiring": a property access, a declaration and a spread, each with a 200-character comment between the `.` / keyword / `...` and the name. They go red if the window is a fixed offset from the match (ablation A3 below). ## Why Both CI logs time out on the same case, "the command sources use no call shape this scan cannot read" (`:1149` on `main`): | Job | Run | That case | Whole file | |---|---|---|---| | 127442019825 | `main` push on `25fea77a` | 5455 ms, `Test timed out in 5000ms` | 7959 ms | | 127669384323 | merge group `pr-30753` | 5123 ms, `Test timed out in 5000ms` | 7115 ms | Every other case in the file passed in both jobs. Each "both commands run X" case took 39–118 ms there, because each one scans both commands from scratch. The cost was the scan, not the masker. On `ea757ff4` (this container, idle, per call): | | `compile.ts` (88,090 chars, 549 `name(` matches) | `validate.ts` (72,485 chars, 451 matches) | |---|---|---| | `codeOnly` masker | 3.48 ms | 1.79 ms | | one call-site scan, slicing from 0 | 19.4–21.3 ms, 21.84 MB of look-behind read | 13.6–16.9 ms, 16.18 MB | | one call-site scan, bounded (this PR) | 1.28 ms, 0.02 MB | 0.59 ms, 0.01 MB | The failing case called `callSitesIn(file)` inside a `filter` over every value import: 76 in `compile.ts` and 67 in `validate.ts`, so 143 full scans. That is about 2.8 s idle here and over 5 s on a loaded Test Core runner. So the premise carried from #30789 holds for the location and the case, but not for the cause it named: the per-character masker is cheap, and the quadratic scan run 143 times is what times out. No timeout was raised, and none is needed (below). ## Tests Before is `origin/main` `ea757ff4`, after is this branch. Same container, 4 CPUs. "Loaded" pins vitest to CPUs 0–1 beside four `node -e 'for(;;){}'` busy loops pinned to the same two CPUs (PIDs recorded and killed by a trap). | | That case | Sum of the 33 cases | Result | |---|---|---|---| | Before, idle (3 runs) | 2902 / 3083 / 2737 ms | 4089 / 4437 / 3965 ms | 33/33 pass | | Before, loaded (3 runs) | 8508 / 8276 / 8531 ms | 12172 / 11658 / 12077 ms | **1 failed, `Test timed out in 5000ms`, 3 of 3 runs** | | After, idle (3 runs) | 3 / 3 / 3 ms | 70 / 70 / 64 ms | 33/33 pass | | After, loaded (3 runs) | 7 / 3 / 11 ms | 170 / 206 / 210 ms | 33/33 pass | | After, that case alone (`-t`, cold cache) | 19 ms idle, 74 ms loaded | | pass | <details><summary>Every case, before and after (ms per run)</summary> | case | before, idle | after, idle | before, loaded | after, loaded | |---|---|---|---|---| | both commands run the shared authoring-rule registry | 46 / 79 / 58 | 17 / 17 / 14 | 141 / 123 / 122 | 47 / 40 / 45 | | all three authoring commands hand the rule table the union-folded stack | 8 / 10 / 11 | 7 / 7 / 6 | 24 / 22 / 26 | 10 / 22 / 26 | | the union-fold guard still rejects a tier that does not reach the fold | 1 / 2 / 3 | 2 / 2 / 1 | 6 / 2 / 9 | 1 / 1 / 1 | | both commands run preflightDeclaredCapabilities | 39 / 60 / 52 | 1 / 1 / 1 | 185 / 118 / 100 | 7 / 1 / 1 | | both commands run lintUnknownStackKeys | 40 / 40 / 43 | 1 / 1 / 1 | 125 / 109 / 118 | 1 / 10 / 9 | | both commands run lintUnknownAuthoringKeys | 39 / 43 / 41 | 1 / 1 / 1 | 120 / 108 / 108 | 1 / 1 / 1 | | both commands run collectAndLintDocs | 36 / 53 / 39 | 1 / 1 / 1 | 104 / 110 / 99 | 1 / 1 / 1 | | both commands run lowerCallables | 38 / 52 / 38 | 1 / 1 / 1 | 116 / 111 / 106 | 4 / 1 / 1 | | both commands run findPermissionSetNameCollisions | 43 / 53 / 43 | 3 / 3 / 3 | 112 / 117 / 113 | 6 / 15 / 21 | | both commands run findNavGroupDiagnostics | 38 / 50 / 38 | 1 / 1 / 1 | 109 / 107 / 109 | 1 / 1 / 1 | | both commands run checkProtocolVersionGap | 36 / 37 / 39 | 1 / 1 / 1 | 109 / 113 / 95 | 1 / 9 / 1 | | both commands run runPerPackageAuthoringRules | 38 / 39 / 39 | 1 / 1 / 1 | 102 / 115 / 120 | 5 / 1 / 1 | | both commands run resolveJsxGateManifest | 36 / 42 / 40 | 1 / 1 / 1 | 114 / 113 / 108 | 1 / 5 / 9 | | both commands run findViewContainerNameRefusals | 36 / 38 / 38 | 1 / 1 / 1 | 105 / 104 / 119 | 1 / 1 / 1 | | both commands run judgePicklistReferences | 38 / 39 / 36 | 1 / 1 / 1 | 145 / 111 / 109 | 1 / 1 / 1 | | both commands run refuseUnbuiltStack | 40 / 39 / 37 | 1 / 1 / 1 | 125 / 109 / 105 | 5 / 1 / 1 | | both commands run collectSeedFiles | 37 / 38 / 37 | 1 / 1 / 1 | 135 / 105 / 98 | 1 / 9 / 1 | | both commands run collectConnectorSpecFiles | 37 / 48 / 36 | 1 / 1 / 1 | 120 / 97 / 99 | 1 / 1 / 5 | | both commands run checkDependencyRanges | 39 / 44 / 38 | 1 / 1 / 1 | 98 / 108 / 111 | 1 / 1 / 1 | | both commands run judgeSeedRecordValues | 36 / 40 / 37 | 1 / 1 / 1 | 140 / 107 / 108 | 1 / 1 / 1 | | both commands run findOverrideDeclarationRefusals | 35 / 41 / 36 | 1 / 1 / 1 | 140 / 98 / 99 | 1 / 1 / 1 | | both commands run findExtensionFieldSuffixRefusals | 36 / 43 / 38 | 1 / 1 / 1 | 114 / 107 / 126 | 5 / 9 / 1 | | both commands run findFlowCycleRefusals | 38 / 44 / 38 | 1 / 1 / 1 | 105 / 104 / 118 | 1 / 1 / 9 | | every call site in compile.ts and validate.ts is classified | 74 / 88 / 74 | 2 / 2 / 2 | 218 / 208 / 301 | 2 / 2 / 2 | | compile.ts hand-wires no gate validate.ts is missing | 38 / 37 / 52 | 1 / 1 / 1 | 102 / 123 / 125 | 11 / 9 / 1 | | the parity scan sees a gate wired into one command only, whatever it is named | 2 / 1 / 1 | 1 / 1 / 1 | 1 / 1 / 1 | 6 / 5 / 1 | | a comment or a string literal is not a wiring | 1 / 0 / 0 | 0 / 0 / 0 | 0 / 0 / 0 | 0 / 0 / 9 | | no ledger entry is stale | 49 / 46 / 47 | 4 / 3 / 4 | 143 / 132 / 130 | 3 / 15 / 3 | | the command sources use no call shape this scan cannot read | 2902 / 3083 / 2737 | 3 / 3 / 3 | 8508 / 8276 / 8531 | 7 / 3 / 11 | | every BUILD_ONLY_GATES entry is still called by the build | 66 / 66 / 69 | 1 / 1 / 1 | 231 / 201 / 183 | 5 / 9 / 14 | | every VALIDATE_ONLY_GATES entry is still validate-only | 90 / 81 / 77 | 2 / 2 / 1 | 218 / 245 / 308 | 2 / 2 / 2 | | all three authoring commands pass a conversion-notice sink to normalizeStackInput | 2 / 2 / 2 | 1 / 2 / 1 | 2 / 2 / 2 | 1 / 10 / 1 | | all three authoring commands render the conversion notice through ONE formatter | 55 / 59 / 51 | 7 / 7 / 7 | 155 / 152 / 171 | 29 / 17 / 26 | | **sum of cases (ms)** | 4089 / 4437 / 3965 | 70 / 70 / 64 | 12172 / 11658 / 12077 | 170 / 206 / 210 | </details> <details><summary>Equivalence: the bounded scan answers exactly what the old one did</summary> A one-off script (not committed) ran both scans over every `.ts`, `.mts` and `.tsx` source under `packages/cli/src`, `packages/cli/test`, `packages/lint/src`, `packages/objectql/src` and `scripts` and compared the accepted call sites match by match (offset and name, which is stricter than the set of names the file uses): 1914 files, 153,667 accepted call sites, 0 files differing. The old scan took 6920 ms over that corpus and the bounded one 419 ms. </details> <details><summary>Ablation: the case still fails on a known-bad input</summary> Each mutation went through `node scripts/ablation-replace.mjs`, which proves the anchor hit and the blob changed, runs the file, and restores with `git checkout HEAD --` (blob equal to HEAD, `git diff HEAD` empty, both checked). No build is needed: the test reads the command sources with `readFileSync`. - **A1, a namespace import in `compile.ts`** (`import * as ablationNs from 'node:os';` above the `path` import; blob `323cddda` to `8a50a982`): "the command sources use no call shape this scan cannot read" failed in 9 ms on "compile.ts uses a namespace import". 1 failed, 32 passed. - **A2, an uncalled, unclassified value import in `compile.ts`** (`import { ablationUnusedValue } from 'node:os';`; blob `323cddda` to `7e8f758b`): the same case failed in 10 ms on "compile.ts imports 1 value(s) it never calls and this file has not classified: ablationUnusedValue". This is the assertion that now reads the hoisted, memoized call-site set. 1 failed, 32 passed. - **A3, a fixed look-behind window in the scan** (`code.slice(Math.max(0, ws - 9), m.index)` replaced by `code.slice(Math.max(0, m.index - 9), m.index)`; blob `b687f79f` to `3bdfff2a`): "a comment or a string literal is not a wiring" failed on the new long-comment property-access control. 1 failed, 32 passed. - After each restore the file ran 33/33 green. </details> ## Acceptance notes - A test-only change: `packages/cli`'s `files` is `dist`, `README.md` and `CHANGELOG.md`, so nothing published moves. No changeset, labelled `skip-changeset`. - The commit message rounds the per-scan read of `compile.ts` to "about 24 MB"; the measured figure is 21.84 MB (table above). - The memoization keys on the full source text, so the negative controls that hand `callSitesOf` fabricated sources get their own entries. No caller mutates the returned set, and the type now says so. ## Verification All on the final head `413f70fd70` (this branch merged with `origin/main` `43ae284339`; the delta against `main` is the one file, +47 / -4), after a whole-tree `turbo run build --filter=!@objectstack/docs` (118/118 tasks, lock VERDICT command-exit 0). - `pnpm --filter @objectstack/cli exec vitest run --project unit` under the verify lock: Test Files 387 passed (387), Tests 5407 passed (5407), VERDICT command-exit 0. The integration tier is left to CI: the diff touches no spawn entry and no integration-tier file. - `pnpm --filter @objectstack/cli typecheck` (`tsc --noEmit` plus `check:test-typecheck`): VERDICT command-exit 0. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 48 families on `413f70fd70`. Each ran sequentially with its exit code captured before any pipe: 48 of 48 exit 0. `pnpm lint`: exit 0. - `dispatch-gates --ran`: "48 derived famil(ies) accounted for — 48 run, 0 NOT-MEASURED", with `pnpm lint` listed as outside the derivation. - Every family CI runs on this PR is read from the PR's check runs, not re-run here. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (No `packages/spec` change.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. (Nothing publishes: test-only, `skip-changeset`.) - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (Not touched.) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_016WV4cytC5Tt2HZ6K6M63Ek)_ <!-- signed: domain:server#1 · os-elon-musk · session_016WV4cytLmJxfAmgPVjUbbW --> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1f92c72 commit 2038c56

1 file changed

Lines changed: 47 additions & 4 deletions

File tree

‎packages/cli/test/validate-build-gate-parity.test.ts‎

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -564,8 +564,21 @@ const NOTICE_PROSE = 'converted at load; conversion';
564564
* Neither command contains a regex literal today, and the failure direction is
565565
* loud rather than silent — a derailed scan blanks call sites, which strands
566566
* ledger entries and reds `no ledger entry is stale` below.
567+
*
568+
* Memoized by source text, so each command is masked once per run however many
569+
* cases read it. The scan is pure, so a source that changed is a new key.
567570
*/
571+
const MASKED = new Map<string, string>();
572+
568573
function codeOnly(src: string): string {
574+
const hit = MASKED.get(src);
575+
if (hit !== undefined) return hit;
576+
const masked = maskSource(src);
577+
MASKED.set(src, masked);
578+
return masked;
579+
}
580+
581+
function maskSource(src: string): string {
569582
const out = src.split('');
570583
const N = src.length;
571584
const blank = (from: number, to: number): void => {
@@ -637,14 +650,33 @@ const NOT_CALLABLE: ReadonlySet<string> = new Set([
637650
* - a gate never CALLED here but handed on as a value (`list.map(gate)`).
638651
* Asserted absent by requiring every value import to be either called or
639652
* declared in a ledger.
653+
*
654+
* ⚠️ Cost, which is linear and must stay so. The three tests below read only
655+
* the text just before a match: the whitespace run in front of it, plus at
656+
* most nine characters before that run (the `\b` before `function`, or the
657+
* `..` before a `.`). `before` is therefore cut there rather than at the start
658+
* of the file, so each match reads a few characters, not the whole file up to
659+
* it, and the answer is the same. Slicing from 0 made one scan of `compile.ts`
660+
* cost about 20 ms (549 matches each re-reading up to 88 KB), while the masker
661+
* costs about 3 ms. The "no call shape" case scanned once per value import, so
662+
* it timed out at vitest's 5 s default on a loaded Test Core runner. Results
663+
* are memoized by source text for the same reason as {@link codeOnly}.
640664
*/
641-
function callSitesOf(src: string): Set<string> {
665+
const CALL_SITES = new Map<string, ReadonlySet<string>>();
666+
667+
function callSitesOf(src: string): ReadonlySet<string> {
668+
const hit = CALL_SITES.get(src);
669+
if (hit !== undefined) return hit;
642670
const code = codeOnly(src);
643671
const found = new Set<string>();
644672
const re = /([A-Za-z_$][\w$]*)\s*\(/g;
645673
let m: RegExpExecArray | null;
646674
while ((m = re.exec(code)) !== null) {
647-
const before = code.slice(0, m.index);
675+
// Start of the whitespace run in front of the match. A blanked comment or
676+
// string is part of that run, so it can be long: walk it, never assume it.
677+
let ws = m.index;
678+
while (ws > 0 && /\s/.test(code[ws - 1])) ws--;
679+
const before = code.slice(Math.max(0, ws - 9), m.index);
648680
// Tail of a longer identifier, or a property access. `...spread(` is a
649681
// call of `spread`, so the dot test must not fire on the third dot.
650682
if (/[\w$]$/.test(before)) continue;
@@ -654,10 +686,11 @@ function callSitesOf(src: string): Set<string> {
654686
if (NOT_CALLABLE.has(m[1])) continue;
655687
found.add(m[1]);
656688
}
689+
CALL_SITES.set(src, found);
657690
return found;
658691
}
659692

660-
const callSitesIn = (file: string): Set<string> => callSitesOf(sourceOf(file));
693+
const callSitesIn = (file: string): ReadonlySet<string> => callSitesOf(sourceOf(file));
661694

662695
/** Is `name` invoked anywhere in this command's source? */
663696
const calls = (file: string, name: string) => callSitesIn(file).has(name);
@@ -1107,6 +1140,15 @@ describe('os validate is the read-only superset of os build (#3782, #4409)', ()
11071140
// under-reports: a spread-applied call is a call, a property access is not.
11081141
expect(callSitesOf('const a = [...lintUnknownStackKeys(s)];\n').has('lintUnknownStackKeys')).toBe(true);
11091142
expect(callSitesOf('const a = registry.findFoo(s);\n').has('findFoo')).toBe(false);
1143+
1144+
// The look-behind is cut at the whitespace run before a match, and a
1145+
// blanked comment is part of that run. A comment longer than any fixed
1146+
// window between the dot (or the keyword) and the name must not turn a
1147+
// property access or a declaration into a call.
1148+
const longComment = `/* ${'x'.repeat(200)} */`;
1149+
expect(callSitesOf(`const a = registry.\n ${longComment}\n findFoo(s);\n`).has('findFoo')).toBe(false);
1150+
expect(callSitesOf(`async function\n ${longComment}\n declared(s) {}\n`).has('declared')).toBe(false);
1151+
expect(callSitesOf(`const a = [...\n ${longComment}\n lintUnknownStackKeys(s)];\n`).has('lintUnknownStackKeys')).toBe(true);
11101152
});
11111153

11121154
it('no ledger entry is stale', () => {
@@ -1170,6 +1212,7 @@ describe('os validate is the read-only superset of os build (#3782, #4409)', ()
11701212
// statement it skipped drops names from the classification demand, which
11711213
// is silent in the wrong direction.
11721214
const values = valueImportsOf(src);
1215+
const called = callSitesOf(src);
11731216
expect(
11741217
importsConsumed.get(src),
11751218
`${file}: the import parse consumed ${importsConsumed.get(src)} of ${importStatementCount(src)} ` +
@@ -1181,7 +1224,7 @@ describe('os validate is the read-only superset of os build (#3782, #4409)', ()
11811224
// (`list.map(gate)`) rather than called, which has no `gate(` call site
11821225
// at all. `formatUnknownAuthoringKey` is exactly that shape today.
11831226
const unaccounted = [...values]
1184-
.filter((name) => !callSitesIn(file).has(name) && !CLASSIFIED.has(name))
1227+
.filter((name) => !called.has(name) && !CLASSIFIED.has(name))
11851228
.sort();
11861229
expect(
11871230
unaccounted,

0 commit comments

Comments
 (0)