-
Notifications
You must be signed in to change notification settings - Fork 1
deep-review: add convergent round thresholds #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| # Iterative review convergence | ||
|
|
||
| Read this reference when an earlier deep-review verdict exists for the same | ||
| PR/MR or the user identifies the current review as a later round. It governs | ||
| severity classification and final disposition in both parallel and serial | ||
| modes. | ||
|
|
||
| ## Determine the round and multiplier | ||
|
|
||
| Round 1 uses the ordinary baseline for a blocking finding. Each successive | ||
| round doubles the threshold for a **new** blocking finding: | ||
|
|
||
| | Round | New-blocker threshold | | ||
| |------:|----------------------:| | ||
| | 1 | 1x (baseline) | | ||
| | 2 | 2x | | ||
| | 3 | 4x | | ||
| | 4 | 8x | | ||
| | N | `2^(N-1)`x | | ||
|
|
||
| For a PR/MR, count completed deep-review panel verdicts from earlier | ||
| invocations; the current invocation is the next round. A retry, duplicate post, | ||
| or repost of one verdict does not create another round. If the user explicitly | ||
| supplies a round, use it. Without review history or explicit round context, use | ||
| round 1. | ||
|
Comment on lines
+23
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The fallback for local branches depends on the user explicitly supplying a round, but Step 1.1 and the skill's argument table define no round option; moreover, a bare integer is parsed as a PR identifier. Thus a user cannot reliably invoke a local second-round review through the documented command interface, and it silently remains at round 1. Add a documented, validated option such as Useful? React with 👍 / 👎. |
||
|
|
||
| ## Classify carried and new findings | ||
|
|
||
| 1. Inventory prior BLOCKING findings by their underlying defect, not by title, | ||
| line number, or wording, which may change between revisions. | ||
| 2. Keep every unresolved prior blocker BLOCKING. It does not need to satisfy the | ||
| current multiplier again. Require evidence before treating it as resolved. | ||
| 3. Treat a finding as new when no earlier verdict classified the underlying | ||
| defect as BLOCKING. Promoting an earlier suggestion or note is also a new | ||
| blocker. A defect shown resolved in an intervening verdict and later | ||
| reintroduced is a new regression. | ||
| 4. Apply the multiplier only when deciding whether a new finding is BLOCKING. | ||
| The multiplier raises the combined bar for evidence, confidence, likelihood | ||
| in supported use, and consequence of merging. It is a comparative decision | ||
| rule, not an arithmetic bug score: at 2x a candidate must be materially more | ||
| compelling than an ordinary blocker; at 4x it needs high confidence and | ||
| severe merge impact; at 8x and beyond only increasingly exceptional, | ||
| well-supported risks should block. | ||
| 5. Report a new finding that misses the current blocking threshold as a | ||
| SUGGESTION or NOTE. Do not suppress it, and do not let it determine the | ||
| disposition. | ||
|
|
||
| Reproducer confirmation strengthens the evidence side of the threshold but | ||
| does not bypass the multiplier for a new finding. An unresolved blocker from an | ||
|
Comment on lines
+48
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On round 2 or later, a new bug whose static evidence does not yet clear the multiplier is downgraded to SUGGESTION/NOTE by step 5, but Phase 4 of Useful? React with 👍 / 👎. |
||
| earlier round continues to block even if no new reproduction is needed. | ||
|
|
||
| ## Arbiter output | ||
|
|
||
| State the round and multiplier in the disposition. Separate carried blockers | ||
| from new findings, and say whether each new BLOCKING finding cleared the | ||
| current threshold. This makes convergence decisions auditable across rounds. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,7 +59,7 @@ MERGE_BASE=$(git merge-base "$BASE_REMOTE/$BASE_BRANCH" HEAD) | |
|
|
||
| If no base ref can be determined, error and exit. | ||
|
|
||
| ## Step 1.4: Detect prior reviews (PR/MR only) | ||
| ## Step 1.4: Detect prior reviews and determine the round | ||
|
|
||
| **GitHub:** | ||
| ```bash | ||
|
|
@@ -80,3 +80,9 @@ to all specialists and the arbiter as context. Specialists should: | |
| - Flag prior findings that remain unresolved | ||
| - Avoid re-raising issues that were already noted and resolved | ||
| - Call out any regressions — issues that were fixed but reappeared | ||
|
|
||
| After collecting this context, read | ||
| [iterative-reviews.md](iterative-reviews.md) to determine the review round, | ||
| threshold multiplier, carried blockers, and new candidates. For a local branch | ||
|
Comment on lines
+84
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For GitLab MRs, the context collected before this newly required round calculation excludes verdicts produced by the skill itself: Useful? React with 👍 / 👎. |
||
| without review history, use explicit round context supplied by the user; if | ||
| none was supplied, treat the invocation as round 1. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On GitHub, the setup query treats every PR comment containing the public footer text as a prior panel verdict, without checking its author or validating the verdict structure. Any participant who quotes or copies that footer can therefore inflate the count used here; several such comments can push the review to a very high multiplier and cause legitimate new blockers to be downgraded. Count only comments created by the expected reviewer identity and matching a complete verdict, or store a verifiable invocation identifier.
Useful? React with 👍 / 👎.