Fix novelty join on search results and make provider aware - #118
Merged
Conversation
get_novelty_for_search_results/1 joins the novelty views onto a raw SQL VALUES list and zips the rows back onto the track list by position. The query has no ORDER BY, so Postgres returns rows in join order and every score lands on the wrong track. Reproduced on PG16. Writes up the cause, the reproduction, four secondary problems in the same function (empty result set is a syntax error, hand-rolled quoting, nil artist raises, view schemas carry an implicit id primary key), the keyed-lookup replacement, and the test cases to add. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_015yYk8F9wZPwjLtc5yNgWn6
get_novelty_for_search_results/1 built a raw SQL VALUES list from the search results, left joined the novelty views onto it, selected only the two novelty columns, then zipped the rows back onto the track list by position. The query has no ORDER BY, so Postgres returns rows in join order rather than input order and every score lands on the wrong track. Because the planner probes the VALUES list from the view side, the low-scoring rows come back first and the brand new tracks systematically collect them. Replace it with two keyed lookups merged in Elixir. Association is now by (provider, external_id) and artist rather than by row position, so result ordering cannot matter. This also clears four other problems in the same function: an empty result set built "VALUES )" and was a syntax error, a nil artist raised in the hand-rolled quoting, escape_quotes/1 was the only thing separating provider-supplied text from the query, and the view schemas carried an implicit id primary key that the views do not have. Also make track novelty provider-aware. recent_plays already carried provider but track_novelty grouped by external_id alone, so tracks sharing an id across providers pooled their play counts, and both join sites matched on external_id alone. Recreate the view grouped by (external_id, provider) and join on both. Adds novelty coverage to queue_test, which had none. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_015yYk8F9wZPwjLtc5yNgWn6
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
2 participants
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.
No description provided.