navidrome: beets writes the retried partial-album matches (REN-350) #1073

2026-09-30T03:02:31Z

Ghost commented

2026-09-30 03:02:31 +00:00

What: beets writes the partial-album matches from the #1070 retry trial. import -W → import, beet write step back, /music writable again. Muse "Black Holes and Revelations" is added to ignore and dropped once from the beets DB (beet rm -a -f, no -d: DB only, no file touched). move/copy/delete and the art plugins stay off.

Why (REN-350): trial job beets-29845530 (2026-09-30 01:30–02:25 UTC, exit 0): 158 albums matched (was 74), 402 skipped, 0 duplicates, 931 items to write. All matches score ≥96%. 0 of the 74 already-written albums changed release. One wrong match: Muse "Black Holes and Revelations" → an "instrumental" release, so it is left out. Years stay original (e.g. Die Apokalyptischen Reiter "Soft & Stronger" 2003 → 1997).

Known tag change: MusicBrainz credits "Swanlights" to "ANOHNI and the Johnsons" (was "Antony and the Johnsons").

Verified: linted only (run.sh sh -n); nothing in the cluster yet.

Revert: revert this PR. That puts back -W and a read-only /music. Tags already written stay in the files.

🤖 Generated with Claude Code

**What:** beets writes the partial-album matches from the #1070 retry trial. `import -W` → `import`, `beet write` step back, `/music` writable again. Muse "Black Holes and Revelations" is added to `ignore` and dropped once from the beets DB (`beet rm -a -f`, no `-d`: DB only, no file touched). move/copy/delete and the art plugins stay off. **Why (REN-350):** trial job `beets-29845530` (2026-09-30 01:30–02:25 UTC, exit 0): 158 albums matched (was 74), 402 skipped, 0 duplicates, 931 items to write. All matches score ≥96%. **0** of the 74 already-written albums changed release. One wrong match: Muse "Black Holes and Revelations" → an "instrumental" release, so it is left out. Years stay original (e.g. Die Apokalyptischen Reiter "Soft & Stronger" 2003 → 1997). Known tag change: MusicBrainz credits "Swanlights" to "ANOHNI and the Johnsons" (was "Antony and the Johnsons"). **Verified:** linted only (run.sh `sh -n`); nothing in the cluster yet. **Revert:** revert this PR. That puts back `-W` and a read-only `/music`. Tags already written stay in the files. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Ghost added 1 commit 2026-09-30 03:02:32 +00:00

navidrome: beets writes the retried partial-album matches, drops one wrong match (REN-350)

ci/woodpecker/pr/ci-manifests-test Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was successful

Details

d95a5e9137

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Ghost approved these changes

2026-09-30 03:04:40 +00:00

Ghost left a comment

Approved at d95a5e91. Checked: diff (1 file); trial log beets-29845530 has the album at exactly /music/Muse/Black Holes and Revelations, matched to 49609c80 (instrumental); first write run (beets-write-hwn74) never touched it. beets 2.14.1 locally: beet rm -a -f 'path:.../Muse/Black Holes and Revelations' removes only that album (sibling 'Black Holes and Revelations Live' kept, all files kept); sorted_walk with the new ignore list skips only that folder. ignore keeps the beets defaults. music PVC is RWX. Revert: revert PR (tags written stay, stated).

Approved at d95a5e91. Checked: diff (1 file); trial log beets-29845530 has the album at exactly /music/Muse/Black Holes and Revelations, matched to 49609c80 (instrumental); first write run (beets-write-hwn74) never touched it. beets 2.14.1 locally: `beet rm -a -f 'path:.../Muse/Black Holes and Revelations'` removes only that album (sibling 'Black Holes and Revelations Live' kept, all files kept); sorted_walk with the new ignore list skips only that folder. ignore keeps the beets defaults. music PVC is RWX. Revert: revert PR (tags written stay, stated).

Ghost merged commit 92ba011aca into master

2026-09-30 03:04:41 +00:00

Ghost referenced this issue from a commit

2026-09-30 03:04:43 +00:00

Merge pull request 'navidrome: beets writes the retried partial-album matches (REN-350)' (#1073) from atlas/ren-350-beets-write-retry into master

review-bot reviewed

2026-09-30 03:09:03 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: unknown), commit d95a5e91375244bfaa8bbe3ac15d54b6aa4eeeec.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

