fix(rating): preserve audio data on Set(); add unit tests

The previous implementation used tag.WriteTo() which only emits the ID3
tag (~311 bytes), leaving audio data behind. Files were truncated to just
the tag on every rating write.

New strategy:
1. Parse the 10-byte ID3v2 header to find the audio start offset.
2. Encode the new tag into an in-memory buffer via WriteTo.
3. Write tag + original audio into a temp file.
4. Try atomic os.Rename (works when Winamp does not hold the file).
5. Fall back to direct O_WRONLY|O_TRUNC write (works while Winamp plays,
   because Winamp opens with FILE_SHARE_WRITE on Windows).

Tests cover: POPM<->stars mapping, id3v2AudioStart, full round-trip
(1-5 stars + unrate) with audio-integrity check, and error cases.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

internal/rating/rating.go

@@ -15,6 +15,7 @@ package rating
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"math/big"
 	"os"
 	"path/filepath"
@@ -50,12 +51,15 @@ func Get(path string) (int, error) {
 // Set writes a 0–5 star rating into the POPM frame of path.
 // stars=0 removes any existing POPM frame (unrated).
 //
-// bogem/id3v2's Save() writes a temp file and renames it over the original,
-// which Windows denies when another process (Winamp) holds the file open
-// without FILE_SHARE_DELETE. Instead we use WriteTo to stream the modified
-// tag+audio into an in-memory buffer, close our read handle, then overwrite
-// the original file in-place — which succeeds because Winamp opens files
-// with FILE_SHARE_WRITE.
+// Strategy: bogem/id3v2's Save() creates a temp file and renames it over
+// the original. The rename fails when Winamp holds the file open (Windows
+// denies rename without FILE_SHARE_DELETE). Instead we:
+//  1. Find where the audio data starts (parse the ID3v2 header size).
+//  2. Write the new ID3 tag to an in-memory buffer via WriteTo.
+//  3. Build the complete new file (tag + audio) in a temp file.
+//  4. Try rename — works when the file is not currently playing.
+//  5. If rename fails, stream the temp file directly into the original file
+//     (requires Winamp to have opened with FILE_SHARE_WRITE, which it does).
 func Set(path string, stars int) error {
 	if !isMP3(path) {
 		return fmt.Errorf("rating.Set: not an MP3 file: %s", filepath.Base(path))
@@ -64,11 +68,17 @@ func Set(path string, stars int) error {
 		return fmt.Errorf("rating.Set: stars must be 0–5, got %d", stars)
 	}
 
+	// ── Step 1: locate audio data in the original file ────────────────────────
+	audioStart, err := id3v2AudioStart(path)
+	if err != nil {
+		return fmt.Errorf("rating.Set: parse header: %w", err)
+	}
+
+	// ── Step 2: encode new tag into memory ────────────────────────────────────
 	tag, err := id3.Open(path, id3.Options{Parse: true})
 	if err != nil {
 		return fmt.Errorf("rating.Set: open: %w", err)
 	}
-
 	tag.DeleteFrames("POPM")
 	if stars > 0 {
 		tag.AddFrame("POPM", id3.PopularimeterFrame{
@@ -77,27 +87,95 @@ func Set(path string, stars int) error {
 			Counter: big.NewInt(0),
 		})
 	}
-
-	// Stream modified tag + audio into memory buffer while file is still open.
-	var buf bytes.Buffer
-	if _, err := tag.WriteTo(&buf); err != nil {
+	var tagBuf bytes.Buffer
+	if _, err := tag.WriteTo(&tagBuf); err != nil {
 		tag.Close()
-		return fmt.Errorf("rating.Set: encode: %w", err)
+		return fmt.Errorf("rating.Set: encode tag: %w", err)
 	}
-	tag.Close() // release read handle before we open for writing
+	tag.Close() // release read handle on original file
 
-	// Overwrite the original file in-place (no rename needed).
-	f, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC, 0644)
+	// ── Step 3: write tag + audio to a temp file ──────────────────────────────
+	dir := filepath.Dir(path)
+	tmp, err := os.CreateTemp(dir, ".rating-*.tmp")
 	if err != nil {
-		return fmt.Errorf("rating.Set: open for write: %w", err)
+		return fmt.Errorf("rating.Set: create temp: %w", err)
 	}
-	defer f.Close()
-	if _, err := f.Write(buf.Bytes()); err != nil {
-		return fmt.Errorf("rating.Set: write: %w", err)
+	tmpPath := tmp.Name()
+	defer os.Remove(tmpPath) // clean up on any exit path
+
+	if _, err := tmp.Write(tagBuf.Bytes()); err != nil {
+		tmp.Close()
+		return fmt.Errorf("rating.Set: write tag to temp: %w", err)
+	}
+
+	// Copy audio data from original file into the temp file.
+	src, err := os.Open(path)
+	if err != nil {
+		tmp.Close()
+		return fmt.Errorf("rating.Set: open src: %w", err)
+	}
+	if _, err := src.Seek(audioStart, io.SeekStart); err != nil {
+		src.Close()
+		tmp.Close()
+		return fmt.Errorf("rating.Set: seek audio: %w", err)
+	}
+	if _, err := io.Copy(tmp, src); err != nil {
+		src.Close()
+		tmp.Close()
+		return fmt.Errorf("rating.Set: copy audio: %w", err)
+	}
+	src.Close()
+	if err := tmp.Close(); err != nil {
+		return fmt.Errorf("rating.Set: close temp: %w", err)
+	}
+
+	// ── Step 4: try atomic rename (works when file is not open by Winamp) ─────
+	if err := os.Rename(tmpPath, path); err == nil {
+		return nil
+	}
+
+	// ── Step 5: rename failed — stream temp into the original file directly ───
+	// This works because Winamp opens MP3s with FILE_SHARE_WRITE.
+	tmpFile, err := os.Open(tmpPath)
+	if err != nil {
+		return fmt.Errorf("rating.Set: open temp for streaming: %w", err)
+	}
+	defer tmpFile.Close()
+
+	dst, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC, 0644)
+	if err != nil {
+		return fmt.Errorf("rating.Set: open dst: %w", err)
+	}
+	defer dst.Close()
+
+	if _, err := io.Copy(dst, tmpFile); err != nil {
+		return fmt.Errorf("rating.Set: stream to dst: %w", err)
 	}
 	return nil
 }
 
+// id3v2AudioStart reads the 10-byte ID3v2 header and returns the byte offset
+// where audio data begins (i.e. the end of the tag). Returns 0 if no ID3v2
+// tag is present.
+func id3v2AudioStart(path string) (int64, error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return 0, err
+	}
+	defer f.Close()
+
+	var hdr [10]byte
+	if _, err := io.ReadFull(f, hdr[:]); err != nil {
+		return 0, nil // file too short — no tag
+	}
+	if string(hdr[:3]) != "ID3" {
+		return 0, nil // no ID3v2 tag
+	}
+	// Bytes 6–9 are a synchsafe integer (7 bits per byte).
+	size := int64(hdr[6])<<21 | int64(hdr[7])<<14 | int64(hdr[8])<<7 | int64(hdr[9])
+	return 10 + size, nil
+}
+
 func isMP3(path string) bool {
 	return strings.EqualFold(filepath.Ext(path), ".mp3")
 }

internal/rating/rating_test.go

@@ -0,0 +1,186 @@
+package rating
+
+import (
+	"bytes"
+	"os"
+	"testing"
+)
+
+// buildMinimalMP3 returns a minimal valid MP3: a 10-byte ID3v2.3 header with
+// no frames (tag size = 0) followed by 4096 bytes of fake audio data (0xFF 0xFB
+// sync word repeated, which is close enough for our purposes).
+func buildMinimalMP3() []byte {
+	var buf bytes.Buffer
+	// ID3v2.3 header: "ID3" + version 2.3.0 + flags 0 + synchsafe size 0
+	buf.Write([]byte{'I', 'D', '3', 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00})
+	// Fake audio data
+	audio := make([]byte, 4096)
+	for i := range audio {
+		audio[i] = byte(i & 0xFF)
+	}
+	buf.Write(audio)
+	return buf.Bytes()
+}
+
+func writeTempMP3(t *testing.T, data []byte) string {
+	t.Helper()
+	f, err := os.CreateTemp(t.TempDir(), "test-*.mp3")
+	if err != nil {
+		t.Fatalf("create temp: %v", err)
+	}
+	if _, err := f.Write(data); err != nil {
+		t.Fatalf("write temp: %v", err)
+	}
+	f.Close()
+	return f.Name()
+}
+
+func TestPopmToStars(t *testing.T) {
+	cases := []struct{ in uint8; want int }{
+		{0, 0}, {1, 1}, {31, 1}, {32, 2}, {95, 2},
+		{96, 3}, {127, 3}, {128, 3}, {159, 3},
+		{160, 4}, {195, 4}, {196, 4}, {223, 4},
+		{224, 5}, {255, 5},
+	}
+	for _, c := range cases {
+		got := popmToStars(c.in)
+		if got != c.want {
+			t.Errorf("popmToStars(%d) = %d, want %d", c.in, got, c.want)
+		}
+	}
+}
+
+func TestStarsToPOPM(t *testing.T) {
+	want := [6]uint8{0, 1, 64, 128, 196, 255}
+	for i, w := range want {
+		got := starsToPOPM(i)
+		if got != w {
+			t.Errorf("starsToPOPM(%d) = %d, want %d", i, got, w)
+		}
+	}
+}
+
+func TestId3v2AudioStart_noTag(t *testing.T) {
+	// File with no ID3 header
+	f, err := os.CreateTemp(t.TempDir(), "noTag-*.mp3")
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.Write([]byte{0xFF, 0xFB, 0x90, 0x00}) // MP3 sync
+	f.Close()
+
+	start, err := id3v2AudioStart(f.Name())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if start != 0 {
+		t.Errorf("got audioStart=%d, want 0", start)
+	}
+}
+
+func TestId3v2AudioStart_withHeader(t *testing.T) {
+	// Header with tag size 0 → audioStart should be 10
+	data := buildMinimalMP3()
+	path := writeTempMP3(t, data)
+
+	start, err := id3v2AudioStart(path)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if start != 10 {
+		t.Errorf("got audioStart=%d, want 10", start)
+	}
+}
+
+func TestSetAndGet_roundtrip(t *testing.T) {
+	original := buildMinimalMP3()
+	path := writeTempMP3(t, original)
+
+	// File should start with no rating
+	r, err := Get(path)
+	if err != nil {
+		t.Fatalf("Get before Set: %v", err)
+	}
+	if r != 0 {
+		t.Errorf("expected 0 before Set, got %d", r)
+	}
+
+	// Set each star value and read back
+	for stars := 1; stars <= 5; stars++ {
+		if err := Set(path, stars); err != nil {
+			t.Fatalf("Set(%d): %v", stars, err)
+		}
+
+		// File must not be truncated — must be larger than tag-only (> original size)
+		info, err := os.Stat(path)
+		if err != nil {
+			t.Fatalf("stat after Set(%d): %v", stars, err)
+		}
+		if info.Size() < int64(len(original)) {
+			t.Errorf("Set(%d) shrunk file: got %d bytes, original was %d",
+				stars, info.Size(), len(original))
+		}
+
+		got, err := Get(path)
+		if err != nil {
+			t.Fatalf("Get after Set(%d): %v", stars, err)
+		}
+		if got != stars {
+			t.Errorf("Get after Set(%d) = %d, want %d", stars, got, stars)
+		}
+	}
+
+	// Unrate (stars=0) must remove POPM
+	if err := Set(path, 0); err != nil {
+		t.Fatalf("Set(0): %v", err)
+	}
+	got, err := Get(path)
+	if err != nil {
+		t.Fatalf("Get after Set(0): %v", err)
+	}
+	if got != 0 {
+		t.Errorf("Get after Set(0) = %d, want 0", got)
+	}
+
+	// Audio data must be intact after all the Set calls
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("ReadFile: %v", err)
+	}
+	audioStart, _ := id3v2AudioStart(path)
+	if audioStart >= int64(len(data)) {
+		t.Fatalf("audioStart %d >= fileSize %d", audioStart, len(data))
+	}
+	got4096 := data[audioStart:]
+	orig4096 := original[10:] // original audio starts at byte 10 (tag size 0)
+	if len(got4096) < len(orig4096) {
+		t.Errorf("audio data truncated: got %d bytes, want %d", len(got4096), len(orig4096))
+	} else {
+		// Last 4096 bytes should match original audio
+		tail := got4096[len(got4096)-len(orig4096):]
+		if !bytes.Equal(tail, orig4096) {
+			t.Error("audio data corrupted after rating Set calls")
+		}
+	}
+}
+
+func TestSet_invalidStars(t *testing.T) {
+	path := writeTempMP3(t, buildMinimalMP3())
+	if err := Set(path, 6); err == nil {
+		t.Error("Set(6) should return error")
+	}
+	if err := Set(path, -1); err == nil {
+		t.Error("Set(-1) should return error")
+	}
+}
+
+func TestSet_notMP3(t *testing.T) {
+	f, err := os.CreateTemp(t.TempDir(), "test-*.flac")
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.Close()
+	if err := Set(f.Name(), 3); err == nil {
+		t.Error("Set on non-MP3 should return error")
+	}
+}