Finish the gha migration and fix the Claude workflow startup failure - #33
Conversation
Repin claude.yml, claude-code-review.yml, and news.yaml from @v1 to @v2. Both Claude callers pass an ANTHROPIC_API_KEY secret that Morrison-Lab/gha only declares at @v2, so every Claude Code run since #30 was rejected before any job started (startup_failure, no logs, no annotations). Migrate the three remaining workflows gha models: check-spelling.yaml -> spellcheck.yml@v2 lint-changed-files.yaml -> lint-changed-lines.yml@v2 version-check.yaml -> version-check.yml@v2 + bump-dev-version.yml@v2 version-check inverts the old convention on purpose: PRs stop touching DESCRIPTION's Version:, and bump-dev-version bumps it after each merge to main. The repo already allows Actions to open PRs and allows auto-merge, and main has no required status checks, so no WORKFLOW_TOKEN is needed. Add the gha capabilities the repo was missing: lint-workflows, lint-yaml, lint-markdown, check-junk-files, check-typos, check-secrets, check-links, check-phi, check-new-line-breaks. The three whole-tree linters start warn-only because the repo has pre-existing drift. Refs #32 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HYDWsGL8cYC3ptLEL4Wayn
Codecov Report✅ All modified and coverable lines are covered by tests. |
Adversarial review finding: gha's check-typos and check-new-line-breaks derive their base from the pull_request event and, with no base to diff against, skip and report success. A push trigger therefore put a green check on main that examined nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HYDWsGL8cYC3ptLEL4Wayn
Adversarial review (codex CLI, GPT-5.1)Reviewed commit: Findings
Verdict: needs work Posted by Claude Code (AI agent) --- not written by a human. |
Dispositions1 and 2 --- Rebutted. Both workflows do exist at Every 3 --- Addressed, in Posted by Claude Code (AI agent) --- not written by a human. |
Two CI failures at ca61080: roxygen2 8.1.0 (2026-08-04) records its version in Config/roxygen2/version rather than RoxygenNote, so `roxygenise()` rewrote DESCRIPTION on every run and docs-check went red. Pre-existing environmental drift rather than anything this branch changed -- it last passed on main on 2026-07-28 -- but it blocks this PR, so fix it here. gha's new lint-markdown flagged a real defect in this branch's own NEWS entry: a list item starting directly on the line after another item's continuation, with no blank line between. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HYDWsGL8cYC3ptLEL4Wayn
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Claude finished review — View run This confirms the PR's technical claim exactly: roxygen2 8.1.0 released 2026-08-04, matching the commit message's stated root cause for the I have enough to finalize the review. Code Review — UCD-SERG/shigella PR #33Prior verdict: Ready for merge posted 2026-08-28T01:38:29Z, reviewing commit What changed since that roundOne additional commit landed after
Verification performed on the new commit
FindingsNone. The new commit is a small, correctly-targeted fix for two CI failures that were introduced by the previously-reviewed commit, both fixes are technically accurate (verified against upstream roxygen2 release notes) and confirmed working by passing CI, and no new workflow, input, or permissions concern was introduced. VerdictReady for merge — the prior round's "Ready for merge" verdict (comment posted 2026-08-28T01:38:29Z, reviewing Reviewed commit: f2a328f |
Fully clean at
|
Resolves three conflicts introduced by #33 landing on main: DESCRIPTION -- both sides independently migrated RoxygenNote to Config/roxygen2/version (roxygen2 8.1.0), in different positions. Kept main's placement and dropped the duplicate; DESCRIPTION is now byte-identical to main's, which also satisfies the new version-check convention. NEWS.md -- kept both sides. Neither entry supersedes the other. inst/WORDLIST -- union of both sides, re-sorted in the file's existing codepoint order (194 entries). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HYDWsGL8cYC3ptLEL4Wayn
check-typos is new on main as of #33 and found 58 items in this PR's added lines. All 58 are false positives, and running typos 1.49.1 locally over the merged tree reproduces the CI count exactly, so the fix is verifiable here. _typos.toml excludes the two file classes that cannot meaningfully be spellchecked -- .bib exports (author surnames; "Leroux-Roels" alone is three findings) and inst/WORDLIST, whose entire purpose is to hold unusual words -- and accepts five domain abbreviations the model code and manuscripts use deliberately: lik, lod, mis, nd, pn. One genuine fix rather than a suppression: an ALL-CAPS emphasis in a comment in inst/scripts/ch3/11_coverage.R ran "OVERestimates" together, which typos splits into OVE + Restimates. Hyphenating it keeps the author's emphasis and lets the checker see two real words. _typos.toml is added to .Rbuildignore alongside the other root config files. typos now exits 0 over the whole tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HYDWsGL8cYC3ptLEL4Wayn
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
…ns.md (#37) * start: add AGENTS.md and CLAUDE.md (closes #35) * Add AGENTS.md and CLAUDE.md; correct the stale copilot-instructions.md 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 * Apply semantic line breaks and unsplice the added list items 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 * Address review round 2: correct false repo-state claims 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 * AGENTS.md: state CLAUDE.md's role as it describes itself 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 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #32
1. Fixes the Claude Code startup failure
Every
Claude Coderun onmainsince #30 concludedstartup_failure--- nineof them, most recently
33132530926.
Both Claude caller stubs pass an
ANTHROPIC_API_KEYsecret while pinned@v1, andMorrison-Lab/ghaonly declares that secret at@v2. GitHubrejects a
workflow_callthat passes a secret the callee does not declare,before any job starts --- which is why the failure has no logs, no annotations,
and no check runs.
Repinned
claude.yml,claude-code-review.yml, andnews.yamlto@v2,which is what the gha README says all three should be on anyway.
2. Migrates the remaining workflows gha models
check-spelling.yaml(insightsengineering action in a rocker container)spellcheck.yml@v2lint-changed-files.yaml(hand-rolled lintr over whole changed files)lint-changed-lines.yml@v2(changed lines)version-check.yaml(hand-rolleddesccomparison)version-check.yml@v2+bump-dev-version.yml@v2version-checkinverts the old convention deliberately. A PR must nowleave
DESCRIPTION'sVersion:alone, andbump-dev-versionbumps it aftereach merge to
main--- two PRs both required to bump the same line areguaranteed to collide. Bypass on a release PR with the
no version incrementlabel (created in this pass, along with
no changelogfor the bump PR).The repo already has the two settings that pair needs ("Allow GitHub Actions
to create and approve pull requests", "Allow auto-merge"), and
mainhas norequired status checks, so no
WORKFLOW_TOKENis needed.Unchanged, because gha models no equivalent:
R-CMD-check.yaml,R-check-docs.yml,check-readme.yaml,pkgdown.yaml,pr-commands.yaml,copilot-setup-steps.yml,phase0-debug.yaml.3. Adds the gha capabilities the repo was missing
lint-workflows,lint-yaml,lint-markdown,check-junk-files,check-typos,check-secrets,check-links,check-phi,check-new-line-breaks.The three whole-tree linters start
fail: falsebecause this repo haspre-existing drift; flipping them to blocking is tracked in #32. The rest are
diff-scoped or already clean against the current tree.
Verification
actionlintover all 24 workflows: no errors (three pre-existinginfo-level shellcheck notes in
copilot-setup-steps.ymlandphase0-debug.yaml, untouched here).yamllintover every new file: clean.spelling::spell_check_package("."):No spelling errors foundafteradding
PHIandghatoinst/WORDLIST.uses:,with:input, andsecrets:entry checked against theMorrison-Lab/ghatree at the currentv2tag (d519c8b).Deferred to #32
test-coverage.yamlmigration --- blocked on its CmdStan install/cachesteps, which gha does not model. Those steps look vestigial (no
.stanfile is tracked on
main,cmdstanris not inDESCRIPTION), but Chapter 2: Stan Model 2 Kronecker simulation (restructure) #13 isan open Stan PR, so that is worth settling first.
check-non-standard-chars.yml--- would fail today on em-dashes in.claude/skills/reprexes/SKILL.md(24 lines) and.github/copilot-instructions.md(16 lines), and the workflow has nopaths-ignore input.
check-ai-tells--- needs threshold tuning against this repo's researchprose.