Raise doc-comment coverage to the 80% threshold (#369) - #570
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
Validation
WalkthroughThe pull request adds workspace Rustdoc coverage measurement, enforces an 80% threshold in CI, documents Rust code and internal support modules, and adds tests for coverage measurement and related support changes. ChangesDocumentation coverage rollout
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 7 warnings)
✅ Passed checks (12 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Your free Security trial is over. An organization admin can activate billing to continue. Comment |
Reviewer's GuideAdds a workspace-wide Rustdoc doc-comment coverage gate (80% threshold) and raises coverage above that bar by documenting core modules and refactoring several files to stay within the 400-line cap, including extracting helper modules for cycle detection, status reporting, diagnostic JSON, time formatting, IR sorting, command error support, and discovery env handling. Flow diagram for the new doc-comment coverage gateflowchart LR
CI["GitHub Actions CI"]
Make_doc_coverage["make doc-coverage"]
Script_doc_coverage["scripts/doc-coverage.py"]
Cargo_rustdoc["cargo rustdoc --show-coverage"]
Threshold_check["coverage >= DOC_COVERAGE_THRESHOLD"]
Success["CI job succeeds"]
Failure["CI job fails"]
CI --> Make_doc_coverage
Make_doc_coverage --> Script_doc_coverage
Script_doc_coverage --> Cargo_rustdoc
Cargo_rustdoc --> Threshold_check
Threshold_check -->|pass| Success
Threshold_check -->|fail| Failure
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai Pre-merge check reconciliation for commit 4978deb:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== HEAD and requested commit =='
git rev-parse HEAD
git merge-base --is-ancestor 4978debc HEAD && echo '4978debc is an ancestor of HEAD'
echo '== Changed files for requested areas =='
git diff --name-status 4978debc^ 4978debc -- \
scripts/doc-coverage.py docs/developers-guide.md .github/workflows/ci.yml Makefile \
tests/makefile_test_target/rustflags.rs src/manifest/glob/validate.rs
echo '== doc-coverage structure =='
ast-grep outline scripts/doc-coverage.py --items all --type function,class
grep -nE '^(def (pinned_toolchain|load_metadata|main)|[[:space:]]*except (RuntimeError|OSError|tomllib\.TOMLDecodeError|json\.JSONDecodeError)|[[:space:]]*raise RuntimeError)' scripts/doc-coverage.py
echo '== relevant doc-coverage implementation =='
sed -n '/^def pinned_toolchain/,/^def doc_targets/p' scripts/doc-coverage.py
sed -n '/^def load_metadata/,/^def main/p' scripts/doc-coverage.py
sed -n '/^def main/,/\A/p' scripts/doc-coverage.py
echo '== CI, Makefile, developer guide =='
grep -n -C 4 'doc-coverage' .github/workflows/ci.yml Makefile docs/developers-guide.md
echo '== named test coverage =='
grep -n -C 5 -E 'doc_coverage|validate_brace_matching_reports_the_outermost_unclosed_brace' \
tests/makefile_test_target/rustflags.rs src/manifest/glob/validate.rsLength of output: 1830 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== doc-coverage structure =='
ast-grep outline scripts/doc-coverage.py --items all --type function,class
echo '== error handling markers =='
grep -nE '^(def (pinned_toolchain|load_metadata|main)|[[:space:]]*except (RuntimeError|OSError|tomllib\.TOMLDecodeError|json\.JSONDecodeError)|[[:space:]]*raise RuntimeError)' scripts/doc-coverage.py || true
echo '== pinned_toolchain =='
sed -n '/^def pinned_toolchain/,/^def doc_targets/p' scripts/doc-coverage.py
echo '== load_metadata =='
sed -n '/^def load_metadata/,/^def main/p' scripts/doc-coverage.py
echo '== main =='
sed -n '/^def main/,$p' scripts/doc-coverage.py
echo '== CI, Makefile, developer guide references =='
grep -n -C 5 'doc-coverage' .github/workflows/ci.yml Makefile docs/developers-guide.md || true
echo '== named test coverage =='
grep -n -C 6 -E 'doc_coverage|validate_brace_matching_reports_the_outermost_unclosed_brace' \
tests/makefile_test_target/rustflags.rs src/manifest/glob/validate.rs || trueLength of output: 10159 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Locate named tests and Makefile contract model =='
rg -n -C 5 -i \
'RustflagsCase|doc[_-]coverage|outermost.*unclosed|brace.*matching' \
--glob '*.rs' --glob '*.py' --glob 'Makefile' .
echo '== Measurement and aggregation implementation =='
sed -n '/^def measure/,/^def label/p' scripts/doc-coverage.py
sed -n '/^def run_measurements/,/^def load_metadata/p' scripts/doc-coverage.py
echo '== All process launches in coverage tool =='
rg -n -C 8 'subprocess\.run\s*\(' scripts/doc-coverage.py
echo '== Relevant test-file discovery =='
fd -t f -e rs . tests | sort | grep -E '(rustflags|validate|glob)' || trueLength of output: 31052
Keep the Unit Architecture check unresolved. Wrap The other static checks pass:
Do not treat the stated manual validation and CI execution as independently verified. This sandbox cannot execute repository code or test commands. 🧠 Learnings usedYou are interacting with an AI system. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai please provide an AI agent prompt requiring addition of the following (with inflected variants) to the local typos dictionary "otherwize" -> "otherwise", "exercize" -> "exercise", "raize" -> "raise" |
This comment was marked as resolved.
This comment was marked as resolved.
|
Warning Your free Security trial is over. An organization admin can activate billing to continue. |
There was a problem hiding this comment.
Sorry @leynos, your pull request is larger than the review limit of 150000 diff characters
|
@coderabbitai please create a new GitHub issue against "otherwize" = "otherwise"
"exercize" = "exercise"
"exercized" = "exercised"
"exercizes" = "exercises"
"exercizing" = "exercising"
"raize" = "raise"
"raized" = "raised"
"raizes" = "raises"
"raizing" = "raising" |
Document propagated and validation errors on fallible helpers, and correct path and temporary-name descriptions to match the implementation.
Document verified fallible contracts and correct review-era documentation. Reject malformed Rustdoc coverage JSON through the controlled error path, secure Makefile interpolation, and allow metrics recorder installation retries.
Document the UTF-8 fallback that makes build-file canonicalization failure non-fatal, while retaining the two genuine error conditions.
Separate module and method documentation guidance, keep the coverage recipe safe for configurable tools, and preserve controlled malformed-payload errors.
Permit the which resolver siblings to use the Windows candidate builder without widening it beyond crate::stdlib::which.
|
@coderabbitai Please investigate the cause of the following issue using codegraph exploration and research, identify a fix and provide an AI coding agent prompt for the fix: |
36624b3 to
e6c9c70
Compare
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Current HEAD =='
git rev-parse HEAD
echo '== Makefile target and Rustflags variables =='
rg -n -C 10 \
'^(doc-coverage|doc-coverage-test|RUSTFLAGS|RUSTDOCFLAGS|POLONIUS|CARGO|export)[[:space:]:?+]?|polonius|doc-coverage\.py' \
Makefile
echo '== Contract-test structure =='
ast-grep outline tests/makefile_test_target/rustflags.rs --items all --type function,struct,enum,impl
rg -n -C 12 \
'doc_coverage|doc-coverage|Polonius|polonius|RustflagsCase|behavioural_rustflags_recipes_are_well_formed_without_inherited_flags|expanded' \
tests/makefile_test_target/rustflags.rs
echo '== Relevant complete regions =='
sed -n '1,220p' Makefile
sed -n '1,260p' tests/makefile_test_target/rustflags.rs
echo '== Similar Makefile recipes that pass flags through the environment =='
rg -n -C 8 \
'\$\$RUSTFLAGS|\$\$RUSTDOCFLAGS|\$\$POLONIUS|RUSTFLAGS=.*polonius|RUSTDOCFLAGS=.*polonius' \
Makefile tests/makefile_test_targetLength of output: 8046 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Rustflags test map =='
ast-grep outline tests/makefile_test_target/rustflags.rs --items all --type function,struct,enum
echo '== doc-coverage case and contract evaluator =='
rg -n -C 16 \
'doc_coverage|doc-coverage|POLONIUS_FLAGS|polonius|RustflagsCase|behavioural_rustflags_recipes_are_well_formed_without_inherited_flags|expanded|environment|variables' \
tests/makefile_test_target/rustflags.rs \
tests/makefile_test_target
echo '== Full Rustflags test module =='
cat -n tests/makefile_test_target/rustflags.rs
echo '== Makefile variable and recipe references =='
rg -n -C 5 \
'POLONIUS_FLAGS|doc-coverage|RUSTFLAGS|RUSTDOCFLAGS|export' \
MakefileLength of output: 96959
The failure is valid in the test harness. It is not a
RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }$${POLONIUS_FLAGS}"This form avoids interpolating a configurable value into the shell command line. In an actual The contract test does not model that exported variable.
You are interacting with an AI system. |
Satisfy the private-item documentation policy for the Windows workspace resolver without changing its lookup behavior.
Supply the Make-exported runtime variable to isolated shell evaluation so the secure doc-coverage recipe is tested faithfully.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message in a code block that I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. Complex Methodscripts/doc-coverage.py: parse_coverage_output What lead to degradation?parse_coverage_output has a cyclomatic complexity of 10, threshold = 9 Why does this problem occur?A Complex Method has a high cyclomatic complexity. The recommended threshold for the Python language is a cyclomatic complexity lower than 9. How to fix it?There are many reasons for Complex Method. Sometimes, another design approach is beneficial such as a) modeling state using an explicit state machine rather than conditionals, or b) using table lookup rather than long chains of logic. In other scenarios, the function can be split using EXTRACT FUNCTION. Just make sure you extract natural and cohesive functions. Complex Methods can also be addressed by identifying complex conditional expressions and then using the DECOMPOSE CONDITIONAL refactoring. Helpful refactoring examplesTo get a general understanding of what this code health issue looks like - and how it might be addressed - we have prepared some diffs for illustrative purposes. SAMPLE# complex_method.js
function postItem(item) {
if (!item.id) {
- if (item.x != null && item.y != null) {
- post(item);
- } else {
- throw Error("Item must have x and y");
- }
+ // extract a separate function for creating new item
+ postNew(item);
} else {
- if (item.x < 10 && item.y > 25) {
- put(item);
- } else {
- throw Error("Item must have an x and y value between 10 and 25");
- }
+ // and one for updating existing items
+ updateItem(item);
}
}
+
+function postNew(item) {
+ validateNew(item);
+ post(item);
+}
+
+function updateItem(item) {
+ validateUpdate(item);
+ put(item);
+}
+ |
This comment was marked as resolved.
This comment was marked as resolved.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Current PR head =='
git rev-parse HEAD
gh pr view 570 --repo leynos/netsuke --json headRefOid,url --jq '{url, headRefOid}'
echo '== Coverage tool interface and failure handling =='
ast-grep outline scripts/doc-coverage.py --items all --type function,class
rg -n -C 8 \
'^(def (measure|load_metadata|main|run_measurements|doc_targets|doc_able_targets|parse_coverage_output|pinned_toolchain|parse_threshold)|class (Coverage|DocTarget))|subprocess\.run\s*\(|except OSError|except RuntimeError|raise RuntimeError' \
scripts/doc-coverage.py
echo '== Coverage tool test map =='
ast-grep outline scripts/tests/test_doc_coverage.py --items all --type function,class
rg -n -C 5 \
'^(def test_|def (single_library_metadata|metadata_for)|class (FakeCargo|FakeResult|CoveragePayloadFailureCase|RustdocFailureCase))|pytest\.mark\.parametrize|script\.(measure|load_metadata|main|run_measurements|doc_targets|parse_coverage_output|pinned_toolchain|parse_threshold)' \
scripts/tests/test_doc_coverage.py
echo '== Makefile doc-coverage variable handling =='
rg -n -C 10 \
'^(doc-coverage|doc-coverage-test):|DOC_COVERAGE_(THRESHOLD|TOOLCHAIN)|export |scripts/doc-coverage\.py|RUSTDOCFLAGS|POLONIUS_FLAGS' \
Makefile
echo '== Developer guide internal-module boundaries =='
rg -n -i -C 8 \
'internal support module|support-module|sort_utils|diagnostic_json_support|error_support|time/format|400-line|design record|ADR' \
docs/developers-guide.md
echo '== Relevant current-PR diff =='
base="$(git merge-base origin/main HEAD 2>/dev/null || git merge-base main HEAD 2>/dev/null || true)"
if [ -n "$base" ]; then
git diff --unified=3 "$base" HEAD -- \
scripts/doc-coverage.py scripts/tests/test_doc_coverage.py Makefile docs/developers-guide.md
fiLength of output: 60077
No GitHub issue is required. No remaining-work prompt is required. This verification is static. Run You are interacting with an AI system. |
Reject malformed Rustdoc count values before aggregating coverage. Keep every invalid payload on the controlled measurement-error path.
Move payload aggregation and count conversion into focused helpers. Preserve all target-qualified measurement-error diagnostics.
|
@coderabbitai Have the following now been resolved? scripts/doc-coverage.py (1)
🤖 Detailed instructionsUse a wyvern agent team to verify each finding against the current code and only fix it if needed. Fix any documentation issues using a scribe agent team. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Use a scrutineer agent for execution of tests and summarizing the results. In |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Current PR head =='
git rev-parse HEAD
gh pr view 570 --repo leynos/netsuke --json headRefOid,url --jq '{url, headRefOid}'
echo '== Production and test symbol maps =='
ast-grep outline scripts/doc-coverage.py scripts/tests/test_doc_coverage.py \
--items all --type function,class
echo '== Coverage aggregation, error translation, and controlled exit handling =='
rg -n -C 12 \
'^(def (aggregate_coverage_payload|parse_coverage_output|coverage_json_error|main|measure|run_measurements)|class (Coverage|DocTarget))|OverflowError|isfinite|with_docs|non-negative|integer|exit code|return 2|return 2\b' \
scripts/doc-coverage.py
echo '== Focused invalid-count tests and test helpers =='
rg -n -C 12 \
'^(def test_.*(coverage|count|invalid|nonfinite|negative|integer|inconsistent|main)|class CoveragePayloadFailureCase)|parse_coverage_output|aggregate_coverage_payload|OverflowError|1e309|with_docs|total' \
scripts/tests/test_doc_coverage.py
echo '== Exact current implementation and test regions =='
for spec in \
'scripts/doc-coverage.py:175:270' \
'scripts/doc-coverage.py:335:390' \
'scripts/tests/test_doc_coverage.py:210:360'
do
file="${spec%%:*}"
rest="${spec#*:}"
start="${rest%%:*}"
end="${rest##*:}"
echo "-- $file, Lines $start-$end --"
cat -n "$file" | sed -n "${start},${end}p"
done
echo '== Relevant change at current head =='
git diff --unified=5 HEAD^ HEAD -- \
scripts/doc-coverage.py scripts/tests/test_doc_coverage.py || trueLength of output: 35019
The test verifies the controlled This verification is static. Run You are interacting with an AI system. |
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat o11y, code safety, documentation and validation coverage as in scope).
|
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat o11y, code safety, documentation and validation coverage as in scope). ❌ Failed checks (1 error, 6 warnings)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Current PR head =='
git rev-parse HEAD
gh pr view 570 --repo leynos/netsuke --json headRefOid,url --jq '{url, headRefOid}'
echo '== Relevant symbol maps =='
ast-grep outline \
src/stdlib/which/env.rs \
src/stdlib/which/env_path_support.rs \
src/stdlib/which/lookup/workspace/posix.rs \
src/stdlib/collections.rs \
src/stdlib/which/options.rs \
--items all --type function,struct,impl
echo '== EnvSnapshot capture contracts and implementation context =='
rg -n -C 14 \
'^\s*/// # Errors|^\s*/// # Errors:|^\s*(pub(?:\([^)]+\))?\s+)?fn\s+(capture|capture_with_env|capture_with_pathext|capture_impl|capture_common)\b|struct EnvSnapshot' \
src/stdlib/which/env.rs
echo '== Related Result contracts =='
rg -n -C 10 \
'^\s*/// # Errors|^\s*(pub(?:\([^)]+\))?\s+)?fn\s+(search_workspace|collect_matching_executables|process_workspace_entry|uniq_filter|collect_flattened_values|resolve_group_key|from_kwargs|current_dir_utf8)\b' \
src/stdlib/which/lookup/workspace/posix.rs \
src/stdlib/collections.rs \
src/stdlib/which/options.rs \
src/stdlib/which/env_path_support.rs
echo '== Recent changes to the affected files =='
git diff --unified=4 HEAD^ HEAD -- \
src/stdlib/which/env.rs \
src/stdlib/which/env_path_support.rs \
src/stdlib/which/lookup/workspace/posix.rs \
src/stdlib/collections.rs \
src/stdlib/which/options.rs || trueLength of output: 1979 🏁 Script executed: #!/bin/bash
set -u
echo '== Current PR head =='
git rev-parse HEAD
gh pr view 570 --repo leynos/netsuke --json headRefOid,url --jq '{url, headRefOid}' || true
echo '== Relevant symbol maps =='
ast-grep outline \
src/stdlib/which/env.rs \
src/stdlib/which/env_path_support.rs \
src/stdlib/which/lookup/workspace/posix.rs \
src/stdlib/collections.rs \
src/stdlib/which/options.rs \
--items all --type function,struct,enum,class || true
echo '== EnvSnapshot capture contracts and implementation context =='
rg -n -C 14 \
'^\s*/// # Errors|^\s*/// # Errors:|^\s*(pub(?:\([^)]+\))?\s+)?fn\s+(capture|capture_with_env|capture_with_pathext|capture_impl|capture_common)\b|struct EnvSnapshot' \
src/stdlib/which/env.rs || true
echo '== Exact environment capture regions =='
cat -n src/stdlib/which/env.rs | sed -n '55,330p' || true
echo '== Related Result contracts =='
rg -n -C 10 \
'^\s*/// # Errors|^\s*(pub(?:\([^)]+\))?\s+)?fn\s+(search_workspace|collect_matching_executables|process_workspace_entry|uniq_filter|collect_flattened_values|resolve_group_key|from_kwargs|current_dir_utf8)\b' \
src/stdlib/which/lookup/workspace/posix.rs \
src/stdlib/collections.rs \
src/stdlib/which/options.rs \
src/stdlib/which/env_path_support.rs || true
echo '== Recent changes to affected files =='
git diff --unified=4 HEAD^ HEAD -- \
src/stdlib/which/env.rs \
src/stdlib/which/env_path_support.rs \
src/stdlib/which/lookup/workspace/posix.rs \
src/stdlib/collections.rs \
src/stdlib/which/options.rs || true
exit 0Length of output: 41154
The checks are partially resolved at current head
Keep this finding open. This work is in scope for the PR. Do not create a follow-up issue. Apply this documentation-only change: The inspection was static. Run the listed validation commands before closing the finding. You are interacting with an AI system. |
Closes #369
Summary
Lifts Rustdoc doc-comment coverage across the workspace from ~65% to over 98% and gates it in CI so the bar cannot regress.
Changes
scripts/doc-coverage.pyrunscargo rustdoc --show-coverageover every workspace lib and bin target, counts private items, and fails below a threshold;make doc-coverage(threshold and toolchain overridable), a CI step alongsidemake lint, and the 80% policy recorded in AGENTS.md.sort_utilssibling module.///docs across the CLI, graph_view, stdlib, runner, process, manifest, AST, test_support, and build_l10n_audit modules. Modules at the 400-line Whitaker cap were split into#[path]siblings (diagnostic_json, status, stdlib/time, cli/discovery, stdlib/command/error, ir/cycle).impl-block methods and only excludes trait-implementation overrides; the script's docstring and AGENTS.md now state that accurately.Notes
Display::fmt,Serialize,Drop, ...) and#[test]items are exempt because rustdoc does not count them.cargo doc --no-depsis warning-free,make doc-coveragereports ~98%, the Whitaker module-cap lint is clean, andmake testis green.Review feedback
scripts/tests/test_doc_coverage.pyadds a 15-case pytest suitethat mocks the script's two subprocess boundaries, so target discovery,
aggregation, threshold exits, malformed cargo output, command failures, and
CLI/toolchain handling are tested in isolation.
make doc-coverage-testrunsit and
make doc-coveragenow depends on it.subprocess.runcalls now catchOSErrorandsurface it as the script's controlled measurement error instead of a bare
traceback; malformed metadata JSON is rejected with an explicit error.
configurable toolchain/threshold into shell quotes; both values are exported
and read from the environment at shell runtime, closing a command-injection
surface.
#[path]support modules(
sort_utils,diagnostic_json_support,command/error_support,time/format) are documented in the developers' guide with ownership,permitted callers, and the 400-line split rule.
References
🤖 Generated with Claude Code
Summary by Sourcery
Raise workspace Rustdoc coverage above the required threshold and enforce it continuously in development and CI.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: