Skip to content

test: bound the duplicate-reference scan cost instead of trusting a timeout (#705) - #746

Merged
HugoXK merged 1 commit into
mainfrom
dev3/705-timing-assertion
Nov 15, 2026
Merged

HugoXK merged 1 commit into
mainfrom
dev3/705-timing-assertion

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 15, 2026

Copy link
Copy Markdown
Owner

Written-by: HugoXK
Author: Fledge Platform Dev-3

Fixes #705

What changes

The two stall tests in test/duplicates.test.ts (:70 slash-less body, :82 many references) asserted elapsed < 500 milliseconds. They now count the characters the reference scan copies, which is the work the quadratic path actually wastes, and which cannot move with the host's load.

CountingText is a String subclass whose slice adds the returned length to a counter, so the test measures the scan's own reads without a clock and without touching product code. The budget is one fixed window per reference — WINDOW_BUDGET = 512, against the scan's actual 256-character window — so 21,845 references get 11,184,640 characters, and a single reference gets 512. A quadratic implementation copies each reference's whole prefix; the broken versions measured 5,942 ms and 4,481 ms, and copy 65,001 and 715,795,115 characters respectively.

Both assertions are fixed in this change. The sibling at :70 carried the same defect and leaving it would have kept one load-dependent red in the file.

Why the old assertion was wrong

It measured the host. Same tree, same commit: 576 ms with two npm run check runs in flight, 1,024 ms on a loaded host, 13/13 for the file alone. ci-ok is a required check, so a red that depends on neighbouring load teaches everyone to re-run until green — which is exactly how a real regression gets waved through later.

Evidence

Green at this tip (forward-merged onto live main; see below): test/duplicates.test.ts 13/13, npm run check exit 0 with 1824/1824 (lint, typecheck, tests, build).

Forward-merged live main 710c118a95c29a0b9adebc494dd20e4b2365473c into this branch, never a rebase: merge commit 3e21a510bac318fcf2be3876e3e15ff6f06a5d25 (parents 0d5cf48cacf0e5ec02e14f4b9136a97165dc33b0 and 710c118a95c29a0b9adebc494dd20e4b2365473c), clean, no conflict — main has not touched test/duplicates.test.ts since the branch was cut. Landing tree f80ed89a0ed3ff63688d0c6f67ac9f849d39e737; npm run check exit 0 with 1824/1824 there.

Red half, and it is the real thing rather than a number that moved: I restored the pre-#113 scan in the worktree — the unanchored REPOSITORY_PATH pattern and text.slice(0, index) instead of the bounded window — and ran the new tests. # pass 11 / # fail 2:

  • a long slash-less body cannot stall the event loop: one reference copied 65001 characters; a quadratic path copies the whole 65003-character prefix
  • a body with many issue references cannot stall the event loop: 21845 references copied 715795115 characters (budget 11184640); a quadratic scan copies the whole prefix for each

Then the bounded scan was restored from git and the same tests pass 13/13. The red is a genuine quadratic implementation failing on the property, not a wall-clock threshold nudged.

Load independence, measured rather than argued: four busy cores spinning on this host, the two stall tests ran 2/2 and passed in 302 ms of wall time. Nothing in the assertion depends on that 302 ms.

Risk

T1 (test/duplicates.test.ts only, +34/−11). No product code changes, so no behaviour can regress; the failure mode if this is wrong is a test that stops catching a quadratic scan, and the red half above is the property it now pins. The counter reads only strings the scan already slices, so it cannot mask a copy — a scan that copies a prefix through another string method would slip past the counter, which is why the red half uses the actual pre-fix code rather than a synthetic one.

Docs-not-needed: test-only; no product behaviour changes.

Ran:

  • npx tsx --test test/duplicates.test.ts — 13/13 at this tip; 11/13 against the restored quadratic scan
  • npx tsx --test --test-name-pattern="stall the event loop" test/duplicates.test.ts under four busy cores — 2/2, 302 ms
  • npm run check — exit 0, 1824/1824 at this tip (after the forward-merge)
  • forward-merge of live main 710c118a into the branch — merge 3e21a51, tree f80ed89a, no conflicts; the run above is on that landing
  • node scripts/changelog.mjs check — 13 fragment(s) ok
  • node scripts/version.mjs check — 0.3.6 ok
  • node scripts/risk-gate.mjs --base 710c118a --head HEAD — risk-tier: T1; basis: test/duplicates.test.ts

Not run: the Node 24 half of the CI matrix (CI runs it), and the rest of the suite under artificial load — the assertion no longer has a clock, so load cannot decide it.

Tip SHA: b29f424
Tree: 7403c7435a7692c75493c294d616635383258800
Base: fc6825c (live main, after #736/#737/#741 merged)
Diff: 1 file, +34/−11

Transport: this seat has no push credential, so the branch is handed over as a bundle (notes/705/705-timing-710c118a.bundle, sha256 4519560985109abcb5c1d68b721f5022ae0f4a023dd7171bb7bc31b58091f48f, ref refs/heads/dev3/705-timing-assertion at 3e21a51, prerequisites e600d932 and 710c118a) and Fledge Platform Lead pushes it to create the branch — same tree, same author, no force.

Push record: Dev-3's bundle could not be applied here because its prerequisite
710c118a is not in the Lead's clone, and the git transport to github.com is
down, so the Lead rebuilt the same single-file change from Dev-3's tested
worktree content (test/duplicates.test.ts, byte-identical to tip 3e21a510's
tree f80ed89a) onto live main through the API Git Data path. Same author
Day Dreamer, same +34/−11, no force-push and no reviewed tip rewritten.

… timeout (#705)

Replayed onto live main by the Lead from Dev-3's tested tip 3e21a510 (same
file content); Dev-3's seat has no push credential.

@HugoXK-bot HugoXK-bot 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.

Verdict: PASS at b29f424
Written-by: HugoXK-bot
Author: Fledge Platform Reviewer
Posted-by: HugoXK-bot

Test-only change (#705): the two duplicate-reference stall tests stop asserting a wall-clock budget and count the characters the reference scan copies.

Reviewed at the tip: test/duplicates.test.ts (+34/−11), one file. The product code is untouched (src/lib/duplicates.ts is not in the diff).

What I checked:

  • the measure observes the real work: CountingText.slice adds the returned length to a counter, and the scan calls text.slice(index - 256, index) (src/lib/duplicates.ts:34-37), so the counter increments on every reference. I ran the fixture outside the suite: one reference in a 65,000-character body reads 256 characters, and 21,845 references read 5,581,355 characters against the 11,184,640 budget — non-zero and far below a prefix copy.
  • the budget is fixed per reference (WINDOW_BUDGET = 512 against the 256-character window), so it does not depend on host load and leaves room for the trim and a wider window without allowing a quadratic copy (which would be tens of thousands per reference).
  • both tests still assert the functional result (issueNumberFromText returns 1; findDuplicatePullRequest returns null), so the measure is attached to the same call the old timeout wrapped.
  • the wall clock is gone from both; no performance.now remains in the file.

Ran (this turn, at this tip):

  • npx tsx --test test/duplicates.test.ts — pass 13/13
  • an out-of-tree probe of the two fixtures printing charactersRead — 256 for one reference, 5,581,355 for 21,845 references

Not run: the full npm run check; ci-ok covers the build at this tip, and no product file changed.

Non-blocking note for the next edit of this test: the probe only overrides slice. A future quadratic path rewritten with substring/substr, or one that stops copying, would read 0 and still satisfy <= budget; adding a lower bound (assert.ok(body.charactersRead > 0), or counting substring too) would make a blind probe fail instead of pass. The current scan does use slice, so the test is meaningful today.

Conclusion: PASS — the assertion now measures the scan's own copying, is non-vacuous at this tip, and no longer depends on host load.

@HugoXK-bot HugoXK-bot added the review:approved Issue reviewed and approved by the Fledge Platform Reviewer label Nov 15, 2026
@HugoXK
HugoXK merged commit ef5a30b into main Nov 15, 2026
32 checks passed
@HugoXK
HugoXK deleted the dev3/705-timing-assertion branch November 15, 2026 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review:approved Issue reviewed and approved by the Fledge Platform Reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: the duplicate-reference timing test goes red under load because it asserts a wall-clock bound

2 participants