Keep comments written while reading one commit - #19
Merged
Merged
Conversation
Reading a change one commit at a time keys its own state file, so that having read a file in one commit says nothing about the next commit's change to it. Drafts were in that file too - and a draft is not the scope's. It is a remark about the pull request, posted against a path and a line of it, and there is one pull request to post it to however the reader chose to walk the change. So a comment written while reading one commit went into that commit's file. Leaving the scope swapped the file back and the remark was gone from the pane; worse, submitting leaves the scope before it posts, so such a draft was never even offered to the host. The reader had written something, watched it be accepted, and it left no trace they could find. The review's own file now stays open behind a scope, and the drafts go to it. What the scope keys is what the scope decides - the viewed flags - which a test pins, because that part was right. Claude-Session: https://claude.ai/code/session_01EgJ3xXgcismuaFmKhF6KD9 Assisted-by: Claude:claude-opus-5:Claude Code
A draft hangs on a line of whatever revision was being read when it was written, and it is found again by content rather than by that number - which is what carries it across a force-push. Re-anchoring ran only when a review opened, and a scope changes the revision without opening anything. So a draft written against one commit kept that commit's line number after the scope was left. Submitting checks the number against the lines the whole change offers a comment on: at best the draft was reported outdated and kept back, at worst the number was commentable there too and the remark went to whatever text now sits at it. Re-anchored wherever a scope changes what is being read, which is the one place all three ways of changing it pass through. Awaited rather than started, because leaving a scope is the last thing a submission does before it posts. Claude-Session: https://claude.ai/code/session_01EgJ3xXgcismuaFmKhF6KD9 Assisted-by: Claude:claude-opus-5:Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A comment drafted in commit-by-commit mode was lost. Two causes, one commit
each.
Where the draft went
Reading a change one commit at a time keys its own state file, so that having
read a file in one commit says nothing about the next commit's change to it.
Drafts were in that file too — and a draft is not the scope's. It is a remark
about the pull request, posted against a path and a line of it, and there is
one pull request to post it to however the reader chose to walk the change.
So a comment written while reading one commit went into that commit's file.
Leaving the scope swapped the file back and the remark vanished from the pane.
Worse: submitting leaves the scope before it posts, so such a draft was
never even offered to the host. The reader had written something, watched it
be accepted, and it left no trace they could find.
The review's own file now stays open behind a scope and the drafts go to it.
Viewed flags stay the scope's — that part was right, and a test now pins it.
Which line it hung on
A draft is found again by content rather than by its line number, which is
what carries it across a force-push. That re-anchoring ran only when a review
opened, and a scope changes the revision on screen without opening anything.
So a draft written against one commit kept that commit's line number once the
scope was left. Submitting checks that number against the lines the whole
change will take a comment on: at best the draft was reported outdated and
kept back, at worst the number was commentable there too and the remark
would have gone to whatever text now sits at it. Re-anchoring now happens
wherever a scope changes what is being read, awaited rather than started,
because leaving a scope is the last thing a submission does before posting.
Testing
Five new tests, written red first — the storage half failed exactly as
reported ("Expected: < 'about the whole change', 'about this commit' > But
was: < 'about this commit' >"). 347 tests, 346 passing, 1 skipped.
Also driven end to end against a real pull request: drafted a comment inside
commit scope, confirmed it landed in the review's state file and not the
commit's, left the scope, and saw it still anchored at the right line with the
file's comment badge. The test draft was removed afterwards; nothing was
submitted.
🤖 Generated with Claude Code