docs: fix the griffe docstring warnings and guard the API reference - #42
Merged
Conversation
griffe validates a Google-style `Args:` section in a class docstring against `Class.parameters`, which is literally `all_members["__init__"].parameters`. Classes whose `__init__` is generated at runtime have no static `__init__`, so that list is empty and every documented name trips both the "no type or annotation" and "does not appear in the function signature" checks -- 86 warnings across models.py, config.py and groups.py. The damage was not just log noise: with `Class.parameters` empty the rendered reference table had no type column and marked every pydantic field `required`, including ones that default to `None` or `[]`. That was live on the published site. Switch the affected classes to `Attributes:`, which griffe resolves from the real class-body annotation. The @DataClass docstrings stay on `Args:` -- griffe's built-in dataclasses extension synthesises an `__init__` there, so they keep both correct types and a defaults column. The resulting mix is deliberate. `ToolCall.name`/`args` move onto `__new__`, where they are genuinely constructor parameters with real annotations. Two `Annotated` fields get an explicit `name (type):` override so the table shows `list[Evaluator]` and `str | Path | None` instead of the raw annotation blob, which also leaked the private `_PathLike` alias. This went unnoticed locally because a warm ./.cache/ replays the cached api-reference render, so the handler never re-collects and the warnings stop printing entirely.
The obvious guard does not work. `zensical build --strict` prints every griffe warning and then reports "No issues found" and exits 0 -- verified against zensical 0.0.51, where the flag is forwarded to the Rust runtime whose issue collector never sees warnings routed through Python's logging. `zensical serve --strict` is blunter still: it prints "Strict mode is currently unsupported." A local build is no guard either, because a warm ./.cache/ replays the cached api-reference render without re-collecting, so the warnings stop printing entirely -- which is how 86 of them survived long enough to reach the published site. So add scripts/check_docstrings.py: load the package with griffe, force every docstring to parse, and exit non-zero on any warning. Verified both ways -- exits 0 on the current tree, and exits 1 listing all 10 warnings when a single class is reverted to `Args:`. Keep --strict for what it does catch (zensical's own broken links and missing anchors), with a comment so nobody mistakes it for a docstring guard.
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.
The bug
The docs build emitted 86 griffe warnings (43 fields × 2 checks) across
models.py,config.pyandgroups.py— and that was not just log noise. It was a live defect on the published site:api-reference/models/renderedtool_calls_includewith an empty type cell and marked it required, when it defaults to[]. Every pydantic field on the published reference was untyped and falsely required.Root cause
griffe is a static analyser. A Google-style
Args:section in a class docstring is validated againstClass.parameters, which is literallyall_members["__init__"].parameters. Classes whose__init__is generated at runtime have no static__init__, so that list is empty — every documented name trips both the "no type or annotation" and "does not appear in the function signature" checks, and the rendered table loses its type column.__init__?@dataclassMessage,TurnContext,TranscriptResult, evaluatorsdataclassesextension synthesises oneBaseModelJudgeConfig,ToolCallArgsConfig,Expect,Turn,Transcript,AgentEvalConfig,GroupConfigNamedTupleAgentReplystrsubclass with only__new__ToolCallIt went unnoticed locally because a warm
./.cache/replays the cachedapi-reference/*render, so the handler never re-collects and the warnings stop printing entirely.The fix
Args:documents a static signature;Attributes:documents fields whose constructor is generated at runtime. griffe's attributes parser resolves each type from the real class-body annotation.Attributes:.@dataclassdocstrings stay onArgs:— griffe proves a real signature there, so they keep correct types and a defaults column. The resulting mix inmodels.pyis deliberate.ToolCall.name/argsmove onto__new__, where they are genuinely constructor parameters with real annotations.Annotatedfields get an explicitname (type):override, so the table showslist[Evaluator]andstr | Path | Nonerather than the raw annotation blob — which also leaked the private_PathLikealias.Rejected: silencing via
warn_unknown_params = false(leaves the wrongrequired/missing types published, and hides genuine typos), andgriffe-pydantic(only labels fields and swaps a template — it does not synthesise__init__, so it would not fix this on its own).The CI guard is not
--strictThe obvious guard does not work.
zensical build --strictprints every griffe warning and then reportsNo issues foundand exits 0 — verified by injecting a regression. At the source (zensical 0.0.51) the flag is forwarded to the Rust runtime, whose issue collector never sees warnings routed through Python'slogging;zensical serve --strictis blunter still and prints "Strict mode is currently unsupported."So this adds
scripts/check_docstrings.py: load the package with griffe, force every docstring to parse, exit non-zero on any warning.--strictis kept for what it does catch (broken links, missing anchors), with a comment so nobody mistakes it for a docstring guard.Known gap, documented in the script: the
Attributes:parser never validates names against the class body, so a typo'd attribute name renders a phantom row silently rather than warning.Verification
Args:.rm -rf .cache sitefirst —--cleanalone does not clear.cache): exit 0, nogriffe:lines.tool_calls_include→list[str]and no longerrequired;__new__→name: str/required,args: ToolArgs | None/None. The remainingrequiredmarkers are all genuinely required.uv run pytest: 457 passed, 7 skipped.ruff/ruff formatclean.docs/llms-full.txtneeds no regeneration —_SKIP_PREFIXESalready excludesapi-reference/.No behaviour change: the source diff is docstring text only.