Repository navigation
Show observed release format and disc numbers in release cases - #2220
sidneyswift wants to merge 3 commits into
Conversation
Reviewers could not tell a single from an album or see disc structure on the release review page, even though the saved Spotify observation carries that metadata. Render the server-projected release_format as one line under the title and prefix track cells with the disc number when the release is multi-disc. The field is optional so responses from an older API still render, in which case the page says "Format not collected". Only observed values are shown: a UPC appears only when the server reports one, reissue and physical format are always stated as unknown, and an observed-but-unknown type stays distinct from an uncollected one. Refs #2139 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No actionable issue was established in the release-format display change. Live API compatibility remains unverified. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Solid & Clean Code |  |
ReleaseCaseFormat is a new 32-line function (lines 9-40). It both handles the observed/uncollected state and builds all display fields, which exceeds the check's 20-line function guideline and combi… |
Refactor ReleaseCaseFormat into small, named helpers. Keep the component responsible for rendering and move release-format state handling and metadata assembly into helpers such as a formatter for the observed ReleaseFormat, with separa… |
✅ Passed checks (2 passed)
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Solid & Clean Code
-
Explanation
ReleaseCaseFormatis a new 32-line function (lines 9-40). It both handles the observed/uncollected state and builds all display fields, which exceeds the check's 20-line function guideline and combines formatting responsibilities in one component.Resolution
Refactor
ReleaseCaseFormatinto small, named helpers. Keep the component responsible for rendering and move release-format state handling and metadata assembly into helpers such as a formatter for the observedReleaseFormat, with separate helpers for disc-count and UPC labels. Keep each function at 20 lines or fewer and preserve the existing tests and output.
- Fix all pre-merge checks with AI
✨ 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
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.
A format arrives, its fields in view
Unknowns stay marked as unknown too
Two discs lend tracks a numbered sign
Observed details follow the line
The case now shows what it can know
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Confidence score: 4/5
- In
ReleaseCaseFormat.tsx, “UPC not observed” makes an uncollected UPC sound checked and absent. Show “UPC not collected” for that state. ReleaseCaseDetails.tsxcombines the header, empty state, tracks, review controls, and sources disclosure in one 142-line component, which makes it harder to maintain. Consider splitting those responsibilities.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="components/Releases/ReleaseCaseFormat.tsx">
<violation number="1" location="components/Releases/ReleaseCaseFormat.tsx:27">
P2: For `upc_state: "uncollected"`, this renders “UPC not observed,” falsely implying collection established that no UPC was present. Render “UPC not collected” for that state.</violation>
</file>
<file name="components/Releases/ReleaseCaseDetails.tsx">
<violation number="1" location="components/Releases/ReleaseCaseDetails.tsx:43">
P2: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
`ReleaseCaseDetails.tsx` is 142 lines and still combines the header, empty state, tracks, review controls, and sources disclosure, exceeding the rule’s 100-line limit. Split the remaining page sections into focused components rather than continuing to grow this component.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| <h2 className="text-xl font-semibold"> | ||
| {current.title ?? "Spotify release"} | ||
| </h2> | ||
| <ReleaseCaseFormat current={current} /> |
There was a problem hiding this comment.
P2: Custom agent: Code Structure and Size Limits for Readability and Single Responsibility
ReleaseCaseDetails.tsx is 142 lines and still combines the header, empty state, tracks, review controls, and sources disclosure, exceeding the rule’s 100-line limit. Split the remaining page sections into focused components rather than continuing to grow this component.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At components/Releases/ReleaseCaseDetails.tsx, line 43:
<comment>`ReleaseCaseDetails.tsx` is 142 lines and still combines the header, empty state, tracks, review controls, and sources disclosure, exceeding the rule’s 100-line limit. Split the remaining page sections into focused components rather than continuing to grow this component.</comment>
<file context>
@@ -39,6 +40,7 @@ export default function ReleaseCaseDetails({
<h2 className="text-xl font-semibold">
{current.title ?? "Spotify release"}
</h2>
+ <ReleaseCaseFormat current={current} />
<p className="mt-1 text-sm text-muted-foreground">
{awaitingMetadata
</file context>
"UPC not observed" claims a collection looked for a UPC and found none. When the server reports upc_state "uncollected" the release line now says "UPC not collected" instead, keeping uncollected distinct from not observed. Move the release format test fixtures into their own module, following the existing streamFixture pattern, so the test file stays focused on the cases. Refs #2139 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
View guided diff | Turn on auto-fix | Re-trigger cubic
database#109 now leaves disc_count null when only part of a release's track list was collected and reports track_coverage. Render "Multiple discs" when a partial list already reaches disc 2, "Disc count unknown" otherwise, and "Track list incomplete" for partial coverage, so a truncated album is never shown as a definite single disc. Moves the shared render helper out of the format test so both test files stay small. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Confidence score: 4/5
ReleaseCaseFormat.tsxhandles onlypartial, so formats withtrack_coverage: "uncollected"lose that state in the UI. Add a separate “Track list not collected” label.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="components/Releases/ReleaseCaseFormat.tsx">
<violation number="1" location="components/Releases/ReleaseCaseFormat.tsx:29">
P2: Observed formats with `track_coverage: "uncollected"` lose the uncollected state because this branch handles only `"partial"`. Add a separate `Track list not collected` label so the UI does not present an uncollected track list as merely unknown.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| format.reported_total_tracks === null | ||
| ? null | ||
| : `${count(format.reported_total_tracks, "track")} reported`, | ||
| format.track_coverage === "partial" ? "Track list incomplete" : null, |
There was a problem hiding this comment.
P2: Observed formats with track_coverage: "uncollected" lose the uncollected state because this branch handles only "partial". Add a separate Track list not collected label so the UI does not present an uncollected track list as merely unknown.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At components/Releases/ReleaseCaseFormat.tsx, line 29:
<comment>Observed formats with `track_coverage: "uncollected"` lose the uncollected state because this branch handles only `"partial"`. Add a separate `Track list not collected` label so the UI does not present an uncollected track list as merely unknown.</comment>
<file context>
@@ -18,10 +18,15 @@ export default function ReleaseCaseFormat({
format.reported_total_tracks === null
? null
: `${count(format.reported_total_tracks, "track")} reported`,
+ format.track_coverage === "partial" ? "Track list incomplete" : null,
format.release_date ? `Released ${format.release_date}` : null,
format.label,
</file context>
| format.track_coverage === "partial" ? "Track list incomplete" : null, | |
| format.track_coverage === "partial" | |
| ? "Track list incomplete" | |
| : format.track_coverage === "uncollected" | |
| ? "Track list not collected" | |
| : null, |
This branch was successfully deployed
1 participant
Summary
Reviewers could not tell a single from an album, or see disc structure, on the release review page, even though the saved Spotify observation carries that metadata. This PR renders the server-projected
release_format(recoupable/database#109, typed in recoupable/api#1021) as one line under the release title, for exampleAlbum · 2 discs · 3 tracks reported · Released 2024-05-17 · <label> · UPC not observed · Reissue/physical format unknown, and prefixes each track cell withDisc N ·when the release is multi-disc. When only part of the track list was collected (track_coverage: partial), the line saysTrack list incompleteand showsMultiple discs(a partial list already reaching disc 2) orDisc count unknown, never a definite disc count. This advances ticket Done-when 1 (distinguish singles, albums and formats on canonical releases with ordered, disc-aware membership).Only observed values are shown: a UPC appears only when the server reports one, reissue and physical format are always stated as unknown, and an observed-but-unknown type (
Format unknown) stays distinct from an uncollected one (Format not collected). The field is optional inlib/releases/types.ts, so a response from an API without the migration still renders. No sorting or grouping changes to the track table.Refs #2139 (child of epic #2116).
Merge sequencing
docs (no change) → database recoupable/database#109 → api recoupable/api#1021 → app (this PR). Safe to merge before the backend because the field is optional; the line then reads
Format not collected. Depends on no other OPEN PR; builds on merged app#2172/#2173/#2178.Verification
pnpm exec vitest run components/Releases lib/releases→ 24 files, 74 tests passed, 0 failed (includesReleaseCaseFormat.test.tsx, 6 tests,ReleaseCaseFormatCoverage.test.tsx, 2 tests, and the unchangedReleaseCasePresentation,ReleaseCaseReview,ReleaseCaseScope,ReleaseCaseAccess,ReleaseCaseRecoverysuites).pnpm exec tsc --noEmit→ 1 error, pre-existing and unrelated (components/VercelChat/tools/SpotifyDeepResearchResult.tsx: cannot find module@/public/brand-logos/spotify.png, because the untracked generatednext-env.d.tsis absent without a Next build); 0 errors in changed files.pnpm exec eslintandpnpm exec prettier --checkon the 8 changed files → clean.ReleaseCaseFormat.test.tsxwas written first; with the component and the two render edits absent, all 5 tests failed (Unable to find an element with the text: /^Album · 2 discs · 3 tracks reported/,... Format not collected, etc.). With the implementation, 5/5 pass.upc_state: "uncollected"readsUPC not collected, notUPC not observed; it failed (expected 'Album · 2 discs · 3 tracks reported ·…' to contain 'UPC not collected') before the label change and passes after. Test fixtures moved tocomponents/Releases/__tests__/releaseFormatFixture.ts, following the existingstreamFixture.tspattern.ReleaseCaseFormatCoverage.test.tsx(50 of 60 tracks collected; partial list reaching disc 2) and the updated observed-but-unknown case were written first and failed 3/8 (Unable to find an element with the text: /^Album · Disc count unknown · 60 tracks reported · Track list incomplete/,/^Album · Multiple discs · .../,/^Format unknown · Disc count unknown · .../). With the render change, all pass. The shared render helper moved tocomponents/Releases/__tests__/renderReleaseCase.tsxso both test files stay under 100 lines.release_format, visual review in a browser, any live provider.Out of scope / known gaps
release_format; the current-case page always renders the live projection.ReleaseCaseDetails.tsxwas already 140 lines on main; this PR adds two lines (142) and does not split it, to avoid conflicts with sibling tickets editing the same page. The cubic comment about it is intentionally left open for the approver; a split belongs in a follow-up.labelis shown as an observed string only; it does not verify rights, ownership or distribution.🤖 Generated with Claude Code
Summary by CodeRabbit