docs: nine claims that were not true, and a rule that stays narrow because it was measured - #221
Merged
Merged
Conversation
…cause it was measured All from an adversarial review of the range that landed today, most of them written by the change that was correcting something else. Corrections carry a higher defect rate than original writing, because a small local edit gets less scrutiny than new code — every long-standing count in this repository verified, and the three wrong ones were written yesterday. money-safety §7 said "answer 200 anyway" for an unsigned delivery, flatly. Every Mollie delivery is unsigned; that is the premise of its whole webhook path. So an agent told to read this skill before writing a handler got the rule #207 had just spent a commit removing. It now carries the exception, which is that a `verify` reaching the network has an `Err` meaning two different things. §3's count was wrong in both halves: nine status fallbacks across five adapters, not eight, and PayTR's ninth refuses an undocumented status outright rather than landing on `Pending` — which is the best of the three answers and the sentence excluded it. `scripts/requirements.txt` said the two named packages pull nineteen; twenty lines, so eighteen. `coverage.py`'s docstring said four provider sections report; three do, and PayPal has none at all — which is the thing #214's own commit message noticed and then fixed in the role file rather than in the script. Two doc comments still said `fraudStatus` is read "the way this crate has always read it", in the change that altered how it is read. The CHANGELOG entry for that named `Provider::charge` and `reporting` as the paths it reached and not `Provider::capture`, whose `/payment/postauth` answer carries the same field. The CHANGELOG for the amount fallback called `price` "the correct figure". It is the basket total; `paidPrice` is what was collected, and they differ by an instalment surcharge this crate models itself. #220 is where that is argued rather than assumed, because changing it means changing #172's decision. `kasapay-review.md` said the role "is registered without the tools to" write. `Edit` and `Write` are absent; `Bash` is not, and `git commit` is reachable through it. Stated as a fence it was not. UNVERIFIED.md D5 records what `Webhook::verify`'s character class assumes about Mollie identifiers. Every comparable reading landed this week got an entry and this one did not, and it is on the delivery path. And the seam detector stays narrow. The review asked it to skip a bare `///` between two blocks, since rustdoc takes the first paragraph either way. Widened that far it matches **380** doc comments, because a summary ending in a full stop, a blank `///`, then a paragraph is the *correct* shape — the rule would have flagged nearly every well-formed comment in the workspace. Fifty means the rule is wrong somewhere; 380 means it is very wrong. The limit is recorded with the number that decided it, so nobody widens it again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 10 minutes Limit details: You’ve used all 10 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
Comment |
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.
The remaining findings from the adversarial review of today's range. Most were written by the change that was correcting something else — every long-standing count in this repository verified, and the three wrong ones were written yesterday. A correction feels small, local and safe, so it gets less scrutiny than new code.
The one that mattered
money-safety §7 said "answer
200anyway" for an unsigned delivery, flatly. Every Mollie delivery is unsigned — that is the premise of its whole webhook path. So an agent told by CLAUDE.md and by every role file to read this skill before writing a webhook handler got the rule #207 had just spent a commit removing. It now carries the exception: averifythat reaches the network has anErrmeaning two different things.Counts, recounted
Pending". It is nine status fallbacks across five adapters, and PayTR's ninth refuses an undocumented status outright — the best of the three answers, and the sentence excluded it.requirements.txtsaid the two named packages pull nineteen. Twenty==lines, so eighteen.coverage.py's docstring said "the other four providers' sections report". Three do. PayPal has none at all — which is what chore(agents): the roles describe the workspace they are in, and money-safety gains a ninth #214's own commit message noticed, and then fixed in the role file rather than in the script that CI runs.Claims about behaviour
Two doc comments still said
fraudStatusis read "the way this crate has always read it" — in the change that altered how it is read. And that CHANGELOG entry namedProvider::chargeandreportingas the paths it reached, notProvider::capture, whose/payment/postauthanswer carries the same field.The CHANGELOG for the amount fallback called
price"the correct figure sitting unused". It is the basket total;paidPriceis what was collected, and they differ by an instalment surcharge this crate models itself. #220 argues that rather than assuming it, because changing the behaviour means changing #172's decision.kasapay-review.mdsaid the role "is registered without the tools to" write.EditandWriteare absent;Bashis not, andgit commitis reachable through it. Stated as a fence it was not.UNVERIFIED.mdD5 records what Mollie's identifier character class assumes. Every comparable reading landed this week got an entry; this one, on the delivery path, did not.The measurement that refused a fix
The review asked the seam detector to skip a bare
///between two blocks, since rustdoc takes the first paragraph as the summary either way. Reasonable — so I measured it before writing it.380 matches.
Because a summary line ending in a full stop, then a blank
///, then a paragraph is the correct shape for a doc comment. Widening the rule would have flagged nearly every well-formed one in the workspace. Fifty means the rule is wrong somewhere; 380 means it is very wrong.So the narrow rule is the right rule, and the limit is now recorded with the number that decided it — including what it leaves uncaught — so nobody widens it again.
CI decides.