Skip to content

Phase 2/now playing - #3

Merged
tukuyomil032 merged 26 commits into
mainfrom
phase-2/now-playing
Jun 5, 2026
Merged

Phase 2/now playing#3
tukuyomil032 merged 26 commits into
mainfrom
phase-2/now-playing

Conversation

@tukuyomil032

@tukuyomil032 tukuyomil032 commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • より多くの歌詞提供経路を追加し、取得成功率を向上しました
  • Bug Fixes

    • 歌詞キャッシュの挙動を改善し、空データの誤キャッシュを防止しました
  • UI Improvements

    • 歌詞表示の幅・余白・行制限・アニメーション調整、フルビュー高さ拡大で可読性を向上
  • Releases

    • アプリのマーケティングバージョンを更新し、ベータ判定に応じたリリース包装を改善
  • Documentation

    • サードパーティ表記を更新しました

tukuyomil032 and others added 3 commits June 5, 2026 08:36
…rics' cache

Combining lyricsPrefetchTask?.cancel() with cache[key] = [] sentinel caused
CancellationError to be treated as 'no lyrics found', poisoning the cache.
Cancelled fetches must be retried on next play.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Without frame(maxWidth: .infinity), Text uses intrinsic width.
Short lines appeared left-aligned because LazyVStack centers relative to
the widest child, not the container.

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

sentinel was set on any non-cancelled failure (album_name mismatch, timeout, network error); once set, lyrics would never load for that song until app restart

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 5, 2026 01:59
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@tukuyomil032, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 32 minutes and 8 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 41fd3e2f-26e3-48a2-beef-c051da2a809f

📥 Commits

Reviewing files that changed from the base of the PR and between 623e51f and 18a0fd1.

📒 Files selected for processing (1)
  • .github/workflows/release.yml
📝 Walkthrough

Walkthrough

リリース/CIワークフローとXcodeプロジェクトの依存を更新し、歌詞取得に LyricsKitFetcher を追加、LyricsStore のキャッシュ挙動と検索シグネチャを変更、UI 表示・通知判定・Info.plist の ATS 例外を修正しています。

Changes

リリースとパッケージ更新

Layer / File(s) Summary
リリースワークフロー更新
.github/workflows/release.yml, .github/workflows/ci.yml, .github/workflows/test.yml
release/ci/test ジョブの runner を macOS-26 系へ更新。release の version 解決を拡張し MARKETING_VERSION を archive に注入、ベータ判定でバンドル名を切替、動的リリース本文生成と prerelease 制御を追加。xcodebuild 呼び出しに skip オプションを追加し、xcbeautify への出力処理を調整。
Xcode プロジェクトと SPM 参照
perch.xcodeproj/project.pbxproj
LyricsKit を SwiftPM 依存として project/pbxproj に追加し、perch ターゲットの Frameworks フェーズと packageProductDependencies を更新。MARKETING_VERSION を 0.3.0 に更新。
依存解決結果とサードパーティ表記
perch.xcodeproj/.../Package.resolved, THIRD_PARTY_NOTICES.md
Package.resolved に複数パッケージピン(LyricsKit 等)を追加・更新し、THIRD_PARTY_NOTICES.md に LyricsKit のライセンス情報を追記。

Now Playing:歌詞取得と表示改善

Layer / File(s) Summary
LyricsKitFetcher の追加
perch/Features/NowPlaying/LyricsKitFetcher.swift
actor として LyricsKitFetcher を追加。netease/qq/kugou の各プロバイダを順次試行し、タイトル一致・行有無・日本語含有判定で歌詞行配列をフィルタして返す。
LyricsStore のキャッシュと検索API改良
perch/Features/NowPlaying/LyricsStore.swift
fetchLyrics は既存キャッシュを優先して返し、外部取得で結果が得られた場合のみキャッシュへ保存。空配列を sentinel として保存するロジックを削除。fetchSearch の album 引数を廃止して album_name をクエリに含めない。
UI 表示調整と表示領域拡大
perch/Features/NowPlaying/LyricsView.swift, perch/Features/NowPlaying/NowPlayingCard.swift
行テキストのフォントを固定化、lineLimit(2)frame(maxWidth:.infinity) を追加。scaleEffect・アニメーション応答・横パディングを調整し、Full view の TimelineView 高さを 160→200 に拡大。
再生通知判定と ATS 例外追加
perch/Features/NowPlaying/NowPlayingManager.swift, perch/Resources/Info.plist
Spotify 通知内の広告判定を Track ID 接頭辞および Name 空判定へ変更し、playerState nil ガードを追加。Info.plist に music.163.com の ATS 例外を追加。

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

ぴょんと跳ねるウサギが言うよ、
新しい歌を探しては、順に試し、
依存を並べ、浮かぶ文字は広く、
キャッシュは賢く、無駄は置かず、
リリースは整って、また跳ねるよ 🐰✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive プルリクエストのタイトル「Phase 2/now playing」は、变更内容全体を指す曖昧なフェーズ参照であり、実装の主要な変更内容(歌詞機能の追加、LyricsKitの統合など)を具体的に説明していません。 タイトルをより具体的に変更してください。例えば「Add lyrics fetching and display with LyricsKit integration」など、主要な機能追加を明確に説明するタイトルが望ましいです。
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch phase-2/now-playing

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refines the Now Playing lyrics experience by adjusting lyrics line layout in the UI and modifying how lyrics are fetched/cached from the LRCLib endpoints.

Changes:

  • Make each lyric line expand to the full available width to improve centered multiline rendering.
  • Adjust LyricsStore caching and simplify the LRCLib search request construction.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
perch/Features/NowPlaying/LyricsView.swift Ensures lyric Text views take full width for more consistent centered layout.
perch/Features/NowPlaying/LyricsStore.swift Updates lyrics fetch flow, including cache handling and the search request signature/query building.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +53 to 57
if let lines = await fetchSearch(title: title, artist: artist) {
cache[key] = lines
return lines
}
cache[key] = [] // sentinel: no lyrics — skip network on repeat plays
return nil

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@perch/Features/NowPlaying/LyricsStore.swift`:
- Around line 45-53: fetchLyrics(title:artist:album:) currently has an unused
album parameter which makes the API contract misleading; either remove the album
parameter from the function signature (and update all callers) or incorporate
album into the logic (include album in the cache key and pass it into
fetchSearch/fetchGet or reintroduce album-aware variants). Locate fetchLyrics,
the cache lookup/assignment (cache[key]), and calls to fetchGet/fetchSearch and
then: A) remove album from fetchLyrics and all call sites if album is not
needed, or B) change key to "\(title)|\(artist)|\(album ?? "")" and update
fetchGet/fetchSearch to accept and use album so the parameter is actually
applied to searches and caching.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2698ca91-9fd0-40c2-b517-2f75b41070a4

📥 Commits

Reviewing files that changed from the base of the PR and between def9b5e and 13128e6.

📒 Files selected for processing (2)
  • perch/Features/NowPlaying/LyricsStore.swift
  • perch/Features/NowPlaying/LyricsView.swift

Comment on lines 45 to +53
func fetchLyrics(title: String, artist: String, album: String?) async -> [LyricsLine]? {
let key = "\(title)|\(artist)"
if let cached = cache[key] { return cached.isEmpty ? nil : cached }
if let cached = cache[key] { return cached }

if let lines = await fetchGet(title: title, artist: artist) {
cache[key] = lines
return lines
}
if let lines = await fetchSearch(title: title, artist: artist, album: album) {
if let lines = await fetchSearch(title: title, artist: artist) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

album 引数が未使用のまま残っておりAPI契約が曖昧です

fetchSearch から album を外したため、fetchLyrics(title:artist:album:)album が実質無意味になっています。呼び出し側の期待値を誤らせるので、fetchLyrics からも引数を削除するか、逆に検索条件/キャッシュキーへ反映して契約を揃えてください。

差分案(引数を削除する場合)
-    func fetchLyrics(title: String, artist: String, album: String?) async -> [LyricsLine]? {
+    func fetchLyrics(title: String, artist: String) async -> [LyricsLine]? {
         let key = "\(title)|\(artist)"
         if let cached = cache[key] { return cached }

         if let lines = await fetchGet(title: title, artist: artist) {
             cache[key] = lines
             return lines
         }
         if let lines = await fetchSearch(title: title, artist: artist) {
             cache[key] = lines
             return lines
         }
         return nil
     }

As per coding guidelines, "Adhere to Swift API Design Guidelines for naming conventions... rely on module separation for disambiguation."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func fetchLyrics(title: String, artist: String, album: String?) async -> [LyricsLine]? {
let key = "\(title)|\(artist)"
if let cached = cache[key] { return cached.isEmpty ? nil : cached }
if let cached = cache[key] { return cached }
if let lines = await fetchGet(title: title, artist: artist) {
cache[key] = lines
return lines
}
if let lines = await fetchSearch(title: title, artist: artist, album: album) {
if let lines = await fetchSearch(title: title, artist: artist) {
func fetchLyrics(title: String, artist: String) async -> [LyricsLine]? {
let key = "\(title)|\(artist)"
if let cached = cache[key] { return cached }
if let lines = await fetchGet(title: title, artist: artist) {
cache[key] = lines
return lines
}
if let lines = await fetchSearch(title: title, artist: artist) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@perch/Features/NowPlaying/LyricsStore.swift` around lines 45 - 53,
fetchLyrics(title:artist:album:) currently has an unused album parameter which
makes the API contract misleading; either remove the album parameter from the
function signature (and update all callers) or incorporate album into the logic
(include album in the cache key and pass it into fetchSearch/fetchGet or
reintroduce album-aware variants). Locate fetchLyrics, the cache
lookup/assignment (cache[key]), and calls to fetchGet/fetchSearch and then: A)
remove album from fetchLyrics and all call sites if album is not needed, or B)
change key to "\(title)|\(artist)|\(album ?? "")" and update
fetchGet/fetchSearch to accept and use album so the parameter is actually
applied to searches and caching.

tukuyomil032 and others added 10 commits June 5, 2026 11:13
LyricsKit NetEase provider uses http://music.163.com/api/search/pc. Without this exception, ATS blocks the request at runtime on macOS.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
trackNumber/popularity checks were unreliable (fields may be absent in newer Spotify). Ads always deliver an empty Name field, so name.isEmpty && playerState == Playing is a more reliable signal.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
LyricsKit is MPL-2.0 licensed. Attribution required; Perch source need not be opened as long as LyricsKit files are unmodified.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove fontSize+1 on active line to prevent unstable word-wrap. Use scaleEffect(1.10) + .bold for visual emphasis instead. Add lineLimit(2) for natural 2-line wrap. Extend maxHeight to 200pt and ease spring to response:0.40.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Integrate LyricsKit SPM (MxIris-LyricsX-Project/LyricsKit v1.9.0, MPL-2.0). New LyricsKitFetcher actor tries NetEase → QQ → Kugou after LRCLIB misses. LyricsLine name collision avoided: Perch type uses timestamp:text: init which is distinct from LyricsCore.LyricsLine.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
scaleEffect(1.10) overflows layout frame by ~5% on each side; 4pt padding was too small. Increase to 16pt to absorb the overflow.

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

NetEase returns Chinese placeholder text for songs without synced lyrics. Detect Japanese titles (hiragana/katakana) and skip results where no lyrics line contains Japanese script.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Changing .regular→.bold on active line caused text to re-wrap differently, making words abruptly jump to line 2 without animation. Use .regular for all lines and increase scaleEffect to 1.13 to compensate.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prepare for v0.3.0-beta.1 release

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Detect beta tags via regex (-[a-zA-Z]) and set IS_BETA flag
- Pass MARKETING_VERSION (base version) to all xcodebuild invocations
- Dynamic prerelease flag in GitHub Release creation
- Compose dynamic release body with correct brew cask name (perch-beta vs perch)
- Add Homebrew tap auto-update step using HOMEBREW_TAP_TOKEN secret
- Compute SHA256 of universal DMG for Homebrew cask formula

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
perch/Features/NowPlaying/LyricsView.swift (1)

28-29: ⚡ Quick win

フル表示でも歌詞が2行で切れる構成になっています。

Line 28 の .lineLimit(2) 固定により、NowPlayingCard のフル歌詞表示でも長い1行が省略されます。コンパクト表示だけ2行制限にして、フル表示は無制限に分けるのが安全です。

