fix(plugin): scope the demand ladder's visit signal to bots and to targets it can act on

packages/plugin/METRICS.md

@@ -139,8 +139,27 @@ Notes that bite:
   `analytics.recordUnmatched` for requests whose UA yields no name). No rows ≠ no traffic.
 - **`queue_health`'s snapshot series and `prerender_ops`' `demand_*` are gauges on a slow cadence**, one set per node
   (`management.backlogSnapshotInterval`, `render.demand.statsInterval`). Chart the latest value;
-  never sum a gauge over time. `overdue` sums across nodes and `fill` averages. The ladder's
-  decision counters are NOT gauges — they are per-interval counts and must be summed.
+  never sum a gauge over time. `overdue` sums across nodes; `fill` takes its **peak** (below). The
+  ladder's decision counters are NOT gauges — they are per-interval counts and must be summed.
+- **`demand_fill` is a sawtooth — read max/p95, never the mean.** It is the set-bit fraction of the
+  newest visit-filter slice, which resets to ~0 at each `sliceMs` rollover and climbs until the next
+  one, so an average reports the middle of the ramp while the ladder's decisions land all along it,
+  including at the top. The false-positive rate is `fill^k` (k = `render.demand.hashes`, 7 by
+  default), and that exponent is what makes the distinction matter rather than nitpicking: a mean of
+  0.75 implies 13% false positives, the 0.986 peak of the same slice implies **91%**. A saturated ring
+  reads "visited" for everything, so the ladder promotes the whole corpus to its floor — cadence
+  stops being demand-driven and no other metric says so. Fix it by cutting what goes in
+  (`render.demand.bots`, and the rotation gate that keeps URLs owning no Target out) before raising
+  `bitsPerSlice`, whose row replicates on every flush.
+- **`demand_fill` and crawl breadth measure different populations on purpose, and are expected to
+  diverge.** Crawl breadth (`GET crawl-breadth`) answers _what did crawlers ask for_ — every bot,
+  every route class, gated only on `analytics.enabled`. The visit ring answers _what can the ladder
+  act on_ — prerender-class URLs that own a Target (or are being minted one), from bots in
+  `render.demand.bots`. From v0.61.0 the second is a strict subset of the first, so `demand_fill`
+  falling while breadth holds flat is the gates working, not the filter breaking. Neither number
+  distinguishes a real crawler from a spoofed User-Agent: bot names come from the UA string alone,
+  with no reverse-DNS or IP verification, so cross-check a search engine's own reporting before
+  treating a per-bot breadth figure as that engine's crawl rate.
 - **`queue_health.overdue` includes in-flight renders** (a leased row keeps its past due time), so
   its healthy floor is the in-flight count, not zero — and it is not comparable with numbers from
   before v0.34.0. The scan is capped by `management.scanCap`: a backlog past the cap reports the cap.

packages/plugin/package.json

