Exempt reexported external modules from the API docstring check - #45
Merged
ChrisRackauckas merged 2 commits intoAug 8, 2026
Merged
Conversation
`run_api_docs`'s rendered half already skips names owned outside the package, on the grounds that a reexported module inherits its defining package's module documentation. The docstrings half had no such exemption, so every facade that does `using Dep` was required to make `Dep` itself carry a module docstring — Julia exports a module's own name, so `:Dep` lands in the facade's public API. Restricted to `Module` values: reexported functions, types and constants still owe a docstring, which the binding lookup resolves at the defining package. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F75XsVeZ94QH3PmCuq6QUF
The 1.10 `@doc` fallback renders an undocumented module as "No docstring or readme file found", which `_has_docstring`'s substring check never matched, so no module is reported as undocumented on the LTS at all. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F75XsVeZ94QH3PmCuq6QUF
This was referenced Aug 8, 2026
ChrisRackauckas
marked this pull request as ready for review
August 8, 2026 18:51
This was referenced Aug 9, 2026
Merged
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.
What changed and why
run_api_docshas two halves, and they disagreed about reexports. The rendered half skips names owned outside the package (_requires_local_rendering, added in #30) on the stated grounds that "a re-exported module inherits the defining package's rendered module documentation". The docstrings half had no such exemption. Julia exports a module's own name, so a facade doingusing Depputs:Depinto its public API — and the docstrings half then demanded thatDepitself carry a module docstring, which is the defining package's obligation, not the reexporter's.This adds
_is_external_module_reexportand applies it to the docstrings filter. It is deliberately narrower than_requires_local_rendering: it exempts onlyModulevalues owned outsidepkg. A reexported function, type or constant still owes a docstring — that one genuinely resolves through the binding to the defining package's docs, so exempting it would hide real gaps.Why this surfaced now
Before #41,
_has_docstringwas!occursin("No documentation found", ...)overBase.Docs.doc. Julia >= 1.11 prints"No docstring found for public module X"for an undocumented module, which that substring never matched — so undocumented module reexports silently passed. #41 switched toBase.Docs.hasdoc, which reports them correctly, and 11 QA lanes across the OrdinaryDiffEq.jl monorepo went red on module names like:SciMLBase,:SciMLLogging,:DiffEqBaseand:StochasticDiffEqCore(https://github.com/SciML/OrdinaryDiffEq.jl/actions/runs/31198062683).Confirmed by pinning rather than by reading: OrdinaryDiffEq.jl master
c11ff74bbGROUP=QApasses on SciMLTesting=2.6.1(88/88) and fails on 2.6.2 withisempty([:SciMLBase, :SciMLLogging]).#41 was also surfacing genuine problems, and this PR preserves those. See the verification below.
Verification
This package's suite discriminates. With the
src/change stashed and only the new tests applied:With the fix applied (
julia +1.12 --project -e 'using Pkg; Pkg.test()'):The OrdinaryDiffEq lane goes green. OrdinaryDiffEq.jl master
c11ff74bb, unmodified except for a[sources]entry pointing SciMLTesting at this branch — note it still resolves the registry SciMLBase v3.43.0 and SciMLLogging v2.0.4, neither of which has a module docstring:(the same tree on registry SciMLTesting 2.6.2 fails 87 passed / 1 failed)
Genuine undocumented names still fail. Both of these lanes are red on master for real reasons, and both are still red with this branch:
lib/OrdinaryDiffEqLowOrderRK,GROUP=QA—:SciMLBasedrops out,:AutoDP5does not:lib/StochasticDiffEqLowOrder,GROUP=QA— 11 names become 8, and every solver name survives:Runic (
Runic.main(["--check","--diff", ...])oversrc/SciMLTesting.jlandtest/runtests.jl) exits 0;typosover the changed files exits 0.Julia 1.10 (LTS)
The first push was red on all three LTS lanes and on Downgrade (which runs on the LTS), on my own new assertion that a package's own undocumented submodule is still reported. Reproduced locally on 1.10 and fixed by guarding that one assertion to 1.11+: the 1.10
@docfallback renders an undocumented module as"No docstring or readme file found", which_has_docstring's substring never matched, so no module is reported as undocumented on the LTS at all. That is exactly why this only broke on 1.11+ — nothing in this PR changes the 1.10 path.Local runs on the final tree:
Not verified
Only the root OrdinaryDiffEq QA lane plus
OrdinaryDiffEqLowOrderRKandStochasticDiffEqLowOrderwere re-run against this branch; the other red sublibrary lanes were not. TheStochasticDiffEqLowOrderlane also has an unrelatedall_qualified_accesses_are_publicUnexpected Pass, untouched here.For a reviewer to push back on
The exemption is module-only by choice.
_requires_local_renderingis broader — it also exempts reexported functions and types from the rendered check — and one could argue the two halves should now be literally the same predicate. I did not do that: the docstrings half resolves through the binding, so a reexported function that is documented at its source already passes, and exempting undocumented ones would have re-hidden:EM,:RKMiland friends.Please ignore until reviewed by @ChrisRackauckas.
🤖 Generated with Claude Code
https://claude.ai/code/session_01F75XsVeZ94QH3PmCuq6QUF