Skip to content

ums: re-audit after programmatic comment edits; sibling-merge clobber hazard - #126

Merged
d-morrison merged 8 commits into
mainfrom
ums-comment-edit-artifacts
Jun 22, 2026
Merged

ums: re-audit after programmatic comment edits; sibling-merge clobber hazard#126
d-morrison merged 8 commits into
mainfrom
ums-comment-edit-artifacts

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Banks two recurring lessons from the sparta #143/#150/#152 arc into memories/debugging.md.

What

  1. Programmatic comment edits leave punctuation/grammar artifacts. Scripted removal of issue refs (and parentheticals/clauses) from comments repeatedly broke the surrounding prose — orphaned commas/periods, comment-marker-plus-bare-punctuation (##.), dangling "see issue.", an exposed repeated word. The reviewer/Copilot flagged ~6 of these across #150 and #152. The note lists the failure shapes, the fix for each, and the audit greps — plus which sed patterns strip cleanly vs which cases need hand edits.

  2. Merging main into a sibling PR can silently clobber an un-customized file. When one PR merges and you sync a sibling that touches the same files, a file the sibling never customized takes main's version with no conflict — so it can end up describing the wrong change. Hit on ums: add rme/Epi 204 working-notes memory #152, where demos/demo.json silently became Genericize incidental project-specific references in skills #143's diplomacy reason. The note says to diff the whole merge result vs the PR's intent (not just resolve marked conflicts) and re-run the PR's own invariant over files main re-touched.

Why

Both bit this session more than once and weren't recorded anywhere; encoding them should pre-empt the same review rounds next time.

🤖 Generated with Claude Code


Generated by Claude Code

… hazard

Lessons from the sparta #143/#150/#152 arc: scripted removal of issue refs from
comments leaves orphaned punctuation/grammar (re-audit before pushing), and
merging main into a sibling PR can silently clobber an un-customized file.
Copilot AI review requested due to automatic review settings June 22, 2026 03:49
@github-actions
github-actions Bot removed the request for review from Copilot June 22, 2026 03:49
@claude

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates memories/debugging.md to capture two recurring review/debugging lessons from the sparta scrub PRs: (1) scripted comment edits can leave behind punctuation/grammar artifacts, and (2) merging main into a sibling PR can silently overwrite (“clobber”) files that weren’t actually customized in the sibling.

Changes:

  • Adds a checklist of common punctuation/grammar failure modes after programmatic comment scrubs, with suggested audits/greps.
  • Adds guidance on verifying merge results when syncing a sibling PR after another PR lands, to catch silent “clean merges” that change PR-specific files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread memories/debugging.md Outdated
Comment thread memories/debugging.md Outdated
Comment thread memories/debugging.md Outdated
Comment thread memories/debugging.md Outdated
Comment thread memories/debugging.md Outdated
Comment thread memories/debugging.md Outdated
…larify example

- Fully-qualify Lacaedemon/sparta#143/#150/#152 (ambiguous as bare refs in ai-config).
- Audit grep used \d (PCRE) — switch to [0-9] for grep -E; note sed -E for the strip patterns.
- Clarify the orphaned-comma example as the two-line continuation case.
- 'a file that B never customized' (grammar).
@claude

This comment has been minimized.

Comment thread memories/debugging.md Outdated
The strip patterns need -E only for the + quantifier; \( \) match literal
parens in ERE. Drop the inaccurate 'and groups' claim.
@claude

This comment has been minimized.

Comment thread memories/debugging.md Outdated
s/^# #[0-9]+: /# / expresses 'strip prefix only' precisely; the ^ anchor makes
the g flag unnecessary.
@claude

This comment has been minimized.

claude added 2 commits June 22, 2026 04:41
From Lacaedemon/sparta#150 — a second driver rebased/force-pushed the branch
three times. Capture the fetch-before-push check, reset-and-verify response,
stand-down-when-origin-is-correct rule, and escalate-on-repeat guidance.
@claude

This comment has been minimized.

…ent'

The check is whether your HEAD is still origin's HEAD (in sync), not a
comparison to your commit's parent.
@claude

This comment has been minimized.

Comment thread memories/debugging.md Outdated
rev-parse HEAD vs origin always differs when you have unpushed local commits;
git log HEAD..origin/<branch> (non-empty = origin moved past you) handles that
case cleanly.
@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

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


Review complete

  • Gather context
  • Read diff vs origin/main
  • Review current state of memories/debugging.md
  • Check for correctness, AI tells, hallucinations
  • Post findings

No new findings. All prior inline findings have been addressed, and the latest commit (62fc436) correctly resolves the rev-parse HEAD imprecision flagged in the previous round.

Technical verification:

  • grep -rnE "^[[:space:]]*#+[[:space:]]*[.,;:]" — Valid ERE; POSIX char classes work with -E; the pattern correctly fires on lines like # , or ##.. ✓
  • s/ \(#[0-9]+\)//g — In sed -E, \( and \) match literal parens (ERE groups use unescaped ()), so this cleanly strips (#NNN). ✓
  • s/^# #[0-9]+: /# /^-anchored, no g. Correct. ✓
  • s/, #[0-9]+,/,/g — Strips ref between commas, leaving one comma. Correct. ✓
  • git log --oneline HEAD..origin/<branch> — Lists commits reachable from origin/<branch> but not from HEAD. Empty when origin is where you left it (regardless of unpushed local commits); non-empty when a parallel push moved origin past you. The parenthetical correctly explains why the bare rev-parse equality check fails in the "have local commits" state. ✓

Cross-repo refs: All three sparta references use fully-qualified form (Lacaedemon/sparta#143, Lacaedemon/sparta#150, Lacaedemon/sparta#152). ✓

AI tells: None. Prose is specific, concrete, and direct throughout. ✓

Hallucinations: None. GitHub's "Update with rebase" button is real; all git commands describe accurate behavior. ✓

This PR is clean and ready to merge.

@d-morrison
d-morrison merged commit 8b9c205 into main Jun 22, 2026
5 checks passed
@d-morrison
d-morrison deleted the ums-comment-edit-artifacts branch June 22, 2026 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants