feat: スキル表示名の畳み込み確定を解除(バラす)できるようにする(ADR-0016 D11 / #496)#516
Conversation
一度確定した表示名・畳み込みグループを機械デフォルトへ戻す導線が無く、 upsert(削除しない)設計で確定行が残り続け完全リセットできなかったギャップを埋める。 Backend: - DELETE /skills/display-decisions を新設(identity 群指定で Layer 3 確定行を削除)。 authz は confirm と同一(当該ユーザーの検出済みスキルに属すること/存在しない 確定行は冪等に無視)。リセット後の最新スキル一覧を返す - repositories/skill.py に delete_by_identities を追加(user_id 固定・ORM 削除) - schemas に SkillDisplayResetRequest を追加、make codegen-types で generated.ts 再生成 Web: - utils/skillDisplay に isResettableGroup / buildResetIdentities を追加(TDD) - useGitHubSkills に reset / resetting を追加(TDD) - スキル一覧の確定済みチップに「解除」ボタンを追加。グループ全メンバーを まとめて解除するため畳み込みも解ける Docs: - ADR-0016 D11 に項 (g)(解除)と変更履歴を追記 テスト: backend 5 ケース / web util 5・hook 2 ケース / E2E 解除フロー 1 本を追加 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an authenticated DELETE flow for resetting confirmed GitHub skill display decisions. The backend removes matching records and returns refreshed skills, while the generated client, hook, grouped UI controls, tests, and ADR support the reset behavior. ChangesSkill display reset
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SkillDisplaySection
participant useGitHubSkills
participant GitHubLinkAPI
participant Backend
User->>SkillDisplaySection: Click reset
SkillDisplaySection->>useGitHubSkills: Reset group identities
useGitHubSkills->>GitHubLinkAPI: Send DELETE request
GitHubLinkAPI->>Backend: Delete display decisions
Backend-->>GitHubLinkAPI: Return refreshed skills
GitHubLinkAPI-->>useGitHubSkills: Updated skill list
useGitHubSkills-->>SkillDisplaySection: Render machine defaults
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/components/github-link/SkillDisplaySection.tsx (1)
60-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePolish reset button text state.
Because
resettingis a single global boolean coming from the hook, clicking "解除" on one group will simultaneously change the text of all visible reset buttons to "解除中...". You can track the clicked group locally to only change the text of the button that was actually clicked, while still keeping all buttons disabled during the request.✨ Proposed optional polish
Add a local state at the top of the component (e.g., around line 37):
const [resettingKey, setResettingKey] = useState<string | null>(null);Then update the button rendering to use the local state for its text:
- {/* 確定済みグループのみ、機械デフォルトへ戻す「解除」を出す(#496) */} - {isResettableGroup(group) && ( - <button - type="button" - className={styles.resetButton} - onClick={() => void reset(buildResetIdentities(group))} - disabled={resetting || confirming || proposing} - aria-label={SKILL_DISPLAY_MESSAGES.resetAriaLabel(group.label)} - > - {resetting - ? SKILL_DISPLAY_MESSAGES.RESETTING - : SKILL_DISPLAY_MESSAGES.RESET} - </button> - )} + {/* 確定済みグループのみ、機械デフォルトへ戻す「解除」を出す(#496) */} + {isResettableGroup(group) && ( + <button + type="button" + className={styles.resetButton} + onClick={async () => { + setResettingKey(group.key); + await reset(buildResetIdentities(group)); + setResettingKey(null); + }} + disabled={resetting || confirming || proposing} + aria-label={SKILL_DISPLAY_MESSAGES.resetAriaLabel(group.label)} + > + {resettingKey === group.key + ? SKILL_DISPLAY_MESSAGES.RESETTING + : SKILL_DISPLAY_MESSAGES.RESET} + </button> + )}🤖 Prompt for 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. In `@web/src/components/github-link/SkillDisplaySection.tsx` around lines 60 - 73, Update SkillDisplaySection’s reset-button state using a local resettingKey state tied to the clicked group, and use it to show RESETTING only for that group’s button. Keep the existing global resetting value for disabling all reset buttons during the request, and clear the local key when the reset operation completes.
🤖 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.
Nitpick comments:
In `@web/src/components/github-link/SkillDisplaySection.tsx`:
- Around line 60-73: Update SkillDisplaySection’s reset-button state using a
local resettingKey state tied to the clicked group, and use it to show RESETTING
only for that group’s button. Keep the existing global resetting value for
disabling all reset buttons during the request, and clear the local key when the
reset operation completes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d4c189f7-4199-4f82-84e4-44800f6f5404
📒 Files selected for processing (16)
backend/app/repositories/skill.pybackend/app/routers/github_link/endpoints.pybackend/app/schemas/github_skill.pybackend/tests/test_skill_display_api.pydocs/adr/0016-github-skill-inference.mdweb/e2e/github-link.spec.tsweb/src/api/generated.tsweb/src/api/githubLink.tsweb/src/api/types.tsweb/src/components/github-link/SkillDisplaySection.module.cssweb/src/components/github-link/SkillDisplaySection.tsxweb/src/constants/messages.tsweb/src/hooks/useGitHubSkills.test.tsweb/src/hooks/useGitHubSkills.tsweb/src/utils/skillDisplay.test.tsweb/src/utils/skillDisplay.ts
global resetting は全解除ボタンの無効化に使い、押されたグループの区別は ローカルの resettingKey で行う。確定済みグループが複数ある場合に全ボタンへ 「解除中...」が出ていたのを、対象グループのボタンだけに出るよう修正。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
概要
スキル表示名の human-in-the-loop 確定(ADR-0016 D11 / #476)で、一度確定した表示名・畳み込みグループを「解除(バラす)」する導線が無い問題(#496)を解消する。upsert(削除しない)設計で確定行が残り続け、機械デフォルトへ完全リセットできなかったギャップを埋める。
Closes #496
設計方針
「解除」= 対象 identity の Layer 3 確定行を DELETE して機械デフォルトへ戻す単一アクション。グループの全メンバー identity をまとめて送ることで「グループ解除(バラす)」と「確定リセット(表示名を戻す)」を同時に満たす。issue の案1(DELETE エンドポイント)を主軸に採用し、案3のメンバー個別分割 UI は作らない(ADR に残課題として明記)。
変更点
Backend
DELETE /skills/display-decisionsを新設(identity 群指定で確定行を削除)。authz は confirm と同一(当該ユーザーの検出済みスキルに属すること/存在しない確定行は冪等に無視)。リセット後の最新一覧を返すrepositories/skill.pyにdelete_by_identities(user_id 固定・ORM 削除)schemasにSkillDisplayResetRequest追加 →make codegen-typesでgenerated.ts再生成Web
utils/skillDisplay:isResettableGroup/buildResetIdentities(TDD)hooks/useGitHubSkills:reset/resetting(TDD)Docs
テスト
ローカル
make cigreen(backend 729 / web 389 / build OK)、E2E(github-link)8 passed。レビュー観点
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests