Skip to content

feat(ab-testing): Flags SDK precompute infrastructure#18807

Open
pettinarip wants to merge 3 commits into
devfrom
feat/ab-testing-flags-sdk
Open

feat(ab-testing): Flags SDK precompute infrastructure#18807
pettinarip wants to merge 3 commits into
devfrom
feat/ab-testing-flags-sdk

Conversation

@pettinarip

@pettinarip pettinarip commented Jul 15, 2026

Copy link
Copy Markdown
Member

Summary

Replaces the legacy A/B testing infrastructure with the Flags SDK precompute pattern. This PR merges inertabTestRoutes is empty, so the proxy no-ops until an experiment is registered. A companion PR stacked on this branch demos two working experiments on its deploy preview.

Why: the legacy ABTestWrapper read headers() at render time, forcing any A/B-tested page into dynamic rendering — running a test on the homepage meant losing edge caching on our highest-traffic page (#17294 → reverted in #17742). With this setup, variants are assigned in the proxy and served as statically prerendered, edge-cached pages. Verified on the #18777 spike preview: variant responses are Netlify Edge: hit.

How it works

  1. proxy.ts matches the path against abTestRoutes, fingerprints request headers (cookie-less, GDPR-compliant), evaluates the route's flags against Matomo experiment config, and packs the result into an HMAC-signed code
  2. It rewrites to /en/ab-code/<code>/<path> — internal-only; the visitor's URL never changes
  3. Each tested route has a thin coded page that prerenders one static page per flag permutation (generateStaticParams)
  4. ABTest renders the variant; ABTestTracker pushes AbTesting::enter + custom dimension 1 to Matomo

Experiments stay Matomo-driven: status, traffic weights, and scheduling are dashboard changes, no deploys. On any failure (Matomo down, no experiment, tampered code) everything falls through to the original page.

Scope & safety

  • Default locale only — route keys are locale-less canonical paths; non-EN visitors get the original page with no tracker (verified: no rewrite header on /es/, /de/)
  • Internal URLs unreachable — direct /ab-code/… visits get case-mangled by the sitewide lowercase 301, fail signature verification, and 404; coded pages re-export the original page's canonical metadata as a second layer
  • Signed codes are dot-encoded (.~): dotted URL segments + trailingSlash: true trigger an origin 308 that leaks the internal URL — this was the root cause that killed feat(ab-testing): add Vercel Flags SDK integration with Matomo #17265, finally pinned down and fixed
  • Bots: fingerprint-assigned like any visitor (stable UA/IP → consistent variant); Matomo filters known bots from reports. No proxy-level bot exclusion — documented as the accepted behavior

Changes

  • src/lib/ab-testing/: defineABFlag helper, identify fingerprinting (client-IP-only x-forwarded-for, fix(ab-test): extract client IP from x-forwarded-for header #17612), Matomo adapter (FNV-1a weighted assignment, mock experiments for local dev), encodeABCode/decodeABCode
  • src/components/AB/: new ABTest, debug panel now drives proxy re-evaluation via flag_override_* cookies (dev/preview only), tracker hardened
  • Removed legacy infra: ABTestWrapper, ClientABTestWrapper, src/lib/ab-testing/server.ts, app/api/ab-config/ (unused since revert: remove homepage A/B test setup #17742)
  • Rewrote docs/ab-testing.md (architecture + step-by-step recipe); updated CLAUDE.md/AGENTS.md
  • .env.example: FLAGS_SECRET, USE_MOCK_EXPERIMENTS (both already configured on Netlify)

Test plan

  • Type-check, lint, unit tests pass
  • Full flow verified on the spike(ab-testing): revive Flags SDK precompute approach (#17265 v2) #18777 spike preview: silent rewrites (200, URL unchanged), edge cache hits on variant pages, override cookies, invalid codes 404, non-EN locales and content routes untouched
  • Examples PR (stacked on this branch) demos two live experiments on its deploy preview

History

Supersedes #17265 (closed: blocked by what turned out to be the dot/trailingSlash 308) and the #18777 spike, where this implementation was developed and verified. #17581 (remove trailingSlash) is not needed.

🤖 Generated with Claude Code

Replaces the legacy ABTestWrapper system (which read headers() at render
time, forcing any A/B-tested page into dynamic rendering) with the Flags
SDK precompute pattern: the proxy assigns variants at the edge via header
fingerprinting and rewrites to signed, statically prerendered variant
pages. Experiments stay Matomo-driven (status, weights, scheduling - no
deploys) and cookie-less.

Merges inert: abTestRoutes is empty, so the proxy no-ops until a route
is registered. See docs/ab-testing.md for the full architecture and the
recipe for adding an experiment.

- src/lib/ab-testing: defineABFlag helper, header fingerprinting with
  client-IP-only x-forwarded-for (#17612), Matomo adapter with FNV-1a
  weighted assignment, dot-encoding for signed codes (dotted URL
  segments + trailingSlash: true trigger an origin 308 that leaks the
  internal URL - the #17265 failure mode)
- ABTest component + cookie-based debug panel (dev/preview) + tracker
  with custom dimension 1 for segment queries
- Removed legacy infra: ABTestWrapper, ClientABTestWrapper, server.ts,
  /api/ab-config
- X-Robots-Tag noindex on /ab-code/* as defense in depth
- Rewrote docs/ab-testing.md; updated CLAUDE.md / AGENTS.md

Verified end-to-end on the deploy preview of #18777 (spike): silent
rewrites, edge cache hits on variant pages, invalid codes 404, non-EN
locales untouched.
@pettinarip pettinarip requested a review from wackerow as a code owner July 15, 2026 13:58
@netlify

netlify Bot commented Jul 15, 2026

Copy link
Copy Markdown

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit 3c96149
🔍 Latest deploy log https://app.netlify.com/projects/ethereumorg/deploys/6a57a59444a3ea00088c4b87
😎 Deploy Preview https://deploy-preview-18807.ethereum.it
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 59 (🟢 up 3 from production)
Accessibility: 95 (no change from production)
Best Practices: 100 (no change from production)
SEO: 98 (no change from production)
PWA: 59 (🔴 down 1 from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions Bot added config ⚙️ Changes to configuration files dependencies 📦 Changes related to project dependencies documentation 📖 Change or add documentation tooling 🔧 Changes related to tooling of the project labels Jul 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔎 First-pass review — 🔧 Needs work

Solid rework — moves A/B assignment to the edge/precompute path so tested pages stay statically prerendered, and it merges inert (abTestRoutes is empty, proxy no-ops). Conventions check out on the code side: the only unused arg is a bare _ in flags.ts, no raw Intl.*, no chain-name touch, tracker/opt-out parsing is hardened. One thing to confirm before merge:

  • package.json version goes backwards: 11.14.011.13.0. Unrelated to the A/B scope and looks like a merge/rebase artifact — please restore 11.14.0 (or confirm the downgrade is intentional).
Analysis

Lane: code (proxy.ts, src/lib/ab-testing/, src/components/AB/) + docs (docs/ab-testing.md, AGENTS.md), deps (flags@^4.2.0), config (.env.example, netlify.toml).

Checked: unused vars (only _ in Array.from callback — allowed), Intl.* wrappers (none used), ChainName (n/a), translatability (variant content is caller-supplied; docs correctly call out getTranslations). Rewrite logic is exact-path + default-locale-only as documented; empty route map means zero runtime effect this PR.

Non-blocking questions/notes:

  • matomo-adapter.ts fetch switched from next: { revalidate: 3600 } to cache: "force-cache" with no revalidation. In the middleware/precompute path this is likely fine, but worth confirming experiment weight/status changes still propagate without a redeploy — the docs promise dashboard-driven updates with "no deploys."
  • TestDebugPanel.setCookie uses Secure — won't be stored on plain-http localhost in stricter browsers (most modern browsers do allow it on localhost, so probably fine; flagging since the docs point devs at the panel under pnpm dev).
  • defineABFlag JSDoc documents @param key/@param numVariants but not @param description.

CI still running — not commenting on pending checks. Routing to an engineer for the version bump call.

Generated by PR Reviewer (team) for #18807 · 90 AIC · ⌖ 28.2 AIC · ⊞ 5.2K ·

@pettinarip

pettinarip commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

Addressed in 9e01d52:

  • Version regression restored to 11.14.0 — confirmed root cause: package.json was checked out wholesale from the spike branch (cut from pre-11.14.0 dev) when assembling this branch. Also audited pnpm-lock.yaml for the same staleness: its diff vs dev is only the flags@4.2.0 package and its @edge-runtime/cookies dependency — no drift reverted.
  • defineABFlag JSDoc completed (@param description).
  • ⏭️ force-cache kept as-is. In the proxy's edge runtime (the only place this fetch runs) Next's data cache doesn't operate, so the hint is best-effort and config propagates in practice; keeping it avoids implying cache semantics the runtime doesn't provide.
  • ⏭️ Secure cookie kept as-is — dev-tooling nit, acceptable (Chrome/Firefox accept Secure on http localhost; Safari devs can use the cookie manually if needed).

Fixes are also propagated to the stacked examples branch (#18808).

- Restore version 11.14.0 (stale package.json from the spike branch)
- Complete defineABFlag JSDoc
@pettinarip pettinarip force-pushed the feat/ab-testing-flags-sdk branch from 8cf047f to 9e01d52 Compare July 15, 2026 14:42
Deploy previews send a blanket X-Robots-Tag: noindex, so the rule can't
be verified pre-production - and if the CDN matched it against the
middleware-rewrite target it would noindex the public A/B-tested pages.
The rule was redundant defense anyway: direct /ab-code visits already
404 via signature verification and coded pages declare the public URL
as canonical.
@pettinarip

Copy link
Copy Markdown
Member Author

Dropped the X-Robots-Tag: noindex rules for /ab-code/* (3c96149). While verifying the demo preview I noticed Netlify sends a blanket noindex on all deploy previews, which means the rule's production behavior can't be verified before merge — and in the worst case (CDN matching header rules against the middleware-rewrite target) it would noindex the public pages under test. It was redundant defense anyway: direct /ab-code/… visits already 404 via signature verification, and coded pages declare the public URL as canonical. Documented the pitfall in docs/ab-testing.md so it doesn't get re-added.

@myelinated-wackerow myelinated-wackerow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@pettinarip looks clean! A few things before the companion PR turns on a real experiment:

1. Matomo config is fetched live in the proxy, uncached and untimed. fetchMatomoExperiments() runs inside decide()precompute() → the proxy (edge middleware), on every request to a registered route (it's what selects the cached variant page, so it can't be cached itself). Two problems:

  • cache: "force-cache" is a no-op in middleware — the Next Data Cache isn't available there, so there's no cross-request reuse. Every hit to a tested route (incl. the homepage) adds a blocking Matomo round-trip on the edge critical path. The page render is cached; the variant lookup isn't.
  • No timeout on the fetch. A refusal is handled, but a slow/hanging Matomo would stall await precompute up to the function timeout on every request to that route.

Can we add a module-level TTL cache (persists within a warm edge instance), an AbortSignal.timeout(...), and dedupe the fetch so multi-flag routes don't multiply calls? Did the #18777 spike give any read on the added edge latency per request?

2. isExperimentActive counts "created" as active. An experiment created but not launched (no start_date) will bucket and track users before it's started. Intended, or should it be "running" only?

Minor:

  • assignedAt: Date.now() is never read by the tracker, and in the static coded page it bakes in build time anyway — probably drop it from ABTestAssignment.
  • Debug override value isn't bounded by numVariants, but it's dev/preview-only and ABTest clamps at render — just noting.

Approving once #1 is handled here or explicitly slated as the first task in the companion PR, with a word on #2.

- Opus 4.8 / @wackerow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config ⚙️ Changes to configuration files dependencies 📦 Changes related to project dependencies documentation 📖 Change or add documentation tooling 🔧 Changes related to tooling of the project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants