refactor: apply peer review findings across codebase

.github/workflows/builder.yml

@@ -32,11 +32,11 @@ jobs:
       - run: go vet ./...
       - name: Check cyclomatic complexity
         run: |
-          go install github.com/fzipp/gocyclo/cmd/gocyclo@latest
+          go install github.com/fzipp/gocyclo/cmd/gocyclo@v0.6.0
           gocyclo -top 20 -ignore '_test\.go$' -avg .
       - name: Check ineffectual assignments
         run: |
-          go install github.com/gordonklaus/ineffassign@latest
+          go install github.com/gordonklaus/ineffassign@v0.2.0
           ineffassign ./...
       - uses: golangci/golangci-lint-action@v9
 

.gitignore

@@ -1,6 +1,10 @@
 .direnv
 /jive-encoder
-testdata/
+testdata/*
+!testdata/0.md
+!testdata/LMP0.flac
+!testdata/linuxmatters-3000x3000.png
 *.mp3
 *.opus
 *.m4a
+coverage.out

AGENTS.md

@@ -19,7 +19,7 @@ nix develop  # Enter NixOS development shell (ffmpeg, lame, mediainfo, just, go)
 just build      # Build binary with version from git tags (CGO_ENABLED=1)
 just test       # Run all Go tests
 just test-encoder  # Integration test: encode testdata/LMP67.flac
-just clean      # Remove build artifacts and test outputs (*.mp3)
+just clean      # Remove build artifacts and test outputs (*.mp3/*.m4a/*.opus)
 ```
 
 ## Architecture
@@ -33,12 +33,11 @@ cmd/jive-encoder/
 internal/
   encoder/               # FFmpeg-based MP3/AAC/Opus encoding via ffmpeg-statigo
     encoder.go           # Core encode pipeline: decode → filter → encode → muxer-native tag
-    preset.go            # Per-format preset table (codec, bitrate, sample fmt/rate, muxer, extension, lowpass, cover)
+    preset.go            # Per-format preset table (codec, bitrate, sample fmt/rate, extension, lowpass, cover)
     metadata.go          # Hugo frontmatter parsing (YAML between --- delimiters) + muxer tag assembly
     stats.go             # Duration/filesize extraction from the encoded file
-  id3/                   # Cover-art scaling and tag-field carrier (no ID3 writer; FFmpeg muxers write tags)
+  artwork/               # Cover-art scaling (no tag writer; FFmpeg muxers write tags)
     artwork.go           # Cover art scaling (1400-3000px range for Apple Podcasts)
-    taginfo.go           # TagInfo carrier for episode metadata fields
   ui/                    # Bubbletea TUI for encoding progress
     encode.go            # Progress model with realtime speed calculation
   cli/                   # Lipgloss-styled output
@@ -67,11 +66,11 @@ third_party/ffmpeg-statigo/  # Git submodule: FFmpeg 8.1 static bindings
 
 ### Encoding Settings
 
-`internal/encoder/preset.go` holds the per-format preset table (the single source of truth for codec, bitrate, sample format, sample rate, muxer, extension, lowpass, cover capability). Mono is the default; `--stereo` selects the stereo bitrate.
+`internal/encoder/preset.go` holds the per-format preset table (the single source of truth for codec, bitrate, sample format, sample rate, extension, lowpass, cover capability). Mono is the default; `--stereo` selects the stereo bitrate.
 
-- **MP3 (default)**: CBR 112/192kbps, 44.1kHz, sample fmt `s16p`, LAME quality 3, 20.5kHz lowpass; `mp3` muxer → `.mp3`
-- **AAC-LC (`--format aac`)**: CBR 64/128kbps, 44.1kHz, sample fmt `fltp`, no lowpass; `ipod` muxer → `.m4a`
-- **Opus (`--format opus`)**: VBR ~32/~48kbps, 48kHz (libopus rejects 44.1kHz), sample fmt `flt` (libopus rejects `fltp`), `vbr=on`, compression_level 10, no lowpass; `opus` muxer → `.opus`
+- **MP3 (default)**: CBR 112/192kbps, 44.1kHz, sample fmt `s16p`, LAME quality 3, 20.5kHz lowpass; emits `.mp3`
+- **AAC-LC (`--format aac`)**: CBR 64/128kbps, 44.1kHz, sample fmt `fltp`, no lowpass; emits `.m4a`
+- **Opus (`--format opus`)**: VBR ~32/~48kbps, 48kHz (libopus rejects 44.1kHz), sample fmt `flt` (libopus rejects `fltp`), `vbr=on`, compression_level 10, no lowpass; emits `.opus`
 
 ### FFmpeg Integration
 
@@ -85,7 +84,7 @@ third_party/ffmpeg-statigo/  # Git submodule: FFmpeg 8.1 static bindings
 - Tagging is FFmpeg muxer-native: standard keys (`title`/`artist`/`album`/`date`/`comment`/`track`) go into an `AVDictionary` on the output format context before `AVFormatWriteHeader`, so each muxer writes its own format: ID3v2.4 (MP3, via the `id3v2_version=4` WriteHeader muxer option), iTunes MP4 atoms (M4A), Vorbis comments (Opus)
 - Title renders `"{episode}: {title}"`; track maps to the episode number; empty fields are skipped
 - Cover is an attached-picture stream (`AVDispositionAttachedPic`) written right after the header, for cover-capable formats (MP3, AAC) only. **Opus has no embedded cover** (text tags only)
-- `bogem/id3v2` is removed. `internal/id3/` holds only `artwork.go` (cover scaling) and `taginfo.go` (the `TagInfo` carrier)
+- `bogem/id3v2` is removed. `internal/artwork/` holds cover scaling; `encoder.Metadata` is the single tag-field carrier from the CLI workflows to the encoder
 
 ## Code Conventions
 

README.md

@@ -35,7 +35,7 @@ Jive Encoder takes your mixed podcast audio (WAV/FLAC) and outputs RSS-ready pod
 
 ### Hugo Mode (Integrated Workflow)
 
-For podcasts using Hugo static site generator and the something like [Castanet](https://github.com/mattstratton/castanet), Jive Encoder reads metadata from episode markdown:
+For podcasts using the Hugo static site generator and a theme like [Castanet](https://github.com/mattstratton/castanet), Jive Encoder reads metadata from episode markdown:
 
 **Hugo mode automatically:**
 - Reads episode title and number from frontmatter
@@ -115,7 +115,7 @@ Flags:
   --comment      Comment URL (defaults to 'https://linuxmatters.sh' in Hugo mode)
   --cover        Cover art path (required in standalone mode)
   --output-path  Output file or directory path
-  --format       Output format: mp3, aac, or opus (default: "mp3")
+  --format       Output format: mp3, aac, or opus (default: mp3)
   --stereo       Encode as stereo at 192kbps (default: mono at 112kbps)
   --version      Show version information
 ```
@@ -126,6 +126,8 @@ Flags:
 
 Where `{ext}` is `.mp3`, `.m4a`, or `.opus` depending on `--format`.
 
+If `--output-path` names a file, its extension must match `--format`.
+
 ### Encoding settings
 
 | Format | Mono | Stereo | Sample rate | Notes |

cmd/jive-encoder/help_test.go

@@ -0,0 +1,79 @@
+package main
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/alecthomas/kong"
+	"github.com/linuxmatters/jive-encoder/internal/cli"
+)
+
+// TestStyledHelpOutput renders --help against the real CLI struct through
+// StyledHelpPrinter. It checks the default annotations and colour handling.
+// The parser mirrors run(): same kong.Name, kong.Description, and kong.Help.
+// Writers go to a buffer and exit is stubbed so help does not end the test.
+func TestStyledHelpOutput(t *testing.T) {
+	// CLI is package-level mutable state shared with other tests. Parsing
+	// applies defaults to it, so snapshot and restore it around the parse.
+	saved := CLI
+	defer func() { CLI = saved }()
+
+	var buf bytes.Buffer
+	parser, err := kong.New(&CLI,
+		kong.Name("jive-encoder"),
+		kong.Description("Drop the mix, ship the show—metadata, cover art, and all."),
+		kong.Help(cli.StyledHelpPrinter),
+		kong.Writers(&buf, &buf),
+		kong.Exit(func(int) {}),
+	)
+	if err != nil {
+		t.Fatalf("failed to build parser: %v", err)
+	}
+
+	// --help triggers the help printer. The stubbed exit means Parse returns
+	// normally afterwards; any residual parse error does not matter here.
+	_, _ = parser.Parse([]string{"--help"})
+
+	out := buf.String()
+	if out == "" {
+		t.Fatal("expected help output, got empty buffer")
+	}
+
+	// Kong type names must never leak into the rendered defaults.
+	for _, leaked := range []string{"(default: STRING)", "(default: BOOL)"} {
+		if strings.Contains(out, leaked) {
+			t.Errorf("help output contains leaked type default %q", leaked)
+		}
+	}
+
+	// The buffer is not a TTY, so colorprofile must degrade to plain text.
+	if strings.Contains(out, "\x1b[") {
+		t.Error("help output contains ANSI escape sequences on a non-TTY writer")
+	}
+
+	stereoLine := findFlagLine(t, out, "--stereo")
+	// --stereo has no Kong default. Its single "(default: mono)" comes from
+	// the flag's help text, so a second occurrence means a rendered default.
+	if got := strings.Count(stereoLine, "(default:"); got != 1 {
+		t.Errorf("--stereo line has %d \"(default:\" occurrences, want 1: %q", got, stereoLine)
+	}
+
+	formatLine := findFlagLine(t, out, "--format")
+	if !strings.Contains(formatLine, "(default: mp3)") {
+		t.Errorf("--format line missing \"(default: mp3)\": %q", formatLine)
+	}
+}
+
+// findFlagLine returns the help output line containing the given flag. It
+// fails the test if the flag is absent.
+func findFlagLine(t *testing.T, out, flag string) string {
+	t.Helper()
+	for line := range strings.SplitSeq(out, "\n") {
+		if strings.Contains(line, flag) {
+			return line
+		}
+	}
+	t.Fatalf("help output has no line containing %q", flag)
+	return ""
+}

cmd/jive-encoder/hugo.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/linuxmatters/jive-encoder/internal/cli"
 	"github.com/linuxmatters/jive-encoder/internal/encoder"
-	"github.com/linuxmatters/jive-encoder/internal/id3"
 )
 
 // Hugo mode metadata defaults for the Linux Matters podcast.
@@ -22,7 +21,7 @@ const (
 type HugoWorkflow struct {
 	// opts carries the parsed CLI fields, populated at construction.
 	opts CLIOptions
-	// hugoMetadata is set during CollectMetadata and read during PostEncode
+	// hugoMetadata is set during CollectMetadata and read during PostEncode.
 	hugoMetadata *encoder.EpisodeMetadata
 }
 
@@ -32,7 +31,7 @@ func (h *HugoWorkflow) Validate() error {
 		return fmt.Errorf("hugo mode requires episode markdown file as second argument")
 	}
 
-	if !strings.HasSuffix(strings.ToLower(h.opts.EpisodeMD), ".md") {
+	if !isMarkdownPath(h.opts.EpisodeMD) {
 		return fmt.Errorf("episode markdown file must have .md extension: %s", h.opts.EpisodeMD)
 	}
 
@@ -51,10 +50,10 @@ func (h *HugoWorkflow) Validate() error {
 
 // CollectMetadata parses Hugo frontmatter, applies defaults and flag overrides,
 // and resolves the cover art path.
-func (h *HugoWorkflow) CollectMetadata() (id3.TagInfo, string, error) {
+func (h *HugoWorkflow) CollectMetadata() (encoder.Metadata, string, error) {
 	metadata, err := encoder.ParseEpisodeMetadata(h.opts.EpisodeMD)
 	if err != nil {
-		return id3.TagInfo{}, "", fmt.Errorf("failed to parse episode metadata: %w", err)
+		return encoder.Metadata{}, "", fmt.Errorf("failed to parse episode metadata: %w", err)
 	}
 	h.hugoMetadata = metadata
 
@@ -79,7 +78,7 @@ func (h *HugoWorkflow) CollectMetadata() (id3.TagInfo, string, error) {
 		episodeNum = h.opts.Num
 	}
 	if _, err := encoder.ParseEpisodeNumber(episodeNum); err != nil {
-		return id3.TagInfo{}, "", fmt.Errorf("invalid episode number: %w", err)
+		return encoder.Metadata{}, "", fmt.Errorf("invalid episode number: %w", err)
 	}
 	if h.opts.Date != "" {
 		date = h.opts.Date
@@ -91,11 +90,11 @@ func (h *HugoWorkflow) CollectMetadata() (id3.TagInfo, string, error) {
 	} else {
 		coverArtPath, err = encoder.ResolveCoverArtPath(h.opts.EpisodeMD, metadata.EpisodeImage)
 		if err != nil {
-			return id3.TagInfo{}, "", fmt.Errorf("failed to resolve cover art: %w", err)
+			return encoder.Metadata{}, "", fmt.Errorf("failed to resolve cover art: %w", err)
 		}
 	}
 
-	tagInfo := id3.TagInfo{
+	tags := encoder.Metadata{
 		EpisodeNumber: episodeNum,
 		Title:         episodeTitle,
 		Artist:        artist,
@@ -104,7 +103,7 @@ func (h *HugoWorkflow) CollectMetadata() (id3.TagInfo, string, error) {
 		Comment:       comment,
 	}
 
-	return tagInfo, coverArtPath, nil
+	return tags, coverArtPath, nil
 }
 
 // PostEncode displays podcast statistics and handles frontmatter comparison and update prompting.
@@ -127,7 +126,8 @@ func (h *HugoWorkflow) PostEncode(stats *encoder.FileStats) error {
 		needsUpdate = true
 	}
 
-	// Prompt user to update frontmatter if values differ or are missing
+	// A mismatch and a missing value both need confirmation, but each gets its
+	// own prompt wording so the user knows which case they are approving.
 	if needsUpdate {
 		promptAndUpdateFrontmatter(h.opts.EpisodeMD, "\nUpdate frontmatter with new values? [y/N]: ", stats.DurationString, stats.FileSizeBytes)
 	} else if h.hugoMetadata.PodcastDuration == "" || h.hugoMetadata.PodcastBytes == 0 {

cmd/jive-encoder/hugo_test.go

@@ -1,6 +1,8 @@
 package main
 
 import (
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -156,17 +158,19 @@ func TestHugoWorkflowValidate(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Construct the workflow with the test inputs threaded through
-			// CLIOptions. A dummy audio file keeps file-existence checks from
-			// masking the argument validation errors we are testing for.
+			episodeMD := tt.episodeMD
+			if !tt.wantErr {
+				episodeMD = existingMarkdownArgument(t, tt.episodeMD)
+			}
+
 			wf := &HugoWorkflow{opts: CLIOptions{
-				EpisodeMD: tt.episodeMD,
+				EpisodeMD: episodeMD,
 			}}
 			err := wf.Validate()
 
 			if tt.wantErr {
 				if err == nil {
-					t.Errorf("HugoWorkflow.Validate() expected error, got nil (EpisodeMD=%q)", tt.episodeMD)
+					t.Errorf("HugoWorkflow.Validate() expected error, got nil (EpisodeMD=%q)", episodeMD)
 					return
 				}
 				if tt.errMatch != "" && !strings.Contains(err.Error(), tt.errMatch) {
@@ -175,10 +179,8 @@ func TestHugoWorkflowValidate(t *testing.T) {
 				return
 			}
 
-			// For valid cases we only check argument validation, not file existence.
-			// File-not-found errors are acceptable here since the files do not exist on disk.
-			if err != nil && !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "not accessible") {
-				t.Errorf("HugoWorkflow.Validate() unexpected error: %v (EpisodeMD=%q)", err, tt.episodeMD)
+			if err != nil {
+				t.Errorf("HugoWorkflow.Validate() unexpected error: %v (EpisodeMD=%q)", err, episodeMD)
 			}
 		})
 	}
@@ -238,22 +240,53 @@ func TestHugoWorkflowValidate_Integration(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Construct the workflow with the test inputs threaded through
-			// CLIOptions. A dummy audio file keeps file-existence checks from
-			// masking the argument validation errors we are testing for.
+			episodeMD := tt.episodeMD
+			if !tt.wantErr {
+				episodeMD = existingMarkdownArgument(t, tt.episodeMD)
+			}
+
 			wf := &HugoWorkflow{opts: CLIOptions{
-				EpisodeMD: tt.episodeMD,
+				EpisodeMD: episodeMD,
 			}}
 			err := wf.Validate()
 
 			if tt.wantErr && err == nil {
 				t.Errorf("HugoWorkflow.Validate() expected error but got nil\n  Description: %s\n  EpisodeMD=%q",
-					tt.description, tt.episodeMD)
+					tt.description, episodeMD)
 			}
-			if !tt.wantErr && err != nil && !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "not accessible") {
+			if !tt.wantErr && err != nil {
 				t.Errorf("HugoWorkflow.Validate() unexpected error: %v\n  Description: %s\n  EpisodeMD=%q",
-					err, tt.description, tt.episodeMD)
+					err, tt.description, episodeMD)
 			}
 		})
 	}
 }
+
+func existingMarkdownArgument(t *testing.T, path string) string {
+	t.Helper()
+
+	root := t.TempDir()
+	workDir := filepath.Join(root, "work")
+	if err := os.MkdirAll(workDir, 0o755); err != nil {
+		t.Fatalf("create test work dir: %v", err)
+	}
+	t.Chdir(workDir)
+
+	arg := path
+	if filepath.IsAbs(path) {
+		arg = filepath.Join(root, strings.TrimPrefix(path, string(filepath.Separator)))
+	}
+
+	target := arg
+	if !filepath.IsAbs(target) {
+		target = filepath.Join(workDir, target)
+	}
+	if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil {
+		t.Fatalf("create markdown fixture dir: %v", err)
+	}
+	if err := os.WriteFile(target, []byte("---\n---\n"), 0o644); err != nil {
+		t.Fatalf("create markdown fixture: %v", err)
+	}
+
+	return arg
+}