Skip to content

RFC: ADR-005 — Feedback loop: PR reviews, issue revision, and ADR evolution #136

@scottschreckengaust

Description

@scottschreckengaust

Summary

Define the protocol for how PR review feedback propagates back to issues, ADRs, and the roadmap. Review comments are not terminal events — they are signals that may require reissuing work, revising issue scope, cancelling approaches, or evolving architectural decisions.

Use case and motivation

Problem today:

  1. PR review comments are addressed locally (fix the code) but systemic issues they surface are not propagated upstream to the issue or ADR.
  2. A reviewer says "this approach is wrong" — but the issue still says "use this approach." The issue body rots.
  3. ADRs are written as immutable truths but the world changes. No protocol exists for challenging or superseding them based on implementation experience.
  4. Review feedback that reveals a design flaw causes rework across multiple PRs in a stack — but no mechanism exists to pause the stack, revise the design, and restart.

After this ADR:

  • PR reviews trigger explicit upstream checks: "Does this feedback change the issue? The ADR? The roadmap item?"
  • Issues are living documents that incorporate implementation learnings
  • ADRs have a defined challenge and supersession protocol
  • Stacked PR chains can be paused and revised when foundations shift

Proposed protocol

1. Review comment classification

Every non-trivial review comment falls into one of:

Type Action Propagates to
Nit (style, naming) Fix in PR Nothing
Bug (logic error) Fix in PR Nothing (unless systemic)
Design concern Pause PR; evaluate Issue body (revise approach?)
Architecture challenge Pause PR; escalate ADR (supersede? amend?)
Scope question Clarify Issue body (narrow/expand scope?)
Blocker (won't approve as-is) Pause PR Issue body (document the blocker)

2. Upstream propagation rules

When a PR review surfaces a design concern or architecture challenge:

  1. Pause — Do not force-merge or dismiss. Do not continue stacked PRs above this one.
  2. Assess — Does this feedback invalidate the issue's approach? The ADR's decision?
  3. Propagate — Update the relevant upstream document:
    • Issue body: add an **UNRESOLVED:** <reviewer concern> section
    • ADR: if the decision is challenged, open a discussion (comment on the ADR's implementing issue)
    • Stacked PRs: comment on all dependent PRs that the base is under revision
  4. Resolve — Either:
    • Revise the approach (update issue body, potentially new ADR)
    • Defend the approach (document why the reviewer's concern doesn't apply)
    • Cancel the work (close the PR, update the issue as cancelled with rationale)
  5. Resume — Once resolved, unblock the PR and dependents

3. ADR evolution protocol

ADRs are decisions, not scripture. They evolve through:

Trigger Response
Implementation reveals the decision doesn't work File a new RFC proposing a successor ADR
Reviewer challenges the architectural premise Add an **UNRESOLVED** to the issue; pause implementation
New information makes the decision obsolete Write successor ADR with Supersedes: ADR-NNN
Decision works but needs refinement Amend via PR (minor — no new ADR needed)

Never silently ignore a challenged decision. Either defend it (with evidence) or supersede it.

4. Issue lifecycle responses to PR feedback

Review outcome Issue body update
PR merged successfully No change needed (acceptance criteria met)
PR requires approach change Revise the "Proposal" section; document why
PR abandoned / cancelled Mark issue as needs re-evaluation; document what was learned
PR reveals missing predecessor Add dependency to issue body; potentially block as **UNRESOLVED**
PR review suggests scope expansion Create sub-issue; don't expand current scope
PR review suggests scope reduction Document what's explicitly removed and why

5. Stacked PR chain revision

When feedback on PR N invalidates PRs N+1 through N+M:

  1. Comment on all affected PRs: "Base is under revision due to feedback on #N"
  2. Do not rebase dependent PRs until the base is stable
  3. If the change is architectural: re-evaluate whether the remaining stack is still valid
  4. If the remaining stack needs redesign: close dependent PRs, revise issue body, re-plan

Open questions

  1. Should there be a time limit for resolving **UNRESOLVED** items before auto-unassigning?
  2. How do we handle reviewer disagreement (two reviewers want different approaches)?
  3. Should ADR amendments require the same admin approval as new ADRs?

Alignment

Aligns with Scale and collaboration — concurrent agents producing PRs that get reviewed need a protocol for incorporating feedback without losing systemic insights.

Dependencies


  • RFC PR: (pending approval)
  • Approved by: (pending)
  • Reviewed by: (pending)

Metadata

Metadata

Assignees

Labels

RFC-proposalRequest for Comments: design proposaldocumentationImprovements or additions to documentation

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions