Skip to content

test(china-policy): interleave the superlinear-rescan measurements (#6985) - #7020

Open
Yigtwxx wants to merge 1 commit into
koala73:mainfrom
Yigtwxx:test/china-policy-superlinear-guard-interleave-6985
Open

test(china-policy): interleave the superlinear-rescan measurements (#6985)#7020
Yigtwxx wants to merge 1 commit into
koala73:mainfrom
Yigtwxx:test/china-policy-superlinear-guard-interleave-6985

Conversation

@Yigtwxx

@Yigtwxx Yigtwxx commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #6985.

The diagnosis in the issue holds: the 8x threshold is fine and the measurement method is what flakes. On main, tests/china-policy-events.test.mts:191-215:

const timeAtSize = (repeat: number): number => {
  ...
  let best = Number.POSITIVE_INFINITY;
  for (let attempt = 0; attempt < 3; attempt += 1) { ... }   // best-of-3 INSIDE one size
  return best;
};

const base = timeAtSize(4_000);
const quadrupled = timeAtSize(16_000);
assert.ok(quadrupled <= base * 8 + 2, ...);

Best-of-N inside one size only absorbs a stall that lands inside that size. Because the two sizes are timed as two contiguous blocks, a stall correlates across every sample of whichever block it overlaps — a single ~20 ms scheduling gap covers a whole block, and the ratio inflates with no change in the parser. Interleaving spreads each size's samples across the full window, so the same gap has to recur on every attempt of the larger size to survive the Math.min. That is what this PR implements.

Two things came out of trying to verify it that are worth more than the diff.

Math.min per size is the right form, and the alternative is measurably wrong

My first implementation went further than the issue asked. Interleaving alone still lets the base come from a quiet attempt and the quadrupled from a busy one, so I made each attempt keep its own pair and let the attempt with the best pair decide. Across 44 rounds under load that looked like a clear win: p90 fell from 5.6x to 3.9x and the worst observation from 7.2x to 4.6x.

It was wrong, and the positive control below is what caught it.

Timing noise is one-sided — a measurement can only be slower than the work, never faster — so the fastest sample of each size is the best available estimate of its true cost, and Math.min per size is an unbiased estimator. Taking the smallest ratio is not: it preferentially selects the attempt whose denominator was stalled. The tighter distribution was bias, not precision. Two observations make that concrete:

  • Over 60 rounds the ratio form reported a minimum of 1.8x. A 4x size step cannot cost 1.8x — the reading is physically impossible, which is the signature of a downward-biased estimator.
  • On the mutation below (a genuinely quadratic parser), the ratio form cleared the gate by 42.1 ms; Math.min per size clears it by 92.2 ms. The bias was eating more than half the guard's teeth.

And it was not theoretical: across 22 isolation runs the ratio form let the known-quadratic control through twice, once at 7.7x against the 8x gate.

So this PR ships the form the issue specifies. The rejected alternative is recorded here because "the ratio is quieter" is a tempting and wrong reason to prefer it.

Positive control

Acceptance criterion (c) asked that a deliberately backtracking implementation still fail the guard, and there is no seam in the production parser for injecting one. Rather than add one, the new keeps its teeth test runs a known-quadratic scan through the same measureScaling harness and asserts it trips the same gate. It walks the same unbalanced input with the unbounded prefix rescan that parsePolicyHtmlFields' closing-tag unwind degenerates into once openTagCounts stops bounding it. It runs at baseRepeat: 250 because a quadratic scan at 16k would dominate the file's runtime.

Its margin is not marginal: over 20 samples the control scales 14.8–19.4x against an 8x gate, with a minimum margin of 14.9 ms above the ceiling.

What I could not show

I could not reproduce the failure itself. Three load models on a 20-core machine — steady oversubscription, a load ramp, and bursts pulsed at the same timescale as one measurement — produced 0 readings above 8x in ~220 measured rounds for either the sequential or the interleaved shape, and no separation between them (worst observations 5.9x vs 6.0x at n=60, 6.6x vs 7.8x under bursts).

The most likely reason is machine size: 16-way concurrency on 20 cores is not the loaded 2–4 core box where you saw it. The first two load models also failed for a reason worth recording — a full six-measurement round takes ~25 ms, and both varied load on a scale of hundreds of milliseconds, so the load was effectively constant across the round and could not produce a between-size mismatch at all.

So the argument for interleaving here is structural rather than measured: it is the same number of samples per size, at the same cost, drawn from the whole window instead of from one contiguous block, and it cannot be worse under one-sided noise. If you would rather see a reproduction before taking it, say so and I will chase it on a constrained-CPU runner instead.

Verification

Isolation. npx tsx --test tests/china-policy-events.test.mts — 27 pass / 0 fail (26 on main; the control is the new one). 25 consecutive runs, 25 green.

Acceptance criterion (b) — loaded and concurrent. Ten consecutive runs of npx tsx --test --test-concurrency=16 over a batch including this file, with 40 CPU spinners contending throughout: 10 pass / 0 fail.

Mutation proofs. Each applied, observed, then restored from a file copy.

  1. A real, semantics-preserving quadratic regression in the parser. Replaced the constant-time early-out in scripts/china-policy/adapters.mjs, if (!openTagCounts.has(tag.name)) return;, with the equivalent-but-linear if (!stack.includes(tag.name)) return; — the kind of simplification a future change could plausibly make. Every correctness test in the file still passes, parses bounded hostile markup correctly included; the timing guard is the only thing that fires:

    quadrupling the input scaled cost 13.0x — linear is ~4x, catastrophic backtracking ~16x (18.8ms → 244.5ms, 92.2ms against the ceiling)

    (Deleting that early-out outright is not a usable mutant: it throws Invalid array length and takes a correctness test down with it, so it proves nothing about the timing gate.)

  2. Linearise the positive control (inner bound index1). The control fails, as it must — scaled only 4.0x ... -2.2ms against the ceiling.

  3. Loosen the gate (base * 8 + 2base * 800 + 2). The control fails at 17.3x. So the control guards the threshold too: a future "just relax the gate" fix cannot land quietly.

Cost. +180 ms on this file — 616 ms on main versus 796 ms on the branch, mean of three runs each. Effectively all of it is the positive control.

Neighbouring gates. npx biome lint tests/china-policy-events.test.mts clean, identical to main. No tsconfig project includes this file (tsconfig.json has "include": ["src"]; tsconfig.contract-tests.json names eight files, none of them this one), so npm run typecheck is unaffected; a standalone tsc --strict over the file reports 39 pre-existing errors, none inside the new block.

Full suite. npm run test:data on the branch and on a clean origin/main at 02840e574. Test count 25210 → 25211, the new control. Comparing failure sets rather than counts, the branch carries exactly one name main does not: pre-push heavy-phase admission (tests/prepush-admission.test.mjs). It is a pre-existing flake and not a regression — the suite builds its own throwaway git repo with five linked worktrees and races workers for two heavy-phase slots, so it does not read this repository's state at all, and on a clean origin/main checkout it fails 4 of 6 isolated runs (3 of 4 on the branch). Every other name matches in both directions.

Design decisions for maintainer review

  1. SCALING_ATTEMPTS stays 3, unchanged from main. More attempts lower the odds of a correlated stall at a linear cost in runtime, but I have no measurement showing 3 is insufficient, so raising it would be guessing.

  2. No third size. The issue floats an optional 64k measurement asserting the ratio does not grow. A 64k input is four times the largest allocation this file makes today, and its ratio would carry exactly the same fragility as the first one unless it were interleaved too — at which point it is a second copy of the assertion already made, for roughly a second of runtime. It would add cost without adding a distinct claim.

  3. The failure message now carries the margin in milliseconds alongside the ratio, because the ratio alone does not say how close the run came to the +2 ms absolute term. That is what made the biased-estimator problem visible.

Out of scope

tests/cross-strait-activity.test.mts:251,269,287,312,630 carry five absolute wall-clock gates of the form assert.ok(elapsedMs < 1_500, ...). That is precisely the anti-pattern the comment at tests/china-policy-events.test.mts:181-183 says this test was written to avoid, and those five will flake under the same concurrency for a simpler reason — one measurement, no ratio, no best-of-N. Different fix, different file; they deserve their own issue rather than a place in this diff.

Type of change

  • Bug fix
  • New feature
  • New data source / feed
  • New map layer
  • Refactor / code cleanup
  • Documentation
  • CI / Build / Infrastructure

Affected areas

  • Map / Globe
  • News panels / RSS feeds
  • AI Insights / World Brief
  • Market Radar / Crypto
  • Desktop app (Tauri)
  • API endpoints (/api/*)
  • Config / Settings
  • Other: tests/china-policy-events.test.mts only — the China policy adapter regression suite. No production code changes.

Checklist

  • Tested on worldmonitor.app variant — N/A, test-only change with no runtime or UI surface.
  • Tested on tech.worldmonitor.app variant (if applicable) — N/A, same reason.
  • New RSS feed domains added to api/rss-proxy.js allowlist (if adding feeds) — N/A, no feeds added.
  • No API keys or secrets committed
  • TypeScript compiles without errors (npm run typecheck) — unaffected by this PR; this file is not in any tsc project. Verified separately with a standalone tsc --strict pass: no errors inside the new block.
  • New or repointed health probes ... (if applicable) — N/A, no health probes are added, repointed or touched.

Documentation Alignment Checklist

  • Claim ledger attached or linked — N/A
  • All required Audit Council role signoffs attached — N/A
  • Generated docs regenerated from proto where applicable — N/A
  • Fixture-backed examples recomputed — N/A
  • Redis writers/readers enumerated for every documented key — N/A

N/A for this section: this PR publishes and changes no documentation claim. It touches one test file and no doc, proto, OpenAPI, example, Redis key, or methodology surface.

Screenshots

N/A — no UI surface.

…oala73#6985)

The 8x gate is a ratio between two timings, so it is only trustworthy when
both of them saw the same machine. Best-of-3 sat inside a single size, and
the two sizes were timed as two contiguous blocks -- which makes a stall
correlate across every sample of whichever block it overlaps. A ~20ms
scheduling gap covers a whole block, and that is what test:data's 16-way
concurrency produces.

Interleaving spreads each size's samples across the full window, so the same
gap has to recur on every attempt of the larger size to survive the Math.min.

Math.min stays per size rather than becoming the minimum of the per-attempt
ratios: timing noise is one-sided, so the fastest sample of each size is the
best estimate of its true cost, while the smallest ratio preferentially
selects the attempt whose denominator was stalled.

Adds a positive control -- a known-quadratic scan run through the same
harness and required to trip the same gate. It earned its place immediately
by refuting the ratio form above, which let it through at 7.7x.

The parser, the threshold, the 4x size step and the hostile inputs are
unchanged.
@vercel

vercel Bot commented Aug 20, 2026

Copy link
Copy Markdown

@Yigtwxx is attempting to deploy a commit to the World Monitor Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added the trust:safe Brin: contributor trust score safe label Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

trust:safe Brin: contributor trust score safe

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(china-policy): the superlinear-rescan guard measures its two sizes sequentially and flakes under concurrent test:data

1 participant