Align revalidate seconds with cache behaviour#17
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
maquchizi
force-pushed
the
align-revalidate-seconds-with-cache-behaviour
branch
from
July 15, 2026 16:42
779e1d7 to
ba18e07
Compare
maquchizi
force-pushed
the
align-revalidate-seconds-with-cache-behaviour
branch
from
July 15, 2026 16:44
ba18e07 to
f4b42c2
Compare
maquchizi
force-pushed
the
align-revalidate-seconds-with-cache-behaviour
branch
from
July 16, 2026 08:10
1b21b4c to
f4b42c2
Compare
* It is now explicit that it only controls the metrics cache
* Next, we'll rename it so that future contributors do not run into
the same confusion
* This drops the Next.js "revalidate" terminology and makes it clearer
that this is its own thing unrelated to Next.js revalidation
* Extract the calculation into a function to avoid drift
maquchizi
force-pushed
the
align-revalidate-seconds-with-cache-behaviour
branch
from
July 16, 2026 08:12
f4b42c2 to
9aa273a
Compare
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9aa273a6e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kilemensi
approved these changes
Jul 17, 2026
kilemensi
left a comment
Member
There was a problem hiding this comment.
Mostly looks good to me ... Only thing I can think of is a metricsCacheSeconds < revalidate is pointless because pages will still show old data until revalidate expires.
METRICS_CACHE_SECONDS was documented as *the* bound on data freshness for all routes. That is wrong for the overview page: `/` is cached by a fixed `revalidate = 60` ISR literal and only regenerates on that interval, so its effective freshness is max(revalidate, METRICS_CACHE_SECONDS). Lowering the metrics cache below 60s buys no extra freshness on `/` — it only speeds up the on-demand detail route.
Collaborator
Author
|
@kilemensi I've captured that more clearly in the docs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
REVALIDATE_SECONDSwas documented as controlling both the server-side Prometheus/data cache and the ISR page-revalidation window, but the two layers never actually agreed:/) hardcodedexport const revalidate = 60, ignoring the configured value./site/[id]) also exportedrevalidate = 60, but readssearchParams, so it's server-rendered on demand — the export was dead.unstable_cache+ Prometheusfetch) honored the configured value.Next.js 16 requires a route's
revalidateto be a statically-analyzable literal, so an env var cannot drive route ISR — meaning the "both layers" contract was impossible to satisfy. On top of that, the overview'supdatedtimestamp was stamped at render time, so it could advance while showing older cached metrics.This PR gives the setting one accurate meaning and makes the timestamp honest:
REVALIDATE_SECONDS→METRICS_CACHE_SECONDS(andconfig.revalidate→config.metricsCacheSeconds). The old name echoed Next's route-segmentrevalidate— exactly the layer it does not control. No back-compat fallback (codebase is still young; a clean break was chosen deliberately).METRICS_CACHE_SECONDSnow means only the data-cache window — how often Grafana is queried. Route rendering is documented as separate:/keeps a fixedrevalidate = 60literal;/site/[id]drops its dead export and is dynamic.updatedvalue now reflects real data-fetch time.getOverview()returns a newOverviewData({ checks, fetchedAt }), withfetchedAtcaptured inside the cached function so it only advances on a cache miss.README.md,docs/configuration.md,docs/architecture.md,.env.example,AGENTS.md) updated to match.Closes #11
Type of change
Checklist
[AGENTS.md](https://claude.ai/AGENTS.md)and[CONTRIBUTING.md](https://claude.ai/CONTRIBUTING.md)pnpm checkpasses — ranpnpm lint:ci(the check-only variant) which passes; did not run the autofixingpnpm checkpnpm testpasses — 103 passingpnpm test:typespassespnpm buildsucceeds — please run in CI; a fullpnpm buildwas verified against an earlier state of the branch but not re-run after the final rename (types + tests + lint pass on the final state)lib/synthetics.test.tsandlib/prometheus.test.tsupdated for the new return shape and config field; assertsfetchedAtis presentdocs/) if behaviour or configuration changedNEXT_PUBLIC_prefixNotes for reviewers
REVALIDATE_SECONDSis removed, not aliased. Any deployment still setting it will silently fall back to the60s default until it's renamed toMETRICS_CACHE_SECONDS. Intentional given the project's age — call out if you'd prefer a deprecation window.METRICS_CACHE_SECONDS=300, the build still reports/atRevalidate 1m. That's now correct and documented — the route re-renders at most every 60s but reads cached data, andupdatedshows the true fetch time (up to 300s old) rather than lying.fetchedAt; the detail page has no "updated" label, sogetSiteHistoryintentionally doesn't carry it.updatedtimestamp — no screenshots needed.Acceptance criteria
REVALIDATE_SECONDShas one accurate, consistently documented meaning.updatedvalue represents metric freshness rather than only render time.README.md,.env.example,docs/configuration.md, anddocs/architecture.mdmatch production behavior.