Skip to content

test(core): the HTML scanner's linear-work checks count work instead of timing it - #5037

Merged
miguel-heygen merged 1 commit into
mainfrom
fix/core-html-scan-test-budget
Oct 4, 2026
Merged

miguel-heygen merged 1 commit into
mainfrom
fix/core-html-scan-test-budget

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

What

The HTML scanner's "stays linear" tests in core/src/compiler/htmlDocument.test.ts no longer use a stopwatch. They count the work the scanner does instead, so they give the same answer on every machine and under any load, and still fail if the scanner turns quadratic.

Why

The 13 "stays fast on …" cases timed injectTagsAtHeadStart on a 2 MB adversarial input against a fixed 1500 ms budget, and the raw-text case against 500 ms. The scanner itself is linear and fast (about 0.1 µs per < on an idle machine), but on a busy machine (load average around 20, parallel test workers) the same input took 2 to 3.7 s and the case failed, then passed when run alone. A fixed wall-clock budget cannot tell a slow machine from a slow algorithm.

How

  • scannedChars(run) wraps the string methods that scan text (indexOf, lastIndexOf, includes, startsWith, charAt, replace, toLowerCase) and RegExp.prototype.exec (which test, replace and match go through) for the duration of the call, adds up the characters each call examines, then restores them. It watches only those methods and exec: text read any other way (bracket reads, at, charCodeAt, split, for..of, spreads) is not counted, so a future rewrite of the scanner's charAt reads to one of those would blind the check; the scanner uses none of them on these paths today. A sticky regex that misses is charged to the end of its input, which can only make the check stricter, never let a quadratic case through.
  • workGrowth(scan) compares the work at n = 20,000 with the work at n = 10,000. Linear work gives about 2; quadratic work gives about 4. Each case expects less than 2.5.
  • The 13 adversarial inputs and the raw-text case are rewritten as functions of n; the raw-text case keeps its two correctness checks on a small input.

No timers remain in the file, and the 13 adversarial inputs are 100 times smaller, so the file runs in about 0.1 s instead of about 0.9 s.

Test plan

  • htmlDocument.test.ts 3 runs in a row, all green, numbers identical each run.
  • Mutants: re-lowercasing the whole document on every markup step in markupStarts (a realistic accidental O(n²)) fails the 4 cases that exercise that path at growth 4.0; searching for a comment's end from the start of the document every time fails "many comments" at 3.98. Taking a string slice inside the loop, which V8 does without copying, stays green (no false alarm).
  • The old budget failing under load is reproduced: 3 of 3 full-suite runs failed on a busy machine, all passed alone.
  • Format, lint and typecheck clean.

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 4, 2026 22:40
@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 2040 (base branch 2040), smooth 1656 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at dce56949. Approving.

What I checked:

  • The counter measures the real scanner. scannedChars patches String.prototype and RegExp.prototype.exec themselves, so the calls it counts are htmlDocument.ts's own: lowerAscii's replace and exec, markupStarts's indexOf, findTagEnd's charAt, isTagAt's startsWith, and skipComment's sticky COMMENT_END.exec. Patching exec on the prototype also routes test and the regex replace through it. The originals are restored in finally.
  • Every case measures real work. I logged the counts. At n = 10,000 each case counts 50,023 to 560,076 characters, at least 5 times n, and every ratio is 2.00 (1.998 to 2.001). No case is linear just because it counts close to nothing.
  • Inputs are built outside the count. repeat and template literals aren't patched. The raw-text case's two correctness checks moved to n = 3 and still compare the whole output.

Tests I ran:

  • htmlDocument.test.ts: 51 of 51, three runs in a row, about 0.11 s of test time each.
  • Mutants I added, beyond the two in the PR body:
    • isTagAt written as lowered.indexOf(token, at) === at, a realistic O(n²) slip: 4 cases fail (raw-text, many <, many , many comments).
    • Re-lowercasing the document for every tag in findDocumentTag: the same 4 fail.
    • Ignoring the unclosedRawText memo in skipRawText: the raw-text case fails.
    • A quadratic loop hidden in bracket reads in findTagEnd: 12 of 13 adversarial cases stay green, and the raw-text case fails only by running 292 s. That is the blind spot your body names. It confirms the counter depends on the scanner keeping its reads to the patched methods.

Reuse: the same stopwatch is one file over, on the same scanner. scenePartsManifest.test.ts:67-78 times addScenePartsManifest (which calls injectTagsAtHeadStart) on 40,000 <head against 1000 ms, which is the same flake on a loaded runner. browserManager.test.ts:223-235 (500 ms) and lint/sourceLocations.test.ts:205-211 (1000 ms) have the same shape. Moving scannedChars and workGrowth into a shared test helper and converting at least the scenePartsManifest case would close the flake for that scanner. That fits a follow-up, so I'm not blocking on it.

Simpler? Hardly. Counting characters examined is the smallest thing that is independent of load. The only trim is includes, which nothing in htmlDocument.ts calls.

Verdict: APPROVE
Reasoning: The counter instruments the scanner's own string and regex calls. Every case counts real, linear work at a ratio of 2.00, and three realistic quadratic slips each fail their cases. The one blind spot is documented in the PR body, and I confirmed it.

— Rames Jusso

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit e703c39 Oct 4, 2026
165 checks passed
@miguel-heygen
miguel-heygen deleted the fix/core-html-scan-test-budget branch October 4, 2026 23:48
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.

2 participants