Skip to content

feat(subsonic): getSimilarSongs and getSimilarSongs2 draw from the artist and their neighbours - #273

Merged
InstaZDLL merged 2 commits into
mainfrom
feat/similar-songs
Oct 15, 2026
Merged

InstaZDLL merged 2 commits into
mainfrom
feat/similar-songs

Conversation

@InstaZDLL

@InstaZDLL InstaZDLL commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

Step E of the enrichment plan, which implements RFC-011. With no provider configured, which is the default, both methods still answer the empty container they always did, for any id the caller can reach.

  • One implementation, two entry points: DomainServices::similar_songs (src/services/artist_enrichment.rs), called by getSimilarSongs and getSimilarSongs2 (src/subsonic/browse.rs).
  • The id is resolved first and for its refusal (artist_behind): an artist id, an album id (standing for its album artist) or a song id (standing for its track artist). An id the caller cannot reach is error 70, exactly like one naming nothing, and no provider is asked about it. An album or a song credited to no artist draws nothing. getSimilarSongs2 documents only artist ids but accepts all three: refusing an id the service can resolve would only make it worse.
  • The artist is in the draw, not the neighbours alone: "a random collection of songs from the given artist and similar artists". count defaults to 50, is capped at 500, and count=0 is an empty container (after the id is resolved).
  • Neighbours are the same ones getArtistInfo shows: the first provider, in the operator's order, whose cached list names an artist the caller can see (the loop is factored into neighbours, shared with feat(enrichment): getArtistInfo and the native artist page read the cache #272's enrichment_of). Every name then draws by canonical name, so an artist the caller sees in two libraries contributes both. Tenancy is the membership bind of song_select!, which keeps a track in a library the caller is not in out of the draw even when its artist shares a name with a visible one.
  • The credit rule is now one macro, artist_credit_clause! in src/services/mod.rs: a track names the artist in its artist role, or its album credits them as album artist. getTopSongs uses the same macro, unchanged in behaviour.
  • JSON shape: similarSongs.song and similarSongs2.song join the array allowlist. They held nothing until now, so nothing had told them apart from an object.
  • Wire change, even when enrichment is off: a malformed id, or one naming nothing the caller can reach, used to get the empty container and now gets an error, as RFC-011 decision 7 requires. tests/subsonic_methods.rs asserted the old answer for id=whatever, and now asserts error 70 for an unknown UUID.
  • Docs: RFC-002's paragraph on the two methods, subsonic-api-guide.md, the dated note in subsonic-compatibility.md (which still said getArtistInfo was empty, so feat(enrichment): getArtistInfo and the native artist page read the cache #272 is corrected there too), and RFC-011's Implemented by line.

Closes #267.

