Repository navigation
feat(picker): サムネイルグリッドのエフェクトピッカー — Browse Effects… から開くウィンドウ (#12 の UI 側) - #65
Merged
Merged
Conversation
…senters Move MenuBarPresenter's private StateObservation into CatalogStateObservation, a class that ends itself when it is let go, so a second Presenter can observe the EffectInteractor without copying the sink, the newest-wins stream and the cancellation check.
The picker's cells come from the same EffectInteractor state as the menu bar, so the chosen cell is always the menu's checked row. The Presenter only observes: the menu's Presenter owns the Interactor's run, and a second start() there would restart the catalog. Thumbnails are a placeholder artwork (monogram and a hue stable across processes) behind an EffectArtwork enum that real images can join later. EffectPickerGeometry lives with the Presenters, not the Views, because Views is coverage-ignored and the column count is logic. Part of #12
A search field over a grid of placeholder cells, laid out by EffectPickerGeometry: the chosen cell has an accent border, a broken package is a disabled cell, and hovering a cell shows its name, author or reason. EffectPickerWindow hosts the view and reports its own close, so the Router decides what a close means. Part of #12
The menu's Presenter hands Browse Effects… to its Router through a weak MenuBarRouting reference. AppRouter owns the picker: it makes at most one window through an injected factory, starts the picker's Presenter while that window is open and stops it when the window closes, from its title bar or from stop(). A close reported by a window the Router already let go changes nothing. AppRouter now imports Views to build the window. The host keeps running when the picker, its only window, closes; the dev harness still quits when its preview window closes, picker open or not. The harness's Effects menu is the host's EffectMenu, so it gets the item too. Part of #12
ARCHITECTURE.md gains the Effect picker decision (one state seen by two views, a picker Presenter that only observes, the geometry in Presenters, the placeholder thumbnail slot, the Router's window lifecycle, and why there is no live preview yet), the AppRouter -> Views edge, and the Layer Summary and physical-table rows. AGENTS.md, the import allow-list, the View-file idiom and Lane 1 of dev verification say the same. Part of #12
Walkthrough
Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly describes the main change: a thumbnail-grid effect picker window opened from “Browse Effects…”. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 11 functions across 25 files. (5 skipped: 5 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
… hue from NFC A name like "[WIP] Ripple" gave the monogram "[R"; each word now gives its first letter or digit, one character upper-cased, and a word with neither is skipped, so the tile holds at most two characters as the Entity documents. Two keys that differ only in how an accent was typed are the same EffectID (String equality is canonical), but their UTF-8 differed, so the same effect could get two colours. The hue now hashes the composed (NFC) form. Part of #12
Views is coverage-ignored and holds declarations only, but the picker's window is AppKit glue with a lifecycle: it makes the NSWindow, shows and closes it and forwards its close. It now sits beside the Router that opens it, so the test that makes the live window counts toward coverage, and the docs no longer call Views declarative while listing a window in it. AppRouter still imports Views for EffectPickerView, the view the window hosts; the Router tests no longer need Views. Part of #12
AGENTS.md said the Router calls only the menu's start()/stop(); it also starts and stops the picker's Presenter with the picker's window, and only the menu's Presenter starts the Interactor. HostApplication called the host an app with no window; the picker is its window now. The seeded property-test targets in ARCHITECTURE.md gain the picker's grid geometry. Part of #12
# Conflicts: # .claude/rules/dev-verification.md # docs/ARCHITECTURE.md # version.txt
# Conflicts: # docs/ARCHITECTURE.md
This was referenced Oct 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
issue #12 の
側の半分。Snap Camera 風のサムネイルグリッドのエフェクトピッカーを足した。メニューバーの Browse Effects… でウィンドウが開き、検索欄の下にエフェクトがセルで並ぶ。選んでいるセルはメニューのチェックの行と常に
。サムネイルは今はプレースホルダ (頭文字と色) で、本物の画像はデータ側のセッションが入れる 
で差し込む。
EffectThumbnailをmake testのプロセスの中でNSHostingViewをオフスクリーンでcacheDisplayした画像 (fixture の 6 件、Rippleを選択、Brokenは manifest が無いパッケージ)。実機のswift run dev-harnessでは撮っていない。EffectPickerPresenterとEffectPickerGeometry(3f321d6): セル (EffectPickerCell: 名前、2 行目、選択中、選べるか、hover の文、artwork) を、メニューとEffectInteractor.stateから作る。2 行目と status 行はメニューのMenuBarTextをそのまま使い、No effects match “…”。Presenter は 観察するだけ で、Interactor の start / stop はしない (メニューの Presenter が持ち主で、2 回目のstart()は catalog の再起動になる)。グリッドの列数とセル幅はEffectPickerGeometry(width:)の純関数で、Viewsが coverage の ignorePresentersに置いた。例の5313d62):EffectPickerViewは検索欄の下のLazyVGrid。選択中のセルはアクセント色の枠、壊れたパッケージは薄く押せないセル、hover で名前と作者 (または選べない594db20、窓の移動はce9f538): メニューの Browse Effects… →MenuBarPresenter.browseEffects()→MenuBarRouting(weak) →AppRouter。Router は注入できるmakePickerWindowでウィンドウを作り (live はEffectPickerWindow:EffectPickerViewをNSWindowに載せ、閉じたことを自分自身付きでViewsではなくAppRouterに置いた)、開いているのは常に 1 枚 (2 回目はstop()で閉じても Presenter を止める。既に手放したウィンドウからの遅れた closeapplicationShouldTerminateAfterLastWindowClosed→false)。ハーネスはプレビューのウィンドウを閉じたら、ピッカーが開いていてもEffectMenuなので、75a5c36):docs/ARCHITECTURE.mdに Key Design Decision Effect picker を足し、依存グラフにAppRouter --> Views、Layer Summary (Entry / Router / View / Presenter / Entity)、Physical Abstraction Table の Menu bar / windows、Menu bar のAGENTS.md、import allow-list (AppRouterがViewsを import する)、View ファイルの idiom (Geometry はPresenters)、Lane 1 の Effects メニューの行と新しい Effect picker の5e407ee、ce9f538、bbf403a): 下の「レビュー」をc84c58b)。main が fix: #64 のマージ後レビューの指摘を直し、0.21.1 に bump する (formatVersion の README・テスト行、HarnessMenu.live() の owner guard ほか) #67 (0.21.1) と feat: carry the manifest thumbnail through to the effect catalog (#12, data half) #66 (0.22.0) で進んだので、そのたびに main を取り込んで数え直したfa61f42): メニューの Presenter の state の観察 (sink→ newest-wins のAsyncStream→ main actor の task) をCatalogStateObservationに切り出し、2 つの Presenter でstateは最新の値を replay するので、artworkはEffectArtworkenum で、今のケースは.placeholder(EffectPlaceholder)だけ。プレースホルダは名前の最初の 2 語の頭文字と、<origin>:<key>の FNV-1a 64 から作る色相 (hashValueと違い、起動ごとに色が変わらない)。EffectCatalogEntry.thumbnailがあるときにEffectArtworkに画像のケースを足し、Presenter (EffectPickerText.swiftのpickerCell) がどちらを使うか決め、EffectArtworkViewがそのケースを描く。他は変わらないAppRouterがViewsを import する: lyra の wireframe とPickerWindowprotocol 越しにしか見ないので、入れていないもの
CameraInteractorに attach が無く、PreviewViewを attach しているのはハーネスのHarnessInteractorだけ)。ピッカーにプレビューを出すには、まずホストにその経路が要るdescriptionの hoverEffectCatalogEntryがdescriptionを持っていない。足すとデータ側のセッションとEffectCatalogEntryでCloses #12はレビュー
独立したレビュアー 5 系統 (ライフサイクル、アーキテクチャ
、
、コメント・docs の正確さ、型
) に diff を見せ、出た指摘 18 件をそれぞれ別の
役が反証を試みた。残った 10 件はすべて直した。
EffectPickerWindowをViewsからAppRouterに移した (ce9f538)。Viewsは coverage[WIP] Rippleが[RだったのをWRに)。色相は NFC に正規化してからハッシュする (アクセントの打ち方だけが違う key はEffectIDなのに、色が 2 つになり得た) (5e407ee)AGENTS.mdの「Router はメニューのstart()/stop()だけを呼ぶ」(ピッカーの Presenter も開閉に合わせて呼ぶ)、HostApplicationの「window の無いアプリ」、seeded property test のbbf403a、ce9f538)反証された 8 件は、
した
か、既に
で押さえてある。CodeRabbit は指摘なし、Copilot はクォータ
でレビューしていない。
MenuBarRoutingが Domain の外の protocol であること、effectPickerPresenterが public であること、画像ケースの読み込み中の扱いなど。いずれもmake format(make lint、make check(全スクリプトmake build、make build-dev-harnessmake test: 1026 tests / 137 suites、known issue 12 件 (すべてFakeGPUGatewayなどの double の契約InMemoryFileSystemGatewayの fakemake test-gpu: 27 tests / 6 suitesEffectPickerGeometryTests、EffectPickerPresenterTests(2 つの Presenter を 1 つの Interactor に載せ、選択が変わるたびにピッカーの選択セルとメニューのチェック行が一致すること、ピッカーが Interactor を start / stop しないことを含む)、AppRouterPickerTests(fake のウィンドウで 1 枚・stop()・ピッカーを開いたまま Router を手放す。live のEffectPickerWindowは 1 回だけ作ってMenuBarPresenterTestsに router への受け渡し 2 件EffectPickerWindow.show()(app をHarnessAppDelegateのwindowWillCloseと delegate のAppRouter.initの既定値AppRouter.pickerWindow(over:onClose:)の 1 行 (live の窓を作る thunk で、窓を作るPart of #12