Skip to content

feat(plugin): probe only sitemap-listed targets (opt-in), and re-render a product that rejoins its sitemap; v0.89.0 - #209

Merged
harper-joseph merged 2 commits into
mainfrom
feat/probe-sitemap-scope
Sep 24, 2026
Merged

harper-joseph merged 2 commits into
mainfrom
feat/probe-sitemap-scope

Conversation

@harper-joseph

Copy link
Copy Markdown
Contributor

Stacked on #207 (fix/departure-uncapped, plugin 0.87.0). Retarget to main after it merges. Reserves plugin 0.89.0.

Why

The change probe walks every owned Target nightly and spends one origin request on each one a rule matches. On a large retail deployment about 25% of probed product targets are in no sitemap, and nearly all of those are products that left the sitemap. Where the sitemap is the availability feed, a product leaves it the day it sells out, and the departure check (departureAction: render) already hard-expires and re-renders its page. Probing that long tail every night buys little and costs one origin request per URL.

The reverse case goes unhandled today. When a product rejoins the sitemap (back in stock), its cached page still carries the out-of-stock snapshot until its next cadence render.

What changes. Everything is off by default.

Piece Option / field Default
Record when a target left its sitemap Target.unlistedAt: Date (new attribute) always recorded; rides existing writes
Probe scope changeProbe.scope: 'all' | 'listed' 'all'
changeProbe.unlistedGrace (ms) 2 * DAY (48h)
Rejoin action ingress.routes[].arrivalAction: 'none' | 'render' 'none'
sitemap.arrival.{enabled, dryRun, maxActions, maxCandidates} true, true, 5000, 50000 (-1 = uncapped, as in #207)

1. Target.unlistedAt

  • Stamp: the prune's unlink patch (sitemapUrl → null) now also sets unlistedAt = new Date(run.startedAt), the start of the walk that did the unlinking.
  • Clear: the REATTACH patch sets unlistedAt: null every time. Target.put (sitemap CREATE/revalidate RENDER, discovery, redirect adoption, the API) replaces the whole row, so it clears the stamp too.
  • Carry: Target.suppress does a whole-row TargetTable.put, so it now copies unlistedAt across next to sitemapUrl. Without that, a recently unlisted target would read as never listed once its verdict healed.
  • The other sitemapUrl writers (reactivate, and the strike and demand patches in RenderQueue) are patches, so they leave the stamp alone. Nothing else writes sitemapUrl.
  • No new writes. The stamp goes into patches the walk already makes, and the tests assert that the unlink is still a single patch.

2. changeProbe.scope

  • 'listed' probes a target only while it has a sitemapUrl, or for unlistedGrace after unlistedAt. These are skipped:
    • targets no sitemap ever listed (discovered)
    • targets unlinked longer ago than the grace
    • targets unlinked before 0.89.0 (they have no stamp)
  • Suppressed targets are still skipped under either scope, as today, and are not counted as out of scope.
  • The check sits in runProbePass after the rule match and before matched:
    • outOfScope counts only rows that would otherwise have cost a probe, i.e. the origin requests saved.
    • matched, which feeds both the canary cohort and the continuous/anchored slice size, describes what is actually probed.
  • The canary applies the same selection in three places: the sweep-built cohort, the bootstrap cohort (ensureCohorts), and every canary pass. A cohort member whose grace ran out after the cohort was built is skipped and counted.
  • Pass stats only, no new metric series:
    • outOfScope goes on the pass record and log line, and is always 0 under 'all'.
    • The console needs no change. Its metric-coverage guard passes (console suite: 277/277).
  • Changes to changeProbe.js are kept to row selection: TARGET_SELECT, one stat, one inScope param plus a check in the row loop, the three pass wirings, and one log warning. processOne is untouched.

3. arrivalAction (the rejoin)

  • The walk decides a rejoin at re-attach time. A REATTACH counts as a rejoin when the point-read row has no sitemapUrl and unlistedAt < run.startedAt, compared strictly.
  • It is acted on after the walk, through actOnWalkCandidates. That executor is the body processDepartures used to have, extracted rather than copied, so departures and arrivals share one path: re-read, decide, cap, dry-run, hard-expire past page.swrTtl, file the URL row at the current minute.
  • The route field is validated exactly like departureAction. A bad value drops the field with a warning, and a passthrough route refuses it. ArrivalAction reuses the DepartureAction strings so the one executor can compare against both.
  • Skip reasons: target-gone, unlinked (unlinked again before the walk ended), suppressed, route-opted-out. Actions: render, or would-render under dry run. Overflow is counted as capped.
  • Reported as arrivals: { considered, capped, outcomes } on the refresh result, the SitemapRefresh progress row, and an Arrival check: log line. Not as sitemap_arrival_* metrics, because a new sitemap_* family would trip the console's emit-site guard and should arrive with a panel to show it on. The departure emit line is byte-identical, so the guard's allow-list still matches.

How shear is handled

A URL that moves to a later child of a paginated index is pruned by the child it left and re-attached by the child that now claims it, inside one walk.

  • What tells the cases apart: the prune stamps the walk's start, not its own clock. A same-walk unlink therefore carries exactly run.startedAt. "Unlinked before this walk began" becomes an exact < comparison with no clock race inside the walk.
  • URL moves to a later child: stamped, then cleared, and never counted as a rejoin. The departure check also drops it as reattached, as before.
  • URL moves to an earlier child: re-attached before its old child is pruned, so it is never stamped.

A test walks each of these through Sitemap.refresh for real.

Dry run: a separate switch

sitemap.arrival.dryRun (default true) is separate from sitemap.departure.dryRun:

  • Why not inherit it: a deployment that has already armed departures and then adds arrivalAction: render would act on its very first walk.
  • What the dry run is for: measuring the rejoin burst. When a truncated child sitemap recovers, every URL it had dropped rejoins in the same walk. That count, and whether maxActions is a ceiling or a no-op, is what an operator needs to see before arming.

Can scope: 'listed' skip the whole corpus?

  • A failed walk cannot shrink the scope. It unlinks nothing:
    • a root that fails to fetch aborts before any prune
    • a failing child is skipped
    • an empty urlset is never reconciled
    • a URL whose route stops classifying as prerender is deferred, not unlinked
  • What can shrink it: a child that parses as valid XML but is truncated, and stays truncated for longer than unlistedGrace. This is the same event sitemap.departure.maxActions guards against, and it shows up first as a sitemap_removed spike and in the departure tally. The grace keeps those URLs probed for 48h while later walks settle whether they really left.
  • Guards added:
    1. A config warning (changeProbe.scope) when scope is 'listed' and departure is not armed. It names which of these is missing: a route departureAction, sitemap.departure.enabled, or dryRun off.
    2. A pass that skips more rule-matched targets than it probes logs a warning. Either the site is the wrong fit for this setting, or its sitemaps stopped listing what they used to.
  • Stated risk: anywhere the sitemap is not the availability feed, 'listed' simply stops watching unlisted pages, and their price or availability changes wait for the cadence render. The option docs say this is safe only with the sitemap as feed and departure armed.

Schema change and restart

  • unlistedAt: Date is added to Target. It is not indexed, so there is no reindex and no migration. Harper picks up the attribute when the component loads, so it ships with the normal release deploy and restart.
  • Existing rows have no stamp. When scope: 'listed' is first enabled, every currently unlisted target is out of scope straight away, including one that departed yesterday. If departure was armed, that product was already re-rendered when it left. Rejoins of those pre-upgrade departures are not detected either; they re-render on cadence, as before.
  • Mixed-version window: a 0.88 node neither stamps nor carries the attribute. The walk runs on the pinned sitemap node, so upgrade that node for stamps to begin. A Target.suppress on an old node during the rollout can drop a stamp.

Known limitations (documented in the option text)

  • A URL that moves between two root sitemaps walked one after the other looks like a departure in the first root's walk and a rejoin in the second's. The departure check already has the same limitation. The cost is one render per such URL, inside the ceilings. Two overlapping walks of the same root (the refresh claim is advisory) can produce the same false rejoin.
  • A revalidate: true walk detects no rejoins. It files every listed URL due now anyway, and its put clears the stamp.
  • A capped arrival is lost for good, the same as a capped departure: the re-attach has already cleared the stamp.
  • Switching to 'listed' in continuous or anchored mode paces the first cycle against the old, larger slice, so that cycle finishes early.

Tests

  • Plugin: node --test 1270/1270 (1224 before this change, 46 new).
  • Console: 277/277.
  • Lint and format: npm run lint && npm run format:check clean.

New test files:

  • test/sitemapArrival.test.js: route-field validation (accept, drop with warning, passthrough refusal, master switch, independent of departure); the isRejoin table, including equal-to-walk-start = shear, a still-attributed target, BigInt, ISO and epoch; the decideArrival table; the candidate cap (-1 = Infinity, 0 = off); the run tally; the defaults.
  • test/sitemapArrivalWalk.test.js: real Sitemap.refresh walks against in-memory tables and a faked origin:
    • the stamp is written in the one unlink patch and cleared by the re-attach patch
    • shear inside one walk is stamped, then cleared, and not a rejoin; a move to an earlier child is never stamped
    • a genuine rejoin in dry run, and armed (pages past the swr window, one URL-keyed schedule row, fromSitemap: true)
    • route opt-out; suppressed; a discovered target listed for the first time
    • caps (maxActions 1 and 0, maxCandidates, -1); the arrival dry run separate from departure's
    • a revalidate walk
    • defaults collect and file nothing
    • Target.suppress carries the stamp
  • test/probeScope.test.js:
    • the predicate: inside and outside the grace, the boundary, grace 0, BigInt, a future stamp, the clock read per row
    • runProbePass selection (listed, recently unlisted, stale, never listed, suppressed, unowned, no rule)
    • the real sweep and canary against a loopback HTTP endpoint, checking what the origin was actually asked: scoped sweep, default sweep, sweep-built cohort, bootstrap cohort, a member aging out between canary passes, default cohort unchanged
    • the warning log line, the config warning (3 causes plus 2 no-warning cases), and enum/min validation

Revert checks

Each mutation was applied to the implementation, the relevant test files were run, and the implementation was restored. All 19 caught:

Mutation Failing tests
scope check removed from runProbePass 5
sweep not wired to scope 3
bootstrap-cohort filter removed 2
canary pass not re-checked 1
grace < → <= 1
prune stamp removed 9
prune stamps its own clock instead of walk start 1
re-attach clear removed 5
isRejoin < → <= (shear leaks) 2
isRejoin ignores attribution 1
suppress drops the stamp 1
processArrivals not called 6
arrival maxActions read raw (no -1) 1
arrival inherits departure dryRun 2
arrivalAction accepts any value 1
passthrough refusal removed 1
decideArrival suppressed check removed 2
config warning removed 1
out-of-scope warning removed 1

Docs

  • configSchema.js (the config source of truth): changeProbe.scope and unlistedGrace, including the safety note; the sitemap.arrival group; the ingress.routes shape line and an arrivalAction paragraph.
  • schema.graphql: comment on unlistedAt.
  • METRICS.md §4b: the two new log lines, plus outOfScope and arrivals as API fields. No metric series changed.
  • README: a scope paragraph in the change-probe section.

🤖 Generated with Claude Code

…er a product that rejoins its sitemap; v0.89.0

Three pieces, all off by default.

Target.unlistedAt. A sitemap walk that unlinks a target (sitemapUrl ->
null) now stamps it with THE START OF THAT WALK, in the same patch; the
re-attach clears it in its own patch, and Target.suppress carries it
through its whole-row put. No new writes. Stamping with the walk start
rather than the prune's clock is what makes shear exact: a URL that
moves to a later child of a paginated index is unlinked and re-attached
inside one walk, so its stamp equals this walk's start and is not a
rejoin; only a stamp from an earlier walk is.

changeProbe.scope ('all' | 'listed', default 'all') and
changeProbe.unlistedGrace (default 48h). Under 'listed' the sweep and the
canary probe a target only while a sitemap lists it or for the grace
after a walk unlinked it; never-listed (discovered) targets and long-gone
products are skipped and counted as outOfScope in the pass stats. The
check sits after the rule match and before `matched`, so the canary
cohort and the continuous-mode slice size describe what is probed; the
bootstrap cohort and every canary pass apply the same selection. A pass
that skips more than it probes logs a warning, and scope 'listed'
without an armed departure is a config warning.

ingress.routes[].arrivalAction ('none' | 'render', default 'none') with
a new sitemap.arrival group (enabled, dryRun default true, maxActions,
maxCandidates; -1 = uncapped, as in #207). A rejoin is decided at
re-attach time and acted on after the walk through the executor the
departure check now shares with it (hard-expire past page.swrTtl, file
the URL row at the current minute). Its own dryRun so arming departures
never arms arrivals unmeasured. Tallied as `arrivals` on the result and
progress row, not as metric series.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a sitemap arrival check feature that detects when a URL rejoins a sitemap after being unlinked in an earlier walk, allowing cached pages to be hard-expired and re-rendered immediately. It adds a new unlistedAt timestamp to targets to distinguish genuine rejoins from same-walk shear, and introduces a changeProbe.scope configuration to limit probing to sitemap-listed targets. The reviewer recommends validating and rejecting invalid configurations at the entry point where they are parsed or applied, rather than implementing defensive guards or fallbacks in downstream utility functions like collectConfigWarnings.

Comment on lines +790 to +813
if (target.changeProbe.scope === 'listed') {
const { departure } = target.sitemap;
const routes = Array.isArray(target.ingress.routes) ? target.ingress.routes : [];
const routeDeparts = routes.some(
(route) =>
route &&
route.mode !== 'passthrough' &&
(route.departureAction === DepartureAction.RENDER || route.departureAction === DepartureAction.EXPIRE)
);
if (!departure.enabled || departure.dryRun || !routeDeparts) {
add(
'warn',
'changeProbe.scope',
"changeProbe.scope is 'listed' but sitemap departure is not armed (" +
(!routeDeparts
? 'no prerender route sets departureAction'
: !departure.enabled
? 'sitemap.departure.enabled is false'
: 'sitemap.departure.dryRun is true') +
') — a product that leaves its sitemap stops being probed after changeProbe.unlistedGrace and ' +
'nothing re-renders it on the way out, so its page keeps its last snapshot until its cadence render'
);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of adding defensive guards (such as optional chaining or fallback objects) inside downstream utility functions like collectConfigWarnings to handle partial or invalid configurations, we should validate and reject invalid configuration at the entry point where it is parsed or applied. This ensures consistency, guarantees existence, and avoids quietly ignoring unsupported inputs.

References
  1. Validate and reject invalid configuration at the entry point (where it is parsed or applied) instead of implementing defensive guards or fallbacks in downstream utility functions to ensure consistency and guarantee existence.
  2. Avoid adding defensive guards (such as optional chaining or null checks) that quietly accept or ignore unsupported inputs. If an input is not supported by the function's contract, let it fail loudly (e.g., by throwing a TypeError) to make type bugs visible, rather than quietly failing open or ignoring the error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping these guards. collectConfigWarnings also checks PROSPECTIVE configs (the config-override dry run), and this branch reads the RAW ingress.routes list, not the compiled one: route entries are validated and dropped individually by routeClass.js, so a malformed entry can legitimately reach here, and throwing would break the config preview rather than report the problem. It is the same shape as the invalidateScope check a few lines below (Array.isArray(...rules) / rule?.invalidateScope).

@harper-joseph

Copy link
Copy Markdown
Contributor Author

Self-review (author side) — no defects found; no changes.

Checked: unlistedAt is stamped with the walk's own start and a rejoin requires a strictly older stamp AND no attribution at re-attach, so same-walk shear and moves between children can't read as rejoins; the re-attach patch clears the stamp unconditionally; Target.suppress carries it across its whole-row put; the post-walk re-read drops targets unlinked again, suppressed or gone; scope: 'all' (default) adds no per-row work; scope is applied after the rule match and before matched, so outOfScope is the origin requests saved and the canary cohort matches the sweep. Will need a rebase over #210 (both touch changeProbe.js row handling) — whichever merges second.

# Conflicts:
#	package-lock.json
#	packages/plugin/METRICS.md
#	packages/plugin/package.json
@harper-joseph
harper-joseph merged commit 986c468 into main Sep 24, 2026
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