Skip to content

Raise doc-comment coverage to the 80% threshold (#369) - #570

Merged
leynos merged 53 commits into
mainfrom
issue-369-raise-doc-comment-coverage-to-80-threshold
Aug 25, 2026
Merged

Raise doc-comment coverage to the 80% threshold (#369)#570
leynos merged 53 commits into
mainfrom
issue-369-raise-doc-comment-coverage-to-80-threshold

Conversation

@leynos

@leynos leynos commented Aug 19, 2026

Copy link
Copy Markdown
Owner

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

  • Measurement and enforcement: new scripts/doc-coverage.py runs cargo rustdoc --show-coverage over every workspace lib and bin target, counts private items, and fails below a threshold; make doc-coverage (threshold and toolchain overridable), a CI step alongside make lint, and the 80% policy recorded in AGENTS.md.
  • PR Kani harnesses for manifest-to-IR safety checks (4.2.1) #336 IR helpers: documented the manifest-to-IR functions, registration and error-message helpers, and the Kani harness generators, including a sort_utils sibling module.
  • Codebase sweep: added /// 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).
  • Metric correction: an empirical probe showed rustdoc counts inherent impl-block methods and only excludes trait-implementation overrides; the script's docstring and AGENTS.md now state that accurately.

Notes

  • Trait-impl methods (Display::fmt, Serialize, Drop, ...) and #[test] items are exempt because rustdoc does not count them.
  • All gates pass: cargo doc --no-deps is warning-free, make doc-coverage reports ~98%, the Whitaker module-cap lint is clean, and make test is green.

Review feedback

  • Testing: scripts/tests/test_doc_coverage.py adds a 15-case pytest suite
    that 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-test runs
    it and make doc-coverage now depends on it.
  • Unit architecture: both subprocess.run calls now catch OSError and
    surface it as the script's controlled measurement error instead of a bare
    traceback; malformed metadata JSON is rejected with an explicit error.
  • Security and privacy: the doc-coverage recipe no longer interpolates the
    configurable toolchain/threshold into shell quotes; both values are exported
    and read from the environment at shell runtime, closing a command-injection
    surface.
  • Developer documentation: new internal #[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:

  • Add a workspace-wide Rustdoc coverage measurement and enforcement command for library and binary targets.

Bug Fixes:

  • Correct glob diagnostics to report the outermost unmatched opening brace.

Enhancements:

  • Raise Rustdoc documentation coverage across the Rust workspace and document the required coverage policy and exemptions.
  • Split oversized implementation modules into private support modules while preserving existing behavior and module boundaries.
  • Improve documentation for Rust APIs, internal data structures, and helper functions across the workspace.
  • Harden coverage measurement error handling and configurable Makefile execution.

Build:

  • Add Makefile targets for coverage measurement and its Python test suite, including configurable threshold and toolchain settings.
  • Enable the private-item missing-docs lint.

CI:

  • Gate CI on the workspace Rustdoc coverage threshold.

Documentation:

  • Document the coverage policy, developer workflow, and private support-module conventions.

Tests:

  • Add isolated pytest coverage for target discovery, aggregation, threshold handling, malformed output, subprocess failures, and CLI options.
  • Add regression coverage for unmatched glob-brace diagnostics.

Chores:

  • Update spelling configuration and Markdown tooling exclusions for the expanded documentation sweep.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Summary

  • Raise workspace-wide Rustdoc coverage from approximately 65% to over 98%.
  • Enforce an 80% minimum through CI and make doc-coverage.
  • Add configurable target discovery, threshold checks, toolchain selection, error handling, and isolated tests for scripts/doc-coverage.py.
  • Document Rust functions, methods, fields, support modules, and documented-item exemptions across the workspace.
  • Split large internal modules into private #[path] support modules without changing runtime behaviour.
  • Add the glob-brace diagnostic regression test and improve Makefile safety.
  • Update contributor guidance and quality gates to support issue #369.

Validation

  • Run Rustdoc without warnings.
  • Run documentation coverage, module-cap, and test checks.
  • Run the coverage-tool pytest suite.

Walkthrough

The 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.

Changes

Documentation coverage rollout

Layer / File(s) Summary
Coverage gate
.github/workflows/ci.yml, Makefile, scripts/doc-coverage.py, scripts/tests/test_doc_coverage.py, AGENTS.md, docs/developers-guide.md, Cargo.toml
Add Rustdoc coverage measurement, configurable thresholds and toolchains, an 80% CI gate, contributor guidance, and comprehensive tests.
Internal module extraction
src/diagnostic_json*.rs, src/ir/cycle*.rs, src/ir/sort_utils.rs, src/status*.rs, src/stdlib/command/error*.rs, src/stdlib/time*.rs, src/stdlib/which/env*.rs
Move diagnostic, cycle-detection, sorting, status, command-error, time-formatting, and PATH-support implementations into dedicated modules.
Rust documentation updates
src/**, build_l10n_audit/**, test_support/**
Add documentation for private and public structures, fields, functions, error paths, parsing, rendering, configuration, and test-support helpers.
Validation and maintenance
src/manifest/glob/validate.rs, src/manifest/glob/tests/pattern.rs, tests/makefile_test_target/rustflags.rs, typos*.toml, .markdownlint-cli2.jsonc
Track nested unmatched braces with a stack, add a regression test, update Makefile contract coverage, and adjust spelling and lint exclusions.

Poem

Rustdoc lights the workspace bright,
Eighty percent guards the night.
Private helpers now speak clear,
CI checks what maintainers hear.
Modules march in tidy rows,
And tested coverage grows.


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error, 7 warnings)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error The PR changes init_metrics to retry global recorder installation after failure, but repository searches show no test calls init_metrics or exercises that retry path. Add a focused test seam for recorder registration. Force the first registration to fail, call init_metrics again, and assert that the second call installs the recorder and enables snapshot output.
Out of Scope Changes check ⚠️ Warning The PR includes an unrelated runtime behaviour change in observability.rs that is not required for issue #369. Remove the unrelated observability behaviour change or link an issue that requires retryable recorder installation.
Developer Documentation ⚠️ Warning Makefile makes doc-coverage depend on doc-coverage-test, which requires uv and Python 3.13; the developer's guide documents only the metric, threshold, and toolchain. Document the doc-coverage-test prerequisite in docs/developers-guide.md, including uv, Python 3.13, and the pinned pytest dependencies.
Testing (Unit And Behavioural) ⚠️ Warning The new CLI/Make gate is tested only by importing doc-coverage.py and mocking subprocess.run; no end-to-end CLI/Make/CI test exists, and init_metrics retry has no test. Add subprocess-level coverage using a fake Cargo executable, assert CLI output and exit codes, cover Make/CI wiring, and test the new recorder failure/retry path.
Testing (Property / Proof) ⚠️ Warning The PR adds a stack-based nested-brace invariant in validate.rs and only a fixed {{} regression; its testing claims do not recommend property testing for generated brace sequences. Add substantive proptest cases for generated nested, escaped, and character-class braces. Prove stack depth, LIFO matching, and outermost-error reporting.
Testing (Compile-Time / Ui) ⚠️ Warning Add a compile-fail or equivalent test: the PR introduces Clippy's missing_docs_in_private_items = "deny", but adds no test for this compile-time rejection; coverage tests are Python-only. Add a focused fixture and Clippy-driven assertion for an undocumented private item. Add stable snapshots for doc-coverage success and failure summaries, with variable fields redacted.
Observability ⚠️ Warning init_metrics now supports recorder-installation retries, but the set_global_recorder failure is discarded with no warning, metric, or trace for this degraded state. Emit a WARN event at the failed set_global_recorder boundary with a stable operation and error category, without recorder details or unbounded values.
Concurrency And State ⚠️ Warning The PR replaces Once with a process-global AtomicBool retry gate, but no test covers concurrent init_metrics() interleavings; only local-recorder increments are tested, and the guide still cl... Add a deterministic barrier-based test for concurrent installation and failure/retry semantics, then update the metrics lifecycle documentation to match the atomic protocol.
✅ Passed checks (12 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the coverage change and references the linked issue number.
Description check ✅ Passed The description explains the coverage implementation, CI enforcement, documentation work, tests, and related changes.
Linked Issues check ✅ Passed The PR addresses issue #369 by adding workspace coverage measurement, documentation, warning-free Rustdoc checks, and an 80% CI threshold.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 118 functions across 25 files. (3 skipped: 3 unsupported.)
User-Facing Documentation ✅ Passed The diff changes unmatched-brace diagnostic positioning, and docs/users-guide.md documents rejection plus outermost-opening and closing-brace positions; other changes are internal or developer tool...
Module-Level Documentation ✅ Passed Static audit found //! headers in all 506 Rust source files, including every changed and new module; split modules state purpose, ownership and callers, and both new Python modules have docstrings.
Unit Architecture ✅ Passed Accept the change: coverage subprocesses are explicit in measure/load_metadata with RuntimeError handling and faked seams; Rust refactors isolate reporting, parsing, and traversal without new hidde...
Domain Architecture ✅ Passed Keep the change: the diff moves existing helpers into private sibling modules; IR stays on IR graph/path types, while JSON, UI, command, time, and PATH concerns remain in adapter modules.
Security And Privacy ✅ Passed Pass: new Cargo calls use argv lists without shell execution; Make quotes coverage inputs; the diff adds no secrets, permissions, or externally reachable sensitive data.
Performance And Resource Use ✅ Passed Accept: the diff adds no hot-path quadratic work or unbounded accumulation; coverage state is processed per target, while runtime refactors preserve existing algorithms.
Architectural Complexity And Maintainability ✅ Passed PASS — Accept the architecture: the coverage tool has a direct CI purpose and isolated tests, while private support modules preserve ownership and the documented 400-line boundary; no new dependenc...
Rust Compiler Lint Integrity ✅ Passed The PR diff adds no broad unused-code suppressions or lint-appeasement anchors; split-module items have real callers, and the two clone lines are pre-existing code moved into support modules.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-369-raise-doc-comment-coverage-to-80-threshold

Warning

Your free Security trial is over. An organization admin can activate billing to continue.


Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds 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 gate

flowchart 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
Loading

File-Level Changes

Change Details Files
Introduce a doc-comment coverage measurement script and CI/Makfile gate at 80% coverage, updating contributor documentation and RUSTFLAGS/RUSTDOCFLAGS policies.
  • Add scripts/doc-coverage.py to run cargo rustdoc --show-coverage over all workspace lib/bin targets and aggregate documented vs total items, including private items.
  • Add make doc-coverage target in Makefile with overridable DOC_COVERAGE_THRESHOLD, DOC_COVERAGE_TOOLCHAIN, and PYTHON, wiring RUSTFLAGS/RUSTDOCFLAGS for polonius and docsrs/deny-warnings.
  • Add doc-coverage step to GitHub Actions CI workflow, running after make lint to reuse the doc database.
  • Extend AGENTS.md with the doc-comment coverage policy (80% threshold), style rules for function/method docs, and rustdoc metric exemptions, and include make doc-coverage alongside existing pre-commit gates.
  • Extend tests/makefile_test_target/rustflags.rs RUSTFLAGS_CASES to cover the doc-coverage recipe and update the description comment.
scripts/doc-coverage.py
Makefile
.github/workflows/ci.yml
AGENTS.md
tests/makefile_test_target/rustflags.rs
Refactor IR cycle detection into a sibling module and tighten its docs and test visibility to respect module size limits and harness needs.
  • Extract CycleDetector, VisitState, CycleSearch, and CycleVisitResult from src/ir/cycle.rs into new src/ir/cycle_detector.rs, keeping traversal state, DFS stack, and missing-dependency tracking there.
  • Adjust src/ir/cycle.rs to describe the sibling cycle_detector module, drop direct camino::Utf8Path import, and re-export CycleDetector, VisitState, CycleSearch, CycleVisitResult privately for test/kani children via super::.
  • Tighten support imports so canonicalize_cycle is only pulled into test/kani builds, and reorganize path_eq/path_cmp usage via support helpers.
  • Update Kani/test harness access patterns to reach detector types through cfg(test)/cfg(kani) re-exports.
src/ir/cycle.rs
src/ir/cycle_detector.rs
src/ir/cycle_support.rs
Split large modules at the 400-line cap by extracting helper siblings for status reporting, diagnostic JSON, time formatting, IR sorting utilities, command errors, discovery env handling, and various other concerns, and document the new structures.
  • Move the indicatif-backed status reporter and helper formatting functions out of src/status.rs into src/status_indicatif.rs, re-exporting IndicatifReporter and selected helpers while limiting test-only imports of STAGE6_INDEX and PIPELINE_STAGE_TOTAL.
  • Move diagnostic JSON support types and helpers (DiagnosticSource/DiagnosticSpan, severity/cause collection, span extraction, fallback payload, etc.) into src/diagnostic_json_support.rs, leaving src/diagnostic_json.rs focused on the document and entry types.
  • Split src/stdlib/time/mod.rs by moving ISO-8601 formatting and Object implementations (TimestampValue, TimeDeltaValue) into src/stdlib/time/format.rs, with mod.rs now focused on query helpers and timedelta resolution.
  • Introduce src/ir/sort_utils.rs for insertion-sort-by, sort_strings, sort_paths, and has_seen_output, and update src/ir/from_manifest_support.rs to use it instead of inline sort/path equality helpers.
  • Move command error support structs and message suffix appenders into src/stdlib/command/error_support.rs, trimming src/stdlib/command/error.rs to high-level error construction with doc-comments.
  • Move discovery env keys and EnvProvider/StdEnvProvider into src/cli/discovery_env.rs, updating src/cli/discovery.rs to import CONFIG_ENV_VAR, DISCOVERY_ENV_KEYS, EnvProvider, StdEnvProvider from the sibling module.
  • Extract numerous other helper modules (status_timing, diagnostic_json_support tests, graph_view HTML layout/escape/style/svg/noscript pieces, help JSON document rendering, manifest glob support/normalize/validate/errors/walk, jinja macro invocation/telemetry, command-list scanner and evaluator, pipeline timing, etc.) and add targeted doc-comments to new structs, enums, and functions.
src/status.rs
src/status_indicatif.rs
src/diagnostic_json.rs
src/diagnostic_json_support.rs
src/stdlib/time/mod.rs
src/stdlib/time/format.rs
src/ir/from_manifest_support.rs
src/ir/sort_utils.rs
src/stdlib/command/error.rs
src/stdlib/command/error_support.rs
src/cli/discovery.rs
src/cli/discovery_env.rs
src/status_timing.rs
src/graph_view/render_html/*.rs
src/graph_view/render_dot.rs
src/runner/help.rs
src/manifest/glob/*.rs
src/manifest/jinja_macros/*.rs
src/ninja_gen_command_list*.rs
src/runner/process/*.rs
test_support/*
Sweep doc-comments across runner, CLI, manifest, stdlib, test_support, and build_l10n_audit modules, clarifying semantics and error behaviour while keeping tests and trait impls exempt.
  • Add module-level //! docs where missing and enrich existing ones per AGENTS.md policy (purpose, utility, rationale) across CLI merge/parsing/config/discovery, runner dispatch/process/graph/help, stdlib helpers (path, command, collections, which, network, config), manifest AST/expand/render/diagnostics/glob, graph_view, localization, locale resolution, ninja_gen, startup_tracing, observability, etc.
  • Add /// doc-comments to public and private functions and methods including error-return semantics (# Errors sections) in modules like runner/process/file_io.rs, process/mod.rs, stdlib/command/.rs, stdlib/which/.rs, stdlib/path/*.rs, manifest rendering/expansion, help_query, cli parsing/localization/diag/environment, config_resolution, host_pattern, build_l10n_audit scanner/keys/compare, test_support utilities (manifest fs helpers, HTTP server, dev_fast sandbox/release, command helper, ninja probe, env_lock, localizer stubs, etc.).
  • Avoid documenting trait-impl methods and test functions, aligning comments and exemptions with rustdoc’s counting behaviour described in AGENTS.md.
  • Document internal structs/enums (many previously undocumented) with concise field-level comments to reduce rustdoc coverage gaps.
src/runner/process/file_io.rs
src/runner/process/mod.rs
src/runner/dispatch.rs
src/runner/graph.rs
src/runner/help_query.rs
src/runner/help.rs
src/runner/process/*.rs
src/cli/*.rs
src/manifest/*.rs
src/stdlib/**/*.rs
src/graph_view/**/*.rs
src/localization/*.rs
src/locale_resolution.rs
src/ninja_gen/*.rs
src/observability*.rs
build_l10n_audit/*.rs
test_support/**/*.rs
Adjust specific behaviours and small logic details uncovered during the documentation and refactor sweep (e.g., rustflags tests, dyndep retention, which environment capture, HTTP fixture config).
  • Update tests/makefile_test_target/rustflags.rs RUSTFLAGS_CASES to inline literal cases instead of helper constructors, add the doc-coverage case, and document the rationale in a comment tied to the 400-line cap.
  • Clarify rustdoc metric description in AGENTS.md (trait impls excluded, inherent methods and modules counted, test code exemptions) and align script docstring and Makefile RUSTDOCFLAGS with those rules.
  • Refine dyn-dep retention and telemetry structs (RetentionPolicy, RetentionPass, RetentionSelection, RetentionSummary) with field docs but preserve retention semantics and limits.
  • Tighten env capture logic in stdlib/which/env.rs and cli/environment.rs with clearer doc-comments and minor internal refactors that do not change the external behaviour.
  • Keep make, HTTP server, and Kani verification helpers functionally unchanged while adding exhaustive comments and a few small validation guards (e.g., poll interval clamping, cache-relative path validation, path canonicalisation error reporting).
tests/makefile_test_target/rustflags.rs
AGENTS.md
Makefile
src/runner/process/dyndep_retention.rs
src/runner/process/dyndep_telemetry.rs
src/stdlib/which/env.rs
src/cli/environment.rs
test_support/src/http/mod.rs
src/stdlib/network/cache.rs
src/cli/config.rs
src/ir/from_manifest_verification.rs

Assessment against linked issues

Issue Objective Addressed Explanation
#369 Ensure all public and private functions and methods carry Rustdoc /// comments explaining their purpose and error semantics where applicable.
#369 Make cargo doc --no-deps run warning-free across the workspace.
#369 Raise and enforce aggregate doc-comment coverage to meet or exceed the 80% threshold.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@pandalump

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Head commit changed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@buzzybee-df12

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]

This comment was marked as resolved.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos

leynos commented Aug 19, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Pre-merge check reconciliation for commit 4978deb:

  • Unit Architecture (Error): resolved in 4978deb — pinned_toolchain() and load_metadata() now wrap file/TOML/JSON failures in RuntimeError, and main() catches that type around toolchain resolution and all measurements, so no fallible path escapes to a traceback.
  • Developer Documentation (Warning): resolved in 4978deb — docs/developers-guide.md's Quality gates section now lists make doc-coverage and explains the threshold/toolchain overrides, the Rustdoc counting rules, and the exemptions.
  • Testing (Overall / Unit and Behavioural / Property and Proof): validated — the coverage tool is exercised under CI (the new Doc coverage step ran make doc-coverage in build-test), the Makefile contract test models the recipe (RustflagsCase::doc_coverage), the script's parsing/validation paths were verified manually (threshold rejection, aggregate math), and the glob brace fix ships with a new regression test (validate_brace_matching_reports_the_outermost_unclosed_brace). Please re-run the checks against the current HEAD.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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.rs

Length 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 || true

Length 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)' || true

Length of output: 31052


@leynos

Keep the Unit Architecture check unresolved.

Wrap subprocess.run() in both measure() and load_metadata() at Lines 147 and 217. subprocess.run() can raise OSError before it returns a result. For example, this occurs when cargo is unavailable or cannot execute. main() only catches RuntimeError, so these failures still escape as a traceback.

The other static checks pass:

  • Confirm the RuntimeError handling for toolchain-file, TOML, metadata-exit, and JSON failures.
  • Confirm the CI Doc coverage step runs make doc-coverage.
  • Confirm docs/developers-guide.md documents the quality gate, threshold, toolchain override, counting rules, and exemptions.
  • Confirm RustflagsCase includes the doc-coverage recipe.
  • Confirm validate_brace_matching_reports_the_outermost_unclosed_brace exists in src/manifest/glob/tests/pattern.rs.

Do not treat the stated manual validation and CI execution as independently verified. This sandbox cannot execute repository code or test commands.


🧠 Learnings used
Learnt from: CR
Repo: leynos/episodic PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-16T19:17:48.287Z
Learning: - Only changes that meet all quality gates should be committed.

You are interacting with an AI system.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@pandalump

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]

This comment was marked as resolved.

@leynos

leynos commented Aug 21, 2026

Copy link
Copy Markdown
Owner Author

@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"

@coderabbitai

This comment was marked as resolved.

@leynos
leynos marked this pull request as ready for review August 21, 2026 16:47
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Warning

Your free Security trial is over. An organization admin can activate billing to continue.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @leynos, your pull request is larger than the review limit of 150000 diff characters

@leynos

leynos commented Aug 21, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai please create a new GitHub issue against leynos/agent-helper-scripts requiring the addition of these to the global typos config:

"otherwize" = "otherwise"
"exercize" = "exercise"
"exercized" = "exercised"
"exercizes" = "exercises"
"exercizing" = "exercising"
"raize" = "raise"
"raized" = "raised"
"raizes" = "raises"
"raizing" = "raising"

leynos added 5 commits August 25, 2026 00:52
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.
@leynos

leynos commented Aug 24, 2026

Copy link
Copy Markdown
Owner Author

@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:

          FAIL [   0.103s] (1807/2371) netsuke-build::makefile_test_target rustflags::behavioural_rustflags_recipes_are_well_formed_without_inherited_flags
    stdout ───
  
      running 1 test
      test rustflags::behavioural_rustflags_recipes_are_well_formed_without_inherited_flags ... FAILED
  
      failures:
  
      failures:
          rustflags::behavioural_rustflags_recipes_are_well_formed_without_inherited_flags
  
      test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 21 filtered out; finished in 0.09s
      
    stderr ───
      Error: doc-coverage should enable Polonius with -Zpolonius=next, expanded to ""
  
      Stack backtrace:
         0: <anyhow::Error>::msg::<alloc::string::String>
         1: __covrec_6133ADC562DAD9A9
         2: makefile_test_target::rustflags::behavioural_rustflags_recipes_are_well_formed_without_inherited_flags::{closure#0}
         3: <makefile_test_target::rustflags::behavioural_rustflags_recipes_are_well_formed_without_inherited_flags::{closure#0} as core::ops::function::FnOnce<()>>::call_once
         4: test::__rust_begin_short_backtrace::<core::result::Result<(), alloc::string::String>, fn() -> core::result::Result<(), alloc::string::String>>
         5: __covrec_C99A239903F758B8
         6: __covrec_687E1DBD267BAAAD
         7: __covrec_4F1C6746856067A4
         8: <std::sys::thread::unix::Thread>::new::thread_start
         9: <unknown>
        10: <unknown>
  
    Cancelling due to test failure: 3 tests still running
          PASS [  38.473s] (1808/2371) netsuke-build::locale_stub_ui_tests stub_env_builders_compile_under_the_same_harness
          PASS [  38.473s] (1809/2371) netsuke-build::locale_stub_ui_tests stub_env_default_does_not_compile
          SLOW [> 60.000s] (─────────) netsuke-build::locale_stub_ui_tests harness_compiles_under_a_split_build_dir
          PASS [  79.964s] (1810/2371) netsuke-build::locale_stub_ui_tests harness_compiles_under_a_split_build_dir
  ────────────
       Summary [ 216.114s] 1810/2371 tests run: 1809 passed (5 slow), 1 failed, 3 skipped
          FAIL [   0.103s] (1807/2371) netsuke-build::makefile_test_target rustflags::behavioural_rustflags_recipes_are_well_formed_without_inherited_flags
  warning: 561/2371 tests were not run due to test failure (run with --no-fail-fast to run all tests, or run with --max-fail)
  error: test run failed

@leynos
leynos force-pushed the issue-369-raise-doc-comment-coverage-to-80-threshold branch from 36624b3 to e6c9c70 Compare August 24, 2026 22:55
codescene-access[bot]

This comment was marked as outdated.

@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

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_target

Length 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' \
  Makefile

Length of output: 96959


@leynos
File: tests/makefile_test_target/rustflags.rs
Symbols: shell_expression, expand, behavioural_rustflags_recipes_preserve_inherited_flags, behavioural_rustflags_recipes_are_well_formed_without_inherited_flags

The failure is valid in the test harness. It is not a doc-coverage recipe defect.

Makefile exports POLONIUS_FLAGS at Line 87. The doc-coverage recipe expands it at shell runtime:

RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }$${POLONIUS_FLAGS}"

This form avoids interpolating a configurable value into the shell command line. In an actual make doc-coverage run, Make exports POLONIUS_FLAGS=-Zpolonius=next, so the shell expands the value correctly.

The contract test does not model that exported variable. expand() removes RUSTFLAGS but does not set POLONIUS_FLAGS. Therefore, the isolated shell expands ${POLONIUS_FLAGS} to "". Do not change the Makefile recipe to $(POLONIUS_FLAGS). That would remove the secure runtime expansion used by doc-coverage.

  • Update the Rustflags contract harness to model Make’s exported POLONIUS_FLAGS.
  • Preserve the existing isolated shell evaluation and the empty-inherited-RUSTFLAGS check.
  • Validate the focused Rust test and the standard formatting and lint targets.

  • 📌 Create a pull request with these changes

You are interacting with an AI system.

Satisfy the private-item documentation policy for the Windows workspace
resolver without changing its lookup behavior.
codescene-access[bot]

This comment was marked as outdated.

Supply the Make-exported runtime variable to isolated shell evaluation so
the secure doc-coverage recipe is tested faithfully.
codescene-access[bot]

This comment was marked as outdated.

@leynos

leynos commented Aug 25, 2026

Copy link
Copy Markdown
Owner Author

@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 Method

scripts/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 examples

To 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);
+}
+

@coderabbitai

This comment was marked as resolved.

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

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
fi

Length of output: 60077


@leynos, all four listed findings are resolved at current head 525cd749533715c049300a8d2739b35a52bbe0fa.

Check Status Evidence
Testing (Overall) Resolved scripts/tests/test_doc_coverage.py has isolated tests for target discovery, coverage aggregation, zero-target coverage, threshold exit codes, invalid metadata, Cargo failures, Rustdoc failures, malformed coverage JSON, invalid coverage payloads, toolchain selection, pinned-toolchain loading, threshold validation, and complete Cargo argument construction. FakeCargo isolates Cargo behaviour.
Unit Architecture Resolved scripts/doc-coverage.py::measure() catches OSError and raises RuntimeError with the Rustdoc measurement diagnostic. load_metadata() does the same for Cargo metadata. main() catches RuntimeError, prints the controlled error message, and returns exit code 2. Direct tests cover both load_metadata() and measure() OSError paths.
Security And Privacy Resolved Makefile exports DOC_COVERAGE_THRESHOLD and DOC_COVERAGE_TOOLCHAIN. The doc-coverage recipe reads them with $$DOC_COVERAGE_TOOLCHAIN and $$DOC_COVERAGE_THRESHOLD at shell runtime. Make does not interpolate either configurable value into the shell command line.
Developer Documentation Resolved docs/developers-guide.md contains the Internal support module boundaries section. It states the 400-line module cap and documents ownership and permitted callers for src/ir/sort_utils.rs, src/diagnostic_json_support.rs, src/stdlib/command/error_support.rs, and src/stdlib/time/format.rs.

No GitHub issue is required. No remaining-work prompt is required.

This verification is static. Run make doc-coverage-test, make check-fmt, make lint, and cargo doc --no-deps in an executable development environment before closing the checks.

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.
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

Move payload aggregation and count conversion into focused helpers.
Preserve all target-qualified measurement-error diagnostics.
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No quality gates enabled for this code.

@leynos

leynos commented Aug 25, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Have the following now been resolved?

scripts/doc-coverage.py (1)

221-226: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Reject invalid coverage counts before aggregation.
Validate finite, non-negative integer counts and require with_docs <= total. Otherwise int() raises uncaught OverflowError for values such as 1e309, while negative or inconsistent values produce invalid aggregate metrics. Add focused tests and return the controlled exit code 2.

🤖 Detailed instructions

Use 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 @scripts/doc-coverage.py around lines 221 - 226, Update the coverage
aggregation around Coverage and per_file to validate total and with_docs as
finite, non-negative integers with with_docs less than or equal to total before
constructing Coverage values; catch invalid inputs, including OverflowError, and
return the controlled exit code 2. Add focused tests covering non-finite,
negative, non-integer, and inconsistent counts.

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

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 || true

Length of output: 35019


@leynos, this finding is resolved at current head 83103b6f39badaa68cc78096eee56ebef9cccfb0.

  • scripts/doc-coverage.py::coverage_count accepts only non-negative int values. It rejects bool, floats, non-finite JSON numbers such as 1e999, negative values, and other non-integer values.
  • scripts/doc-coverage.py::coverage_from_entry rejects invalid relationships where with_docs > total.
  • scripts/doc-coverage.py::parse_coverage_output catches KeyError, TypeError, ValueError, and OverflowError. It converts them to the existing controlled RuntimeError diagnostic.
  • scripts/doc-coverage.py::main catches that RuntimeError and returns exit code 2.

scripts/tests/test_doc_coverage.py::test_main_rejects_invalid_coverage_counts covers these cases:

  • non-finite: {"total": 1e999, "with_docs": 0}
  • negative: {"total": -1, "with_docs": 0}
  • non-integer: {"total": 1.5, "with_docs": 0}
  • inconsistent: {"total": 1, "with_docs": 2}

The test verifies the controlled RuntimeError from run_measurements() and verifies exit code 2 from main().

This verification is static. Run make doc-coverage-test and the configured Ruff checks before closing the finding.

You are interacting with an AI system.

@leynos

leynos commented Aug 25, 2026

Copy link
Copy Markdown
Owner Author

@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).

Finding Status Evidence
src/stdlib/which/lookup/workspace/posix.rs and related Result contracts Partially resolved search_workspace, collect_matching_executables, and process_workspace_entry have # Errors sections. uniq_filter, collect_flattened_values, resolve_group_key, and WhichOptions::from_kwargs also have the required sections. current_dir_utf8 moved to src/stdlib/which/env_path_support.rs and has the required section.
src/stdlib/which/env.rs capture contracts Unresolved EnvSnapshot::capture, EnvSnapshot::capture_with_env, and both platform variants of EnvSnapshot::capture_with_pathext return Result but have no # Errors sections. Both capture_impl variants and capture_common use inline # Errors: prose. Replace this with the standard # Errors heading, a blank Rustdoc line, and the error contract.

@leynos

leynos commented Aug 25, 2026

Copy link
Copy Markdown
Owner Author

@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)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error The PR changes init_metrics to retry global recorder installation after failure, but repository searches show no test calls init_metrics or exercises that retry path. Add a focused test seam for recorder registration. Force the first registration to fail, call init_metrics again, and assert that the second call installs the recorder and enables snapshot output.
Developer Documentation ⚠️ Warning Makefile makes doc-coverage depend on doc-coverage-test, which requires uv and Python 3.13; the developer's guide documents only the metric, threshold, and toolchain. Document the doc-coverage-test prerequisite in docs/developers-guide.md, including uv, Python 3.13, and the pinned pytest dependencies.
Testing (Unit And Behavioural) ⚠️ Warning The new CLI/Make gate is tested only by importing doc-coverage.py and mocking subprocess.run; no end-to-end CLI/Make/CI test exists, and init_metrics retry has no test. Add subprocess-level coverage using a fake Cargo executable, assert CLI output and exit codes, cover Make/CI wiring, and test the new recorder failure/retry path.
Testing (Property / Proof) ⚠️ Warning The PR adds a stack-based nested-brace invariant in validate.rs and only a fixed {{} regression; its testing claims do not recommend property testing for generated brace sequences. Add substantive proptest cases for generated nested, escaped, and character-class braces. Prove stack depth, LIFO matching, and outermost-error reporting.
Testing (Compile-Time / Ui) ⚠️ Warning Add a compile-fail or equivalent test: the PR introduces Clippy's missing_docs_in_private_items = "deny", but adds no test for this compile-time rejection; coverage tests are Python-only. Add a focused fixture and Clippy-driven assertion for an undocumented private item. Add stable snapshots for doc-coverage success and failure summaries, with variable fields redacted.
Observability ⚠️ Warning init_metrics now supports recorder-installation retries, but the set_global_recorder failure is discarded with no warning, metric, or trace for this degraded state. Emit a WARN event at the failed set_global_recorder boundary with a stable operation and error category, without recorder details or unbounded values.
Concurrency And State ⚠️ Warning The PR replaces Once with a process-global AtomicBool retry gate, but no test covers concurrent init_metrics() interleavings; only local-recorder increments are tested, and the guide still cl... Add a deterministic barrier-based test for concurrent installation and failure/retry semantics, then update the metrics lifecycle documentation to match the atomic protocol.

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

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 || true

Length 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 0

Length of output: 41154


@leynos

The checks are partially resolved at current head 83103b6f39badaa68cc78096eee56ebef9cccfb0.

Finding Status Evidence
src/stdlib/which/lookup/workspace/posix.rs and related Result contracts Resolved search_workspace, collect_matching_executables, and process_workspace_entry have standard # Errors sections. uniq_filter, collect_flattened_values, resolve_group_key, WhichOptions::from_kwargs, and current_dir_utf8 also have standard # Errors sections.
src/stdlib/which/env.rs capture contracts Unresolved EnvSnapshot::capture, EnvSnapshot::capture_with_env, and both platform variants of EnvSnapshot::capture_with_pathext return Result<Self, ResolveError> without # Errors sections. Both capture_impl variants and capture_common use the non-standard single-line # Errors: form.

Keep this finding open. This work is in scope for the PR. Do not create a follow-up issue.

Apply this documentation-only change:

Update the remaining environment-capture Rustdoc contracts.

File:
- `src/stdlib/which/env.rs`

Symbols:
- `EnvSnapshot::capture`
- `EnvSnapshot::capture_with_env`
- `EnvSnapshot::capture_with_pathext` for `#[cfg(windows)]`
- `EnvSnapshot::capture_with_pathext` for `#[cfg(not(windows))]`
- `EnvSnapshot::capture_impl` for `#[cfg(not(windows))]`
- `EnvSnapshot::capture_impl` for `#[cfg(windows)]`
- `capture_common`

Work from the current codegraph at:

    83103b6f39badaa68cc78096eee56ebef9cccfb0

Problem:
- These functions return `Result<_, ResolveError>`.
- `capture`, `capture_with_env`, and both platform variants of
  `capture_with_pathext` have no `# Errors` section.
- Both `capture_impl` variants and `capture_common` use the non-standard form:

      /// # Errors: ...

Required changes:
1. Add a standard Rustdoc error section to every listed function:

      /// # Errors
      ///
      /// Returns a [`ResolveError`] when ...

2. Use the error contract that matches the delegated implementation.

   For `EnvSnapshot::capture`, `EnvSnapshot::capture_with_env`, and the
   non-Windows `EnvSnapshot::capture_with_pathext`, state that capture can fail
   when:
   - the working directory cannot be read or converted to UTF-8;
   - `PATH` cannot be read or parsed; or
   - a `PATH` entry cannot be processed because it is not valid UTF-8.

   For the Windows `EnvSnapshot::capture_with_pathext`, also state that it can
   fail under the same working-directory and `PATH` conditions. Do not claim
   that `PATHEXT` parsing fails. `parse_pathext` returns `Vec<String>` and does
   not return `Result`.

3. Replace each inline `# Errors:` line on both `capture_impl` variants and
   `capture_common` with the standard heading, a blank Rustdoc line, and an
   accurate error description.

4. Keep the implementation unchanged.
   Do not change function signatures, `cfg` attributes, error types, tests, or
   unrelated documentation.

5. Keep terminology accurate:
   - `current_dir_utf8` can fail while reading the working directory or when
     converting it to UTF-8.
   - `parse_path_entries` can fail when a `PATH` component is not valid UTF-8.
   - `PATHEXT` parsing does not produce `ResolveError`.

Validate in an executable development environment:

    make check-fmt
    make lint
    cargo doc --no-deps

Report:
1. The changed file.
2. The functions that received or had standardised `# Errors` sections.
3. The error conditions documented for Windows and non-Windows capture paths.
4. The result of each validation command.
5. Any command that did not run, with the reason.

The inspection was static. Run the listed validation commands before closing the finding.

You are interacting with an AI system.

@leynos
leynos merged commit 7517885 into main Aug 25, 2026
18 checks passed
@leynos
leynos deleted the issue-369-raise-doc-comment-coverage-to-80-threshold branch August 25, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Raise doc-comment coverage to 80 % threshold

4 participants