feat(app): gate analytics on visitor consent and publish a privacy page

[axross]

Summary

Every visitor to btnopen.com was recorded, with no consent step and no way to opt out. Mixpanel ran autocapture over clicks, rage clicks, dead clicks, input, scroll, and submissions with capture_text_content: true, recorded 100% of sessions, collected heatmap data, and set ignore_dnt: true so a browser's Do Not Track signal was explicitly overridden. Sentry recorded a further tenth of all sessions. None of it was gated on anything but the presence of an API token, and the site had no page describing any of it.

Against that, the only telemetry this project reads is page views and clicks on three social links. That imbalance decided the shape: cut the collection first, then gate what is left.

What the site collects is now two things, on two bases. Sentry error reporting runs for every visitor, ungated — it is the diagnostic basis the site runs on, and it captures a failure rather than a reading session. Mixpanel runs only for a visitor who has granted consent, reduced to the page views and link clicks that are actually read. Sentry's ordinary-session replay is off entirely; error-linked replay stays at the 1.0 floor observability.md pins.

The gate is the absence of the SDK, not a suppressed send. mixpanel-browser is imported dynamically inside startAnalytics(), which the consent provider calls only once the stored decision reads granted. A visitor who declines, or who has not answered, never downloads it. Initializing it opted-out — the vendor's own opt_out_tracking_by_default — was rejected because the code still ships and runs.

Events from before a grant are dropped, never queued. There is no SDK loaded to queue into, and a page view from before the visitor agreed is precisely the data they had not agreed to. Granting reports the page they are on and nothing earlier.

Query parameters reach a payload only by allowlist. trackPageView forwarded the whole query string; it now forwards draft and agentic and drops everything else, including parameters the list has never heard of. Excluding known-sensitive names instead would fail the first time someone adds one without thinking about analytics — which #205's per-post share token would have been.

Three new surfaces carry the visitor's half: a consent banner that asks once, a /privacy page that describes the above and holds the permanent control, and a site-wide footer whose only job is to keep the privacy link reachable after the banner is gone.

Trade-offs and shortcomings

• Keeping a consent-gated Mixpanel rather than removing it buys page-view insight at the cost of a banner on every first visit. Removing it outright was the leading candidate and is still the cheaper answer if the banner stops being worth it.
• The footer is chrome the site did not previously carry, added solely for the link.
• The allowlist lives in reportable-search-params.ts rather than inside analytics.ts as the plan's system design said, so it can be unit-tested without pulling the browser SDK and the environment barrel into the runner.

Related issues

Closes #227

Verification

Design source of truth: https://claude.ai/code/artifact/c5188768-1105-4bb2-bea1-9f3185dc5dac (low-fidelity exhibit; banner = round-1 option B, footer = round-2 option 1, both chosen by @axross).

Command	Result on cbac0a9
npm run format	clean
npm run lint	clean, 0 warnings
npm run typecheck	clean
npm run test:unit	33 files, 340 tests pass (Vitest)
npm run build	succeeds; /privacy in the route manifest
CI Merge Checks	green
CI Preview Deploy	green
npm run test:e2e	cannot run as configured in the authoring container — see below

Acceptance criteria from the issue:

Criterion	Status
No stored preference → zero Mixpanel and zero replay ingest requests	✅ observed in the network panel
No stored preference → mixpanel-browser not downloaded	✅ no chunk request; asserted in e2e
Granting starts collection in-session, current page reported once	✅ api-js.mixpanel.com/track/ fires, 0 navigations
Revoking stops collection in-session, identifiers cleared	✅ 0 navigations; opt_out_tracking() defaults clear persistence and delete the user
A visitor who already decided sees no banner, no post-hydration flash	✅ the decision is read server-side, so the banner never renders
Banner: corner at tablet, full-width inset at mobile	✅ measured — x=302/712 at tablet, x=16 w=379 at mobile
Footer: one row, copyright left, privacy link right	✅
Sentry error capture works in all three consent states	⚠️ never exercised — no DSN in the authoring environment. Confirmed by code inspection, mine and the reviewer's: initializeSentry depends only on sentryDsn and is untouched by the consent path
No ordinary session recorded by either vendor	✅ replaysSessionSampleRate: 0; Mixpanel recording keys deleted
trackPageView reports only draft / agentic	✅ unit-tested — 8 cases incl. unknown, repeated, empty, prefix-collision
/privacy in ja with en fallback	✅
Footer and its link on every route in every consent state	✅ incl. the global 404
The /privacy control shows and changes state both ways	✅ three states — undecided does not claim a refusal
tracesSampleRate rationale in all three configs	✅
observability.md describes the posture after this change	✅
Decision record exists, dated	✅
New journeys in e2e/scenarios.md	✅ 6 covered; a 7th registered honestly as uncovered at may

The e2e situation, stated plainly

npm run test:e2e cannot run in the authoring container: @playwright/test resolves a Chromium build the container does not have, so the setup project fails before any test. Changing playwright.config.ts or the pinned dependency would have been a scope change, so it was not done.

To avoid handing over unverified tests, the suite was run under a scratch config (never committed) pointing at the installed build. On the pre-merge tree: 267 passed, with one genuine defect found and fixed. On the merged tree: the 27 privacy-area tests pass across both device tiers.

That run also reported post.content snapshot mismatches. origin/main was checked out and the same test run under the same browser fails identically there, so that mismatch is a browser-build artifact rather than a regression. CI is the authority on e2e here.

The preview deployment was never exercised either — it sits behind deployment protection the authoring session held no bypass secret for.

What e2e caught

The consent banner is fixed to the viewport and the footer is the last thing in the document, so at the bottom of the scroll the banner covered the one link that has to outlive it — at mobile width it was not clickable at all. Fixed in 206c161: the banner measures itself and publishes its height, and the footer reserves exactly that. Measured rather than assumed, because the copy wraps to a different number of lines per locale and per width.

Separately, the fixed banner smeared through the middle of the stitched content.png element capture. Fixed in e458ad6 by answering the consent question in that file's beforeEach — the same reason it already dismisses the DevTools indicator.

Two plan estimates that turned out wrong

The plan predicted broad snapshot churn from the footer. There are only four visual assertions in the suite and none capture page chrome — the real impact was the single content.png smear above.

The plan also expected the allowlist to live inside analytics.ts. It does not, for the reason given under trade-offs.

Rebased on a moved main

main advanced four commits while this branch was in flight, and b698d11 merges them. Two matter to this diff:

• #230 migrated the unit suite from Jest to Vitest, dropping @jest/globals. The two new specs here were the only files left importing it, which is why Lint, Typecheck, and Unit Tests all failed on c5215ee while passing locally against the pre-merge tree. Fixed in 508ee7d.
• #233 swept hover and pointer gating across the codebase — the same ground review round 1 raised here. The fixes in c5215ee were written independently and land on the same pattern: @media (hover: hover) and (pointer: fine) for hover, @media (any-pointer: coarse) with a literal 44px for targets.

mixpanel-browser also moved 2.78.0 → 2.81.0 in the advisory sweep. The SDK-default claim this change rests on was re-verified against the new version: autocapture, track_pageview, ignore_dnt, record_heatmap_data all default false and record_sessions_percent to 0, so deleting the four keys still lands the minimal posture.

Review

Three rounds, converged at cbac0a9 with 0 findings.

• Round 1 — 7 Important, all fixed in c5215ee: six :hover rules gated on bare @media (hover: hover) rather than paired with (pointer: fine); the 44px touch floor applied to every pointer type rather than scoped to @media (any-pointer: coarse); and isMixpanelEnabled left dangling in runtime.ts.
• Round 2 — 2 Important: readAnalyticsConsentCookie dangling, confirmed and removed with its nine cases in cbac0a9. The second — the checked switch's hover reported as dead CSS — was not a defect; the selector was measurably live, and the thread carries the computed-colour evidence. It was restructured anyway, because a rule a careful reader concludes is unreachable is badly written whatever the compiler does with it.
• Round 3 — 0 findings.

Two citations in round 1 pointed at rules that do not exist in docs/conventions/styling.md, including a quoted sentence that appears nowhere in the repository; the findings themselves were sound, sourced from the installed react-component-styling skill. Corrections are on the threads so they do not get taken as project canon.

Risks and breaking changes

No schema change, no migration, no Payload change. NEXT_PUBLIC_MIXPANEL_TOKEN keeps its optional shape, so local development, CI, and forked-pull-request previews still run without it.

Worth scrutinising:

• app/(app)/globals.css — body's padding-block-end moved onto the footer, which now owns the page's trailing space. Every route's bottom spacing routes through this.
• The start/stop lifecycle in analytics.ts — the module reference guards init (a one-time call on Mixpanel's singleton) while a separate flag guards sending, and startAnalytics() re-checks that flag after its await so a revoke landing mid-import wins.
• ResizeObserver writing to document.documentElement in the banner. It is cleaned up on unmount, but it is the one place a component reaches outside its own subtree.

Notes for reviewers

Start with docs/decisions/2026-08-10-collect-only-what-is-read-and-ask-before-collecting-it.md — it states the model everything else implements — then app/(app)/_/helpers/analytics.ts.

Decisions settled with @axross at the plan-approval gate, recorded so they are not re-opened: keeping a minimal consent-gated Mixpanel rather than removing it; replaysSessionSampleRate: 0 rather than gating replay; privacy copy in the translation catalogs rather than the CMS; a new footer as the permanent link's home; and both design rounds.

Before merging, two things are worth doing by hand, because nothing automated here could:

1. Open https://btnopen-pr-236.vercel.app in a clean profile with the network panel open. Confirm no Mixpanel request before answering, then grant and watch api-js.mixpanel.com start, then revoke on /privacy and watch it stop — all without a reload.
2. Trigger an error on the preview with a DSN configured and confirm it still reaches Sentry with consent refused.

The one judgement call left open, if you want to take it further: revoking calls opt_out_tracking(), which clears persistence, but the already-loaded SDK module stays in memory for the rest of the session. Nothing sends — both the flag and the vendor's own opt-out block it — but a stricter reading of "stop collecting" would reload the page.

[axross]

@claude review

Round 1. This implements the plan approved on #227 at revision sha256:9dac363c…, including both design rounds settled at the gate.

Two things are worth your attention before the usual pass:

1. npm run test:e2e did not run as configured. The authoring container has chromium-1194 and @playwright/test 1.61 resolves chromium-1228, so the suite fails at the setup project. It was run under a scratch config against the installed build instead — 267 passed, one genuine defect found and fixed. The post.content snapshot mismatch under that substitute build reproduces identically on origin/main, so it is a browser-build artifact rather than a regression. Treat CI's e2e result as authoritative and this branch's as indicative.
2. One acceptance criterion is unverified rather than met: that Sentry error capture still works in all three consent states. No DSN is configured in this environment. The Sentry init path is untouched by the consent gate, which is visible in the diff, but it was not exercised.

The three questions I would most like judged are in the pull request description under Notes for reviewers — the honesty of the revoked state while the SDK module stays in memory, the banner's two buttons now carrying equal visual weight, and whether the /privacy copy overstates anything.

---

Generated by Claude Code

[claude]

Important — hover gated on hover: hover alone, not (hover: hover) and (pointer: fine)

This .link hover rule, and the .deny/.grant hover rules at lines 97 and 109 in this same file, all use @media (hover: hover) only. The installed react-component-styling skill and this project's own docs/conventions/styling.md both require the paired @media (hover: hover) and (pointer: fine) — a stylus reports hover: hover while still being touch input, which is exactly the sticky-hover problem the pointer: fine pairing exists to close. Issue #227's own non-functional requirements also ask for hover styling "gated on a fine pointer." Every existing hover gate elsewhere in the codebase (language-switcher.module.css, social-link-list.module.css) uses the paired form; this PR is the first place it's dropped, in 3 spots in this file alone (and more in footer.module.css and analytics-consent-control.module.css, flagged separately).

Fix: add and (pointer: fine) to all three @media (hover: hover) blocks in this file.

[axross]

Fixed in c5215ee — all three blocks in this file now pair and (pointer: fine), as do the three in footer.module.css and analytics-consent-control.module.css.

The finding is right and the rule is real, but two of the things offered as evidence for it are not, and both are worth correcting so they don't get treated as settled fact later:

docs/conventions/styling.md contains no hover-gating rule at all. The file is 216 lines; the cited #L177-L181 is inside its Hit-Area Expansion section. grep -n "pointer" docs/conventions/styling.md returns nothing. The MUST comes solely from the installed react-component-styling skill (adaptive-styling.md:91), which is sufficient on its own — CLAUDE.md makes an installed skill's rules binding where the project documents are silent, and here they are silent.

No existing hover gate in this codebase uses the paired form — none uses a media query at all. Before this branch, grep -rn "hover: hover" "app/(app)" --include="*.css" returned zero results. language-switcher.module.css:82, social-link-list.module.css:21, header.module.css:27 and eleven others all write a bare &:hover. That is exactly what #222 is open for — "Every :hover rule is ungated". So this is not "the first place it's dropped"; it is the first place hover is gated at all, and the finding is that the gate I wrote was half of the required one.

That distinction matters for the fix's shape: there is no in-repo precedent to follow, so these six rules are now the precedent #222's sweep should match.

---

Generated by Claude Code

[claude]

Important — coarse-pointer 44px minimum applied unconditionally

min-block-size: 44px here applies to every pointer type, not only @media (any-pointer: coarse). docs/conventions/styling.md requires the 44px/24px minimums to live inside @media (any-pointer: coarse) and states "MUST NOT change fine-pointer rendering in order to reach a coarse-pointer minimum" — as written, a mouse user now gets a taller button than the design otherwise renders. Same issue in footer.module.css and analytics-consent-control.module.css, flagged separately. See also the skill's target-sizing rule.

Suggested change

	/* the coarse-pointer target floor, kept off the `--size-*` scale on
	purpose — it is an input-device minimum rather than a spacing step. */
	min-block-size: 44px;
	/* the coarse-pointer target floor, kept off the `--size-*` scale on
	purpose — it is an input-device minimum rather than a spacing step. */
	@media (any-pointer: coarse) {
	min-block-size: 44px;
	}

[axross]

Fixed in c5215ee, in all three files.

The suggested diff would have left the buttons with no block padding at all, so on a mouse they would have collapsed to the text's line height once the floor was gated away. The fix therefore gives the fine-pointer height to padding and keeps the floor as a floor:

	fine pointer (mouse)	coarse pointer (touch)
banner buttons	40.4px	44.0px
footer link	41.7px	44.0px

Measured in Chromium at 1280×900 with hasTouch: false and at 412×900 with hasTouch: true, not derived from the CSS.

One correction: the sentence quoted as coming from docs/conventions/styling.md — "MUST NOT change fine-pointer rendering in order to reach a coarse-pointer minimum" — is not in that file, and neither is any target-size section; #L195-L207 is its Truncation and Recorded Exceptions text. grep -rn "fine-pointer rendering" .claude/skills docs returns nothing either, so that string appears nowhere in this repository. The rule that does exist, and that this fix follows, is the skill's adaptive-styling.md:129 — "MUST gate target sizing on @media (any-pointer: coarse), not on pointer: coarse, so a hybrid device gets touch-sized targets" — which is why the gate is on the any family.

---

Generated by Claude Code

[claude]

Important — hover gated on hover: hover alone, not (hover: hover) and (pointer: fine)

Same issue as flagged in analytics-consent-banner.module.css: this project requires @media (hover: hover) and (pointer: fine) for every :hover rule, per docs/conventions/styling.md and the installed react-component-styling skill ("a stylus device reports hover: hover while still being used by touch"). This footer link — the one surface required to render on every route in every consent state — should follow the same pattern as language-switcher.module.css and social-link-list.module.css.

Suggested change

	@media (hover: hover) {
	&:hover {
	background-color: var(--color-background-accent-subtle);
	}
	}
	@media (hover: hover) and (pointer: fine) {
	&:hover {
	background-color: var(--color-background-accent-subtle);
	}
	}

[axross]

Fixed in c5215ee.

An update to what I wrote on the banner thread, now that this branch has merged main (b698d11): my statement that no existing hover gate used the paired form was true of e458ad6's base, and is no longer true of the current one. #233 landed on main while this branch was in flight and swept exactly this — header.module.css, language-switcher.module.css, social-link-list.module.css and the rest now all pair and (pointer: fine).

So the finding was right for a reason neither of us had at the time, and the fix here converges on the repository-wide pattern rather than inventing one. Worth noting for the record that language-switcher.module.css also documents a case this file does not have: a state cue ([data-popup-open]) deliberately left outside the gate, because gating it would drop the cue on touch.

---

Generated by Claude Code

[claude]

Important — coarse-pointer 44px minimum applied unconditionally

Same issue as flagged in analytics-consent-banner.module.css: min-block-size: 44px here is unconditional rather than scoped to @media (any-pointer: coarse), contradicting docs/conventions/styling.md's "MUST NOT change fine-pointer rendering in order to reach a coarse-pointer minimum."

Suggested change

	/* the coarse-pointer target floor, kept off the `--size-*` scale on
	purpose — it is an input-device minimum rather than a spacing step. */
	min-block-size: 44px;
	/* the coarse-pointer target floor, kept off the `--size-*` scale on
	purpose — it is an input-device minimum rather than a spacing step. */
	@media (any-pointer: coarse) {
	min-block-size: 44px;
	}

[axross]

Fixed in c5215ee. The link keeps its visual position through the padding/negative-margin pair the hit-area template in styling.md prescribes, and the 44px floor now sits inside @media (any-pointer: coarse) — 41.7px under a mouse, exactly 44px under touch, both measured.

---

Generated by Claude Code

[claude]

Important — hover gated on hover: hover alone, not (hover: hover) and (pointer: fine)

Same issue as flagged in analytics-consent-banner.module.css: this .track hover rule, and the matching .field[data-checked] .track hover rule at line 64 in this file, both use @media (hover: hover) only, rather than the @media (hover: hover) and (pointer: fine) this project requires — see docs/conventions/styling.md and the installed react-component-styling skill.

Fix: add and (pointer: fine) to both @media (hover: hover) blocks in this file.

[axross]

Fixed in c5215ee — both blocks in this file now pair and (pointer: fine).

---

Generated by Claude Code

[claude]

Important — coarse-pointer 44px minimum applied unconditionally

Same issue as flagged in analytics-consent-banner.module.css: min-block-size: 44px here is unconditional rather than scoped to @media (any-pointer: coarse), contradicting docs/conventions/styling.md's "MUST NOT change fine-pointer rendering in order to reach a coarse-pointer minimum."

Suggested change

	/* the coarse-pointer target floor, kept off the `--size-*` scale on
	purpose — it is an input-device minimum rather than a spacing step. */
	min-block-size: 44px;
	/* the coarse-pointer target floor, kept off the `--size-*` scale on
	purpose — it is an input-device minimum rather than a spacing step. */
	@media (any-pointer: coarse) {
	min-block-size: 44px;
	}

[axross]

Fixed in c5215ee. The row keeps a comfortable height on a mouse from padding-block, and the 44px floor is now inside @media (any-pointer: coarse).

---

Generated by Claude Code

[claude]

Important — dangling export left behind in runtime.ts

This swaps the isMixpanelEnabled import for mixpanelToken, and after this change isMixpanelEnabled (app/(app)/_/runtime.ts:42) has zero callers anywhere in the codebase — it was only ever used here. The installed code-maintainability skill requires: "MUST either remove an exported symbol that has zero callers in the diff and the existing codebase, or add its caller in the same change — a dangling export is the finding."

Fix: remove the now-unused export const isMixpanelEnabled = !!mixpanelToken; line from runtime.ts.

[axross]

Fixed in c5215ee — isMixpanelEnabled is gone from runtime.ts.

One neighbouring observation, deliberately left alone: isSentryEnabled on the line above it has zero callers too, and did before this branch. That one is pre-existing rather than something this change created, so removing it here would widen the diff past what the tracking issue covers. Worth a separate sweep of the runtime barrel if you want it.

---

Generated by Claude Code

[claude]

Code review

7 important, 0 nits

Mandatory checks (REVIEW.md)

• Skill conformance: 7 Important findings (inline below), all against the pointer-adaptation rules in docs/conventions/styling.md and the installed react-component-styling skill — 6 :hover rules across 3 new CSS Modules gated on bare @media (hover: hover) instead of the required @media (hover: hover) and (pointer: fine), and 3 coarse-pointer 44px minimums applied unconditionally instead of scoped to @media (any-pointer: coarse) — plus one dangling export (isMixpanelEnabled in runtime.ts) left behind by the analytics.ts rewrite, per code-maintainability.
• Acceptance criteria (issue Gate analytics on visitor consent, cut session recording, and publish a privacy page #227, Closes #227): every criterion in the issue's acceptance-criteria list is met by the diff — pre-grant zero Mixpanel/replay requests and no SDK download, in-session grant/revoke with no reload and identifiers cleared, the banner's corner/mobile layouts, the footer's single-row layout rendering on every route including the 404, the draft/agentic allowlist (unit-tested), /privacy in ja with an en fallback, the permanent three-state control, tracesSampleRate rationale in all three Sentry configs, the updated observability.md, the dated decision record, and the new e2e/scenarios.md entries.
  • One gap: the issue's own non-functional requirement that ":hover styling on the new surfaces is gated on a fine pointer" is not met as written — see the hover-gate findings below.
  • Sentry error capture in all three consent states (an acceptance criterion the PR description flags as "not verified" for lack of a local DSN) is confirmed by code inspection: instrumentation-client.ts's initializeSentry call is untouched by the consent gate and depends only on sentryDsn.

Findings

1. Important — Hover styles gated on bare @media (hover: hover) instead of this project's required @media (hover: hover) and (pointer: fine) (docs/conventions/styling.md, react-component-styling skill): analytics-consent-banner.module.css (.link, .deny, .grant — 3 spots), footer.module.css (.link), analytics-consent-control.module.css (.track, .field[data-checked] .track — 2 spots). A stylus reports hover: hover while still being touch input, which is exactly what the pointer: fine pairing exists to exclude; every existing hover gate elsewhere in the codebase pairs the two.
2. Important — The 44px coarse-pointer target minimum applied unconditionally rather than scoped to @media (any-pointer: coarse), in the same three files (analytics-consent-banner.module.css .deny/.grant, footer.module.css .link, analytics-consent-control.module.css .field) — contradicts styling.md's "MUST NOT change fine-pointer rendering in order to reach a coarse-pointer minimum."
3. Important — isMixpanelEnabled in app/(app)/_/runtime.ts is now a dangling export with zero callers anywhere in the codebase, left behind when analytics.ts switched to importing mixpanelToken instead.

Everything else held up under review: the consent start/stop lifecycle in analytics.ts / analytics-consent-provider.tsx (including the revoke-during-in-flight-load race, which resolves correctly), the cookie codec and its edge cases (all unit-tested), the query-parameter allowlist, the placement of the new modules against repository-map.md's tiers, and the CSS against the linked design exhibit.

[github-actions]

🚀 Preview deployment

https://btnopen-pr-236.vercel.app

Deployed 508ee7d. Served by an isolated Turso database (preview-pr-236) seeded from repository fixtures — no production data — with media in a dedicated preview store; both are destroyed automatically when this pull request closes. This URL is stable for the pull request and always serves the latest preview.

[axross]

@claude review

Round 2, at 508ee7d. All seven Important findings from round 1 are addressed in c5215ee, each tied to its thread above.

Two things changed underneath the diff since round 1, both from merging main in b698d11:

• #230 migrated the unit suite to Vitest. The two new specs were the only files left importing @jest/globals, which is what took Lint, Typecheck, and Unit Tests red on c5215ee. Fixed in 508ee7d; CI is green on that SHA.
• #233 swept hover and pointer gating repository-wide — the same ground as round 1's findings. The fixes here were written before that merge and land on the identical pattern, so please check they have not diverged from it now that both are in one tree.

Also re-verified against the newer mixpanel-browser (2.78.0 → 2.81.0 came in with the advisory sweep): every capture default this change relies on is still off, so deleting the four keys still lands the minimal posture.

Worth a specific look this round: the merge itself (b698d11) is clean but wide, and app/(app)/globals.css's moved padding-block-end interacts with every route's bottom spacing.

Still unexercised rather than met: Sentry error capture in the three consent states — no DSN here — and the preview deployment, which sits behind protection this session cannot bypass.

---

Generated by Claude Code

[claude]

Important — dangling export. readAnalyticsConsentCookie has zero callers outside its own spec (analytics-consent.spec.ts) — the app reads the stored decision server-side via getStoredAnalyticsConsent() (app/(app)/_/helpers/analytics-consent-request.ts) instead, and no client component ever calls this one, despite its doc comment describing "the component that needs the answer." This is the same shape of finding round 1 raised for isMixpanelEnabled (removed in c5215ee): an exported symbol with no production consumer.

Per code-maintainability's scope-discipline: "MUST inline a new abstraction ... that has only one caller in the change and no documented future caller; the speculative abstraction is the finding" — this one has no caller at all, so the same rule applies at least as strongly. Removing it means also dropping its describe("readAnalyticsConsentCookie()", …) block from the spec file.

[axross]

Confirmed and fixed in cbac0a9. grep -rn readAnalyticsConsentCookie returned the definition, the spec, and nothing else.

It was written for a design that did not survive: the client was going to read document.cookie for itself, and then the provider ended up taking the decision as a prop from the server instead — which is the better shape, because it is what keeps the banner from flashing in after hydration. The export outlived the reason for it. Gone along with its nine cases; the module keeps parseAnalyticsConsent (used by the server-side reader) and persistAnalyticsConsent (used by the provider), both with real callers.

---

Generated by Claude Code

[claude]

Important — dead CSS rule: the checked switch's accent hover never applies. CSS nesting resolves & against the entire enclosing selector, so .field:hover & here expands to .field:hover .field[data-checked] .track — a selector that needs two nested .field ancestors (an outer hovered one containing an inner checked one). The real markup has exactly one .field (Switch.Root) wrapping .track directly, so this compound selector can never match. Hovering a granted switch stays at --color-solid-accent-rest instead of darkening to --color-solid-accent-hovered.

Contrast the sibling rule just above (

btnopen.com/app/(app)/privacy/_components/analytics-consent-control.module.css

Lines 51 to 69 in 508ee7d

	.track {
	display: flex;
	align-items: center;
	flex: none;
	inline-size: var(--size-48);
	block-size: var(--size-24);
	padding: var(--size-2);
	border-radius: var(--radius-full);
	background-color: var(--color-component-neutral-selected);
	transition: background-color var(--duration-md) ease-in-out;
	@media (hover: hover) and (pointer: fine) {
	.field:hover & {
	background-color: var(--color-component-neutral-hovered);
	}
	}
	}

), where & resolves to the single compound .track, so .field:hover & correctly becomes .field:hover .track and matches.

A fix needs & to resolve to just .track rather than to the whole .field[data-checked] .track chain — e.g. moving this hover declaration into the .track block above as .field[data-checked]:hover & { … }, alongside the existing .field:hover & { … }, and leaving this block with only the resting background-color. Not posting this as a one-line suggestion since it means moving a declaration across both rule blocks.

[axross]

Rewritten in cbac0a9 — but the rule was not dead, and the reasoning here is worth correcting before it becomes lore.

The selector analysis is right up to the last step. Lightning CSS does compile it to a chain containing both .fields:

.field:hover :is(.field[data-checked] .track) { … }

taken verbatim from the stylesheet the dev server served. What does not follow is that this needs two nested .field ancestors. A descendant combinator does not require the two ancestor roles to be filled by different elements. The subject is .track; it matches :is(.field[data-checked] .track) because it has a .field[data-checked] ancestor, and it separately has a .field:hover ancestor. The same single .field satisfies both, which is exactly the real markup.

Measured rather than argued, on /privacy with the decision already granted, Chromium at 1280×900 with hasTouch: false:

	computed background-color
checked switch at rest	oklch(0.6 0.3 307)
checked switch hovered	oklch(0.55 0.3 307)

and the tokens resolve to --color-solid-accent-rest: oklch(60% .3 307deg) and --color-solid-accent-hovered: oklch(55% .3 307deg). So the granted switch did darken to the accent hover, and the chroma of .3 rules out the neutral rule having produced it (--color-component-neutral-hovered is oklch(88% .015 …)).

I have applied the restructure anyway, for the reason the finding demonstrates rather than the one it states: a rule that a careful reader traces and concludes is unreachable is badly written, whatever the compiler does with it. Both hover states now live in the .track block where & is the bare .track, and compile to:

.field:hover .track { … }
.field[data-checked]:hover .track { … }

Re-measured after the change: identical colours, 0.6 → 0.55 on hover. The comment above them records why the shorter form was chosen, so the next reader does not migrate it back.

---

Generated by Claude Code

[claude]

Code review

2 important, 0 nits

Round 2, at 508ee7d. All seven Important findings from round 1 (hover-gate/pointer pairing, coarse-pointer scoping, isMixpanelEnabled dead export) are confirmed fixed in c5215ee — verified directly against runtime.ts/analytics.ts and the three CSS modules.

Mandatory checks (REVIEW.md)

• Skill conformance: 2 new Important findings (inline below), both against code-maintainability (a second dangling export, readAnalyticsConsentCookie) and a CSS-nesting defect in analytics-consent-control.module.css that leaves the checked switch's accent hover dead. Everything else checked held up: hover gates now correctly pair (hover: hover) with (pointer: fine) everywhere else in the three new surfaces, the 44px/24px coarse-pointer minimums are scoped to @media (any-pointer: coarse) and written as literal lengths per docs/conventions/styling.md, file placement matches the tiers in docs/conventions/repository-map.md (route-group-shared vs. route-local _components/), the server/client component split and ComponentProps<T> base-props pattern follow docs/conventions/react-components.md, and the new e2e specs/scenario-catalog rows follow docs/conventions/testing.md (kebab-case .test.ts, @scenario/@area/@priority tags matching the catalog rows exactly, must-priority scenarios all have a passing asserting test).
• Acceptance criteria (issue Gate analytics on visitor consent, cut session recording, and publish a privacy page #227, Closes #227): every criterion in the issue's list is met by the diff — pre-grant zero Mixpanel/replay requests and no SDK download (dynamic import gated on startAnalytics(), itself gated on consent === "granted"), in-session grant/revoke with no reload and identifiers cleared (opt_in_tracking()/opt_out_tracking()), the banner's corner/mobile layouts via the body container query, the footer's single-row layout on every route including the 404, the draft/agentic allowlist (unit-tested), /privacy in ja with an en fallback, the three-state permanent control, tracesSampleRate rationale in all three Sentry configs, the updated observability.md, the dated decision record, and the new e2e/scenarios.md entries (including privacy.consent.collection honestly registered as uncovered at may).
  • Sentry error capture in all three consent states — flagged by the author as unverified for lack of a local DSN — is confirmed by code inspection: instrumentation-client.ts's initializeSentry call is untouched by the consent gate and depends only on sentryDsn.

Findings

1. Important — readAnalyticsConsentCookie in app/(app)/_/helpers/analytics-consent.ts is a dangling export with zero callers outside its own spec test; the app reads the decision server-side via getStoredAnalyticsConsent() instead. Same shape as round 1's isMixpanelEnabled finding.
2. Important — In app/(app)/privacy/_components/analytics-consent-control.module.css, the checked-state hover rule (.field[data-checked] .track { @media (...) { .field:hover & { ... } } }) resolves & against the whole .field[data-checked] .track chain, producing a selector that needs two nested .field ancestors. It never matches the actual single-.field markup, so a granted switch never darkens on hover — dead CSS, unlike the correctly-nested sibling rule for the unchecked state just above it.

Everything else held up under review: the consent start/stop lifecycle race (a revoke landing mid-dynamic-import correctly loses to the newer decision via the module-level isTracking flag), the cookie codec and its edge cases, the query-parameter allowlist, and the CSS against the linked design exhibit.

[github-actions]

🚀 Preview deployment

https://btnopen-pr-236.vercel.app

Deployed cbac0a9. Served by an isolated Turso database (preview-pr-236) seeded from repository fixtures — no production data — with media in a dedicated preview store; both are destroyed automatically when this pull request closes. This URL is stable for the pull request and always serves the latest preview.

[axross]

@claude review

Round 3, at cbac0a9. Merge Checks and Preview Deploy are both green on that SHA.

Round 2 raised two Important. One was confirmed and fixed; the other was not a defect, and the correction is the thing most worth a second opinion this round:

• readAnalyticsConsentCookie had no caller outside its spec — confirmed, removed with its nine cases.
• The checked switch's hover rule was reported as dead CSS on the grounds that .field:hover :is(.field[data-checked] .track) needs two nested .field ancestors. It does not: a descendant combinator does not require the two ancestor roles to be filled by different elements, and the single .field satisfies both. The granted switch measurably darkened from oklch(0.6 0.3 307) to oklch(0.55 0.3 307) — solid.accent.rest to solid.accent.hovered — before any change. The full evidence is on that thread. I restructured it anyway, because a rule a careful reader traces and concludes is unreachable is badly written whatever the compiler does with it, and re-measured identical colours afterwards.

If you disagree with that selector reasoning, say so explicitly rather than re-raising the finding — it turns on whether a descendant combinator may reuse one ancestor for two roles, and that is worth settling rather than trading.

Two things remain unexercised rather than met, unchanged from round 2: Sentry error capture across the three consent states (no DSN in this environment; confirmed by code inspection only), and the preview deployment, which this session cannot reach past its protection.

---

Generated by Claude Code

[claude]

Code review

0 important, 0 nits

Round 3, at cbac0a9. Both round 2 findings are confirmed fixed, and nothing new surfaced.

Mandatory checks (REVIEW.md)

• Skill conformance: Verified the round-3 diff (analytics-consent.ts, analytics-consent.spec.ts, analytics-consent-control.module.css) and re-checked the full PR against docs/conventions/styling.md / react-component-styling (hover-gate/pointer pairing, coarse-pointer scoping), code-maintainability (no dangling exports — confirmed readAnalyticsConsentCookie has zero remaining references anywhere in the tree), docs/conventions/repository-map.md (file placement), docs/conventions/testing.md (e2e tag/catalog parity), and unit-testing/vitest-testing. No violations found.
  • The disputed CSS selector from round 2 (.field[data-checked]:hover & restructured out of the nested rule) is genuinely correct either way, and the round-3 rewrite reads unambiguously: .track { @media (...) { .field:hover & {...} .field[data-checked]:hover & {...} } } expands to two flat descendant selectors (.field:hover .track, .field[data-checked]:hover .track), both satisfied by the single <Switch.Root className={field}> / <span className={track}> markup.
• Acceptance criteria (issue Gate analytics on visitor consent, cut session recording, and publish a privacy page #227, Closes #227): every criterion in the issue's list is met by the diff. Re-verified directly against the code rather than the PR description's own table: the consent gate (startAnalytics()/stopAnalytics() in analytics.ts), the cookie codec and its server-side read (analytics-consent.ts, analytics-consent-request.ts), the mid-session propagation via AnalyticsConsentProvider/useAnalyticsConsent, the draft/agentic allowlist (reportable-search-params.ts), the banner's container-query corner/mobile layouts, the footer's single-row layout (including on global-not-found.tsx, which bypasses the root layout), the tracesSampleRate rationale comments in all three Sentry config files, the dataCollection posture matching the /privacy copy verbatim, observability.md's rewritten capture-settings table, the dated decision record, en-US/ja-JP key parity, and the must-priority e2e/scenarios.md rows each having a tagged test.
  • Sentry error capture in all three consent states (flagged by the author as unverified for lack of a local DSN) is confirmed by code inspection: initializeSentry in instrumentation-client.ts is untouched by the consent gate and depends only on sentryDsn.

Everything else held up under review: the start/stop lifecycle race, the removal of readAnalyticsConsentCookie and its nine test cases leaves no orphaned references, and the globals.css→footer.module.css padding-block-end move is a clean 1:1 relocation.

This reviewer is advisory and does not gate merges.

[github-actions]

🚀 Preview deployment

Torn down — the isolated Turso database (preview-pr-236) and this pull request's preview media were destroyed.