Skip to content

refactor(main): resolve code smells via workflow abstraction#18

Merged
flexiondotorg merged 7 commits intomainfrom
refactor
Mar 17, 2026
Merged

refactor(main): resolve code smells via workflow abstraction#18
flexiondotorg merged 7 commits intomainfrom
refactor

Conversation

@flexiondotorg
Copy link
Contributor

@flexiondotorg flexiondotorg commented Mar 17, 2026

Summary

This refactor addresses six entrenched code smells through workflow abstraction and dependency inversion. The run() function previously contained 288 lines of mode-specific branching, primitive type handling, and tight coupling to the id3 package. By introducing a Workflow interface, mode-specific logic is now encapsulated in isolated implementations (Hugo, Standalone), reducing run() to 63 lines of orchestration code. The id3 package no longer imports the CLI package, eliminating circular dependency risk. Duplicated style declarations are consolidated in a shared location.

Changes

  • Extract Workflow interface defining mode-specific operations (build, validate, apply)
  • Implement HugoWorkflow and StandaloneWorkflow with isolated mode logic
  • Decompose run() from 288 lines to 63 lines via workflow delegation
  • Invert id3 package dependencies: artwork logging callbacks replace cli package imports
  • Consolidate duplicated style declarations into shared cli package
  • Unify loose metadata variables into TagInfo struct
  • Remove dead code (unused ctx assignment in encoder)
  • Reduce main_test.go by 618 lines via simplified test surface

Testing

  • All existing tests pass (mode-specific workflows tested in isolation)
  • HugoWorkflow verified with comprehensive template and metadata tests
  • StandaloneWorkflow verified with standalone-specific validation and application tests
  • Integration tests simplified by workflow abstraction; call paths unchanged

- Replace manual rune loop with strings.Map in sanitiseForFilename
- Use os.ReadFile instead of os.Open + bytes.Buffer.ReadFrom in
  artwork.go
- Use slices.Insert for inserting multiple elements in metadata.go
- Modernise benchmark loops from b.N to b.Loop()

Signed-off-by: Martin Wimpress <code@wimpress.io>
- Remove unused `ctx` variable assignment from main.go
- Replace duplicate lipgloss style declarations in styles.go with
  references to shared cli package styles
- Consolidate colour variables, removing unused ones (successColor,
  errorColor, highlightColor, textColor, borderColor)
- Move successStyle, errorStyle, highlightStyle, keyStyle, valueStyle,
  and boxStyle to shared cli package equivalents

Signed-off-by: Martin Wimpress <code@wimpress.io>
- Change ScaleCoverArt to accept logFn callback instead of importing cli
- Update callers to pass cli.PrintCover or test-appropriate callbacks
- Remove dead addCoverArt function and CoverArtPath field from TagInfo

Signed-off-by: Martin Wimpress <code@wimpress.io>
…ions

- Introduce Workflow interface with Validate(), CollectMetadata(),
  PostEncode()
- Implement HugoWorkflow: parses metadata from frontmatter, handles
  post-encode comparison and update prompting for podcast_duration and
  podcast_bytes
- Implement StandaloneWorkflow: builds metadata from CLI flags, displays
  stats
- Add newWorkflow() factory to instantiate correct workflow based on
  mode

This abstraction enables refactoring to simplify run().

Signed-off-by: Martin Wimpress <code@wimpress.io>
- Remove ~105 lines of inline mode-specific validation, metadata
  collection, and post-encode logic
- Replace with three workflow method calls: Validate(),
  CollectMetadata(), PostEncode()
- Eliminate seven loose variables (episodeNum, episodeTitle, artist,
  album, date, comment, coverArtPath) and hugoMetadata struct
- Thread tagInfo directly from CollectMetadata() to ID3 tag writing

Signed-off-by: Martin Wimpress <code@wimpress.io>
Wired workflow interface fully into run(), reducing it to under 100
lines with no mode-specific branching. Extracted encoding block into
encode() helper and moved validateHugoMode/validateStandaloneMode tests
into workflow implementations.

Fixes:
- Lowercase 5 error strings (ST1005)
- Remove trailing blank line (gofumpt)
- Remove unused error return from addCoverArtData (unparam)

Eliminates 7 loose metadata variables and 22+ linter errors.

Signed-off-by: Martin Wimpress <code@wimpress.io>
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 13 files

Confidence score: 3/5

  • There are concrete validation gaps in cmd/jivedrop/hugo.go and cmd/jivedrop/standalone.go: non-IsNotExist/non-ENOENT os.Stat errors are not handled, so unreadable/inaccessible paths can pass checks and fail later at runtime.
  • This creates user-facing failure risk (late errors instead of early validation), which keeps this in a moderate-risk range rather than a safe-to-merge-without-follow-up case.
  • The cmd/jivedrop/main.go issue is lower severity but still visible: background goroutine cover-art prints can interfere with the Bubble Tea progress UI output.
  • Pay close attention to cmd/jivedrop/hugo.go, cmd/jivedrop/standalone.go, and cmd/jivedrop/main.go - validation should reject inaccessible files early, and UI logging should not clash with the progress renderer.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cmd/jivedrop/hugo.go">

<violation number="1" location="cmd/jivedrop/hugo.go:32">
P2: Handle non-`IsNotExist` `os.Stat` errors during validation; unreadable files currently pass validation and fail later.</violation>
</file>

<file name="cmd/jivedrop/standalone.go">

<violation number="1" location="cmd/jivedrop/standalone.go:32">
P2: Handle non-ENOENT `os.Stat` errors here; inaccessible audio or cover paths currently pass validation.</violation>
</file>

<file name="cmd/jivedrop/main.go">

<violation number="1" location="cmd/jivedrop/main.go:222">
P2: Avoid printing cover-art status from the background goroutine while the Bubble Tea progress UI is running.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

- Replace os.IsNotExist checks with full error handling in Hugo and
  Standalone workflow validation; catches permission errors, broken
  symlinks, and I/O failures early
- Pass nil to cli.PrintCover callback in main.go to suppress logging
  during Bubble Tea UI rendering and prevent stdout corruption of
  progress display
- Update tests for both workflow implementations

Signed-off-by: Martin Wimpress <code@wimpress.io>
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 5 files (changes from recent commits).

Requires human review: Large architectural refactor (+2010 lines) restructuring core logic, metadata handling, and execution flow; requires human review for verification of structural changes.

@flexiondotorg flexiondotorg merged commit 852cd7e into main Mar 17, 2026
18 checks passed
@flexiondotorg flexiondotorg deleted the refactor branch March 17, 2026 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant