Answers #16 — the first item of the roadmap's (#73) Phase 1, and the head of the hard dependency chain #16 → #7, #38.
What was wrong
An album the user holds 2 of 10 tracks of showed a green tick reading "is in your library". The mechanism was our own writer: tagwriter wrote track and disc numbers and dropped the totals, so autotagging a folder made the release MBID-matched — which is what earns the tick — while erasing the one field GetAlbumCompleteness reads. The evidence for "2 of 10" was destroyed by the act that produced the tick.
Commits
Commit
Issue
fix(tagwriter): declare the track and disc totals when tagging
FieldTotalTracks / FieldTotalDiscs written as the ID3 n/N form and as Vorbis TRACKTOTAL/DISCTOTAL.
The autotag apply pass and the download importer fill them from the release's own tracklist.
dbsync persists the track total to audio_files.total_tracks, so the album page agrees with the file without waiting for a rescan.
backend/tagtotals is the per-disc derivation, once.
Four things that fail silently, and what pins each
Per disc, not per release.GetAlbumCompletenesssums the total per disc, so a release total on every file doubles a two-disc album's expectation. tagtotals_test.go and buildchanges_test.go.
TRACKTOTAL/DISCTOTAL and no other spelling.dhowden/tag's Vorbis reader looks at exactly those two keys, so TOTALTRACKS — which xiph lists and several taggers write — or a 1/12 inside TRACKNUMBER writes successfully and reads back as no total. totals_test.go asserts the round trip through metadata.ExtractTags, the reader the scan uses, rather than through the bytes.
ID3's number and total share one frame, so writing either alone must read the other off the existing tag. A total with no number is not written — /12 parses as track 0.
A single-track download is not totalled. A RecordingMBID anchor resolves Expected to that one track, so the same code would tag a track off a twelve-track album "1 of 1" — and a declared total outranks the catalog total that would have answered correctly.
Also: autotag's field constants are a second copy of tagwriter's (deliberately, to keep autotag out of the write pipeline's import graph). A key that drifts neither fails to compile nor fails to write — the writer just finds nothing under the name it looks for — so autotagservice, the one package importing both, now pins them.
Verification actually run
make lint — 0 issues, all three build configurations.
go test ./backend/... — clean; go test -tags indexbuild ./backend/explore/... ./cmd/... and go test -tags dev ./backend/testctl/... clean.
make skill-check — clean. bindings-check, codegen-check, go-vet via the pre-commit hooks; the full suite via pre-push.
#104 — the WAV round-trip test failed with every field zero, not just the totals: dhowden/tag has no RIFF reader, so nothing tagwriter puts in a WAV's id3 chunk is visible to metadata.ExtractTags. Pre-existing, invisible, and out of scope here — wav_test.go reads that chunk itself, which is why no test ever noticed.
Not done here
Steps 2 and 3 of #16's Direction stay open under #38: the catalog fallback (2) already landed as completenessAnswer(), and the badge call-site audit (3) is the part that overlaps it. #16 is closed by the footer because step 1 is the defect the report describes.
Answers #16 — the first item of the roadmap's (#73) Phase 1, and the head of the hard dependency chain `#16 → #7, #38`.
## What was wrong
An album the user holds 2 of 10 tracks of showed a green tick reading "is in your library". The mechanism was our own writer: `tagwriter` wrote track and disc *numbers* and dropped the totals, so autotagging a folder made the release MBID-matched — which is what earns the tick — while erasing the one field `GetAlbumCompleteness` reads. The evidence for "2 of 10" was destroyed by the act that produced the tick.
## Commits
| Commit | Issue |
| --- | --- |
| `fix(tagwriter): declare the track and disc totals when tagging` | #16 |
## What it does
- `FieldTotalTracks` / `FieldTotalDiscs` written as the ID3 `n/N` form and as Vorbis `TRACKTOTAL`/`DISCTOTAL`.
- The autotag apply pass and the download importer fill them from the release's own tracklist.
- `dbsync` persists the track total to `audio_files.total_tracks`, so the album page agrees with the file without waiting for a rescan.
- `backend/tagtotals` is the per-disc derivation, once.
## Four things that fail silently, and what pins each
- **Per disc, not per release.** `GetAlbumCompleteness` *sums* the total per disc, so a release total on every file doubles a two-disc album's expectation. `tagtotals_test.go` and `buildchanges_test.go`.
- **`TRACKTOTAL`/`DISCTOTAL` and no other spelling.** `dhowden/tag`'s Vorbis reader looks at exactly those two keys, so `TOTALTRACKS` — which xiph lists and several taggers write — or a `1/12` inside `TRACKNUMBER` writes successfully and reads back as *no total*. `totals_test.go` asserts the round trip through `metadata.ExtractTags`, the reader the scan uses, rather than through the bytes.
- **ID3's number and total share one frame**, so writing either alone must read the other off the existing tag. A total with no number is not written — `/12` parses as track 0.
- **A single-track download is not totalled.** A `RecordingMBID` anchor resolves `Expected` to that one track, so the same code would tag a track off a twelve-track album "1 of 1" — and a declared total outranks the catalog total that would have answered correctly.
Also: `autotag`'s field constants are a second copy of `tagwriter`'s (deliberately, to keep `autotag` out of the write pipeline's import graph). A key that drifts neither fails to compile nor fails to write — the writer just finds nothing under the name it looks for — so `autotagservice`, the one package importing both, now pins them.
## Verification actually run
- `make lint` — 0 issues, all three build configurations.
- `go test ./backend/...` — clean; `go test -tags indexbuild ./backend/explore/... ./cmd/...` and `go test -tags dev ./backend/testctl/...` clean.
- `make skill-check` — clean. `bindings-check`, `codegen-check`, `go-vet` via the pre-commit hooks; the full suite via pre-push.
- New: `TestWriteTotals_RoundTripsInEveryFormat` (mp3/flac/ogg/wav), `TestWriteMp3Totals_PartialUpdateKeepsTheOtherHalf`, `TestWriteTrackTags_PersistsTheTotal`, `TestFor`, `TestBuildChanges_Totals`, `TestImportWritesTheAlbumTotals`, `TestImportWritesNoTotalsForATrackDownload`, `TestAutotagAndTagwriterAgreeOnFieldNames`.
## Filed, not fixed
**#104** — the WAV round-trip test failed with *every* field zero, not just the totals: `dhowden/tag` has no RIFF reader, so nothing `tagwriter` puts in a WAV's `id3 ` chunk is visible to `metadata.ExtractTags`. Pre-existing, invisible, and out of scope here — `wav_test.go` reads that chunk itself, which is why no test ever noticed.
## Not done here
Steps 2 and 3 of #16's Direction stay open under #38: the catalog fallback (2) already landed as `completenessAnswer()`, and the badge call-site audit (3) is the part that overlaps it. #16 is closed by the footer because step 1 is the defect the report describes.
Closes #16
An album the user holds 2 of 10 tracks of showed a green tick reading
"is in your library", and the mechanism was our own writer. tagwriter
wrote track and disc *numbers* and dropped the totals, so autotagging a
folder made the release MBID-matched -- which is what earns the tick --
while erasing the one field GetAlbumCompleteness reads. The evidence
for "2 of 10" was destroyed by the act that produced the tick.
FieldTotalTracks and FieldTotalDiscs are written as the ID3 "n/N" form
and as Vorbis TRACKTOTAL/DISCTOTAL; the autotag apply pass and the
download importer fill them from the release's own tracklist; and
dbsync persists the track total to audio_files.total_tracks so the
album page agrees with the file without waiting for a rescan.
Five things about it are load-bearing, and four fail silently:
- The total is per *disc*, not per release, because that is what the
tag form declares and what GetAlbumCompleteness sums per disc. A
release total on every file multiplies a two-disc album's expectation
by two, which no library can satisfy. backend/tagtotals is that
derivation once, since the two callers must not import the writer or
each other.
- The Vorbis names are TRACKTOTAL and DISCTOTAL and no other spelling.
dhowden/tag reads exactly those two keys, so TOTALTRACKS -- which
xiph lists and several taggers write -- or a "1/12" packed into
TRACKNUMBER writes successfully and reads back as no total at all.
The tests therefore assert the round trip through the reader the scan
uses, not through the bytes.
- ID3's number and total share one frame, so writing either alone must
read the other off the existing tag or discard it. A total with no
number is not written: "/12" parses as track 0.
- The totals are written unconditionally rather than on a diff. The
case this exists for is a file declaring no total at all, which
compares equal to nothing and is exactly what a "only if it changed"
guard skips.
- A single-track download is not totalled. A RecordingMBID anchor
resolves Expected to that one track, so the same code would tag a
track off a twelve-track album "1 of 1" -- and a declared total
outranks the catalog total that would have answered correctly.
autotag's field constants are a second copy of tagwriter's, deliberately
so autotag stays out of the write pipeline's import graph. A key that
drifts neither fails to compile nor fails to write -- the writer simply
finds nothing under the name it looks for -- so autotagservice, the one
package importing both, now pins them.
Steps 2 and 3 of the issue stay open under #38: the catalog fallback
already landed as completenessAnswer(), and the badge call-site audit is
the part that overlaps it.
Closes#16
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Answers #16 — the first item of the roadmap's (#73) Phase 1, and the head of the hard dependency chain
#16 → #7, #38.What was wrong
An album the user holds 2 of 10 tracks of showed a green tick reading "is in your library". The mechanism was our own writer:
tagwriterwrote track and disc numbers and dropped the totals, so autotagging a folder made the release MBID-matched — which is what earns the tick — while erasing the one fieldGetAlbumCompletenessreads. The evidence for "2 of 10" was destroyed by the act that produced the tick.Commits
fix(tagwriter): declare the track and disc totals when taggingWhat it does
FieldTotalTracks/FieldTotalDiscswritten as the ID3n/Nform and as VorbisTRACKTOTAL/DISCTOTAL.dbsyncpersists the track total toaudio_files.total_tracks, so the album page agrees with the file without waiting for a rescan.backend/tagtotalsis the per-disc derivation, once.Four things that fail silently, and what pins each
GetAlbumCompletenesssums the total per disc, so a release total on every file doubles a two-disc album's expectation.tagtotals_test.goandbuildchanges_test.go.TRACKTOTAL/DISCTOTALand no other spelling.dhowden/tag's Vorbis reader looks at exactly those two keys, soTOTALTRACKS— which xiph lists and several taggers write — or a1/12insideTRACKNUMBERwrites successfully and reads back as no total.totals_test.goasserts the round trip throughmetadata.ExtractTags, the reader the scan uses, rather than through the bytes./12parses as track 0.RecordingMBIDanchor resolvesExpectedto that one track, so the same code would tag a track off a twelve-track album "1 of 1" — and a declared total outranks the catalog total that would have answered correctly.Also:
autotag's field constants are a second copy oftagwriter's (deliberately, to keepautotagout of the write pipeline's import graph). A key that drifts neither fails to compile nor fails to write — the writer just finds nothing under the name it looks for — soautotagservice, the one package importing both, now pins them.Verification actually run
make lint— 0 issues, all three build configurations.go test ./backend/...— clean;go test -tags indexbuild ./backend/explore/... ./cmd/...andgo test -tags dev ./backend/testctl/...clean.make skill-check— clean.bindings-check,codegen-check,go-vetvia the pre-commit hooks; the full suite via pre-push.TestWriteTotals_RoundTripsInEveryFormat(mp3/flac/ogg/wav),TestWriteMp3Totals_PartialUpdateKeepsTheOtherHalf,TestWriteTrackTags_PersistsTheTotal,TestFor,TestBuildChanges_Totals,TestImportWritesTheAlbumTotals,TestImportWritesNoTotalsForATrackDownload,TestAutotagAndTagwriterAgreeOnFieldNames.Filed, not fixed
#104 — the WAV round-trip test failed with every field zero, not just the totals:
dhowden/taghas no RIFF reader, so nothingtagwriterputs in a WAV'sid3chunk is visible tometadata.ExtractTags. Pre-existing, invisible, and out of scope here —wav_test.goreads that chunk itself, which is why no test ever noticed.Not done here
Steps 2 and 3 of #16's Direction stay open under #38: the catalog fallback (2) already landed as
completenessAnswer(), and the badge call-site audit (3) is the part that overlaps it. #16 is closed by the footer because step 1 is the defect the report describes.Closes #16