fix(#399): site/ — GA4 + consent banner on all 4 pages#400
Conversation
- Copy gtag.js + Consent Mode v2 init + cookie consent banner from index.html to how-it-works.html, architecture.html, skills.html. - Wrap each block with begin/end markers so future sync edits are greppable across pages. - Remove dead `anonymize_ip: true` from gtag config (no-op in GA4 — all IPs auto-anonymized; carryover from Universal Analytics). Closes #399
✅ Deploy Preview for apexyard ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
atlas-apex
left a comment
There was a problem hiding this comment.
Code Review: PR #400
Commit: e7ceb2eb47b4ff6c2b7cdb58817d04a2b5898294
(Submitting as --comment because GitHub blocks --request-changes on self-authored PRs. Treat as a blocker; the merge gate should refuse anyway because CI is red — see below.)
Summary
Adds GA4 + Consent Mode v2 + cookie consent banner to the three site pages that were missing them (how-it-works.html, architecture.html, skills.html), bringing them to parity with index.html. Also removes the dead anonymize_ip: true UA-era flag from the existing index.html gtag config. Cherry-pick-style release branch off main, single commit, 4 files touched, zero framework code.
Checklist Results
- Architecture & Design: Pass (HTML/CSS/JS copy-paste; no architecture surface)
- Code Quality: Pass (byte-identical blocks across 4 pages; markers paired)
- Testing: Partial (deploy-preview smoke check listed as a post-merge step, which is correct for a Netlify-driven workflow)
- Security: Pass (Consent Mode v2 ordering correct: default-denied → localStorage replay → config; no secrets; CSP already covers GA4 endpoints per PR body)
- Performance: Pass (script is
async; one extra script tag per page) - PR Description & Glossary: Pass (4 narrative summary bullets, 7-term glossary, both skip markers justified inline)
- Summary Bullet Narrative: Pass (every bullet has verb + rationale)
- Technical Decisions (AgDR): N/A (pure copy-paste of an existing pattern;
<!-- agdr: not-applicable -->marker present and justified) - Adopter Handbooks: N/A (migration-safety is blocking but doesn't trigger — no migration files; clean-architecture-layers and commit-message-quality are advisory and clean)
Issues Found
BLOCKER — Red CI: site-counts drift detection failed
The <meta name="llm:*"> token-count / doc-length tags on two of the three newly-instrumented pages weren't refreshed to reflect the +126 lines of GA4 + consent code added by this PR. Per .claude/rules/pr-quality.md § "No Red CI Before Merge", red CI blocks merge regardless of cause.
CI output (https://github.com/me2resh/apexyard/actions/runs/26367005290/job/77612431706):
DRIFT: site/architecture.html — meta token-count=8864 chars=35458
vs actual tokens=10201 chars=40807 (diff: 13% tokens / 13% chars; >5% tolerance)
DRIFT: site/skills.html — meta token-count=10658 chars=42632
vs actual tokens=11920 chars=47681 (diff: 10% tokens / 10% chars; >5% tolerance)
Required updates (recomputed locally with wc -c < <file>; tokens = chars / 4):
| File | Current meta | Required meta |
|---|---|---|
site/architecture.html line 22 |
content="8864" |
content="10201" |
site/architecture.html line 23 |
content="35458 chars" |
content="40807 chars" |
site/skills.html line 22 |
content="10658" |
content="11920" |
site/skills.html line 23 |
content="42632 chars" |
content="47681 chars" |
After the edit, re-running bash .claude/hooks/tests/test_site_counts.sh locally should report all three drift-checked pages within tolerance. Note: adding the meta refresh will itself nudge chars/tokens by ~10 bytes — well inside the 5% tolerance.
Handbook Findings
None. The migration-safety handbook (blocking) doesn't trigger because no migration files are touched. The clean-architecture-layers and commit-message-quality handbooks (advisory) have nothing to flag.
Suggestions (non-blocking — for follow-up)
Drift-check scope coverage: the LLM-meta loop in .claude/hooks/tests/test_site_counts.sh:230 iterates over index.html, architecture.html, skills.html only — it doesn't cover site/how-it-works.html. Locally that file now drifts by ~22% (actual 8065 tok / 32263 chars vs meta 6572 tok / 26291 chars) but CI doesn't fail because the loop skips it. Worth a follow-up framework ticket to add how-it-works.html to the loop (and refresh its meta in the process), so a future PR that touches only how-it-works.html will get the same drift catch. Out of scope here.
Per-page coverage parity verification: PR body's coverage table shows gtag-refs=6 per page. I see 2 explicit G-G3EMR3CB02 references per page (script src + config). The "6" likely counts function calls including gtag('consent', 'default', ...), the localStorage replay update, gtag('js', ...), gtag('config', ...), the gtag('consent', 'update', ...) inside the banner IIFE, and the function gtag(){...} definition. Cross-page byte-identity confirmed via diff of the three marker-bounded blocks against index.html — all three blocks (gtag, consent, consent-css) are identical across all 4 pages.
Verdict
REQUEST CHANGES (submitted as COMMENT due to GitHub self-PR restriction) — red CI on site-counts drift detection. Refresh the four meta-tag values listed above, push, CI should go green. Everything else in the diff is clean.
Reviewed by Rex (Code Reviewer Agent)
Reviewed commit: e7ceb2eb47b4ff6c2b7cdb58817d04a2b5898294
The GA4 + consent block additions in the prior commit grew each page by ~5–6K chars. Refreshing the per-page LLM payload-size meta tags on all 4 site files (not just the 2 the test_site_counts.sh loop scans) so token-budget-aware clients see accurate costs. - index.html: 23189 → 23697 tok / 92757 → 94791 chars - how-it-works.html: 6572 → 8065 tok / 26291 → 32263 chars - architecture.html: 8864 → 10201 tok / 35458 → 40807 chars - skills.html: 10658 → 11920 tok / 42632 → 47681 chars Verified: test_site_counts.sh PASS — all scanned files within 5% of actual. Refs #399
atlas-apex
left a comment
There was a problem hiding this comment.
Re-review: PR #400
Commit: 399f605c00dba3d99b766681d016c17d7742254b
Summary
Fixup commit 399f605 addresses the sole blocker from the prior REQUEST_CHANGES (the red site-counts drift detection CI). The operator went beyond the minimal fix — refreshing meta tags on all 4 site pages, not just the 2 the loop currently scans. CI is green; drift on the new HEAD is effectively 0%.
Checklist Results
- ✅ Architecture & Design: N/A (static HTML/CSS/JS copy-paste, no architectural surface)
- ✅ Code Quality: Pass
- ✅ Testing: Pass (
site-counts drift detectionCI green; local verification confirms 0% drift) - ✅ Security: Pass (no auth / crypto / secrets touched; Consent Mode v2 defaults
denied, gated on explicit grant) - ✅ Performance: N/A
- ✅ PR Description & Glossary: Pass
- ⚠ Summary Bullet Narrative: Pass (all bullets explain what + why)
- ✅ Technical Decisions (AgDR):N/A (skip marker present + valid — pure copy-paste of pre-existing implementation)
- ✅ Adopter Handbooks: N/A (none load for this diff)
Drift verification on new HEAD
Local measurement against 399f605:
| File | Actual chars | Declared chars | Drift |
|---|---|---|---|
site/index.html |
94,791 | 94,791 | 0.00% |
site/how-it-works.html |
32,263 | 32,263 | 0.00% |
site/architecture.html |
40,808 | 40,807 | 0.00% |
site/skills.html |
47,681 | 47,681 | 0.00% |
Loop tolerance is 5%; observed drift is effectively zero across all 4 files (1-byte discrepancy on architecture.html is the literal \n count rounding the chars-divided-by-4 token estimate by one — well inside tolerance).
Earlier PASS findings still hold
- Scope unchanged — diff still confined to exactly 4
site/*.htmlfiles; no scope leak into framework code (hooks / skills / rules / agents) - GA4 + consent blocks byte-identical across all 4 pages:
gtagblock sha256:f53ea9ed…(all 4 pages identical)consent-cssblock sha256:5549da23…(all 4 pages identical)consentblock sha256:1759b8de…(all 4 pages identical)
anonymize_ipabsent —grep -c anonymize_ip site/*.htmlreturns 0 across the board- Begin/end markers paired — 3 begin + 3 end per file (gtag / consent / consent-css)
- Glossary, summary bullets, skip markers — unchanged from prior review; still pass
Fixup-commit hygiene
- Only 8 lines changed in
399f605(4 files × 2 lines each — exactly the meta-tag pair) - No unintended churn; commit message accurately describes what changed and acknowledges the over-scope (4 files updated, not just the 2 the loop scans — operator chose accuracy over minimal compliance, which is the right call)
Issues Found
None.
Suggestions
- nit (carryover from prior review) — the
test_site_counts.shloop is missinghow-it-works.html. The operator already noted this is out of scope; worth filing as a follow-up so the next meta-tag refresh PR doesn't have to remember to update the unscanned files manually. - nit (cosmetic) — PR body Testing block reports
gtag-refs=6per file; observed count isgtag-refs=12. Likely a stale pre-fixup figure. Not a real issue (the parity property — same count across all 4 files — holds), but worth a one-line update if the author touches the description again.
Verdict
APPROVED
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: 399f605c00dba3d99b766681d016c17d7742254b
…h#400) * fix(me2resh#399): site/ — GA4 + consent banner on all 4 pages - Copy gtag.js + Consent Mode v2 init + cookie consent banner from index.html to how-it-works.html, architecture.html, skills.html. - Wrap each block with begin/end markers so future sync edits are greppable across pages. - Remove dead `anonymize_ip: true` from gtag config (no-op in GA4 — all IPs auto-anonymized; carryover from Universal Analytics). Closes me2resh#399 * fix(me2resh#399): refresh llm:token-count + llm:doc-length meta tags The GA4 + consent block additions in the prior commit grew each page by ~5–6K chars. Refreshing the per-page LLM payload-size meta tags on all 4 site files (not just the 2 the test_site_counts.sh loop scans) so token-budget-aware clients see accurate costs. - index.html: 23189 → 23697 tok / 92757 → 94791 chars - how-it-works.html: 6572 → 8065 tok / 26291 → 32263 chars - architecture.html: 8864 → 10201 tok / 35458 → 40807 chars - skills.html: 10658 → 11920 tok / 42632 → 47681 chars Verified: test_site_counts.sh PASS — all scanned files within 5% of actual. Refs me2resh#399 --------- Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
…h#402) - Append [2.0.2] section above [2.0.1] - Bump site/index.html version pill v2.0.1 → v2.0.2 - Site-only release; framework unchanged - Underlying fix: me2resh#399 / PR me2resh#400 (GA4 + consent on all 4 pages) Refs me2resh#401 Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
…sh-divergence 8 past release squash-commits (v1.2.0→v2.2.0 + site hotfix #400) were never synced back into dev, leaving main↔dev diverged on ~50 files. dev is the verified content superset (CHANGELOG complete, GA4/consent present, deletions are dev-intentional), so we keep dev's tree and record main as an ancestor to clear the merge conflict. Real release-sync follows. Refs #508
…h#400) * fix(me2resh#399): site/ — GA4 + consent banner on all 4 pages - Copy gtag.js + Consent Mode v2 init + cookie consent banner from index.html to how-it-works.html, architecture.html, skills.html. - Wrap each block with begin/end markers so future sync edits are greppable across pages. - Remove dead `anonymize_ip: true` from gtag config (no-op in GA4 — all IPs auto-anonymized; carryover from Universal Analytics). Closes me2resh#399 * fix(me2resh#399): refresh llm:token-count + llm:doc-length meta tags The GA4 + consent block additions in the prior commit grew each page by ~5–6K chars. Refreshing the per-page LLM payload-size meta tags on all 4 site files (not just the 2 the test_site_counts.sh loop scans) so token-budget-aware clients see accurate costs. - index.html: 23189 → 23697 tok / 92757 → 94791 chars - how-it-works.html: 6572 → 8065 tok / 26291 → 32263 chars - architecture.html: 8864 → 10201 tok / 35458 → 40807 chars - skills.html: 10658 → 11920 tok / 42632 → 47681 chars Verified: test_site_counts.sh PASS — all scanned files within 5% of actual. Refs me2resh#399 --------- Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
…h#402) - Append [2.0.2] section above [2.0.1] - Bump site/index.html version pill v2.0.1 → v2.0.2 - Site-only release; framework unchanged - Underlying fix: me2resh#399 / PR me2resh#400 (GA4 + consent on all 4 pages) Refs me2resh#401 Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
Summary
gtag.js+ Consent Mode v2 default block copied verbatim fromindex.htmlintohow-it-works.html,architecture.html, andskills.html. Pre-fix, only/was tracked; any visit landing directly on the 3 other pages (Twitter/LinkedIn shares from the v2.0 launch, search results, LLM citations fromllms.txtwhich explicitly lists all 4) was invisible to GA4 and — worse — got no consent UI at all (a GDPR gap, not just a measurement gap).localStorage.ay-consentpersistence. A user who lands on/how-it-worksfirst now gets the consent choice on that page, and the choice is honoured site-wide on subsequent navigation (same localStorage key).<!-- begin: gtag -->/<!-- end: gtag -->,<!-- begin: consent -->/<!-- end: consent -->, and/* begin: consent-css *///* end: consent-css */. Static-HTML site has no build step, so a shared partial doesn't exist; the markers make it greppable when the GA4 ID, Consent Mode config, or banner copy needs to change in one place.'anonymize_ip': truefromgtag('config', ...). This was a Universal Analytics flag that truncated the last octet of the IP; in GA4 all IPs are auto-anonymized by default and the parameter is a no-op. Removing it kills dead config without changing behaviour.Testing
grep anonymize_ip site/*.htmlreturns 0 hits across the board (dead config removed)git diff --name-only origin/main..HEADreturns exactly 4 site files; no scope leak into hooks / skills / rules / agentsscript-src 'self' 'unsafe-inline' https://www.googletagmanager.com+img-src ... https://www.google-analytics.com+connect-src ... https://analytics.google.com); no CSP changes neededanalytics_storage/how-it-works,/architecture,/skills(currently flat-zero on those paths)AgDR note
<!-- agdr: not-applicable -->marker present at the top: this PR is a pure HTML/CSS/JS copy-paste with no architectural decisions, no library choices, no new dependencies. The framework's existing Consent Mode v2 implementation is being applied to additional pages, not changed.Leak-protection skip note
<!-- private-refs: allow -->marker present at the top: the body legitimately referencesyard.apexscript.com, the framework's own public marketing domain. The leak-protection hook trips on theapexscripttoken because of name-collision with a registered project (a recurring false-positive — same exception as the v2.0.0 launch PRs).Out of scope (deliberately deferred)
policies.google.com/privacy(Google's policy). For full GDPR compliance the site should host its own privacy policy explaining what data the framework's marketing site collects, who it shares with, and how to request deletion. Separate ticket — out of scope for an analytics-coverage fix.Glossary
G-G3EMR3CB02for this site.denied/granted) before any tracking fires. Required for sites serving EU traffic post-DMA.analytics_storagedenied; flipped tograntedafter the user clicks Accept.anonymize_ip: truelocalStorage.ay-consentgranted/denied). One key for the whole site, so a choice made on one page is honoured on all subsequent navigation.<!-- begin: gtag --> ... <!-- end: gtag -->etc.) wrapping each cross-page block. Greppable anchors for future sync edits — no build step to do it automatically.upstream/main(NOT dev), commit the fix, PR to main. Avoids the v1.4→v2.0 squash divergence on dev. Same pattern as the v2.0.1 mobile UX fix (PR #396).Closes #399