Track whether review comments have been addressed #4

Closed
opened 2026-08-09 16:27:41 +00:00 by a73x · 1 comment
a73x commented 2026-08-09 16:27:41 +00:00 (Migrated from git.a73x.sh)

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

Inline comments are anchored to their revision (state.rs:197) so they never drift, but nothing records whether one was ever answered. With five threads across four revisions a reviewer re-reads all five each round to find the two that still matter.

Blocker: inline comments have no identity. Comment carries commit_id (state.rs:83); InlineComment does not (state.rs:190-198). No migration needed - state is derived by walking the DAG, so the event commit OID is in hand when the comment is applied and is simply discarded.

Adds PatchCommentResolve { comment, revision } and PatchCommentReopen { comment } as ordinary events, so existing (clock, oid) ordering handles concurrency and disagreement stays in the record with attribution.

Anyone may resolve; the distinction between an author's claim and a reviewer's confirmation is displayed, not enforced.

Recording a new revision never auto-resolves or auto-reopens; where a later revision touches a resolved comment's lines that is surfaced as a hint.

Also adds 'patch diff --answers ', resolving to the interdiff from the comment's revision to its resolution's revision, scoped to the comment's file. Depends on the rebase-aware interdiff issue.


Migrated from git-collab issue 5e7d36ddfdf8e9ea520895b0728e59681ff611f9.

Original author: a73x dev@a73x.sh.

Close reason: merged in patch 28fc6751

Spec: docs/superpowers/specs/2026-08-09-comment-resolution-design.md Inline comments are anchored to their revision (state.rs:197) so they never drift, but nothing records whether one was ever answered. With five threads across four revisions a reviewer re-reads all five each round to find the two that still matter. Blocker: inline comments have no identity. Comment carries commit_id (state.rs:83); InlineComment does not (state.rs:190-198). No migration needed - state is derived by walking the DAG, so the event commit OID is in hand when the comment is applied and is simply discarded. Adds PatchCommentResolve { comment, revision } and PatchCommentReopen { comment } as ordinary events, so existing (clock, oid) ordering handles concurrency and disagreement stays in the record with attribution. Anyone may resolve; the distinction between an author's claim and a reviewer's confirmation is displayed, not enforced. Recording a new revision never auto-resolves or auto-reopens; where a later revision touches a resolved comment's lines that is surfaced as a hint. Also adds 'patch diff <id> --answers <comment>', resolving to the interdiff from the comment's revision to its resolution's revision, scoped to the comment's file. Depends on the rebase-aware interdiff issue. --- Migrated from git-collab issue `5e7d36ddfdf8e9ea520895b0728e59681ff611f9`. Original author: a73x <dev@a73x.sh>. Close reason: merged in patch 28fc6751
a73x commented 2026-08-09 18:00:30 +00:00 (Migrated from git.a73x.sh)

Confirmed from a real review that the missing comment identity is the blocker, exactly as this spec says.

A reviewer working entirely through the CLI could not: reply to a comment, mark one resolved, see which were outstanding, or delete a probe comment left by mistake. InlineComment carries author, file, line, body, timestamp, revision and nothing else (state.rs:190-198).

Three separate issues now all reduce to the same missing field - this one, e804b3e4 (comments cannot be edited or deleted) and threading. Carrying the event commit OID into the derived comment, as this spec proposes, unblocks all three at once and costs nothing, since state is derived by walking the DAG and the OID is already in hand.

Also wanted during the same review and worth folding in here: commenting on a line RANGE rather than a single line. Several review points spanned a hunk and had to be anchored to one line with the range described in prose. InlineComment.line is a single u32.

Confirmed from a real review that the missing comment identity is the blocker, exactly as this spec says. A reviewer working entirely through the CLI could not: reply to a comment, mark one resolved, see which were outstanding, or delete a probe comment left by mistake. InlineComment carries author, file, line, body, timestamp, revision and nothing else (state.rs:190-198). Three separate issues now all reduce to the same missing field - this one, e804b3e4 (comments cannot be edited or deleted) and threading. Carrying the event commit OID into the derived comment, as this spec proposes, unblocks all three at once and costs nothing, since state is derived by walking the DAG and the OID is already in hand. Also wanted during the same review and worth folding in here: commenting on a line RANGE rather than a single line. Several review points spanned a hunk and had to be anchored to one line with the range described in prose. InlineComment.line is a single u32.
Sign in to join this conversation.
No milestone
No project
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
a73x/git-collab#4
No description provided.