Remove UNIQUE constraint from playlog table and extend cleanup to 365 days - #5714
Draft
dmoo500 wants to merge 7 commits into
Draft
Remove UNIQUE constraint from playlog table and extend cleanup to 365 days#5714dmoo500 wants to merge 7 commits into
dmoo500 wants to merge 7 commits into
Conversation
Contributor
|
✅ The title and description are good to go. Thanks! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to preserve individual play events for yearly listening statistics.
Changes:
- Removes the playlog table’s inline uniqueness constraint.
- Adds schema migration 59.
- Extends retention from 90 to 365 days.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
controllers/music/constants.py |
Bumps database schema version. |
controllers/music/database.py |
Changes playlog schema and retention. |
controllers/music/migrations.py |
Rebuilds existing playlog tables. |
providers/plex/constants.py |
Updates retention-related commentary. |
providers/lastfm_recommendations/constants.py |
Updates retention-related commentary. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dmoo500
marked this pull request as draft
August 15, 2026 17:24
… days - Remove UNIQUE(item_id, provider, media_type, userid) constraint to allow accurate tracking of multiple plays of the same item - Increase playlog cleanup period from 90 to 365 days to enable year-long statistics - Add migration (schema v59) that recreates playlog table without constraint while preserving all existing data - Update comments referencing the old 90-day retention period
dmoo500
force-pushed
the
fix/playlog-unique-constraint
branch
from
August 15, 2026 17:34
6817d27 to
8a9809c
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Suppressed comments (5)
music_assistant/controllers/music/controller.py:2656
- [CRITICAL] The helper appends another default limit to this already-limited query, so every partial/resume write fails before it can update or insert a row; pass
limit=0here.
latest_rows = await self.database.get_rows_from_query(
f"SELECT id FROM {DB_TABLE_PLAYLOG} WHERE "
+ " AND ".join(f"{key} = :{key}" for key in PLAYLOG_CONFLICT_KEYS)
+ " ORDER BY timestamp DESC LIMIT 1",
conflict_params,
music_assistant/controllers/music/controller.py:2651
- [CRITICAL] Updating the latest row for every partial play can overwrite a prior completed event: after one completed play, stopping the next replay halfway changes that completed row to
fully_played=False, so the historical play disappears from statistics. Update only an incomplete row for the current playback (for example, scoped byqueue_id) and insert a new resume row when none exists.
else:
# For resume-state updates, update the most recent row for this item/user
# First, get the id of the latest row
conflict_params = {key: entry[key] for key in PLAYLOG_CONFLICT_KEYS}
music_assistant/controllers/music/controller.py:1748
- [CRITICAL] This query also receives the helper's default appended limit, producing invalid SQL (
LIMIT 1 LIMIT 500 OFFSET 0) whenever playback speed is read; passlimit=0for the embedded limit.
db_rows = await self.database.get_rows_from_query(
f"SELECT * FROM {DB_TABLE_PLAYLOG} WHERE "
"item_id = :item_id AND provider = :provider AND media_type = :media_type AND userid = :userid "
"ORDER BY timestamp DESC LIMIT 1",
music_assistant/providers/plex/constants.py:42
- [PROBLEM] The cache is explicitly the fallback after Plex rotates a mix out of its hub, but it now expires 275 days before the corresponding recently-played entry; a user returning after 90 days will see the retained mix and get
MediaNotFoundErrorwhen opening it. Keep this cache at least as long as playlog retention.
# rotating the mix out of its hub. 90 days is chosen as a reasonable cache window
# (playlog retention is 365 days, see controllers/music/database.py: _cleanup_database).
music_assistant/controllers/music/database.py:272
- [PROBLEM] Removing uniqueness also removes the only index led by the full item/user identity, while all three new latest-row lookups filter on those four columns and run on normal resume updates. Add a non-unique covering index such as
(item_id, provider, media_type, userid, timestamp DESC)so the 365-day append-only table does not require scanning a user's provider/media history.
[playback_speed] REAL NOT NULL DEFAULT 1.0
);"""
- Add limit=0 to get_resume_position and get_playback_speed queries to disable automatic pagination (prevents 'LIMIT 1 LIMIT 500' syntax error) - Deduplicate recently_played results by (item_id, provider, media_type) to prevent repeated plays from occupying all result slots
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
music_assistant/controllers/music/controller.py:722
- [CRITICAL] The outer join does not reapply
userid,queue_id,fully_played, oruser_initiatedfilters, so any row sharing the grouped item and timestamp is returned; multi-user writes deliberately share one timestamp, which guarantees duplicates in user-scoped results. Rank the already-filtered rows directly and select one deterministic row per item instead.
f"ON p.item_id = latest.item_id "
f" AND p.provider = latest.provider "
f" AND p.media_type = latest.media_type "
f" AND p.timestamp = latest.max_timestamp "
music_assistant/controllers/music/controller.py:2663
- [CRITICAL] A completed write that omits
playback_speedinserts the schema default1.0, so provider syncs and ordinary API calls reset an audiobook/podcast's saved custom speed even thoughmark_item_playedpromises to preserve it. Copy the latest stored speed into the appended row when the caller does not supply one.
# For completed plays, always insert a new row to preserve history
if entry.get("fully_played"):
- Add limit=0 to disable pagination (prevents LIMIT 1 LIMIT 500 syntax error) - Check if latest row is incomplete before updating to prevent overwriting completed play history - Add fully_played to SELECT and id DESC to ORDER BY for deterministic selection - If latest row is completed, INSERT new row for new playback session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
music_assistant/controllers/music/controller.py:722
- [CRITICAL] The outer join is not scoped by
userid,queue_id,fully_played, or the other subquery filters, so rows from another user with the same item and timestamp also join (the all-user write path deliberately gives users the same timestamp), producing duplicates and consuming result slots. Rank the filtered rows themselves and return only row 1 per item.
f"ON p.item_id = latest.item_id "
f" AND p.provider = latest.provider "
f" AND p.media_type = latest.media_type "
f" AND p.timestamp = latest.max_timestamp "
music_assistant/controllers/music/controller.py:2667
- [CRITICAL] This insert defaults
playback_speedto 1.0 when a completed provider report omits it, resetting the custom speed even thoughmark_item_played(..., playback_speed=None)promises to preserve the stored value. Carry the latest speed forward whenever an inserted entry omits this column.
await self.database.execute_write(
f"INSERT INTO {DB_TABLE_PLAYLOG} ({', '.join(columns)}) "
f"VALUES ({', '.join(f':{column}' for column in columns)})",
entry,
music_assistant/providers/plex/constants.py:42
- [PROBLEM] Keeping playlog rows for 365 days while expiring this fallback after 90 days leaves Plex mixes in Recently Played that can no longer be resolved once Plex rotates them out of the hub. Match the cache lifetime to the new playlog retention so every retained mix remains replayable.
# rotating the mix out of its hub. 90 days is chosen as a reasonable cache window
# (playlog retention is 365 days, see controllers/music/database.py: _cleanup_database).
music_assistant/controllers/music/database.py:686
- [PROBLEM] Removing the unique index also removes the only item-leading index, but every resume update/read now searches this append-only table by item/provider/media/user and orders by timestamp. With 365 days of events, these hot paths scan and sort many rows; retain the same key as a non-unique latest-state index.
# speed up recency lookups (smart shuffle / dedup) by user and time window
When inserting a completed play (fully_played=1), mark any outstanding incomplete rows (fully_played=0) for the same item/user as completed. This prevents completed audiobooks/podcasts from remaining in in_progress_items() indefinitely, while preserving the history rows.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Suppressed comments (4)
music_assistant/controllers/music/controller.py:723
- [CRITICAL] The outer join is not constrained by the subquery filters: a second row with the same item and timestamp but a different user, queue, or completion state also joins and can leak into these results. Rank rows inside the filtered set and select rank 1 so every returned row actually satisfied
where_clause.
f"ON p.item_id = latest.item_id "
f" AND p.provider = latest.provider "
f" AND p.media_type = latest.media_type "
f" AND p.timestamp = latest.max_timestamp "
f"ORDER BY p.timestamp DESC"
music_assistant/controllers/music/controller.py:2670
- [CRITICAL] A normal audiobook/podcast session first creates an incomplete resume row; marking that row completed and then inserting the completion records two completed events for one play, doubling play statistics. Remove the superseded resume row (or update it without the additional insert) before recording the single completion event.
await self.database.execute_write(
f"UPDATE {DB_TABLE_PLAYLOG} SET fully_played = 1 "
f"WHERE fully_played = 0 "
f"AND {' AND '.join(f'{key} = :{key}' for key in PLAYLOG_CONFLICT_KEYS)}",
music_assistant/controllers/music/database.py:272
- [CRITICAL] Removing this constraint also disables the
INSERT OR REPLACEbehavior inmedia/audiobooks.py:580-595; each provider sync now appends another resume row, while its unorderedget_rowat lines 562-570 can inspect stale state. Route that path through the latest-row resume update and make its read deterministic as part of this schema change.
[playback_speed] REAL NOT NULL DEFAULT 1.0
);"""
tests/controllers/music/test_music_migrations.py:194
- [PROBLEM] These tests use raw
INSERTs and only prove that the schema accepts duplicate keys; they never exercise the new_upsert_playlogstate/event contract. Add integration coverage showing partial→complete creates exactly one completed event, repeated provider state synchronization is idempotent, and two actual completed playbacks create exactly two events.
# After migration, multiple plays of the same item by the same user create separate rows
await database.execute(
f"INSERT INTO {DB_TABLE_PLAYLOG} (item_id, provider, media_type, name, userid, "
"timestamp, fully_played, seconds_played, queue_id, user_initiated) "
"VALUES ('1', 'library', 'track', 'Test Track', 'user1', 200, 1, 195, 'queue2', 1)"
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Suppressed comments (2)
music_assistant/controllers/music/controller.py:2681
- [CRITICAL] A five-minute time window collapses legitimate repeat plays—for example, completing a short track twice updates the first event instead of recording two plays, contradicting the repeat behavior covered by
tests/controllers/player_queues/test_play_report_dedup.py:34-41; deduplicate provider reports with an explicit report/session identity rather than elapsed time.
# Check for recent completed play (within 5 minutes) to avoid duplicates from provider syncs
recent_threshold = int(time.time()) - 300
recent_completed = await self.database.get_rows_from_query(
f"SELECT id, user_initiated, timestamp FROM {DB_TABLE_PLAYLOG} WHERE "
+ " AND ".join(f"{key} = :{key}" for key in PLAYLOG_CONFLICT_KEYS)
+ f" AND fully_played = 1 AND timestamp >= {recent_threshold} "
+ "ORDER BY timestamp DESC LIMIT 1",
music_assistant/controllers/music/database.py:686
- [PROBLEM] Dropping the unique constraint also removes the only index led by
(item_id, provider, media_type, userid), so the new upsert/resume queries repeatedly scan an ever-growing year of history; retain equivalent lookup performance with a non-unique composite index.
# speed up recency lookups (smart shuffle / dedup) by user and time window
1. recently_played: JOIN with id for deterministic row selection 2. _upsert_playlog: Copy entry dict to prevent cross-user mutation 3. audiobooks.py: Migrate _set_playlog to append-only _upsert_playlog 4. Preserve playback_speed when creating new resume row after completed play
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Suppressed comments (4)
music_assistant/controllers/music/controller.py:2728
- [CRITICAL] A completed provider sync can omit
playback_speed; when no completion exists in the five-minute window, this insert applies the1.0default andget_playback_speed()then reads that newest row, losing the user's saved speed. Carry the latest non-null speed into completed inserts just as the resume insert branch does.
# Insert new completed play
await self.database.execute_write(
f"INSERT INTO {DB_TABLE_PLAYLOG} ({', '.join(columns)}) "
f"VALUES ({', '.join(f':{column}' for column in columns)})",
entry,
music_assistant/controllers/music/controller.py:2685
- [CRITICAL] The five-minute heuristic merges legitimate consecutive plays—for example, replaying a three-minute track—so the append-only history and statistics still undercount. Provider-state idempotency needs an explicit event/session identity or a separate synchronization path rather than a wall-clock window.
# Check for recent completed play (within 5 minutes) to avoid duplicates from provider syncs
recent_threshold = int(time.time()) - 300
recent_completed = await self.database.get_rows_from_query(
f"SELECT id, user_initiated, timestamp FROM {DB_TABLE_PLAYLOG} WHERE "
+ " AND ".join(f"{key} = :{key}" for key in PLAYLOG_CONFLICT_KEYS)
+ f" AND fully_played = 1 AND timestamp >= {recent_threshold} "
+ "ORDER BY timestamp DESC LIMIT 1",
music_assistant/providers/plex/constants.py:42
- [CRITICAL] The playlog now retains Plex mixes for 365 days, but their only fallback metadata still expires after 90; once Plex rotates a mix out of its hub,
_find_mix_by_key()returnsNone, so a retained recently-played mix becomes unresolvable after day 90. Keep this cache for at least the playlog retention window.
# Mix title/artwork are cached so replay from recently-played survives Plex
# rotating the mix out of its hub. 90 days is chosen as a reasonable cache window
# (playlog retention is 365 days, see controllers/music/database.py: _cleanup_database).
MIX_CACHE_EXPIRATION = 86400 * 90
music_assistant/controllers/music/database.py:272
- [PROBLEM] Removing the unique key also removes the only index led by
(item_id, provider, media_type, userid), so every latest-resume/speed lookup now scans the user's timestamp index, with an append-only 365-day table making that cost grow continuously. Recreate the same key as a non-unique index, ideally followed bytimestamp DESC, id DESC.
[playback_speed] REAL NOT NULL DEFAULT 1.0
);"""
Comment on lines
+712
to
+727
| query = ( | ||
| f"SELECT p.* FROM {DB_TABLE_PLAYLOG} p " | ||
| f"INNER JOIN (" | ||
| f" SELECT item_id, provider, media_type, MAX(timestamp) as max_timestamp, " | ||
| f" MAX(id) as max_id " | ||
| f" FROM {DB_TABLE_PLAYLOG} " | ||
| f" WHERE {where_clause} " | ||
| f" GROUP BY item_id, provider, media_type, timestamp " | ||
| f") latest " | ||
| f"ON p.item_id = latest.item_id " | ||
| f" AND p.provider = latest.provider " | ||
| f" AND p.media_type = latest.media_type " | ||
| f" AND p.timestamp = latest.max_timestamp " | ||
| f" AND p.id = latest.max_id " | ||
| f"ORDER BY p.timestamp DESC, p.id DESC" | ||
| ) |
Comment on lines
+2671
to
+2675
| # Retire any outstanding incomplete rows for this item/user | ||
| await self.database.execute_write( | ||
| f"UPDATE {DB_TABLE_PLAYLOG} SET fully_played = 1 " | ||
| f"WHERE fully_played = 0 " | ||
| f"AND {' AND '.join(f'{key} = :{key}' for key in PLAYLOG_CONFLICT_KEYS)}", |
Comment on lines
+2734
to
+2737
| latest_rows = await self.database.get_rows_from_query( | ||
| f"SELECT id, fully_played FROM {DB_TABLE_PLAYLOG} WHERE " | ||
| + " AND ".join(f"{key} = :{key}" for key in PLAYLOG_CONFLICT_KEYS) | ||
| + " ORDER BY timestamp DESC, id DESC LIMIT 1", |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this implement/fix?
Implements an append-only playlog architecture to allow accurate tracking of multiple plays of the same item while maintaining resume-state functionality for audiobooks and podcasts.
Changes:
_upsert_playlog: completed plays (fully_played=True) INSERT new rows to preserve history, resume-state updates (fully_played=False) UPDATE the latest rowget_resume_positionandget_playback_speedto deterministically select the latest entry usingORDER BY timestamp DESC LIMIT 1Migration:
SQLite doesn't support
DROP CONSTRAINT, so the migration recreates the table without the constraint while preserving all existing data. The migration explicitly lists columns and usesCOALESCEto handle missing fields for backward compatibility.Related issue (if applicable):
Types of changes
bugfixChecklist
pre-commit run --all-filespasses.pytestpasses, and tests have been added/updated undertests/where applicable.music-assistant/modelsis linked.music-assistant/frontendis linked.