feat(discover): explain the taste match on both clients — #2377 (clients)
"Matches your taste in shoegaze and dream pop." replaces the seed
attribution when the candidate's own tags overlap the taste profile.
The preference order is the point of slice 6: the tag reason describes the
MUSIC ("sounds like what you like"), while seed attribution describes the
graph ("adjacent to something you played"). When we can say the former, it
is strictly the better explanation. When we can't — the common case, since
tag coverage for out-of-library artists is partial by nature (#2376) — the
card falls back to attribution rather than going blank.
Both clients share the wording, Oxford comma included, and both have tests
asserting the exact strings. That's deliberate: identical copy across two
codebases silently diverges unless something fails when it does.
Android caps at 3 tags client-side even though the server already does.
The server contract could widen; a run-on subtitle shouldn't be how we
find out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -179,6 +179,7 @@ private fun ArtistSuggestionWire.toDomain(): ArtistSuggestionRef = ArtistSuggest
|
||||
name = name,
|
||||
imageUrl = imageUrl,
|
||||
attribution = attribution.map { it.toDomain() },
|
||||
matchedTags = matchedTags,
|
||||
)
|
||||
|
||||
private fun SeedContributionWire.toDomain(): SeedContributionRef = SeedContributionRef(
|
||||
|
||||
@@ -61,7 +61,7 @@ internal fun SuggestionTile(
|
||||
// Once parked, the "because you liked X" line is no longer the
|
||||
// useful thing to say — confirming what just happened is.
|
||||
val secondary =
|
||||
if (snoozed) "Not right now — hidden for a while" else s.attributionText
|
||||
if (snoozed) "Not right now — hidden for a while" else s.reasonText
|
||||
if (secondary.isNotEmpty()) {
|
||||
Text(
|
||||
text = secondary,
|
||||
|
||||
@@ -52,6 +52,8 @@ data class ArtistSuggestionRef(
|
||||
val name: String,
|
||||
val imageUrl: String = "",
|
||||
val attribution: List<SeedContributionRef> = emptyList(),
|
||||
/** Taste-profile tags this candidate matches, strongest first (#2377). */
|
||||
val matchedTags: List<String> = emptyList(),
|
||||
) {
|
||||
val attributionText: String
|
||||
get() {
|
||||
@@ -66,6 +68,30 @@ data class ArtistSuggestionRef(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The subtitle line for the card.
|
||||
*
|
||||
* Prefers the taste-tag reason over seed attribution when we have one,
|
||||
* because it describes the MUSIC ("sounds like what you like") rather than
|
||||
* the graph ("adjacent to something you played") — the whole point of
|
||||
* milestone #268 slice 6. Falls back to attribution, which is the common
|
||||
* case: tag coverage for out-of-library artists is partial by nature
|
||||
* (#2376), so most candidates have no matched tags.
|
||||
*
|
||||
* Kept in lockstep with the web client's reasonText() in
|
||||
* SuggestionFeed.svelte — same wording, same Oxford comma.
|
||||
*/
|
||||
val reasonText: String
|
||||
get() {
|
||||
val tags = matchedTags.take(MAX_ATTRIBUTION_PHRASES)
|
||||
return when (tags.size) {
|
||||
0 -> attributionText
|
||||
1 -> "Matches your taste in ${tags[0]}."
|
||||
2 -> "Matches your taste in ${tags[0]} and ${tags[1]}."
|
||||
else -> "Matches your taste in ${tags[0]}, ${tags[1]}, and ${tags[2]}."
|
||||
}
|
||||
}
|
||||
|
||||
companion object {
|
||||
private const val MAX_ATTRIBUTION_PHRASES = 3
|
||||
}
|
||||
|
||||
@@ -35,6 +35,13 @@ data class ArtistSuggestionWire(
|
||||
val name: String = "",
|
||||
@SerialName("image_url") val imageUrl: String = "",
|
||||
val attribution: List<SeedContributionWire> = emptyList(),
|
||||
/**
|
||||
* Tags this candidate shares with the user's taste profile, strongest
|
||||
* first (max 3, #2377). Absent for most candidates — tag coverage for
|
||||
* out-of-library artists is partial by nature (#2376) — so the default
|
||||
* empty list is the common case, not an error.
|
||||
*/
|
||||
@SerialName("matched_tags") val matchedTags: List<String> = emptyList(),
|
||||
)
|
||||
|
||||
/**
|
||||
|
||||
+85
@@ -0,0 +1,85 @@
|
||||
package com.fabledsword.minstrel.models
|
||||
|
||||
import org.junit.jupiter.api.Test
|
||||
import kotlin.test.assertEquals
|
||||
|
||||
/**
|
||||
* The card's subtitle line (#2377). Wording is kept in lockstep with the web
|
||||
* client's reasonText() in SuggestionFeed.svelte — these assertions are the
|
||||
* record of what that wording IS, so a change on one client without the other
|
||||
* shows up as a failure rather than as silent divergence between the two
|
||||
* surfaces.
|
||||
*/
|
||||
class ArtistSuggestionReasonTest {
|
||||
|
||||
private val seeds = listOf(
|
||||
SeedContributionRef(name = "Seed", isLiked = true),
|
||||
)
|
||||
|
||||
private fun suggestion(
|
||||
matched: List<String> = emptyList(),
|
||||
attribution: List<SeedContributionRef> = seeds,
|
||||
) = ArtistSuggestionRef(
|
||||
mbid = "mb",
|
||||
name = "Candidate",
|
||||
attribution = attribution,
|
||||
matchedTags = matched,
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `one matched tag reads in the singular`() {
|
||||
assertEquals(
|
||||
"Matches your taste in shoegaze.",
|
||||
suggestion(matched = listOf("shoegaze")).reasonText,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `two matched tags join with and`() {
|
||||
assertEquals(
|
||||
"Matches your taste in shoegaze and dream pop.",
|
||||
suggestion(matched = listOf("shoegaze", "dream pop")).reasonText,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `three matched tags use an Oxford comma`() {
|
||||
assertEquals(
|
||||
"Matches your taste in a, b, and c.",
|
||||
suggestion(matched = listOf("a", "b", "c")).reasonText,
|
||||
)
|
||||
}
|
||||
|
||||
// The server caps at 3, but the client must not render a run-on line if a
|
||||
// future server sends more.
|
||||
@Test
|
||||
fun `more than three matched tags are capped at three`() {
|
||||
assertEquals(
|
||||
"Matches your taste in a, b, and c.",
|
||||
suggestion(matched = listOf("a", "b", "c", "d", "e")).reasonText,
|
||||
)
|
||||
}
|
||||
|
||||
// The COMMON case: most candidates have no cached tags (#2376), so the card
|
||||
// must fall back to seed attribution rather than going blank.
|
||||
@Test
|
||||
fun `no matched tags falls back to seed attribution`() {
|
||||
assertEquals("Because you liked Seed.", suggestion().reasonText)
|
||||
}
|
||||
|
||||
// Nothing to say at all — a candidate with neither tags nor attribution
|
||||
// yields an empty line, which the tile suppresses rather than rendering as
|
||||
// a blank row.
|
||||
@Test
|
||||
fun `neither tags nor attribution yields an empty line`() {
|
||||
assertEquals("", suggestion(attribution = emptyList()).reasonText)
|
||||
}
|
||||
|
||||
// The taste reason WINS over attribution when both exist: describing the
|
||||
// music beats describing the similarity graph.
|
||||
@Test
|
||||
fun `a taste match supersedes seed attribution`() {
|
||||
val got = suggestion(matched = listOf("shoegaze")).reasonText
|
||||
assertEquals("Matches your taste in shoegaze.", got)
|
||||
}
|
||||
}
|
||||
@@ -332,6 +332,10 @@ export type ArtistSuggestion = {
|
||||
score: number;
|
||||
attribution: SeedContribution[]; // up to 3 entries, ordered by contribution DESC
|
||||
image_url?: string; // resolved on-demand from Lidarr; absent → card placeholder
|
||||
// Tags this candidate shares with your taste profile, strongest first (max 3,
|
||||
// #2377). Absent for most candidates — tag coverage for artists you don't own
|
||||
// is partial by nature (#2376) — so the card falls back to seed attribution.
|
||||
matched_tags?: string[];
|
||||
};
|
||||
|
||||
// One parked suggestion — "not right now", not a dislike. The server only
|
||||
|
||||
@@ -44,6 +44,20 @@
|
||||
return next;
|
||||
}
|
||||
|
||||
// "Matches your taste in shoegaze and dream pop." — the reason line when the
|
||||
// candidate's own tags overlap the taste profile (#2377). Preferred over seed
|
||||
// attribution because it describes the MUSIC ("sounds like what you like")
|
||||
// rather than the graph ("adjacent to something you played"), which is the
|
||||
// whole point of slice 6. Falls back when there are no matched tags, which is
|
||||
// the common case: coverage is partial by nature (#2376).
|
||||
function reasonText(s: ArtistSuggestion): string {
|
||||
const tags = s.matched_tags ?? [];
|
||||
if (tags.length === 0) return attributionText(s.attribution);
|
||||
if (tags.length === 1) return `Matches your taste in ${tags[0]}.`;
|
||||
if (tags.length === 2) return `Matches your taste in ${tags[0]} and ${tags[1]}.`;
|
||||
return `Matches your taste in ${tags[0]}, ${tags[1]}, and ${tags[2]}.`;
|
||||
}
|
||||
|
||||
function attributionText(attribution: SeedContribution[]): string {
|
||||
if (attribution.length === 0) return '';
|
||||
const verb = (s: SeedContribution) => (s.is_liked ? 'liked' : 'played');
|
||||
@@ -151,7 +165,7 @@
|
||||
title={s.name}
|
||||
imageUrl={s.image_url}
|
||||
state={cardState(s)}
|
||||
attribution={attributionText(s.attribution)}
|
||||
attribution={reasonText(s)}
|
||||
onRequest={() => onRequest(s)}
|
||||
onSnooze={() => onSnooze(s)}
|
||||
onUnsnooze={() => onUnsnooze(s.mbid, s.name)}
|
||||
|
||||
@@ -244,3 +244,49 @@ describe('SuggestionFeed snooze (#2375)', () => {
|
||||
expect(screen.queryByText(/listen to something or like an artist/i)).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe('SuggestionFeed taste-tag reason (#2377)', () => {
|
||||
const tagged = (over: Partial<ArtistSuggestion> = {}): ArtistSuggestion => ({
|
||||
...oneSeed,
|
||||
mbid: 'mbT',
|
||||
name: 'Tag Match',
|
||||
...over
|
||||
});
|
||||
|
||||
test('matched tags replace seed attribution as the reason line', () => {
|
||||
setSuggestions([tagged({ matched_tags: ['shoegaze', 'dream pop'] })]);
|
||||
render(SuggestionFeed);
|
||||
expect(
|
||||
screen.getByText(/matches your taste in shoegaze and dream pop\./i)
|
||||
).toBeInTheDocument();
|
||||
// The graph-adjacency line is superseded — describing the music beats
|
||||
// describing the graph when we can do both.
|
||||
expect(screen.queryByText(/because you liked/i)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('one matched tag reads in the singular', () => {
|
||||
setSuggestions([tagged({ matched_tags: ['shoegaze'] })]);
|
||||
render(SuggestionFeed);
|
||||
expect(screen.getByText(/matches your taste in shoegaze\./i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('three matched tags use an Oxford comma, matching the seed copy', () => {
|
||||
setSuggestions([tagged({ matched_tags: ['a', 'b', 'c'] })]);
|
||||
render(SuggestionFeed);
|
||||
expect(screen.getByText(/matches your taste in a, b, and c\./i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// The common case: most candidates have no cached tags, and the card must
|
||||
// still explain itself rather than going blank.
|
||||
test('no matched tags falls back to seed attribution', () => {
|
||||
setSuggestions([oneSeed]);
|
||||
render(SuggestionFeed);
|
||||
expect(screen.getByText(/because you liked seed\./i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('an empty matched_tags array also falls back, not to an empty line', () => {
|
||||
setSuggestions([tagged({ matched_tags: [] })]);
|
||||
render(SuggestionFeed);
|
||||
expect(screen.getByText(/because you liked seed\./i)).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user