Skip to content

Exempt reexported external modules from the API docstring check - #45

Merged
ChrisRackauckas merged 2 commits into
mainfrom
exempt-reexported-modules-from-docstring-check
Aug 8, 2026
Merged

Exempt reexported external modules from the API docstring check#45
ChrisRackauckas merged 2 commits into
mainfrom
exempt-reexported-modules-from-docstring-check

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Aug 8, 2026

Copy link
Copy Markdown
Member

What changed and why

run_api_docs has 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 doing using Dep puts :Dep into its public API — and the docstrings half then demanded that Dep itself carry a module docstring, which is the defining package's obligation, not the reexporter's.

This adds _is_external_module_reexport and applies it to the docstrings filter. It is deliberately narrower than _requires_local_rendering: it exempts only Module values owned outside pkg. 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_docstring was !occursin("No documentation found", ...) over Base.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 to Base.Docs.hasdoc, which reports them correctly, and 11 QA lanes across the OrdinaryDiffEq.jl monorepo went red on module names like :SciMLBase, :SciMLLogging, :DiffEqBase and :StochasticDiffEqCore (https://github.com/SciML/OrdinaryDiffEq.jl/actions/runs/31198062683).

Confirmed by pinning rather than by reading: OrdinaryDiffEq.jl master c11ff74bb GROUP=QA passes on SciMLTesting =2.6.1 (88/88) and fails on 2.6.2 with isempty([: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:

ERROR: LoadError: Some tests did not pass: 357 passed, 1 failed, 3 errored, 0 broken.
run_api_docs docstrings check: Test Failed at test/runtests.jl:1166
  Expression: c[:fail] == 0 && c[:error] == 0
run_api_docs docstrings check: Error During Test at test/runtests.jl:1178
  Expression: SciMLTesting._is_external_module_reexport(UndocumentedModuleReexportFixture, :OwnedModule)
  UndefVarError: `_is_external_module_reexport` not defined in `SciMLTesting`

With the fix applied (julia +1.12 --project -e 'using Pkg; Pkg.test()'):

Test Summary: | Pass  Total   Time
SciMLTesting  |  361    361  39.2s
     Testing SciMLTesting tests passed

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:

  [0bca4576] SciMLBase v3.43.0
  [a6db7da4] SciMLLogging v2.0.4
  [09d9d899] SciMLTesting v2.6.3 `.../wt-scimltesting`
...
Test Summary:           | Pass  Total   Time
Quality Assurance Tests |   88     88  11.7s
     Testing OrdinaryDiffEq tests passed

(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:SciMLBase drops out, :AutoDP5 does not:

# SciMLTesting 2.6.2
│   undocumented = 2-element Vector{Symbol}:
│     :AutoDP5
└     :SciMLBase
ERROR: LoadError: Some tests did not pass: 19 passed, 1 failed, 0 errored, 0 broken.

# SciMLTesting 2.6.3 (this branch)
│   undocumented = 1-element Vector{Symbol}:
└     :AutoDP5
ERROR: LoadError: Some tests did not pass: 19 passed, 1 failed, 0 errored, 0 broken.

lib/StochasticDiffEqLowOrder, GROUP=QA — 11 names become 8, and every solver name survives:

old (2.6.2 behaviour): [:DiffEqBase, :EM, :LambaEM, :LambaEulerHeun, :PCEuler, :RKMil, :RKMilCommute, :SciMLBase, :SimplifiedEM, :SplitEM, :StochasticDiffEqCore]
new (patched):         [:EM, :LambaEM, :LambaEulerHeun, :PCEuler, :RKMil, :RKMilCommute, :SimplifiedEM, :SplitEM]
exempted:              [:DiffEqBase, :SciMLBase, :StochasticDiffEqCore]

ERROR: LoadError: Some tests did not pass: 15 passed, 1 failed, 1 errored, 3 broken.

Runic (Runic.main(["--check","--diff", ...]) over src/SciMLTesting.jl and test/runtests.jl) exits 0; typos over 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 @doc fallback 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:

julia +1.12 --project -e 'using Pkg; Pkg.test()'   ->  SciMLTesting | 361 361  40.0s   tests passed
julia +1.10 --project -e 'using Pkg; Pkg.test()'   ->  SciMLTesting | 345 345  36.2s   tests passed

Not verified

Only the root OrdinaryDiffEq QA lane plus OrdinaryDiffEqLowOrderRK and StochasticDiffEqLowOrder were re-run against this branch; the other red sublibrary lanes were not. The StochasticDiffEqLowOrder lane also has an unrelated all_qualified_accesses_are_public Unexpected Pass, untouched here.

For a reviewer to push back on

The exemption is module-only by choice. _requires_local_rendering is 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, :RKMil and friends.

Please ignore until reviewed by @ChrisRackauckas.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F75XsVeZ94QH3PmCuq6QUF

ChrisRackauckas and others added 2 commits August 8, 2026 06:13
`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
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.

2 participants