ums: diff-range selection, and verifying a reviewer's proposed fix

[d-morrison]

UMS from driving d-morrison/gha#318 (the claude-code-review heredoc-unwrap fix) through three review rounds to merge. Two learnings, both from mistakes or near-misses in that session.

1. memories/git.md — picking the diff range when self-checking a PR

New section. Both wrong choices hit in the same session:

• git diff origin/main..HEAD (two dots) compares tips, so a branch that is behind main renders main's newer commits as deletions. On docs(memories): migrate local-only general memories into ai-config #318 this made a sibling PR's changelog fragment look deleted by the PR under review — it was one step from being reported to the user as a finding, until ... showed the real four-file diff.
• git diff origin/main...HEAD (three dots) sees only committed work. A non-ASCII/em-dash self-check run this way printed clean while the em-dashes sat uncommitted in the working tree.

The section names what each of the three forms actually answers, and adds the worktree-comparing git diff origin/main as a second fix for the by-hand case — the existing "diff-scoped check no-ops on an uncommitted diff" section's fix is "commit first," which isn't always what you want mid-edit. It cross-references that section rather than restating it, and notes the trap that merging main in makes all three forms agree, which is exactly when it stops feeling worth thinking about.

2. shared/workflow/address-every-comment.md — verify a proposed fix, not just a suggestion block

The existing bullet covers a wrong literal inside a ```suggestion block. This extends it to a fix a reviewer describes in prose, where the sharpest test is the reviewer's own example: a finding that ships a repro case has handed you a test fixture, so run the proposed fix against that case before adopting it.

On #318 the review correctly found that a heredoc-terminator regex lacked an end-of-line anchor and suggested adding one. Tested against the reviewer's own indented-EOF example, the suggested anchor still truncated the body — the terminator's leading [ \t]* accepted a space-indented closing line real bash rejects. Adopting it verbatim would have converted a partial diagnosis into a shipped bug while the thread read as settled. The fix that shipped matches whole lines against the tag, the way bash itself ends a heredoc, removing the failure mode rather than narrowing it.

Checks

check-new-line-breaks (diff-scoped, run after committing), check-links, validate-skills, check-memory-file-size, check-vendored-drift all clean; sync-codex-skill-wrappers.py regenerates no changes. No non-ASCII punctuation in any added line.

---

Generated by Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @d-morrison's task in 2m 20s —— View job

---

Review

• Read changed files
• Review memories/git.md additions
• Review shared/workflow/address-every-comment.md additions
• Check for hallucinations and factual accuracy
• Post findings

---

Verdict: Ready for merge

No findings. Both additions are factually accurate, internally consistent, and follow the repo's conventions.

memories/git.md — diff-range selection section

The three-form description is technically correct:

• git diff origin/main..HEAD (two dots) = tip comparison: changes from origin/main to HEAD, so main's newer commits (files in origin/main but not in HEAD) appear as deletions. ✓
• git diff origin/main...HEAD (three dots) = merge-base comparison: exactly what GitHub's PR diff shows. ✓
• git diff origin/main (no second ref) = working-tree comparison against origin/main, including both staged and unstaged uncommitted edits. ✓
• The closing observation — that after merging origin/main into the branch all three forms agree on committed content — is correct (the merge base becomes origin/main, so .. and ... collapse to the same comparison). ✓
• The cross-reference to "the diff-scoped-no-op section above" correctly points to the check-new-line-breaks false-clean entry immediately preceding this section. ✓

shared/workflow/address-every-comment.md — prose-described fix verification

The bash heredoc termination claim ("space-indented closing line real bash rejects") is accurate: bash terminates a heredoc only on a line that exactly matches the delimiter (<<- strips leading tabs only, not spaces), so a [ \t]*-prefixed regex accepting space-indented lines is a real bug. ✓

The fix principle ("matching whole lines against the tag — how bash itself ends a heredoc") is correct and eliminates the failure mode cleanly. ✓

The new paragraph appropriately generalizes the existing suggestion-block guidance without duplicating it, and the motivating example is internally consistent with gha#318's description in the PR body.

No em-dashes, curly quotes, or other non-ASCII punctuation found. Semantic line breaks look clean.

Copilot declined to review (quota exhausted); verdict is on this review alone.

[d-morrison]

Clean on the first round — no findings, so nothing to Address, Rebut, or Defer.

Confirming the state after CI finished, since the verdict posted about a minute before its own gate job did (require-review still read queued at verdict time):

• All 7 check runs completed, every one success — validate, check-new-line-breaks, claude-review, require-review, gather-context.
• No inline review threads — the review was a single top-level comment, so the only open conversation is this all-clear exchange.
• main has not moved since the branch was cut; re-checked with git merge-tree against the current merge base — zero conflicts.

One caveat worth stating rather than papering over, per fully-clean.md's fifth case: Copilot declined to review for quota exhaustion, so this verdict rests on claude-review alone. That's a reviewer that never ran, not a second clean opinion.

Ready for merge — leaving the merge to you.

---

Generated by Claude Code