feat: migrate create_new_artist MCP tool from Recoup-Chat to recoup-api#114
Conversation
Migrate the create_new_artist MCP tool from Recoup-Chat to recoup-api. This tool creates a new artist account and optionally copies a conversation to the new artist. Components added: - insertAccountArtistId: Associate artist with owner account - selectAccountWithSocials: Fetch account with socials and info - createArtistInDb: Orchestrate artist creation (account, info, owner link, org link) - copyRoom: Copy conversation to new artist - registerCreateNewArtistTool: MCP tool registration with zod schema - artists/index.ts: Artist tools index All 22 new unit tests pass (460 total). Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds artist creation backend: a DB orchestration (createArtistInDb), MCP tool to expose it, room-copy utility, and supporting Supabase helpers, plus comprehensive unit tests for each new behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant MCP as MCP Client
participant Tool as create_new_artist Tool
participant Artist as createArtistInDb
participant DB as Database
participant Room as copyRoom
MCP->>Tool: invoke(name, accountId, organizationId?, activeConversationId?)
Tool->>Artist: createArtistInDb(name, ownerAccountId, organizationId?)
Artist->>DB: insertAccount(name)
DB-->>Artist: account
Artist->>DB: insertAccountInfo(account.id)
DB-->>Artist: account_info
Artist->>DB: selectAccountWithSocials(account.id)
DB-->>Artist: account + socials + info
Artist->>DB: insertAccountArtistId(ownerAccountId, account.id)
DB-->>Artist: artist_relation
alt organizationId provided
Artist->>DB: addArtistToOrganization(account.id, organizationId)
DB-->>Artist: done
end
Artist-->>Tool: CreateArtistResult
alt activeConversationId provided
Tool->>Room: copyRoom(activeConversationId, account.id)
Room->>DB: selectRoom(sourceRoomId)
DB-->>Room: room_data
Room->>DB: insertRoom(newRoomId, data)
DB-->>Room: newRoomId
Room-->>Tool: newRoomId
end
Tool-->>MCP: success(artist + newRoomId?)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
lib/rooms/__tests__/copyRoom.test.ts (1)
28-33: Minor: Unused mock variable.
mockNewRoomis defined but never used in any assertions. The tests correctly verify behavior throughmockInsertRoomcalls and the returned room ID. Consider removing this unused variable for cleaner test code.🧹 Suggested cleanup
- const mockNewRoom = { - id: "generated-uuid-123", - account_id: "account-456", - artist_id: "new-artist-999", - topic: "Original Conversation", - }; - beforeEach(() => {lib/rooms/copyRoom.ts (1)
37-39: Consider logging errors before returning null.The catch block silently swallows all errors, which could make debugging difficult in production. While returning
nullis appropriate for the function's contract, logging the error would aid troubleshooting.🔧 Optional improvement
- } catch { + } catch (error) { + console.error("Failed to copy room:", error); return null; }lib/artists/__tests__/createArtistInDb.test.ts (1)
83-94: Consider adding a test for organization linkage failure.The test suite covers most failure scenarios, but there's no test for when
addArtistToOrganizationfails. Based on the implementation increateArtistInDb.ts, this would also result innullbeing returned.🧪 Suggested additional test
+ it("returns null when organization linkage fails", async () => { + mockInsertAccount.mockResolvedValue(mockAccount); + mockInsertAccountInfo.mockResolvedValue(mockAccountInfo); + mockSelectAccountWithSocials.mockResolvedValue(mockFullAccount); + mockInsertAccountArtistId.mockResolvedValue({ id: "rel-123" }); + mockAddArtistToOrganization.mockRejectedValue(new Error("Org linkage failed")); + + const result = await createArtistInDb("Test Artist", "owner-456", "org-789"); + + expect(result).toBeNull(); + });lib/mcp/tools/artists/__tests__/registerCreateNewArtistTool.test.ts (1)
68-93: Consider adding a test forcopyRoomfailure.The current tests verify
copyRoomsuccess, but ifcopyRoomfails (returnsnullor throws), the handler will still return success withnewRoomId: null. You may want to add a test to document this expected behavior, or consider whether acopyRoomfailure should be surfaced to the user.📝 Example test for copyRoom failure behavior
it("returns success with null newRoomId when copyRoom fails", async () => { const mockArtist = { id: "artist-123", account_id: "artist-123", name: "Test Artist", image: null, }; mockCreateArtistInDb.mockResolvedValue(mockArtist); mockCopyRoom.mockResolvedValue(null); const result = await registeredHandler({ name: "Test Artist", account_id: "owner-456", active_conversation_id: "source-room-111", }); expect(mockCopyRoom).toHaveBeenCalledWith("source-room-111", "artist-123"); // Verify the result still indicates success but with null newRoomId expect(result).toEqual({ content: [ { type: "text", text: expect.stringContaining("Successfully created artist"), }, ], }); });lib/supabase/accounts/selectAccountWithSocials.ts (2)
1-32: Consider renaming to follow the "get[Descriptive]" convention for complex queries.Per the coding guidelines, complex Supabase queries with joins should be named
get[Descriptive].ts. Since this function performs joins acrossaccounts,account_socials, andaccount_info, consider renaming:
- File:
getAccountWithSocials.ts- Function:
getAccountWithSocialsThis distinguishes it from simple single-table selects and follows the established pattern. Based on coding guidelines.
27-29: Silent error handling may complicate debugging.The function returns
nullfor both "not found" and "database error" scenarios. While this simplifies the API, consider logging errors for observability, especially for unexpected database failures.📝 Suggested improvement with error logging
if (error || !data) { + if (error && error.code !== "PGRST116") { + // Log unexpected errors (PGRST116 = row not found, which is expected) + console.error("selectAccountWithSocials error:", error); + } return null; }lib/mcp/tools/artists/registerCreateNewArtistTool.ts (1)
77-81: Consider handlingcopyRoomfailure more explicitly.When
copyRoomfails and returnsnull, the tool still reports success withnewRoomId: null. While this may be intentional (artist creation succeeded, room copy is optional), it could make debugging harder if users expect the room to be copied.Consider either:
- Adding a warning in the message when room copy fails
- Logging the failure for observability
💡 Optional enhancement
// Copy the conversation to the new artist if requested let newRoomId: string | null = null; if (active_conversation_id) { newRoomId = await copyRoom(active_conversation_id, artist.account_id); } + const roomCopyFailed = active_conversation_id && !newRoomId; + const result: CreateNewArtistResult = { artist: { account_id: artist.account_id, name: artist.name, image: artist.image, }, artistAccountId: artist.account_id, - message: `Successfully created artist "${name}". Now searching Spotify for this artist to connect their profile...`, + message: roomCopyFailed + ? `Successfully created artist "${name}", but failed to copy the conversation. Now searching Spotify for this artist to connect their profile...` + : `Successfully created artist "${name}". Now searching Spotify for this artist to connect their profile...`, newRoomId, };lib/artists/createArtistInDb.ts (2)
10-28: Consider using stricter types forunknownfields.The fields
account_info,account_socials, andonboarding_datauseunknowntypes. While this is safe, typing these more precisely would improve type safety and developer ergonomics if the structure is known.
85-87: Silent error swallowing hinders debugging.The catch block discards the error without logging, making it difficult to diagnose failures in production. Consider logging the error for observability.
💡 Suggested improvement
- } catch (error) { + } catch (error) { + console.error("Failed to create artist in DB:", error); return null; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
lib/artists/__tests__/createArtistInDb.test.tslib/artists/createArtistInDb.tslib/mcp/tools/artists/__tests__/registerCreateNewArtistTool.test.tslib/mcp/tools/artists/index.tslib/mcp/tools/artists/registerCreateNewArtistTool.tslib/mcp/tools/index.tslib/rooms/__tests__/copyRoom.test.tslib/rooms/copyRoom.tslib/supabase/account_artist_ids/__tests__/insertAccountArtistId.test.tslib/supabase/account_artist_ids/insertAccountArtistId.tslib/supabase/accounts/__tests__/selectAccountWithSocials.test.tslib/supabase/accounts/selectAccountWithSocials.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Apply Single Responsibility Principle (SRP) - one exported function per file
Apply DRY principle (Don't Repeat Yourself) - extract shared logic into reusable utilities
Files:
lib/mcp/tools/artists/index.tslib/supabase/accounts/__tests__/selectAccountWithSocials.test.tslib/rooms/copyRoom.tslib/supabase/account_artist_ids/__tests__/insertAccountArtistId.test.tslib/rooms/__tests__/copyRoom.test.tslib/mcp/tools/artists/registerCreateNewArtistTool.tslib/mcp/tools/index.tslib/artists/createArtistInDb.tslib/supabase/accounts/selectAccountWithSocials.tslib/artists/__tests__/createArtistInDb.test.tslib/mcp/tools/artists/__tests__/registerCreateNewArtistTool.test.tslib/supabase/account_artist_ids/insertAccountArtistId.ts
!(lib/supabase)/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER import
@/lib/supabase/serverClientoutside oflib/supabase/directory
Files:
lib/mcp/tools/artists/index.tslib/supabase/accounts/__tests__/selectAccountWithSocials.test.tslib/rooms/copyRoom.tslib/supabase/account_artist_ids/__tests__/insertAccountArtistId.test.tslib/rooms/__tests__/copyRoom.test.tslib/mcp/tools/artists/registerCreateNewArtistTool.tslib/mcp/tools/index.tslib/artists/createArtistInDb.tslib/supabase/accounts/selectAccountWithSocials.tslib/artists/__tests__/createArtistInDb.test.tslib/mcp/tools/artists/__tests__/registerCreateNewArtistTool.test.tslib/supabase/account_artist_ids/insertAccountArtistId.ts
lib/supabase/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
lib/supabase/**/*.{ts,tsx}: All Supabase database calls must be inlib/supabase/[table_name]/[function].ts
Supabase select functions should be namedselect[TableName].ts
Supabase insert functions should be namedinsert[TableName].ts
Supabase update functions should be namedupdate[TableName].ts
Supabase delete functions should be nameddelete[TableName].ts
Complex Supabase queries with joins should be namedget[Descriptive].ts
Files:
lib/supabase/accounts/__tests__/selectAccountWithSocials.test.tslib/supabase/account_artist_ids/__tests__/insertAccountArtistId.test.tslib/supabase/accounts/selectAccountWithSocials.tslib/supabase/account_artist_ids/insertAccountArtistId.ts
🧠 Learnings (3)
📚 Learning: 2026-01-14T22:12:03.883Z
Learnt from: CR
Repo: Recoupable-com/Recoup-API PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T22:12:03.883Z
Learning: Applies to lib/supabase/**/*.{ts,tsx} : Supabase select functions should be named `select[TableName].ts`
Applied to files:
lib/supabase/accounts/__tests__/selectAccountWithSocials.test.tslib/supabase/accounts/selectAccountWithSocials.ts
📚 Learning: 2026-01-14T22:12:03.883Z
Learnt from: CR
Repo: Recoupable-com/Recoup-API PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T22:12:03.883Z
Learning: Applies to lib/supabase/**/*.{ts,tsx} : Supabase insert functions should be named `insert[TableName].ts`
Applied to files:
lib/supabase/account_artist_ids/__tests__/insertAccountArtistId.test.tslib/supabase/account_artist_ids/insertAccountArtistId.ts
📚 Learning: 2026-01-14T22:12:03.883Z
Learnt from: CR
Repo: Recoupable-com/Recoup-API PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T22:12:03.883Z
Learning: Applies to lib/@(auth|chats|emails|ai|x402|trigger)/**/*.{ts,tsx} : When needing database access in domain folders (lib/auth/, lib/chats/, etc.), first check if a function exists in `lib/supabase/[table_name]/`, create it if not, then import and use it
Applied to files:
lib/supabase/accounts/selectAccountWithSocials.tslib/supabase/account_artist_ids/insertAccountArtistId.ts
🧬 Code graph analysis (11)
lib/mcp/tools/artists/index.ts (1)
lib/mcp/tools/artists/registerCreateNewArtistTool.ts (1)
registerCreateNewArtistTool(50-102)
lib/supabase/accounts/__tests__/selectAccountWithSocials.test.ts (1)
lib/supabase/accounts/selectAccountWithSocials.ts (1)
selectAccountWithSocials(18-32)
lib/rooms/copyRoom.ts (3)
lib/supabase/rooms/selectRoom.ts (1)
selectRoom(12-22)lib/uuid/generateUUID.ts (1)
generateUUID(8-20)lib/supabase/rooms/insertRoom.ts (1)
insertRoom(8-14)
lib/supabase/account_artist_ids/__tests__/insertAccountArtistId.test.ts (1)
lib/supabase/account_artist_ids/insertAccountArtistId.ts (1)
insertAccountArtistId(15-37)
lib/rooms/__tests__/copyRoom.test.ts (1)
lib/rooms/copyRoom.ts (1)
copyRoom(13-40)
lib/mcp/tools/artists/registerCreateNewArtistTool.ts (4)
lib/artists/createArtistInDb.ts (1)
createArtistInDb(38-88)lib/mcp/getToolResultError.ts (1)
getToolResultError(9-16)lib/rooms/copyRoom.ts (1)
copyRoom(13-40)lib/mcp/getToolResultSuccess.ts (1)
getToolResultSuccess(9-11)
lib/mcp/tools/index.ts (1)
lib/mcp/tools/artists/index.ts (1)
registerAllArtistTools(9-11)
lib/supabase/accounts/selectAccountWithSocials.ts (1)
types/database.types.ts (1)
Tables(3819-3844)
lib/artists/__tests__/createArtistInDb.test.ts (1)
lib/artists/createArtistInDb.ts (1)
createArtistInDb(38-88)
lib/mcp/tools/artists/__tests__/registerCreateNewArtistTool.test.ts (1)
lib/mcp/tools/artists/registerCreateNewArtistTool.ts (1)
registerCreateNewArtistTool(50-102)
lib/supabase/account_artist_ids/insertAccountArtistId.ts (1)
types/database.types.ts (1)
Tables(3819-3844)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (15)
lib/supabase/account_artist_ids/__tests__/insertAccountArtistId.test.ts (1)
1-60: LGTM!The test suite provides comprehensive coverage for
insertAccountArtistId:
- Success path with correct table and payload verification
- Error handling when insert fails
- Edge case when no data is returned
The mock chain correctly mirrors the Supabase query pattern used in the implementation.
lib/rooms/__tests__/copyRoom.test.ts (1)
39-84: LGTM!Excellent test coverage for all
copyRoomscenarios:
- Verifies correct data flow from source room to new room
- Tests default topic fallback when source topic is null
- Confirms early return when source room doesn't exist
- Validates error handling returns null on insertion failure
lib/supabase/account_artist_ids/insertAccountArtistId.ts (1)
1-37: LGTM!Well-structured Supabase insert function that follows the project conventions:
- Correct file naming (
insert[TableName].ts) per coding guidelines- Proper location within
lib/supabase/account_artist_ids/- Comprehensive error handling for both database errors and missing data
- Good JSDoc documentation explaining the function's purpose
lib/rooms/copyRoom.ts (1)
13-36: LGTM!Clean implementation that correctly:
- Validates source room existence before proceeding
- Preserves the original
account_idwhile updatingartist_id- Provides a sensible default topic
- Uses dedicated helper functions for DB operations and UUID generation
lib/artists/__tests__/createArtistInDb.test.ts (2)
1-62: LGTM!Well-organized test setup with:
- Clear mock definitions for all external dependencies
- Properly structured mock data that mirrors the actual database schema
- Clean
beforeEachhook to reset mocks between tests
96-135: LGTM!Thorough failure scenario coverage that correctly verifies:
- Subsequent steps are not called after a failure
- All failure paths return
nullas expected- The cascade of dependencies is properly tested
lib/supabase/accounts/__tests__/selectAccountWithSocials.test.ts (1)
1-62: LGTM!The test file provides comprehensive coverage for
selectAccountWithSocials:
- Properly mocks the Supabase client query chain
- Tests success path with full account data including socials and info
- Tests not-found scenario with appropriate error code
- Tests generic query failure
The mock setup in
beforeEachcorrectly mirrors the chained query pattern used in the implementation.lib/mcp/tools/artists/index.ts (1)
1-11: LGTM!Clean implementation that follows the established pattern for tool registration modules (consistent with
registerAllTaskTools,registerAllImageTools, etc.). The centralized entry point will make it easy to add more artist-related tools in the future.lib/mcp/tools/index.ts (1)
19-19: LGTM!The integration of
registerAllArtistToolsinto the main tool registration flow is correct. The import placement and function call follow the existing patterns in this file.Also applies to: 28-28
lib/mcp/tools/artists/__tests__/registerCreateNewArtistTool.test.ts (1)
1-148: Good test coverage for the tool registration.The tests comprehensively cover:
- Tool registration with correct name and description
- Artist creation success path
- Room copying when
active_conversation_idis provided- Organization ID propagation
- Error handling for both null returns and exceptions
lib/mcp/tools/artists/registerCreateNewArtistTool.ts (3)
1-6: LGTM!Imports are clean and well-organized. No imports from
@/lib/supabase/serverClientoutside oflib/supabase/directory, which complies with coding guidelines.
8-28: LGTM!The Zod schema is well-defined with clear descriptions for each field. The use of
.optional().nullable()fororganization_idcorrectly allows bothundefinedandnullvalues, supporting both omission and explicit null for personal artists as documented.
30-42: LGTM!Type definitions are properly structured.
CreateNewArtistArgsis correctly inferred from the Zod schema, andCreateNewArtistResultprovides a clear contract for the tool response.lib/artists/createArtistInDb.ts (2)
1-5: LGTM!Imports are properly structured, using the individual database helper modules rather than the
serverClientdirectly. This complies with the coding guideline to not import@/lib/supabase/serverClientoutside oflib/supabase/.
66-84: LGTM!The result object construction correctly maps fields from the retrieved artist data, with appropriate null coalescing for optional info fields.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Add created_at and updated_at fields to AccountWithSocials type - Derive created_at from timestamp field, updated_at from account_info - Update CreateArtistResult interface to use unknown for Json fields - Update test to account for new fields Co-Authored-By: Claude Opus 4.5 <[email protected]>
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
- Replace manual 18-line interface with type extension - Simplify return by spreading artist instead of manual field mapping - Type automatically inherits from Supabase schema Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Only required for organization API keys creating artists on behalf of other accounts - Falls back to extra.accountId from API key context if not provided Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Use Pick to reference CreateArtistResult instead of duplicating fields - Fix image access to use account_info[0]?.image (matches DRYed CreateArtistResult) - Update test mocks to include account_info array structure Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 participant
Summary
create_new_artistMCP tool from Recoup-Chat to recoup-apiComponents Added
insertAccountArtistId.ts: Associate artist with owner accountselectAccountWithSocials.ts: Fetch account with socials and infocreateArtistInDb.ts: Orchestrate artist creation (account, info, owner link, org link)copyRoom.ts: Copy conversation to new artistregisterCreateNewArtistTool.ts: MCP tool registration with zod schemaartists/index.ts: Artist tools indexTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Tools
✏️ Tip: You can customize this high-level summary in your review settings.