How I tested

  • cargo test --all-features: every target passes. enrichment grows from 14 to 18 tests.
  • cargo clippy --all-targets --all-features -- -D warnings, cargo fmt --all --check.
  • The new tests insert their accounts as rows and mint the Subsonic key at run time (the helper subsonic_key, now shared with feat(enrichment): getArtistInfo and the native artist page read the cache #272's façade test). No literal goes into a cryptographic call.
  • Each test was made to fail under the change it guards, by an inversion applied and reverted mechanically:
test inversion
an_album_id_and_a_song_id_reach_the_same_artist_as_the_artist_id only artist ids are accepted
the_artist_s_own_tracks_are_in_the_draw the draw takes the neighbours alone
no_track_from_a_library_the_caller_is_not_in the membership bind is dropped from song_select! ((m.user_id=? OR 1))
count_absent_yields_at_most_fifty the default count is 500 instead of 50

Summary by CodeRabbit

  • Nouvelles fonctionnalités
    • getSimilarSongs et getSimilarSongs2 peuvent renvoyer jusqu’à 500 morceaux de l’artiste associé à un artiste, un album ou un morceau, ainsi que de ses voisins accessibles. Par défaut, 50 morceaux sont demandés.
    • Les identifiants inconnus ou inaccessibles renvoient l’erreur Subsonic 70. Sans fournisseur de métadonnées configuré, les méthodes renvoient un conteneur vide.
  • Documentation
    • Mise à jour de la documentation de l’API et des notes de compatibilité sur ces méthodes.

…tist and their neighbours

Both methods resolve an artist, album or song id first and for its
refusal, then answer a random draw of at most count (default 50) from
the artist's own tracks and their visible neighbours', read from the
enrichment cache. With no provider configured they stay the empty
container they always were.

The credit rule that decides which tracks are an artist's is now one
macro, shared with getTopSongs.

Closes #267.

Signed-off-by: InstaZDLL <github.105mh@8shield.net>
@github-actions github-actions Bot added type: feat New feature scope: server Server core (Rust) scope: docs Docs, README, assets scope: subsonic Subsonic / OpenSubsonic compatibility and removed type: feat New feature labels Oct 15, 2026
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: InstaZDLL/waveflow-server/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 9f63b728-89aa-4988-b968-381ae82b1a50
📥 Commits

Reviewing files that changed from the base of the PR and between decf573 and 0aee67e.

📒 Files selected for processing: 1
  • tests/enrichment.rs

Limit details: You’ve used the included review currently available. Your 89 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

getSimilarSongs et getSimilarSongs2 acceptent un identifiant d’artiste, d’album ou de piste. Avec l’enrichissement activé, elles renvoient jusqu’à count pistes accessibles de cet artiste et de ses voisins visibles. Sans enrichissement, elles renvoient un conteneur vide.

Changes: Chansons similaires

Layer / File(s) Summary
Résolution et tirage des pistes
src/services/artist_enrichment.rs, src/services/mod.rs, src/services/artists.rs, tests/enrichment.rs
Le service résout l’artiste associé à l’identifiant fourni. Il sélectionne les voisins visibles et tire des pistes parmi celles accessibles. Les tests vérifient les identifiants acceptés, le tirage et les limites d’accès.
Méthodes Subsonic et réponses
src/subsonic/browse.rs, src/subsonic/mod.rs, src/subsonic/protocol.rs, tests/enrichment.rs, tests/subsonic_methods.rs, docs/rfcs/RFC-002-waveflow-server-v2.md, docs/rfcs/RFC-011-metadata-enrichment.md, docs/subsonic-api-guide.md, docs/subsonic-compatibility.md
Les deux méthodes appellent le service et construisent leur conteneur de réponse. Les réponses JSON sérialisent les pistes en tableau. Les tests et la documentation décrivent le comportement, les paramètres et les erreurs.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Dispatch as subsonic::dispatch
  participant Browse as browse::similar_songs
  participant Service as DomainServices::similar_songs
  participant Resolution as artist_behind
  participant Neighbours as neighbours
  Dispatch->>Browse: transmet id, count et conteneur
  Browse->>Service: transmet user_id, id et limit
  Service->>Resolution: résout l’artiste accessible
  Service->>Neighbours: sélectionne les voisins visibles depuis les réponses en cache
  Service-->>Browse: renvoie les pistes similaires
  Browse-->>Dispatch: construit le conteneur demandé
Loading
Merge Risk: ⚪ Minimal · up to 0aee6

The similar-songs methods now return scoped recommendations, with error 70 for inaccessible IDs and empty results when enrichment is off. No merge-blocking risk is evident.

Security architecture risk: ⚪ Minimal · up to 0aee6

The new recommendation flow preserves library access controls before resolving resources and returning songs. Provider metadata supplies candidate names rather than access rights. No material security risk introduced or worsened by this change was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The added effective reach is recommendation metadata across the requesting account's accessible libraries and initiation of shared provider-cache fills for reachable artists. The inspected flow does not grant another account's library access or let request parameters choose a provider destination or credential.

Trust Boundaries and Controls

  • observed — Attacker-selected resource IDs encounter membership checks before provider work. Artist lookup is caller-scoped, and album and track resolution explicitly join library membership. Foreign and nonexistent IDs share NotFound handling; test assertions verify rejection of foreign artist and album IDs without provider requests.

Resilience and Maintainability Implications

  • inferred — A recommendation request can end while its independently spawned cache fill continues, but that fill stores provider metadata rather than caller authorization. Cancellation, repetition, and cache refresh therefore do not substitute cached permissions for current membership checks. Transactional replacement and cleanup guards contain partial-write and stranded-reservation failure modes.
Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed Le titre décrit clairement l’ajout de getSimilarSongs et getSimilarSongs2 avec des morceaux de l’artiste et de ses voisins.
Description check Passed La description couvre le contexte, les changements principaux et les tests exécutés. La section « Notes » est facultative. La section « Changes » et la confirmation finale du modèle sont absentes, mai…
Linked Issues check Passed L’implémentation répond aux exigences de #267. DomainServices::similar_songs est partagé par getSimilarSongs et getSimilarSongs2. artist_behind résout un identifiant d’artiste, d’album ou de p…
Out of Scope Changes check Passed Aucun changement sans lien avec #267 n’est établi. Le macro artist_credit_clause!, la sérialisation JSON, les tests et la documentation soutiennent l’implémentation de la fonctionnalité ou son contr…
Docstring Coverage Passed Docstring coverage is 84.21% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 8 files.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added size: m 50-200 lines type: feat New feature labels Oct 15, 2026
@InstaZDLL InstaZDLL self-assigned this Oct 15, 2026
Comment thread tests/enrichment.rs Fixed
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
@github-actions github-actions Bot added type: feat New feature and removed type: feat New feature labels Oct 15, 2026
@InstaZDLL
InstaZDLL merged commit fd32ed6 into main Oct 15, 2026
15 checks passed
@InstaZDLL
InstaZDLL deleted the feat/similar-songs branch October 15, 2026 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: docs Docs, README, assets scope: server Server core (Rust) scope: subsonic Subsonic / OpenSubsonic compatibility size: m 50-200 lines type: feat New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(subsonic): similar songs

2 participants