Skip to content

CLAUDE.md: never just theorize -- investigate empirically - #355

Merged
dem-extra1 merged 1 commit into
mainfrom
claude/review-pr-337-32tq6j
Jul 28, 2026
Merged

CLAUDE.md: never just theorize -- investigate empirically#355
dem-extra1 merged 1 commit into
mainfrom
claude/review-pr-337-32tq6j

Conversation

@dem-extra1

Copy link
Copy Markdown
Collaborator

Banks a standing instruction from the maintainer, with the cases from this
week's org-move work that motivated it.

It lands here rather than in Morrison-Lab/ai-config, where a global rule
would normally live, because a session scoped to d-morrison/gha cannot add a
repository under another owner — add_repo refuses the cross-tier add. That
constraint is itself now documented two sections up, from the same work.

The rule

A hypothesis that is cheap to test must be tested before it is asserted, and
certainly before it is acted on or reported. The failure mode isn't being
wrong — it's being wrong and confident, because a stated hypothesis reads to
everyone else like a finding.

Why these three examples

Each is a place in #351/#352 where skipping the check cost real time, and each
points at an artifact that was available the whole time:

Guess What would have settled it
An org-wide job failure is an Actions spending limit — then, once disproved, anything but billing The job page's own banner: The job was not started because your account is locked due to a billing issue
lychee is failing on redirects check-links/lychee.default.toml301 is in accept
These lines are too long (269 flagged, then 31) lint-qmd/.markdownlint.qmd.jsonc{ line_length: 80, code_blocks: false, tables: false }, plus markdownlint ignoring a line with no space past the limit. The linter's actual count was 1

Both configs live in this repo, which is what makes the third case
embarrassing rather than merely unlucky. The section states the general form:
a model of a checker is worth using only once it reproduces that checker's
known result on a known input.

The corollary that cost the most

A run of access failures is not evidence a question can't be investigated.
Three authenticated routes failed in a row — a 301 from the REST API, an
out-of-scope refusal from the MCP tools, a 403 from the agent proxy — and
then a plain public https://github.com/... URL answered it immediately. The
file already gives this advice for reading files out of unscoped
repositories; this generalizes it.

Verification

  • No non-ASCII in the added lines (check-non-standard-chars territory).
  • No added line over 80 characters.
  • The repo's own check-new-line-breaks reports the new prose clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VCBscsqbuxXgaZKLNxef7d


Generated by Claude Code

…ed it

A standing instruction from the maintainer, banked here rather than in
Morrison-Lab/ai-config because a session scoped to d-morrison/gha cannot add
a repository under another owner (add_repo refuses the cross-tier add), which
is itself now documented two sections up.

The rule is that a cheap-to-test hypothesis must be tested before it is
asserted, and the section is written around the three places in the org-move
work (#351, #352) where skipping that cost real time:

- An org-wide job failure was blamed on an Actions spending limit, then on
  anything but billing once that was disproved, while the job page's own
  banner read "your account is locked due to a billing issue" throughout.
- Two guesses at markdownlint's MD013 flagged 269 and 31 lines against the
  linter's actual 1, and a guess at lychee's redirect handling was wrong
  because 301 is in the bundled config's `accept` list. Both configs are in
  this repo.
- Which repositories moved was answerable from the lychee run's own redirect
  and error lists rather than from reasoning about it.

It also records the corollary that cost the most time: a run of access
failures is not evidence a question cannot be investigated. Three
authenticated routes failed in a row -- a 301 from the REST API, an
out-of-scope refusal from the MCP tools, a 403 from the agent proxy -- before
a plain public URL answered it immediately.

Verified: no non-ASCII in the added lines, no added line over 80 characters,
and the repo's own check-new-line-breaks reports the new prose clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCBscsqbuxXgaZKLNxef7d
Copilot AI review requested due to automatic review settings July 28, 2026 22:38

Copilot AI left a comment

Copy link
Copy Markdown

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.

Comment thread CLAUDE.md
ignores a line with no space past the limit.
A model of a checker is only worth using once it reproduces that checker's
known result on a known input.
- **Prefer the run's own artifacts to your inference about them.**

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.

Ambiguous terminology: "artifacts." Elsewhere in this same file, "artifact"/"artifacts" is used exclusively for a literal GitHub Actions uploaded artifact object (e.g. upload-review-execution's description: "uploads it as a workflow artifact"). Here the example given is lychee's printed redirect/error output — plain console/log text, not an uploaded artifact — so a reader searching this file for "artifact" could reasonably (and wrongly) read this bullet as "go check the uploaded GH Actions artifact file." Notably, this PR's own changelog fragment (changelog.d/claude-md-investigate-empirically.added.md) independently reaches for the more accurate word — "prefer a run's own output to inference about it" — for the identical concept, which is good evidence the mismatch is real rather than a stretch.

This falls under this file's own item 3, "Challenge ambiguous phrasing and terminology".

Suggested change
- **Prefer the run's own artifacts to your inference about them.**
- **Prefer the run's own output to your inference about it.**

Comment thread CLAUDE.md
Comment on lines +932 to +934
A job that fails with no logs still has an error banner on its job page,
reachable with `WebFetch` on the run URL even when the API will not serve
it. gha#351/#352: an org-wide job failure was attributed to an Actions

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.

Minor factual-accuracy nit: this states unconditionally that a job's error banner is "reachable with WebFetch on the run URL even when the API will not serve it." WebFetch itself fails on authenticated/private URLs (and an unauthenticated GET to a private repo's Actions run page 404s rather than serving content), so this only holds for a public repo — exactly the qualifier the sibling section two sections earlier already applies carefully: "any public repo's files are fetchable with curl (or WebFetch)...".

Low severity/non-blocking: Morrison-Lab/gha itself and every repo currently in REVDEPS.md are public, so the bullet's advice is correct for the incident it cites (gha#351/#352) and for this repo as it stands today — and the section's own closing paragraph does mention "a plain public https://github.com/... URL", so the concept isn't entirely missing, just not carried into this specific bullet. Worth a one-clause fix (e.g. "...on the run URL — for a public repo — even when...") so a reader skimming just the bullet list doesn't come away with the unconditional version, especially if this section is ever promoted into the shared ai-config corpus for use in other (possibly private) repos.

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review

Reviewed the new ## Never just theorize -- investigate empirically CLAUDE.md section (+46 lines) and its matching changelog fragment (+10 lines) — a pure-prose PR, no code/workflow files touched.

Fact-checking: every checkable factual claim in the new text verified accurate against the real repo files and the cited issues — check-links/lychee.default.toml's accept list does include 301, lint-qmd/.markdownlint.qmd.jsonc's MD013 config matches exactly as quoted, the gha#351/#352 narratives (billing-banner misdiagnosis, qwt/rme/rpt org-move resolution) match those PRs' own content, and the "same principle already appears above" back-reference genuinely points to earlier content (line 703 vs. the new section at 919), not a forward reference. I also specifically checked whether lychee's redirect-following behavior undermines the third bullet's "redirect and error lists" method (it doesn't — lychee records every followed redirect in a distinct category, separate from a plain 200, so the described method is sound) and whether the new section's "test before asserting" rule contradicts the existing "Cheap self-check" shortcut elsewhere in the file (it doesn't — that shortcut is itself a previously-validated proxy, which is exactly the standard the new section's own second bullet sets).

Two smaller findings survived independent verification and are posted inline:

  • CLAUDE.md:949 — "artifacts" is ambiguous here: this file's only other uses of the word mean a literal GH Actions uploaded artifact, but the example given is lychee's printed console output. The PR's own changelog fragment already uses the more precise word ("output") for the same concept.
  • CLAUDE.md:932-934 — the claim that a job's error banner is reachable via WebFetch "even when the API will not serve it" is stated unconditionally, but only holds for public repos (as a sibling section two sections earlier carefully qualifies). Low severity/non-blocking since every repo currently in scope here is public.

No CLAUDE.md-compliance or bug-agent findings survived validation beyond these two; several other candidate issues (a lychee-redirect logic concern, a contradiction with the "Cheap self-check" section, two lower-severity completeness nits) were checked and ruled out as false alarms or too minor to raise.

Verdict

Ready for merge (the two inline findings above are minor wording/precision nits, non-blocking).

@github-actions

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


Code review

Reviewed the new ## Never just theorize -- investigate empirically CLAUDE.md section (+46 lines) and its matching changelog fragment (+10 lines) — a pure-prose PR, no code/workflow files touched.

Fact-checking: every checkable factual claim in the new text verified accurate against the real repo files and the cited issues — check-links/lychee.default.toml's accept list does include 301, lint-qmd/.markdownlint.qmd.jsonc's MD013 config matches exactly as quoted, the gha#351/#352 narratives (billing-banner misdiagnosis, qwt/rme/rpt org-move resolution) match those PRs' own content, and the "same principle already appears above" back-reference genuinely points to earlier content (line 703 vs. the new section at 919), not a forward reference. I also specifically checked whether lychee's redirect-following behavior undermines the third bullet's "redirect and error lists" method (it doesn't — lychee records every followed redirect in a distinct category, separate from a plain 200, so the described method is sound) and whether the new section's "test before asserting" rule contradicts the existing "Cheap self-check" shortcut elsewhere in the file (it doesn't — that shortcut is itself a previously-validated proxy, which is exactly the standard the new section's own second bullet sets).

Two smaller findings survived independent verification and are posted inline:

  • CLAUDE.md:949 — "artifacts" is ambiguous here: this file's only other uses of the word mean a literal GH Actions uploaded artifact, but the example given is lychee's printed console output. The PR's own changelog fragment already uses the more precise word ("output") for the same concept.
  • CLAUDE.md:932-934 — the claim that a job's error banner is reachable via WebFetch "even when the API will not serve it" is stated unconditionally, but only holds for public repos (as a sibling section two sections earlier carefully qualifies). Low severity/non-blocking since every repo currently in scope here is public.

No CLAUDE.md-compliance or bug-agent findings survived validation beyond these two; several other candidate issues (a lychee-redirect logic concern, a contradiction with the "Cheap self-check" section, two lower-severity completeness nits) were checked and ruled out as false alarms or too minor to raise.

Verdict

Ready for merge (the two inline findings above are minor wording/precision nits, non-blocking).

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $9.3532 (review) — run

@dem-extra1
dem-extra1 merged commit 64ef14f into main Jul 28, 2026
29 of 31 checks passed
@dem-extra1
dem-extra1 deleted the claude/review-pr-337-32tq6j branch July 28, 2026 23:34
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