Skip to content

Stop the grader scanner reading English as code - #1727

Merged
Alex Weininger (alexweininger) merged 1 commit into
feat/CoRfrom
alexweininger-import-scanner
Aug 26, 2026
Merged

Stop the grader scanner reading English as code#1727
Alex Weininger (alexweininger) merged 1 commit into
feat/CoRfrom
alexweininger-import-scanner

Conversation

@alexweininger

Copy link
Copy Markdown
Member

feat/CoR is red for everyone right now, and this is why.

stage-graders.ts found imports with a regex over raw source:

const SPECIFIER = /(?:\bfrom|\bimport)\s*\(?\s*['"]([^'"]+)['"]/g;

#1721 (551e0e81) added a doc comment to graderHarness.ts:48:

the marker is what turns a red from "mysterious failure" into "known gap, here is the fix".

The scanner read from "mysterious failure" as an import of a package by that name, could not find it in node_modules, and threw. Scanning that file yields exactly four "specifiers": node:fs, node:path, ../src/artifacts/validationTypes.ts, and mysterious failure. A prose sentence was failing the build.

Reproduced on a pristine origin/feat/CoR worktree with none of my code present:

Error: evals/graders/graderHarness.ts imports 'mysterious failure' from node_modules.
    at collect (evals/msbench/stage-graders.ts:80:19)
STAGE-GRADERS ON PRISTINE feat/CoR EXIT=1

The judgement was right and the detector was naive. Refusing to stage a tree that would die four minutes into a paid run is exactly what this guard is for. So the refusal is kept and only the detection is replaced.

Why not ts.preProcessFile

It was the first choice — the compiler's own scanner, handling every case for free. It cannot be used here, and the reason is worth recording so nobody spends an afternoon rediscovering it.

run.sh stages graders on every invocation, including --skip-build, deliberately: they are read straight off the working tree, and staging a stale copy would grade the wrong contract. But --skip-build skips the root npm install, and the MSBench eval job runs exactly that path — checkout, setup-node, download the VSIX artifact, run.sh --skip-build. There is no node_modules on that host at all, at the root or in evals/.

Worse, typescript is not a declared dependency of the root manifest:

lockfile paths providing typescript:
   node_modules/typescript -> 5.9.3 (dev)
declared by: @microsoft/api-extractor, @microsoft/vscode-azext-eng, @typescript-eslint/*

It is hoisted transitively from somebody else's dev dependency. Depending on that would make the staging guard hostage to npm's hoisting.

stage-graders.ts imports node: builtins and nothing else, and run.sh promises "a clean machine with az login already done should be able to execute this unmodified". That property is load-bearing, and a third-party import would quietly delete it — on the one path where failure costs money.

So: a small tokenizer, no dependencies. This closes the category error rather than narrowing it — the comment-stripper I had tested would still have read a string literal containing from "x" as an import; this does not.

The test set, run in CI

npm run imports:self-test, 13 cases, wired into the contracts job. The three non-negotiable ones first:

  ✔ block comment containing from "not-a-module" is ignored
  ✔ string literal containing from "not-a-module" is ignored
  ✔ a genuine bare import is still found — the guard must be able to fail
  ✔ line comment containing from "not-a-module" is ignored
  ✔ template literal containing from "not-a-module" is ignored
  ✔ template hole containing quotes does not desynchronise the scan
  ✔ regex literal containing quotes does not swallow later imports
  ✔ dynamic import is found
  ✔ re-export is found
  ✔ type-only import is found, as before
  ✔ multi-line import statement is found
  ✔ templated dynamic import is not reported as a static specifier
  ✔ division is not mistaken for a regex literal
PASS: 13/13 scanner cases.

The regex-literal case is not hypothetical: the deleted regex itself contains ['"], so a scanner that mistook it for a string would swallow the rest of any file containing one.

Proof the guard can still fail — the property everything else is in service of. Adding a real bare import to a grader:

Error: evals/graders/validate-requirements.ts imports 'lodash' from node_modules.
EXIT=1

And the staged output is unchanged: the same 10 files as before the sentence was written.

The sentence stays, as a regression fixture

graderHarness.ts:48 keeps its wording, now labelled. Reworded, it would disguise a parser defect as a typo and leave the trap armed for whatever sentence someone writes next. Left in place with the tokenizer behind it, it is a deliberately adversarial line: swap the scanner back for a regex and the build breaks there immediately.

One line of self-criticism

An hour before finding this, my own coverage scanner in #1726 was counting 'containerApps' and 'stackDeclaration' as error codes because it matched a prefix rather than a call shape. Same defect, same afternoon. Regex over source that doesn't know what a comment is will eventually read English as code.

Verification

All five credential-free gates pass locally — typecheck, imports:self-test, drift, certify (81/81), lint — plus stage-graders.ts end to end. Base feat/CoR at c8ea2cfe, 5 files, +436/−5.

Unblocks #1725, #1720, every future run.sh submission, and #1726. No MSBench run was submitted.

`feat/CoR` is red for everyone. `stage-graders.ts` found imports with a regex
over raw source:

    /(?:\bfrom|\bimport)\s*\(?\s*['"]([^'"]+)['"]/g

#1721 added a doc comment to graderHarness.ts reading `turns a red from
"mysterious failure" into "known gap"`. The scanner read `from "mysterious
failure"` as an import of a package by that name, could not find it in
node_modules, and failed the build. A prose sentence broke the branch.

The judgement was right and the detector was naive: refusing to stage a tree
that would die inside the container is exactly what this guard is for. So the
refusal is kept and the detection is replaced.

The TypeScript compiler ships this scanner and was the first choice. It cannot
be used here. `run.sh` stages graders on every invocation including
--skip-build, and the MSBench `eval` job runs exactly that path: checkout,
setup-node, download artifact, run.sh --skip-build. There is no node_modules on
that host at all, and `typescript` is not even a declared dependency of the root
manifest — it is hoisted transitively from api-extractor and typescript-eslint.
stage-graders.ts imports node: builtins and nothing else, and run.sh promises a
clean machine can run it unmodified. Importing a third-party module would delete
that property on the one path where failure costs money.

So: a small tokenizer, no dependencies, that knows what comments, string
literals, template literals and regex literals are. That closes the category
error rather than narrowing it — a string literal containing `from "x"` is
ignored too, which comment-stripping would not have caught.

The sentence in graderHarness.ts stays, now labelled as a regression fixture:
if anyone swaps the tokenizer back for a regex, the build breaks there
immediately rather than on whatever sentence somebody writes next.

13 scanner cases run in CI, including the three that matter: a comment and a
string literal that look like imports are ignored, and a genuine
`import x from 'lodash'` still throws. Verified by adding a real bare import to
a grader and confirming stage-graders still exits 1.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@alexweininger
Alex Weininger (alexweininger) merged commit 5853938 into feat/CoR Aug 26, 2026
5 checks passed
@alexweininger
Alex Weininger (alexweininger) deleted the alexweininger-import-scanner branch August 26, 2026 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant