Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions memories/github.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,17 @@ The GitHub MCP tool surface used in remote/web sessions lives in
So this needs no external `jq` and no `date -d`, which is GNU-only and absent on macOS.
(Morrison-Lab/ai-config#908, 2026-07-30: the `clean-worktrees` merged-PR guard shipped the string comparison.
Review caught it, and the repro above confirmed the failure direction before the fix went in.)
- **`gh pr list --state merged` plus a low `--limit` can miss recent merges:**
The list is ordered by PR list order, effectively number/creation, before your `--jq` filter runs.
That means an old, low-numbered PR that merged recently can sit below a page of higher-numbered PRs and never reach the filter.
The result looks scoped by time while silently excluding the very merge checkpoint you were polling for.
Use a query whose filter matches the question, such as `gh search prs --repo <owner>/<repo> --merged-at ">=<date>"`, or query each PR of interest directly.
If you use `gh pr list --state merged`, set `--limit` far beyond the expected count and report how many merged PRs the command examined, not only how many passed the `mergedAt` filter.
- **Do:** use `gh search prs --repo <owner>/<repo> --merged-at ">=<date>"`, direct `gh pr view <N>`, or an intentionally over-wide list with an examined count when answering "what merged since T".
- **Don't:** trust `gh pr list --state merged --limit N --json mergedAt --jq '.[] | select(.mergedAt > T)'` as a time-window query.
(Morrison-Lab/ai-config#969, 2026-08-01: `gh pr list --state merged --limit 15 --json number,mergedAt` plus a `mergedAt > 2026-08-01T08:00:00Z` filter returned only #1019, merged at `09:03:13Z`, and missed #969, merged at `09:14:38Z`.
Raw `--limit 6` output showed #1013 at `05:36Z` before #1012 at `05:45Z`, proving the page was not sorted by merge time.
Raising the limit to 30 returned both #1019 and #969.)
- **`gh pr edit` exits 1 on repos with Projects Classic — use `gh api` to update PR body.** `gh pr edit <N> --body "..."` / `--body-file <f>` returns exit code 1 with a GraphQL deprecation warning (`Projects (classic) is being deprecated…`). Sometimes the edit lands anyway; **sometimes it does not apply at all** (seen on sparta 2026-06-30: three `gh pr edit --body-file` attempts left the body unchanged with the `SHA_PLACEHOLDER` still in place). Either way, don't trust it — verify with `gh api repos/<o>/<r>/pulls/<N> --jq .body`, and just use the REST PATCH directly, which always exits 0 and applies: `gh api -X PATCH repos/<o>/<r>/pulls/<N> -f body="..."`. For a multi-line body, read it from a file with `-F body=@<path>` (capital `-F` to pull the field value from the file) rather than cramming it into `-f body="..."`.
- **PR description image embeds: use `raw.githubusercontent.com`, not `github.com/.../raw/...`.** Embedding a committed file in a PR body with `![](https://github.com/<owner>/<repo>/raw/<sha>/<path>)` may not render — the reviewer will flag it. The correct raw-content domain is `https://raw.githubusercontent.com/<owner>/<repo>/<sha>/<path>`. Reference the full commit SHA so the image keeps rendering after the branch is deleted on merge.
- **`raw.githubusercontent.com` FOLLOWS repository-rename redirects, so a `200` under the OLD owner proves nothing — only a `200` under the NEW owner is decisive.** To test whether a repo has moved, probe the *new* name and treat `404` there as "did not move". Run a known-moved repo as a control first, or the probe silently answers backwards: `d-morrison/gha` still returned `200` on `raw.githubusercontent.com` well after it became `Morrison-Lab/gha`, so an old-name probe reports every repo as "not moved". The REST API is not a substitute — behind an agent proxy `api.github.com/repos/<o>/<r>` can return `403` for every repo regardless of existence, which answers nothing in either direction. This matters before any blanket owner rewrite: probing all nine `d-morrison/*` references in ucdavis/bcs under the new owner showed only `gha` and `ai-config` had moved, so a find-and-replace would have broken `macros`, `altdoc`, `snapr`, `stats-allowlist`, `diffviewer`, `equation-anchors`, and `rme`. Note the bare `d-morrison` *username* (a `reviewer:` input, author metadata) is unaffected by a repo/org rename and must not be swept along. The Actions-side consequences of the same rename are in `github-actions.md` ("A repo/org rename breaks Actions `uses:` refs"). (2026-07-28.)
Expand Down
32 changes: 32 additions & 0 deletions shared/workflow/address-every-comment.md
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,38 @@ Matching whole lines against the tag -- how bash itself ends a heredoc --
removed the whole lazy-quantifier/anchor failure mode instead of narrowing
it; the reply carried the failing output of the suggested form.)

**A reviewer's corrected citation is another factual claim, so verify the
replacement before adopting it.**
The finding can be right: the citation in the PR can name the wrong source.
That does not make the reviewer's proposed source right.
A replacement issue or PR number is a fresh provenance claim, and it needs the
same check as the original citation.
For text provenance, prefer history over word association:
`git log -S "<exact line>" -- <file>` asks which commit introduced the line,
while matching a word in another PR plus a nearby merge time only builds a
story.
Keep the review's conclusion when it is right, but set aside the replacement
when the evidence points elsewhere, and say which query decided it.

- **Do:** verify a proposed replacement citation with the source's own history
before editing the PR to use it.
- **Do:** use `git log -S "<exact line>" -- <file>` or an equivalent
provenance query when the question is which PR introduced text.
- **Don't:** adopt a reviewer's corrected issue or PR number because the
original was wrong.
- **Don't:** use word overlap and same-day timing as a substitute for source
history.

(Morrison-Lab/ai-config#971 round 2, 2026-08-01: a review correctly found that
PR #955 did not cover a "default nobody chose" case record.
It then proposed #951 as the source because #951's `memories/tools.md` entry
used the word "default" and merged the same day.
That was the wrong default and the wrong file: #951 did not touch
`shared/workflow/metacognitive-monitoring.md`.
`git log -S "An unexamined default" -- shared/workflow/metacognitive-monitoring.md`
identified #947 as the source for the default half, while #955 supplied the
handed-premise half in the same fragment.)

**The highest-yield version of that check: when a comment names an edge case
in its own prose and also supplies a fix, run the fix against that edge
case.**
Expand Down
43 changes: 43 additions & 0 deletions shared/workflow/challenge-redundant-content.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,46 @@ the same procedure copied into several unrelated places), say so and route it
to `find-overlap` (or `consolidate-skills`/`consolidate-memory` for
skills/memories), rather than trying to fix everything found along the way in
the current review.

## Use the overlap instrument, not literal grep

A redundancy finding is exactly where phrase grep is most tempting, and where
it is least trustworthy.
This corpus uses semantic line breaks and inline markup, so a copied claim can
span a newline or differ only by backticks while still being the same prose.
Use the existing overlap instrument for the broad sweep:
`scripts/find-near-duplicates.py` ranks corpus pairs by normalized word
shingles and reports the number of units and pairs it examined.
Its default target set and top-pair threshold are tuned for near-duplicate
files, not for proving that a short copied run between long `shared/` files is
absent.
So treat it as the reusable starting point and corpus-wide reading list, not as
a clean-result certificate for a narrow review finding.
When the question is a narrower single-file audit, use the same algorithmic
shape with a run-reporting matcher:
normalize whitespace and markup on both sides, lower-case, compare word
n-grams, and report the examined scope and every overlapping run.

- **Do:** run `scripts/find-near-duplicates.py` for the broad candidate list,
or run an equivalent normalized n-gram matcher that reports overlapping runs
for the files under review.
- **Do:** report the examined files, pairs, or runs alongside any overlap found,
then read the overlapping bodies before deciding whether consolidation loses
nothing.
- **Don't:** use literal grep as evidence that no matching prose exists in a
semantic-line-break corpus.
- **Don't:** treat `scripts/find-near-duplicates.py`'s default no-finding result
as proof a short copied passage is absent from long shared files.
- **Don't:** treat a high shingle score as the disposition; it is a pointer to
read, not a duplicate verdict.

(Morrison-Lab/ai-config#969, 2026-08-01: after review flagged one uncited
copy between `batch-merge-and-resolve.md` and `sync-with-main.md`, a
10-gram shingle sweep normalized backticks, asterisks, underscores, whitespace,
and case across `shared/workflow/`, `shared/writing/`, and
`shared/principles/`.
It found four overlapping runs: the flagged generalization, a second uncited
copy about `markdownlint`'s `blanks-around-lists` explanation that review had
missed, and two already-cited overlaps.
A literal grep for the `blanks-around-lists` wording in `sync-with-main.md`
returned nothing because the phrase crossed a semantic line break.)
34 changes: 34 additions & 0 deletions shared/workflow/sync-with-main.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,40 @@ Dropped before implementation, with the reasoning recorded in both the issue
and the PR body, and the neighbouring fragments cross-linked to the skill
instead.)

**A routine merge from `main` can create the duplicate inside your own diff.**
The collision above lands before you write, so the duplicate is redundant on
arrival.
A later `main` merge is quieter: both branches were non-duplicative when they
were written, and the duplicate appears only when you bring the other branch's
text into yours.
Git reports a clean merge because the two copies sit in different files.
Diff-scoped added-line checks do not help either, because the duplicated lines
already existed on one side or the other.
So after merging `main` into a prose branch, run the duplicate check against
the branch's full current diff and the neighbouring corpus, not only against
lines added by the merge commit.

- **Do:** after a `main` merge, re-run a cross-file duplication check over the
merged branch's whole prose diff.
- **Do:** treat a reviewer finding on such duplication as correct even when
each copy was independently right before the merge.
- **Don't:** assume a conflict-free `main` merge preserved DRY, or that the
duplicate would have appeared in an added-lines-only scan.
- **Don't:** answer by asking which branch "introduced" the duplication;
the merge introduced the state that made both copies coexist.

(Morrison-Lab/ai-config#969, 2026-08-01: #969 added
`shared/workflow/batch-merge-and-resolve.md` with a blockquote generalizing
that an added-lines-only instrument is unsound when a defect can be introduced
by deleting a line.
PR #966 independently added the same generalization to
`shared/workflow/sync-with-main.md` and merged after #969's branch was written.
`git show 50afe818:shared/workflow/sync-with-main.md`, normalized for
whitespace and markup, did not contain the phrase, so the duplication did not
exist at #969's pre-merge head.
The round-2 merge from `main` brought #966's copy in, and the round-3 review
correctly flagged the two uncited copies.)

**Two PRs that each append a new terminal numbered subsection to the same
file (e.g. `### 5. ...` in a `CLAUDE.md` review-guidelines list) will
conflict on merge even when neither side's content actually disagrees.**
Expand Down
Loading