Skip to content

Add AGENTS.md and CLAUDE.md, and correct the stale copilot-instructions.md - #37

Merged
d-morrison merged 6 commits into
mainfrom
docs/agent-config
Aug 28, 2026
Merged

Add AGENTS.md and CLAUDE.md, and correct the stale copilot-instructions.md#37
d-morrison merged 6 commits into
mainfrom
docs/agent-config

Conversation

@d-morrison

@d-morrison d-morrison commented Aug 28, 2026

Copy link
Copy Markdown
Member

Closes #35

The claude-review bot on #33 reported the gap itself:

No CLAUDE.md exists anywhere in this repo, so compliance was judged solely
against the SERG lab manual.

Every automated review this repo receives has been running without
repo-specific context.

Shape: one source of truth, two thin pointers

Adopts UCD-SERG/serocalculator's
shape rather than ucdavis/bcs's or Morrison-Lab/gha's. Those carry 10--20 KB
in each of AGENTS.md and CLAUDE.md, which triplicates the content --- and one
of the three drifting is precisely the problem this PR also has to fix.

.github/copilot-instructions.md stays the source of truth. AGENTS.md
(112 lines) and CLAUDE.md (56 lines) are orientation that defers to it.

repo AGENTS.md CLAUDE.md copilot-instructions .claude/settings.json
UCD-SERG/serocalculator yes yes yes yes
ucdavis/bcs yes yes yes yes
Morrison-Lab/gha yes yes -- --
UCD-SERG/serodynamics -- yes yes yes
d-morrison/rme -- yes yes yes
UCD-SERG/shigella (before) -- -- yes --

The stale part

copilot-instructions.md's Continuous Integration section still listed
lint-changed-files.yaml and check-spelling.yaml as required checks. Both
were removed in #33 earlier tonight. It now describes what actually runs,
separates the seven hand-maintained workflows from the Morrison-Lab/gha
callers, and records the lint-changed-lines backlog as pre-existing rather
than as something a given PR introduced.

Three conventions that were written down nowhere

  • Version: must not change in a PR. version-check inverted on
    2026-08-27 and bump-dev-version now owns the bump, so older branches and
    habit both point the wrong way. This is the one most likely to bite, so it is
    in its own section, in AGENTS.md, and appended to the NEVER list.
  • Two accept-lists with different owners. inst/WORDLIST ({spelling},
    codepoint-sorted) and _typos.toml (crate-ci/typos). Adding a word to the
    wrong one silently fails.
  • gha caller pins. Read the callee at its pinned tag before editing a
    with: or secrets: block: a caller passing a secret the tag does not
    declare is rejected before any job starts, with no logs, no annotations, and
    no check run --- so it is invisible on the PR. That is exactly what Finish the gha migration and fix the Claude workflow startup failure #33 fixed.

No dependency on #31

The prose deliberately does not assert that _typos.toml exists. That file
arrives with #31, which is still open, so both new files describe where such
terms belong rather than pointing at a path this branch does not have. The two
PRs' file sets are disjoint apart from NEWS.md and inst/WORDLIST, which are
append-only and merge cleanly, so there is no merge-order constraint between
them.

Verification

Correction to this PR's first push. It claimed the added lines had been
verified with gha's check-new-line-breaks locally. That run was vacuous:
the checker diffs <base>...HEAD, and I ran it with the work staged but not
committed, so it examined an empty range and reported clean over 200-odd added
lines. CI, running the identical script at @v2, found ten. 9e94c41 fixes
those, plus six list-item splices, and records the mistake.

Two things worth carrying forward from that:

  • A clean result over a large added-line population is itself a signal to
    check the population, not a pass.
  • lint-markdown's fail: false does not cover its companions, and
    two of the three default to failing --- fail-on-item-splices and
    fail-on-table-splits default true, while fail-on-long-code-blocks
    defaults false. That is why this PR went red on a workflow configured
    warn-only. (An earlier version of this section said all three default to
    failing; a review of the ai-config entry recording this caught it, and the
    defaults above are read off Morrison-Lab/gha@v2 directly.)

Re-run after committing, against the real diff:

  • check-new-line-breaks.py with NLB_BASE_REF=origin/main --- clean.
  • check_list_item_splices.mjs, check_table_splits.mjs,
    check_code_block_length.mjs --- clean.
  • typos 1.49.1 --- clean.
  • spelling::spell_check_package(".") --- clean, after adding allowlist to
    inst/WORDLIST.
  • Every relative link in both new files resolved to an existing path.
  • AGENTS.md, CLAUDE.md, and .claude added to .Rbuildignore, matching
    how serocalculator and bcs handle the same files.

Deferred

Wiring use-ai-config into the two gha bot callers is #36. It changes the
behaviour of a currently-working reviewer on every PR, so it wants its own diff
and a deliberate decision rather than riding along with a documentation change.

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

The claude-review bot on #33 reported it directly: "No CLAUDE.md exists
anywhere in this repo, so compliance was judged solely against the SERG lab
manual." Every automated review here has been running without repo-specific
context.

Adopts UCD-SERG/serocalculator's shape rather than ucdavis/bcs's or
Morrison-Lab/gha's. Those carry 10-20 KB in each of AGENTS.md and CLAUDE.md,
which triplicates the content across three files -- and one of them drifting
is precisely the problem this commit also has to fix. So
.github/copilot-instructions.md stays the single source of truth, and the two
new files are short orientation that defers to it.

copilot-instructions.md was stale: its Continuous Integration section still
listed lint-changed-files.yaml and check-spelling.yaml as required checks,
both removed in #33. It now describes what actually runs, separates the
hand-maintained workflows from the Morrison-Lab/gha callers, and records the
lint-changed-lines backlog as pre-existing.

Three conventions that govern every PR here were written down nowhere:

  - Version: must not change in a PR. version-check inverted on 2026-08-27
    and bump-dev-version now owns the bump, so older branches and habit both
    point the wrong way. Added to the NEVER list too.
  - inst/WORDLIST and _typos.toml are separate accept-lists with different
    owners; adding a word to the wrong one silently fails.
  - gha caller pins, and the fact that a caller passing a secret the pinned
    tag does not declare fails with no logs, no annotations, and no check run.

.claude/settings.json adds a read-only permissions allowlist, as four of the
five configured peer repos carry.

Prose deliberately does not assert that _typos.toml exists: that file arrives
with #31, which is still open, so both files describe where such terms belong
rather than pointing at a path this branch does not have.

Verified locally: gha's own check-new-line-breaks over the added lines,
typos, spelling::spell_check_package(), and every relative link resolved.

Closes #35

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HYDWsGL8cYC3ptLEL4Wayn
@d-morrison
d-morrison marked this pull request as ready for review August 28, 2026 04:56
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Correcting my own claim on the parent commit. It said the added lines had
been verified with gha's check-new-line-breaks locally. That run was
vacuous: the checker diffs `<base>...HEAD`, and I ran it with the work
staged but not yet committed, so it examined an empty range and reported
"No lines missing semantic breaks" over 200-odd added lines. CI, running
the identical script at @v2, found ten. The tell was there to be read --
a clean result over that much new prose is not plausible -- and I did not
read it.

Re-run after committing, the same script reproduces CI exactly. Fixed:

  - 10 lines packing more than one sentence, now one sentence per line.
  - 6 list items merged onto a previous item's continuation line with no
    blank line between, which lint-markdown flags separately. Note that
    lint-markdown's `fail: false` does not cover this: the splice,
    code-block-length, and table-split companions each have their own
    toggle, and those default to failing.

copilot-instructions.md is wrapped at 72 columns throughout, so the added
prose is now stylistically distinct from the rest of the file. That is the
intended direction -- check-new-line-breaks is diff-scoped precisely so a
repository can adopt it without a whole-corpus reflow first.

Verified post-commit: check-new-line-breaks, check_list_item_splices.mjs,
check_table_splits.mjs, check_code_block_length.mjs, typos, and
spelling::spell_check_package() all clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HYDWsGL8cYC3ptLEL4Wayn
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Four blocking findings, all real, all accepted.

1 and 2 are the same mistake, and it is the one worth naming: I wrote the
layout and the "known-failing checks" sections from the PR #31 worktree
rather than from this branch. `inst/scripts/` and every `.stan` file exist
only on the chapter branches -- `git ls-files inst` on `main` returns
`inst/WORDLIST` and `inst/extdata/model.jags.r`, and nothing else. So this
PR asserted a directory layout that is not in the tree it ships to, and told
future readers to treat a `lint-changed-lines` failure there as a known
backlog when that check passes on `main`.

Both files now describe `main` as the thin skeleton it is, say that the
chapter code arrives with the feature branches, and tell the reader to derive
a layout with `git ls-files` rather than from prose. The backlog note is kept
but scoped to the branches that actually carry it.

That also exposed pre-existing errors in copilot-instructions.md that this PR
is the right place to fix: four references to `inst/stan/`, a directory that
has never existed. Stan models live in `inst/extdata/`.

4. `.claude/settings.json` was too loose for a list framed as read-only.
Dropped `gh api:*`, which permits `-X POST` and `-X DELETE`; dropped
`devtools::*`, which is R namespace syntax that can never match a Bash
invocation and was already covered by `Rscript -e`; narrowed `git branch:*`
to `git branch --list:*`, since the wide form permits `-D` and `-m`.

5. The version rule contradicts the lab manual, whose quality-assurance
checklist still lists "Version number has been incremented" -- and both new
files declare that manual authoritative. Verified against the source
(`coding-practices/quality-assurance-checklist.qmd:11`). Both files now name
the conflict, say the checklist predates `bump-dev-version`, and say the
manual still governs everything else.

Also linked `{spelling}` per the manual's package-link convention.

`.Rbuildignore` still does not ignore `_typos.toml`; that file arrives with
#31, which adds the entry, and duplicating it here would collide.

Verified post-commit against the real diff: check-new-line-breaks,
check_list_item_splices.mjs, check_table_splits.mjs, typos, and
spelling::spell_check_package() all clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HYDWsGL8cYC3ptLEL4Wayn
@d-morrison

Copy link
Copy Markdown
Member Author

Round 2 dispositions --- all four Addressed in 3efbcb9

1 (Stan/JAGS location) and 2 (inst/scripts/ backlog) --- Addressed, and
they were the same mistake.
I wrote the layout and "known-failing checks"
sections from the PR #31 worktree rather than from this branch. Verified:

$ git ls-tree -r --name-only origin/main inst
inst/WORDLIST
inst/extdata/model.jags.r
$ git ls-tree -r --name-only origin/main | grep -c '\.stan$'
0

So this PR asserted a directory layout absent from the tree it ships to, and
told readers to treat a lint-changed-lines failure in inst/scripts/ as a
known backlog --- when that check passes on main. Both files now describe
main as the thin skeleton it is, say the chapter code arrives with the
feature branches, and tell the reader to derive a layout with git ls-files
rather than from prose. The backlog note is kept, scoped to the branches that
carry it.

That also surfaced pre-existing errors this PR is the right place to fix:
copilot-instructions.md referenced inst/stan/ in four places, a directory
that has never existed. Corrected to inst/extdata/.

4 (.claude/settings.json) --- Addressed. Dropped gh api:* (permits
-X POST / -X DELETE) and devtools::* (R namespace syntax that can never
match a Bash invocation, already covered by Rscript -e); narrowed
git branch:* to git branch --list:*, since the wide form permits -D and
-m.

5 (lab-manual conflict) --- Addressed. Verified against the source rather
than the rendered page:
UCD-SERG/lab-manual's coding-practices/quality-assurance-checklist.qmd:11
reads - [ ] Version number has been incremented. Both files now name the
conflict explicitly, say the checklist predates bump-dev-version, and say
the manual still governs everything else.

Minor --- one Addressed, one Deferred. {spelling} now uses the manual's
backtick-plus-hyperlink convention. .Rbuildignore still does not ignore
_typos.toml: that file arrives with #31, which adds the entry itself, so
duplicating it here would collide for no benefit while the file does not
exist on this branch.

On the previous round

Round 1's issue 3 was mine to have caught. The verification claim on the
first push was made from a checker run I did before committing, and
check-new-line-breaks diffs <base>...HEAD --- so it measured an empty
range and reported clean over 200-odd added lines. Every checker result quoted
above was re-run after committing, against the real diff.

Posted by Claude Code (AI agent) --- not written by a human.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Round 3 non-blocking nit. AGENTS.md said CLAUDE.md 'says the same things',
while CLAUDE.md says it points here and adds only Claude-specific material.
The second is the accurate one, and it is the framing the whole
one-source-two-pointers shape depends on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HYDWsGL8cYC3ptLEL4Wayn
@d-morrison

Copy link
Copy Markdown
Member Author

Round 3 dispositions

Wording tension between AGENTS.md:5 and CLAUDE.md:6-7 --- Addressed in
27ec745. AGENTS.md said CLAUDE.md "says the same things", while
CLAUDE.md says it points back and adds only Claude-specific material. The
second is the accurate one, and it is the framing the whole
one-source-two-pointers shape depends on, so AGENTS.md now states it that
way.

.Rbuildignore does not ignore _typos.toml --- Deferred, deliberately.
That file does not exist on this branch; it arrives with #31, which adds the
.Rbuildignore entry in the same commit. Adding it here would collide with
that PR for no benefit while the file is absent. If #31 is closed rather than
merged, this becomes a real gap and should be picked up then.

Also worth recording, since this PR's own framing depended on it: main gained
use-ai-config: true on both Claude callers while this PR was open (#36 /
#38), and that turns out to be a no-op in behaviour --- Morrison-Lab/gha@v2
already defaults the input to true in both workflows. The reviewer here has
been loading the shared ai-config corpus all along. Both merged commits
record intent rather than change it, which is worth doing, and my #36 was
wrong to claim the reviewer "works from whatever prose the repo happens to
carry."

Posted by Claude Code (AI agent) --- not written by a human.

@github-actions

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


Both files now describe the relationship identically: AGENTS.md is the cross-agent contract, CLAUDE.md points back to it and adds only Claude-specific material. The wording tension is fully resolved. I have everything needed to write the final review.

Review

This is a follow-up round on three prior review rounds on this PR:

  • Round 1 (posted 2026-08-28T05:05:38Z, Needs more work) — 5 blocking issues.
  • Round 2 (posted 2026-08-28T05:11:46Z, Needs more work) — 4 of 5 still open.
  • Round 3 (posted 2026-08-28T07:38:08Z, Ready for merge) — all 5 blocking issues resolved; only two non-blocking nits remained: the .Rbuildignore/_typos.toml deferral (deliberate, tied to PR Dissertation draft: chapters 1–3 and the two chapter manuscripts #31) and a wording tension between AGENTS.md:5 and CLAUDE.md:6-7 about whether CLAUDE.md "says the same things" as AGENTS.md or "adds only what is specific."

What changed since round 3. Exactly one substantive commit landed after the last-reviewed commit (afed624b): 27ec7454 ("AGENTS.md: state CLAUDE.md's role as it describes itself"), plus a routine merge of origin/main (8098c1a, bringing in an unrelated DESCRIPTION version bump from main that is not part of this PR's diff — confirmed absent from the saved .claude-review-pr.diff). I verified this against git diff afed624b 27ec7454, which shows only:

-Claude Code reads [`CLAUDE.md`](CLAUDE.md), which says the same things.
+Claude Code reads [`CLAUDE.md`](CLAUDE.md), which points back here for these
+conventions and adds only what is specific to Claude Code.

This is exactly the fix the author described in their round 3 disposition reply: "AGENTS.md said CLAUDE.md 'says the same things', while CLAUDE.md says it points back and adds only Claude-specific material. The second is the accurate one... so AGENTS.md now states it that way."

Independently re-verified: AGENTS.md:5-6 (at HEAD) now reads "Claude Code reads CLAUDE.md, which points back here for these conventions and adds only what is specific to Claude Code," and CLAUDE.md:6-9 reads "AGENTS.md is the cross-agent contract and carries the same conventions; read it rather than duplicating its content here... This file adds only what is specific to Claude Code." The two files now describe their relationship identically in both directions — the round-1/2/3 wording tension is fully closed, not just reworded.

I also re-confirmed the round-3 "Ready for merge" fixes are still intact in the current tree (git show HEAD:... used to bypass the local working copy, where the review harness has locally deleted .claude/settings.json and CLAUDE.md from the working directory — evidently for this session's own sandboxing, not a PR defect; the commit object itself has both files intact):

  • No inst/stan/ references anywhere; all consistently inst/extdata/.
  • inst/scripts/ backlog claim is correctly scoped to "branches that carry inst/scripts/," not asserted as fact about main.
  • .claude/settings.json (git show HEAD:.claude/settings.json) has no gh api:*, no devtools::*, and git branch --list:* (not the unscoped git branch:*) — all entries read-only.
  • The Version:/lab-manual conflict is explicitly acknowledged in both AGENTS.md and .github/copilot-instructions.md ("This deliberately departs from the lab manual...").
  • A fresh scripted scan of every + line in the current diff for the "two sentences packed onto one line" pattern (grep -E '^\+' ... | grep -E '\. [A-Z]') returns nothing — semantic-line-break compliance holds.

Still open, non-blocking (unchanged from round 3, deliberately deferred): .Rbuildignore doesn't ignore _typos.toml; the author's round 3 reply explains this is deliberately deferred to #31, which adds both the file and its ignore entry together. Not a defect.

No new issues were introduced by this round's commit.

Verdict

Ready for merge — the prior verdict at #37 (comment) stands, and the one outstanding non-blocking wording nit it carried forward is now also resolved by 27ec7454. No new findings this round.

Reviewed commit: 27ec745

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $0.6377 (review) — run

@d-morrison
d-morrison merged commit d858deb into main Aug 28, 2026
27 checks passed
@d-morrison
d-morrison deleted the docs/agent-config branch August 28, 2026 07:52
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.

Add AGENTS.md and CLAUDE.md, and correct the now-stale copilot-instructions.md

1 participant