This PR flips the nightly beets cron job from the REN-350 dry-run trial (import -W, read-only /music) to live writes: it drops the -W flag, makes /music writable again, adds an explicit beet write step with before/after diff logging, and one-time-drops a wrongly-matched Muse album (adding it to ignore so it isn't re-matched). The change is narrowly scoped to kubernetes/apps/navidrome/beets.yaml and is backed by a real trial run (158/560 albums matched, all ≥96%, 0 of the 74 previously-written albums changed release). I traced the shell logic and the ignore/incremental/write interaction carefully and did not find a correctness bug.

No blocking issues.

🟡 Suggestions

run.sh: no hard failure if beet write leaves residual diffs. After beet write, the script recomputes write-after.txt and only logs items still differing after write (kubernetes/apps/navidrome/beets.yaml:126-127) — it doesn't fail the job if that count is nonzero. Today this relies entirely on wrc=$? (beets.yaml:124) reflecting any per-file write error. If beet write ever returns 0 while silently skipping a subset of files (e.g. a permission error on one path that doesn't propagate to the overall exit code), the job would report green despite an incomplete write. Consider failing the job when the after-write diff count is nonzero, since you already compute it.
Defense-in-depth note on the /music mount. Dropping readOnly: true (beets.yaml:186-189) means the container-level write barrier is gone; going forward, "beets never moves/copies/deletes" is enforced only by the beets config (move/copy/delete: no, unchanged in this diff) rather than by Kubernetes. That's clearly a deliberate, trial-verified tradeoff per the PR description, just worth being aware of if the beets config ever changes without matching scrutiny.

🟢 Nits

The new ignore entry 'Black Holes and Revelations' (beets.yaml:44-46) matches on basename only (beets ignore has no directory-scoped syntax), so it would also skip any other folder with that exact name anywhere under /music. Effectively a non-issue in practice given how specific the name is, just noting the mechanism isn't Muse/-scoped.

Tests

No automated tests apply to this manifest change beyond what the repo already gates: .woodpecker/ci-manifests-test.yml's kustomize-build step will build the kustomization and catch structural/YAML breakage, and the PR body notes run.sh was checked with sh -n. I re-derived the embedded run.sh from the diff and walked it by hand (no logic branch left unreachable, rc/wrc capture and combination at beets.yaml:109,124,132 is correct, the one-time .drop-ren350 guard is idempotent). The real functional validation here is the out-of-band trial job (beets-29845530) described in the PR body, which is the appropriate form of "test" for this kind of stateful, slow, MusicBrainz-rate-limited operation — there's no practical way to unit-test it in CI.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `CLI default` (complexity: unknown), commit `d95a5e91375244bfaa8bbe3ac15d54b6aa4eeeec`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR flips the nightly beets cron job from the REN-350 dry-run trial (`import -W`, read-only `/music`) to live writes: it drops the `-W` flag, makes `/music` writable again, adds an explicit `beet write` step with before/after diff logging, and one-time-drops a wrongly-matched Muse album (adding it to `ignore` so it isn't re-matched). The change is narrowly scoped to `kubernetes/apps/navidrome/beets.yaml` and is backed by a real trial run (158/560 albums matched, all ≥96%, 0 of the 74 previously-written albums changed release). I traced the shell logic and the `ignore`/`incremental`/`write` interaction carefully and did not find a correctness bug. No blocking issues. ## 🟡 Suggestions - **`run.sh`: no hard failure if `beet write` leaves residual diffs.** After `beet write`, the script recomputes `write-after.txt` and only logs `items still differing after write` (`kubernetes/apps/navidrome/beets.yaml:126-127`) — it doesn't fail the job if that count is nonzero. Today this relies entirely on `wrc=$?` (`beets.yaml:124`) reflecting any per-file write error. If `beet write` ever returns 0 while silently skipping a subset of files (e.g. a permission error on one path that doesn't propagate to the overall exit code), the job would report green despite an incomplete write. Consider failing the job when the after-write diff count is nonzero, since you already compute it. - **Defense-in-depth note on the `/music` mount.** Dropping `readOnly: true` (`beets.yaml:186-189`) means the container-level write barrier is gone; going forward, "beets never moves/copies/deletes" is enforced only by the beets config (`move/copy/delete: no`, unchanged in this diff) rather than by Kubernetes. That's clearly a deliberate, trial-verified tradeoff per the PR description, just worth being aware of if the beets config ever changes without matching scrutiny. ## 🟢 Nits - The new `ignore` entry `'Black Holes and Revelations'` (`beets.yaml:44-46`) matches on basename only (beets `ignore` has no directory-scoped syntax), so it would also skip any other folder with that exact name anywhere under `/music`. Effectively a non-issue in practice given how specific the name is, just noting the mechanism isn't `Muse/`-scoped. ## Tests No automated tests apply to this manifest change beyond what the repo already gates: `.woodpecker/ci-manifests-test.yml`'s `kustomize-build` step will build the kustomization and catch structural/YAML breakage, and the PR body notes `run.sh` was checked with `sh -n`. I re-derived the embedded `run.sh` from the diff and walked it by hand (no logic branch left unreachable, `rc`/`wrc` capture and combination at `beets.yaml:109,124,132` is correct, the one-time `.drop-ren350` guard is idempotent). The real functional validation here is the out-of-band trial job (`beets-29845530`) described in the PR body, which is the appropriate form of "test" for this kind of stateful, slow, MusicBrainz-rate-limited operation — there's no practical way to unit-test it in CI. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#1073