docs/superpowers/specs/2026-08-09-comment-resolution-design.md

Ref: main   Size: 6.1 KiB   History

# Comment Resolution — Design

Date: 2026-08-09
Status: Draft. Depends on `2026-08-09-patch-revision-refs-design.md`.

## Purpose

Review feedback is preserved today but not usable across rounds. An inline
comment is anchored to the revision it was written on (`state.rs:197`), so it
never drifts, but nothing records whether it was ever answered. With five threads
across four revisions, a reviewer re-reads all five every round to find the two
that still matter.

That is the same tax the branch workflow imposes, arriving through a different
door. Under a pull request the correspondence between feedback and the change
answering it is lost at the force-push, and authors reconstruct it with
`fix(pr): address review` commits that then pollute the history. The patch model
can keep the correspondence, and — because revisions are immutable and
interdiffs are rebase-aware — can *show* it rather than assert it.

## Blocker: inline comments have no identity

`Comment` carries `commit_id: Oid`, the OID of the event commit that created it
(`state.rs:83`). `InlineComment` does not (`state.rs:190-198`). There is
therefore no way to name the comment being resolved.

No migration is needed to fix this. State is derived by walking the event DAG, so
the event commit's OID is in hand when the comment is applied — it is simply
discarded. Carrying it through populates the field for every existing comment on
the next state rebuild.

Comments are addressed by OID prefix, like patch and issue ids, with the same
ambiguous-prefix error.

## Model

Two events:

```rust
PatchCommentResolve { comment: String, revision: Option<u32> }
PatchCommentReopen  { comment: String }
```

`revision` is the revision the resolution is claimed against, defaulting to the
patch's latest at the time the event is written.

Derived state gains a field:

```rust
pub struct InlineComment {
    // ...
    pub commit_id: Oid,
    pub resolved: Option<Resolution>,
}

pub struct Resolution {
    pub by: Author,
    pub revision: Option<u32>,
    pub timestamp: String,
}
```

### Who may resolve

Anyone. The event is signed and attribution is in the record, so the interesting
distinction is displayed rather than enforced: a resolution by the patch author
is a *claim*, one by the comment's author is a *confirmation*, and the UI says
which. Building permissions into this would add a rule with no threat behind it —
the parties are collaborators, and disagreement has a representation already.

Disagreement is `PatchCommentReopen`. Both events stay in the record with their
authors, so "the author said this was fixed and the reviewer disagreed" is
history rather than a lost state transition.

### Ordering

Resolve and reopen are ordinary events, ordered by the existing `(clock, oid)`
rule. Last writer wins per comment. Two people resolving concurrently converge on
the same answer on every machine, which is the same guarantee every other event
already has. No new conflict machinery.

### Revisions do not auto-resolve or auto-reopen

Recording a new revision does not change any comment's resolution state, even if
that revision touches the lines a resolved comment anchored to. Deriving state
transitions from graph observation would surprise both parties — the author
would see their claim silently revoked, and the reviewer would see threads
reopen without anyone reopening them.

Where a later revision does touch a resolved comment's lines, that is surfaced as
a *hint* alongside the thread, not a state change.

## Showing the answer

The feature that a pull request cannot offer:

```console
$ git-collab patch diff a1b2c3d4 --answers 7f3a9c2
```

Resolves to the interdiff from the comment's revision to its resolution's
revision, scoped to the file the comment anchored to. Because revision refs are
immutable and the interdiff is rebase-aware, this works after the author has
squashed, amended and rebased — the exact conditions under which GitHub has
nothing left to diff.

This is the dependency on the revision-refs design. Built against today's flat
tree-to-tree `interdiff` (`patch.rs:501-525`), the output would include every
upstream commit rebased over and would be worse than useless for answering a
one-line comment.

Where the comment carries no revision — data written before reviews were
revision-scoped — there is no from-side and the command reports that rather than
guessing.

## Surfaces

- `patch show`: unresolved threads first, resolved ones collapsed to one line
  naming who resolved them and at which revision.
- `patch log`: each revision annotated with how many comments it was claimed to
  resolve.
- `patch list` and the dashboard: unresolved count per patch, since "has
  unanswered feedback" is the state a reviewer filters on.
- Web UI: same grouping as `patch show`.

## Out of scope

- **Thread comments.** `Comment` has no revision (`state.rs:77-84`) and no anchor;
  it is conversation, not an actionable item. If it turns out reviewers use
  thread comments for actionable feedback, revisit — but do not build it on
  speculation.
- **Reviews.** A review already has a lifecycle: one current vote per author per
  revision, with a new vote superseding the old (`state.rs:633-642`). Reviews
  resolve themselves by being re-cast and need nothing here.
- Requiring all comments resolved before a patch may merge. Merge is detected
  from the graph, not gated (`state.rs:460`), and gating it would need an
  enforcement point that does not exist and that the design test would reject.

## Testing

- A comment resolved on r1 and a patch revised to r3 shows as resolved at r1.
- Reopening after resolving leaves both events in the record with their authors.
- Concurrent resolve and reopen from two clones converge identically on both.
- `--answers` on a comment resolved across a rebase shows only the author's
  changes, not upstream churn.
- `--answers` on a comment with no revision reports the absence rather than
  emitting a diff.
- Recording a new revision that touches a resolved comment's lines leaves it
  resolved, and surfaces the hint.
- Resolving by ambiguous OID prefix errors the same way patch and issue prefixes
  do.
- Inline comments written before this change gain their `commit_id` on rebuild,
  with no migration step.