From 36ba4f6a5dacbb49bbfae17181cba686b8d0205b Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 18 Sep 2026 15:24:33 -0400 Subject: [PATCH 1/2] feat(plugin): explain a URL's render cadence, including the ladder rung; v0.77.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /prerender_admin/explain` exists to answer "why does this URL behave this way", and it could not answer it for the one property operators ask about most. It reported Target.renderInterval and stopped — but that is the CEILING, not the cadence. The value a row is actually scheduled from is resolveEffectiveInterval(url, target), which folds four inputs (route interval, stored interval, default, ladder rung) through two clamps (the route's demandFloor, the route's interval as a ceiling), and the answer is routinely none of the numbers an operator can see. Measured on a production cluster this week: a PDP route configured `renderInterval: 96h` was rendering every 48h. 95.7% of its targets carried a ladder rung, the route's `demandFloor: 48h` equalled the ladder's slowest rung, so `max(rung, floor)` clamped every rung to exactly 48h and the configured 96h ceiling never bound once. The knob named "floor" was the real cadence and the knob named "ceiling" was inert. Establishing that took fifteen explain calls plus reading Target.demandInterval out of the table by hand for 162 URLs and working the algebra backwards — because `demandInterval` was not in explain's select at all. So: select it, and add a `cadence` block reporting the whole chain — every input, which one supplied the base, and what clamped the result. `clampedBy` is the field to read first: 'floor' means the ladder wanted this page faster and the route refused (seeing it across a route means the ladder has no dynamic range there), 'ceiling' means a stored rung is slower than the route allows (what a rung outliving a lowered interval looks like), null means no rung. `explainCadence` lives in util/routeClass.js beside the resolver, not in the admin view that renders it, and derives from the same functions rather than recomputing the algebra. A second implementation would be a second thing to keep correct, and its failure mode is the worst available to a diagnostic: a view that confidently explains a cadence the scheduler is not using. A test cross-checks the two across the input matrix rather than trusting they stay in step. MERGE AFTER #181 (v0.76.0). Open PRs #165, #177 and #178 reserve v0.73.0, v0.74.0 and v0.75.0. Co-Authored-By: Claude Opus 5 (1M context) --- package-lock.json | 2 +- packages/plugin/package.json | 2 +- .../plugin/src/resources/PrerenderAdmin.js | 18 ++++- packages/plugin/src/util/routeClass.js | 55 ++++++++++++++ packages/plugin/test/routeClass.test.js | 75 +++++++++++++++++++ 5 files changed, 149 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index f3c5b61..90a0617 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8570,7 +8570,7 @@ }, "packages/plugin": { "name": "@harperfast/prerender", - "version": "0.72.0", + "version": "0.77.0", "license": "Apache-2.0", "dependencies": { "fast-xml-parser": "^5.0.9", diff --git a/packages/plugin/package.json b/packages/plugin/package.json index c9f114b..93f9ca1 100644 --- a/packages/plugin/package.json +++ b/packages/plugin/package.json @@ -1,6 +1,6 @@ { "name": "@harperfast/prerender", - "version": "0.72.0", + "version": "0.77.0", "type": "module", "description": "Configurable Harper plugin for prerendering pages for bots and crawlers", "license": "Apache-2.0", diff --git a/packages/plugin/src/resources/PrerenderAdmin.js b/packages/plugin/src/resources/PrerenderAdmin.js index c47ad65..e425fdf 100644 --- a/packages/plugin/src/resources/PrerenderAdmin.js +++ b/packages/plugin/src/resources/PrerenderAdmin.js @@ -99,7 +99,13 @@ import { MAX_REASON_LENGTH, CLUSTER_SCOPE as CLUSTER_INVALIDATION, } from '../util/invalidation.js'; -import { inspectRoutes, resolveEffectiveInterval, routeScopes, routeScopeForUrl } from '../util/routeClass.js'; +import { + explainCadence, + inspectRoutes, + resolveEffectiveInterval, + routeScopes, + routeScopeForUrl, +} from '../util/routeClass.js'; import { CLUSTER_SCOPE } from '../util/queueControl.js'; import { getResidencyByUrl } from '../util/residency.js'; import { fetchScheduleFromPeer } from '../util/peer.js'; @@ -1481,6 +1487,11 @@ export class PrerenderAdmin extends Resource { 'sitemapUrl', 'schedulerNode', 'renderInterval', + // The demand ladder's stored rung. Selected for the `cadence` block below, and + // WITHOUT IT THIS VIEW CANNOT EXPLAIN ITS OWN SUBJECT: the rung is what + // `resolveEffectiveInterval` actually schedules from, so a view that reports + // `renderInterval` and omits this reports the ceiling as if it were the cadence. + 'demandInterval', 'state', 'suppressedReason', 'suppressedAt', @@ -1557,6 +1568,11 @@ export class PrerenderAdmin extends Resource { return json({ ...explanation, + // How this URL's render cadence resolves, with every input and the clamp that decided it. + // Null when there is no target: cadence is a property of a URL in the rotation, and + // reporting the route's interval for a URL that owns no target would read as a schedule + // that does not exist. See `explainCadence` for why this is worth a block of its own. + cadence: target ? explainCadence(canonicalUrl, target) : null, rows: { renderTarget: target ?? null, // Already described (locally or by the owner) — see above. diff --git a/packages/plugin/src/util/routeClass.js b/packages/plugin/src/util/routeClass.js index 1338a5a..cfe333c 100644 --- a/packages/plugin/src/util/routeClass.js +++ b/packages/plugin/src/util/routeClass.js @@ -527,6 +527,61 @@ export const demandFloorFor = (url) => entryFloor(classifyUrl(url).entry); * ladder re-decides. The ladder will snap it into the floored list on its next decision; until * then this must not credit a cadence the ladder no longer grants. */ +/** + * The whole cadence resolution for one URL, as data — every input, which one won, and what + * clamped it. + * + * WHY THIS EXISTS, and why it lives HERE rather than in the admin view that renders it. The + * resolution has four inputs (route interval, stored interval, default, ladder rung) and two + * clamps (the route's `demandFloor`, the route's own interval as a ceiling), and the answer is + * routinely NONE of the numbers an operator can see. A deployment whose PDP route reads + * `renderInterval: 96h` was measured rendering every 48h, because 95.7% of its targets carried a + * rung and the route's `demandFloor: 48h` clamped every one of them to exactly the floor — so the + * configured ceiling never bound, the knob named "floor" was the real cadence, and the only way to + * discover that was to read `Target.demandInterval` out of the table by hand for a sample of URLs + * and work the algebra backwards. That is a diagnosis nobody should have to repeat. + * + * It is DERIVED FROM THE SAME FUNCTIONS THE SCHEDULER USES, never recomputed alongside them. A + * second implementation of this algebra would be a second thing to keep correct, and the failure + * it produces is the worst kind available to a diagnostic: a view that confidently explains a + * cadence the scheduler is not using. + * + * `clampedBy` is the field to read first: + * 'floor' — the ladder wanted this page faster and the route's demandFloor refused. Seeing this + * on most of a route means the ladder has no dynamic range there at all. + * 'ceiling' — the stored rung is slower than the route allows, so the route's interval won. A + * rung outliving a lowered route interval looks like this. + * null — no rung; the base interval is the cadence. + */ +export const explainCadence = (url, target = {}) => { + const { entry } = classifyUrl(url); + const storedInterval = target?.renderInterval ?? null; + const base = baseInterval(entry, storedInterval); + const floor = entryFloor(entry); + const rung = Number(target?.demandInterval); + const hasRung = Number.isFinite(rung) && rung > 0; + const floored = hasRung && floor !== null ? Math.max(rung, floor) : hasRung ? rung : null; + + return { + effectiveInterval: resolveEffectiveInterval(url, target ?? {}), + // Which input supplied the BASE — the ceiling the ladder is clamped into. `baseInterval` + // resolves route > stored > default, so this mirrors that order rather than re-deriving it. + baseFrom: + entry && entry.renderInterval !== null && entry.renderInterval !== undefined + ? 'route' + : Number.isFinite(Number(storedInterval)) && Number(storedInterval) > 0 + ? 'stored' + : 'default', + baseInterval: base, + routeInterval: entry?.renderInterval ?? null, + storedInterval, + defaultInterval: config.render.defaultInterval, + demandInterval: hasRung ? rung : null, + demandFloor: floor, + clampedBy: !hasRung ? null : floored > rung ? 'floor' : base < floored ? 'ceiling' : null, + }; +}; + export const resolveEffectiveInterval = (url, { renderInterval, demandInterval } = {}) => { const { entry } = classifyUrl(url); const base = baseInterval(entry, renderInterval); diff --git a/packages/plugin/test/routeClass.test.js b/packages/plugin/test/routeClass.test.js index f2fefe3..f5d375f 100644 --- a/packages/plugin/test/routeClass.test.js +++ b/packages/plugin/test/routeClass.test.js @@ -13,6 +13,7 @@ import { PASSTHROUGH, PRERENDER, UNCLASSIFIED, + explainCadence, } from '../src/util/routeClass.js'; const ROUTES = [ @@ -397,3 +398,77 @@ test('resolveEffectiveInterval clamps a stored rung UP to the route demandFloor' // Unset rung stays at the ceiling regardless of the floor. assert.equal(resolveEffectiveInterval(catalog, {}), DAY_MS); }); + +// ---- explainCadence ------------------------------------------------------------------------- +// +// The resolution has four inputs and two clamps, and the answer is routinely none of the numbers +// an operator can see. These pin the case that actually cost a day of diagnosis on a production +// cluster, plus the two ways it can be misread. + +const PDP = 'https://example.com/product/prd-1/thing.jsp'; + +test('explainCadence: demandFloor EQUAL to the slowest rung collapses every graded page onto the floor', () => { + // The production shape: route says 96h, ladder rungs top out at 48h, floor is 48h. Every rung + // clamps up to 48h, the 96h ceiling never binds, and the knob named "floor" IS the cadence. + forwarded({ + ingress: { + routes: [{ match: 'prefix', path: '/product/prd-', renderInterval: 96 * HOUR_MS, demandFloor: 48 * HOUR_MS }], + }, + }); + for (const rung of [6, 12, 24, 48]) { + const c = explainCadence(PDP, { renderInterval: 24 * HOUR_MS, demandInterval: rung * HOUR_MS }); + assert.equal(c.effectiveInterval, 48 * HOUR_MS, `rung ${rung}h should resolve to the 48h floor`); + assert.equal(c.baseInterval, 96 * HOUR_MS, 'the route still supplies the base'); + assert.equal(c.baseFrom, 'route'); + } + // Only the rungs the floor actually raised report `floor`; a rung already at the floor was not + // clamped by anything, and saying otherwise would overstate what the floor is doing. + assert.equal(explainCadence(PDP, { demandInterval: 6 * HOUR_MS }).clampedBy, 'floor'); + assert.equal(explainCadence(PDP, { demandInterval: 48 * HOUR_MS }).clampedBy, null); +}); + +test('explainCadence: a rung slower than the route reports the ceiling, not the floor', () => { + // A rung outliving a lowered route interval looks like this. + forwarded({ + ingress: { routes: [{ match: 'prefix', path: '/product/prd-', renderInterval: 24 * HOUR_MS }] }, + }); + const c = explainCadence(PDP, { demandInterval: 96 * HOUR_MS }); + assert.equal(c.effectiveInterval, 24 * HOUR_MS); + assert.equal(c.clampedBy, 'ceiling'); + assert.equal(c.demandInterval, 96 * HOUR_MS, 'the stale rung is still reported, not hidden'); +}); + +test('explainCadence: with no rung the base is the cadence, and its source is named', () => { + forwarded({ ingress: { routes: [{ match: 'prefix', path: '/product/prd-' }] } }); + + const stored = explainCadence(PDP, { renderInterval: 6 * HOUR_MS }); + assert.equal(stored.effectiveInterval, 6 * HOUR_MS); + assert.equal(stored.baseFrom, 'stored'); + assert.equal(stored.clampedBy, null); + assert.equal(stored.demandInterval, null); + + const fallback = explainCadence(PDP, {}); + assert.equal(fallback.baseFrom, 'default'); + assert.equal(fallback.effectiveInterval, config.render.defaultInterval); +}); + +test('explainCadence never disagrees with the resolver the scheduler actually uses', () => { + // The whole hazard of a diagnostic that recomputes: a view that confidently explains a cadence + // the scheduler is not using. Cross-check the reported value against resolveEffectiveInterval + // across the matrix rather than trusting that the two implementations stayed in step. + forwarded({ + ingress: { + routes: [{ match: 'prefix', path: '/product/prd-', renderInterval: 96 * HOUR_MS, demandFloor: 48 * HOUR_MS }], + }, + }); + for (const renderInterval of [null, 6 * HOUR_MS, 24 * HOUR_MS]) { + for (const demandInterval of [null, 6 * HOUR_MS, 48 * HOUR_MS, 96 * HOUR_MS]) { + const target = { renderInterval, demandInterval }; + assert.equal( + explainCadence(PDP, target).effectiveInterval, + resolveEffectiveInterval(PDP, target), + `disagreed for ${JSON.stringify(target)}` + ); + } + } +}); From 1738aa4b9b30e9fa7a639b99ad955e51b7a7e944 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 18 Sep 2026 16:05:36 -0400 Subject: [PATCH 2/2] fix(plugin): report the clamp that actually bound, and warn on an unreachable floor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `clampedBy` computed `floored` BEFORE the ceiling clamp and tested the floor first, so a route whose `demandFloor` is SLOWER than its `renderInterval` reported `clampedBy: 'floor'` while the answer came from the ceiling. Measured at route 24h with floor 48h: a 6h rung resolves to 24h, and the field the docs say to read first named a clamp that did not produce it — beside a reported 48h floor and a 24h cadence, three numbers an operator cannot reconcile. That is the failure this block exists to prevent: a view that confidently explains a cadence the scheduler is not using. The ceiling is tested first now. The existing cross-check matrix could not have caught it — every case used floor 48h under a 96h route, so `floor > base` was never exercised, and the matrix only asserts `effectiveInterval`, which correctly delegates to the resolver. Also warns at compile time when `demandFloor > renderInterval`, since that combination can never take effect — the route interval is clamped last — and it is precisely the misconfiguration someone opens `explain` to diagnose. Warned rather than corrected: which of the two numbers the author meant is not knowable from here. Co-Authored-By: Claude Opus 5 (1M context) --- packages/plugin/src/util/routeClass.js | 22 +++++++++++++++++- packages/plugin/test/routeClass.test.js | 31 +++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/plugin/src/util/routeClass.js b/packages/plugin/src/util/routeClass.js index cfe333c..d71af35 100644 --- a/packages/plugin/src/util/routeClass.js +++ b/packages/plugin/src/util/routeClass.js @@ -157,6 +157,18 @@ const compileEntry = (raw, source, warn) => { } } + // A floor SLOWER than the route's own interval can never take effect — `resolveEffectiveInterval` + // clamps to the interval last — so it is always a mistake, and a silent one: `explain` then reports + // a `demandFloor` that is not the cadence and never will be. Warned, not corrected, because which + // of the two numbers the author meant is not knowable from here. + if (demandFloor !== null && renderInterval !== null && demandFloor > renderInterval) { + warn( + `demandFloor (${demandFloor}ms) on route "${raw.match} ${raw.path}" is SLOWER than its renderInterval ` + + `(${renderInterval}ms), so it can never apply — the route interval is the ceiling and is clamped last. ` + + `One of the two is wrong.` + ); + } + // Optional per-route sitemap-departure action. Same drop-the-FIELD rule as the three above: a // typo here must not change how the path is SERVED. Normalizing to `none` rather than leaving // the raw string is what lets every consumer read the field without re-validating it. @@ -578,7 +590,15 @@ export const explainCadence = (url, target = {}) => { defaultInterval: config.render.defaultInterval, demandInterval: hasRung ? rung : null, demandFloor: floor, - clampedBy: !hasRung ? null : floored > rung ? 'floor' : base < floored ? 'ceiling' : null, + // THE CEILING IS TESTED FIRST, and the order is the whole correctness of this field. `floored` + // is the value BEFORE `Math.min(..., base)`, so asking "did the floor raise the rung?" first + // reports `floor` even when the ceiling then overrode it — which is precisely the + // `demandFloor > renderInterval` misconfiguration an operator opens `explain` to diagnose. + // Measured against the resolver at route 24h / floor 48h: a 6h rung resolves to 24h, and the + // old order called that `floor` while reporting a 48h floor and a 24h cadence — three numbers + // that cannot be reconciled, with the field the docs say to read first pointing at a clamp that + // did not produce the answer. + clampedBy: !hasRung ? null : base < floored ? 'ceiling' : floored > rung ? 'floor' : null, }; }; diff --git a/packages/plugin/test/routeClass.test.js b/packages/plugin/test/routeClass.test.js index f5d375f..6b0df9d 100644 --- a/packages/plugin/test/routeClass.test.js +++ b/packages/plugin/test/routeClass.test.js @@ -14,6 +14,7 @@ import { PRERENDER, UNCLASSIFIED, explainCadence, + inspectRoutes, } from '../src/util/routeClass.js'; const ROUTES = [ @@ -472,3 +473,33 @@ test('explainCadence never disagrees with the resolver the scheduler actually us } } }); + +test('explainCadence: a floor SLOWER than the route reports the ceiling — the clamp that actually bound', () => { + // The misconfiguration `explain` exists to diagnose, and the one the old precedence got wrong: + // `floored` is computed BEFORE the ceiling clamp, so testing "did the floor raise the rung?" first + // reported `floor` while the answer came from the ceiling — a 48h floor, a 24h cadence and a + // `clampedBy` naming a clamp that did not produce it. Three numbers that cannot be reconciled. + forwarded({ + ingress: { + routes: [{ match: 'prefix', path: '/product/prd-', renderInterval: 24 * HOUR_MS, demandFloor: 48 * HOUR_MS }], + }, + }); + for (const rung of [6, 24, 48]) { + const c = explainCadence(PDP, { demandInterval: rung * HOUR_MS }); + assert.equal(c.effectiveInterval, 24 * HOUR_MS, `rung ${rung}h`); + assert.equal(c.clampedBy, 'ceiling', `rung ${rung}h must report the clamp that bound`); + assert.equal(c.demandFloor, 48 * HOUR_MS, 'the unreachable floor is still reported, not hidden'); + } +}); + +test('a demandFloor slower than the route interval is warned about at compile time', () => { + const warnings = []; + inspectRoutes( + [{ match: 'prefix', path: '/product/prd-', renderInterval: 24 * HOUR_MS, demandFloor: 48 * HOUR_MS }], + [] + ).warnings.forEach((w) => warnings.push(w)); + assert.ok( + warnings.some((w) => w.includes('demandFloor') && w.includes('can never apply')), + `expected a demandFloor>interval warning, got ${JSON.stringify(warnings)}` + ); +});