Skip to content

refactor: picker 非依存な state mutator を抽出 (PR 1/2)#124

Merged
kyu08 merged 7 commits into
flexphere:mainfrom
shusann01116:refactor/extract-picker-state-mutators
Apr 22, 2026
Merged

refactor: picker 非依存な state mutator を抽出 (PR 1/2)#124
kyu08 merged 7 commits into
flexphere:mainfrom
shusann01116:refactor/extract-picker-state-mutators

Conversation

@shusann01116
Copy link
Copy Markdown
Collaborator

Summary

  • files.lua / scope.lua の Telescope picker 内に混在していた「state 変更 + gh API 呼び出し」ロジックを picker 非依存のコア mutator (apply_viewed_toggle / apply_reviewed_toggle) に抽出
  • 既存 toggle_*_in_pickertoggle_*_in_telescope にリネームし、コア mutator に委譲する薄い adapter に再構成
  • 挙動は完全に不変(state 更新タイミング・通知文言・picker refresh 順序すべて保持)

Motivation

後続 PR(snacks.picker 対応)の前提を整えるためのリファクタ。2 picker 実装が並列するとコード重複が発生するため、先に state 操作と picker UI 更新の責務を分離する。

詳細仕様は docs/superpowers/specs/2026-04-22-snacks-picker-support-design.md(※ ローカル限定ドキュメント)。

Changes

ファイル 内容
lua/fude/files.lua M.apply_viewed_toggle(path, on_done) 新設、toggle_viewed_in_pickertoggle_viewed_in_telescope リネーム+委譲
lua/fude/scope.lua M.apply_reviewed_toggle(sha) 新設、toggle_reviewed_in_pickertoggle_reviewed_in_telescope リネーム+委譲
tests/fude/files_spec.lua apply_viewed_toggle の 4 ケース追加(UNVIEWED→VIEWED / VIEWED→UNVIEWED / gh error / nil pr_node_id)
tests/fude/scope_spec.lua apply_reviewed_toggle の 3 ケース追加(false→true / true→false / nil sha)
CLAUDE.md apply_* export を module 説明に追記、Key Patterns に picker adapter 命名規約を追加

Test plan

  • make all 全 pass(lint / format-check / 全 spec)
  • 新規テスト合計 7 ケース全 pass
  • 既存 200+ テストに回帰なし
  • telescope / quickfix モード挙動は不変(コードレビューで確認)

Follow-up

  • PR 2: file_list_mode = "snacks" 追加と snacks.picker 対応本体。本 PR merge 後に rebase して上げる予定。

🤖 Generated with Claude Code

shusann01116 and others added 5 commits April 22, 2026 19:03
Adds picker-agnostic core mutator for viewed state transitions.
Separates state mutation and gh API interaction from picker-specific
UI updates, enabling reuse across different picker implementations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…elescope

Makes the telescope-specific scope of the adapter explicit in the name
and delegates state mutation to apply_viewed_toggle. No behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds picker-agnostic core mutator for reviewed commit state transitions.
Separates state mutation from picker-specific UI updates, enabling reuse
across different picker implementations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…in_telescope

Makes the telescope-specific scope of the adapter explicit in the name
and delegates state mutation to apply_reviewed_toggle. No behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds apply_viewed_toggle / apply_reviewed_toggle to the files.lua /
scope.lua module descriptions and introduces the picker-agnostic state
mutator pattern under Key Patterns.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@shusann01116 shusann01116 marked this pull request as ready for review April 22, 2026 11:36
@kyu08 kyu08 requested a review from Copilot April 22, 2026 11:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Telescope picker 内に混在していた「state 更新 +(必要に応じて)GitHub API 呼び出し」を、picker 非依存な apply_* mutator と Telescope adapter (toggle_*_in_telescope) に分離し、将来の picker 実装追加(snacks.picker 等)に備えるリファクタです。

Changes:

  • files.luaapply_viewed_toggle(path, on_done) を追加し、Telescope 側は toggle_viewed_in_telescope へ移行(薄い委譲に整理)
  • scope.luaapply_reviewed_toggle(sha) を追加し、Telescope 側は toggle_reviewed_in_telescope へ移行
  • 新規 mutator のテスト追加と、CLAUDE.md への export / 命名規約の追記

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lua/fude/files.lua viewed toggle を picker 非依存 mutator + Telescope adapter に分離
lua/fude/scope.lua reviewed toggle を picker 非依存 mutator + Telescope adapter に分離
tests/fude/files_spec.lua apply_viewed_toggle の遷移/エラー/ガード動作のテスト追加
tests/fude/scope_spec.lua apply_reviewed_toggle のトグル/ガード動作のテスト追加
CLAUDE.md apply_* export と picker adapter 命名規約のドキュメント更新

Comment thread tests/fude/files_spec.lua Outdated
Comment on lines +375 to +378
vim.wait(50)
assert.is_false(gh_called)
assert.is_false(invoked)
end)
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

この待機は固定スリープになっており、CI負荷次第で失敗/不安定になり得ます。vim.wait(timeout, condition)gh_calledinvoked が true になったら即終了する形にすると、意図(呼ばれないことの保証)と安定性が上がります。

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

4afd5f1 で修正しました。gh_calledinvoked 両方をチェックする condition で wait するようにし、fired を 1 つの assert で集約しました。ご指摘ありがとうございます。

Comment thread tests/fude/files_spec.lua Outdated
Comment on lines +358 to +361
vim.wait(100)
assert.is_false(invoked)
assert.is_nil(config.state.viewed_files["src/baz.lua"])
end)
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

この2つのテストは vim.wait(100) / vim.wait(50) で固定時間スリープしているため、実行環境が遅い場合にフレークしやすいです。vim.wait(timeout, condition) の condition に invoked / gh_called の変化を入れて「呼ばれていないこと」をタイムアウトまで監視する形にすると、意図が明確で安定します。

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

4afd5f1 で修正しました。固定 sleep から vim.wait(timeout, function() return invoked end) に置換し、戻り値 firedassert.is_false(fired) で検証する形にしました。これで「呼ばれない」の意図が明示的になり、誤ってコールバックが fire した場合は早期に検出できます。ご指摘ありがとうございます。

shusann01116 and others added 2 commits April 22, 2026 21:07
Replaces fixed vim.wait(100) / vim.wait(50) sleeps in apply_viewed_toggle
tests with vim.wait(timeout, condition) so the wait exits early if the
callback unexpectedly fires, and the fired return value makes "must not
fire" the explicit assertion. Improves CI stability and clarifies intent.

Addresses Copilot review comments on PR flexphere#124.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…here#124

Captures the Copilot review insight that "callback must not fire"
assertions should use vim.wait(timeout, condition) instead of fixed
sleeps, so the wait exits early on unexpected invocation and the
intent is explicit in code.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kyu08 kyu08 merged commit 28b47aa into flexphere:main Apr 22, 2026
5 checks passed
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.

3 participants