fix(tagwriter): declare the track and disc totals when tagging
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m46s
CI / e2e (pull_request) Successful in 6m18s

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
This commit is contained in:
2026-08-18 18:19:54 -04:00
parent e049a71458
commit 4b9114fd8d
17 changed files with 805 additions and 21 deletions
+49 -5
View File
@@ -1547,11 +1547,55 @@ shape as the encoding probe beside it.
What neither side can give is *which* tracks are missing, only how many
— so an incomplete album still browses, and that is now the exception
rather than every album load. Two smaller consequences: existing databases
read "unknown" until a rescan repopulates the column (which degrades to
exactly the old behaviour, so nothing breaks), and our own `tagwriter`
writes track and disc *numbers* but not totals, so autotagging a folder
currently degrades the field this rests on.
rather than every album load. One smaller consequence: existing databases
read "unknown" until a rescan repopulates the column, which degrades to
exactly the old behaviour, so nothing breaks.
**And our own writers declare the total, because for a long time they
did not.** `tagwriter` wrote track and disc *numbers* and dropped the
totals, so autotagging an album actively **erased** the evidence this
rests on: the release became MBID-matched — a green tick — while the
field `GetAlbumCompleteness` reads stayed absent, which is exactly the
"2 of 10 tracks, reported as in your library" the report described.
`FieldTotalTracks` / `FieldTotalDiscs` are written by the autotag apply
pass and by the download importer, and `dbsync` persists the track
total to the row so the album page agrees with the file without waiting
for a rescan.
Five things about it are load-bearing, and four of them 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 written on every file multiplies a two-disc
album's expectation by two, and no library can then satisfy it.
`backend/tagtotals` is that derivation, once, because the two callers
must not import each other or the writer.
- **The Vorbis names are `TRACKTOTAL` and `DISCTOTAL` and no other
spelling.** `dhowden/tag`'s Vorbis reader looks at exactly those two
keys, so a perfectly reasonable `TOTALTRACKS`, or a `1/12` inside
`TRACKNUMBER`, is written successfully and reads back as no total at
all. The tests assert the round trip through the reader the *scan*
uses rather than through the bytes, for that reason.
- **ID3's number and total share one frame**, so writing either alone
has to read the other off the existing tag or it silently discards
it. A total with no number is not written: `/12` is what a reader
parses as track 0.
- **The totals are written unconditionally, not on a diff.** The case
this exists for is a file that declares *no* total, which compares
equal to nothing and is exactly what a "only if it changed" guard
skips.
- **A single-track download must not be 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 otherwise have answered
correctly. Confidently wrong is worse than absent here, which is the
same rule `Known` exists for.
One gap this did not close, and it is older: **`dhowden/tag` has no
RIFF reader**, so nothing the tag writer puts in a WAV's `id3 ` chunk
is visible to `metadata.ExtractTags` — not the totals and not the title
either. `wav_test.go` reads that chunk itself, which is why no test
ever noticed.
**The absence is what gets marked, not the presence.** The tracklist
put a green tick against every owned track and a legend underneath