Skip to content

chore(valuation): remove legacy client-side lead-capture ping (chat#1885) - #53

Merged
sweetmantech merged 2 commits into
mainfrom
chore/valuation-lead-remove-legacy-ping
Jul 23, 2026
Merged

chore(valuation): remove legacy client-side lead-capture ping (chat#1885)#53
sweetmantech merged 2 commits into
mainfrom
chore/valuation-lead-remove-legacy-ping

Conversation

@sweetmantech

@sweetmantech sweetmantech commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What

Follow-up to recoupable/api#785, which moved valuation lead-capture (Attio upsert + team Telegram alert) server-side into POST /api/valuation so every caller captures a lead. With that in place the marketing funnel's client-side ping is redundant — and worse, it would double-fire (two Attio notes + two Telegram pings) for every funnel valuation.

This removes the browser ping so a funnel valuation captures exactly one lead, from the shared api. Closes the de-dupe half of the "Valuation lead-capture + Telegram parity" item in recoupable/chat#1885.

Removed

  • The captureRunLead(...) call in hooks/useCatalogValuation.ts (and the now-unused user from usePrivy).
  • lib/valuation/captureRunLead.ts, lib/valuation/captureValuationLead.ts.
  • The app/api/valuation/lead route + its Attio/Telegram helpers: upsertValuationLead, leadAttributes, leadNoteContent, valuationLeadInput.
  • lib/attio/* (the valuation-lead-only Attio primitives), lib/telegram.ts, lib/format/usd.ts — each confirmed used only by the lead chain.

23 files deleted; no dangling imports (Running TypeScript passes in pnpm build).

Kept (deliberately)

  • lib/attio.ts — the separate subscribe/CRM createAttioContact integration (used by app/api/subscribe), unrelated to valuation leads.
  • Valuation display helpers: lib/valuation/formatUsd.ts, formatCompact.ts, runValuation.ts, toArtist.ts.

Verification

  • Remaining lib/valuation tests green (6 tests).
  • pnpm build: ✓ Compiled successfully + TypeScript pass. Full static export needs a real NEXT_PUBLIC_PRIVY_APP_ID (a placeholder trips Privy's ID validation prerendering /advisory/book) — a known local-export limitation; the Vercel preview completes the build.
  • End-to-end (funnel valuation → exactly one Attio lead + one Telegram ping) verified against the preview together with api#785, once ATTIO_API_KEY is set on the api project — results to follow in a comment.

Merge order

Merge api#785 first, then this, back-to-back. The brief window between them (funnel valuations double-firing) is cosmetic — one extra Attio note + one extra ping.

🤖 Generated with Claude Code


Summary by cubic

Remove the legacy client-side valuation lead-capture ping and the /api/valuation/lead route now that capture runs server-side in POST /api/valuation (recoupable/api#785). Prevents duplicate Attio notes and Telegram alerts so each funnel valuation captures exactly one lead; merge after recoupable/api#785 (chat#1885).

  • Bug Fixes

    • Stop double-firing; one Attio upsert and one Telegram alert per valuation via the shared API.
  • Refactors

    • Removed the captureRunLead call and unused user from useCatalogValuation.
    • Deleted the legacy /api/valuation/lead chain and helpers: lib/valuation/*, lib/attio/*, lib/telegram, lib/format/usd; kept lib/attio.ts (subscribe/CRM) and valuation display helpers. Also removed a stray pr24-option-b-valuation-54M.png screenshot.

Written for commit 0927c4b. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Changes

    • Removed valuation lead capture and related notifications after valuation runs.
    • Valuation results no longer trigger CRM updates or messaging notifications.
    • Removed supporting lead-formatting and integration functionality.
  • Tests

    • Removed automated coverage for valuation lead capture, CRM synchronization, notifications, and related helpers.

…885)

Lead-capture (Attio upsert + team Telegram alert) now fires server-side
from POST /api/valuation for every caller (api#785), so the marketing
funnel's client-side path is redundant and would double-fire (two Attio
notes + two pings per funnel valuation).

Remove the browser ping and the whole now-orphaned chain:
- the `captureRunLead` call in useCatalogValuation (+ the unused `user`);
- captureRunLead / captureValuationLead;
- the `/api/valuation/lead` route and its Attio/Telegram helpers
  (upsertValuationLead, leadAttributes, leadNoteContent, valuationLeadInput);
- lib/attio/* (valuation-lead-only primitives), lib/telegram, lib/format/usd.

Kept: lib/attio.ts (the subscribe/CRM contact integration) and the
valuation display helpers (formatUsd, formatCompact, runValuation, toArtist).

A funnel valuation now captures exactly one lead, from the shared api.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 23, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marketing Ready Ready Preview Jul 23, 2026 10:02pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 65735c93-c793-4838-8c5a-fa18a300dc2c

📥 Commits

Reviewing files that changed from the base of the PR and between 61e79f0 and 0927c4b.

📒 Files selected for processing (24)
  • app/api/valuation/lead/__tests__/route.test.ts
  • app/api/valuation/lead/route.ts
  • hooks/useCatalogValuation.ts
  • lib/attio/__tests__/addRecordToList.test.ts
  • lib/attio/__tests__/assertPersonByEmail.test.ts
  • lib/attio/__tests__/createNote.test.ts
  • lib/attio/__tests__/isRecordInList.test.ts
  • lib/attio/addRecordToList.ts
  • lib/attio/assertPersonByEmail.ts
  • lib/attio/createNote.ts
  • lib/attio/isRecordInList.ts
  • lib/attio/request.ts
  • lib/format/usd.ts
  • lib/telegram.ts
  • lib/valuation/__tests__/captureValuationLead.test.ts
  • lib/valuation/__tests__/leadAttributes.test.ts
  • lib/valuation/__tests__/leadNoteContent.test.ts
  • lib/valuation/__tests__/upsertValuationLead.test.ts
  • lib/valuation/captureRunLead.ts
  • lib/valuation/captureValuationLead.ts
  • lib/valuation/leadAttributes.ts
  • lib/valuation/leadNoteContent.ts
  • lib/valuation/upsertValuationLead.ts
  • lib/valuation/valuationLeadInput.ts
💤 Files with no reviewable changes (23)
  • lib/attio/isRecordInList.ts
  • lib/valuation/captureValuationLead.ts
  • lib/valuation/leadNoteContent.ts
  • lib/attio/tests/assertPersonByEmail.test.ts
  • lib/attio/createNote.ts
  • lib/format/usd.ts
  • app/api/valuation/lead/route.ts
  • app/api/valuation/lead/tests/route.test.ts
  • lib/telegram.ts
  • lib/valuation/tests/leadAttributes.test.ts
  • lib/valuation/captureRunLead.ts
  • lib/valuation/valuationLeadInput.ts
  • lib/attio/assertPersonByEmail.ts
  • lib/attio/tests/isRecordInList.test.ts
  • lib/attio/request.ts
  • lib/attio/addRecordToList.ts
  • lib/valuation/tests/leadNoteContent.test.ts
  • lib/valuation/tests/upsertValuationLead.test.ts
  • lib/valuation/tests/captureValuationLead.test.ts
  • lib/attio/tests/addRecordToList.test.ts
  • lib/valuation/leadAttributes.ts
  • lib/valuation/upsertValuationLead.ts
  • lib/attio/tests/createNote.test.ts

📝 Walkthrough

Walkthrough

Valuation lead capture was removed from the client hook and server route, along with its Attio, Telegram, formatting, input, and test modules. The hook now runs valuations and updates state without initiating lead-capture side effects.

Changes

Valuation lead-capture removal

Layer / File(s) Summary
Remove client and server lead capture
hooks/useCatalogValuation.ts, app/api/valuation/lead/route.ts, lib/valuation/*, lib/attio/*, lib/telegram.ts, lib/format/usd.ts
The valuation hook no longer invokes client-side lead capture, while the API route and supporting Attio, Telegram, formatting, input, and lead-processing modules were removed along with their tests.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • recoupable/marketing#26: Added the valuation lead route and related Attio, Telegram, formatting, and test modules removed here.
  • recoupable/marketing#27: Added the valuation lead integrations and per-run Attio and Telegram processing removed here.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main user-facing change: removing the legacy client-side lead-capture ping.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/valuation-lead-remove-legacy-ping

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 25 files

Re-trigger cubic

@sweetmantech

Copy link
Copy Markdown
Contributor Author

Preview verification — funnel valuation, no client-side lead ping

Also removed a stray screenshot (pr24-option-b-valuation-54M.png) that had been committed to the branch by mistake.

Tested the live funnel on the preview (marketing-git-chore-valuation-lead-remove-legacy-ping): searched an artist → Value my catalog → Privy sign-in (new account sweetmantech+jul2320261655@gmail.com) → valuation ran and rendered.

Funnel valuation result

The core change — client no longer pings /api/valuation/lead

Captured all XHR/fetch during the run. The funnel makes exactly one app call — the valuation — and zero calls to the removed lead route:

Request Count
POST …/api/valuation (the valuation) 1
POST …/api/valuation/lead (the removed client ping) 0
GET /api/spotify/search (typeahead) + Privy auth (expected)
  • /api/valuation/lead route → 404 on the preview (GET + POST) — the route + its helpers are deleted.
  • Funnel valuation still works end-to-end: $3.6M (range $2.5M–$5.0M), 207 tracks, "Get the full report" CTA.

On the "exactly one lead" end-state

This preview's funnel calls test-recoup-api (per lib/config.ts, non-prod → test api), which doesn't yet carry api#785's server-side lead-capture — so no lead fired here (confirmed: no Attio Person for the test email). That's expected for the preview, not a regression from this PR.

The prod end-state is: funnel → recoup-api (has api#785, merged 96959eb1) → the api captures exactly one Attio lead + one Telegram ping, and this PR removes the second (client) fire. api#785's server-side capture was verified directly against Attio (api#785 results). So: api#785 fires one + marketing#53 removes the client one = exactly one, no double-send.

pnpm build + TS pass (Vercel CI green). Ready to merge (completes the api#785 → marketing#53 pair).

@sweetmantech
sweetmantech merged commit 5703dd8 into main Jul 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant