Skip to content

feat(filter): multi-select source-repo include filter#175

Merged
ryota-murakami merged 2 commits into
mainfrom
feat/source-repo-multi-select-filter
May 26, 2026
Merged

feat(filter): multi-select source-repo include filter#175
ryota-murakami merged 2 commits into
mainfrom
feat/source-repo-multi-select-filter

Conversation

@ryota-murakami

@ryota-murakami ryota-murakami commented May 26, 2026

Copy link
Copy Markdown
Contributor

What & why

Replaces the single-select source-repo filter (selectedSource: RepositoryId | null) with a multi-select INCLUDE filter (selectedSources: RepositoryId[]).

  • Empty selection → show all skills, including source-less local skills (unchanged default).
  • Non-empty selection → show only skills whose source is in the selection; source-less local skills are hidden.

When the filter hides local skills, an inline metadata hint surfaces the count ("N local skills hidden"), and the bulk-delete confirm dialog names the in-scope repositories plus reassures that hidden locals stay untouched.

Key changes

  • uiSliceselectedSources[] with setSelectedSources / toggleSource / clearSelectedSources; the selection is pruned when its repos disappear. SourceFilterSummary (repositoryIds + localHiddenCount) feeds the bulk-delete confirm scope.
  • selectorsselectSourceFilterViewModel builds the compact trigger label, dropdown rows, filter pills, and localHiddenCount, memoized via reselect.
  • MainContent — dropdown migrated from RadioGroupDropdownMenuCheckboxItem rows plus Show all / Select all items; pills collapse past SOURCE_FILTER_MAX_VISIBLE_REPOS into a "from N repos" summary.
  • bulkDeleteCopy — appends a repo-scope sentence + hidden-locals reassurance to the destructive confirm.
  • formatRepositoryFacetLabel — new util, middle-ellipsis past REPOSITORY_FACET_LABEL_MAX_CHARS (28), used by the compact trigger label only.

Deliberate design decisions

  1. Single-row delete passes sourceSummary: null. A single-row delete carries no repo-filter scope to surface — the confirm dialog stays focused on that one row, so it intentionally omits the multi-repo scope/hidden-locals sentences that the bulk path appends.
  2. SourceLink uses setSelectedSources([source]) (replace, not toggle). Clicking a skill's source link is a focused "show me this repo" jump — it replaces the active selection rather than additively toggling, matching the intent of navigating to a single repo's skills.

A matching nuance: the destructive bulk-delete confirm names a single repo in full (never the truncated trigger label) — width is unconstrained there and knowing the exact source matters most before a destructive action. Truncation is reserved for the space-constrained toolbar trigger.

Quality gates

  • pnpm validate — ✅ green (1123 tests across lint, test, typecheck, dead-code, dupes, health, storybook build)
  • pnpm test:e2e — ✅ green (43 passed)

Version is intentionally unchanged — release version bumps are owned exclusively by /electron-release per project policy.

Summary by CodeRabbit

  • 新機能

    • リポジトリフィルターが複数選択に対応。複数リポジトリを同時に絞り込めます。
    • リポジトリ表示の短縮ラベル(長すぎる名前の省略表示)と表示上限を導入。
  • 改善

    • カードからのフィルタクリアで全スキル表示が確実に戻るよう修正。
    • 削除確認ダイアログの説明を強化し、フィルタ範囲やローカルスキルの影響を明確化。
  • テスト

    • マルチ選択や削除文言などに対応したテストを追加・更新。

Review Change Stack

Replace the single-select source-repo filter (selectedSource:
RepositoryId | null) with a multi-select INCLUDE filter (selectedSources:
RepositoryId[]). Empty selection shows all skills, including source-less
local skills; a non-empty selection shows only skills whose source is in
the selection and hides source-less local skills.

- uiSlice: selectedSources[] with setSelectedSources / toggleSource /
  clearSelectedSources; selectedSources pruned when its repos disappear;
  SourceFilterSummary (repositoryIds + localHiddenCount) feeds the
  bulk-delete confirm scope
- selectors: selectSourceFilterViewModel builds the compact trigger
  label, dropdown rows, filter pills, and localHiddenCount, memoized via
  reselect
- MainContent: dropdown migrated from RadioGroup to
  DropdownMenuCheckboxItem rows plus Show all / Select all items; pills
  collapse past SOURCE_FILTER_MAX_VISIBLE_REPOS into a "from N repos"
  summary; inline "N local skills hidden" metadata hint
