Code Review

<!-- GENERATED BY cargo-anvil. DO NOT EDIT DIRECTLY. -->

Code review

This skill applies when reviewing pull requests. It does not apply to the coding agent when it writes code — see AGENTS.md for that.

Every rule below exists because a review comment was filed, argued, and withdrawn. The cost of a wrong comment is not zero: the author has to reproduce your claim, disprove it, and write the rebuttal.

The evidence rule

Comment only on what you can see in the diff, in a file you have read, or in this repository's own configuration. Anything else — what an API's signature is, what a constant's value is, what compiles, what the local convention is — is a guess. If you cannot point at the evidence, do not file the comment.

When you do file one, name the evidence: the file and line you read, the config key you checked, the sibling cases you compared against.

1. Never predict build, lint, or test outcomes

Every pull request is validated by CI. Do not claim code "won't compile", "fails to build", "has a type error", "is missing an import", "won't link", or "will throw at runtime" — CI is the source of truth, and you are not running it.

This is the single largest source of withdrawn comments. Real examples, all wrong:

• "size_of is not in the prelude" — it is, in edition 2024, and this workspace's Clippy rejects the explicit import as redundant.
• "Future is not in scope" — also in the 2024 prelude.
• "Duration::from_mins does not exist" — it exists on the pinned toolchain, and a Clippy lint here requires it over from_secs(15 * 60).
• "LocalKey has no set, use with(|cell| …)" — LocalKey<Cell<T>> has set/get.
• "array::from_fn takes two generic parameters" — it takes three (T, N, and the closure).
• "self.0.1 is not valid tuple access" — rustc splits a float literal in field position back into .0 and .1.
• "splatting a List[string] will throw" — PowerShell splatting enumerates generic lists.

If you believe there is a real logic, correctness, security, or design problem, describe that without predicting a compiler, linter, or test result.

2. Do not assert an API from memory

Signatures, generic arity, trait bounds, and constant values are the things you are most confident and most often wrong about. Before writing "X does not exist", "X takes N arguments", or "X equals V":

• Read the definition, or the vendored header, or the crate's docs.
• If you cannot, say what you observed and ask, rather than asserting.

One withdrawn comment built an entire boundary-condition bug report on WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH being u32::MAX. It is 0.

3. Check the edition and toolchain before "not in scope"

A missing use is not evidence of a missing import. Read Cargo.toml for edition and rust-toolchain.toml for the pinned version first. The 2024 prelude added Future, IntoFuture, and the std::mem size/align functions, among others — code that looks under-imported for edition 2015 is usually correct.

The same applies to MSRV: a constructor you do not recognise may simply be newer than your training data and older than the pin.

4. Convention claims need evidence from this repo

Do not write "this repo prefers X" or "the convention here is Y" from a single observed file. Before claiming a convention:

1. Check the machine-enforced source first — clippy.toml, rustfmt.toml, lint tables, AGENTS.md, .editorconfig. If a tool already permits both forms, there is no convention to enforce.
2. Count the counter-examples. "Prefer unwrap over expect in tests" was filed against a workspace where 60+ test files use expect, and where clippy.toml sets allow-unwrap-in-tests = true precisely to allow both.
3. Never contradict a finding you made earlier in the same review.

If a convention is real but only partly applied, say so and scope the request — filing it on five of twenty sites leaves the codebase less consistent than before, not more.