Fix Ads conversion value, gate on Measurement ID, unify payloads, add events and tests

[next-devin]

Handoff from the review filed as #2 to #7. Each issue maps to a bullet below; the tracker is rewritten around one guarded send() and one item builder so the fixes are easy to check line by line.

What changes

• Google Ads conversion reports tax as the value and the AW tag is never configured #2 Google Ads: conversion value is total_incl_tax (was total_tax). The snippet now runs gtag('config', '<AW-id>') when Ads tracking is on, which send_to needs to attribute. Help text asks for the AW- prefix, matching how send_to is built.
• Enabling the app without a Measurement ID injects a broken gtag on every page #3 Empty ID: snippet and tracker are gated on google_analytics_measurement_id being non-empty. send() checks window.top.gtag exists before calling, so themes without the global_header hook do not throw in the tracker frame.
• Debug mode is ignored for logged-in shoppers #4 Debug mode: user_id and debug_mode are one config object.
• Ecommerce payloads use string numbers and an inconsistent item shape across the funnel #5 Payloads: all money fields go through parseFloat; price and discount are per unit (line total ÷ quantity, rounded to cents); every event uses the same item shape (item_id = product id, sku, item_variant, item_category where the payload has it); page fields are no longer inside items[] (gtag on the top window collects them).
• Map the platform's newer storefront events to GA4 (view_item_list, remove_from_cart, add_shipping_info, add_payment_info) #6 Coverage: product_category_viewed → view_item_list, product_removed_from_cart → remove_from_cart, checkout_shipping_method_submitted → add_shipping_info (with shipping_tier). New "Skip Test Orders" setting, default off so existing installs do not change behaviour.
• README and settings labels still say 29 Next and Adwords #7 Naming: labels, help text, README.

Deliberately not included

• add_payment_info: the platform emits no payment-step event.
• Consent Mode v2 defaults: sending denied defaults without a CMP to update them would blank out measurement for every merchant, so it needs its own design.

How to verify

• npm test runs tracking.js under the platform's globals (app, analytics, window.top) with payloads from the event-tracking docs and asserts every mapped event. CI workflow added.
• Live check: install on a test store with a Measurement ID and "Enable Debug Mode", then walk PDP → add to cart → checkout → shipping → purchase in GA4 DebugView. I could not push the app build myself (needs the app's App Kit credentials), so this is the reviewer's step.

Closes #2, closes #3, closes #4, closes #5, closes #6, closes #7

🤖 Generated with Claude Code

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

This incremental push is documentation-only: the README now spells out that value is the sum of the lines' price_excl_tax (with the tax-inclusive fallback reserved for payloads missing the field), and the block comment above lineTotal is expanded to describe the same fallback plus the zero-price and total_discount semantics. No code, settings, or tests changed since d643ea4, and the expanded comment is consistent with the implementation on lines 107-110 and the fixture-driven tests.

Files Reviewed (2 incremental files)

• README.md - 0 issues
• tracking.js - 0 issues

Previous Review Summaries (7 snapshots, latest commit d643ea4)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit d643ea4)

Status: No Issues Found | Recommendation: Merge

This incremental push introduces price_excl_tax as the preferred price source so line totals and unit prices exclude tax when the payload provides it; price_incl_tax remains a fallback. Behaviour reconciles across cartLineItem, checkoutEcommerce, and cartLineEvent, the purchase.tax and Ads conversion.value are now sourced from the test fixture's total_tax and total_incl_tax, and the tests assert the new values (159.98 + 39.99 = 199.97 item revenue, 14 tax, 218.96 Ads conversion).

Files Reviewed (4 incremental files)

• CHANGELOG.md - 0 issues
• README.md - 0 issues
• tracking.js - 0 issues
• tests/tracking.test.js - 0 issues

Previous review (commit c9956c6)

Status: No Issues Found | Recommendation: Merge

The two outstanding items from the previous review are fixed in this push:

• tracking.js:78-79 — comment now correctly describes truncation (rather than "dropped, not truncated").
• tracking.js:155-164 — product_category_viewed captures the currency from the first priced product in the full list, before applying the MAX_ITEMS slice, so an over-limit list no longer loses its currency. The paired test assertion (tests/tracking.test.js:122) now expects 'USD', matching the new behaviour.