- bulkDeleteCopy: appends a repo-scope sentence and a hidden-locals
  reassurance to the destructive confirm; a single repo is named in full
  (never the truncated trigger label)
- formatRepositoryFacetLabel: new util, middle-ellipsis past
  REPOSITORY_FACET_LABEL_MAX_CHARS (28) for the compact trigger only

Two deliberate design decisions:
- single-row delete passes sourceSummary: null — a single row carries no
  repo-filter scope to surface in the confirm dialog
- SourceLink uses setSelectedSources([source]) (replace, not toggle): a
  focused "show me this repo" jump, not an additive selection

validate green (1123 tests); e2e green (43 passed). Version intentionally
unchanged — release bumps are owned by /electron-release.
@vercel

vercel Bot commented May 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
skills-desktop Ready Ready Preview, Comment May 26, 2026 7:13pm

Request Review

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 09017735-78f1-4297-9491-da787e6488f6

📥 Commits

Reviewing files that changed from the base of the PR and between 1591cd5 and 306e3f3.

📒 Files selected for processing (1)
  • src/renderer/src/utils/formatRepositoryFacetLabel.ts

Walkthrough

リポジトリフィルタを単一選択(ラジオ)からマルチ選択(チェックボックス)へ移行。Redux state を selectedSources[] に変更し、selectors、MainContent の dropdown/pill 表示、削除ダイアログ文言、ユーティリティ(ラベル省略)、テストを一貫して更新しています。

Changes

Repository filter: single-select to multi-select refactor

Layer / File(s) Summary
Redux state: selectedSources array + bulk scope snapshot
src/renderer/src/redux/slices/uiSlice.ts, src/renderer/src/redux/slices/uiSlice.test.ts, src/shared/constants.ts
`selectedSource: RepositoryId
Selectors: filtered skills & source filter view model
src/renderer/src/redux/selectors.ts, src/renderer/src/redux/selectors.test.ts
selectFilteredSkillsselectedSources の集合メンバーシップでフィルタ(選択時は source undefined の local を除外)。新規 selectSourceFilterViewModel が dropdownRows、triggerLabel/AriaLabel、validRepoIds、localHiddenCount、isSelectAllDisabled を返す。
MainContent: dropdown, checkbox rows, and pills
src/renderer/src/components/layout/MainContent.tsx, src/renderer/src/components/layout/MainContent.browser.test.tsx
ドロップダウンをラジオ→チェックボックスへ置換。handleToggleSource、Show all / Select all の bulk 操作、sourceFilter 依存のピル集約(SOURCE_FILTER_MAX_VISIBLE_REPOS)と localHiddenCount 注記を実装。テストで checkbox フローと hidden-locals caveat を追加。
Components: dispatch wiring and small fixes
src/renderer/src/components/skills/SourceLink.tsx, src/renderer/src/components/sidebar/SourceCard.tsx, src/renderer/src/components/skills/SkillItem.tsx, src/renderer/src/components/skills/SkillsList.tsx, 各テスト
SourceLinksetSelectedSources([source]) を dispatch。SourceCard.handlePathClickclearSelectedSources() を呼ぶよう修正。SkillItem の単一行削除で sourceSummary: null をペイロードに含める。SkillsList 等のセレクタ import を複数形へ更新。
Bulk delete: source scope text generation
src/renderer/src/components/skills/bulkDeleteCopy.tsx, src/renderer/src/components/skills/bulkDeleteCopy.test.ts
renderBulkDeleteDescription が `SourceFilterSummary
Empty-state message: selectedSources input
src/renderer/src/components/skills/skillsListHelpers.ts, src/renderer/src/components/skills/skillsListHelpers.test.ts
EmptyMessageContextselectedSourceselectedSources[]formatSelectedSourcesPhrase を追加し、空状態メッセージの分岐を配列対応へ更新。
Utility: repository label abbreviation & constants
src/renderer/src/utils/formatRepositoryFacetLabel.ts, src/renderer/src/utils/formatRepositoryFacetLabel.test.ts, src/shared/constants.ts
formatRepositoryFacetLabel を追加(28 文字超過時に先頭/末尾を残して中央を "..." で省略)。SOURCE_FILTER_MAX_VISIBLE_REPOS = 3REPOSITORY_FACET_LABEL_MAX_CHARS = 28 を追加。
Git: ignore tool artifact cache
.gitignore
.understand-anything/ を ignore に追加。

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR title clearly and concisely describes the main change: conversion of single-select source repository filter to multi-select.
Docstring Coverage ✅ Passed Docstring coverage is 90.00% which is sufficient. The required threshold is 80.00%.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/source-repo-multi-select-filter

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

