review: Remap drafts to remote change head

.agents/docs/cli.md

@@ -74,6 +74,61 @@ Rendering belongs at the command or handler boundary.
 Domain operations should not need to know
 whether a result will be printed as text, JSON, or another representation.
 
+## User-Facing Logs
+
+Log messages are part of the standard-error interface.
+Write them for a user following the operation,
+not as an internal trace of the implementation.
+
+We use two forms of logging based on purpose:
+structured logs and printf-style logs.
+
+### Structured Logs
+
+Structured logs use full sentences (capitalized, punctuated, and complete)
+with structured attributes for dynamic values.
+Use these when reporting a single event, or for debug-level logs.
+
+```go
+log.Warn(
+	"Could not load remote change data. Using local data.",
+	"changeID", changeID,
+	"error", err,
+)
+```
+
+Structured log attributes get user-friendly camelCase keys.
+Errors use the `"error"` key.
+When an error is recoverable,
+state the fallback or consequence in the message.
+
+### printf-style Logs
+
+printf-style logs use a lowercase prefix for the subject of the message,
+then a colon and a lowercase statement of the action or result.
+Use these when repeated messages report progress for individual items.
+
+```go
+log.Infof("%v: restacked onto %v", branch, onto)
+log.Infof("%v: anchor moved to %v", draft.ID, anchor)
+```
+
+Exception: Debug level logs always use structured logging.
+
+#### printf formatting verbs
+
+Use `%v` when Go's default formatting is the intended presentation,
+including for strings and integers.
+Use another formatting verb only when its distinct presentation
+is part of the output contract,
+such as quoting, a non-decimal base, padding, or precision.
+Use `%q` for quoted strings.
+
+Use `WithPrefix` for stable subsystem context,
+such as a forge or a long-running operation.
+Keep dynamic branch, change, draft, and other item identities
+in the message or structured attributes.
+
 ## Generated Files
 
 Run `mise run generate`

.agents/docs/style.md

@@ -116,6 +116,8 @@ Use `make` for slices only when specifying length or capacity.
 ## Logging
 
 Use `internal/silog.Logger`.
+For user-facing message content and formatting,
+follow the logging conventions in `.agents/docs/cli.md`.
 In tests, use `silogtest.New(t)` by default
 so log output is attached to the test.
 Use `silog.Nop()` only when the test needs to suppress logging entirely.

AGENTS.md

@@ -9,7 +9,7 @@ Read the relevant guide before editing that kind of code or prose:
 | Package boundaries, dependencies, constructors, APIs | `.agents/docs/design.md` |
 | Go implementation style and symbol ordering | `.agents/docs/style.md` |
 | Code comments and symbol documentation | `.agents/docs/comments.md` |
-| Command behavior, flags, output, generated CLI docs | `.agents/docs/cli.md` |
+| Command behavior, flags, user-facing logs and output, generated CLI docs | `.agents/docs/cli.md` |
 | Unit tests, test scripts, mocks, regression tests | `.agents/docs/testing.md` |
 | Documentation, changelog, release-facing prose | `.agents/docs/docs-and-release.md` |
 | Branches, commits, stacks, PR publishing boundaries | `.agents/docs/git-workflow.md` |

internal/git/diff_wt.go

@@ -31,7 +31,33 @@ func (w *Worktree) OpenBranchDiff(
 	ctx context.Context,
 	base, head string,
 ) (io.ReadCloser, error) {
-	cmd := w.gitCmd(ctx, "diff", base+"..."+head)
+	return w.openDiff(ctx, base+"..."+head)
+}
+
+// OpenCommitDiff starts a unified diff that maps the first commit's tree to
+// the second commit's tree. Rename detection is enabled and hunk context is
+// omitted because callers use the result to map changed lines.
+//
+// The caller must close the returned reader to wait for Git and receive its
+// exit status.
+func (w *Worktree) OpenCommitDiff(
+	ctx context.Context,
+	from, to string,
+) (io.ReadCloser, error) {
+	return w.openDiff(
+		ctx,
+		"--find-renames",
+		"--unified=0",
+		from,
+		to,
+	)
+}
+
+func (w *Worktree) openDiff(
+	ctx context.Context,
+	args ...string,
+) (io.ReadCloser, error) {
+	cmd := w.gitCmd(ctx, "diff", args...)
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
 		return nil, fmt.Errorf("pipe stdout: %w", err)
@@ -43,20 +69,20 @@ func (w *Worktree) OpenBranchDiff(
 		)
 	}
 
-	return &branchDiffReader{
+	return &diffReader{
 		ReadCloser: stdout,
 		cmd:        cmd,
 	}, nil
 }
 
-// branchDiffReader waits for the Git process after closing its stdout pipe.
-type branchDiffReader struct {
+// diffReader waits for the Git process after closing its stdout pipe.
+type diffReader struct {
 	io.ReadCloser
 	cmd *gitCmd
 }
 
 // Close releases the pipe and reports the Git process exit status.
-func (r *branchDiffReader) Close() error {
+func (r *diffReader) Close() error {
 	closeErr := r.ReadCloser.Close()
 	waitErr := r.cmd.Wait()
 	if waitErr != nil {

internal/git/diff_wt_test.go

@@ -72,3 +72,30 @@ func TestWorktree_OpenBranchDiff(t *testing.T) {
 		assert.ErrorContains(t, diff.Close(), "diff: git command failed")
 	})
 }
+
+func TestWorktree_OpenCommitDiff(t *testing.T) {
+	t.Parallel()
+
+	mockExecer := git.NewMockExecer(gomock.NewController(t))
+	_, wt := git.NewFakeRepository(t, "", mockExecer)
+
+	mockExecer.EXPECT().
+		Start(gomock.Any()).
+		DoAndReturn(func(cmd *exec.Cmd) error {
+			assert.Equal(t, []string{
+				"git", "diff", "--find-renames", "--unified=0", "old", "new",
+			}, cmd.Args)
+			_, err := io.WriteString(cmd.Stdout, "diff output\n")
+			return errors.Join(err, cmd.Stdout.(io.Closer).Close())
+		})
+	mockExecer.EXPECT().
+		Wait(gomock.Any()).
+		Return(nil)
+
+	diff, err := wt.OpenCommitDiff(t.Context(), "old", "new")
+	require.NoError(t, err)
+	got, err := io.ReadAll(diff)
+	require.NoError(t, err)
+	require.NoError(t, diff.Close())
+	assert.Equal(t, "diff output\n", string(got))
+}

internal/handler/review/comment.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"strings"
 
 	"go.abhg.dev/gs/internal/forge"
@@ -29,14 +30,28 @@ func (h *DraftHandler) SaveCommentDraft(
 	ctx context.Context,
 	req *CommentRequest,
 ) error {
+	// Capture the anchor's source revision before the editor can leave this
+	// process waiting while another process advances the branch.
+	branch, err := h.Service.LookupBranch(ctx, req.Branch)
+	if err != nil {
+		if errors.Is(err, state.ErrNotExist) {
+			return fmt.Errorf("branch not tracked: %s", req.Branch)
+		}
+		return fmt.Errorf("get branch: %w", err)
+	}
 	body, err := h.commentBody(ctx, req.Message)
 	if err != nil {
 		return err
 	}
 	draft, err := h.Store.AddReviewDraft(
 		ctx,
 		req.Branch,
-		review.Draft{ID: 0, Body: body, Anchor: req.Anchor},
+		review.Draft{
+			ID:         0,
+			Body:       body,
+			Anchor:     req.Anchor,
+			CommitHash: branch.Head,
+		},
 	)
 	if err != nil {
 		return fmt.Errorf("save draft comment: %w", err)
@@ -239,20 +254,30 @@ func (h *DraftHandler) commentBody(
 	return body, nil
 }
 
-// loadPatch parses the selected branch's review diff.
+// loadPatch parses the review diff ending at head.
 // Closing the diff reader also reports failures from the Git process.
 func (h *Handler) loadPatch(
 	ctx context.Context,
-	base, branch string,
+	base, head string,
 ) (*reviewdiff.Patch, error) {
-	diff, err := h.Worktree.OpenBranchDiff(ctx, base, branch)
+	diff, err := h.Worktree.OpenBranchDiff(ctx, base, head)
 	if err != nil {
 		return nil, fmt.Errorf("open diff: %w", err)
 	}
+	patch, err := parsePatch(diff)
+	if err != nil {
+		return nil, fmt.Errorf("parse diff: %w", err)
+	}
+	return patch, nil
+}
+
+// parsePatch consumes and closes a Git diff. Closing the reader waits for the
+// Git process, so a command failure is reported together with any parse error.
+func parsePatch(diff io.ReadCloser) (*reviewdiff.Patch, error) {
 	patch, err := reviewdiff.Parse(diff)
 	err = errors.Join(err, diff.Close())
 	if err != nil {
-		return nil, fmt.Errorf("parse diff: %w", err)
+		return nil, err
 	}
 	return patch, nil
 }

internal/handler/review/handler.go

@@ -34,9 +34,10 @@ type Handler struct {
 
 // DraftHandler coordinates workflows that only access local drafts.
 type DraftHandler struct {
-	Log    *silog.Logger // required
-	Store  Store         // required
-	Editor CommentEditor // required
+	Log     *silog.Logger // required
+	Service Service       // required
+	Store   Store         // required
+	Editor  CommentEditor // required
 }
 
 // ThreadHandler coordinates review-thread resolution changes.
@@ -50,6 +51,8 @@ type ThreadHandler struct {
 // Worktree provides the Git operations used by review workflows.
 type Worktree interface {
 	OpenBranchDiff(context.Context, string, string) (io.ReadCloser, error)
+	OpenCommitDiff(context.Context, string, string) (io.ReadCloser, error)
+	PeelToCommit(context.Context, string) (git.Hash, error)
 }
 
 var _ Worktree = (*git.Worktree)(nil)