CLAUDE.md says which files a test keeps free of a stated count (closes #2035) - #2039
Merged
Merged
Conversation
#2035) `CLAUDE.md`'s *Never state how many of anything a file holds* names the tests that enforce it -- `tests/release_notes_test.py` for `CHANGELOG.md` and `RELEASE_NOTES.md`, `tests/vendored_data_test.py` for `tests/_data/README.md` -- and names no file the rule governs without one. `SECURITY.md` is such a file, so the sentence reads as coverage the suite does not give. The decision is that it stays with the reading, and the file says so where it names the tests. What makes a test the instrument for the files it does name is a defect a reading cannot see. `merge=union` resolves an append-point collision by keeping both sides' added lines, so a branch carrying an edit to a count paragraph restores it on rebase with nothing in the merge output to say so; and `tests/_data/README.md`'s count is an edit every branch vendoring a vector makes, which is a conflict for the others and a wrong number where two of them pick the same one. `git check-attr merge -- SECURITY.md` answers `unspecified`, and its numerals sit in prose rather than at an append point, so an edit to one conflicts the ordinary way. Naming the file in `_FILES` is the small move and an inert one. The patterns there are keyed on the paragraph each forbids -- an entry count, the size of the breaking-changes list, and the cross-reference to it -- and none of them matches `SECURITY.md`, nor the sentence [ISS 2035](#2035) reports, which `6065ade8` repaired. A guard that cannot fail on its own subject is what `test_the_patterns_still_match` exists to refuse within the module's scope, and adding a file to the tuple would put one in the tree. A guard of `tests/vendored_data_test.py`'s shape -- a numeral anywhere, past an allowlist naming each permitted line verbatim -- does catch that sentence. Its cost is the allowlist: an exemption for every `path:line` citation, where an edit anywhere in a cited module moves the number and `tests/security_citations_test.py` is the gate that then reads it, and an exemption for every protocol constant, a key size and a field width among them. And the numeral the issue reports summarises a roster the sentence itself lists, which no pattern can tell from a sentence naming a structure -- `these three` for the call sites named beside it, `the other three`, `Those three` for the libsecp256k1 entry points. Each would take an exemption recording a judgement, which is the reading written down rather than a check performed. Whether the first two of those should lose their numeral the way this issue's own sentence did is [ISS 2038](#2038), filed from this branch's review: the roster they count is call sites of this tree and grows, where `Those three` closes the enumeration it counts. `tests/release_notes_test.py` carries the same decision at `_FILES`, which is where somebody minded to add a third file is standing. closes #2035 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sn2BerFVJcUtGF8Dm1Amd3
Contributor
There was a problem hiding this comment.
Sorry @fametrano, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 1 day and 3 hours by commenting @sourcery-ai review. Upgrade to get a review now.
Contributor
Reviewer's GuideThe PR clarifies that Flow diagram for the SECURITY.md count-rule decisionflowchart TD
A[SECURITY.md is governed by the count-wording rule] --> B{Does the existing test instrument fit?}
B -->|No| C[release_notes_test.py patterns match nothing in SECURITY.md]
B -->|No| D[merge attribute is unspecified; no silent restore mechanism]
B -->|No| E[vendored-data-style guard would require fragile exemptions]
C --> F[Keep SECURITY.md maintained by hand]
D --> F
E --> F
F --> G[Document the decision in CLAUDE.md, test comments, and CHANGELOG.md]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CLAUDE.md's Never state how many of anything a file holds named thetests that enforce it and no file the rule governs without one.
SECURITY.mdis such a file, so the sentence read as coverage the suitedoes not give. The decision is that it stays with the reading, and this is
that decision written where the rule is.
Three grounds, each measured rather than argued:
tests/release_notes_test.py's_FORBIDDENpatterns answerNoneagainst the pre-repair
SECURITY.md(git show 6065ade8^:SECURITY.md),the file that still said "being four other places a secret meets the
curve" — while answering true on 4 of 4
_RESURRECTEDstrings. Sonaming the file in
_FILESwould have been green on the exact sentenceISS 2035 was filed
for, which is what
test_the_patterns_still_matchexists to refuse.git check-attr mergeanswersunionforCHANGELOG.mdandRELEASE_NOTES.mdandunspecifiedforSECURITY.md— one run oversix paths, so the
unspecifiedis discriminated and not a silent zero.The silent-restore mechanism a test answers for is not there; what is
left is staleness, and the paragraph says the file is kept by hand.
guard of
tests/vendored_data_test.py's shape flags 48 of 481 proselines, 9 of them
path:linecitations, each wanting an exemption pinnedverbatim — where an edit anywhere in a cited module moves the number
tests/security_citations_test.pythen reads.What lands is a paragraph in
CLAUDE.md, a comment abovetests/release_notes_test.py's_FILESwhere somebody minded to add athird file is standing, and one
CHANGELOG.mdentry. 3 files, 43insertions, 0 deletions; no test function, id or collection count moves.
Gates at
2839272f, the pushed tip: lint exit 0 (47Passed, 0Failed), tree clean after;pytestexit 0, 32605 passed, 78 skipped,TOTAL 57179 0 9886 100.00%;sphinx-build -n -Wexit 0; thehref="#../"sweep exit 1 with a planted control the sweep exits 0 on;pinned
markdownlint-cli2v0.23.2 check-only exit 0, and exit 1 namingMD032/MD022on a copy with the blank line above this branch's ownheading deleted.
Reviewed at
3dd2efb3(CLEARED). Two changes since that sha, both proseand both from the review itself: the reviewer's non-blocking finding on
the
CLAUDE.mdparagraph's closing clause is taken, so the clause nolonger labels a live class of
SECURITY.mdsentences as descriptions thefile carries; and the commit body now points the question it raises at
ISS 2038, filed from
this review, rather than settling it here.
closes #2035
🤖 Generated with Claude Code
https://claude.ai/code/session_01Sn2BerFVJcUtGF8Dm1Amd3
Summary by Sourcery
Document why SECURITY.md remains outside the automated stated-count checks while clarifying the intended scope of the existing test.
Bug Fixes:
Enhancements:
Documentation: