Findings an author declined are raised again on every later push #28

Open
opened 2026-10-04 04:38:33 +00:00 by jercik · 0 comments
Owner

Findings that an author declined come back on every later push. Nothing records the decline, so the next review raises the same claim again.

Evidence

Measured on 2026-10-03 across 135 runs of the restated-sets lens on 92 PRs:

  • 107 of 284 claims (38%) repeat a claim from an earlier push of the same PR, at the same path and snippet.
  • The wrapper's dispositions table has 0 rows.
  • align#307 went four rounds on one section.
  • Pointers written to fix one finding were flagged by the next one (claims 01M41J7PKRSR796Z28F2HGESPY and 01M41HHJ3GFA1JQS8XQXV0ADC4).

Where dispositions are reconciled today

  • src/reconcile/dispositions.ts only projects dispositions that already exist in the service onto PR threads as replies and resolutions. Its header says it never writes dispositions and imports neither the service session nor the credentials.
  • src/wrapper/orchestrator.ts calls deps.dispositions.reconcile last, with the claims it walked from the review session. The DispositionProjector contract is in src/contract/types.ts.
  • src/reconcile/conversation-markers.ts builds the reply body and the review:disposition: marker (src/review/constants.ts). Nothing reads an author's reply as input.
  • Groupings and verdicts live within one review, so nothing carries over to the next push.

Proposed shape

  1. Record each author decline with its file, the anchored snippet, and the reason.
  2. On the next push, give the recorded declines to the lenses and to triage.
  3. Don't re-raise a declined finding unless its text changed.

Open questions

  • How to match "the same finding" after nearby edits move or reword the anchor.
  • When a decline expires.
  • Where matching should happen: wrapper, service, or prompts.
  • Whether agreed-but-deferred findings should be treated the same way.
Findings that an author declined come back on every later push. Nothing records the decline, so the next review raises the same claim again. ## Evidence Measured on 2026-10-03 across 135 runs of the `restated-sets` lens on 92 PRs: - 107 of 284 claims (38%) repeat a claim from an earlier push of the same PR, at the same path and snippet. - The wrapper's `dispositions` table has 0 rows. - align#307 went four rounds on one section. - Pointers written to fix one finding were flagged by the next one (claims 01M41J7PKRSR796Z28F2HGESPY and 01M41HHJ3GFA1JQS8XQXV0ADC4). ## Where dispositions are reconciled today - `src/reconcile/dispositions.ts` only projects dispositions that already exist in the service onto PR threads as replies and resolutions. Its header says it never writes dispositions and imports neither the service session nor the credentials. - `src/wrapper/orchestrator.ts` calls `deps.dispositions.reconcile` last, with the claims it walked from the review session. The `DispositionProjector` contract is in `src/contract/types.ts`. - `src/reconcile/conversation-markers.ts` builds the reply body and the `review:disposition:` marker (`src/review/constants.ts`). Nothing reads an author's reply as input. - Groupings and verdicts live within one review, so nothing carries over to the next push. ## Proposed shape 1. Record each author decline with its file, the anchored snippet, and the reason. 2. On the next push, give the recorded declines to the lenses and to triage. 3. Don't re-raise a declined finding unless its text changed. ## Open questions - How to match "the same finding" after nearby edits move or reword the anchor. - When a decline expires. - Where matching should happen: wrapper, service, or prompts. - Whether agreed-but-deferred findings should be treated the same way.
Sign in to join this conversation.
No labels
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
j4k-oss/review-wrapper#28
No description provided.