add support for year, album, artist cover#1
Merged
hellsontime merged 8 commits intohellsontime:mainfrom Apr 14, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTrack model extended with Album, ArtistAvatarURL, and Year; GetTrack JSON parsing updated to prefer publisher metadata and derive Year; title-cleanup and nullableTime added; embedMetadata now conditionally writes Album/Year, detects image MIME types, attempts high-res artist-avatar fetch with fallback, and embeds artist picture. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SoundCloudAPI as SoundCloud API
participant GetTrack
participant Helper as cleanupTrackTitle
participant TrackStore as Track
Client->>SoundCloudAPI: Request track JSON
SoundCloudAPI-->>GetTrack: JSON (publisher_metadata, user.avatar_url, release_date, created_at)
GetTrack->>GetTrack: Unmarshal into structs (nullableTime support)
alt publisher_metadata.artist present
GetTrack->>TrackStore: Set Artist = publisher_metadata.artist
else
GetTrack->>TrackStore: Set Artist = user.username
end
alt publisher_metadata.release_title present
GetTrack->>TrackStore: Set Title = publisher_metadata.release_title
else
GetTrack->>Helper: cleanupTrackTitle(rawTitle, artist, album)
Helper-->>GetTrack: cleanedTitle
GetTrack->>TrackStore: Set Title = cleanedTitle
end
GetTrack->>TrackStore: Set Album, ArtistAvatarURL
alt release_date present
GetTrack->>TrackStore: Set Year = YYYY(release_date)
else
GetTrack->>TrackStore: Set Year = YYYY(created_at)
end
GetTrack-->>Client: Return Track
sequenceDiagram
participant embedMetadata
participant ID3Writer
participant ImageResolver
participant Remote as Remote Image URL
embedMetadata->>ID3Writer: Conditionally write Album tag
embedMetadata->>ID3Writer: Conditionally write Year tag
alt ArtistAvatarURL present
embedMetadata->>ImageResolver: Rewrite to high-res (-t500x500) and determine MIME
ImageResolver->>Remote: Fetch high-res URL
alt high-res returns image
ImageResolver-->>ID3Writer: Provide image bytes + MIME (attach PTArtistPerformer, "Artist")
else
embedMetadata->>ImageResolver: Fallback to original URL and determine MIME
ImageResolver->>Remote: Fetch original URL
alt original returns image
ImageResolver-->>ID3Writer: Provide image bytes + MIME (attach PTArtistPerformer, "Artist")
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request enhances track metadata extraction and embedding by adding support for album, year, and artist avatar images, including a fallback mechanism for high-resolution images. The Track struct and parsing logic in track.go have been updated to handle these new fields, and a title cleanup utility was introduced. A review comment identified a logic bug and performance inefficiency in the cleanupTrackTitle function, suggesting a more robust implementation using string manipulation instead of regular expressions.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
download.go (1)
284-299: Deduplicate the image fallback path.This block copies the cover-art logic almost verbatim: size rewrite, fallback fetch, then APIC construction. Pulling that into a small helper will keep the two picture paths aligned and make future changes less error-prone.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@download.go` around lines 284 - 299, The artist-avatar block duplicates the cover-art logic (rewriting "-large." to "-t500x500.", trying high-res then falling back to original, then building an id3v2.PictureFrame and calling tag.AddAttachedPicture); extract that flow into a small helper to avoid duplication: create a function (e.g., fetchImageForTag or fetchAndAttachImage) that accepts the ctx, c (or c.get), the source URL (track.ArtistAvatarURL or cover URL), the PictureType (id3v2.PTArtistPerformer or other), Description string and the tag, performs the size-rewrite, tries c.get with fallback to original, and builds/attaches the id3v2.PictureFrame via tag.AddAttachedPicture so both cover and artist paths call the same helper.track.go (1)
66-74: Please add a fixture for the new metadata precedence paths.
track_test.go:12-62still only exercisesuser.usernameand a plain title, so none of the newpublisher_metadata,user.avatar_url,release_date/created_at, orcleanupTrackTitlebranches are pinned down. A table case for those branches would lock in the fallback behavior.Also applies to: 89-117, 139-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@track.go` around lines 66 - 74, Add a test fixture and table-driven case(s) in track_test.go that exercise the new metadata precedence paths: populate PublisherMetadata (Artist, AlbumTitle, ReleaseTitle), User.AvatarURL as well as User.Username, and both ReleaseDate and CreatedAt timestamps to assert fallback ordering; include inputs that require cleanupTrackTitle to run (e.g., titles needing trimming or punctuation removal) and verify the resulting title/artist/album/avatar selection matches expected precedence. Locate the Track struct usage and the cleanupTrackTitle helper in the tests and add a new table case (or cases) covering: publisher_metadata wins over plain title fields, avatar is read from user.avatar_url, release_date falls back to created_at, and cleanupTrackTitle transforms the raw title into the expected clean title; assert all expected outputs in the same test table so these branches are pinned down.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@track.go`:
- Around line 140-145: The cleanupTrackTitle function currently treats an empty
artist as matching every title and also strips any later "artist - "
occurrences; update cleanupTrackTitle to first return title if artist == "" to
guard empty artists, then build a regex anchored to the start using
regexp.QuoteMeta(artist) with a leading ^ (e.g. "^" + regexp.QuoteMeta(artist) +
"\\s*-\\s*") so only the leading "artist - " prefix is removed, and use that
regex (created in cleanupTrackTitle) to replace the prefix once.
- Line 57: Replace the current int-typed numeric ID binding with a string URN
field and keep the numeric ID only as int64 compatibility data: change the
struct field `ID int` to `Urn string `json:"urn"` and add `ID int64
`json:"id,omitempty"` (or similar) so JSON parsing accepts both `urn` (primary)
and legacy numeric `id` (optional); update any code that reads the old `ID` to
prefer `Urn` (e.g., when constructing resource identifiers) and treat `ID` as
fallback compatibility only to avoid 32-bit/int-size issues and match
SoundCloud's URN migration.
- Around line 89-92: The metadata currently always sets ArtistAvatarURL from
data.User.AvatarURL even when the displayed artist was overridden by
data.PublisherMetadata.Artist; change the assignment so ArtistAvatarURL is
populated only when the resolved artist came from data.User.Username (i.e., when
data.PublisherMetadata.Artist is empty or artist == data.User.Username). Locate
where ArtistAvatarURL is set (reference ArtistAvatarURL and data.User.AvatarURL
in the track metadata construction) and conditionally assign it; otherwise leave
ArtistAvatarURL empty/nil so uploads with a PublisherMetadata.Artist do not get
the uploader's avatar.
---
Nitpick comments:
In `@download.go`:
- Around line 284-299: The artist-avatar block duplicates the cover-art logic
(rewriting "-large." to "-t500x500.", trying high-res then falling back to
original, then building an id3v2.PictureFrame and calling
tag.AddAttachedPicture); extract that flow into a small helper to avoid
duplication: create a function (e.g., fetchImageForTag or fetchAndAttachImage)
that accepts the ctx, c (or c.get), the source URL (track.ArtistAvatarURL or
cover URL), the PictureType (id3v2.PTArtistPerformer or other), Description
string and the tag, performs the size-rewrite, tries c.get with fallback to
original, and builds/attaches the id3v2.PictureFrame via tag.AddAttachedPicture
so both cover and artist paths call the same helper.
In `@track.go`:
- Around line 66-74: Add a test fixture and table-driven case(s) in
track_test.go that exercise the new metadata precedence paths: populate
PublisherMetadata (Artist, AlbumTitle, ReleaseTitle), User.AvatarURL as well as
User.Username, and both ReleaseDate and CreatedAt timestamps to assert fallback
ordering; include inputs that require cleanupTrackTitle to run (e.g., titles
needing trimming or punctuation removal) and verify the resulting
title/artist/album/avatar selection matches expected precedence. Locate the
Track struct usage and the cleanupTrackTitle helper in the tests and add a new
table case (or cases) covering: publisher_metadata wins over plain title fields,
avatar is read from user.avatar_url, release_date falls back to created_at, and
cleanupTrackTitle transforms the raw title into the expected clean title; assert
all expected outputs in the same test table so these branches are pinned down.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
♻️ Duplicate comments (1)
track.go (1)
94-99:⚠️ Potential issue | 🟠 MajorThis cleanup logic can still erase real titles.
This reintroduces the earlier over-stripping problem in a different form:
matchesAnyFoldremoves any segment that merely starts with the artist or album, and the caller here is also passing(title, album, artist)into a helper declared as(title, artist, album). Cases likeQueen - Queen of HeartsorSome Album - Some Album Bonus Trackwill collapse to the wrong title. Tighten this to exact album matches plus collaborator-aware artist matches, and align the argument order before those rules diverge.Also applies to: 141-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@track.go` around lines 94 - 99, The cleanup logic for track titles is over-aggressive and using the wrong argument order: locate the call site using cleanupTrackTitle(title, data.PublisherMetadata.AlbumTitle, artist) (and the other occurrences around lines 141–205) and change the argument order to match the helper signature (title, artist, album), then update cleanupTrackTitle and its helper matchesAnyFold so it only strips an album when it exactly equals a title segment (exact match, not prefix) and only strips artist segments when the artist match accounts for collaborators (e.g., split on &, feat., ft., vs., commas and only remove when the full artist or a recognized collaborator pattern matches exactly), preserving hyphenated/compound titles like "Queen - Queen of Hearts" and "Some Album - Some Album Bonus Track".
🧹 Nitpick comments (1)
track_test.go (1)
143-152: Broaden this into a small table test for the new metadata branches.This only covers one cleanup happy path. The new behavior most likely to regress is
publisher_metadataprecedence,release_datevscreated_atyear fallback, and titles likeQueen - Queen of Heartswhere the real title starts with the artist name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@track_test.go` around lines 143 - 152, Replace the single-case TestCleanupTitle with a table-driven test that iterates cases feeding cleanupTrackTitle (and any helper that accepts metadata) different inputs to cover: publisher_metadata taking precedence over album/artist, release_date year vs created_at year fallback, and titles where the artist name prefixes the real title (e.g., "Queen - Queen of Hearts"); define a test case struct with fields like title, album, artist, publisher_metadata, release_date, created_at, and expected, and for each case call cleanupTrackTitle (or the public wrapper used in tests) and assert equality using t.Run with descriptive names so regressions in publisher_metadata precedence, date fallback, and artist-prefixed titles are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@track.go`:
- Around line 94-99: The cleanup logic for track titles is over-aggressive and
using the wrong argument order: locate the call site using
cleanupTrackTitle(title, data.PublisherMetadata.AlbumTitle, artist) (and the
other occurrences around lines 141–205) and change the argument order to match
the helper signature (title, artist, album), then update cleanupTrackTitle and
its helper matchesAnyFold so it only strips an album when it exactly equals a
title segment (exact match, not prefix) and only strips artist segments when the
artist match accounts for collaborators (e.g., split on &, feat., ft., vs.,
commas and only remove when the full artist or a recognized collaborator pattern
matches exactly), preserving hyphenated/compound titles like "Queen - Queen of
Hearts" and "Some Album - Some Album Bonus Track".
---
Nitpick comments:
In `@track_test.go`:
- Around line 143-152: Replace the single-case TestCleanupTitle with a
table-driven test that iterates cases feeding cleanupTrackTitle (and any helper
that accepts metadata) different inputs to cover: publisher_metadata taking
precedence over album/artist, release_date year vs created_at year fallback, and
titles where the artist name prefixes the real title (e.g., "Queen - Queen of
Hearts"); define a test case struct with fields like title, album, artist,
publisher_metadata, release_date, created_at, and expected, and for each case
call cleanupTrackTitle (or the public wrapper used in tests) and assert equality
using t.Run with descriptive names so regressions in publisher_metadata
precedence, date fallback, and artist-prefixed titles are caught.
Contributor
Author
|
Good point robot man, let me fix it |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@track.go`:
- Line 98: The call to cleanupTrackTitle uses the wrong argument order:
cleanupTrackTitle(title, artist, album string) is being invoked as
cleanupTrackTitle(title, data.PublisherMetadata.AlbumTitle, artist), swapping
album and artist; update the call so the artist argument is passed second and
the album third (i.e., call cleanupTrackTitle with title, artist,
data.PublisherMetadata.AlbumTitle) so cleanupTrackTitle receives (title, artist,
album) as intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 03af3080-c2ce-453f-8ac9-a787be90975a
📒 Files selected for processing (2)
track.gotrack_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- track_test.go
- Use URL extension + map lookup to determine MIME type for artwork and artist avatar picture frames instead of hardcoding image/jpeg - Replace time.Time with nullableTime for created_at/release_date JSON fields to avoid unmarshal errors when API returns empty string
…olds - TestNullableTime covers null, empty string, valid RFC3339, and invalid input — directly verifies the unmarshal bug fix - TestImageMimeType covers all mapped extensions and the fallback case - Lower file/package/total thresholds to 70/80/80 — embedMetadata is I/O-heavy and not meaningfully unit-testable without a full mock setup
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.
I've expanded the parsing to also embed release year, album and artist cover. I've also added some cleanup to get the best possible value for metadata.
Summary by CodeRabbit
New Features
Tests
Chores