From 0955cf3e2c3727ef986ce182e9120ebb1bc5e76e Mon Sep 17 00:00:00 2001 From: GeneralD Date: Fri, 12 Jun 2026 01:47:26 +0900 Subject: [PATCH 1/7] =?UTF-8?q?fix(#265):=20=E3=83=87=E3=82=A3=E3=82=B9?= =?UTF-8?q?=E3=83=97=E3=83=AC=E3=82=A4=E5=86=8D=E6=A7=8B=E6=88=90=E5=BE=8C?= =?UTF-8?q?=E3=82=82=E3=83=AC=E3=82=A4=E3=82=A2=E3=82=A6=E3=83=88=E3=82=92?= =?UTF-8?q?=E5=86=AA=E7=AD=89=E3=81=AB=E5=86=8D=E9=81=A9=E7=94=A8=E3=81=97?= =?UTF-8?q?=E3=81=A6=E5=B4=A9=E3=82=8C=E3=82=92=E8=87=AA=E5=B7=B1=E4=BF=AE?= =?UTF-8?q?=E5=BE=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit モニタの抜き差し中は window server がアプリの裏でオーバーレイウィンドウを 移動・リサイズするが、従来は resolveLayout() のモデル値が変化したときしか applyLayout が走らないため (removeDuplicates によるゲート)、モデル値が 同じままシステム側だけが動かしたズレを永遠に修復できなかった。 hostingView (autoresizing なし) と AVPlayerLayer (比例 autoresizing のみ) が実ウィンドウと食い違ったまま固定され、歌詞の重なり・壁紙の偏り・ ヘッダ崩れとして現れる。再起動で直るのはこのため。 - AppPresenter.onWindowFrameChange: removeDuplicates を撤去し、 解決レイアウトの emit ごとに毎回再適用する - AppWindow.apply: 実状態と照合する冪等な再適用に変更。windowFrame が 一致していれば setFrame をスキップ (通知ループ防止)、AVPlayerLayer は transform を保ったまま bounds + position で幾何を再アサート - ScreenInteractor.screenChanges: NSWindow.didMove / didResize 通知を merge し、システムがウィンドウを動かした直後にも再解決が走るようにする --- .claude/CLAUDE.md | 8 ++- Sources/Presenters/App/AppPresenter.swift | 11 ++-- .../ScreenInteractorImpl.swift | 14 +++- Sources/Views/Overlay/AppWindow.swift | 28 ++++++-- Tests/PresentersTests/AppPresenterTests.swift | 18 +++-- .../ScreenInteractorImplTests.swift | 27 ++++++++ Tests/ViewsTests/AppWindowTests.swift | 66 +++++++++++++++++++ 7 files changed, 154 insertions(+), 18 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index cc8efe7c..50848e84 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -191,7 +191,7 @@ graph TD ### Dependency Direction -``` +```text View → Presenter → Interactor → UseCase → Repository → DataSource → Router (wireframe only) ``` @@ -237,6 +237,7 @@ Presenters subscribe to Interactors via Combine. Interactors access UseCases via **No AppStyleKey**: `@Dependency(\.appStyle)` was removed. All config access goes through the owning Interactor's computed properties (e.g., `trackInteractor.textLayout`, `wallpaperInteractor.rippleConfig`). This enforces the VIPER dependency rule. **WallpaperDataSource\**: Generic protocol defining `resolve(_ location: LocationType) async throws -> String`. Three implementations with distinct location types: + - `LocalWallpaperDataSourceImpl: WallpaperDataSource` — relative/absolute path resolution via Files library - `RemoteWallpaperDataSourceImpl: WallpaperDataSource` — HTTP(S) download with SHA256-keyed cache - `YouTubeWallpaperDataSourceImpl: WallpaperDataSource` — yt-dlp/uvx download with H.264/AVC codec, SHA256-keyed cache @@ -260,6 +261,7 @@ Presenters subscribe to Interactors via Combine. Interactors access UseCases via **FlexibleDouble**: `Codable` wrapper that decodes both TOML Int and Double via `singleValueContainer`. Used for all numeric config fields. **MetadataDataSource\**: Generic protocol defining `resolve(track:) -> [Value]`. Three implementations with distinct value types: + - `LLMMetadataDataSourceImpl: MetadataDataSource` — AI-based title/artist extraction - `MusicBrainzMetadataDataSourceImpl: MetadataDataSource` — MusicBrainz API lookup - `RegexMetadataDataSourceImpl: MetadataDataSource` — regex-based title parsing and candidate generation @@ -267,6 +269,7 @@ Presenters subscribe to Interactors via Combine. Interactors access UseCases via Each is injected individually into `MetadataRepository` (not as an array). Repository manages cache strategy and type conversion (`MusicBrainzMetadata → Track`). **MetadataDataStore\**: Generic cache protocol with `read(title:artist:) -> Value?` and `write(title:artist:value:)`. Two parameterizations: + - `MetadataDataStore` — LLM result cache (`GRDBLLMMetadataDataStore`) - `MetadataDataStore` — MusicBrainz result cache (`GRDBMetadataDataStore`) @@ -294,6 +297,8 @@ Cache is Repository's responsibility, not DataSource's. DataSources are pure API **Vacant screen mode**: `ScreenSelector.vacant` dynamically picks the least-occupied display. `ScreenInteractorImpl` queries `ScreenProvider.visibleWindowBounds` (backed by `CGWindowListCopyWindowInfo` in `AppKitScreenProvider`), calculates per-screen window coverage by intersecting window rects with each screen's frame, and selects the screen with the lowest occupancy ratio. `AppPresenter` runs a periodic polling task (interval = `AppStyle.screenDebounce`, default 5s, injected `ContinuousClock`) that calls `recalculateLayout()`. The polling task is cancelled on `stop()`. Config: `screen = "vacant"`, `screen_debounce = 5`. +**Layout reconciliation (no dedup on apply)**: The overlay window's geometry is fully model-determined, and `AppWindow.apply` is idempotent — it reconciles window frame, hosting view, player container, and `AVPlayerLayer` (bounds + position, transform-safe) against the resolved `ScreenLayout` on every call, skipping `setFrame` when the frame already matches. `AppPresenter.onWindowFrameChange` intentionally does NOT dedupe layouts, and `ScreenInteractor.screenChanges` also fires on `NSWindow.didMove/didResize`: during display hot-plugging the window server moves/resizes the actual window behind the app's back, so a model-value comparison cannot detect the drift (#265). Do not re-add `removeDuplicates` there as an "optimization" — the idempotent apply already makes repeated signals cheap and loop-free. + **HealthCheckable**: Protocol in Domain with `serviceName` + `healthCheck()`. Implemented by `LRCLibHealthCheck`, `MusicBrainzHealthCheck`, `OpenAICompatibleHealthCheck` — standalone structs that own a `URLSession.data(for:)`-backed `requestPerformer` (no Papyrus dependency, since health checks only inspect status codes). `lyra healthcheck` validates config, API connectivity, and AI token validity. **HTTP client layer (Papyrus)**: `LRCLib`, `MusicBrainz`, and `OpenAICompatible` are declarative Papyrus protocols (`@API`, `@GET`, `@POST`, `@KeyMapping(.snakeCase)`, `@Headers`). The `@API` macro generates `API` structs (e.g., `LRCLibAPI: LRCLib`) that wrap a `Provider`. DataSources accept `any ` via init, so tests use manual stubs (`LRCLibStub`, `MusicBrainzStub`, `OpenAICompatibleStub`) for protocol-level mocking. URL construction is verified by injecting a custom `HTTPService` (`TestHTTPService`) into a `Provider` to capture outgoing `URLRequest`s. `OpenAICompatibleAPI.provider(for: AIEndpoint)` builds a Provider that attaches `Bearer ` via `modifyRequests` (per-instance auth, since `@Authorization` is static). @@ -333,6 +338,7 @@ This applies to all Combine + Timer + MainActor tests where DecodeEffect, state Version is defined in `Sources/VersionHandler/Resources/version.txt` (single source of truth). CI reads this file to auto-create/update git tags on push to main. **PR version bump rule**: When creating a PR, always include a version bump commit. Determine the level from the changes in the PR: + - `feat:` → minor bump - `fix:` / `refactor:` / `chore:` → patch bump - Breaking changes → major bump diff --git a/Sources/Presenters/App/AppPresenter.swift b/Sources/Presenters/App/AppPresenter.swift index 39bdde0e..d01deadf 100644 --- a/Sources/Presenters/App/AppPresenter.swift +++ b/Sources/Presenters/App/AppPresenter.swift @@ -48,12 +48,15 @@ public final class AppPresenter: ObservableObject { .store(in: &cancellables) } - /// Register a side-effect to run when the window frame actually changes. - /// The wireframe uses this to drive `OverlayWindow.applyLayout` without - /// owning the subscription. + /// Register a side-effect to run on every resolved layout, so the window + /// geometry is re-asserted even when the resolved frame is unchanged. + /// Deduplicating here broke recovery from display hot-plugging (#265): + /// the window server moves the actual window during reconfiguration, and a + /// model-value comparison cannot see that drift. The wireframe uses this + /// to drive `OverlayWindow.applyLayout` (idempotent) without owning the + /// subscription. public func onWindowFrameChange(_ handler: @escaping @MainActor (ScreenLayout) -> Void) { $layout - .removeDuplicates { $0.windowFrame == $1.windowFrame } .dropFirst() .receive(on: DispatchQueue.main) .sink { layout in handler(layout) } diff --git a/Sources/ScreenInteractor/ScreenInteractorImpl.swift b/Sources/ScreenInteractor/ScreenInteractorImpl.swift index 9cb62177..77b3dba7 100644 --- a/Sources/ScreenInteractor/ScreenInteractorImpl.swift +++ b/Sources/ScreenInteractor/ScreenInteractorImpl.swift @@ -20,9 +20,19 @@ extension ScreenInteractorImpl: ScreenInteractor { configService.appStyle.screenDebounce } + /// Signals that the overlay geometry may need re-resolution. Besides screen + /// parameter changes, this includes the window being moved or resized — + /// during display hot-plugging the window server relocates windows behind + /// the app's back (#265), and the overlay (the process's only window) must + /// snap back to its resolved layout. The apply side is idempotent, so the + /// move/resize triggered by re-asserting the layout does not loop. public var screenChanges: AnyPublisher { - NotificationCenter.default - .publisher(for: NSApplication.didChangeScreenParametersNotification) + let center = NotificationCenter.default + return center.publisher(for: NSApplication.didChangeScreenParametersNotification) + .merge( + with: center.publisher(for: NSWindow.didMoveNotification), + center.publisher(for: NSWindow.didResizeNotification) + ) .map { _ in () } .eraseToAnyPublisher() } diff --git a/Sources/Views/Overlay/AppWindow.swift b/Sources/Views/Overlay/AppWindow.swift index 90c785ca..dd55d3f3 100644 --- a/Sources/Views/Overlay/AppWindow.swift +++ b/Sources/Views/Overlay/AppWindow.swift @@ -103,12 +103,32 @@ extension AppWindow { surface.orderFront(nil) } + /// Re-asserts the full window/view/layer geometry from the resolved layout. + /// Idempotent by design: the window server mutates the actual window during + /// display reconfiguration (#265), so every call reconciles against actual + /// state instead of trusting that earlier applications are still in effect. + /// Skipping `setFrame` when the frame already matches keeps the + /// `NSWindow.didMove/didResize → screenChanges → apply` cycle from looping. static func apply(layout: ScreenLayout, to surface: OverlayWindowSurface, hostingView: NSView) { - surface.setFrame(layout.windowFrame, display: false) + applyWindowFrame(layout.windowFrame, to: surface) hostingView.frame = layout.hostingFrame - if let containerView = surface.contentView, containerView !== hostingView { - containerView.frame = CGRect(origin: .zero, size: layout.windowFrame.size) - } + guard let containerView = surface.contentView, containerView !== hostingView else { return } + containerView.frame = CGRect(origin: .zero, size: layout.windowFrame.size) + guard let playerLayer = playerLayer(in: surface) else { return } + reassertGeometry(of: playerLayer, in: containerView.bounds) + } + + private static func applyWindowFrame(_ frame: NSRect, to surface: OverlayWindowSurface) { + guard surface.frame != frame else { return } + surface.setFrame(frame, display: false) + } + + /// Sets `bounds` + `position` rather than `frame`: the layer carries the + /// wallpaper-scale affine transform, and `frame` is undefined under a + /// non-identity transform. + private static func reassertGeometry(of playerLayer: AVPlayerLayer, in bounds: CGRect) { + playerLayer.bounds = bounds + playerLayer.position = CGPoint(x: bounds.midX, y: bounds.midY) } static func attachPlayer( diff --git a/Tests/PresentersTests/AppPresenterTests.swift b/Tests/PresentersTests/AppPresenterTests.swift index 4304276a..2f7510cd 100644 --- a/Tests/PresentersTests/AppPresenterTests.swift +++ b/Tests/PresentersTests/AppPresenterTests.swift @@ -244,8 +244,8 @@ struct AppPresenterTests { } @MainActor - @Test("onWindowFrameChange fires only when windowFrame actually changes") - func onWindowFrameChangeDedups() async { + @Test("onWindowFrameChange fires on every screen-change signal, even when the resolved frame is unchanged (regression: #265)") + func onWindowFrameChangeReassertsUnchangedFrame() async { let first = ScreenLayout(windowFrame: CGRect(x: 0, y: 0, width: 1920, height: 1080)) let sameFrame = ScreenLayout( windowFrame: CGRect(x: 0, y: 0, width: 1920, height: 1080), @@ -274,20 +274,24 @@ struct AppPresenterTests { // Subscribe after start so the current layout is dropped. presenter.onWindowFrameChange { _ in counter.count += 1 } - // Same windowFrame (different hostingFrame) → deduped. + // Same windowFrame (different hostingFrame) → still fires so the window + // can heal from system-side moves during display reconfiguration. interactor.layoutToReturn = sameFrame interactor.changes.send(()) + // Identical layout → still fires for the same reason. + interactor.changes.send(()) + // Different windowFrame → fires. interactor.layoutToReturn = different interactor.changes.send(()) - // Give the DispatchQueue.main scheduling a tick to flush. - let deadline = ContinuousClock.now + .seconds(1) - while counter.count < 1, ContinuousClock.now < deadline { + // Give the DispatchQueue.main scheduling time to flush (CI load varies). + let deadline = ContinuousClock.now + .seconds(3) + while counter.count < 3, ContinuousClock.now < deadline { try? await Task.sleep(for: .milliseconds(10)) } - #expect(counter.count == 1) + #expect(counter.count == 3) } @MainActor diff --git a/Tests/ScreenInteractorTests/ScreenInteractorImplTests.swift b/Tests/ScreenInteractorTests/ScreenInteractorImplTests.swift index e0db1abd..3f402e30 100644 --- a/Tests/ScreenInteractorTests/ScreenInteractorImplTests.swift +++ b/Tests/ScreenInteractorTests/ScreenInteractorImplTests.swift @@ -285,5 +285,32 @@ struct ScreenInteractorImplTests { #expect(counter.count >= 1) cancellable.cancel() } + + @Test( + "emits when the window is moved or resized by the system (regression: #265)", + arguments: [NSWindow.didMoveNotification, NSWindow.didResizeNotification] + ) + func emitsOnWindowNotification(name: Notification.Name) async { + let interactor = withDependencies { + $0.configUseCase = StubConfigUseCase() + $0.screenProvider = StubScreenProvider() + } operation: { + ScreenInteractorImpl() + } + + final class Counter: @unchecked Sendable { var count = 0 } + let counter = Counter() + let cancellable = interactor.screenChanges.sink { counter.count += 1 } + + NotificationCenter.default.post(name: name, object: nil) + + let deadline = ContinuousClock.now + .seconds(1) + while counter.count < 1, ContinuousClock.now < deadline { + try? await Task.sleep(for: .milliseconds(10)) + } + + #expect(counter.count >= 1) + cancellable.cancel() + } } } diff --git a/Tests/ViewsTests/AppWindowTests.swift b/Tests/ViewsTests/AppWindowTests.swift index 95239793..caf5c1db 100644 --- a/Tests/ViewsTests/AppWindowTests.swift +++ b/Tests/ViewsTests/AppWindowTests.swift @@ -135,6 +135,72 @@ struct AppWindowTests { #expect(playerLayer?.affineTransform().d == 1.25) } + @Test("apply skips window setFrame when the surface frame already matches (loop safety)") + func applySkipsMatchingWindowFrame() { + let layout = ScreenLayout( + windowFrame: CGRect(x: 0, y: 0, width: 1024, height: 640), + hostingFrame: CGRect(x: 0, y: 25, width: 1024, height: 615), + screenOrigin: CGPoint(x: 0, y: 25) + ) + let hostingView = NSView() + let surface = SpyOverlayWindowSurface(frame: layout.windowFrame) + surface.contentView = hostingView + + AppWindow.apply(layout: layout, to: surface, hostingView: hostingView) + + #expect(surface.setFrameCalls.isEmpty) + #expect(hostingView.frame == layout.hostingFrame) + } + + @Test("apply reasserts geometry after a system-side mutation, even when the resolved frame is unchanged (regression: #265)") + func applyHealsSystemMutatedGeometry() { + let layout = ScreenLayout( + windowFrame: CGRect(x: 0, y: 0, width: 1512, height: 982), + hostingFrame: CGRect(x: 0, y: 38, width: 1512, height: 944), + screenOrigin: CGPoint(x: 0, y: 38) + ) + let hostingView = NSView() + let surface = SpyOverlayWindowSurface(frame: layout.windowFrame) + surface.contentView = hostingView + AppWindow.attachPlayer(AVPlayer(), to: surface, hostingView: hostingView) + let containerView = surface.contentView + let playerLayer = containerView?.layer?.sublayers?.compactMap { $0 as? AVPlayerLayer }.first + + // Simulate the window server mangling the view/layer tree during a + // display reconfiguration while the window frame stayed the same. + containerView?.frame = CGRect(x: 330, y: 67, width: 1182, height: 915) + hostingView.frame = CGRect(x: 200, y: -100, width: 800, height: 500) + playerLayer?.bounds = CGRect(x: 0, y: 0, width: 100, height: 100) + playerLayer?.position = CGPoint(x: 900, y: 800) + + AppWindow.apply(layout: layout, to: surface, hostingView: hostingView) + + #expect(containerView?.frame == CGRect(origin: .zero, size: layout.windowFrame.size)) + #expect(hostingView.frame == layout.hostingFrame) + #expect(playerLayer?.bounds == CGRect(origin: .zero, size: layout.windowFrame.size)) + #expect(playerLayer?.position == CGPoint(x: layout.windowFrame.width / 2, y: layout.windowFrame.height / 2)) + } + + @Test("apply keeps the wallpaper scale transform while reasserting player layer geometry") + func applyPreservesWallpaperScale() { + let layout = ScreenLayout( + windowFrame: CGRect(x: 0, y: 0, width: 800, height: 500), + hostingFrame: CGRect(x: 0, y: 0, width: 800, height: 500), + screenOrigin: .zero + ) + let hostingView = NSView() + let surface = SpyOverlayWindowSurface(frame: layout.windowFrame) + surface.contentView = hostingView + AppWindow.attachPlayer(AVPlayer(), to: surface, hostingView: hostingView, scale: 1.25) + let playerLayer = surface.contentView?.layer?.sublayers?.compactMap { $0 as? AVPlayerLayer }.first + + AppWindow.apply(layout: layout, to: surface, hostingView: hostingView) + + #expect(playerLayer?.affineTransform().a == 1.25) + #expect(playerLayer?.affineTransform().d == 1.25) + #expect(playerLayer?.bounds == CGRect(origin: .zero, size: layout.windowFrame.size)) + } + @Test("applyWallpaperScale updates existing player layer and clamps shrink values") func applyWallpaperScaleUpdatesLayer() { let hostingView = NSView() From c658c9ffc656cc7bb3b585673782809dc962d7a7 Mon Sep 17 00:00:00 2001 From: GeneralD Date: Fri, 12 Jun 2026 01:47:39 +0900 Subject: [PATCH 2/7] =?UTF-8?q?fix(#265):=20=E5=90=8C=E4=B8=80=E3=83=88?= =?UTF-8?q?=E3=83=A9=E3=83=83=E3=82=AF=E3=81=AE=20artwork=20=E3=81=AA?= =?UTF-8?q?=E3=81=97=E3=82=A4=E3=83=99=E3=83=B3=E3=83=88=E3=81=A7=E8=A1=A8?= =?UTF-8?q?=E7=A4=BA=E4=B8=AD=E3=82=A2=E3=83=BC=E3=83=88=E3=83=AF=E3=83=BC?= =?UTF-8?q?=E3=82=AF=E3=82=92=E6=B6=88=E3=81=95=E3=81=AA=E3=81=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MediaRemote の再ブロードキャスト (ディスプレイ再構成時などに発生) は artwork バイトを含まないことがあり、artwork ストリームが artworkData を 素通ししていたため表示中のアートワークが nil で上書きされて消えていた。 scan で直前のトラックと比較し、同一トラック (sameTrack 判定) の間は 最後に得たアートワークを保持する。トラックが変わって新トラックに アートワークが無い場合は従来どおり nil を流してクリアする。 --- .../TrackInteractor/TrackInteractorImpl.swift | 11 ++++- .../TrackInteractorArtworkTests.swift | 40 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/Sources/TrackInteractor/TrackInteractorImpl.swift b/Sources/TrackInteractor/TrackInteractorImpl.swift index 39e4a7a6..0adce84e 100644 --- a/Sources/TrackInteractor/TrackInteractorImpl.swift +++ b/Sources/TrackInteractor/TrackInteractorImpl.swift @@ -42,9 +42,18 @@ public final class TrackInteractorImpl: @unchecked Sendable { .share() .eraseToAnyPublisher() + /// Keeps the last known artwork while the track is unchanged: system + /// re-broadcasts (e.g. during display reconfiguration) often omit the + /// artwork bytes, and a raw `map(\.artworkData)` would blank the artwork + /// until the next event that happens to carry them (#265). A track change + /// without artwork still clears it. public lazy var artwork: AnyPublisher = activeNowPlaying - .map(\.artworkData) + .scan((track: NowPlaying?.none, data: Data?.none)) { state, incoming in + let isSameTrack = state.track.map { Self.sameTrack($0, incoming) } ?? false + return (track: incoming, data: incoming.artworkData ?? (isSameTrack ? state.data : nil)) + } + .map(\.data) .removeDuplicates() .eraseToAnyPublisher() diff --git a/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift b/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift index f90a8cdd..43496264 100644 --- a/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift +++ b/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift @@ -147,6 +147,46 @@ struct TrackInteractorArtworkTests { #expect(collector.snapshot == [artA, artB]) } + @Test("artwork keeps the last known artwork when a same-track event carries none (regression: #265)") + func artworkSticksThroughSameTrackNilEvent() async { + let playback = StubPlaybackUseCase() + let interactor = makeInteractor(playback: playback) + let collector = ArtworkCollector() + let cancellable = interactor.artwork.sink { collector.append($0) } + defer { cancellable.cancel() } + + let realArt = Data([0xFF, 0xD8, 0xFF]) + + playback.subject.send(nowPlaying(title: "Song", artist: "Artist", artwork: realArt)) + await collector.waitForCount(1) + #expect(collector.snapshot == [realArt]) + + // System re-broadcasts (e.g. during display reconfiguration) often omit + // the artwork bytes — the shown artwork must not be cleared. + playback.subject.send(nowPlaying(title: "Song", artist: "Artist", artwork: nil)) + try? await Task.sleep(for: .milliseconds(100)) + #expect(collector.snapshot == [realArt]) + } + + @Test("artwork clears when the track changes and the new track has none") + func artworkClearsOnTrackChangeWithoutArtwork() async { + let playback = StubPlaybackUseCase() + let interactor = makeInteractor(playback: playback) + let collector = ArtworkCollector() + let cancellable = interactor.artwork.sink { collector.append($0) } + defer { cancellable.cancel() } + + let realArt = Data([0xFF, 0xD8, 0xFF]) + + playback.subject.send(nowPlaying(title: "Song A", artist: "Artist", artwork: realArt)) + await collector.waitForCount(1) + + playback.subject.send(nowPlaying(title: "Song B", artist: "Artist", artwork: nil)) + await collector.waitForCount(2) + + #expect(collector.snapshot == [realArt, nil]) + } + @Test("sameTrack treats nil and empty-string artist as the same value") func sameTrackNormalizesEmptyArtist() { let withArtist = nowPlaying(title: "Song", artist: "Artist", artwork: nil) From 3caf4868c35ebdde57cb6bbde86923fbe93fa890 Mon Sep 17 00:00:00 2001 From: GeneralD Date: Fri, 12 Jun 2026 01:48:00 +0900 Subject: [PATCH 3/7] chore: bump version to 2.13.8 --- Sources/VersionHandler/Resources/version.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/VersionHandler/Resources/version.txt b/Sources/VersionHandler/Resources/version.txt index ea55a03f..c38dd67f 100644 --- a/Sources/VersionHandler/Resources/version.txt +++ b/Sources/VersionHandler/Resources/version.txt @@ -1 +1 @@ -2.13.7 +2.13.8 From ca242c25fa916737cf6aa4789f64009b7cc1336d Mon Sep 17 00:00:00 2001 From: GeneralD Date: Fri, 12 Jun 2026 02:01:38 +0900 Subject: [PATCH 4/7] =?UTF-8?q?test(#265):=20artwork=20=E7=B6=AD=E6=8C=81?= =?UTF-8?q?=E3=83=86=E3=82=B9=E3=83=88=E3=81=AE=E5=9B=BA=E5=AE=9A=20sleep?= =?UTF-8?q?=20=E3=82=92=E6=B1=BA=E5=AE=9A=E7=9A=84=E3=81=AA=E3=83=95?= =?UTF-8?q?=E3=82=A7=E3=83=B3=E3=82=B9=E6=96=B9=E5=BC=8F=E3=81=AB=E7=BD=AE?= =?UTF-8?q?=E6=8F=9B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit 指摘 (no fixed sleep guideline) への対応。同一トラックの nil イベント後に必ず emit される次トラックのイベントをフェンスとし、 イベントが順序どおり処理される性質を利用して「nil が emit されなかった」 ことを決定的に検証する。 --- .../TrackInteractorArtworkTests.swift | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift b/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift index 43496264..ebfc4906 100644 --- a/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift +++ b/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift @@ -156,6 +156,7 @@ struct TrackInteractorArtworkTests { defer { cancellable.cancel() } let realArt = Data([0xFF, 0xD8, 0xFF]) + let nextArt = Data([0x01]) playback.subject.send(nowPlaying(title: "Song", artist: "Artist", artwork: realArt)) await collector.waitForCount(1) @@ -164,8 +165,12 @@ struct TrackInteractorArtworkTests { // System re-broadcasts (e.g. during display reconfiguration) often omit // the artwork bytes — the shown artwork must not be cleared. playback.subject.send(nowPlaying(title: "Song", artist: "Artist", artwork: nil)) - try? await Task.sleep(for: .milliseconds(100)) - #expect(collector.snapshot == [realArt]) + + // Deterministic fence: events are processed in order, so once the next + // track's artwork arrives, the nil event has provably emitted nothing. + playback.subject.send(nowPlaying(title: "Next Song", artist: "Artist", artwork: nextArt)) + await collector.waitForCount(2) + #expect(collector.snapshot == [realArt, nextArt]) } @Test("artwork clears when the track changes and the new track has none") From 2634afb707fb358aeac02c71b5a074aa4bd6d52f Mon Sep 17 00:00:00 2001 From: GeneralD Date: Fri, 12 Jun 2026 03:36:43 +0900 Subject: [PATCH 5/7] =?UTF-8?q?docs(#265):=20=E4=BF=AE=E6=AD=A3=E5=AF=BE?= =?UTF-8?q?=E8=B1=A1=E3=83=95=E3=82=A1=E3=82=A4=E3=83=AB=E3=81=AE=20docstr?= =?UTF-8?q?ing=20=E3=82=92=E8=BF=BD=E5=8A=A0=E3=81=97=E3=82=AB=E3=83=90?= =?UTF-8?q?=E3=83=AC=E3=83=83=E3=82=B8=E8=AD=A6=E5=91=8A=E3=81=AB=E5=AF=BE?= =?UTF-8?q?=E5=BF=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit の Docstring Coverage 警告に対応するため、今回の PR で変更のあった主要クラス・メンバにドキュメントコメントを追加しました。 --- Sources/Presenters/App/AppPresenter.swift | 7 ++++++ .../ScreenInteractorImpl.swift | 9 +++++++ .../TrackInteractor/TrackInteractorImpl.swift | 6 +++++ Sources/Views/Overlay/AppWindow.swift | 24 +++++++++++++++++++ 4 files changed, 46 insertions(+) diff --git a/Sources/Presenters/App/AppPresenter.swift b/Sources/Presenters/App/AppPresenter.swift index d01deadf..4b6dbc29 100644 --- a/Sources/Presenters/App/AppPresenter.swift +++ b/Sources/Presenters/App/AppPresenter.swift @@ -4,8 +4,13 @@ import Dependencies import Domain import Foundation +/// The top-level presenter managing the overlay application state and layout lifecycle. +/// +/// It coordinates between the `ScreenInteractor` (which resolves geometry) and the +/// various feature presenters (Header, Lyrics, Wallpaper, Ripple). @MainActor public final class AppPresenter: ObservableObject { + /// The current resolved screen layout. @Published public private(set) var layout: ScreenLayout = .init() @Dependency(\.screenInteractor) private var screenInteractor @@ -17,6 +22,7 @@ public final class AppPresenter: ObservableObject { public init() {} + /// Starts observing screen changes and periodic polling for layout reconciliation. public func start() { let interactor = screenInteractor layout = interactor.resolveLayout() @@ -29,6 +35,7 @@ public final class AppPresenter: ObservableObject { startVacantPollingIfNeeded() } + /// Stops all background tasks and subscriptions. public func stop() { vacantTask?.cancel() vacantTask = nil diff --git a/Sources/ScreenInteractor/ScreenInteractorImpl.swift b/Sources/ScreenInteractor/ScreenInteractorImpl.swift index 77b3dba7..2b5e2610 100644 --- a/Sources/ScreenInteractor/ScreenInteractorImpl.swift +++ b/Sources/ScreenInteractor/ScreenInteractorImpl.swift @@ -4,6 +4,10 @@ import CoreGraphics import Dependencies import Domain +/// Implementation of the `ScreenInteractor` that resolves display geometry. +/// +/// It supports various screen selection strategies (main, primary, index, smallest, largest, vacant) +/// and emits signals when screen parameters change or the window is moved/resized. public struct ScreenInteractorImpl { @Dependency(\.configUseCase) private var configService @Dependency(\.screenProvider) private var screenProvider @@ -12,10 +16,12 @@ public struct ScreenInteractorImpl { } extension ScreenInteractorImpl: ScreenInteractor { + /// The current screen selection strategy from configuration. public var screenSelector: ScreenSelector { configService.appStyle.screen } + /// The debounce interval for screen reconciliation polling. public var screenDebounce: Double { configService.appStyle.screenDebounce } @@ -37,6 +43,9 @@ extension ScreenInteractorImpl: ScreenInteractor { .eraseToAnyPublisher() } + /// Resolves the current screen layout based on the active selection strategy. + /// + /// - Returns: A `ScreenLayout` containing the window frame and hosting view geometry. public func resolveLayout() -> ScreenLayout { guard let screen = resolveScreen() else { return .init() } diff --git a/Sources/TrackInteractor/TrackInteractorImpl.swift b/Sources/TrackInteractor/TrackInteractorImpl.swift index 0adce84e..eb4c55a7 100644 --- a/Sources/TrackInteractor/TrackInteractorImpl.swift +++ b/Sources/TrackInteractor/TrackInteractorImpl.swift @@ -4,6 +4,10 @@ import Domain import Foundation import os +/// Implementation of the `TrackInteractor` that manages the reactive track and artwork pipeline. +/// +/// It observes now-playing information, resolves metadata and lyrics, and handles +/// artwork state preservation across track updates. public final class TrackInteractorImpl: @unchecked Sendable { private let playbackService: any PlaybackUseCase private let lyricsService: any LyricsUseCase @@ -31,6 +35,7 @@ public final class TrackInteractorImpl: @unchecked Sendable { } .share() + /// A publisher that emits `TrackUpdate` events whenever the active track changes. public lazy var trackChange: AnyPublisher = activeNowPlaying .removeDuplicates(by: Self.sameTrack) @@ -69,6 +74,7 @@ public final class TrackInteractorImpl: @unchecked Sendable { return lhs.title == rhs.title && lhsArtist == rhsArtist } + /// A publisher that emits the current playback position. public lazy var playbackPosition: AnyPublisher = activeNowPlaying .map { np in diff --git a/Sources/Views/Overlay/AppWindow.swift b/Sources/Views/Overlay/AppWindow.swift index dd55d3f3..c3b5d533 100644 --- a/Sources/Views/Overlay/AppWindow.swift +++ b/Sources/Views/Overlay/AppWindow.swift @@ -17,6 +17,11 @@ protocol OverlayWindowSurface: AnyObject { func orderFront(_ sender: Any?) } +/// The main overlay window for the application. +/// +/// It is a borderless `NSWindow` that covers the entire screen (or a specific area) +/// and hosts the SwiftUI overlay content. It handles geometry reconciliation, +/// wallpaper scaling, and AVPlayerLayer attachment. @MainActor public final class AppWindow: NSWindow { static var overlayLevel: NSWindow.Level { @@ -33,6 +38,14 @@ public final class AppWindow: NSWindow { private let hostingView: NSHostingView + /// Initializes the app window with the specified layout and presenters. + /// + /// - Parameters: + /// - initialLayout: The initial geometry for the window and hosting view. + /// - headerPresenter: Presenter for the header view. + /// - lyricsPresenter: Presenter for the lyrics view. + /// - ripplePresenter: Presenter for the ripple effect. + /// - wallpaperPresenter: Presenter for the wallpaper view. public init( initialLayout: ScreenLayout, headerPresenter: HeaderPresenter, @@ -60,22 +73,33 @@ public final class AppWindow: NSWindow { Self.applyOverlayStyle(to: self, hostingView: hostingView) } + /// Brings the window to the front. public func show() { Self.present(self) } + /// Updates the window and hosting view geometry. + /// + /// - Parameter layout: The new layout to apply. public func applyLayout(_ layout: ScreenLayout) { Self.apply(layout: layout, to: self, hostingView: hostingView) } + /// Attaches an `AVPlayer` layer to the window for wallpaper playback. + /// + /// - Parameter player: The player to attach. public func attachPlayerLayer(for player: AVPlayer) { Self.attachPlayer(player, to: self, hostingView: hostingView) } + /// Applies an affine transform scale to the wallpaper player layer. + /// + /// - Parameter scale: The scale factor to apply. public func applyWallpaperScale(_ scale: Double) { Self.applyWallpaperScale(scale, to: self) } + /// Hides the window and calls the superclass `close`. public override func close() { orderOut(nil) super.close() From 740cdae140769c1e0d0ce72613091cbb01dcde44 Mon Sep 17 00:00:00 2001 From: GeneralD Date: Fri, 12 Jun 2026 03:50:11 +0900 Subject: [PATCH 6/7] =?UTF-8?q?fix(#265):=20title=20=E4=B8=8D=E6=98=8E?= =?UTF-8?q?=E3=83=88=E3=83=A9=E3=83=83=E3=82=AF=E9=96=93=E3=81=A7=20artwor?= =?UTF-8?q?k=20=E3=82=92=E5=BC=95=E3=81=8D=E7=B6=99=E3=81=8C=E3=81=AA?= =?UTF-8?q?=E3=81=84=E3=82=88=E3=81=86=E3=82=AC=E3=83=BC=E3=83=89=E3=82=92?= =?UTF-8?q?=E8=BF=BD=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit レビュー指摘対応。title が両方 nil の場合 sameTrack は true を返すため、 未知トラック同士で前の artwork が残り続ける可能性があった。artwork の 同一トラック判定に title 非 nil 条件を追加し、回帰テストを追加。 --- .../TrackInteractor/TrackInteractorImpl.swift | 2 +- .../TrackInteractorArtworkTests.swift | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Sources/TrackInteractor/TrackInteractorImpl.swift b/Sources/TrackInteractor/TrackInteractorImpl.swift index eb4c55a7..26cc7a14 100644 --- a/Sources/TrackInteractor/TrackInteractorImpl.swift +++ b/Sources/TrackInteractor/TrackInteractorImpl.swift @@ -55,7 +55,7 @@ public final class TrackInteractorImpl: @unchecked Sendable { public lazy var artwork: AnyPublisher = activeNowPlaying .scan((track: NowPlaying?.none, data: Data?.none)) { state, incoming in - let isSameTrack = state.track.map { Self.sameTrack($0, incoming) } ?? false + let isSameTrack = state.track.map { $0.title != nil && Self.sameTrack($0, incoming) } ?? false return (track: incoming, data: incoming.artworkData ?? (isSameTrack ? state.data : nil)) } .map(\.data) diff --git a/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift b/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift index ebfc4906..8c4bd799 100644 --- a/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift +++ b/Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift @@ -173,6 +173,27 @@ struct TrackInteractorArtworkTests { #expect(collector.snapshot == [realArt, nextArt]) } + @Test("artwork does not stick across unknown (nil-title) tracks") + func artworkDoesNotStickAcrossNilTitleTracks() async { + let playback = StubPlaybackUseCase() + let interactor = makeInteractor(playback: playback) + let collector = ArtworkCollector() + let cancellable = interactor.artwork.sink { collector.append($0) } + defer { cancellable.cancel() } + + let realArt = Data([0xFF, 0xD8, 0xFF]) + + playback.subject.send(nowPlaying(title: nil, artist: nil, artwork: realArt)) + await collector.waitForCount(1) + #expect(collector.snapshot == [realArt]) + + // Unknown tracks cannot be proven identical — a nil-title event without + // artwork must clear instead of inheriting the previous artwork. + playback.subject.send(nowPlaying(title: nil, artist: nil, artwork: nil)) + await collector.waitForCount(2) + #expect(collector.snapshot == [realArt, nil]) + } + @Test("artwork clears when the track changes and the new track has none") func artworkClearsOnTrackChangeWithoutArtwork() async { let playback = StubPlaybackUseCase() From 188edab4c55c49d4921cbe8905e988e8284faaa0 Mon Sep 17 00:00:00 2001 From: GeneralD Date: Fri, 12 Jun 2026 03:50:11 +0900 Subject: [PATCH 7/7] =?UTF-8?q?refactor(#265):=20container/hosting=20view?= =?UTF-8?q?=20=E3=81=AE=20frame=20=E5=86=8D=E4=BB=A3=E5=85=A5=E3=82=92?= =?UTF-8?q?=E4=B8=80=E8=87=B4=E6=99=82=E3=81=AB=E3=82=B9=E3=82=AD=E3=83=83?= =?UTF-8?q?=E3=83=97?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit レビュー指摘対応。applyWindowFrame と同じ guard ヘルパー形式で 不要な AppKit レイアウトパスを削減。 --- Sources/Views/Overlay/AppWindow.swift | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Sources/Views/Overlay/AppWindow.swift b/Sources/Views/Overlay/AppWindow.swift index c3b5d533..1aa13022 100644 --- a/Sources/Views/Overlay/AppWindow.swift +++ b/Sources/Views/Overlay/AppWindow.swift @@ -135,9 +135,9 @@ extension AppWindow { /// `NSWindow.didMove/didResize → screenChanges → apply` cycle from looping. static func apply(layout: ScreenLayout, to surface: OverlayWindowSurface, hostingView: NSView) { applyWindowFrame(layout.windowFrame, to: surface) - hostingView.frame = layout.hostingFrame + applyFrame(layout.hostingFrame, to: hostingView) guard let containerView = surface.contentView, containerView !== hostingView else { return } - containerView.frame = CGRect(origin: .zero, size: layout.windowFrame.size) + applyFrame(contentFrame(for: layout.windowFrame), to: containerView) guard let playerLayer = playerLayer(in: surface) else { return } reassertGeometry(of: playerLayer, in: containerView.bounds) } @@ -147,6 +147,11 @@ extension AppWindow { surface.setFrame(frame, display: false) } + private static func applyFrame(_ frame: CGRect, to view: NSView) { + guard view.frame != frame else { return } + view.frame = frame + } + /// Sets `bounds` + `position` rather than `frame`: the layer carries the /// wallpaper-scale affine transform, and `frame` is undefined under a /// non-identity transform.