spike: evaluate and improve the PR review path
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 初心者へのやさしさ
- 20/100
調査の方向性
Start by reading the user-level pr-triage, pr-review, and pr-scoped-reviewer.md files named in the issue, along with the relevant PR diffs and follow-up fixes. Build the back-test table and record whether each bug was caught, dropped, or never raised, then use those results to assess the proposed changes. Done means the table and findings are posted, any adopted changes are re-tested against it, and the stated skill changes are complete.
索引モデルが issue の本文から書いたものです。
説明
Found while auditing the pipeline for #400 (piece 7). The PR review path (pr-triage, which runs pr-review and /code-review medium and then triages the findings) often returns a clean review, and the owner suspects it is missing real bugs. There is no data either way. This spike measures it and, if the data says so, improves it.
All of the review skills and agents involved are user-level files on the owner's machine (~/.claude/skills/pr-triage, ~/.claude/skills/pr-review, ~/.claude/agents/pr-scoped-reviewer.md), so there is no repo change unless the spike produces one.
Background
- The built-in
/code-reviewwas too critical about small issues and kept producing new changes on each pass, so a PR almost never got a clean review. pr-review(a thin wrapper aroundpr-scoped-reviewer) fixed the noise but seemed to miss bugs.pr-triagecombines both and adds a verify-and-rank step with a human walkthrough. It is the only one still in use.
Hypotheses to test (from reading the skill text, not from run data)
pr-scoped-reviewermay only cite lines the diff adds or changes and is told to omit anything in doubt, so it cannot report an omission (a caller, validator, migration or mapping that should have changed and did not).- The filters stack: the reviewer drops what it cannot prove, then the triage step drops what is speculative or pre-existing, so a real bug can be lost as "speculative".
- Neither reviewer sees the issue's acceptance criteria, so "does it do what was asked" is checked only by
implementation-validatoron pipeline PRs. - Neither reviewer is told this repo's known traps (enum casing, journal carry-forward, timezone, compose volumes).
- The two reviewers are the same model on the same diff, so agreement between them adds little confidence about what both missed.
Scope
- Build a back-test set: merged PRs that were later followed by a fix (a
bugfix/branch, afix:commit or a bug issue that traces to the earlier PR). Record for each one the bug, the PR it came from, and the category (omission, logic, project-specific trap, other). - Run the current
pr-triagereviewers (pr-scoped-reviewerand/code-review medium) on each original PR diff, read-only. Record whether the bug was caught, found but dropped in verification, or never raised, and which layer lost it. - Over the next several live
pr-triageruns, note whether anything on the dropped list later proves real. - From the results, decide which of these to apply, if any:
- allow omission findings that cite the changed line and the unchanged code that should have changed
- a "needs a human look" bucket for speculative critical or important candidates, in place of silent drops
- pass the issue's acceptance criteria and the CLAUDE.md Safe Change Rules to the reviewers
- drop one of the two reviewers if it finds nothing the other does not
- Change
pr-triageStep 2 to callpr-scoped-reviewerdirectly with the diff it already fetched, then retire thepr-reviewskill. Do these two together: deletingpr-reviewfirst would breakpr-triage, which invokes it. This also removes the duplicate diff fetch. - Remove the "shell quoting mangles" sentences in
pr-triage(Step 5b item 4 and Step 5c item 5). The--input <file>mechanism for the review JSON stays. #400 decided this cut in piece 2.
Acceptance criteria
- A back-test table is posted on this issue: each known bug, the PR it came from, and the result (caught, dropped in verification, or never raised) for the current review path.
- The findings say which layer lost the misses: reviewer scope, the verification filter, or missing context.
- Each change from scope item 4 is either applied or rejected, with a reason that points at the table.
- After any change, the back-test is re-run and the result is posted. The miss rate is no worse, and the noise on a few recent clean PRs has not clearly grown.
pr-triageno longer invokespr-review, and thepr-reviewskill is gone (scope item 5).
Out of scope
- The validator's role in the feature-factory pipeline. #400 decided it checks the diff against the approved brief only; revisit if this spike finds omissions need catching before the PR.
- Running
pr-triagefrom the pipeline. #400 decided it stays manual; the pipeline prints/pr-triage <N>after the PR is created. - Any change to the built-in
/code-review.
- 主要言語
- C#
- スター
- 1
- フォーク
- 0
- 平均マージ
- 2時間 2分
- マージ済み PR(30日)
- 98
環境構築
- Dockerfile または Docker Compose ファイルあり
- プルリクエストのテンプレートなし
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
nevridge/TaskFlow のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
メンテナーはふだん 1 日以内に返信
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 65/100
メンテナーはふだん 1 日以内に返信
-
tech-debt
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
メンテナーはふだん 1 日以内に返信
nevridge/TaskFlow の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 74/100
juandresrodca/AdminForge#54 ·
-
要トリアージ 辞書
難易度 1/5 1〜3時間 初心者へのやさしさ 86/100
yksr-melt/Meltype#1625 · リアクション 1 件 ·
メンテナーはふだん 1 日以内に返信
-
Docs typoオープン
難易度 1/5 1時間未満 初心者へのやさしさ 78/100
MonoGame/docs.monogame.github.io#328 · コメント 1 件 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100
space-wizards/RobustToolbox#7197 ·
メンテナーはふだん 1 日以内に返信
-
documentation high priority
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
lachydotmcg/capri-desktop#201 ·
メンテナーはふだん 1 日以内に返信