Plan: Recover missing conversion_attributions rows (Fixes 1 + 3 + 6)
Status: Shipped. Core plan implemented in PR #727. One post-ship change:
ConversionAttribution.sourceCodecolumn was designed but never reached prod and was dropped in PR #730 — MMS codes route throughutm_contentinstead. References tosourceCodein 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:
writeOrCarryForwardAttributionwrites uselessfbp-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.)- 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 readsfbpdirectly fromsession.metadataintrackPurchaseEventToMeta, 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_idhas anymigration_submissionsrow, 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:
- 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.
-
Delete
hasPersistableAttribution(lines 120–132). It is byte-identical tohasAttributionData, has been since PR #721 (Cause 4), and becomes structurally unused after Step 2. Removing it is implicit cleanup, not Fix 4 scope creep. -
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 })
}
- Update
writeConversionAttribution(lines 154–173) to gate persistence withhasRealAdAttribution(replacing the currenthasAttributionData+hasPersistableAttributiondouble-guard). This stops the direct-write paths (/api/contact,/api/leads/email-capture,/api/promo/redeem,/api/comp/reservations,ReservationCreationServicefor 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):
- If
userIdmissing → return (existing). - NEW: If
isMigrationUser(tx, userId)→ emit info signal, return. - Call
findPriorAttributionForUser(tx, userId). If found → copy fields, source ='prior-attribution-carry-forward', write row, fire alert. - Else if
userFbclidFirstTouch→ existing rescue path (unchanged), source ='user-fbclid-rescue'. - 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:
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).writeOrCarryForwardAttribution: skips entirely for migration user— givenmigration_submissionsrow exists for the user, asserts noconversion_attributionrow is created andSentry.captureMessageis called withtags.reason === 'migration_user'.writeOrCarryForwardAttribution: writes directly when metadata has real ad attribution— regression test for existing behavior (utm only path).findPriorAttributionForUser: prefers most recent across source types— seed 3 rows (INQUIRY 30d ago, TOUR 7d ago, HOURLY 1d ago), assert HOURLY is returned.findPriorAttributionForUser: ignores fbp-only rows— seed a row with onlyfbpset, plus an older row withutm_source, assert older row is returned.carryForwardAttribution: emits warning when nothing found— given a clean user with no prior attribution and nofbclidFirstTouch, assertSentry.captureMessagewas called withlevel: 'warning'andtags.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 editapps/api/src/services/shared/AttributionService.test.ts(514 lines) — primary test editapps/api/src/services/payment/FulfillmentService.ts— passstripeRefat lines 132, 252, 557, 711apps/api/src/services/payment/PaymentIntentHandler.ts— passstripeRefat lines 234, 335apps/api/src/services/payment/ReservationCreationService.ts:755-771— TOUR call site, may need migration-user-aware path (already covered if check lives insidecarryForwardAttribution)apps/api/src/lib/error-reporting.ts— pattern reference only (don't modify; useSentry.captureMessagedirectly as inCreditTransferService.ts)apps/api/prisma/schema.prisma— read forMigrationSubmissionline 801,AttributionSourceTypeenum,ConversionAttributionmodel. No edits.
Verification
Pre-merge
pnpm --filter @mg/api typecheck:quickclean.cd apps/api && ./scripts/run-tests.sh src/services/shared/AttributionService.test.ts— all green, including the 6 new tests.cd apps/api && ./scripts/run-tests.sh src/app/api/contact/route.attribution.test.ts— verify no regression on direct writers.- Manual sanity check via
pnpm dev --test: hit a checkout-initialize-* endpoint withattribution: { fbp: 'x' }only → confirm no row written. Hit withattribution: { utm_source: 'google' }→ confirm row written. - Code-review the diff against the rules in
~/.claude/CLAUDE.md(no inline comments, smallest reasonable change, no handrolled utils — searchedpackages/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 containuserId, 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:attributionexceeding 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'suserFbclidFirstTouchrescue 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_touchon User — schema change, separate PR. - Fix 5: localStorage migration for UTMs (30d TTL) +
_gclidcookie — 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).