fix: preserve repeated playlist positions in Connect results - #80
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed September 13, 2026, 2:39 AM ET / 06:39 UTC. ClawSweeper reviewWhat this changesPreserves 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 Review scores
Verification
How this fits togetherspogo'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
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest 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. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
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 --jsonagainst 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.