💡 提案パッチ
--- a/perch/Features/NowPlaying/LyricsView.swift
+++ b/perch/Features/NowPlaying/LyricsView.swift
@@
 struct LyricsView: View {
     let lines: [LyricsLine]
     let elapsedTime: TimeInterval
     var fontSize: CGFloat = 13
+    var maxLineLimit: Int? = nil
@@
-                            .lineLimit(2)
+                            .lineLimit(maxLineLimit)
--- a/perch/Features/NowPlaying/NowPlayingCard.swift
+++ b/perch/Features/NowPlaying/NowPlayingCard.swift
@@
                         LyricsView(
                             lines: lyrics,
                             elapsedTime: state.liveElapsed(at: ctx.date) ?? 0,
-                            fontSize: 12
+                            fontSize: 12,
+                            maxLineLimit: 2
                         )
@@
                 LyricsView(
                     lines: lyrics,
                     elapsedTime: state.liveElapsed(at: ctx.date) ?? 0,
-                    fontSize: 14
+                    fontSize: 14,
+                    maxLineLimit: nil
                 )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@perch/Features/NowPlaying/LyricsView.swift` around lines 28 - 29, The lyrics
Text view in LyricsView.swift is hard-capped with .lineLimit(2), causing
full-display lyrics in NowPlayingCard to be truncated; change the modifier to be
conditional (e.g., .lineLimit(isCompact ? 2 : nil) or .lineLimit(isExpanded ?
nil : 2)) using the existing state/prop that distinguishes compact vs full views
(pass or use the NowPlayingCard/ LyricsView property like isCompact/isExpanded),
so compact mode keeps 2 lines but full mode removes the line limit while keeping
.frame(maxWidth: .infinity).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/release.yml:
- Around line 199-200: 現在のワークフローは常に git commit -m "chore: update ${FORMULA_NAME}
to ${VERSION}" を実行するため、差分がないとコミットが失敗して再実行時にジョブが落ちます。git commit
を実行する前にワークツリーに変更があるかをチェック(例: git diff --quiet --exit-code または git status
--porcelain を使う)し、変更がなければ commit/push をスキップして正常終了するように修正してください(対象箇所: 現在の git
commit ... と git push の実行ブロック)。
- Around line 25-27: Add a required workflow_dispatch input for the tag and
validate it in the "Resolve version" step: instead of relying on GITHUB_REF_NAME
fallback, read the provided input (e.g. inputs.tag) into TAG and then verify TAG
matches the expected semver-like prefixed format (e.g. ^v\d+\.\d+\.\d+(-.*)?$);
if the regex check fails, exit the job with a clear error so VERSION="${TAG#v}"
and BASE_VERSION="${VERSION%%-*}" are only computed for valid tags. Update
references that use TAG (MARKETING_VERSION and release URL) to rely on the
validated TAG value.

In `@perch/Features/NowPlaying/LyricsKitFetcher.swift`:
- Around line 65-68: titleIsJapanese and lyricsContainJapanese only check
hiragana/katakana and miss kanji, so Japanese-only titles/lyrics bypass the
filter; update both functions (titleIsJapanese and lyricsContainJapanese) to
also test Unicode ranges for CJK Unified Ideographs (e.g. 0x4E00–0x9FFF) and at
minimum CJK Extension A (0x3400–0x4DBF); you may also include supplementary
ranges (0x20000–0x2A6DF, 0x2A700–0x2B73F) if you need broader coverage—add those
scalar-range checks alongside the existing hiragana/katakana checks so Kanji
characters are detected as Japanese.

In `@perch/Resources/Info.plist`:
- Around line 7-16: The ATS exception for "music.163.com" is too broad and
currently allows insecure HTTP for all subdomains; update the Info.plist by
removing or narrowing the exception: either remove the entire "music.163.com"
entry under NSExceptionDomains (preferred) or at minimum remove the
NSIncludesSubdomains key or set it to false and remove or set
NSExceptionAllowsInsecureHTTPLoads to false so only explicit, minimal exceptions
remain; target the keys NSAppTransportSecurity, NSExceptionDomains, the
"music.163.com" domain entry, NSExceptionAllowsInsecureHTTPLoads and
NSIncludesSubdomains when making the change.

---

Nitpick comments:
In `@perch/Features/NowPlaying/LyricsView.swift`:
- Around line 28-29: The lyrics Text view in LyricsView.swift is hard-capped
with .lineLimit(2), causing full-display lyrics in NowPlayingCard to be
truncated; change the modifier to be conditional (e.g., .lineLimit(isCompact ? 2
: nil) or .lineLimit(isExpanded ? nil : 2)) using the existing state/prop that
distinguishes compact vs full views (pass or use the NowPlayingCard/ LyricsView
property like isCompact/isExpanded), so compact mode keeps 2 lines but full mode
removes the line limit while keeping .frame(maxWidth: .infinity).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 319b3e81-fe9f-4874-875b-c4b26d4c0e00

📥 Commits

Reviewing files that changed from the base of the PR and between 13128e6 and 7d87363.

📒 Files selected for processing (10)
  • .github/workflows/release.yml
  • THIRD_PARTY_NOTICES.md
  • perch.xcodeproj/project.pbxproj
  • perch.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
  • perch/Features/NowPlaying/LyricsKitFetcher.swift
  • perch/Features/NowPlaying/LyricsStore.swift
  • perch/Features/NowPlaying/LyricsView.swift
  • perch/Features/NowPlaying/NowPlayingCard.swift
  • perch/Features/NowPlaying/NowPlayingManager.swift
  • perch/Resources/Info.plist
✅ Files skipped from review due to trivial changes (1)
  • THIRD_PARTY_NOTICES.md

Comment thread .github/workflows/release.yml Outdated
Comment thread .github/workflows/release.yml Outdated
Comment on lines +65 to +68
private func titleIsJapanese(_ title: String) -> Bool {
title.unicodeScalars.contains {
($0.value >= 0x3040 && $0.value <= 0x309F) || ($0.value >= 0x30A0 && $0.value <= 0x30FF)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

日本語判定が漢字を見落としており、フィルタ条件が抜けます。

titleIsJapanese / lyricsContainJapanese がひらがな・カタカナのみ判定のため、漢字のみの日本語タイトル・歌詞で意図した日本語フィルタが効かなくなります。

差分案
-    private func titleIsJapanese(_ title: String) -> Bool {
-        title.unicodeScalars.contains {
-            ($0.value >= 0x3040 && $0.value <= 0x309F) || ($0.value >= 0x30A0 && $0.value <= 0x30FF)
-        }
-    }
+    private func titleIsJapanese(_ title: String) -> Bool {
+        title.unicodeScalars.contains { scalar in
+            switch scalar.value {
+            case 0x3040...0x309F, // Hiragana
+                 0x30A0...0x30FF, // Katakana
+                 0x4E00...0x9FFF, // CJK Unified Ideographs (Kanji)
+                 0x3400...0x4DBF, // CJK Extension A
+                 0xFF66...0xFF9D: // Halfwidth Katakana
+                return true
+            default:
+                return false
+            }
+        }
+    }

-    private func lyricsContainJapanese(_ lines: [LyricsLine]) -> Bool {
-        lines.contains { line in
-            line.text.unicodeScalars.contains {
-                ($0.value >= 0x3040 && $0.value <= 0x309F) || ($0.value >= 0x30A0 && $0.value <= 0x30FF)
-            }
-        }
-    }
+    private func lyricsContainJapanese(_ lines: [LyricsLine]) -> Bool {
+        lines.contains { line in
+            line.text.unicodeScalars.contains { scalar in
+                switch scalar.value {
+                case 0x3040...0x309F, 0x30A0...0x30FF, 0x4E00...0x9FFF, 0x3400...0x4DBF, 0xFF66...0xFF9D:
+                    return true
+                default:
+                    return false
+                }
+            }
+        }
+    }

Also applies to: 71-75

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@perch/Features/NowPlaying/LyricsKitFetcher.swift` around lines 65 - 68,
titleIsJapanese and lyricsContainJapanese only check hiragana/katakana and miss
kanji, so Japanese-only titles/lyrics bypass the filter; update both functions
(titleIsJapanese and lyricsContainJapanese) to also test Unicode ranges for CJK
Unified Ideographs (e.g. 0x4E00–0x9FFF) and at minimum CJK Extension A
(0x3400–0x4DBF); you may also include supplementary ranges (0x20000–0x2A6DF,
0x2A700–0x2B73F) if you need broader coverage—add those scalar-range checks
alongside the existing hiragana/katakana checks so Kanji characters are detected
as Japanese.

Comment on lines +7 to +16
<key>NSAppTransportSecurity</key>
<dict>
<key>NSExceptionDomains</key>
<dict>
<key>music.163.com</key>
<dict>
<key>NSExceptionAllowsInsecureHTTPLoads</key>
<true/>
<key>NSIncludesSubdomains</key>
<true/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

ATS例外が広すぎ、平文HTTPを広範囲で許可しています。

Line 13-16 の設定だと music.163.com 配下全体で非TLS通信が許可され、改ざん・盗聴リスクが残ります。少なくともサブドメイン許可は外し、可能なら HTTP 例外自体を撤去してください。

🔒 最小限のスコープ縮小案
 <key>music.163.com</key>
 <dict>
     <key>NSExceptionAllowsInsecureHTTPLoads</key>
     <true/>
-    <key>NSIncludesSubdomains</key>
-    <true/>
 </dict>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@perch/Resources/Info.plist` around lines 7 - 16, The ATS exception for
"music.163.com" is too broad and currently allows insecure HTTP for all
subdomains; update the Info.plist by removing or narrowing the exception: either
remove the entire "music.163.com" entry under NSExceptionDomains (preferred) or
at minimum remove the NSIncludesSubdomains key or set it to false and remove or
set NSExceptionAllowsInsecureHTTPLoads to false so only explicit, minimal
exceptions remain; target the keys NSAppTransportSecurity, NSExceptionDomains,
the "music.163.com" domain entry, NSExceptionAllowsInsecureHTTPLoads and
NSIncludesSubdomains when making the change.

tukuyomil032 and others added 11 commits June 5, 2026 13:44
- Add bundle_name output (perch-beta or perch) based on tag format
- Pass PRODUCT_NAME to all xcodebuild archive steps
- Fix DMG staging to use bundle_name for .app path
- Remove Compute SHA256 and Update Homebrew tap steps (tap handles itself)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
LyricsKit 1.9.0 requires swift-tools-version 6.2.0; macos-15 ships
with Swift 6.1.0 which fails to resolve the dependency

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- ci.yml build job: macos-latest -> macos-26 (Swift 6.2 for LyricsKit 1.9.0)
- release.yml: add version input to workflow_dispatch so manual runs work
  correctly from any branch (uses input instead of GITHUB_REF_NAME)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove --renderer github-actions from xcbeautify so Swift compile errors appear in raw step logs instead of being swallowed as annotations. Change release.yml tail -5 to tail -100 for same reason.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Build fails instantly after package resolution (< 2ms) — not a compile error. xcbeautify filters the actual error. Raw xcodebuild output needed to see what fails.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Xcode 26 CI runner (26.4.1) requires macros from FrameworkToolbox to be explicitly trusted. Add defaults write IDESkipPackagePluginFingerprintValidatation to all three workflows (ci, test, release).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace defaults write workaround with -skipPackagePluginValidation xcodebuild flag. Xcode 16+ extends this flag to also skip Swift macro trust validation, which blocks FrameworkToolbox macros on CI runner.

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

Check if macro error persists when xcbeautify is removed from ci.yml only. Verifying if -skipPackagePluginValidation is effective on Xcode 26.4.1 CI runner.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Xcode 26 added a dedicated -skipMacroValidation flag separate from -skipPackagePluginValidation. The latter only skips plugin validation; macros (FrameworkToolboxMacros etc from FrameworkToolbox via LyricsKit) need the former.

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

PRODUCT_NAME=perch-beta passed via CLI propagates to ALL SPM targets, causing 'Multiple commands produce perch-beta.bundle' from CryptoSwift/Defaults/KeyboardShortcuts resource bundles. Solution: build with default PRODUCT_NAME (perch), then rename app bundle post-archive for beta.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
.github/workflows/test.yml (1)

21-21: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

macos-26 ランナーが存在しない可能性があります。

release.yml と同じ問題です。GitHub Actions で利用可能な macOS ランナーを確認してください。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test.yml at line 21, The workflow uses an unavailable
runner identifier "runs-on: macos-26"; replace that value with a supported
GitHub Actions macOS runner (for example "macos-13" or "macos-latest") wherever
"runs-on: macos-26" appears (also check release.yml for the same issue), commit
the change, and verify the workflow runs by comparing against the official list
of available macOS runners in GitHub Actions.
.github/workflows/ci.yml (1)

21-21: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

macos-26 ランナーが存在しない可能性があります。

release.yml と同じ問題です。GitHub Actions で利用可能な macOS ランナーを確認してください。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml at line 21, The workflow uses an invalid runner
identifier "runs-on: macos-26"; update that value to a supported GitHub Actions
macOS runner (e.g., "macos-latest" or a specific supported version like
"macos-13" / "macos-12") to match the release.yml change and ensure the job can
run; locate and replace the "runs-on: macos-26" entry in the CI workflow with
the chosen valid runner.
.github/workflows/release.yml (1)

30-34: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

バージョン入力に対する検証が不足しています。

Line 31 で github.event.inputs.version をシェル変数に直接展開していますが、入力値の検証が行われていません。不正な形式や悪意のある入力(シェルメタ文字を含む文字列など)が渡された場合、予期しない動作やコマンドインジェクションのリスクがあります。

🔒 入力検証を追加する修正案
       - name: Resolve version
         id: version
         run: |
           if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
             TAG="${{ github.event.inputs.version }}"
+            # セマンティックバージョニング形式の検証
+            if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.-]+)?$ ]]; then
+              echo "::error::Invalid version format: $TAG (expected: vX.Y.Z or vX.Y.Z-prerelease)"
+              exit 1
+            fi
           else
             TAG="${GITHUB_REF_NAME}"
           fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/release.yml around lines 30 - 34, The workflow directly
expands github.event.inputs.version into TAG when github.event_name ==
"workflow_dispatch", which risks injection or invalid version formats; update
the logic around TAG assignment (the branch using TAG="${{
github.event.inputs.version }}") to validate and sanitize the input first (e.g.,
check it matches an allowed pattern such as a semantic version regex or a strict
whitelist), reject or fail the job on invalid input, and only assign the
sanitized value to TAG (or fallback to a safe default); ensure this validation
runs before any use of github.event.inputs.version and include clear error
messages when rejecting the input.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/release.yml:
- Line 19: The workflow accepts a raw workflow_dispatch input named version and
assigns it to TAG in the "Resolve version" step; add strict validation to reject
anything that doesn't match your allowed tag pattern (e.g., semantic version or
repo tag format) before using it in bash. In the "Resolve version" step,
validate github.event.inputs.version against a strict regex (reject and fail the
job if it doesn't match), and only then assign it to TAG (avoid direct use of
unvalidated input); reference the workflow_dispatch input name version and the
TAG variable to locate where to add the check.

---

Duplicate comments:
In @.github/workflows/ci.yml:
- Line 21: The workflow uses an invalid runner identifier "runs-on: macos-26";
update that value to a supported GitHub Actions macOS runner (e.g.,
"macos-latest" or a specific supported version like "macos-13" / "macos-12") to
match the release.yml change and ensure the job can run; locate and replace the
"runs-on: macos-26" entry in the CI workflow with the chosen valid runner.

In @.github/workflows/release.yml:
- Around line 30-34: The workflow directly expands github.event.inputs.version
into TAG when github.event_name == "workflow_dispatch", which risks injection or
invalid version formats; update the logic around TAG assignment (the branch
using TAG="${{ github.event.inputs.version }}") to validate and sanitize the
input first (e.g., check it matches an allowed pattern such as a semantic
version regex or a strict whitelist), reject or fail the job on invalid input,
and only assign the sanitized value to TAG (or fallback to a safe default);
ensure this validation runs before any use of github.event.inputs.version and
include clear error messages when rejecting the input.

In @.github/workflows/test.yml:
- Line 21: The workflow uses an unavailable runner identifier "runs-on:
macos-26"; replace that value with a supported GitHub Actions macOS runner (for
example "macos-13" or "macos-latest") wherever "runs-on: macos-26" appears (also
check release.yml for the same issue), commit the change, and verify the
workflow runs by comparing against the official list of available macOS runners
in GitHub Actions.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4f9690f0-5083-4ae4-9869-44decea3244c

📥 Commits

Reviewing files that changed from the base of the PR and between 7d87363 and 623e51f.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • .github/workflows/release.yml
  • .github/workflows/test.yml

jobs:
release:
runs-on: macos-15
runs-on: macos-26

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# GitHub Actions で利用可能な macOS ランナーイメージを確認
gh api /meta | jq -r '.actions_runner_images[] | select(.os == "macos") | .runner_image'

Repository: tukuyomil032/Perch

Length of output: 120


🌐 Web query:

GitHub Actions で利用可能な macOS ランナーの最新バージョンは何ですか?

💡 Result:

2026年6月5日時点で、GitHub Actionsで利用可能な最新のmacOSランナーは「macos-26」です[1][2]。 GitHub ActionsのGitHubホスト型ランナーにおけるmacOS環境は、Appleシリコン(arm64)およびIntel(x64)の両アーキテクチャで提供されており、以下のラベルで利用可能です[2]。 ・macos-26:標準のmacOSランナー(arm64)[1][2] ・macos-26-intel:標準のmacOSランナー(x64)[1][2] ・macos-26-large:Largeランナー(x64)[1][2] ・macos-26-xlarge:Extra-largeランナー(arm64)[1][2] これらのランナーイメージは、GitHubの公式リポジトリ(actions/runner-images)で継続的に更新されており、OSのバージョンはmacos-26.4が採用されています[3][4][5]。なお、古いmacOS 14ランナーイメージについては deprecation(非推奨)が開始されており、将来的にサポートが終了する予定であるため、最新のmacos-26への移行が推奨されています[3][6]。

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE=".github/workflows/release.yml"
if [ ! -f "$FILE" ]; then
  echo "Missing $FILE" >&2
  exit 1
fi
# show around the mentioned lines (line numbers in the review comment: 19 and 31)
sed -n '1,220p' "$FILE" | nl -ba | sed -n '1,220p'
echo "---- tail ----"
tail -n 80 "$FILE" | nl -ba -w2 -s': ' | head -n 120

Repository: tukuyomil032/Perch

Length of output: 105


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE=".github/workflows/release.yml"
# Print with line numbers the regions around line 19 and 31, plus nearby logic.
for r in "1,60p" "15,45p" "55,120p" "120,220p"; do
  echo "==== $r ===="
  sed -n "$r" "$FILE" | nl -ba -w3 -s': ' 
done

Repository: tukuyomil032/Perch

Length of output: 139


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE=".github/workflows/release.yml"

for r in "1,60p" "15,45p" "55,120p" "120,220p"; do
  echo "==== $r ===="
  sed -n "$r" "$FILE" | nl -ba -w3 -s': '
done

Repository: tukuyomil032/Perch

Length of output: 139


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE=".github/workflows/release.yml"
sed -n '1,120p' "$FILE" | cat -n

Repository: tukuyomil032/Perch

Length of output: 4794


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE=".github/workflows/release.yml"
echo "---- matches: inputs.version ----"
rg -n "inputs\.version|github\.event\.inputs\.version" "$FILE" || true
echo "---- matches: TAG= ----"
rg -n "TAG\s*=" "$FILE" || true
echo "---- matches: shell quoting around TAG ----"
rg -n "TAG.*\\$\\{\\{.*inputs\\.version" "$FILE" || true

Repository: tukuyomil032/Perch

Length of output: 372


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE=".github/workflows/release.yml"
echo "---- show around line ~19 (runner) ----"
sed -n '10,35p' "$FILE" | cat -n

Repository: tukuyomil032/Perch

Length of output: 838


.github/workflows/release.ymlmacos-26 は問題なし、workflow_dispatchversion 入力をサニタイズ必須

  • runs-on: macos-26 は GitHub ホストランナーとして提供されているため、ランナー不存在による失敗懸念は不要です。
  • Resolve version ステップの TAG="${{ github.event.inputs.version }}" はユーザー入力が bash 内でコマンド置換等として解釈され得るため、version を厳密な正規表現で検証して不正入力を拒否してください(例:期待するタグ形式のみ許可)。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/release.yml at line 19, The workflow accepts a raw
workflow_dispatch input named version and assigns it to TAG in the "Resolve
version" step; add strict validation to reject anything that doesn't match your
allowed tag pattern (e.g., semantic version or repo tag format) before using it
in bash. In the "Resolve version" step, validate github.event.inputs.version
against a strict regex (reject and fail the job if it doesn't match), and only
then assign it to TAG (avoid direct use of unvalidated input); reference the
workflow_dispatch input name version and the TAG variable to locate where to add
the check.

tukuyomil032 and others added 2 commits June 5, 2026 14:53
…-build broken in Xcode 26)

x86_64-only xcodebuild archive fails in Xcode 26.4.1 because FrameworkToolbox macro plugin binaries are host (arm64) only and not found at the expected x86_64 install path. Remove separate x86_64 archive step and instead copy the universal archive and use lipo -thin x86_64 to extract the x86_64 slice.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
workflow_dispatch does not create a git tag automatically. Explicitly pass tag_name so the action can create/find the tag when triggered manually.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@tukuyomil032
tukuyomil032 merged commit 48f54e8 into main Jun 5, 2026
9 checks passed
@tukuyomil032
tukuyomil032 deleted the phase-2/now-playing branch June 9, 2026 08:00
tukuyomil032 added a commit that referenced this pull request Jun 24, 2026
Revert two regressions introduced on the liquid glass morph branch:

- IslandGlassSurface.surface() ZStack alignment back to default .center; .top forced expanded VStack to be flush with the top edge and clipped compact content under the rounded shape.

- NowPlayingMorphContent compactLayer MarqueeText width back to 120pt; .infinity caused the marquee to dominate the HStack and squeeze the WaveformView out of the pill.

Restores Image #3 compact pill appearance. Expanded UI restoration follows in subsequent phases (preset-aware RootIslandView).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tukuyomil032 added a commit that referenced this pull request Jun 24, 2026
…artwork-free

main's NowPlayingMorphContent.compactLayer put a Color.clear placeholder where the artwork visually sits, then painted the real artwork on a separate ZStack layer. That kept the HStack's width math (22 + 6 + 120 + 6 + waveform) symbolic — the layout never had to actually find ~174pt of horizontal space because the artwork wasn't really inside the HStack.

NowPlayingCompact was painting the real 22pt artwork inside the HStack, so the inner views fought over real pixels and the marquee/waveform got squeezed out of a 150pt Capsule (Image #5). Wrap the HStack in a ZStack(alignment: .leading), give it a Color.clear(22) placeholder, and overlay artworkThumbnail with .padding(.leading, 8) on top. Width math matches main; visuals match Image #3.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

2 participants