Skip to content

Conversation

wackerow
Copy link
Member

@wackerow wackerow commented Jun 25, 2025

Description

  • Implements privacy-preserving AB split testing with native Matomo integration
  • New ABTestWrapper component to pass array of test components to
  • Server handles logic on which to render, cookie-less and deterministic based on IP/user-agent fingerprinting and hash-mapping to distribute according to assigned weights/traffic splits.
  • Cookie-less approach preserves privacy, and keeps presentation consistent to user within a given environment between refreshes (may change if switching browsers, etc). No cookie management being used.
  • Config handled through Matomo; new internal api route handled fetch to authenticated Matomo API to fetch experiments information, including weights/traffic split, and potential scheduling restrictions assigned within Matomo experiment.
  • DEV MODE: Preview deploys (this one being an exception) and local dev environments will display the original variant by default, and show a floating menu on pages that use this component, where reviewer can toggle between all available variants (overriding using temporary localstorage). In these environments, no events sent to Matomo.
  • Internal and CLAUDE docs updated

Workflow summary

  1. Design team sets up goals and an experiment in Matomo
  2. Dev side builds variant components and wraps old and new versions in the ABTestWrapper component
  3. Review PR contents using override menu
  4. Deploy to launch.
  5. End test via schedule or manually within Matomo

Preview link

https://deploy-preview-15716--ethereumorg.netlify.app/en/stablecoins/

wackerow added 10 commits June 16, 2025 20:11
- ✅ Removed `NEXT_PUBLIC_` prefix from all A/B testing env vars
- ✅ Updated documentation to reflect new naming convention
  - Add core AB testing library with TypeScript types
  - Implement weighted variant assignment and cookie persistence
  - Add server-side configuration system with environment variables
  - Support multi-variant testing with configurable weights
  - Include proper error handling and development logging
  - Create ABTestWrapper for server-side variant rendering
  - Add ABTestTracker for Matomo integration and cookie management
  - Implement development debug panel with variant switching
  - Support fallback rendering when tests are disabled
  - Move AB components to dedicated directory structure
  - Remove old AB testing files from utils and root components
  - Update import paths to use new organized structure
  - Update stablecoins page to use new AB testing components
  - Add translation keys for new AB test variants
  - Fix cookie prefix synchronization between server and client
  - Prevent infinite re-render loops in server components
  - Improve debug panel state management with useTransition
  - Add proper loading states and error handling
  - Enhance debug panel UX with reset buttons and tooltips
Copy link

