Use addedAt for playlist song ordering instead of id

[Copilot]

getPlaylistWithSongs was ordering by playlistSongs.id (auto-increment PK) rather than the semantic timestamp field. While typically equivalent, id ordering is semantically incorrect and fragile.

Changes

• server/storage.ts: Switch orderBy(playlistSongs.id) → orderBy(playlistSongs.addedAt), consistent with getLikedSongs which orders by songLikes.createdAt

---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

---

Summary by cubic

Refactored getPlaylistWithSongs to use innerJoin and sort songs by addedAt for correct playlist order. The method now returns only playlists with valid songs, reducing null checks and improving query semantics.

• Refactors

  • Inner joins between playlists, playlist_songs, and songs; streamlined select/mapping to drop nullable song fields.
  • Order by playlistSongs.addedAt instead of id.

• Migration

  • If callers need playlists with zero songs, use a separate fetch.

^{Written for commit 53b8e78. Summary will update on new commits.}

[qodo-code-review]

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Order playlist songs by addedAt timestamp for semantic correctness

🐞 Bug fix

Walkthroughs

Description

• Order playlist songs by addedAt timestamp instead of auto-increment id
• Ensures semantic correctness and consistency with getLikedSongs ordering
• Fixes fragile ordering that relied on insertion order rather than explicit timestamp

Diagram

flowchart LR
  A["getPlaylistWithSongs<br/>method"] -->|"previously ordered by"| B["playlistSongs.id<br/>auto-increment PK"]
  A -->|"now ordered by"| C["playlistSongs.addedAt<br/>semantic timestamp"]
  C -->|"consistent with"| D["getLikedSongs<br/>ordering pattern"]

Loading

File Changes

1. server/storage.ts 🐞 Bug fix +1/-1

Replace id ordering with addedAt timestamp

• Changed orderBy(playlistSongs.id) to orderBy(playlistSongs.addedAt) in getPlaylistWithSongs
 method
• Uses semantic timestamp field for playlist song ordering instead of primary key
• Aligns with similar ordering pattern used in getLikedSongs method

server/storage.ts

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Non-deterministic song ordering 🐞 Bug ✓ Correctness

Description

Ordering only by playlistSongs.addedAt can produce inconsistent song order when multiple rows
share the same timestamp (common with limited timestamp precision) or when addedAt is NULL. This
can cause playlists to appear to “shuffle” between requests even when data hasn’t changed.

Code

server/storage.ts[168]

+      .orderBy(playlistSongs.addedAt);

Evidence

The updated query orders by a nullable, non-unique timestamp column. The schema does not enforce
addedAt as NOT NULL, and the codebase already demonstrates explicit NULL ordering handling
elsewhere (nullsLast()), which is missing here.

server/storage.ts[159-170]
shared/schema.ts[70-75]
server/storage.ts[137-144]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`getPlaylistWithSongs` orders songs only by `playlistSongs.addedAt`. Since `addedAt` is nullable and not guaranteed unique, playlists can return songs in a non-deterministic order (ties/NULLs), causing inconsistent UI ordering.

### Issue Context
- `playlistSongs.addedAt` is defined without `.notNull()`, so NULLs are possible.
- A similar pattern (`getLikedSongs`) explicitly uses `.nullsLast()` when ordering by a timestamp.

### Fix Focus Areas
- server/storage.ts[159-169]
- shared/schema.ts[70-75]

### Suggested change
- Import `asc` from `drizzle-orm`.
- Update the query to use deterministic ordering, e.g.:
 - `.orderBy(asc(playlistSongs.addedAt).nullsLast(), asc(playlistSongs.id))`
 - (Optionally) if you want to preserve older behavior for NULL timestamps, consider ordering by `id` when `addedAt` is NULL via `nullsLast()` plus the `id` tiebreaker.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[cubic-dev-ai]

No issues found across 1 file

[qodo-code-review]

1. Non-deterministic song ordering 🐞 Bug ✓ Correctness

Ordering only by playlistSongs.addedAt can produce inconsistent song order when multiple rows
share the same timestamp (common with limited timestamp precision) or when addedAt is NULL. This
can cause playlists to appear to “shuffle” between requests even when data hasn’t changed.

Agent Prompt

### Issue description
`getPlaylistWithSongs` orders songs only by `playlistSongs.addedAt`. Since `addedAt` is nullable and not guaranteed unique, playlists can return songs in a non-deterministic order (ties/NULLs), causing inconsistent UI ordering.

### Issue Context
- `playlistSongs.addedAt` is defined without `.notNull()`, so NULLs are possible.
- A similar pattern (`getLikedSongs`) explicitly uses `.nullsLast()` when ordering by a timestamp.

### Fix Focus Areas
- server/storage.ts[159-169]
- shared/schema.ts[70-75]

### Suggested change
- Import `asc` from `drizzle-orm`.
- Update the query to use deterministic ordering, e.g.:
  - `.orderBy(asc(playlistSongs.addedAt).nullsLast(), asc(playlistSongs.id))`
  - (Optionally) if you want to preserve older behavior for NULL timestamps, consider ordering by `id` when `addedAt` is NULL via `nullsLast()` plus the `id` tiebreaker.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[Copilot]

Pull request overview

Updates playlist song ordering to use the semantic addedAt timestamp (instead of the auto-increment id) so playlist tracks return in the order they were added.

Changes:

• Change getPlaylistWithSongs ordering from playlistSongs.id to playlistSongs.addedAt.

[Copilot]

playlistSongs.addedAt is nullable in the schema (timestamp().defaultNow() without .notNull()), so ordering by it can put legacy/null rows first and yield unexpected playlist ordering. Consider ordering with null handling (e.g., nulls last) and adding a deterministic tiebreaker (e.g., then by playlistSongs.id), or making addedAt non-nullable + backfilling via migration.