fix(roster): one idempotent attach path for valuation claims

Merged
SW
2mo ago
+506-548

Implements the api item of recoupable/chat#1965 — one idempotent implementation of "link an artist account to a user account" and one of "resolve-or-create the canonical artist", with attach failures surfaced in the valuation Telegram alert. Deletion-led: net −95 LOC.

No database PR needed: the UNIQUE (account_id, artist_id) constraint this upsert targets already shipped in database migration 20260708200000_join_row_unique_constraints.sql (merged 2026-07-08). Verified against prod 2026-08-18: ON CONFLICT (account_id, artist_id) is accepted and inserts nothing on an existing pair; 0 duplicate pairs across all 1,582 rows. No docs PR: POST /api/artists' public contract is unchanged.

What changed

  • insertAccountArtistId → upsertAccountArtistId (renamed per review): an upsert on (account_id, artist_id) with ignoreDuplicates: true (mirrors upsertSongs). Returns void — no caller used the row. Roster prechecks deleted at every call site (resolveOrCreateArtist, linkArtistToAccount, attachCanonicalArtistToAccount). setAccountArtistPin keeps its select — it distinguishes update-pin vs insert-with-pin, which an ignore-duplicates upsert cannot.
  • lib/valuation/linkSearchedArtistToAccount.ts deleted (near line-for-line duplicate of resolveOrCreateArtist); runValuationHandler calls resolveOrCreateArtist directly.
  • selectSongArtists throws on query error instead of returning null — the ?? [] at its attach call site turned failed queries into "no links, fall through". deleteArtist (the only other caller) keeps fail-closed behavior via its own catch.
  • attachCanonicalArtistToAccount shrinks to ISRC → dominant artist → shared link call, and no longer swallows errors.
  • createSnapshotCatalog no longer attaches — it returns the measured ISRCs; each surface owns its attach policy at exactly one catch site:
    • createCatalogHandler (claim + re-claim unified via the extracted resolveClaimedCatalog): best-effort, a failed attach never fails the claim.
  • getArtistPublicProfile (landed in #840 after the branch point) updated for the new selectSongArtists throw contract: a songs-graph query error degrades the unauthenticated artist page to an empty catalog list instead of a 500.
    • runValuationHandler: single try/catch around attach → fallback resolve → enrich; the error is carried into the lead capture.
  • captureValuationLead: one new line in the existing Telegram alert — Roster: attached ✓ / Roster: ATTACH FAILED — <error> / Roster: nothing attached.

Verification (local)

  • TDD red→green per unit; full suite 824 files / 4,566 tests green.
  • tsc --noEmit: error set in touched domains byte-identical to main baseline (no new errors).
  • eslint clean on all changed files.
  • git grep confirms one link implementation (the upsert) and one resolve-or-create (resolveOrCreateArtist) remain.

Preview verification (double valuation run on a test account → exactly 1 roster artist, 1 link row, Roster: attached ✓ in the alert) to follow as a PR comment.

Merge note: independent — no other PR in this train.

🤖 Generated with Claude Code

Summary by cubic

Unifies roster attach for valuation claims behind a single idempotent upsert and makes failures visible. Previously, check-then-insert paths and swallowed errors produced silent empty-roster claims; now a (account_id, artist_id) upsert handles idempotency, callers choose failure policy, and the public artist page degrades to an empty catalog list instead of 500 on songs-graph errors.

  • Replace insertAccountArtistId with upsertAccountArtistId (upserts on (account_id, artist_id) with ignoreDuplicates); delete all roster prechecks. setAccountArtistPin still selects to distinguish update vs insert-with-pin.

  • Remove valuation-only linkSearchedArtistToAccount; fallback now uses the shared resolveOrCreateArtist.

  • attachCanonicalArtistToAccount now throws on query/link failure; selectSongArtists throws on query error; getArtistPublicProfile catches and degrades to an empty catalog list; deleteArtist keeps fail-closed behavior via its own catch.

  • createSnapshotCatalog stops attaching and returns measured ISRCs; createCatalogHandler uses new resolveClaimedCatalog (reclaims or creates, returns ISRCs) and runs a best‑effort attach; runValuationHandler attaches canonical from ISRCs, falls back to resolveOrCreateArtist, enriches, and forwards any error text to captureValuationLead.

  • captureValuationLead adds a Telegram line: “Roster: attached ✓” / “Roster: ATTACH FAILED — ” / “Roster: nothing attached”.

  • Refactor: extract resolveClaimedCatalog; rename files/tests to upsertAccountArtistId.

  • No database changes required (targets existing unique (account_id, artist_id) constraint). Internal migration: callers updated to handle thrown errors from selectSongArtists and to use upsertAccountArtistId.

Summary by CodeRabbit

  • New Features

    • Artist links now attach reliably without creating duplicates.
    • Valuation searches can resolve or create artists and attach them to the account.
    • Catalog processing now supports both new and existing catalogs consistently.
    • Lead notifications include artist attachment status.

  • Bug Fixes

    • Artist deletion is prevented when dependency checks fail or songs still reference the artist.
    • Lookup and relationship errors now surface consistently instead of being silently treated as missing data.
    • Catalog and artist association failures no longer interrupt valuation requests.