CHANGELOG.md was updated consistently (line 5 says the tracker configures the AW- tag; line 11 says lists are "truncated" to GA4's 200-item limit).

Files Reviewed (4 incremental files)

• CHANGELOG.md - 0 issues
• tests/tracking.test.js - 0 issues
• tracking.js - 0 issues
• (no other files changed since 56e9501)

Previous review (commit 56e9501)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
tracking.js	78	Comment says over-limit events are "dropped, not truncated", but slice(0, MAX_ITEMS) ships a partial event (test on tests/tracking.test.js:113-129 already shows currency becoming undefined for an over-limit list whose priced product is past index 200).

SUGGESTION

File	Line	Issue
CHANGELOG.md	5	Says the AW- tag is configured in the snippet, but line 11 (added in the same PR's latest follow-up) says it is configured by the tracker. Patch 6 removed the snippet-side gtag('config', 'AW-...') — this bullet is stale.

Previously Reported — Resolved in This Push

All three findings from the previous review are fixed:

• snippets/global-header.html — Django cut filter line removed; the snippet no longer configures the Ads tag (test tests/tracking.test.js:216-223 confirms no adwords reference remains).
• snippets/global-header.html — gate mismatch closed by moving the Ads config into tracking.js (single source of truth on adsId / adsLabel).
• tracking.js:21 — adsLabel now validates as /^[A-Za-z0-9_-]+$/; tests cover Abc/Def, ab'c, and a valid AbC_d-9.

Also addressed in this push: window.parent inside try/catch so cross-origin parents never throw; send_to set on every GA4 event; checkout value rebuilt from the lines (item revenue) while shipping/tax keep their own params; coupon omitted when absent; lines without a product dropped; item lists capped at 200; checkout events without a payload dropped; new startup gtag('config', adsId) only fires when the settings validate.

Files Reviewed (5 incremental files)

• CHANGELOG.md - 1 issue
• README.md - 0 issues
• snippets/global-header.html - 0 issues (previously reported issues resolved)
• tests/tracking.test.js - 0 issues (over-limit behaviour already covered by view_item_list test, but documents the truncation the comment disclaims)
• tracking.js - 1 issue

Fix these issues in Kilo Cloud

Previous review (commit 56a6421)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	2
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
snippets/global-header.html	17	Django's cut filter removes every occurrence of AW-, so inputs like aw-123-aw-456 produce a malformed id (AW-123-456) — diverges from tracking.js which strips only the prefix.
snippets/global-header.html	16	Snippet accepts any truthy trimmed id (e.g. aw123 → AW-AW123 in gtag('config', ...)), while the tracker strictly requires /^\d+$/. The snippet configures an Ads tag whose events the tracker would later drop.

SUGGESTION

File	Line	Issue
tracking.js	21	adsLabel is only trim()-validated; a label containing /, ', or whitespace produces a malformed send_to: adsId + '/' + adsLabel and the Ads conversion silently fails to attribute.

Files Reviewed (4 incremental files)

• CHANGELOG.md - 0 issues
• README.md - 0 issues
• snippets/global-header.html - 2 issues
• tests/tracking.test.js - 0 issues
• tracking.js - 1 issue

Fix these issues in Kilo Cloud

Previous review (commit 9cd6842)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	1
WARNING	0
SUGGESTION	0

Issue Details (click to expand)

CRITICAL

File	Line	Issue
snippets/global-header.html	1	.strip is not a Liquid string method — google_analytics_measurement_id.strip evaluates to nil, so the entire {% if %} block is skipped and window.top.gtag is never installed. send() in tracking.js then silently no-ops every event. Use the Liquid filter syntax `

Files Reviewed (4 incremental files)

• README.md - 0 issues
• snippets/global-header.html - 1 issue
• tests/tracking.test.js - 0 issues
• tracking.js - 0 issues

Fix these issues in Kilo Cloud

Previous review (commit 3c9d8ec)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (3 incremental files)

• snippets/global-header.html - 0 issues
• tests/tracking.test.js - 0 issues
• tracking.js - 0 issues

Previously Reported — Addressed in this push

The outstanding SUGGESTION from the prior review (filter only excluded undefined, letting null through to String(product.id)) is resolved in commit 3c9d8ece:

• tracking.js line 82 — filter now uses product.id != null, catching both null and undefined.
• tracking.js lines 95–97 — product_viewed now bails when the payload or its id is missing, so neither 'null' nor 'undefined' can reach item_id.
• tests/tracking.test.js — category test now covers a { id: null } entry, and a new test asserts view_item is skipped for undefined and id-less payloads.

Previous review (commit 85e4ab9)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
tracking.js	82	New product_category_viewed filter uses product.id !== undefined, which lets null through; String(null) ships as item_id: 'null' to GA4. Use product.id != null and mirror the guard in product_viewed (line 95) where event.data || {} still emits item_id: 'undefined' on missing payloads.

Previously Reported — Addressed in this push

All 7 prior findings were resolved in commit bd759692:

• snippets/global-header.html — |escapejs / |urlencode added; JS-context XSS surface closed.
• tracking.js line 39 — null entries filtered out of product_category_viewed before mapping.
• tracking.js line 41 — multi-variant PDPs now omit sku (single-variant fallback retained).
• tracking.js line 78 — view_item_list now carries item_list_id (stable page path) alongside the title.
• tracking.js line 143 — AW-<digits> regex validates the conversion id; bare numeric ids no longer fire conversion.
• tracking.js line 50 — unit price/discount left undefined when quantity is not a positive number; test added.
• tests/tracking.test.js line 95 — begin_checkout test added with multi-line cart, coupon, value, and item-shape assertions.

Files Reviewed (3 incremental files)

• snippets/global-header.html - 0 new issues (previously reported XSS closed)
• tests/tracking.test.js - 0 new issues (new tests cover all addressed behaviors)
• tracking.js - 1 issue

Fix these issues in Kilo Cloud

---

_{Reviewed by minimax-m3 · Input: 42.8K · Output: 1.5K · Cached: 271.7K}

[next-devin]

Second review pass (gstack: 2 specialists + Claude and Codex adversarial passes, verified read-only against oscar-prime)

Pushed as one commit on top of the Kilo fixes:

• window.parent, not window.top. The platform's tracker iframe is a direct child of the storefront (pixels/customer_event_iframes.html uses parent.customerEventManager). With top, an embedded storefront (theme preview, a page framing the store) is cross-origin and every handler threw. Now every parent access is inside try/catch; tests boot with a null parent and a throwing proxy.
• Checkout value is item revenue. GA4 defines value on begin_checkout/add_shipping_info/purchase as the sum of price × quantity; total_incl_tax is the grand total, so revenue was inflated by shipping and tax on every order. Value is now rebuilt from the lines; shipping/tax stay in their own params; the Ads conversion keeps the order total by design (README says so).
• send_to on every GA4 event, so a second Google tag on the page does not also receive them.
• Ads id normalised identically in snippet and tracker: strip, upper-case, bare digits get the AW- prefix, label trimmed. This also removes the upgrade risk where a store with a bare numeric id would silently lose conversions.
• Guards: coupon omitted when absent (was "" → "(not set)"), checkout lines without a product dropped, item lists capped at GA4's 200-item limit, checkout events without a payload dropped.
• Tests: 16, including a string-level guard that every {{ }} inside the snippet's script block ends in escapejs/urlencode.

Two things are deliberately not here: purchase dedupe (platform-owned, pixels/generators.py one-hour cache guard), and whether total_tax includes shipping tax (platform question).

Process, per the app-development-flow docs

This PR is the code review. Distribution is App Kit: nak build && nak push from the app directory pushes to the Partner account and auto-deploys to the app's development store; installed stores only update after a semver Release is created on the app's page in the Partner account. nak setup needs the Partner account password, so that step is yours. Suggested release version: 2.0.0 (event payload semantics change).

[next-devin]

Release-candidate walk on the development store (aptest.29next.store, 2026-09-03)

Deployed with nak push; trackers live as customer events 137 (GA4) and 138 (GTM). Events read from window.dataLayer on the storefront (gtag pushes its arguments there; the GTM tracker pushes directly). Two dummy orders placed with the "Pay with a check" method.

Storefront event	GA4 (gtag)	GTM (dataLayer)
product_viewed → view_item	value 59.99, item_id 111, sku, item_category, send_to	same, after ecommerce: null
product_category_viewed → view_item_list	2 items, item_list_id = path, currency USD	same
product_added_to_cart → add_to_cart	value 49.99, unit price 49.99, discount 10	same
product_removed_from_cart → remove_from_cart	value 109.98, quantity 2	same
checkout_started → begin_checkout	value = Σ lines, coupon absent when none	same
checkout_shipping_method_submitted → add_shipping_info	shipping_tier "Express"	same
checkout_completed → purchase (order 110986)	value 59.99, shipping 7.99, tax 4.2, transaction_id 110986	same

Order 110985 (before the last commit) showed value 64.19 because the order line's price_incl_tax includes tax while tax was also sent; the last commit switches line prices to price_excl_tax with an incl fallback, and order 110986 confirms 59.99.

Not exercised on this store: the Ads conversion (no Ads id configured on aptest; covered by unit tests). Side findings during the walk, filed separately: NextCommerceCo/klaviyo#166 (Klaviyo tracker throws on the first add of a session) and NextCommerceCo/oscar-prime#5767 (one throwing tracker blocks the rest). The store also needed {% pixels %} added to its Spark theme before any tracker loaded (Spark fix in progress; Velin fixed in Sellmore-Co/velin-theme#3).