Skip to content

⚡ Bolt: Optimize playlist song fetching and fix type errors #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Krosebrook wants to merge 1 commit into
base: main
Choose a base branch
from
Open

⚡ Bolt: Optimize playlist song fetching and fix type errors #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions .jules/bolt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2026-02-10 - Atomic Database Updates
**Learning:** Read-modify-write patterns for counters (like `playCount`) cause race conditions and extra DB round trips.
**Action:** Use atomic SQL updates (e.g., `playCount = playCount + 1`) with `returning()` to ensure data integrity and performance.

## 2026-02-11 - Drizzle Query Instability
**Learning:** The `.nullsLast()` method on `desc()` sort operations causes type errors (`Property 'nullsLast' does not exist`) in this environment.
**Action:** Avoid `.nullsLast()` and rely on default null handling (or explicit `sql` fragments) for sorting.
24 changes: 7 additions & 17 deletions server/storage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
type Playlist,
type InsertPlaylist,
} from "@shared/schema";
import { eq, desc, and, inArray, sql, getTableColumns } from "drizzle-orm";
import { eq, desc, asc, and, inArray, sql, getTableColumns } from "drizzle-orm";
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

inArray is still imported from drizzle-orm but no longer used in this file after the playlist query rewrite. Please remove it from the import list to keep the module clean and avoid reintroducing unused-query patterns later.

Suggested change
import { eq, desc, asc, and, inArray, sql, getTableColumns } from "drizzle-orm";
import { eq, desc, asc, and, sql, getTableColumns } from "drizzle-orm";

Copilot uses AI. Check for mistakes.

export interface IStorage {
// Song CRUD
Expand Down Expand Up @@ -140,7 +140,7 @@ export class DatabaseStorage implements IStorage {
.from(songs)
.innerJoin(songLikes, eq(songs.id, songLikes.songId))
.where(eq(songLikes.userId, userId))
.orderBy(desc(songLikes.createdAt).nullsLast());
.orderBy(desc(songLikes.createdAt));
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 16, 2026

Choose a reason for hiding this comment

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

P2 Badge Restore NULLS LAST for liked-song timestamp sort

Changing the sort to orderBy(desc(songLikes.createdAt)) alters behavior for nullable timestamps: in Postgres, DESC places NULL values first unless NULLS LAST is explicit, so any song_likes rows with missing created_at will now surface at the top instead of the bottom. This is a real data-path risk because createdAt is defined without .notNull() in shared/schema.ts, so nulls are schema-valid; keep nulls-last semantics via an explicit SQL ordering or enforce non-null timestamps.

Useful? React with 👍 / 👎.

Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

songLikes.createdAt is nullable in the schema (timestamp(...).defaultNow() without .notNull()), and removing .nullsLast() changes Postgres’ default sort behavior for DESC (nulls will come first). If you want stable “most recently liked first” ordering, either make createdAt non-nullable in the schema/migration or use an explicit ORDER BY ... NULLS LAST via a sql fragment/coalesce workaround.

Suggested change
.orderBy(desc(songLikes.createdAt));
.orderBy(desc(sql`coalesce(${songLikes.createdAt}, to_timestamp(0))`));

Copilot uses AI. Check for mistakes.
}

// === Playlists ===
Expand All @@ -160,22 +160,12 @@ export class DatabaseStorage implements IStorage {
const playlist = await this.getPlaylist(id);
if (!playlist) return undefined;

const playlistSongRows = await db.select({ songId: playlistSongs.songId })
.from(playlistSongs)
.where(eq(playlistSongs.playlistId, id));

const songIds = playlistSongRows.map(r => r.songId).filter(id => typeof id === 'number' && !isNaN(id));

if (songIds.length === 0) {
return { ...playlist, songs: [] };
}

const songsResult = await db.select()
// Optimized: Single query with innerJoin to avoid N+1 and ID mapping overhead
const songsList = await db.select(getTableColumns(songs))
.from(songs)
.where(inArray(songs.id, songIds));

const songMap = new Map(songsResult.map(s => [s.id, s]));
const songsList = songIds.map(id => songMap.get(id)).filter((s): s is Song => !!s);
.innerJoin(playlistSongs, eq(songs.id, playlistSongs.songId))
.where(eq(playlistSongs.playlistId, id))
.orderBy(asc(playlistSongs.addedAt));

return { ...playlist, songs: songsList };
Comment on lines 160 to 170
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 16, 2026

Choose a reason for hiding this comment

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

medium

Great job on optimizing this function to reduce the number of database queries! While this is a solid improvement from 3 queries to 2, we can actually consolidate this into a single query using Drizzle's relational query features. This would make it even more efficient by avoiding the separate getPlaylist call.

By using db.query.playlists.findFirst with the with clause, you can fetch the playlist and all its related songs in one go.

Suggested change
const playlist = await this.getPlaylist(id);
if (!playlist) return undefined;
const playlistSongRows = await db.select({ songId: playlistSongs.songId })
.from(playlistSongs)
.where(eq(playlistSongs.playlistId, id));
const songIds = playlistSongRows.map(r => r.songId).filter(id => typeof id === 'number' && !isNaN(id));
if (songIds.length === 0) {
return { ...playlist, songs: [] };
}
const songsResult = await db.select()
// Optimized: Single query with innerJoin to avoid N+1 and ID mapping overhead
const songsList = await db.select(getTableColumns(songs))
.from(songs)
.where(inArray(songs.id, songIds));
const songMap = new Map(songsResult.map(s => [s.id, s]));
const songsList = songIds.map(id => songMap.get(id)).filter((s): s is Song => !!s);
.innerJoin(playlistSongs, eq(songs.id, playlistSongs.songId))
.where(eq(playlistSongs.playlistId, id))
.orderBy(asc(playlistSongs.addedAt));
return { ...playlist, songs: songsList };
const playlistData = await db.query.playlists.findFirst({
where: eq(playlists.id, id),
with: {
playlistSongs: {
orderBy: asc(playlistSongs.addedAt),
with: {
song: true,
},
},
},
});
if (!playlistData) {
return undefined;
}
const { playlistSongs, ...playlist } = playlistData;
const songsList = playlistSongs.map((ps) => ps.song).filter((s): s is Song => !!s);
return { ...playlist, songs: songsList };

}
Expand Down