Skip to content

fix: preserve repeated playlist positions in Connect results - #80

Merged
steipete merged 1 commit into
mainfrom
fix/phase-five-playlist-order
Sep 13, 2026
Merged

fix: preserve repeated playlist positions in Connect results#80
steipete merged 1 commit into
mainfrom
fix/phase-five-playlist-order

Conversation

@steipete

Copy link
Copy Markdown
Collaborator

Connect playlist listings shared a decoder that treated every collection as a set keyed by URI. A playlist containing [a, b, a] therefore returned only [a, b] even though its total was three. Make shared decoding preserve the sequence, and explicitly deduplicate only library callers. Reuse the existing path reader and a shared total fallback instead of fixed-key plumbing.

Regression proof: repeated playlist positions failed before the fix and pass afterward. Library regressions verify both deduplication and the existing missing/zero/nonzero total rules against the original main implementation and the refactor. A review suggestion to preserve raw zero was rejected because both baseline and current code intentionally fall back to the unique fetched count; that behavior remains unchanged.

All eight packages pass under the race detector; lint, gofumpt and import-order checks pass. Isolated Codex autoreview is scoped-clean through P2. Rebase onto the other bug fixes changed only the changelog merge; the reviewed implementation, tests and docs are byte-identical.

Built-CLI proof: playlist tracks playlist1 --json against a synthetic HTTPS Connect response makes one request and returns [a, b, a], with total three. The previous binary returned [a, b] for the same fixture. The proof build only installs a temporary test CA; normal Connect auth-cache, hash-cache, request, decode and CLI rendering paths run unchanged. No real Spotify account was contacted.

@clawsweeper

clawsweeper Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 13, 2026
@clawsweeper

clawsweeper Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 13, 2026, 2:39 AM ET / 06:39 UTC.

ClawSweeper review

What this changes

Preserves repeated tracks in Connect playlist results while retaining library deduplication, with regression tests and updated documentation.

Merge readiness

Ready for maintainer review

This remains a useful, focused fix: pinned main and v0.12.0 still discard repeated playlist entries. No blocking correctness or security defect was found, and collaborator-authored work is protected from cleanup closure.

Priority: P2
Reviewed head: 81f71d7e6c0b0d3ed91c3eea5c38c35eb8a83825

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused repair with useful regression coverage, preserved library contracts, and no identified blocking defect.
Proof confidence 🌊 off-meta tidepool Not applicable: The collaborator-authored PR is exempt from the external contributor proof gate. Its supplied body reports a built-CLI HTTPS fixture run through Connect decoding and rendering, showing repeated entries restored; authenticated Spotify behavior was not exercised.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The collaborator-authored PR is exempt from the external contributor proof gate. Its supplied body reports a built-CLI HTTPS fixture run through Connect decoding and rendering, showing repeated entries restored; authenticated Spotify behavior was not exercised.
Evidence reviewed 7 items Defect remains on pinned main: The playlist decoder calls the shared collection decoder, whose URI-keyed seen map removes repeated entries.
Latest release also retains the defect: The v0.12.0 source has the same shared playlist deduplication; the requested behavior is not already shipped there.
Sequence and library semantics: The introduced implementation appends playlist entries in response order, applies deduplication only to library callers, and preserves missing/zero total fallback behavior. Tests cover [a, b, a] and three library total cases.
Findings None None.
Security None None.

How this fits together

spogo's Connect client decodes Spotify collection responses for playlist and library commands. The resulting items and total flow into JSON, plain-text, and human-readable CLI output.

flowchart TD
 A[Playlist or library command] --> B[Connect response]
 B --> C[Decode collection items]
 C --> D{Collection type}
 D -->|Playlist| E[Preserve order and repeats]
 D -->|Library| F[Deduplicate entities]
 E --> G[CLI items and total]
 F --> G
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +27/-36 (net -9); tests +49 The fix reduces production code while adding focused sequence and library compatibility coverage.

Technical review

Best possible solution:

Keep playlist decoding sequence-preserving and library deduplication explicit, without changing output schemas or total-count fallback rules.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: a Connect playlist response containing [a, b, a] reaches URI deduplication on pinned main and becomes [a, b]. This review did not execute tests or the application.

Is this the best way to solve the issue?

Yes. Separating ordered decoding from library deduplication fixes the responsible layer while preserving existing library behavior and matching the Web API client's playlist semantics.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 3e084f45091d.

Labels

Label changes:

  • add P2: Repairs incorrect playlist listing output with a bounded decoding change.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The collaborator-authored PR is exempt from the external contributor proof gate. Its supplied body reports a built-CLI HTTPS fixture run through Connect decoding and rendering, showing repeated entries restored; authenticated Spotify behavior was not exercised.

Label justifications:

  • P2: Repairs incorrect playlist listing output with a bounded decoding change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The collaborator-authored PR is exempt from the external contributor proof gate. Its supplied body reports a built-CLI HTTPS fixture run through Connect decoding and rendering, showing repeated entries restored; authenticated Spotify behavior was not exercised.

Evidence

What I checked:

  • Defect remains on pinned main: The playlist decoder calls the shared collection decoder, whose URI-keyed seen map removes repeated entries. (internal/spotify/connect_extract_collections.go:75, 3e084f45091d)
  • Latest release also retains the defect: The v0.12.0 source has the same shared playlist deduplication; the requested behavior is not already shipped there. (internal/spotify/connect_extract_collections.go:75, 64ea6efc2dec)
  • Sequence and library semantics: The introduced implementation appends playlist entries in response order, applies deduplication only to library callers, and preserves missing/zero total fallback behavior. Tests cover [a, b, a] and three library total cases. (internal/spotify/connect_extract_collections.go:63, 81f71d7e6c0b)
  • Output path preserves decoded entries: The playlist command passes the returned item slice directly to rendering and JSON output; rendering iterates without deduplication. The existing Web API client likewise preserves repeated playlist entries. (internal/cli/playlist.go:107, 81f71d7e6c0b)
  • Supplied built-CLI experiment: The fully supplied PR body, captured under sourceRevision 818621927605f0a42205ad929810e7aa7c9e7bff0a90de3ee592d9cedfbe5236, reports one synthetic HTTPS Connect request through the built CLI: the old binary returned [a, b], while the patched binary returned [a, b, a] with total three. It explicitly says no real Spotify account was contacted. No independent runtime execution was performed during this read-only review. (81f71d7e6c0b)
  • Prior area history: Local history and GitHub commit metadata identify prior extraction-layer work by steipete and liked-track decoding work by masonc15. Some older blobs were unavailable to local follow/blame inspection, so these are routing signals rather than claims of causal introduction. (internal/spotify/connect_extract_collections.go, dfc7bed8f620)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit d3bc118 into main Sep 13, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant