Skip to content

Align revalidate seconds with cache behaviour#17

Merged
maquchizi merged 5 commits into
mainfrom
align-revalidate-seconds-with-cache-behaviour
Jul 17, 2026
Merged

Align revalidate seconds with cache behaviour#17
maquchizi merged 5 commits into
mainfrom
align-revalidate-seconds-with-cache-behaviour

Conversation

@maquchizi

@maquchizi maquchizi commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

What & why

REVALIDATE_SECONDS was documented as controlling both the server-side Prometheus/data cache and the ISR page-revalidation window, but the two layers never actually agreed:

  • The overview route (/) hardcoded export const revalidate = 60, ignoring the configured value.
  • The detail route (/site/[id]) also exported revalidate = 60, but reads searchParams, so it's server-rendered on demand — the export was dead.
  • Only the data layer (unstable_cache + Prometheus fetch) honored the configured value.

Next.js 16 requires a route's revalidate to 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's updated timestamp 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:

  • Renamed REVALIDATE_SECONDSMETRICS_CACHE_SECONDS (and config.revalidateconfig.metricsCacheSeconds). The old name echoed Next's route-segment revalidate — exactly the layer it does not control. No back-compat fallback (codebase is still young; a clean break was chosen deliberately).
  • METRICS_CACHE_SECONDS now means only the data-cache window — how often Grafana is queried. Route rendering is documented as separate: / keeps a fixed revalidate = 60 literal; /site/[id] drops its dead export and is dynamic.
  • The overview updated value now reflects real data-fetch time. getOverview() returns a new OverviewData ({ checks, fetchedAt }), with fetchedAt captured inside the cached function so it only advances on a cache miss.
  • Docs (README.md, docs/configuration.md, docs/architecture.md, .env.example, AGENTS.md) updated to match.

Closes #11

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Refactor / chore

Checklist

  • I read [AGENTS.md](https://claude.ai/AGENTS.md) and [CONTRIBUTING.md](https://claude.ai/CONTRIBUTING.md)
  • pnpm check passes — ran pnpm lint:ci (the check-only variant) which passes; did not run the autofixing pnpm check
  • pnpm test passes — 103 passing
  • pnpm test:types passes
  • pnpm build succeeds — please run in CI; a full pnpm build was verified against an earlier state of the branch but not re-run after the final rename (types + tests + lint pass on the final state)
  • Added/updated tests for new logic — lib/synthetics.test.ts and lib/prometheus.test.ts updated for the new return shape and config field; asserts fetchedAt is present
  • Updated docs (README / docs/) if behaviour or configuration changed
  • No secrets committed and no Grafana credential given a NEXT_PUBLIC_ prefix

Notes for reviewers

  • Breaking config change: REVALIDATE_SECONDS is removed, not aliased. Any deployment still setting it will silently fall back to the 60s default until it's renamed to METRICS_CACHE_SECONDS. Intentional given the project's age — call out if you'd prefer a deprecation window.
  • Behavior to sanity-check: with METRICS_CACHE_SECONDS=300, the build still reports / at Revalidate 1m. That's now correct and documented — the route re-renders at most every 60s but reads cached data, and updated shows the true fetch time (up to 300s old) rather than lying.
  • Scoping decision: only the overview surfaces fetchedAt; the detail page has no "updated" label, so getSiteHistory intentionally doesn't carry it.
  • No UI changes beyond the source of the updated timestamp — no screenshots needed.

Acceptance criteria

  • The intended route-cache and data-cache behavior is explicitly decided.
  • REVALIDATE_SECONDS has one accurate, consistently documented meaning.
  • The overview's updated value represents metric freshness rather than only render time.
  • README.md, .env.example, docs/configuration.md, and docs/architecture.md match production behavior.
  • Tests or build-time assertions cover the chosen cache behavior where practical.

@vercel

vercel Bot commented Jul 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
probeview Ready Ready Preview, Comment Jul 17, 2026 10:08am

Request Review

@maquchizi
maquchizi force-pushed the align-revalidate-seconds-with-cache-behaviour branch from 779e1d7 to ba18e07 Compare July 15, 2026 16:42
@maquchizi
maquchizi force-pushed the align-revalidate-seconds-with-cache-behaviour branch from ba18e07 to f4b42c2 Compare July 15, 2026 16:44
@maquchizi
maquchizi requested a review from a team July 16, 2026 07:11
@maquchizi
maquchizi force-pushed the align-revalidate-seconds-with-cache-behaviour branch from 1b21b4c to f4b42c2 Compare July 16, 2026 08:10
  * 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
maquchizi force-pushed the align-revalidate-seconds-with-cache-behaviour branch from f4b42c2 to 9aa273a Compare July 16, 2026 08:12
@kilemensi

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 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".

Comment thread docs/configuration.md Outdated

@kilemensi kilemensi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.
@maquchizi

Copy link
Copy Markdown
Collaborator Author

@kilemensi I've captured that more clearly in the docs.

@maquchizi
maquchizi merged commit 926df25 into main Jul 17, 2026
5 checks passed
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.

[Bug] Align REVALIDATE_SECONDS with actual cache behavior

2 participants