docs: fix #431's false snapshot sentence; record the second-copies rule and review ritual in CLAUDE.md - #513
Merged
Merged
Conversation
The snapshots section told a contributor that a hand-edited snapshot "fails in the Rust suite rather than in review". It does not. The Rust tests hold the partition, and ordering is not a partition property, so the misordered file on #427 passed 4/4 locally and was rejected by CI's public-api job three hours later. Say which half is caught where, and that a hand-edit is provisional until CI agrees. Closes #431.
Two decisions from the 2026-08 review that were resolved on the tickets and never written down. #319: a second hand-maintained copy of a fact needs a diff test named beside it, no generator required, enforced by the guards that already exist plus review attention. #323: architecture reviews are chartered as wayfinder maps when the reviewer judges enough has changed, and the every-time mechanism is the fresh-context adversarial review of each PR by an agent that did not write it. CLAUDE.md is a symlink to AGENTS.md, so the section lands there. Refs #319, #321, #323.
Reviewer's GuideCorrects the public API snapshot documentation to accurately separate local partition checks from CI ordering validation, and records resolved repository practices for second-copy safeguards and review responsibilities in the standing rules shared through Sequence diagram for fresh-context PR reviewsequenceDiagram
participant Author
participant PR
participant Reviewer as FreshContextAgent
Author->>PR: Submit PR
PR->>Reviewer: Request adversarial review
Reviewer->>PR: Review without author context
Reviewer-->>Author: Report defects or approve
opt Enough architecture has changed
Reviewer->>PR: Charter wayfinder map review
end
Flow diagram for public API snapshot validationflowchart LR
Edit[Hand-edit snapshot] --> Local[public_api_snapshots.rs partition checks]
Local -->|wrong tier| Fail[Local Rust suite fails]
Local -->|correct tier| CI[CI public-api job regenerates and diffs]
CI -->|wrong ordering| Reject[CI rejects snapshot]
CI -->|matches generated output| Accept[Snapshot accepted]
Flow diagram for second-copy safeguardsflowchart LR
Fact[First copy of fact] --> Copy[Second hand-maintained copy]
Copy --> Guard[Test named beside copy]
Guard --> Diff[Diffs second copy against first]
Diff -->|matches| Allowed[Copy allowed]
Diff -->|drifts| Fix[Review identifies and fixes drift]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="AGENTS.md" line_range="260" />
<code_context>
+ because the charter month produced three reviews unprompted and the trigger was
+ never the missing part. The mechanism that does run every time is smaller: the
+ fresh-context adversarial review of each PR by an agent that did not write it,
+ which is where the last three real defects were caught — #427's layering
+ violation, #415's four-hole guard, #428's dropped `Absent`.
</code_context>
<issue_to_address>
**nitpick:** The review-ritual paragraph misidentifies #427's defect as a layering violation, while this change's own rationale identifies #427 as the misordered `public-api.rest.txt` snapshot that passed local tests and failed CI. This gives readers an incorrect example of the defects supposedly caught by fresh-context review.
**Suggested fix:** Replace "#427's layering violation" with the misordered public-API snapshot defect, or cite the issue that actually contained the layering violation.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| because the charter month produced three reviews unprompted and the trigger was | ||
| never the missing part. The mechanism that does run every time is smaller: the | ||
| fresh-context adversarial review of each PR by an agent that did not write it, | ||
| which is where the last three real defects were caught — #427's layering |
There was a problem hiding this comment.
nitpick: The review-ritual paragraph misidentifies #427's defect as a layering violation, while this change's own rationale identifies #427 as the misordered public-api.rest.txt snapshot that passed local tests and failed CI. This gives readers an incorrect example of the defects supposedly caught by fresh-context review.
Suggested fix: Replace "#427's layering violation" with the misordered public-API snapshot defect, or cite the issue that actually contained the layering violation.
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
1 participant
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 changes in here: the second commit adds a standing-rules section to
AGENTS.md(whichCLAUDE.mdsymlinks to), recording two decisions that were resolved on their tickets and never written down. It is a separate commit so it can be read on its own.1.
docs/development.md: say where a hand-edited snapshot is actually caught (#431)The snapshots section told a contributor that a hand-edited snapshot "fails in the Rust suite rather than in review". It does not.
public_api_snapshots.rsholds the partition (every row in exactly one file, promise rows at anapipath), and ordering is not a partition property, so #427's misorderedpublic-api.rest.txtpassed 4/4 locally and was rejected by CI'spublic-apijob three hours later. Sincecargo-public-apineeds nightly and a pinned version, the devcontainer cannot regenerate at all, which makes hand-editing the normal path in there rather than an exception.The sentence now says which half is caught where, and that a hand-edit is provisional until the
public-apijob agrees.Option two on the ticket, teaching the Rust test the generator's ordering, is not taken. The ticket's own condition was that the rule be shown to be derivable first:
cargo public-api's order is its own, not plainsort, and encoding a guess would be worse than the gap it closes. Option three (regeneration inside the container) stays rejected for the reason the ticket names, that it needs nightly plus a pinned tool and any.devcontainer/change invalidates the prebuild.2.
CLAUDE.md: two standing rules (#319, #321, #323)Both were decided and closed on their tickets, with the closing comments saying "recorded in CLAUDE.md (PR to follow)". This is that PR.
docs/, because a register is itself a second copy and would need its own guard. Enforcement is nothing new, just the existing guards plus a reviewer noticing.Checks
test_agents_doc.py,test_docs_prose.py,test_public_api_snapshots_doc.py,test_docs_links.py: 99 passed. The reworded paragraph keeps the### The public-API snapshotsheading, thescripts/public-api-snapshots.shmention, and thecanonical/ #352 phrases those guards match on, and adds no em or en dash todocs/.Closes #431.
🤖 Generated with Claude Code
Summary by Sourcery
Clarify public API snapshot validation and record the project’s standing rules for duplicated facts and review practices.
Bug Fixes:
Enhancements:
Documentation: