[harper-joseph]

Settle is ~80% of a render, and the plugin's store guard is statusCode === 200 && content — so a page that is already non-indexable at DOMContentLoaded can never be stored no matter how long it settles. navigation.skipSettleWhenNonIndexable (default false) decides the verdict against the pre-settle DOM right after page.goto and returns without settling when the answer is already no.

Measured on the kohls fleet, from the workers' own phaseMs:

phase	mean
navTtfb	431 ms
navTotal (full goto)	1,450 ms
settle	8,872 ms
postProcess	322 ms
whole render	10,882 ms

Non-indexable renders are 12.7% of that fleet's throughput and cost the same as indexable ones (10,878 ms vs 11,013 ms mean) — we pay full price to learn the answer. A pre-settle bail lands at ~1.7 s instead, which is ~10% of fleet capacity on that corpus.

The verdict logic moves into a shared indexVerdict() used by both the new bail and the long-standing post-settle check, so the two cannot drift on what "non-indexable" means.

Not independently reviewed by an outside model — every local outside leg was unavailable (see Review coverage). The GitHub review bots are the first outside lens on this diff.

For the human reviewer

1. Default false, so this ships inert. A site whose canonical or robots tag is written by script rather than served in the document would see pages skipped that a full render would have kept. That risk is real but unmeasurable from here, so the flag is opt-in and kohls has to set it. If you'd rather it default on, that's the call to make.
2. Pre-settle DOM, not an HTTP-response abort. Aborting on the response body would save ~1 s more per bail (navigation is only 13% of a render), but it decides on pre-parse HTML and can't reuse the post-settle check. I judged ~1.2% more throughput not worth Fetch-domain interception plus a second, weaker copy of the verdict. Reasonable to disagree.
3. It costs one extra page.evaluate on the ~87% of renders that ARE indexable, to save the settle phase on the ~13% that aren't. A CDP evaluate is single-digit ms against an 8.9 s settle, so it's strongly net-positive — but it is new work on the hot path, and it's inherent: the post-settle DOM can differ, which is the whole point, so the second evaluate can't be cached away.
4. Sitemap-listed urls are exempt (job.isFromSitemap), because the post-settle path stores their content even when non-indexable. Bailing would change what gets cached rather than only how long it took, and that would break the blast-radius guarantee redirect.test.ts pins. The exemption is asserted in the new tests too.
5. timings.settle is now absent for bailed renders, so the workers' phaseMs.settle describes only renders that actually settled. That matches what the existing redirect bail already does, but it does shift the meaning of that stat if you track it.
6. The canonical-variant case follows canonical.strict rather than introducing new policy: lenient keeps its own target and settles, strict skips. Both directions are pinned.

Verification

Route: new integration test. packages/browser/test/earlySettleSkip.test.ts — 11 tests driving renderOnce (real Chrome) against a local origin, asserting on timings.settle to prove the phase was skipped rather than merely fast.

node --test test/earlySettleSkip.test.ts   → 11/11 pass

The durations in that run are themselves the evidence: bailed renders 117–329 ms, settled renders 690–847 ms.

Covered: noindex skips settle; canonical-elsewhere skips settle; non-200 skips settle and still posts its status; an indexable page is unaffected and still serializes; a verdict injected after DOMContentLoaded is still caught post-settle (the safety property — it can only skip, never wrongly keep); a sitemap url is exempt; disabled-by-default still settles; canonical.strict governs the variant case; indexVerdict() unit cases; a bail still reports a client-side redirect it landed on; and — the case redirect.test.ts never covered — a client-side redirect to an indexable destination still renders through with the flag on.

Full gates, all green: browser 131, plugin 715, console 116. npm run lint clean, format:check clean, tsc clean.

Not verified: the throughput claim itself. The phase numbers above are measured production observations, but the post-change fleet effect can only be confirmed after deploy — the signal to watch is the existing render_outcome time_ms series for type: non-candidate, whose mean should fall from ~10.9 s toward ~1.7 s. No new instrumentation needed.

Review coverage

Generated by Claude (Opus 5). No independent outside-model coverage — read from the run's own progress.json (run-id d92c15d5):

lens	outcome
codex (graded)	failed — auth: refresh token was already used, needs codex login
gemini (agy)	skipped — not installed on this machine
cursor-grok	failed — no receipt: cursor-agent not installed (exit 127)
cursor-composer	skipped — pruned by leg policy
claude (fallback)	in flight at PR-creation time — same family, explicitly not independent

Per the cross-model skill's degraded-run rule a same-family advisory pass is not coverage, so the GitHub review bots are the first outside lens. The fallback pass did finish after the PR was opened; its findings are logged below because two of them were real, but it does not change the coverage line.

Findings acted on before review

My own read of the diff, and the same-family pass, independently surfaced the same two defects — both fixed in 54a0bd0:

1. A bail could swallow a client-side redirect. A client-side redirect can fire before DOMContentLoaded, so a bail could land on the destination and return a verdict with no redirectedTo. That matters more than it looks: RenderQueue.js:555 has a dedicated branch that deletes the source and pre-suppresses the destination when a redirect lands somewhere non-indexable — "so neither keeps rendering at full cadence" — and it only fires when redirectedTo is set. Losing it would instead suppress the source (4 rechecks over ~28 days) and leave the destination to be re-discovered and rendered from scratch. Roughly 5 renders per occurrence, in the wrong direction for a change whose whole point is load.
2. The pre-settle page.evaluate was unguarded, so an in-flight navigation destroying the execution context turned a previously-rendering page into a failed render. It now falls through to settle, which reproduces the pre-flag behavior exactly.

One finding I checked and refuted (dffab0e): the pass argued the non-200 arm was dead code — that the status >= 400 handler's ac.abort() makes goto reject, so a 404 never reaches the bail, and the /gone test should therefore fail. It doesn't. That abort only gates subresource interception; the main document has already answered when it fires, so goto resolves and a 404 settles in full — measured at 566 ms of wasted settle when I removed the arm on the strength of the review. The arm is restored, and the test now asserts navTotal is set (goto resolved) alongside settle being absent, so the saving is attributed to the bail and can't pass for the wrong reason again. Worth noting the pass said plainly that it could not execute the suite; that is exactly the class of claim execution settles.

Also addressed: the rationale had been duplicated across the config docstring, the renderer block, the README and the test header — trimmed to one statement per audience.

_{Human-Review-Need: 4 @ dffab0e}

[gemini-code-assist]

Code Review

This pull request introduces a performance optimization via the skipSettleWhenNonIndexable configuration option, allowing the renderer to skip the settle phase if the pre-settle DOM indicates the page is non-indexable. This is supported by refactoring the indexability check into a shared indexVerdict helper and adding comprehensive unit tests. Feedback suggests wrapping the page.evaluate call in a standard try...catch block to safely handle both synchronous and asynchronous exceptions across the Puppeteer interface boundary.