[Docs] Check the API pages against the checkout before mkdocs reads it - #46
Merged
Merged
Conversation
The API pages name their ops one by one, and every build checks TileOPs out fresh, so an op that leaves `__all__` upstream aborts the next build with `Could not collect` part-way through the log. scripts/check_api_pages.py reads `_FAMILIES` and each family's `__all__` with ast — importing a family pulls in torch, which the docs environment does not install — and names the op, and the page that names it, before mkdocs starts. An op the pages name and the checkout does not export fails the run. An op exported with no page is printed and does not: TileOPs adds ops on its own schedule, and neither an unrelated pull request here nor the daily refresh of the whole site is the place to stop for one. The check runs in all three workflows that check TileOPs out.
There was a problem hiding this comment.
🟡 Changes recommended
The new checker has a failure-path logic issue (typo-only families) and a parsing correctness gap (non-string ast.Constants accepted), which can produce misleading results or silently accept invalid data.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a pre-mkdocs validation step to ensure docs/api/ mkdocstrings identifiers remain consistent with the exported TileOPs op surface, preventing mid-build Could not collect failures when upstream __all__ changes.
Changes:
- Introduces
scripts/check_api_pages.pyto parse TileOPs_FAMILIES/__all__viaastand compare against::: tileops.<family>.<Op>directives underdocs/api/. - Wires the check into the three GitHub Actions workflows that check out TileOPs (
checks.yml,deploy.yml,render-benchmarks.yml). - Documents the new check and its intent in
CLAUDE.md.
File summaries
| File | Description |
|---|---|
| scripts/check_api_pages.py | New AST-based consistency check between docs ::: tileops.* directives and TileOPs exported ops. |
| CLAUDE.md | Documents the new check and when it runs. |
| .github/workflows/checks.yml | Runs the API pages check after the TileOPs checkout and deps install. |
| .github/workflows/deploy.yml | Runs the API pages check before generating benchmarks and deploying. |
| .github/workflows/render-benchmarks.yml | Runs the API pages check before the daily refresh render/deploy. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+35
to
+47
| for node in ast.parse(path.read_text(encoding="utf-8")).body: | ||
| if not isinstance(node, ast.Assign): | ||
| continue | ||
| if not any(isinstance(t, ast.Name) and t.id == variable for t in node.targets): | ||
| continue | ||
| # Anything but a literal of strings is refused rather than read past: a | ||
| # name silently dropped here is an op this check would stop looking at. | ||
| if not isinstance(node.value, ast.Tuple | ast.List): | ||
| raise SystemExit(f"{path}: {variable} is not a list or tuple literal") | ||
| names = [e.value for e in node.value.elts if isinstance(e, ast.Constant)] | ||
| if len(names) != len(node.value.elts): | ||
| raise SystemExit(f"{path}: {variable} holds something other than plain strings") | ||
| return names |
Comment on lines
+74
to
+75
| if not any(pages.get(family) for family in families): | ||
| raise SystemExit(f"no `::: tileops.<family>.<Op>` identifier under {args.docs}") |
A name in `__all__` that is not a string was read as an op name; it is now refused with the rest of a non-literal list. `ast.parse` gets the filename, so a syntax error in the checkout names the file it is in. Pages holding nothing but a mistyped family raised the generic "no identifier" error, which hid the family the message would have named. That guard is for a regex that stopped matching, so it now asks whether any identifier was found at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
problems
__all__upstream aborts the next build. That is how the daily refresh broke (run 33930934575), and the report was aCould not collectline part-way through a mkdocs log.changes
scripts/check_api_pages.pycompares the::: tileops.<family>.<Op>identifiers underdocs/api/against the op list of a TileOPs checkout: families from_FAMILIES, ops from each family's__all__, read withastbecause importing a family pulls in torch and the docs environment does not install it.checks.yml,deploy.ymlandrender-benchmarks.yml— the three workflows that check TileOPs out — with the same command in each.__all__fails rather than being read past, so no op is silently dropped from the comparison.tileops.trace.api._Trace, belongs to no family and is left alone.checks
tile-ai/TileOPs@main: 183 ops, on the page and exported, exit 0. Fullmkdocs buildwith that checkout: 0 errors, no warning outside griffe.ruff check scripts hooks.py tests,pytest: pass.__all__; a family name typo; a starred__all__; an:::line indented or CRLF-terminated; a docs directory with no identifiers; a missing checkout.