Skip to content

Plan: Recover missing conversion_attributions rows (Fixes 1 + 3 + 6)

Status: Shipped. Core plan implemented in PR #727. One post-ship change: ConversionAttribution.sourceCode column was designed but never reached prod and was dropped in PR #730 — MMS codes route through utm_content instead. References to sourceCode in the carry-forward data block below reflect the original design and do not match the current schema.

Context

Per docs/fix-missing-attribution-rows/investigation.md, the conversion_attributions table holds 35 rows in prod against months of traffic. 0 of 18 truly self-serve MONTHLY lockouts since 2026-04-01 captured attribution. Two structural bugs:

  1. writeOrCarryForwardAttribution writes useless fbp-only rows from the Meta Pixel cookie and short-circuits the carry-forward path that would otherwise inherit real attribution from a prior INQUIRY. (Cause 1.)
  2. Carry-forward only inherits from INQUIRY. 14 of 18 self-serve lockouts skipped the contact form entirely, so there's no INQUIRY to carry from — even when prior TOUR/HOURLY/etc. attribution rows exist. (Cause 3.)

Both failures are silent — no Sentry signal, no log, no metric. (Cause 6.)

Decisions locked with Aaron during interview:

  • fbp-only never gets persisted in conversion_attributions. CAPI already reads fbp directly from session.metadata in trackPurchaseEventToMeta, independent of this table — so removing fbp-only rows costs us nothing operationally.
  • Carry-forward lookback: no limit. Use the most recent prior attribution row regardless of age.
  • Migration-tenant users get no ad attribution. If user_id has any migration_submissions row, skip the write entirely with an info-level signal.
  • Out of scope (deferred): Fix 2 (gclid first-touch on User), Fix 4 (delete hasPersistableAttribution — incidentally removed since it becomes unused), Fix 5 (UTM localStorage TTL), Fix 7 (cleanup of 11 historical bad rows, CHECK constraint, audit trigger).

Intended outcome (acceptance criteria from investigation doc, restated): self-serve MONTHLY lockout coverage ≥ 50% one week post-deploy, zero new fbp-only rows, every silent-skip point emits a diagnosable Sentry event.

Files to modify

