Render a newly declared sitemap URL soon, not a full interval later (v0.74.0)

[harper-joseph]

The gap

A sitemap CREATE calls Target.put with no explicit time, so the first render lands at getInitialRenderTime:

return currentMinuteMs(Date.now() + (fnv1a32(jitterSeed(key)) % safeInterval));

— jitter across the target's whole render interval. On a 48h PDP cadence, a product published this morning can wait two days to be rendered once, while the sitemap has been declaring it the entire time. A declaration is the strongest signal a site gives that a URL matters.

That jitter exists for a real reason: the first ingest of a large sitemap must not stampede the queue. But it's sized for bulk population and applies just as hard to the handful of genuinely new URLs a mature corpus gains each day — measured on one deployment, 65–134 creates/day against ~847k products.

The change

New sitemap.newTargets:

option	default
window	15m	jitter window for a new target's first render. 0 disables and restores the old behaviour.
maxPerRun	5000	creates per walk that may take the fast path; past it, fall back to full-interval jitter.

Jittered across the window, not set to "now" — for precisely the reason the interval jitter exists: a batch of creates must land across minutes rather than firing into one.

The cap is the bulk-population guard. A first ingest creating hundreds of thousands of targets exceeds it immediately, and everything past it takes the old path — so the case the original jitter was written for is unchanged. Steady-state churn never approaches it.

Only the first render moves. Target.put still files effectiveInterval from the route/stored cadence, so every render after this one is on the normal schedule. This is not a cadence change.

The cap is per walk, not per child sitemap — an index fanning out to 17 children must not multiply it by 17 — so it reads the run's own running total via fastPathTaken().

Observability

createdSoon on the result, the progress row and the finish log, plus a sitemap_created_soon metric. created - created_soon is the overflow that fell back to full-interval jitter, which is the number that tells you whether the cap bound.

Tests

npm test in packages/plugin: 1108 pass, 0 fail. Lint and format:check clean.

• time.test.js: over 200 URLs, a 15-minute window confines every first render to the window while a 48h interval spreads far past it — and the windowed times are still spread across many distinct minutes, pinning that this is jitter and not a single instant.
• sitemapRun.test.js: the cap is per walk and carries across children, and createdSoon is a strict subset of created so the overflow stays readable.

Scope

Sitemap creates only. Bot-discovered targets (handlePageScheduling) still take full-interval jitter — a crawler hitting an unknown URL is a much weaker signal than a site declaring one, and that path has its own discovery gates. Worth revisiting separately.

Versioning

v0.74.0, leaving v0.73.0 to #165 which is still open. If this should merge first, renumber it to 0.73.0 and bump #165 — merge order has to ascend.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a fast-path configuration (newTargets) for newly declared sitemap URLs, allowing their first render to be scheduled within a shorter jitter window (default 15 minutes) instead of the full render interval, up to a configurable cap per walk. Feedback on these changes suggests ensuring the fast path is only applied when the jitter window is shorter than the route's render interval to prevent unintended delays. Additionally, it is recommended to clarify the minimum window size required for effective jitter and enforce a maximum limit of 2147483647 on the window configuration to avoid Node.js timeout overflow warnings.

[gemini-code-assist]

If a route has a very short renderInterval (e.g., 5 or 10 minutes) that is shorter than the newTargetWindow (default 15 minutes), using the fast path would actually delay the first render compared to the standard full-interval jitter. Additionally, counting such cases as createdSoon would pollute the metric since no actual acceleration occurred.

Consider ensuring the fast path is only taken when newTargetWindow < renderInterval.

Suggested change

	const fast = newTargetWindow > 0 && run.fastPathTaken() < newTargetCap;
	const fast = newTargetWindow > 0 && newTargetWindow < renderInterval && run.fastPathTaken() < newTargetCap;

[gemini-code-assist]

Since getInitialRenderTime floors the calculated time to the minute (currentMinuteMs), any window value less than 1 * MINUTE (60,000 ms) will not effectively jitter the renders across multiple minutes. Instead, they will almost all collapse into the same minute, potentially causing a queue stampede.

Consider updating the option description to clarify that the window should be at least 2 * MINUTE to achieve multi-minute spreading, or enforce a minimum of 1 * MINUTE (60000) when window > 0.

Additionally, ensure that this configuration option enforces a maximum limit of 2147483647 (the maximum 32-bit signed integer) to prevent potential TimeoutOverflowWarning issues in Node.js if passed to setTimeout.

				window: option(
					15 * MINUTE,
					'Jitter window for a newly declared target’s first render. Small values approximate ' +
						'"immediately" while still spreading a batch across minutes rather than firing it into ' +
						'one. Values should be at least 2 * MINUTE to ensure effective multi-minute jitter. ' +
						'"0" disables the fast path entirely and restores full-interval jitter.',
					{ unit: 'ms', min: 0, max: 2147483647 }
				),

References

1. In Node.js, configuration options representing timeouts or delays passed to 'setTimeout' must enforce a maximum limit of 2147483647 (the maximum 32-bit signed integer). Exceeding this limit causes Node.js to emit a 'TimeoutOverflowWarning' and execute the callback immediately (after 1ms), which can lead to unexpected immediate timeouts and system failures.

[harper-joseph]

Reviewed as part of the combined train. Trial-merged with #165/#178/#181/#182 — 1148 tests pass together. Two notes, one of which is a number I think needs reconciling before window is sized.

Confirmed safe — I went looking and did not find a problem

• Claim floor is untouched. Target.put → writeSchedule → lowerFloorFor → lowerFloorTo, a CAS-min. Both the old due time (now + [0, interval)) and the new one (now + [0, window)) are ≥ the current minute, and readFloorMinute clamps to nowMinute − guard on every read, so neither can push the floor below where the guard band already sits.
• It does not bypass the jitter — it calls the same getInitialRenderTime(cacheUrl, newTargetWindow) with a smaller interval, so the FNV-1a-over-URL-prefix seed, the minute flooring, and the device-variant co-scheduling property (the jitterSeed URL-half rule) are all preserved. Only the width changes. That's the right way to do it.
• Cap arithmetic reads correctly: run.fastPathTaken() < newTargetCap is checked before the increment, so exactly maxPerRun take the fast path, and 0 disables cleanly.

1. The creates/day figure is off by ~25× somewhere

The PR body sizes window against 65–134 sitemap creates/day. kohls' own config comment says the daily pass created 134. But a separate measurement in the project notes puts target creation at ~27.2k discovered/day at roughly 8× the sitemap rate, implying ~3.4k sitemap creates/day.

Those can't both be right, and the gap matters: at 134/day the fast path is a rounding error; at 3.4k/day a single walk can approach maxPerRun: 5000, and 5,000 creates in a 15-minute window is ~333/min ≈ 20,000 renders/hr-equivalent — roughly 60% of documented fleet headroom for those 15 minutes. It fits, but window and maxPerRun want sizing together against headroom rather than independently.

I did not re-measure; flagging rather than asserting which is right.

Two smaller scale notes: maxPerRun is per Sitemap.refresh() call and refreshAllSitemaps makes one call per root, so N roots multiply the cap by N. And it's a per-walk budget, which is per-6h today but becomes per-few-minutes if #165's stated follow-up (dropping refreshInterval toward polling range) lands — nothing links the two settings. Worth documenting maxPerRun as requiring re-sizing whenever refreshInterval moves.

2. Gemini's open inline comment looks valid

If a route's effective interval is shorter than window, the "fast" path is slower than the default jitter and still counts as createdSoon. Not reachable at kohls (page.minTtl 24h, PDP route 96h), but newTargetWindow > 0 && newTargetWindow < interval is a free guard.

3. Merge mechanics with #165

You conflict in five files — package.json/lock, METRICS.md, configSchema.js, and two hunks in Sitemap.js (progressFields and the finish log). All additive; keep both sides throughout, and merge the log line into one template.

Two traps I hit doing this: a union resolve of configSchema.js drops the closing brace of #165's conditional group and fails to parse ~500 lines later; and keep both METRICS.md rows — metrics.sitemapRun takes a free-form series string, so test/metrics.test.js cannot detect a dropped row and a one-sided resolution ships created_soon unreadable.

Also: sitemap_created_soon has no console surface, so the rollout readout is raw prerender_ops queries for now.

🤖 Generated with Claude Code

[harper-joseph]

Both inline notes addressed in 6fef613, pushed to this branch.

The window-vs-interval guard is in: newTargetWindow > 0 && newTargetWindow < renderInterval && …. You were right that it matters for the metric as much as the behaviour — without it a window wider than the route's cadence makes the "fast" path slower than the jitter it replaces and still counts as createdSoon, so the metric reports an acceleration that did not happen. Not reachable on this corpus (shortest interval is a day), but it is free.

The sub-minute note is now in the option description rather than enforced as a minimum, because 0 has to stay meaningful as the kill switch and a hard floor would make the boundary between "off" and "too small" unreadable. It says plainly that below ~2 minutes getInitialRenderTime's minute-flooring collapses a walk's creates onto one minute — the stampede the jitter exists to prevent, reached by asking for less jitter. And max: 2147483647 is added, for the same reason sweepInterval has it: a larger delay is not "effectively never", it overflows the signed 32-bit timer and fires immediately.

1148 tests pass on the branch, lint and format clean.

🤖 Generated with Claude Code