apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsx (2)

141-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Include errorCode in the analytics event.

onError receives errorCode and logs it, but doesn't pass it to trackCaptcha. Including it as a custom param would make captcha_challenge_failed events with reason: 'turnstileError' actionable in analytics dashboards without cross-referencing logs.

♻️ Pass errorCode to trackCaptcha

       onError={(errorCode) => {
         exchangeRequestIdRef.current += 1

         logCaptcha.error('Challenge errored', { errorCode, hostname: window.location.hostname })
-        trackCaptcha({ action: 'captcha_challenge_failed', reason: 'turnstileError' })
+        trackCaptcha({ action: 'captcha_challenge_failed', reason: 'turnstileError', errorCode })
         setCaptchaJwt(null)
       }}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsx`
around lines 141 - 147, The CaptchaWidget.container.tsx onError handler logs
errorCode but does not send it with the captcha analytics event, so the failure
data is incomplete. Update the onError callback in CaptchaWidget.container to
pass errorCode through to trackCaptcha as an additional custom param alongside
action and reason, keeping the existing logCaptcha.error call unchanged.

---

29-31: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Consider memoizing trackCaptcha.

createCowTracker is called in the component body, producing a new closure on every render. Since Turnstile uses key={siteKey}, callbacks are refreshed on siteKey change regardless, so this is not a bug — but wrapping in useMemo keyed on siteKey avoids needless allocations and makes the enabled-dependency explicit.

♻️ Wrap in useMemo

+  const trackCaptcha = useMemo(
+    () => createCowTracker(CowSwapAnalyticsCategory.CAPTCHA, {
+      enabled: siteKey !== TURNSTILE_DEMO_INTERACTIVE_SITE_KEY,
+    }),
+    [siteKey]
+  )
-  const trackCaptcha = createCowTracker(CowSwapAnalyticsCategory.CAPTCHA, {
-    enabled: siteKey !== TURNSTILE_DEMO_INTERACTIVE_SITE_KEY,
-  })

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsx`
around lines 29 - 31, The `trackCaptcha` callback in `CaptchaWidget.container`
is recreated on every render because `createCowTracker` is called directly in
the component body. Wrap the `createCowTracker(CowSwapAnalyticsCategory.CAPTCHA,
{ enabled: siteKey !== TURNSTILE_DEMO_INTERACTIVE_SITE_KEY })` call in
`useMemo`, keyed on `siteKey`, so the closure is only rebuilt when the site key
changes and the `enabled` dependency is explicit.

libs/analytics/src/CowAnalytics.ts (1)

3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant union member in AnalyticsEvent.

EventOptions & Record<string, unknown> is assignable to EventOptions, so the third union member adds no new type-level coverage. The union collapses to string | EventOptions. If the intent is to signal that arbitrary extra keys are allowed, a comment or a dedicated branded type would be clearer than a redundant intersection.

♻️ Simplify the union

-export type AnalyticsEvent = string | EventOptions | (EventOptions & Record<string, unknown>)
+export type AnalyticsEvent = string | EventOptions

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libs/analytics/src/CowAnalytics.ts` at line 3, Simplify the AnalyticsEvent
type by removing the redundant EventOptions & Record<string, unknown> union
member, since it is already covered by EventOptions. Update the exported type in
CowAnalytics to use only string | EventOptions, and if extra arbitrary fields
are intended, make that intent explicit elsewhere with a clearer type or comment
rather than keeping an equivalent union branch.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Finding: [QUESTION] Should custom analytics params protect GTM’s reserved names?

• Location: libs/analytics/src/gtm/CowAnalyticsGtm.ts:264
• I do not see a current regression in this PR: the new captcha/affiliate events use custom fields like reason, result, and failureReason.
• The edge case is the new generic custom-param path. GTM uses event as the event name, and this code builds official fields first, then spreads getAdditionalEventParams(...) last. A future caller passing { event: 'wrong_name' } could overwrite the canonical event: event.action.
• This pattern is not new only in this PR: toGtmEvent() has a similar “official fields first, extra fields last” shape. The difference is that this PR makes arbitrary custom params a first-class path through AnalyticsEvent / createCowTracker, so this may be a good moment to define the reserved-key behavior centrally.

Suggested fix

• Filter reserved GTM output keys like event from custom params, or spread custom params before the official GTM fields so official fields always win.
• Add a small CowAnalyticsGtm unit test showing { action: 'captcha_challenge_failed', event: 'wrong_name' } still sends event: 'captcha_challenge_failed'.

Verify this finding against current code. Fix only if still valid, keep the change minimal, and validate with a targeted unit test.

Context:
- libs/analytics/src/CowAnalytics.ts now allows custom params through AnalyticsEvent.
- libs/analytics/src/createCowTracker.ts forwards arbitrary custom params.
- libs/analytics/src/gtm/CowAnalyticsGtm.ts spreads getAdditionalEventParams(gtmEvent) after canonical fields, so a custom `event` key could override the GTM event name.
- toGtmEvent() has a similar shape, so prefer a central reserved-key policy if practical.

Generated using the pr-review skill from the CoW Protocol skills repo.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes