Skip to content

ci: gate docstring quality and coverage in CI (#616)#689

Open
planetf1 wants to merge 8 commits intogenerative-computing:mainfrom
planetf1:ci/doc-quality-gate-616
Open

ci: gate docstring quality and coverage in CI (#616)#689
planetf1 wants to merge 8 commits intogenerative-computing:mainfrom
planetf1:ci/doc-quality-gate-616

Conversation

@planetf1
Copy link
Contributor

@planetf1 planetf1 commented Mar 18, 2026

Misc PR

Type of PR

  • Bug Fix
  • New Feature
  • Documentation
  • Other

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_mismatch scanner to audit_coverage.py — flags Attributes: sections on TypedDict classes that document phantom fields or omit declared ones (mirrors the existing param_mismatch logic for functions).

Pre-commit hook updated to use --fail-on-quality; stays stages: [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

  • Tests added to the respective file if code was changed
  • New code has 100% coverage if code as added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

@github-actions
Copy link
Contributor

The PR description has been updated. Please fill out the template for your PR to be reviewed.

@planetf1
Copy link
Contributor Author

@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

@planetf1 planetf1 marked this pull request as ready for review March 18, 2026 14:33
@planetf1 planetf1 requested a review from a team as a code owner March 18, 2026 14:33
Copy link
Member

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

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)

@mergify
Copy link

mergify bot commented Mar 18, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert|release)(?:\(.+\))?:

@planetf1
Copy link
Contributor Author

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

@planetf1 planetf1 force-pushed the ci/doc-quality-gate-616 branch from 364fb81 to 71b4bde Compare March 18, 2026 15:38
Copy link
Member

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

I am in favor, but would be good for @jakelorocco to weigh in too

@planetf1
Copy link
Contributor Author

A good time to consider forcing

Screenshot 2026-03-18 at 16 25 06

@planetf1 planetf1 force-pushed the ci/doc-quality-gate-616 branch from 19261e3 to 58a7f44 Compare March 18, 2026 16:26
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.
@planetf1 planetf1 force-pushed the ci/doc-quality-gate-616 branch from 58a7f44 to dfb95c4 Compare March 18, 2026 22:37
@planetf1
Copy link
Contributor Author

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?

@planetf1
Copy link
Contributor Author

planetf1 commented Mar 18, 2026

vv This was the regression

Screenshot 2026-03-18 at 22 43 25 Screenshot 2026-03-18 at 22 43 35

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

ci: gate doc quality in CI and pre-commit to prevent future degradation

2 participants