Skip to content
Merged
8 changes: 7 additions & 1 deletion .claude/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ graph TD

### Dependency Direction

```
```text
View → Presenter → Interactor → UseCase → Repository → DataSource
→ Router (wireframe only)
```
Expand Down Expand Up @@ -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\<LocationType\>**: Generic protocol defining `resolve(_ location: LocationType) async throws -> String`. Three implementations with distinct location types:

- `LocalWallpaperDataSourceImpl: WallpaperDataSource<LocalWallpaper>` — relative/absolute path resolution via Files library
- `RemoteWallpaperDataSourceImpl: WallpaperDataSource<RemoteWallpaper>` — HTTP(S) download with SHA256-keyed cache
- `YouTubeWallpaperDataSourceImpl: WallpaperDataSource<YouTubeWallpaper>` — yt-dlp/uvx download with H.264/AVC codec, SHA256-keyed cache
Expand All @@ -260,13 +261,15 @@ 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\<Value\>**: Generic protocol defining `resolve(track:) -> [Value]`. Three implementations with distinct value types:

- `LLMMetadataDataSourceImpl: MetadataDataSource<Track>` — AI-based title/artist extraction
- `MusicBrainzMetadataDataSourceImpl: MetadataDataSource<MusicBrainzMetadata>` — MusicBrainz API lookup
- `RegexMetadataDataSourceImpl: MetadataDataSource<Track>` — regex-based title parsing and candidate generation

Each is injected individually into `MetadataRepository` (not as an array). Repository manages cache strategy and type conversion (`MusicBrainzMetadata → Track`).

**MetadataDataStore\<Value\>**: Generic cache protocol with `read(title:artist:) -> Value?` and `write(title:artist:value:)`. Two parameterizations:

- `MetadataDataStore<Track>` — LLM result cache (`GRDBLLMMetadataDataStore`)
- `MetadataDataStore<MusicBrainzMetadata>` — MusicBrainz result cache (`GRDBMetadataDataStore`)

Expand Down Expand Up @@ -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 `<Protocol>API` structs (e.g., `LRCLibAPI: LRCLib`) that wrap a `Provider`. DataSources accept `any <Protocol>` 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 <apiKey>` via `modifyRequests` (per-instance auth, since `@Authorization` is static).
Expand Down Expand Up @@ -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
18 changes: 14 additions & 4 deletions Sources/Presenters/App/AppPresenter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -29,6 +35,7 @@ public final class AppPresenter: ObservableObject {
startVacantPollingIfNeeded()
}

/// Stops all background tasks and subscriptions.
public func stop() {
vacantTask?.cancel()
vacantTask = nil
Expand All @@ -48,12 +55,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()

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

severity

.removeDuplicates() を削除したことで意図通り「自己修復」が可能になりましたが、高頻度な通知が発生した場合に備え、必要であれば applyLayout の直前で throttledebounce を入れることも検討の余地があります。現状の apply が十分に軽量であればこのままで問題ありません。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

action

現状維持とします。apply は全ジオメトリ書き込みが一致時スキップの冪等実装になっており(188edab で view frame もスキップ対象に追加)、無変化シグナルの実コストはほぼ比較のみです。また didMove/didResize が高頻度になるのはディスプレイ再構成中の短いバーストに限られ、throttle/debounce を挟むとまさに #265 の自己修復が遅延する側に働くため、入れない判断としました。

.receive(on: DispatchQueue.main)
.sink { layout in handler(layout) }
Expand Down
23 changes: 21 additions & 2 deletions Sources/ScreenInteractor/ScreenInteractorImpl.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -12,21 +16,36 @@ 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
}

/// 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<Void, Never> {
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()
}

/// 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() }

Expand Down
17 changes: 16 additions & 1 deletion Sources/TrackInteractor/TrackInteractorImpl.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<TrackUpdate, Never> =
activeNowPlaying
.removeDuplicates(by: Self.sameTrack)
Expand All @@ -42,9 +47,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<Data?, Never> =
activeNowPlaying
.map(\.artworkData)
.scan((track: NowPlaying?.none, data: Data?.none)) { state, incoming in

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

severity

同一トラック判定に Self.sameTrack を使用していますが、NowPlayingtitleartist が共に nil の場合(未知のトラック同士)も true を返します。通常、再生中であればタイトルは存在しますが、より安全にするならタイトルが非 nil であることも条件に含めることを検討してください。

Suggested change
.scan((track: NowPlaying?.none, data: Data?.none)) { state, incoming in
let isSameTrack = state.track.map { $0.title != nil && Self.sameTrack($0, incoming) } ?? false

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

action

修正しました。ご提案どおり artwork の同一トラック判定に title 非 nil 条件を追加し、未知トラック同士で artwork が引き継がれないことを検証する回帰テスト (artworkDoesNotStickAcrossNilTitleTracks) も追加しています。740cdae を参照。

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)
.removeDuplicates()
.eraseToAnyPublisher()

Expand All @@ -60,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<PlaybackPosition, Never> =
activeNowPlaying
.map { np in
Expand Down
2 changes: 1 addition & 1 deletion Sources/VersionHandler/Resources/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.13.7
2.13.8
59 changes: 54 additions & 5 deletions Sources/Views/Overlay/AppWindow.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -33,6 +38,14 @@ public final class AppWindow: NSWindow {

private let hostingView: NSHostingView<OverlayContentView>

/// 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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -103,12 +127,37 @@ 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)
hostingView.frame = layout.hostingFrame
if let containerView = surface.contentView, containerView !== hostingView {
containerView.frame = CGRect(origin: .zero, size: layout.windowFrame.size)
}
applyWindowFrame(layout.windowFrame, to: surface)
applyFrame(layout.hostingFrame, to: hostingView)
guard let containerView = surface.contentView, containerView !== hostingView else { return }
applyFrame(contentFrame(for: layout.windowFrame), to: containerView)
guard let playerLayer = playerLayer(in: surface) else { return }

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

severity

containerView.frame を設定する前に、現在のフレームと比較するガードを入れることで、不要な AppKit のレイアウトパスをさらに削減できます(applyWindowFrame と同様のパターン)。

Suggested change
guard let playerLayer = playerLayer(in: surface) else { return }
if containerView.frame != CGRect(origin: .zero, size: layout.windowFrame.size) {
containerView.frame = CGRect(origin: .zero, size: layout.windowFrame.size)
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

action

対応しました。提案の if 文形式ではなく、既存の applyWindowFrame と揃えた guard ヘルパー (applyFrame(_:to:)) として実装し、containerView に加えて hostingView の frame 再代入にも同じスキップを適用しています。188edab を参照。

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)
}

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.
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(
Expand Down
18 changes: 11 additions & 7 deletions Tests/PresentersTests/AppPresenterTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions Tests/ScreenInteractorTests/ScreenInteractorImplTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
}
Loading
Loading