Skip to content

workflow: read a repo's canonical contributor doc before starting work - #1389

Merged
d-morrison merged 2 commits into
mainfrom
workflow/canonical-doc-before-starting
Aug 10, 2026
Merged

workflow: read a repo's canonical contributor doc before starting work#1389
d-morrison merged 2 commits into
mainfrom
workflow/canonical-doc-before-starting

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Incident

UCD-SERG/serocalculator#661 (2026-08-10): serocalculator/CLAUDE.md opens by naming .github/copilot-instructions.md "the source of truth for repository-specific style and workflow" and calling itself "a short orientation." A session read only the short file before starting work on that PR. A docs-only change shipped with no DESCRIPTION dev-version bump, and version-check.yaml caught it --- copilot-instructions.md line 645 states the requirement outright: "ALWAYS increment dev version number to be one ahead of main branch before requesting PR review." A second commit fixed the CI failure that a first read would have avoided.

Why the rule was invisible

The rule already existed, word for word, at memories/preferences.md's "Before opening a PR, read the repo's own agent/contributor instructions" bullet. It went unconsulted because memories/ files are not auto-loaded into a session's context the way CLAUDE.md and its @shared/... fragments are --- a session has to take the extra step of grepping/reading a memory file on demand, and nothing prompted that here.

What this PR does

  • Adds shared/workflow/read-canonical-doc-before-starting.md: read a repo's fuller canonical doc before starting work, not just before pushing --- distinct from ardi.md's pre-push self-review, which assumes the conventions are already known. Includes the incident above as its case record.
  • Wires it into CLAUDE.md with a new section between "Claim a GitHub PR/issue before working on it" and "Open a PR immediately after claiming an issue," per check-purpose-before-reusing.md's guidance to place new fragments deliberately rather than force them into an oversized file (ardi.md is already 1521 lines, well over the 1200-line advisory threshold, so the new content is not added there).
  • Points memories/preferences.md's existing (more detailed, R-package-specific) bullet at the new fragment, and fixes a stale citation on the same line: it named tools.md for the "R-package PR CI gates" section, which actually lives in memories/r-quarto.md.
  • Bumps test_check_context_closure.py's pinned anchored-import count from 79 to 80, per that test's own documented remedy for an @-import addition to CLAUDE.md.

Verification

  • check-context-closure.py: exits 0; the new @shared/workflow/read-canonical-doc-before-starting.md import resolves.
  • check-links.py: 1352 relative links across 474 files, none broken.
  • check-memory-file-size.py: no memory file exceeds 1200 lines (preferences.md is 1017).
  • markdownlint-cli2: 0 issues in 500 files.
  • Punctuation scan (added lines, three-dot range): 0 banned glyphs in 102 added lines.
  • check-new-line-breaks (real script from a d-morrison/gha checkout): no lines missing semantic breaks.
  • Full offline scripts/test_*.py suite: all green except two suites (test_ai_session, test_compare_shell_forms) that are pre-existing failures on origin/main itself, unrelated to this diff, and already tracked in Two script test suites fail on a clean main: test_ai_session (4 cases) and test_compare_shell_forms (1 case) #1382.

Placement decision

Considered adding the rule directly into ardi.md's existing "Self-review against the project's own stated conventions before every push" section, since that's the nearest existing fragment on this topic. Rejected: that section is about self-reviewing a diff against already-known conventions before a push; this rule is about the moment before anything is known, at the start of work, and ardi.md is already oversized. A new, small, focused fragment (shared/workflow/check-open-prs-before-duplicating.md-sized) was the better fit.

Closes nothing --- no existing tracking issue covers this; this PR is the tracking record and the fix in one, consistent with CLAUDE.md's "Encoding reusable feedback into ai-config" section.


Generated by Claude Code

claude added 2 commits August 10, 2026 06:59
UCD-SERG/serocalculator#661 (2026-08-10): serocalculator's CLAUDE.md
names .github/copilot-instructions.md as the source of truth and calls
itself a short orientation, but a session read only the short file
before starting work. A docs-only PR shipped with no DESCRIPTION
dev-version bump, and version-check.yaml caught it, costing a second
commit.

The rule already existed almost verbatim in memories/preferences.md,
but memories/ files are not auto-loaded into a session's context the
way CLAUDE.md and its @shared/... fragments are, so it went
unconsulted. Add shared/workflow/read-canonical-doc-before-starting.md,
wire it into CLAUDE.md, and point preferences.md's existing (more
detailed, R-package-specific) bullet at it. Also fix a stale citation
in that bullet: it named "tools.md" for the R-package PR CI gates
section, which actually lives in memories/r-quarto.md.
Adding the @shared/workflow/read-canonical-doc-before-starting.md
import in the prior commit bumped CLAUDE.md's anchored-import count
from 79 to 80, per test_check_context_closure.py's own documented
remedy for exactly this failure.
@d-morrison
d-morrison requested a balanced review from Copilot August 10, 2026 07:06

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.

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

@github-actions

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


Code Review: PR #1389 — "workflow: read a repo's canonical contributor doc before starting work"

Scope note: This PR modifies the ai-config corpus itself (a Markdown/Python knowledge-base repo for Claude Code conventions), not an R package or Quarto site. The SERG lab manual's R/Quarto-specific sections (coding style for R objects, function length limits, {here} usage, tidyverse idioms) don't apply to this diff — there's no R or Quarto code here. I evaluated this diff against the repo's own stated conventions (CLAUDE.md and its shared/ fragments) instead, which is the applicable standard for this repository.

What changed

  • Adds shared/workflow/read-canonical-doc-before-starting.md: a new rule that a repo's fuller canonical contributor doc (e.g. .github/copilot-instructions.md) should be read at the start of work, not just before pushing.
  • Wires it into CLAUDE.md via @-import, placed between "Claim a GitHub PR/issue" and "Open a PR immediately after claiming an issue."
  • Repoints an existing memories/preferences.md bullet's stale citation from tools.md to r-quarto.md, and cross-links it to the new fragment.
  • Bumps scripts/test_check_context_closure.py's pinned anchored-import-count regression test from 79 to 80.

Verification performed

  • Confirmed memories/tools.md does not contain "R-package PR CI gates" and memories/r-quarto.md does (line 269) — the citation fix is correct, and memories/MEMORY.md's own table already pointed to r-quarto.md for this topic, so this closes a pre-existing inconsistency.
  • Ran the full local test suite (scripts/test_*.py, 12 files): all pass except test_ai_session.py and test_compare_shell_forms.py, which fail identically and are confirmed pre-existing/tracked in ai-config#1382 (verified the issue exists and matches the exact failure signatures reproduced on this branch).
  • check-context-closure.py, check-links.py (1352 links, 0 broken), and check-memory-file-size.py all ran clean, matching the PR body's claims.
  • Verified every citation in the new fragment against live sources: ardi.md's "Self-review against the project's own stated conventions" section title (exists), keep-checkouts-fresh.md's "a bare citation is invisible..." quote (exact match), and — most importantly — the external incident citation UCD-SERG/serocalculator#661: confirmed via gh that this PR is real, merged, and its second commit is literally titled "Bump dev version to pass version-check," and confirmed serocalculator/.github/copilot-instructions.md line 645 reads verbatim "ALWAYS increment dev version number to be one ahead of main branch before requesting PR review," and serocalculator/CLAUDE.md calls copilot-instructions.md "the source of truth... a short orientation" word-for-word. No hallucinations.
  • Checked semantic-line-break and ASCII-punctuation compliance on all added lines (no banned glyphs, no multi-sentence lines) and confirmed markdown links resolve correctly.
  • All CI checks (validate, new-line-breaks / check-new-line-breaks) report SUCCESS.

Findings

No blocking issues.

One optional, non-blocking observation (not tied to a specific enforceable rule, so not posted as a blocking finding): in scripts/test_check_context_closure.py's history-annotation comment, prior entries cite this repo's own PR/issue number for each import-count bump (ai-config#1065, ai-config#1205, ai-config#1325, ai-config#1334), while this entry cites the external UCD-SERG/serocalculator#661 instead. The citation itself is accurate (verified above) and the PR body explains why no ai-config tracking issue exists, but a future reader searching this repo's own tracker for "what changed this pin" won't find this PR (#1389) referenced the way earlier entries reference their own PR/issue. This is a minor documentation-consistency nit, not a defect — no action required.

ℹ️ Inline comments were unavailable in this environment; findings appear above with path references.

Verdict

Verdict

Ready for merge

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $11.2961 (review) — run

@d-morrison
d-morrison merged commit 443f043 into main Aug 10, 2026
8 checks passed
@d-morrison
d-morrison deleted the workflow/canonical-doc-before-starting branch August 10, 2026 07:18
d-morrison added a commit that referenced this pull request Aug 10, 2026
…ypo (#1390)

Record two learnings from UCD-SERG/serocalculator#661's ai-config follow-up
(#1389):

- memories/github.md: a session's egress proxy can block GraphQL entirely
  (403, "only the pinned set of PR-review operations is served") even while
  gh api user / gh api repos/<o>/<r> both return 200 over REST --- a second,
  distinct root cause for the check-pr-fully-clean.py failure already
  tracked in #1330, alongside the pre-existing
  GraphQL rate-limit case.
- memories/github-mcp-tools.md: issue_write's method enum only accepts
  "create" or "update" --- guessing "add_comment" fails loudly and safely
  (no data touched), distinct from the already-documented silent
  clobbering when method: "update" is called with just a comment string
  as body.


Claude-Session: https://claude.ai/code/session_01NbqvD8bSKpeTjDwYpQSHeR

Co-authored-by: Claude <noreply@anthropic.com>
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