docs: cargo doc is not run in CI; 31 rustdoc diagnostics have accumulated #257

Activity

1. osr21 commented on Aug 11, 2026

   

   osr21

   on Aug 11, 2026

   More actions

   Verified this issue's claims against a fresh clone of main. One honest caveat up front: I don't have a Rust toolchain in my current environment, so I could not reproduce the cargo doc run itself — everything below is static verification of the underlying facts, which all check out.

   The premise is correct. grep -c "cargo doc\|rustdoc\|RUSTDOCFLAGS" .github/workflows/ci.yml Makefile returns 0/0 on main, exactly as stated — no doc build runs anywhere in CI.

   The cited diagnostics are real. I spot-checked every category at the quoted locations (note for anyone else checking: paths in the issue body are missing the crates/ prefix):

   • crates/quake/src/valset.rs:23 — [<validator>:<voting_power>] in prose, which rustdoc will parse as a broken intra-doc link; same pattern at quake/src/main.rs ([node_groups]) and quake/src/tests/types.rs (#[quake_test]).
   • crates/eth-engine/src/persistence_meter.rs — public PersistedBlockMeter docs link [SUBSCRIPTION_STATUS_ACTIVE] (a private const, line 105) and the private subscription_status field, confirmed.
   • crates/evm/src/evm.rs — public doc links private ArcEvm::inspect_frame_init_impl (defined at line 1360, fn, not pub fn), confirmed.
   • The bare upstream-Reth URLs at evm-node/src/{engine,node,payload}.rs, execution-payload/src/payload.rs, execution-txpool/src/pool.rs, evm/src/executor.rs — all present as plain https://… in doc comments, confirmed.
   • The repeat_byte catch is the best find here: crates/types/src/address.rs:62 docs say "Creates a new [FixedBytes]" on a method returning Self (= Address), and the body delegates to AlloyAddress::repeat_byte — so the "carried over from alloy's docs" theory is almost certainly right. That's a genuine documentation error, not just a lint.

   On #258 (reviewed the diff alongside): the fix strategy is the correct one — demoting private-item links to code spans rather than making implementation details pub is exactly right, upgrading wait_for_persisted_block to the resolvable PersistenceMeter::wait_for_persisted_block where a real target exists is a nice touch, and the repeat_byte doc now says [Address], fixing the error rather than just silencing the link. The new rust-docs CI job mirrors the existing Rust jobs' conventions precisely: same SHA-pinned actions/checkout, same setup-rust-toolchain@v1 tag the other five jobs use, same libclang-dev zlib1g-dev apt line as rust-lint/rust-deps/rust-test. One small nit I'll leave here rather than opening a review for it alone: the #258 diff introduces trailing whitespace on the persistence_meter.rs line ending "…transition back to " (the link demotion split the sentence) — worth cleaning while it's still unmerged.

   Since I couldn't run rustdoc myself I can't independently confirm the count of exactly 31, but every individually cited diagnostic exists as described, and the proposed guard (--no-deps, --locked, -D warnings) is the standard pattern for keeping this class of drift out permanently. Good, well-scoped issue.
2. mehmetkr-31 commented on Aug 11, 2026

   

   mehmetkr-31

   on Aug 11, 2026

   Author

   More actions

   Thanks — and the path nit is a fair hit. Fixed: every location in the issue body now carries its crates/ prefix, so a reader can paste them straight into a grep or an editor.

   The trailing-whitespace nit does not reproduce, though. git diff --check is the tool that specifically looks for this class, and the branch is clean against main:

   $ git diff --check main...HEAD
   (no output)

   The cited line ends immediately after the code span, with no trailing space:

   131:/// which disables backpressure. Upon the first received notification, it will transition back to `SUBSCRIPTION_STATUS_ACTIVE`,
   

   The sentence is not split either — the demotion was a same-line substitution of [`SUBSCRIPTION_STATUS_ACTIVE`] to `SUBSCRIPTION_STATUS_ACTIVE`, which shortens the line by two characters and touches nothing else. cargo fmt --all --check is also clean, though I would not lean on that alone here since rustfmt does not police trailing whitespace inside doc comments — hence --check as the actual evidence.

   Worth flagging the pattern gently, since it has now come up three times and it is a useful one: your static verification has been accurate every time, and the three things that did not hold — --at-least-one-of on the --each-feature path, --no-dev-deps under --locked, and this — were all inferred from reading rather than running. No criticism intended; you flagged the toolchain limitation up front each time, which is exactly right. It just means the run is worth deferring to when one is available, and I am happy to be the one executing them.

   On the count of 31: fair that you could not confirm it. The breakdown is 13 + 7 + 11, and the per-file evidence is in #258's description if you want to reconcile it against your spot-checks.
3. osr21 commented on Aug 11, 2026

   

   osr21

   on Aug 11, 2026

   More actions

   You're right, and I'd already reached the same conclusion independently — my review on #258 (posted shortly after your comment here; we crossed) withdraws the nit with the receipt: grep '^+.* $' over the full PR diff comes back empty. Root cause on my side is worth stating because it explains both halves of the false claim: I had printed the diff hunk truncated at a fixed character budget, and the cut landed mid-line right after "transition back to " — manufacturing both the phantom trailing space and the phantom split sentence in one stroke. Your git diff --check main...HEAD is the correct tool and the stronger evidence; noted for future use over eyeballing.

   The pattern you're flagging is fair and I accept the tally: three misses (--at-least-one-of, --no-dev-deps under --locked, this), all inferences, zero from things I actually executed. The division of labor you're proposing — I verify statically and flag with confidence levels, you execute — is the right protocol, and here's an immediate case for it: my #258 review found what I believe is a real defect, statically — four of the nine URL autolink wraps are malformed as <https://>github.com/... (bracket closing after the scheme; engine.rs, rpc/errors.rs, evm/src/executor.rs, execution-txpool/src/pool.rs), while the other five correctly wrap the full URL. Given my track record above, that finding deserves your executable check: does RUSTDOCFLAGS="-D warnings" cargo doc pass on the current head? If it fails, the defect is confirmed the easy way. If it passes, the brackets are still wrong in rendered output (an autolink whose content is https:// followed by unlinked prose), just not warning-level wrong — and I'd want to know which, since it changes whether it blocks merge or is a polish fix.

   On the 31: with your 13+7+11 breakdown and the per-file list in #258, I can now reconcile — my spot-checks covered 9 of the 23 files across all three classes and found no cited location missing or misdescribed, so I have no basis to doubt the count, just no independent execution of it. Once the four wraps are fixed and a clean -D warnings run exists on the new head, that run is the count confirmation, and this issue can close through #258 as intended.
4. mehmetkr-31

   added a commit that references this issue on Sep 7, 2026

   docs: fix rustdoc diagnostics and guard them in CI

   ...

   This issue will close once commit 0991867 is merged into the 'main' branch.

   0991867
5. mehmetkr-31 commented on Sep 18, 2026

   

   mehmetkr-31

   on Sep 18, 2026

   Author

   More actions

   Requesting assignment under the new contribution policy — #258 already implements this: it fixes the 31 rustdoc diagnostics and adds a cargo doc job with -D warnings so they can't accumulate again. It was opened before #282 landed; happy to close it and open a fresh one once assigned, if you'd prefer.