feat: add lyra track CLI command#149
Conversation
LaunchAgentのPATHが最小限のため/opt/homebrew/binの yt-dlp/ffmpegが見つからない問題を修正。 findExecutableInPathでwell-known pathsを直接チェックしてから /usr/bin/whichにフォールバック。 CLAUDE.mdにGit Workflowルールを追記。 Closes #144
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
独立スクリプト(now-playing)の機能をlyra本体に統合。 既存のPlaybackUseCase/MetadataUseCase/LyricsUseCaseを活用し、 再生中楽曲情報をJSON出力するサブコマンドを追加した。 主な変更: - TrackCommand: --resolve(-r)でメタデータ解決、--lyrics(-l)で歌詞取得 - NowPlayingInfo: CLI出力用Codableレスポンス型 - NowPlayingRepository/PlaybackUseCase: fetch()ワンショット取得を追加 - RootCommand: @main + AsyncParsableCommandに移行、Mainターゲット廃止 - HealthcheckCommand: AsyncParsableCommandに移行しRunLoop削除 Closes #148
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis pull request integrates a now-playing CLI command ( Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as TrackCommand
participant PB as PlaybackUseCase
participant MD as MetadataUseCase
participant LY as LyricsUseCase
participant Enc as JSONEncoder
CLI->>PB: fetchNowPlaying()
PB-->>CLI: NowPlaying (title, artist, elapsed)
alt resolve flag enabled
CLI->>MD: resolveMetadata(track)
MD-->>CLI: [Track] candidates
CLI->>CLI: Select resolved title/artist
end
alt lyrics flag enabled
CLI->>LY: fetchLyrics(title, artist)
LY-->>CLI: LyricsContent (lines with timestamps)
CLI->>CLI: Parse syncedLyrics<br/>Compute currentLyric
end
CLI->>Enc: encode(NowPlayingInfo)
Enc-->>CLI: JSON string
CLI->>CLI: Print output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a new lyra track CLI subcommand to output the current “now playing” state as JSON, integrating existing Playback/Metadata/Lyrics layers and simplifying the CLI entrypoint by making CLI the executable target.
Changes:
- Add
lyra track(--resolve/--lyrics) and introduceNowPlayingInfoJSON response model (plusLyricLine: Codable). - Add one-shot retrieval APIs:
NowPlayingRepository.fetch()andPlaybackUseCase.fetchNowPlaying()(with accompanying tests). - Migrate CLI entrypoint to
@main RootCommand: AsyncParsableCommand, converthealthcheckto async, and remove theMaintarget.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/TrackInteractorTests/TrackInteractorRaceTests.swift | Updates stub to satisfy new PlaybackUseCase.fetchNowPlaying() requirement. |
| Tests/PlaybackUseCaseTests/PlaybackUseCaseTests.swift | Adds tests for PlaybackUseCase.fetchNowPlaying() and updates repository mock to support fetch(). |
| Tests/NowPlayingRepositoryTests/NowPlayingRepositoryTests.swift | Adds coverage for NowPlayingRepository.fetch() behavior across MediaRemote results. |
| Sources/WallpaperDataSource/FindExecutable.swift | Improves executable discovery by checking common Homebrew/system paths before falling back to which. |
| Sources/PlaybackUseCase/PlaybackUseCaseImpl.swift | Implements fetchNowPlaying() by delegating to repository fetch(). |
| Sources/NowPlayingRepository/NowPlayingRepositoryImpl.swift | Implements one-shot fetch() via a single MediaRemote poll. |
| Sources/Main/main.swift | Removes old entrypoint (RootCommand.main()), superseded by @main in CLI. |
| Sources/Entity/NowPlayingInfo.swift | Adds NowPlayingInfo: Codable payload used by lyra track. |
| Sources/Entity/LyricLine.swift | Adds Codable conformance for JSON output of synced lyric lines. |
| Sources/Domain/Dependencies/UseCase/PlaybackUseCase.swift | Extends use case protocol with one-shot fetchNowPlaying(). |
| Sources/Domain/Dependencies/Repository/NowPlayingRepository.swift | Extends repository protocol with one-shot fetch(). |
| Sources/CLI/Resources/version.txt | Bumps CLI version string. |
| Sources/CLI/Commands/TrackCommand.swift | New async subcommand to output now-playing info (optionally resolving metadata and/or fetching lyrics). |
| Sources/CLI/Commands/RootCommand.swift | Makes CLI the entrypoint via @main + async root command and registers TrackCommand. |
| Sources/CLI/Commands/HealthcheckCommand.swift | Converts to AsyncParsableCommand and removes manual RunLoop/locking. |
| Package.swift | Removes Main executable target and makes CLI the executable product/entry target. |
| Package.resolved | Updates resolved dependencies/pins due to package graph change. |
| CLAUDE.md | Updates architecture documentation to reflect CLI as the entrypoint and documents track behavior/APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1 +1 @@ | |||
| 2.5.5 | |||
| 2.5.7 | |||
There was a problem hiding this comment.
Version bump doesn’t follow the repo’s documented semver rule for PRs: this PR is titled feat: so it should be a minor bump (CLAUDE.md:267-270), but version.txt is bumped as a patch from 2.5.5 → 2.5.7. Please bump to the next minor (e.g., 2.6.0) instead.
| 2.5.7 | |
| 2.6.0 |
| private func resolvedMetadata(track: Track) async -> (String, String, [Track]) { | ||
| @Dependency(\.metadataUseCase) var metadataUseCase | ||
|
|
||
| guard resolve || lyrics else { return (track.title, track.artist, []) } | ||
|
|
||
| let candidates = await metadataUseCase.resolveCandidates(track: track) | ||
| let title = candidates.first?.title ?? track.title | ||
| let artist = candidates.first?.artist ?? track.artist | ||
| return (title, artist, candidates) |
There was a problem hiding this comment.
--resolve and --lyrics are documented as independent flags (PR description / CLAUDE.md ‘Track command’), but this guard resolves metadata when either flag is set. As a result, lyra track --lyrics will still call MetadataUseCase.resolveCandidates and may output resolved title/artist even though --resolve wasn’t requested. Consider changing this logic so metadata resolution is controlled solely by --resolve (or update the flag help/docs to reflect that --lyrics implies resolution).
There was a problem hiding this comment.
This is intentional. --lyrics requires resolveCandidates to generate clean search candidates — without it, raw MediaRemote titles (e.g. YouTube titles with brackets, noise words) produce poor LRCLIB hits. The candidates are passed directly to fetchLyrics(candidates:). The flags are independent in that -r alone resolves without fetching lyrics, and -l alone fetches lyrics (which internally needs resolution for accuracy).
| private func encode(_ info: NowPlayingInfo) -> String { | ||
| let encoder = JSONEncoder() | ||
| encoder.outputFormatting = [.prettyPrinted, .sortedKeys] | ||
| return (try? encoder.encode(info)).flatMap { String(data: $0, encoding: .utf8) } ?? "{}" | ||
| } |
There was a problem hiding this comment.
encode(_:) silently falls back to "{}" if JSON encoding fails, which can hide real runtime issues (e.g. non-conforming Double values) and still exit with status 0. Since run() is already async throws, consider making encode throwing (or throwing a ValidationError) so failures are surfaced to the caller instead of producing potentially misleading output.
There was a problem hiding this comment.
Same as above — NowPlayingInfo is all standard Codable optionals. Encoding failure is not realistic here. The {} fallback is a defensive default, not an error suppression.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/CLI/Commands/TrackCommand.swift`:
- Around line 54-59: The current guard (guard resolve || lyrics) causes
metadataUseCase.resolveCandidates(track:) to run when only --lyrics is set;
change the control flow so resolveCandidates is invoked only when resolve is
true. Replace the guard condition with a guard resolve else { return
(track.title, track.artist, []) } (or equivalently branch on resolve first) so
that if --lyrics is provided but --resolve is not, the function returns the
original track title/artist and an empty candidates array without calling
metadataUseCase.resolveCandidates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 358bf5c9-aed7-400d-9711-b1616274a6d0
📒 Files selected for processing (18)
CLAUDE.mdPackage.resolvedPackage.swiftSources/CLI/Commands/HealthcheckCommand.swiftSources/CLI/Commands/RootCommand.swiftSources/CLI/Commands/TrackCommand.swiftSources/CLI/Resources/version.txtSources/Domain/Dependencies/Repository/NowPlayingRepository.swiftSources/Domain/Dependencies/UseCase/PlaybackUseCase.swiftSources/Entity/LyricLine.swiftSources/Entity/NowPlayingInfo.swiftSources/Main/main.swiftSources/NowPlayingRepository/NowPlayingRepositoryImpl.swiftSources/PlaybackUseCase/PlaybackUseCaseImpl.swiftSources/WallpaperDataSource/FindExecutable.swiftTests/NowPlayingRepositoryTests/NowPlayingRepositoryTests.swiftTests/PlaybackUseCaseTests/PlaybackUseCaseTests.swiftTests/TrackInteractorTests/TrackInteractorRaceTests.swift
💤 Files with no reviewable changes (1)
- Sources/Main/main.swift
CI failed: 1 flaky race condition test failure in TrackInteractorRaceTests.swift where 'rapid track change cancels stale resolution' test intermittently fails due to timing-sensitive assertion on Track B resolution completion.OverviewAnalyzed 1 CI log from job 69529850652. The test suite ran 362 tests successfully with 1 failure: a flaky race condition test in TrackInteractorRaceTests.swift. This appears to be a timing-sensitive intermittent issue rather than a deterministic build or code defect introduced by the PR changes. FailuresRace Condition Test Timeout in TrackInteractorRaceTests (confidence: medium)
Summary
Code Review ✅ Approved 1 resolved / 1 findingsAdds ✅ 1 resolved✅ Edge Case: JSON encoding errors silently swallowed, fallback may mislead
Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
概要
再生中楽曲情報をJSON出力する
lyra trackサブコマンドを追加。既存のPlaybackUseCase/MetadataUseCase/LyricsUseCaseを活用し、独立スクリプト (now-playing) の機能をlyra本体に統合した。変更内容
lyra trackサブコマンド追加（--resolve/--lyricsフラグ独立）NowPlayingInfoレスポンス型（Entity）、LyricLineにCodable追加NowPlayingRepository/PlaybackUseCaseにワンショットfetch()追加HealthcheckCommandをAsyncParsableCommandに移行、RunLoop削除RootCommandを@main+AsyncParsableCommandに移行、Mainターゲット廃止背景・動機
~/.config/bin/now-playingとして独立していたスクリプトの機能をlyra本体のCLIに統合し、コードの重複を解消する。lyraの既存DI配線をそのまま活用できるため、新規登録は不要。Closes #148
テスト計画
NowPlayingRepository.fetch()テスト（.info / .noInfo / .eof）PlaybackUseCase.fetchNowPlaying()テストlyra track実行確認（再生なし→空JSON）lyra track/lyra track -r/lyra track -l/lyra track -rlを確認Summary by CodeRabbit
New Features
lyra trackCLI command that outputs now-playing information as JSON--resolve/-rand--lyrics/-lflags to the track commandChores