Skip to content
Merged

Test #686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions lib/research/playcounts/__tests__/mapUnmappedAlbumTracks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import generateAccessToken from "@/lib/spotify/generateAccessToken";
import getTracks from "@/lib/spotify/getTracks";
import { upsertSongs } from "@/lib/supabase/songs/upsertSongs";
import { upsertSongIdentifiers } from "@/lib/supabase/song_identifiers/upsertSongIdentifiers";
import { linkSongsToArtists } from "@/lib/songs/linkSongsToArtists";
import { queueRedisSongs } from "@/lib/songs/queueRedisSongs";
import { SpotifyRateLimitError } from "@/lib/spotify/SpotifyRateLimitError";

vi.mock("@/lib/spotify/generateAccessToken", () => ({ default: vi.fn() }));
Expand All @@ -13,6 +15,8 @@ vi.mock("@/lib/supabase/songs/upsertSongs", () => ({ upsertSongs: vi.fn() }));
vi.mock("@/lib/supabase/song_identifiers/upsertSongIdentifiers", () => ({
upsertSongIdentifiers: vi.fn(),
}));
vi.mock("@/lib/songs/linkSongsToArtists", () => ({ linkSongsToArtists: vi.fn() }));
vi.mock("@/lib/songs/queueRedisSongs", () => ({ queueRedisSongs: vi.fn() }));

const ALBUMS = [
{
Expand Down Expand Up @@ -55,6 +59,32 @@ describe("mapUnmappedAlbumTracks", () => {
expect([...mapped.entries()]).toEqual([["t_new", "ISRC_NIKES"]]);
});

it("links captured songs to their Spotify artists and queues them for note enrichment", async () => {
vi.mocked(getTracks).mockResolvedValue({
tracks: [
{
id: "t_new",
name: "Nikes on My Feet",
external_ids: { isrc: "ISRC_NIKES" },
artists: [{ id: "a1", name: "Mac Miller" }],
},
],
error: null,
} as never);

await mapUnmappedAlbumTracks(ALBUMS, new Set(["t_mapped", "t_noisrc"]));

// Root-cause fix: captured songs get the same enrichment as the manual flow —
// artists linked + queued for notes — so they aren't "missing info" in the catalog.
expect(linkSongsToArtists).toHaveBeenCalledWith([
expect.objectContaining({
isrc: "ISRC_NIKES",
spotifyArtists: [{ id: "a1", name: "Mac Miller" }],
}),
]);
expect(queueRedisSongs).toHaveBeenCalledWith([expect.objectContaining({ isrc: "ISRC_NIKES" })]);
});

it("returns an empty map without Spotify calls when everything is mapped", async () => {
const mapped = await mapUnmappedAlbumTracks(ALBUMS, new Set(["t_mapped", "t_new", "t_noisrc"]));

Expand Down
34 changes: 30 additions & 4 deletions lib/research/playcounts/mapUnmappedAlbumTracks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import { upsertSongs } from "@/lib/supabase/songs/upsertSongs";
import { upsertSongIdentifiers } from "@/lib/supabase/song_identifiers/upsertSongIdentifiers";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Custom agent: Enforce Clear Code Style and Maintainability Practices

File exceeds the 100-line limit (107 lines). Split secondary concerns (artist linking and Redis queuing) into a separate module or helper to bring this file back under 100 lines and preserve single responsibility.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/research/playcounts/mapUnmappedAlbumTracks.ts, line 98:

<comment>File exceeds the 100-line limit (107 lines). Split secondary concerns (artist linking and Redis queuing) into a separate module or helper to bring this file back under 100 lines and preserve single responsibility.</comment>

<file context>
@@ -72,6 +91,13 @@ export async function mapUnmappedAlbumTracks(
+    // manual/CSV flow — link artists (auto-creating the artist account) and
+    // queue note generation — so valuation tracks aren't "missing info" and
+    // render in the catalog view rather than being filtered out.
+    await linkSongsToArtists(songs);
+    await queueRedisSongs(songs);
+
</file context>

import { SpotifyAlbumPlayCounts } from "@/lib/apify/spotify/fetchSpotifyAlbumPlayCounts";
import { SpotifyRateLimitError } from "@/lib/spotify/SpotifyRateLimitError";
import { getSpotifyArtists } from "@/lib/songs/getSpotifyArtists";
import { linkSongsToArtists } from "@/lib/songs/linkSongsToArtists";
import { queueRedisSongs } from "@/lib/songs/queueRedisSongs";
import { SongWithSpotify } from "@/lib/songs/getSongsByIsrc";

/**
* Self-mapping bootstrap (chat#1794): resolve ISRCs for actor tracks that have
Expand Down Expand Up @@ -49,20 +53,35 @@ export async function mapUnmappedAlbumTracks(
name: track.name,
albumId: context.albumId,
albumName: context.albumName,
spotifyArtists: getSpotifyArtists(track.artists ?? []),
},
];
});
if (resolved.length === 0) return new Map();

// Dedupe by ISRC: reissues put the same recording on several albums in one
// batch, and upsertSongs' DO UPDATE cannot affect the same row twice.
const songsByIsrc = new Map(
// batch, and upsertSongs' DO UPDATE cannot affect the same row twice. Carry
// the Spotify artists through for enrichment.
const songsByIsrc = new Map<string, SongWithSpotify>(
resolved.map(r => [
r.isrc,
{ isrc: r.isrc, name: r.name ?? null, album: r.albumName ?? null },
{
isrc: r.isrc,
name: r.name ?? null,
album: r.albumName ?? null,
spotifyArtists: r.spotifyArtists,
},
]),
);
await upsertSongs([...songsByIsrc.values()]);
const songs = [...songsByIsrc.values()];

await upsertSongs(
songs.map(song => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { spotifyArtists, ...record } = song;
return record;
}),
);
await upsertSongIdentifiers(
resolved.flatMap(r => [
{ song: r.isrc, platform: "spotify", identifier_type: "track_id", value: r.trackId },
Expand All @@ -72,6 +91,13 @@ export async function mapUnmappedAlbumTracks(
]),
);

// Root cause (chat#1801): give captured songs the same enrichment as the
// manual/CSV flow — link artists (auto-creating the artist account) and
// queue note generation — so valuation tracks aren't "missing info" and
// render in the catalog view rather than being filtered out.
await linkSongsToArtists(songs);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Artist-link enrichment failure now aborts mapping output. This causes successful identifier upserts to be treated as failed bootstrap and retried unnecessarily.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/research/playcounts/mapUnmappedAlbumTracks.ts, line 98:

<comment>Artist-link enrichment failure now aborts mapping output. This causes successful identifier upserts to be treated as failed bootstrap and retried unnecessarily.</comment>

<file context>
@@ -72,6 +91,13 @@ export async function mapUnmappedAlbumTracks(
+    // manual/CSV flow — link artists (auto-creating the artist account) and
+    // queue note generation — so valuation tracks aren't "missing info" and
+    // render in the catalog view rather than being filtered out.
+    await linkSongsToArtists(songs);
+    await queueRedisSongs(songs);
+
</file context>
Suggested change
await linkSongsToArtists(songs);
await linkSongsToArtists(songs).catch(error => {
console.error("[playcounts] artist linking failed:", error);
});

await queueRedisSongs(songs);

return new Map(resolved.map(r => [r.trackId, r.isrc]));
} catch (error) {
if (error instanceof SpotifyRateLimitError) throw error;
Expand Down
Loading