File Change
apps/api/src/services/shared/AttributionService.ts Core changes: new predicate, broaden carry-forward, migration check, Sentry signals
apps/api/src/services/shared/AttributionService.test.ts Add 6 tests (listed below)
apps/api/tests/api/.../*.test.ts (existing) Mock Sentry.captureMessage in any test that exercises attribution paths and currently doesn't — minimal touches

No schema changes. No frontend changes. No new files.

Implementation

Step 1 — Add hasRealAdAttribution and replace the short-circuit predicate

In AttributionService.ts:

  1. Add helper alongside existing hasAttributionData (line 105):
function hasRealAdAttribution(attribution: Attribution): boolean {
  return !!(
    attribution.utmSource ||
    attribution.utmMedium ||
    attribution.utmCampaign ||
    attribution.utmTerm ||
    attribution.utmContent ||
    attribution.fbclid ||
    attribution.gclid
  )
}

Distinct from hasAttributionData (which also accepts fbp/fbc). hasAttributionData stays — extractAttributionFromStripeMetadata uses it to detect "any metadata at all" before assembling the object, which is the right semantic there.

  1. Delete hasPersistableAttribution (lines 120–132). It is byte-identical to hasAttributionData, has been since PR #721 (Cause 4), and becomes structurally unused after Step 2. Removing it is implicit cleanup, not Fix 4 scope creep.

  2. Update writeOrCarryForwardAttribution (lines 294–307) to use the new predicate AND check migration status:

export async function writeOrCarryForwardAttribution(tx, key, sourceType, sourceId, stripeMetadata) {
  if (key.userId && (await isMigrationUser(tx, key.userId))) {
    Sentry.captureMessage('Attribution skipped: migration user', {
      level: 'info',
      tags: { service: 'attribution', operation: 'writeOrCarryForward', reason: 'migration_user', sourceType },
      extra: { userId: key.userId, sourceId, sourceType },
    })
    return
  }
  const directAttribution = extractAttributionFromStripeMetadata(stripeMetadata)
  if (directAttribution && hasRealAdAttribution(directAttribution)) {
    await writeConversionAttribution(tx, sourceType, [sourceId], directAttribution)
    return
  }
  // Fall through to carry-forward (metadata may have only fbp/fbc, or nothing)
  await carryForwardAttribution(tx, key, sourceType, sourceId, { directAttribution, stripeMetadata })
}
  1. Update writeConversionAttribution (lines 154–173) to gate persistence with hasRealAdAttribution (replacing the current hasAttributionData + hasPersistableAttribution double-guard). This stops the direct-write paths (/api/contact, /api/leads/email-capture, /api/promo/redeem, /api/comp/reservations, ReservationCreationService for TOUR) from also writing fbp-only rows. Same predicate everywhere is the simplest, most consistent semantics.

Step 2 — Add isMigrationUser helper

In AttributionService.ts, add module-private helper:

async function isMigrationUser(tx: TransactionClient, userId: string): Promise<boolean> {
  const ms = await tx.migrationSubmission.findFirst({ where: { userId }, select: { id: true } })
  return ms !== null
}

MigrationSubmission has @@index([userId]) per schema line 818 — single indexed lookup, cheap.

Step 3 — Broaden carry-forward to all source types

Replace findPriorInquiry (lines 214–227) with findPriorAttributionForUser:

async function findPriorAttributionForUser(
  tx: TransactionClient,
  userId: string
): Promise<{ row: ConversionAttribution; sourceType: AttributionSourceType } | null> {
  // Gather all source IDs owned by this user across attribution-producing tables
  const [inquiries, reservations, promoRedemptions, creditBalances] = await Promise.all([
    tx.inquiry.findMany({ where: { userId, deletedAt: null }, select: { id: true } }),
    tx.reservation.findMany({ where: { userId, deletedAt: null }, select: { id: true } }),
    tx.promoRedemption.findMany({ where: { userId }, select: { id: true } }),
    tx.creditBalance.findMany({ where: { userId }, select: { id: true } }),
  ])

  const inquiryIds = inquiries.map((r) => r.id)
  const reservationIds = reservations.map((r) => r.id)
  const promoIds = promoRedemptions.map((r) => r.id)
  const creditIds = creditBalances.map((r) => r.id)

  // Single query: most recent attribution row across all of this user's sources
  // that has real ad attribution (utm/fbclid/gclid, not just fbp/fbc).
  const row = await tx.conversionAttribution.findFirst({
    where: {
      AND: [
        {
          OR: [
            inquiryIds.length ? { sourceType: 'INQUIRY', sourceId: { in: inquiryIds } } : undefined,
            reservationIds.length
              ? { sourceType: { in: ['TOUR', 'HOURLY', 'MONTHLY', 'WAITLIST'] }, sourceId: { in: reservationIds } }
              : undefined,
            promoIds.length ? { sourceType: 'PROMO_REDEMPTION', sourceId: { in: promoIds } } : undefined,
            creditIds.length ? { sourceType: 'CREDIT_PURCHASE', sourceId: { in: creditIds } } : undefined,
          ].filter(Boolean),
        },
        {
          OR: [
            { utmSource: { not: null } },
            { utmMedium: { not: null } },
            { utmCampaign: { not: null } },
            { utmTerm: { not: null } },
            { utmContent: { not: null } },
            { fbclid: { not: null } },
            { gclid: { not: null } },
          ],
        },
      ],
    },
    orderBy: { createdAt: 'desc' },
  })

  return row ? { row, sourceType: row.sourceType } : null
}

Validate the exact Reservation source-types list against the AttributionSourceType enum in the Prisma schema before final implementation — keep this list in sync.

Update carryForwardAttribution (lines 229–292):

  1. If userId missing → return (existing).
  2. NEW: If isMigrationUser(tx, userId) → emit info signal, return.
  3. Call findPriorAttributionForUser(tx, userId). If found → copy fields, source = 'prior-attribution-carry-forward', write row, fire alert.
  4. Else if userFbclidFirstTouch → existing rescue path (unchanged), source = 'user-fbclid-rescue'.
  5. Else → NEW: emit warning signal "no prior attribution found", return.

Keep the existing email-based fallback semantics: findPriorAttributionForUser queries by userId only (sufficient — by the time fulfillment / TOUR flow runs, the user record exists). The old findPriorInquiry did an email OR userId match; in the new world, every relevant caller has userId set.

AdAttributionSource (line 11) gains 'prior-attribution-carry-forward'; keep 'inquiry-carry-forward' as an alias OR map it through to the new value (decide at implementation time; doesn't affect external behavior).

Step 4 — Sentry signals

Four signal points, all using Sentry.captureMessage directly (codebase has no addBreadcrumb usage; do not introduce). Pattern mirrors CreditTransferService.ts:356, 434.

Point Level Tag reason Extra
writeOrCarryForwardAttribution: migration user info migration_user userId, sourceId, sourceType
writeOrCarryForwardAttribution: fell through, metadata had only fbp/fbc info metadata_only_fbp_fbc userId, sourceId, sourceType, metadataPresent (object of booleans)
carryForwardAttribution: no prior attribution and no fbclidFirstTouch warning no_prior_attribution userId, sourceId, sourceType, hasUserEmail, userCreatedAt
carryForwardAttribution: rescued via user.fbclidFirstTouch (no prior attribution row) info rescued_fbclid_first_touch userId, sourceId, sourceType

Common tags on all four: service: 'attribution', operation: 'writeOrCarryForward' or 'carryForward', sourceType.

PII rule (from observed pattern in reconcile-auth-users): extra carries IDs and booleans only. No raw email, no raw fbclid/gclid token values. Use metadataPresent: { hasUtmSource: boolean, hasFbclid: boolean, hasGclid: boolean, hasFbp: boolean, hasFbc: boolean }. The Stripe paymentIntentId (or session/setupIntent id) goes in extra.stripeRef so a future agent can fetch raw metadata via Stripe API if needed — pass it in from each caller of writeOrCarryForwardAttribution.

This requires extending the writeOrCarryForwardAttribution signature with an optional diagnostic?: { stripeRef?: string } parameter. Callers in FulfillmentService.ts (lines 132, 252, 557, 711) and PaymentIntentHandler.ts (lines 234, 335) get a small update to pass paymentIntent.id / session.id / subscription.id. Trivial diff per call site.

Step 5 — Tests

Add to AttributionService.test.ts:

  1. writeOrCarryForwardAttribution: falls through to carry-forward when metadata has only fbp — given a user with a prior INQUIRY row containing UTMs, and current metadata { fbp: '...' }, asserts a new row is written for the MONTHLY source with the prior UTMs copied (not the fbp-only data).
  2. writeOrCarryForwardAttribution: skips entirely for migration user — given migration_submissions row exists for the user, asserts no conversion_attribution row is created and Sentry.captureMessage is called with tags.reason === 'migration_user'.
  3. writeOrCarryForwardAttribution: writes directly when metadata has real ad attribution — regression test for existing behavior (utm only path).
  4. findPriorAttributionForUser: prefers most recent across source types — seed 3 rows (INQUIRY 30d ago, TOUR 7d ago, HOURLY 1d ago), assert HOURLY is returned.
  5. findPriorAttributionForUser: ignores fbp-only rows — seed a row with only fbp set, plus an older row with utm_source, assert older row is returned.
  6. carryForwardAttribution: emits warning when nothing found — given a clean user with no prior attribution and no fbclidFirstTouch, assert Sentry.captureMessage was called with level: 'warning' and tags.reason === 'no_prior_attribution'.

Mock @sentry/nextjs per the pattern at apps/api/tests/api/cron/reconcile-access-codes.api.test.ts:

vi.mock('@sentry/nextjs', () => ({
  captureException: vi.fn(),
  captureMessage: vi.fn(),
}))
import * as Sentry from '@sentry/nextjs'

Existing tests (carryForwardAttribution: copies from prior inquiry, inquiry wins over fbclidFirstTouch, etc.) need to be reviewed and likely updated since the function now looks at conversion_attribution rows directly instead of joining through inquiry. Most should be semantically equivalent — but the test for "inquiry exists but no CA row" path may need adjustment.

Step 6 — Update direct writers' writeConversionAttribution callers (no behavioral change)

The five direct call sites (contact/route.ts:223,313, leads/email-capture/route.ts:65, comp/reservations/route.ts:266, promo/redeem/route.ts:43, ReservationCreationService.ts:757) now silently no-op when their input is fbp-only because of Step 1.4's predicate change in writeConversionAttribution. This is the desired behavior. No caller change needed, but verify the existing contact/route.attribution.test.ts still passes; if any test was asserting a row was written for fbp-only input, the test was masking the bug and should be updated.

Critical files referenced

  • apps/api/src/services/shared/AttributionService.ts (305 lines) — primary edit
  • apps/api/src/services/shared/AttributionService.test.ts (514 lines) — primary test edit
  • apps/api/src/services/payment/FulfillmentService.ts — pass stripeRef at lines 132, 252, 557, 711
  • apps/api/src/services/payment/PaymentIntentHandler.ts — pass stripeRef at lines 234, 335
  • apps/api/src/services/payment/ReservationCreationService.ts:755-771 — TOUR call site, may need migration-user-aware path (already covered if check lives inside carryForwardAttribution)
  • apps/api/src/lib/error-reporting.ts — pattern reference only (don't modify; use Sentry.captureMessage directly as in CreditTransferService.ts)
  • apps/api/prisma/schema.prisma — read for MigrationSubmission line 801, AttributionSourceType enum, ConversionAttribution model. No edits.

Verification

Pre-merge

  1. pnpm --filter @mg/api typecheck:quick clean.
  2. cd apps/api && ./scripts/run-tests.sh src/services/shared/AttributionService.test.ts — all green, including the 6 new tests.
  3. cd apps/api && ./scripts/run-tests.sh src/app/api/contact/route.attribution.test.ts — verify no regression on direct writers.
  4. Manual sanity check via pnpm dev --test: hit a checkout-initialize-* endpoint with attribution: { fbp: 'x' } only → confirm no row written. Hit with attribution: { utm_source: 'google' } → confirm row written.
  5. Code-review the diff against the rules in ~/.claude/CLAUDE.md (no inline comments, smallest reasonable change, no handrolled utils — searched packages/shared-utils/, no attribution helpers there, this is API-scoped).

Post-deploy (one week)

Re-run the snapshot queries from docs/fix-missing-attribution-rows/investigation.md against prod:

-- Self-serve MONTHLY coverage (target ≥ 50%, was 0%)
WITH lockouts AS (
  SELECT r.id, r.user_id,
         CASE
           WHEN EXISTS(SELECT 1 FROM migration_submissions ms WHERE ms.user_id = r.user_id) THEN 'migrated'
           WHEN EXISTS(SELECT 1 FROM payments p WHERE p.reservation_id = r.id AND p.checkout_invitation_id IS NOT NULL) THEN 'invited'
           ELSE 'self_serve'
         END AS path
  FROM reservations r
  WHERE r.type='MONTHLY' AND r.created_at >= NOW() - INTERVAL '7 days'
)
SELECT path, COUNT(*) AS n,
       COUNT(*) FILTER (WHERE id IN (SELECT source_id FROM conversion_attributions WHERE source_type='MONTHLY')) AS with_attr
FROM lockouts GROUP BY path;

-- Zero new fbp-only rows
SELECT COUNT(*) FROM conversion_attributions
WHERE created_at >= '<deploy_date>'
  AND fbp IS NOT NULL
  AND utm_source IS NULL AND utm_medium IS NULL AND utm_campaign IS NULL
  AND utm_term IS NULL AND utm_content IS NULL AND fbclid IS NULL AND gclid IS NULL;
-- Expected: 0

Sentry verification

In metrognome org → mg-api project, filter service:attribution issues:

  • no_prior_attribution (warning) issue should populate with events. Each event's "Extra" panel should contain userId, sourceId, sourceType, hasUserEmail, userCreatedAt, stripeRef — enough for a future agent to fetch the raw Stripe metadata + cross-reference the user row.
  • migration_user (info), metadata_only_fbp_fbc (info), rescued_fbclid_first_touch (info) should each show as separate issues for clean filtering.
  • Set a Sentry alert: warning-level events on service:attribution exceeding 5/day → notify. (Configure in Sentry UI; not part of the code PR.)

Risk notes

  • The new carry-forward query runs at every fulfillment. Four indexed Prisma queries per fulfillment is acceptable (fulfillment already does ~10+ DB ops). If perf becomes an issue, consolidate into one raw SQL with subqueries.
  • No lookback window means a user who clicked an ad in 2024 and bought a lockout today gets credited. This was Aaron's explicit choice; revisit if attribution reporting starts showing ancient campaigns winning credit.
  • carryForwardAttribution's userFbclidFirstTouch rescue path stays. It's the lowest-quality data (only fbclid, no UTM context), but it's a real signal and still 1/273 users hit it. Removing it would lose that signal; keeping it costs nothing.
  • Existing 5 fbp-only rows and 6 all-NULL rows in prod stay until Fix 7 is done in a follow-up. They'll continue to show up in legacy queries but no new ones will be added.

Out-of-scope follow-ups (track separately)

  • Fix 2: capture gclid_first_touch on User — schema change, separate PR.
  • Fix 5: localStorage migration for UTMs (30d TTL) + _gclid cookie — frontend PR.
  • Fix 7: delete the 11 bad historical rows + add CHECK constraint + audit trigger — data-ops PR.
  • ADR in docs/decisions/ documenting the fbp-only persistence decision (Cause 4).