Repository navigation
feat(plugin): probe only sitemap-listed targets (opt-in), and re-render a product that rejoins its sitemap; v0.89.0 - #209
Conversation
…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>
There was a problem hiding this comment.
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.
| 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' | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
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
- 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.
- 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.
There was a problem hiding this comment.
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).
|
Self-review (author side) — no defects found; no changes. Checked: |
# Conflicts: # package-lock.json # packages/plugin/METRICS.md # packages/plugin/package.json
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.
Target.unlistedAt: Date(new attribute)changeProbe.scope: 'all' | 'listed''all'changeProbe.unlistedGrace(ms)2 * DAY(48h)ingress.routes[].arrivalAction: 'none' | 'render''none'sitemap.arrival.{enabled, dryRun, maxActions, maxCandidates}true, true, 5000, 50000(-1= uncapped, as in #207)1.
Target.unlistedAtsitemapUrl → null) now also setsunlistedAt = new Date(run.startedAt), the start of the walk that did the unlinking.unlistedAt: nullevery time.Target.put(sitemap CREATE/revalidateRENDER, discovery, redirect adoption, the API) replaces the whole row, so it clears the stamp too.Target.suppressdoes a whole-rowTargetTable.put, so it now copiesunlistedAtacross next tositemapUrl. Without that, a recently unlisted target would read as never listed once its verdict healed.sitemapUrlwriters (reactivate, and the strike and demand patches in RenderQueue) are patches, so they leave the stamp alone. Nothing else writessitemapUrl.2.
changeProbe.scope'listed'probes a target only while it has asitemapUrl, or forunlistedGraceafterunlistedAt. These are skipped:runProbePassafter the rule match and beforematched:outOfScopecounts 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.ensureCohorts), and every canary pass. A cohort member whose grace ran out after the cohort was built is skipped and counted.outOfScopegoes on the pass record and log line, and is always 0 under'all'.changeProbe.jsare kept to row selection:TARGET_SELECT, one stat, oneinScopeparam plus a check in the row loop, the three pass wirings, and one log warning.processOneis untouched.3.
arrivalAction(the rejoin)sitemapUrlandunlistedAt < run.startedAt, compared strictly.actOnWalkCandidates. That executor is the bodyprocessDeparturesused to have, extracted rather than copied, so departures and arrivals share one path: re-read, decide, cap, dry-run, hard-expire pastpage.swrTtl, file the URL row at the current minute.departureAction. A bad value drops the field with a warning, and a passthrough route refuses it.ArrivalActionreuses theDepartureActionstrings so the one executor can compare against both.target-gone,unlinked(unlinked again before the walk ended),suppressed,route-opted-out. Actions:render, orwould-renderunder dry run. Overflow is counted ascapped.arrivals: { considered, capped, outcomes }on the refresh result, theSitemapRefreshprogress row, and anArrival check:log line. Not assitemap_arrival_*metrics, because a newsitemap_*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.
run.startedAt. "Unlinked before this walk began" becomes an exact<comparison with no clock race inside the walk.reattached, as before.A test walks each of these through
Sitemap.refreshfor real.Dry run: a separate switch
sitemap.arrival.dryRun(defaulttrue) is separate fromsitemap.departure.dryRun:arrivalAction: renderwould act on its very first walk.maxActionsis a ceiling or a no-op, is what an operator needs to see before arming.Can
scope: 'listed'skip the whole corpus?unlistedGrace. This is the same eventsitemap.departure.maxActionsguards against, and it shows up first as asitemap_removedspike and in the departure tally. The grace keeps those URLs probed for 48h while later walks settle whether they really left.changeProbe.scope) whenscopeis'listed'and departure is not armed. It names which of these is missing: a routedepartureAction,sitemap.departure.enabled, ordryRunoff.'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: Dateis added toTarget. 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.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.Target.suppresson an old node during the rollout can drop a stamp.Known limitations (documented in the option text)
revalidate: truewalk detects no rejoins. It files every listed URL due now anyway, and itsputclears the stamp.'listed'in continuous or anchored mode paces the first cycle against the old, larger slice, so that cycle finishes early.Tests
node --test1270/1270 (1224 before this change, 46 new).npm run lint && npm run format:checkclean.New test files:
test/sitemapArrival.test.js: route-field validation (accept, drop with warning, passthrough refusal, master switch, independent of departure); theisRejointable, including equal-to-walk-start = shear, a still-attributed target, BigInt, ISO and epoch; thedecideArrivaltable; the candidate cap (-1= Infinity, 0 = off); the run tally; the defaults.test/sitemapArrivalWalk.test.js: realSitemap.refreshwalks against in-memory tables and a faked origin:fromSitemap: true)maxActions1 and 0,maxCandidates,-1); the arrival dry run separate from departure'srevalidatewalkTarget.suppresscarries the stamptest/probeScope.test.js:runProbePassselection (listed, recently unlisted, stale, never listed, suppressed, unowned, no rule)Revert checks
Each mutation was applied to the implementation, the relevant test files were run, and the implementation was restored. All 19 caught:
runProbePass<→<=isRejoin<→<=(shear leaks)isRejoinignores attributionprocessArrivalsnot calledmaxActionsread raw (no-1)dryRunarrivalActionaccepts any valuedecideArrivalsuppressed check removedDocs
configSchema.js(the config source of truth):changeProbe.scopeandunlistedGrace, including the safety note; thesitemap.arrivalgroup; theingress.routesshape line and anarrivalActionparagraph.schema.graphql: comment onunlistedAt.METRICS.md§4b: the two new log lines, plusoutOfScopeandarrivalsas API fields. No metric series changed.README: a scope paragraph in the change-probe section.🤖 Generated with Claude Code