@codecov-commenter

codecov-commenter commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.08911% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.77%. Comparing base (e4a51b2) to head (306e3f3).

Files with missing lines Patch % Lines
src/renderer/src/components/layout/MainContent.tsx 69.23% 7 Missing and 1 partial ⚠️
src/renderer/src/redux/selectors.ts 97.14% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #175      +/-   ##
==========================================
+ Coverage   56.17%   56.77%   +0.60%     
==========================================
  Files         184      185       +1     
  Lines        4691     4766      +75     
  Branches      986     1002      +16     
==========================================
+ Hits         2635     2706      +71     
- Misses       1832     1836       +4     
  Partials      224      224              
Flag Coverage Δ
unittests 56.77% <91.08%> (+0.60%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot 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.

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 `@src/renderer/src/utils/formatRepositoryFacetLabel.ts`:
- Around line 24-25: 現在の実装はハードコードした 12/13 を使っているため
REPOSITORY_FACET_LABEL_MAX_CHARS を変更すると不整合が起きます。formatRepositoryFacetLabel 内で
REPOSITORY_FACET_LABEL_MAX_CHARS と省略記号長("..." の長さ、3)から headChars と tailChars
を算出し(例: head = Math.floor((MAX - ellipsis)/2), tail = (MAX - ellipsis) -
head)、その headChars/tailChars を使って source.slice(0, headChars) と
source.slice(-tailChars) を組み合わせて返すように修正してください。Ensure you reference
REPOSITORY_FACET_LABEL_MAX_CHARS and the existing slice-based return when
locating where to change.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b17471a8-5f55-44bb-bacf-d21506a708a4

📥 Commits

Reviewing files that changed from the base of the PR and between e4a51b2 and 1591cd5.

📒 Files selected for processing (20)
  • .gitignore
  • src/renderer/src/components/layout/MainContent.browser.test.tsx
  • src/renderer/src/components/layout/MainContent.tsx
  • src/renderer/src/components/sidebar/SourceCard.tsx
  • src/renderer/src/components/skills/SkillItem.browser.test.tsx
  • src/renderer/src/components/skills/SkillItem.tsx
  • src/renderer/src/components/skills/SkillsList.tsx
  • src/renderer/src/components/skills/SourceLink.browser.test.tsx
  • src/renderer/src/components/skills/SourceLink.tsx
  • src/renderer/src/components/skills/bulkDeleteCopy.test.ts
  • src/renderer/src/components/skills/bulkDeleteCopy.tsx
  • src/renderer/src/components/skills/skillsListHelpers.test.ts
  • src/renderer/src/components/skills/skillsListHelpers.ts
  • src/renderer/src/redux/selectors.test.ts
  • src/renderer/src/redux/selectors.ts
  • src/renderer/src/redux/slices/uiSlice.test.ts
  • src/renderer/src/redux/slices/uiSlice.ts
  • src/renderer/src/utils/formatRepositoryFacetLabel.test.ts
  • src/renderer/src/utils/formatRepositoryFacetLabel.ts
  • src/shared/constants.ts

Comment thread src/renderer/src/utils/formatRepositoryFacetLabel.ts Outdated
…onstant

CodeRabbit flagged the hardcoded 12/13 head/tail split in
formatRepositoryFacetLabel as decoupled from REPOSITORY_FACET_LABEL_MAX_CHARS:
changing the constant would silently break the "exactly MAX chars" invariant.

Derive head/tail from the constant (head = floor(budget/2), tail = the
remainder) so the split re-balances automatically. Output is unchanged for the
current value (12 + "..." + 13 = 28), so the existing test and JSDoc example
stay valid.
@ryota-murakami ryota-murakami merged commit d2838ef into main May 26, 2026
11 checks passed
@ryota-murakami ryota-murakami deleted the feat/source-repo-multi-select-filter branch May 26, 2026 19:17
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