@@ -1,6 +1,6 @@
 {
 	"name": "@harperfast/prerender",
-	"version": "0.60.0",
+	"version": "0.61.0",
 	"type": "module",
 	"description": "Configurable Harper plugin for prerendering pages for bots and crawlers",
 	"license": "Apache-2.0",

packages/plugin/src/configSchema.js

@@ -1195,6 +1195,18 @@ export const configSchema = group('Prerender plugin configuration.', {
 						'nothing.',
 					{ min: 0, max: 1 }
 				),
+				bots: option(
+					['*'],
+					'Bots whose visits count as demand, by the bot name the analytics registry resolves ' +
+						"(analytics.bots / derived names / the literal 'other'), compared case-insensitively. " +
+						"['*'] (default) counts every bot; [] counts none, so every target rests at its route " +
+						'cadence; a list counts exactly those. Same shape and matching rules as ' +
+						'`ingress.discoveryBots`, and worth setting for a related reason: cadence is render ' +
+						'budget, so whoever this counts decides where that budget goes, and a third-party ' +
+						'crawler walking the corpus breadth-first promotes pages no search engine asked for. ' +
+						'It is also the first lever on ring saturation — see `bitsPerSlice`.',
+					{ itemType: 'string' }
+				),
 				sliceMs: option(
 					6 * HOUR,
 					'Time resolution of the visit ring. Cannot be coarser than the fastest rung or that rung ' +
@@ -1211,8 +1223,21 @@ export const configSchema = group('Prerender plugin configuration.', {
 					1 << 20,
 					'Bloom filter bits per ring slice, rounded UP to a power of two at use (byte sizing and ' +
 						'probe spread both require it). ~1M bits holds ~100k distinct URLs per slice at ~1% ' +
-						'false positives. False positives promote a page nobody asked for — wasted renders, ' +
-						'never staleness — and there are no false negatives.',
+						'false positives. There are no false negatives, so a visited page is never demoted for ' +
+						'lack of evidence.\n\n' +
+						'SIZE THIS AGAINST THE DISTINCT-URL RATE, and treat overshoot as a correctness problem ' +
+						'rather than a cost one. A slice holding n distinct URLs fills to `1 - e^(-kn/m)`, and ' +
+						'the false-positive rate is `fill^k` — so it degrades not gradually but off a cliff: at ' +
+						'the default k=7 and m=1M, 100k URLs fills to 0.49 (~0.7% false), 320k to 0.88 (~41%), ' +
+						'640k to 0.986 (~91%). Past that the ring answers "visited" for essentially everything, ' +
+						'the ladder promotes the whole corpus to its floor, and nothing about the failure is ' +
+						'loud — the cadence just stops being demand-driven. Watch `demand_fill` at its PEAK, ' +
+						'not its mean (it is a sawtooth that resets each slice).\n\n' +
+						'Raising this is the last lever, not the first: the row is `bitsPerSlice / 8` bytes and ' +
+						'REPLICATES on every flush, which is how the per-worker version of this write produced ' +
+						'a transaction log two orders of magnitude larger than the state it carried. Cut what ' +
+						'goes in first — `bots` above, and the rotation gate in `recordDemand` — since a URL ' +
+						'the ladder can never act on is pure fill.',
 					{ min: 1024 }
 				),
 				hashes: option(7, 'Bloom hash count (k).', { min: 1, max: 32 }),

packages/plugin/src/http_handlers/bot_request.js

@@ -1,6 +1,6 @@
 import { setTimeout as sleep } from 'node:timers/promises';
 import { CacheKey } from '../util/cacheKey.js';
-import { getBotName, botMayDiscover } from '../util/userAgent.js';
+import { getBotName, botMayDiscover, botCountsAsDemand } from '../util/userAgent.js';
 import { isPrerenderCandidate } from '../util/indexSignals.js';
 import { canonicalizeUrl } from '../util/url.js';
 import { config } from '../config.js';
@@ -49,24 +49,14 @@ export async function handleBotRequest(request) {
 			recordCrawl(request.botName, cacheUrl);
 		}
 
-		// Demand signal for the render ladder. Deliberately OUTSIDE the `recordBots` gate: that
-		// gate is about analytics volume, whereas this feeds scheduling — a deployment that turns
-		// analytics down must not silently demote its whole corpus for lack of observed traffic.
-		// Keyed on the device-free URL, since cadence resolves per URL and dropping the device
-		// split halves the distinct count the filter carries. No-op unless render.demand.enabled.
-		// Prerender-class only: those are the only keys the ladder ever probes (proxied and
-		// unclassified paths own no Target), and recording the plentiful junk URLs the CDN
-		// over-forwards would only raise the filter's fill factor — at high fill the
-		// false-positive rate explodes and the ladder degenerates into promoting everything.
-		if (routeClass === PRERENDER) recordVisit(cacheUrl);
-
 		// Debug/observability info surfaced as x-harper-* response headers (only when the
 		// debug header is present). `route` is the matched route entry, if any; `routeClass`
 		// decides whether this request is cached and scheduled at all.
 		const info = { route, routeClass };
 
 		const resource = await resolveResource({ request, url, cacheUrl, deviceType, routeClass, info });
 		maybeSchedule(resource, routeClass, route, request.botName);
+		recordDemand({ resource, routeClass, route, cacheUrl, botName: request.botName, cacheStatus: info.cacheStatus });
 		// DEMAND-DRIVEN HEAL, default off and a no-op unless an invalidation is what cost this request
 		// its cache serve (`info.invalidatedBy` is set only when the epoch was consulted, which happens
 		// only when the page would otherwise have been served). Detached inside, like maybeSchedule —
@@ -374,6 +364,49 @@ function maybeSchedule(resource, routeClass, route, botName) {
 	setImmediate(handlePageScheduling, resource);
 }
 
+// Cache statuses that never looked for a page row, so they can neither prove nor disprove that a
+// Target exists: a non-GET (`bypass`) and a deliberate cache skip (`skip`). They are not `miss`
+// either — a miss LOOKED and found nothing — so they take neither branch below and record no
+// demand. That is the conservative reading of an ambiguous request, not a claim that on-demand
+// traffic is illegitimate: a render-now MISS still records, because it looked, found nothing, and
+// is about to mint the Target that makes the URL real.
+const NO_PAGE_LOOKUP = new Set(['bypass', 'skip']);
+
+// Demand signal for the render ladder (util/visitFilter.js -> util/demandLadder.js). Keyed on
+// the device-free URL, since cadence resolves per URL and dropping the device split halves the
+// distinct count the filter carries. No-op unless `render.demand.enabled`.
+//
+// DELIBERATELY OUTSIDE the `recordBots` analytics gate: that gate is about analytics volume,
+// whereas this feeds scheduling — turning analytics down must not silently demote the corpus for
+// lack of observed traffic.
+//
+// BOTH GATES BELOW EXIST TO HOLD THE RING'S FILL FACTOR DOWN, and that is a correctness concern,
+// not a tidiness one. Fill sets the false-positive rate (~fill^k), and a saturated ring does not
+// fail loudly — it answers "visited" for everything, so the ladder promotes the whole corpus to
+// its floor and the visit signal stops being a signal. `bitsPerSlice` is sized for the URLs the
+// ladder can actually act on, so anything else recorded here is spent budget.
+//
+//   ROTATION. The ladder only ever probes URLs that own a Target. `cacheStatus` is the sharper
+//   test than `resource.miss`: 'stale', 'invalidated', 'blob-missing' and 'blob-timeout' all
+//   served from the origin but FOUND a page row, and a page row exists only where a Target does,
+//   so treating those as unvisited would demote pages that are merely late. 'miss' is the one
+//   status that proves nothing was found, and it still counts when THIS request is what puts the
+//   URL into the rotation — a cacheable 200 on a route that mints targets. It does not when the
+//   route is discovery-gated (combinatorial facet URLs, which own no Target and never will) or
+//   the origin returned no page (404s for URLs that predate the deployment).
+//
+//   BOT. `render.demand.bots`, mirroring `ingress.discoveryBots`. Cadence is render budget, so a
+//   deployment usually wants it allocated by the engines it serves rather than by every crawler
+//   that walks the corpus.
+export function recordDemand({ resource, routeClass, route, cacheUrl, botName, cacheStatus }) {
+	if (routeClass !== PRERENDER) return;
+	const owned = cacheStatus !== 'miss' && !NO_PAGE_LOOKUP.has(cacheStatus);
+	const minting = cacheStatus === 'miss' && resource.statusCode === 200 && route?.discoverTargets !== false;
+	if (!owned && !minting) return;
+	if (!botCountsAsDemand(botName)) return;
+	recordVisit(cacheUrl);
+}
+
 // On-demand render: force an immediate one-off render and wait for the fresh result,
 // bypassing both the cache and the origin proxy. Returns { resource, renderNowStatus }
 // where renderNowStatus is 'hit' (fresh render served) or 'timeout' (fell back).

packages/plugin/src/metrics.js

@@ -465,9 +465,15 @@ export const METRICS = Object.freeze({
 			'(promoted/demoted/held/skipped_cold/single_rung/promoted_fast/fast/graded) are per-interval/per-run counts whose `total` is the meaningful ' +
 			'sum (`count` is flushes/runs); serve_error, page_age_negative, invalidation_error, ' +
 			'invalidation_reenqueue, probe_canary_trip, probe_invalidated and discovery_gated are counters; config_warnings is a slow gauge (latest value); ' +
-			'demand_fill is a per-worker gauge — average it, never sum ' +
-			'(fill = set-bit fraction of the newest visit-filter slot; a k=7 probe false-positives at ~fill^7, ' +
-			'and false positives promote pages nobody visited — watch it before trusting the histogram). ' +
+			'demand_fill is a per-worker gauge — never sum it, and READ ITS PEAK, NOT ITS MEAN. It is the ' +
+			'set-bit fraction of the newest visit-filter slot, which resets to ~0 at every slice rollover ' +
+			'and climbs until the next one, so it is a sawtooth: averaging over a window reports the middle ' +
+			'of the ramp while the decisions that matter are made at the top of it. A k=7 probe ' +
+			'false-positives at ~fill^7, so a mean of 0.75 (13%) and a peak of 0.986 (91%) are the same slice ' +
+			'and only the second one is the answer — take max/p95 across buckets. False positives promote ' +
+			'pages nobody visited, and a saturated ring promotes the whole corpus to its floor without ' +
+			'raising any other alarm: watch this before trusting the histogram, and see ' +
+			'`render.demand.bitsPerSlice` for what to do when it is high. ' +
 			'unrouted’s bucket slot is bounded by ingress.report.maxBuckets per class. The per-level ladder ' +
 			'histogram exists only in the demand-ladder log line.',
 		dimensions: {

packages/plugin/src/util/userAgent.js

@@ -179,17 +179,21 @@ export function deriveBotName(ua) {
 	return null;
 }
 
-// ---- discovery allowlist -------------------------------------------------------------------
+// ---- bot allowlists ------------------------------------------------------------------------
 
-let discoverySet = null; // lowercase Set, or null meaning "every bot" ('*' present)
-let discoveryFrom; // the config array the current set was built from
-
-const compileDiscovery = (bots) => {
+// Shared by the two allowlists below: `['*']` compiles to null, meaning "every bot", and
+// anything else to a lowercase Set. Both cache on the IDENTITY of the config array they were
+// built from — applyOptions replaces config with a fresh object on every change, so an identity
+// check is enough to notice an edit and cheap enough for the request path.
+const compileAllowlist = (bots) => {
 	const names = (Array.isArray(bots) ? bots : []).filter((name) => typeof name === 'string' && name !== '');
 	if (names.some((name) => name === '*')) return null;
 	return new Set(names.map((name) => name.toLowerCase()));
 };
 
+let discoverySet = null; // lowercase Set, or null meaning "every bot" ('*' present)
+let discoveryFrom; // the config array the current set was built from
+
 /**
  * May a visit labeled `botName` create a NEW target (traffic discovery)?
  *
@@ -200,15 +204,41 @@ const compileDiscovery = (bots) => {
  * so a config spelling like 'googlebot' still matches the registry's 'Googlebot'.
  *
  * This gates CREATION only. Serving is untouched (an unminted URL still proxies to the
- * origin), and so is everything keyed on targets that already exist: the demand ladder's
- * visit signal, invalidation reenqueue, and the sitemap pipeline (its targets are declared,
- * not discovered).
+ * origin), and so are invalidation reenqueue and the sitemap pipeline (its targets are
+ * declared, not discovered). The demand ladder's visit signal has its OWN allowlist —
+ * `botCountsAsDemand` below — because the two questions are different: minting corpus is
+ * permanent, while counting demand only reallocates cadence within a budget that already
+ * exists, so a deployment can reasonably answer them differently.
  */
 export const botMayDiscover = (botName) => {
 	if (config.ingress.discoveryBots !== discoveryFrom) {
-		discoverySet = compileDiscovery(config.ingress.discoveryBots);
+		discoverySet = compileAllowlist(config.ingress.discoveryBots);
 		discoveryFrom = config.ingress.discoveryBots;
 	}
 	if (discoverySet === null) return true;
 	return typeof botName === 'string' && discoverySet.has(botName.toLowerCase());
 };
+
+let demandSet = null; // lowercase Set, or null meaning "every bot" ('*' present)
+let demandFrom; // the config array the current set was built from
+
+/**
+ * Does a visit labeled `botName` count as demand for the render ladder?
+ *
+ * `render.demand.bots` has the same shape and matching rules as `ingress.discoveryBots`:
+ * `['*']` (default) counts every bot, `[]` counts none (every target rests at its route
+ * cadence), a list counts exactly those, compared case-insensitively.
+ *
+ * Cadence is render budget, so whoever this counts decides where that budget goes: a scraper
+ * walking the corpus breadth-first promotes pages no search engine asked for. It also bounds
+ * what reaches the Bloom ring, whose false-positive rate rises with fill — see `recordDemand`
+ * in http_handlers/bot_request.js for why that is a correctness concern and not just a cost.
+ */
+export const botCountsAsDemand = (botName) => {
+	if (config.render.demand.bots !== demandFrom) {
+		demandSet = compileAllowlist(config.render.demand.bots);
+		demandFrom = config.render.demand.bots;
+	}
+	if (demandSet === null) return true;
+	return typeof botName === 'string' && demandSet.has(botName.toLowerCase());
+};