netlify bot commented Jun 25, 2025

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit 8ecfd30
🔍 Latest deploy log https://app.netlify.com/projects/ethereumorg/deploys/6866b4a0ee737f0008d60f93
😎 Deploy Preview https://deploy-preview-15716--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 50 (🔴 down 3 from production)
Accessibility: 96 (🟢 up 2 from production)
Best Practices: 91 (🔴 down 1 from production)
SEO: 99 (no change from production)
PWA: 59 (no change 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 content 🖋️ This involves copy additions or edits documentation 📖 Change or add documentation tooling 🔧 Changes related to tooling of the project translation 🌍 This is related to our Translation Program labels Jun 25, 2025
@minimalsm
Copy link
Contributor

@claude can you review what we have in this draft PR so far

Copy link
Contributor

claude bot commented Jun 25, 2025

Expand to read:

Claude finished @minimalsm's task —— View job


AB Testing PR Review

I've completed my review of the AB testing implementation. This is a well-structured and thoughtful implementation with good separation of concerns. Here's my detailed feedback:

Review Tasks:

  • Read AB testing library implementation
  • Review React components (TestWrapper, TestDebugPanel, TestTracker)
  • Analyze page integration (stablecoins page)
  • Check documentation and configuration
  • Review Matomo integration changes
  • Provide comprehensive feedback on architecture and code quality

Architecture & Design 👍

Strengths:

  • Server-side rendering: Ensures users see consistent variants on first load, preventing layout shifts
  • Clean separation: Server utilities separate from client components, following Next.js App Router patterns
  • Type safety: Good TypeScript implementation with proper interfaces
  • Cookie persistence: 30-day persistence with proper settings
  • Weighted distribution: Flexible variant assignment with configurable weights

Issues Found 🔍

Critical Issue: Matomo Integration

File: src/components/Matomo.tsx:7-9

const push = (args: unknown[]) => {
  console.log(`[Matomo] push called with args:`, args)
}

The Matomo push function is mocked and only logs to console. This means AB test tracking won't work in production. The real push function is imported but overridden.

Recommendation: Remove the mock and use the real Matomo push function.

Cookie Setting Inconsistency

Files: src/lib/ab-testing/server.ts:56-72 and src/lib/ab-testing/actions.ts:15-25

In server.ts, cookie setting is wrapped in try/catch with a comment about SSR failures, while actions.ts uses await cookies(). This suggests potential inconsistency in cookie handling patterns.

Recommendation: Standardize cookie setting approach and ensure error handling is consistent.

Type Safety Gaps

File: src/lib/ab-testing/types.ts:22

export type ABTestVariants = [Element, Element, ...Element[]]

While this enforces minimum 2 variants at compile time, runtime errors could still occur if fewer elements are provided to ABTestWrapper.

Recommendation: Add runtime validation in ABTestWrapper.

Code Quality Assessment ✅

Positive Aspects:

  • Error handling: Proper try/catch blocks and graceful degradation
  • Development experience: Excellent debug panel with variant forcing
  • Documentation: Comprehensive guide in docs/ab-testing.md
  • Environment configuration: Well-structured env var pattern
  • Security considerations: Appropriate cookie settings

Minor Improvements:

  1. Debug Panel Scope: Currently shows in all non-production environments

    // src/components/AB/TestWrapper.tsx:36
    {process.env.NODE_ENV !== "production" && (

    Consider limiting to development only: process.env.NODE_ENV === "development"

  2. Server Action Error Handling:

    // src/components/AB/TestDebugPanel.tsx:38-40
    } catch (error) {
      console.error("Failed to force variant:", error)
    }

    Could provide user feedback for failures.

  3. Magic Numbers: 30-day expiry is hardcoded in multiple places. Consider extracting to constants.

Implementation Review 📋

Page Integration (app/[locale]/stablecoins/page.tsx:594-616)

The AB test integration is clean and follows the expected pattern:

<ABTestWrapper
  testKey="AppTest"
  variants={[
    <div key="original">...</div>,
    <div key="Variation1">...</div>,
  ]}
/>

Environment Configuration (.env.example)

Well-documented with clear examples and naming conventions. The template structure is excellent for onboarding new tests.

Security Considerations 🔒

  • httpOnly: false is intentional for debugging but means cookies are accessible via JavaScript
  • Cookie secure flag properly set based on environment
  • No sensitive data exposure in environment variables

Performance Impact 📊

  • Server-side assignment minimizes client-side JavaScript
  • Debug panel only loads in non-production
  • Minimal bundle size impact

Recommendations 🎯

High Priority:

  1. Fix Matomo integration - Remove the mocked push function
  2. Standardize cookie handling - Ensure consistent patterns between server and actions

Medium Priority:
3. Add runtime validation for variant count
4. Extract magic numbers to constants
5. Improve error user feedback in debug panel

Low Priority:
6. Consider limiting debug panel to development only
7. Add integration tests for the AB testing flow

Overall Assessment ⭐

This is a solid implementation that follows best practices for server-side AB testing. The architecture is well-thought-out and the developer experience is excellent. The main blocker is the Matomo integration issue, which prevents tracking from working in production.

Recommendation: Address the Matomo integration issue and this will be ready for production use.


Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

early review :) looking good

@wackerow wackerow changed the title [wip] AB testing prototype feat: privacy-preserving A/B split testing Matomo integration Jul 2, 2025
@wackerow wackerow marked this pull request as ready for review July 2, 2025 21:11
Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

Much better with the server side approach! nice job @wackerow

Maybe Im missing something but i don't see the panel in the preview deploy and not sure exactly how to test it.

Other than that, the code looks fine 👍🏼

{},
{
headers: {
"Cache-Control": "s-max-age=300, stale-while-revalidate=600",
Copy link
Member

Choose a reason for hiding this comment

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

do we need cache control for the error response?

@wackerow wackerow merged commit ebc74b8 into dev Jul 3, 2025
6 checks passed
@wackerow wackerow deleted the ab-testing branch July 3, 2025 17:47
This was referenced Jul 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits documentation 📖 Change or add documentation tooling 🔧 Changes related to tooling of the project translation 🌍 This is related to our Translation Program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants