ci: gate docstring quality and coverage in CI (#616)#689
ci: gate docstring quality and coverage in CI (#616)#689planetf1 wants to merge 8 commits intogenerative-computing:mainfrom
Conversation
|
The PR description has been updated. Please fill out the template for your PR to be reviewed. |
|
@psschwei @HendrikStrobelt @jakelorocco We are finally at a state where the docs build is clean, and reporting no issues. I recommend we now make this a hard-fail. We're had at least one PR merged which regressed some docstrings (albeit minor), and another pr in the queue with quite a few changes needed. By ensuring we validate as part of CI we'll maintain the docs in a clean state Note this does also add a slightly looser pre-commit. That is optional if we think too annoying |
psschwei
left a comment
There was a problem hiding this comment.
I'm in favor of adding, but one question: is the output of our docstring checker the kind of thing that could be easily fed into an agent to produce the appropriate docstrings / make the check pass? (thinking of (a) keeping contribution process easy for new folks, and (b) how we can use agents at some point to do this for us)
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
|
Yes, much of the docs have been written and restructured by AI - long term the ability to generate too is on the cards. I think there will be some incremental improvements to make scan results easier... |
364fb81 to
71b4bde
Compare
psschwei
left a comment
There was a problem hiding this comment.
I am in favor, but would be good for @jakelorocco to weigh in too
19261e3 to
58a7f44
Compare
Add a hard-fail docstring quality gate to the docs-publish workflow: - New 'Docstring quality gate' step runs --quality --fail-on-quality --threshold 100; fails if any quality issue is found or coverage drops below 100% (both currently pass in CI) - Existing audit_coverage step (soft-fail, threshold 80) retained for the summary coverage metric Add typeddict_mismatch checks to audit_coverage.py: - typeddict_phantom: Attributes: documents a field not declared in the TypedDict - typeddict_undocumented: declared field absent from Attributes: section - Mirrors the existing param_mismatch logic for functions Pre-commit: enable --fail-on-quality on the manual-stage hook (CI is the hard gate; hook remains stages: [manual] as docs must be pre-built). Update CONTRIBUTING.md and docs/docs/guide/CONTRIBUTING.md with TypedDict docstring requirements and the two new audit check kinds.
58a7f44 to
dfb95c4
Compare
|
An intrinsics updated has introduced some documentation errors - not fatal: missing args, and missing returns (10 omissions) -- but having seen 2 other PRs potentially doing this too, this is why we MUST start enforcement, not just the checks, which were the facilitator of improving the docs. @jakelorocco @HendrikStrobelt What do you think? Are you ok with this in principle? If so I will add the fix-ups into this PR tomorrow and get it approved/merged ASAP. Any concerns? |
… and fix hints audit_coverage.py: - Add file/line fields to every issue dict (repo-relative path + def line) - _print_quality_report now shows [file:line] per issue, per-kind Fix:/Ref: hints linking to CONTRIBUTING.md anchors, and emits ::error file=...,line=... GHA annotations so issues appear inline in PR diffs - Cap GHA annotations at 10 per check kind with "N more in job log" notice - Add _KIND_FIX_HINTS and _gha_file_annotation helpers; _CONTRIB_DOCS_URL constant validate.py: - Convert all check functions from list[str] to list[dict] errors (file/line/message) - Add line-number tracking to validate_source_links, validate_internal_links, validate_anchor_collisions, and validate_doc_imports - Emit per-error GHA annotations with file/line; shared 20-annotation budget across all checks so every category gets representation in PR diff - Fix icon bug: summary rows now use correct pass/fail icon - Group detailed errors by check type with section headers docs-publish.yml: - Add --orphans and --output /tmp/quality_report.json to quality gate step - Upload quality_report.json as docstring-quality-report artifact (30-day retention) pyproject.toml: - cli/**/*.py: suppress only D2/D3/D4xx style rules; enable D1xx (missing docstrings) as a ruff-level complement to the audit_coverage quality gate docs/docs/guide/CONTRIBUTING.md: - Add CI docstring checks reference section with per-kind tables (fix instructions + anchors) for all 11 check kinds across 4 categories - Add callout explaining GHA annotation cap (10 per kind) and where to find the full list (job log + JSON artifact)
…y links
audit_coverage.py (audit_nav_orphans):
- Probe docs/docs/docs.json before docs/mint.json so both Mintlify v1
and v2 nav configs are supported
- Extend _extract to handle plain string page entries used by docs.json
(v2 uses "pages": ["api/..."] strings; v1 used {"page": "api/..."} dicts)
- Previously mint.json was never found, nav_refs stayed empty, and every
MDX file was reported as an orphan
docs-publish.yml (Write job summary):
- When the quality gate fails, render a prominent markdown callout with a
direct link to the CI docstring checks reference section in CONTRIBUTING.md
- Add a per-kind fix reference table with clickable anchor links to each
category section (missing/short, args/returns, class Option C, TypedDict)
- Per-kind Ref: URLs in the raw log are inside a text block and do
not render as links in the step summary; this table surfaces them rendered
…ped notice docs-publish.yml: - Parse per-kind counts from _print_quality_report section headers in the quality gate log (e.g. "Missing docstrings (12)") and show them as a comma-separated breakdown in the Docstring Quality table cell instead of just the total — gives developers an immediate view of which categories are failing without expanding the log audit_coverage.py: - Remove the "skipped (pass --quality to enable)" GHA notice emitted by the coverage-only step; there is always a dedicated quality gate step immediately after so the notice was misleading and redundant
audit_coverage.py: - Coverage miss section now shows a structured Fix:/Ref: block with the exact generate-ast.py command and a link to CONTRIBUTING.md#validating-docstrings - Missing symbols listed one per line (symbol indented under module) for scannability instead of comma-joined on one long line - Emit a ::error or ::warning GHA annotation with symbol/module counts when coverage symbols are undocumented validate.py: - Add _CHECK_FIX_HINTS dict mapping each check label to a (fix text, ref URL) pair, covering all 8 check types with specific fix instructions and links into CONTRIBUTING.md (root or guide as appropriate) - _print_check_errors now prints Fix:/Ref: under each section header, matching the pattern established by _print_quality_report
audit_coverage.py: - Add missing_param_type check: fires when Args: section exists but one or more concrete params lack Python type annotations; naturally non-overlapping with no_args (which fires when section is absent) - Add missing_return_type check: fires when Returns: section is documented but the function has no return annotation; naturally non-overlapping with no_returns (annotation exists but section absent) - Add fix hints and CONTRIBUTING.md anchors for both new check kinds - Update kind_labels and iteration order in _print_quality_report generate-ast.py: - Add remove_internal_modules() post-generation filter step - Uses AST-based import analysis: a submodule is internal when the parent __init__.py imports from at least one sibling submodule but not from this one (import-based visibility, not __all__ name-matching) - Conservative: keeps module when parent imports nothing (indeterminate) or __init__.py cannot be parsed - _CONFIRMED_INTERNAL_MODULES hardcoded set for known internals where parent imports nothing (json_util, backend_instrumentation); these should eventually be renamed with _ prefix per Python convention - Package index files (stem == parent dir) are never filtered docs.json: nav regenerated by build; internal modules removed from nav CONTRIBUTING.md: add missing_param_type / missing_return_type to CI docstring checks reference table docs-publish.yml: add both new kinds to summary kind_short and kind_anchors fix-reference table



Misc PR
Type of PR
Description
Adds a hard-fail docstring quality gate to the docs-publish workflow (
--quality --fail-on-quality --threshold 100). Both checks currently pass in CI (100% coverage, 0 quality issues).Also adds a
typeddict_mismatchscanner toaudit_coverage.py— flagsAttributes:sections onTypedDictclasses that document phantom fields or omit declared ones (mirrors the existingparam_mismatchlogic for functions).Pre-commit hook updated to use
--fail-on-quality; staysstages: [manual]since it requires pre-built docs. CI is the hard gate.Contribution docs updated with TypedDict docstring requirements and the two new check kinds.
Testing