Skip to content

A click should never wait on a prefetch that never started - #5

Merged
ephnagler merged 2 commits into
mainfrom
prefetch-peek-and-suite-speed
Sep 12, 2026
Merged

ephnagler merged 2 commits into
mainfrom
prefetch-peek-and-suite-speed

Conversation

@ephnagler

Copy link
Copy Markdown

Two fixes, both found while proving the navigation timeout in a browser.

peekInFlight handed navigate() work that had not begun

A warm lands in inFlight when it is requested, not when it reaches the
network. With both slots of MAX_CONCURRENT_PREFETCH busy, a queued warm has
sent no bytes — and navigate() awaited it anyway, stalling the click for as
long as the queue took, bounded only by the 10s nav timeout.

That inverts the guarantee the limiter's own comment makes:

leaves the connection budget for real navigation, which always bypasses this queue

A nav firing its own fetch does bypass it. A nav deduping against a queued warm
inherited the wait.

How it surfaced. While testing the timeout, two stalled warms held both
slots, the target's warm never started, and the nav awaited it for the full ten
seconds. The timeout did its job — but it should not have been the thing doing
the job.

Gated on a started set. Bypassing costs at most one speculative duplicate, for
a page the buyer has already navigated to. Wasted background bandwidth beats a
stalled click.

The suite spent 4s waiting on a cap jsdom never releases

syncHeadStyles waits up to 2s for injected stylesheets. jsdom fires neither
load nor error on an injected <link>, so two tests waited the full cap
every run.

unref alone does not fix this — it stops a timer holding the event loop open,
it does not make the await resolve. The fixture now dispatches load the way
a browser does, so the product's timeout is unchanged and the tests stop paying
for a jsdom gap. (The unref is added too, for the process-exit case.)

11.1s → 7.1s.

Checks

  • 147 tests passing (one new, covering the queued-vs-started distinction)
  • npm run typecheck clean
  • npm run build clean

🤖 Generated with Claude Code

https://claude.ai/code/session_01AK7NgrSeLCJzR3Tv5iwDVF

ephnagler pushed a commit that referenced this pull request Sep 12, 2026
The node 22 CI failure turned out to be a test leak rather than a limiter
bug, and the autofocus message turned out not to be benign — both worth
recording with the reasoning, since both were filed under a conclusion
that was the wrong way round.

Also corrects a dangling reference: STATE filed the peekInFlight finding
as "backlog item 8", but item 8 there is the Bucket H bridge snippet. The
finding was never in the backlog at all — it went straight into PR #5.
Numbers in BACKLOG.md are now stated to be stable identifiers, so closing
an item leaves a gap instead of renumbering everything other documents
cite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
ephnagler pushed a commit that referenced this pull request Sep 12, 2026
The node 22 CI failure turned out to be a test leak rather than a limiter
bug, and the autofocus message turned out not to be benign — both worth
recording with the reasoning, since both were filed under a conclusion
that was the wrong way round.

Also corrects a dangling reference: STATE filed the peekInFlight finding
as "backlog item 8", but item 8 there is the Bucket H bridge snippet. The
finding was never in the backlog at all — it went straight into PR #5.
Numbers in BACKLOG.md are now stated to be stable identifiers, so closing
an item leaves a gap instead of renumbering everything other documents
cite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
Ephriam Nagler and others added 2 commits September 12, 2026 07:19
peekInFlight handed navigate() any entry in `inFlight`, but a warm lands
there when it is requested, not when it reaches the network. With both
slots of MAX_CONCURRENT_PREFETCH busy, a queued warm has sent no bytes —
and awaiting it stalls the navigation for as long as the queue takes,
bounded only by the 10s nav timeout.

That is the opposite of what the limiter's own comment promises: "leaves
the connection budget for real navigation, which always bypasses this
queue." A nav firing its own fetch does bypass it. A nav deduping against
a queued warm inherited the wait.

Found while proving the nav timeout in a browser: two stalled warms held
both slots, the target's warm never started, and the nav awaited it for
the full ten seconds. The timeout did its job — but it should not have
been the thing doing the job.

Gate on a `started` set instead. Bypassing costs at most one speculative
duplicate, for a page the buyer has already navigated to; wasted
background bandwidth beats a stalled click.

Also cuts 4s from the suite. jsdom fires neither load nor error on an
injected <link rel=stylesheet>, so syncHeadStyles waited out its real 2s
cap in two tests. The fixture now dispatches load the way a browser does,
rather than shortening the product's timeout to suit the tests. 11.1s →
7.1s, 147 passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AK7NgrSeLCJzR3Tv5iwDVF
CI was red on node 22.13 and green on 24, failing `prefetch never runs
more than two requests at once` — a test the branch does not touch. The
failure shape named the cause: `served == 6` and `peak == 2` both passed
while `inFlight == 1` failed, and that combination is only reachable if a
SEVENTH fetch ran against a stub that had accounted for six.

warmupNavLinks schedules its work 200ms out and kept no handle, so
_resetPrefetchForTests could not cancel it. Each test tears its document
down and builds a fresh one, so a warmup scheduled by an earlier test
fired inside a later one and warmed the new document's nav links. Nothing
was wrong with the limiter; node's timer scheduling only decided which
test caught the stray. The added tests shifted the timing enough to make
an old leak land somewhere visible.

Hold the cancel thunk for every scheduled warmup and drop it on reset.
Harmless in a browser either way — one document, and warmedFrom stops a
repeat — but a handle is the right shape regardless.

The regression test asserts on fetch traffic after a reset rather than on
the concurrency counters: the stray request is the damage, wherever it
happens to land. Verified to fail without the cancel.

No change to config keys, hook or event names, bucket rules, install
layout or the --json contract, so init, audit, the skill and the README
are untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
@ephnagler
ephnagler force-pushed the prefetch-peek-and-suite-speed branch from 84cb5f1 to 912a076 Compare September 12, 2026 11:20
@ephnagler
ephnagler merged commit 932850e into main Sep 12, 2026
6 checks passed
@ephnagler
ephnagler deleted the prefetch-peek-and-suite-speed branch September 12, 2026 11:22
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