Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 32 additions & 19 deletions doc/RFC-pr-state-machine.md
Original file line number Diff line number Diff line change
Expand Up @@ -360,33 +360,40 @@ recovery, orphan adoption after a crash). Nothing new is invented for writes.
malformed answers and incomplete coverage cannot resolve it. The request
and result shapes are defined in [the SDK contract below](#sdk-boundary).

An author can dispute a finding with a 👎 reaction on its bot comment
or a reply in its thread. A dispute removes that finding from the ready
gate without proving it fixed. Disputed findings stay in the ledger and
are counted on the ready page so the maintainer sees the disagreement.
Review evidence and dispute state are stored separately; withdrawing a
dispute restores blocking status unless a valid explicit fix remains.
An author can dispute a finding with a 👎 reaction on the bot's inline
review comment. When GitHub cannot place a blocker/major finding inline,
the bot provides one marker-deduplicated PR issue comment for that finding;
the author's 👎 there has the same meaning. A reply can clarify the
finding, but an ordinary reply is not unambiguous evidence of disagreement
and does not by itself clear the gate. A dispute removes that finding from
the ready gate without proving it fixed. Disputed findings stay in the
ledger and are counted on the ready page so the maintainer sees the
disagreement. Review evidence and dispute state are stored separately;
withdrawing a dispute restores blocking status unless a valid explicit
fix remains.

**Disputes are an evidence source with their own deadline, not a
by-product of the next review.** Thread replies arrive through the
repo-wide comment feed the reconciler already reads every cycle.
Reactions have no feed, so every PR with ≥ 1 **unresolved** blocker/major
— open *or disputed*; only `fixed` ends a finding's life — carries a
by-product of the next review.** Reactions have no feed, so every PR
with ≥ 1 **unresolved** blocker/major — open *or disputed*; only `fixed`
ends a finding's life — carries a
`DISPUTE_SCAN_INTERVAL` deadline (default 15 min), folded into the row's
`next_deadline_at`. On that deadline the reconciler reads the reactions
on each unresolved finding's stored `comment_id` from the findings
ledger, whatever head that comment was posted on (a carried finding
keeps the comment id of its first publication until the re-review
posts a new one), one request per comment, counted against
`PR_STATE_REQUEST_BUDGET`; the existing daily feedback scan is too slow
for this and is left as is. Either source updates `open_findings` /
on each unresolved finding's stored `comment_id` and `comment_kind`
from the findings ledger, whatever head that comment was posted on (a
carried finding keeps its original reaction target unless a re-review
places a newer inline comment). A missing inline ID is recovered only
when review head, anchor and finding identity match; otherwise a bounded
issue-comment lookup precedes the fallback POST, and an incomplete lookup
cannot cause a duplicate. These requests count against the separate
`DISPUTE_SCAN_REQUEST_BUDGET`; the existing daily feedback scan is too slow
Comment on lines +387 to +388

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Define how the scan budget fits the cycle cap

Charging these reads to a separate DISPUTE_SCAN_REQUEST_BUDGET contradicts the reconciler contract at lines 511–514, which says every cycle is bounded by PR_STATE_REQUEST_BUDGET reads. The new budget is not otherwise defined with a default or an aggregate relationship to that cap, so on PRs with many unresolved findings an implementation following this paragraph can exceed the documented per-cycle request limit. Specify the new budget and update the overall cycle-cap contract to account for it.

Useful? React with 👍 / 👎.

for this and is left as is. Reaction evidence updates `open_findings` /
`disputed_findings` on the `pr_state` row locally, with no model call, so
the projection can move to `ready_for_maintainer` in the same cycle once
nothing blocking remains. Because disputed findings stay in the scan set,
a 👎 removed later re-opens the finding on the next scan and the PR
leaves ready — the page already sent is not retracted, and a later return
to ready advances `ready_entry_seq` and pages again. Reactions and
replies from anyone other than the PR author are ignored for this
to ready advances `ready_entry_seq` and pages again. Reactions from
anyone other than the PR author are ignored for this
purpose.

The quality-readiness verdict (`ready` / `concerns` / `needs_rework`) is
Expand Down Expand Up @@ -689,7 +696,7 @@ blocked, timed out, rerouted all land in a column).
| --- | --- |
| projection, reconciler, sweep, clocks, our labels, notices, dashboard | `omni-reviewbot` |
| review verdict, published `findings`, explicit `finding_rechecks`, quality verdict and head binding | this repo, via the SDK (`build_review_result`, `build_quality_result`) |
| `open_findings` / `disputed_findings` counts | the bot, because they combine SDK per-finding results with reaction and reply data only the bot reads |
| `open_findings` / `disputed_findings` counts | the bot, because they combine SDK per-finding results with author-reaction data only the bot reads |

### SDK boundary

Expand Down Expand Up @@ -993,6 +1000,12 @@ cooldown; production deployment and feature activation remain pending.
finding's original comment from an older head is found after a push
because the scan follows the stored comment
id.
- A body-only blocker has a separate, author-reactable PR comment. A lost
inline-ID read recovers only the same finding on the same reviewed head;
a different finding at the same anchor cannot clear this blocker. A lost
fallback POST response is adopted without duplicate delivery, and an
incomplete marker search never posts again. Ordinary replies do not
silently count as disputes.
- Contract tests in this repo: Direct and Strict carry typed finding IDs
and source heads; rechecks carry the current head, an explicit outcome
and evidence. Cover complete `fixed`, `still_affected`, `unverified`,
Expand Down
Loading