diff --git a/.github/workflows/shared.yml b/.github/workflows/shared.yml index c859bbab1..b113d87c3 100644 --- a/.github/workflows/shared.yml +++ b/.github/workflows/shared.yml @@ -143,8 +143,8 @@ jobs: # nav entries without a page and pages without a nav entry, `zensical build # --strict` fails on broken .md links, `pymdownx.snippets: check_paths: # true` fails on a deleted `docs_src/` include, and the post-build steps - # fail on unresolved cross-references, inventory download failures, and - # broken non-markdown link targets. + # fail on order-dependent API rendering, unresolved cross-references, + # inventory download failures, and broken non-markdown link targets. # Until this job existed the docs were only ever built post-merge by # `deploy-docs.yml`, so those failures went green on the PR and broke the next # deploy of main. This is the check path; `deploy-docs.yml` stays the deploy diff --git a/mkdocs.yml b/mkdocs.yml index 5b3f77799..4c0cd06ad 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -188,6 +188,13 @@ plugins: handlers: python: paths: [src, src/mcp-types] + # Zensical renders pages in undefined (filesystem-dependent) order + # against one shared griffe collection, so a cross-package re-export + # (`mcp` -> `mcp_types`) resolves only if its target package happens + # to have been collected first. Chasing exported aliases into their + # packages at load time makes resolution order-independent; + # scripts/docs/check_render_order.py enforces that property. + load_external_modules: true options: relative_crossrefs: true members_order: source diff --git a/scripts/docs/build.sh b/scripts/docs/build.sh index 8dce3afd4..8f545761b 100755 --- a/scripts/docs/build.sh +++ b/scripts/docs/build.sh @@ -4,8 +4,9 @@ # # Zensical runs no MkDocs plugins or hooks, so the build is three steps: # materialise the API reference pages and the concrete config, build the -# site strictly, then generate llms.txt and the per-page markdown -# renditions. This script is the single owner of that recipe, dependency +# site strictly (plus the order-independence and cross-reference checks +# Zensical doesn't do itself), then generate llms.txt and the per-page +# markdown renditions. This script is the single owner of that recipe, dependency # sync included — CI (shared.yml, docs-preview.yml) and scripts/build-docs.sh # all call it. The toolchain detection in docs-preview.yml and build-docs.sh # keys on this file's path and expects the site under site/. @@ -31,6 +32,11 @@ rm -rf .cache site uv run --frozen --no-sync python scripts/docs/build_config.py uv run --frozen --no-sync zensical build -f mkdocs.gen.yml --strict +# The build above renders pages in one arbitrary (filesystem-dependent) +# order; prove the API reference renders in hostile orders too — see the +# check's docstring for the failure mode this guards. +uv run --frozen --no-sync python scripts/docs/check_render_order.py + # Zensical stays green even under --strict when a cross-reference fails to # resolve (rendered as literal bracket text) or an objects.inv inventory # fails to download (every link through it silently degrades to plain text); diff --git a/scripts/docs/check_render_order.py b/scripts/docs/check_render_order.py new file mode 100644 index 000000000..a0f09b2cd --- /dev/null +++ b/scripts/docs/check_render_order.py @@ -0,0 +1,110 @@ +"""Fail the docs build when API rendering depends on page processing order. + +Zensical discovers pages with an unsorted directory walk and renders them +all through one shared mkdocstrings handler, so the griffe collection that +resolves `::: module` blocks accumulates in filesystem-dependent order, and +nothing in the toolchain checks that the order is safe: a cross-package +re-export that only resolved when its target package had been collected +earlier built fine on one machine and died with `AliasResolutionError` on +another (a GitHub runner-image update reshuffled readdir order and broke +every CI docs build this way). `load_external_modules: true` in `mkdocs.yml` +makes resolution order-independent; this check enforces that property, +because a regular build only ever exercises one arbitrary order. + +mkdocstrings applies module loading and per-page options only on the first +collect of a package (later pages find the package already collected), so +each package under `docs/api/` gets the two hostile sides of that +asymmetry, each from a fresh handler with an empty collection: + +- its subpages first and its package index last, so pages rendering + cross-package re-exports (the index above all) come after a plain + subpage has already collected the package and nothing they declare + themselves can still affect collection; +- its package index alone, so the page with the most re-exports is itself + the first collect over an empty collection. + +Only the package's own pages are rendered: resolving `mcp` re-exports +without ever rendering an `mcp_types` page is exactly the property under +test. + +Usage: + python scripts/docs/check_render_order.py [--config mkdocs.gen.yml] + +Run after `build_config.py` has produced the config and the `docs/api/` +tree. +""" + +from __future__ import annotations + +import argparse +import traceback +from pathlib import Path + +# Sibling modules, same direct-invocation pattern as build_config.py: +# gen_ref_pages owns the docs/api layout, llms_txt owns the page-URL mapping. +import gen_ref_pages +from llms_txt import page_url +from zensical.compat import mkdocstrings as zensical_mkdocstrings +from zensical.config import parse_config +from zensical.markdown.render import render + +API_DIR = gen_ref_pages.API_DIR +DOCS_DIR = API_DIR.parent + + +def _passes(package: str, pages: list[Path]) -> list[tuple[str, list[Path]]]: + """The labeled render orders exercising both sides of the first-collect asymmetry.""" + index = API_DIR / package / "index.md" + if index not in pages: + return [(f"'{package}'", pages)] + subpages = [page for page in pages if page != index] + if not subpages: + return [(f"'{package}' index-alone", [index])] + return [(f"'{package}' index-last", [*subpages, index]), (f"'{package}' index-alone", [index])] + + +def _render(page: Path) -> None: + """Render one page the way Zensical's Rust core drives the Python side.""" + rel = page.relative_to(DOCS_DIR).as_posix() + render(page.read_text(encoding="utf-8"), rel, page_url(rel)) + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--config", default=str(gen_ref_pages.ROOT / "mkdocs.gen.yml"), help="Built config to render with" + ) + args = parser.parse_args() + parse_config(args.config) + + packages: dict[str, list[Path]] = {} + for page in sorted(API_DIR.rglob("*.md")): + packages.setdefault(page.relative_to(API_DIR).parts[0], []).append(page) + if not packages: + raise SystemExit(f"check_render_order: no pages under {API_DIR} (run build_config.py first)") + + for package in sorted(packages): + for label, order in _passes(package, packages[package]): + # Fresh Handlers -> empty griffe collection. Autorefs anchors + # accumulate across passes, but they play no part in collection + # or alias resolution (check_crossrefs owns link health). + zensical_mkdocstrings.reset() + for position, page in enumerate(order): + try: + _render(page) + # Top-level handler: any exception from any page fails the + # check; the traceback identifies whether the order was at + # fault or something else broke (network, missing file). + except Exception: + traceback.print_exc() + rel = page.relative_to(DOCS_DIR).as_posix() + raise SystemExit( + f"check_render_order: {rel} failed at position {position + 1}/{len(order)} of the" + f" {label} order (traceback above; an AliasResolutionError means API rendering" + " depends on page order — see `load_external_modules` in mkdocs.yml)" + ) from None + print(f"check_render_order: {label} order OK ({len(order)} pages)") + + +if __name__ == "__main__": + main() diff --git a/scripts/docs/gen_ref_pages.py b/scripts/docs/gen_ref_pages.py index 1a09d30de..2340e1846 100644 --- a/scripts/docs/gen_ref_pages.py +++ b/scripts/docs/gen_ref_pages.py @@ -64,22 +64,21 @@ def _compact_index(module: griffe.Module, documented: set[str]) -> str | None: """Build a compact page body for a module that re-exports from outside its own subtree. mkdocstrings renders a re-export whose canonical documentation lives on - another page as a full duplicate of it: deterministically for aliases - within one top-level package (`mcp.client.auth` re-exporting from - `mcp.shared.auth`), and order-dependently across top-level packages - (`from mcp_types import y` + `__all__` renders the duplicate only when - the other package happens to be loaded already, and silently omits the - member when it isn't). Modules whose exports all live in their own - subtree (`mcp_types` re-exporting its private `._types` module, or a - module whose `__all__` lists only its own definitions) are unaffected - and keep the plain `::: module` stub (return `None`): their page is - itself the canonical rendering. - - For an affected module, pin the semantics instead of inheriting the - accident: every export whose canonical page exists elsewhere under the API - reference becomes a link to it, and only exports documented nowhere else - (re-exports from private modules) keep their full body here, via an - explicit `members:` list. + another page as a full duplicate of it, whether the alias stays within + one top-level package (`mcp.client.auth` re-exporting from + `mcp.shared.auth`) or crosses packages (`from mcp_types import y` + + `__all__` — `load_external_modules` in mkdocs.yml has the collector chase + exported cross-package aliases when their package is first collected, so + the target package is loaded regardless of page order). Modules whose + exports all live in their own subtree (`mcp_types` re-exporting its + private `._types` module, or a module whose `__all__` lists only its own + definitions) are unaffected and keep the plain `::: module` stub (return + `None`): their page is itself the canonical rendering. + + For an affected module, replace the duplicates: every export whose + canonical page exists elsewhere under the API reference becomes a link to + it, and only exports documented nowhere else (re-exports from private + modules) keep their full body here, via an explicit `members:` list. """ prefix = f"{module.path}." exports: dict[str, griffe.Object | griffe.Alias] = {} @@ -138,18 +137,7 @@ def _compact_index(module: griffe.Module, documented: set[str]) -> str | None: entry += f" — {summary}" sections.setdefault(_KIND_SECTIONS[target.kind], []).append(entry) - # Rendering the stub resolves the cross-package aliases again, in - # mkdocstrings' own collection. On a warm incremental rebuild the target - # package's pages can all be cache hits, so nothing else loads it and the - # resolution crashes (AliasResolutionError); preloading pins it. The - # module's own root package needs no pin: rendering the stub loads it. - preload = sorted( - {member.target_path.split(".")[0] for member in exports.values() if member.is_alias} - - {module.path.split(".")[0]} - ) body = [f"::: {module.path}", " options:"] - if preload: - body += [" preload_modules:", *(f" - {pkg}" for pkg in preload)] if inline: body += [" members:", *(f" - {name}" for name in inline)] else: diff --git a/scripts/docs/llms_txt.py b/scripts/docs/llms_txt.py index 1c549b15c..614690f3e 100644 --- a/scripts/docs/llms_txt.py +++ b/scripts/docs/llms_txt.py @@ -93,7 +93,7 @@ def _dest_md_uri(src_uri: str) -> str: return "index.md" if directory == PurePosixPath(".") else f"{directory}/index.md" -def _page_url(src_uri: str) -> str: +def page_url(src_uri: str) -> str: """The directory URL of a page relative to the site root (`servers/tools/`, `""` for the home page).""" return _dest_md_uri(src_uri).removesuffix("index.md") @@ -260,7 +260,7 @@ def rewrite(match: re.Match[str]) -> str: raise _BuildError(f"llms_txt: cannot resolve link target {target!r} in {src_uri}") if linked.endswith(".md"): # Pages without a markdown rendition (the api/ stubs) link to their HTML instead. - url = _dest_md_uri(linked) if linked in prose else _page_url(linked) + url = _dest_md_uri(linked) if linked in prose else page_url(linked) else: url = linked # assets are published at their docs-relative path return f"{opening}{site_url}{url}{anchor or ''}{title or ''}{closing}" @@ -316,7 +316,7 @@ def generate(site_dir: Path) -> None: # same one `_title` falls back to). h1 = _prose_h1(markdown) body = markdown if h1 is None else markdown[: h1.start()] + markdown[h1.end() :] - full += [f"# {title}", "", f"Source: {site_url}{_page_url(src_uri)}", "", body.strip(), ""] + full += [f"# {title}", "", f"Source: {site_url}{page_url(src_uri)}", "", body.strip(), ""] index.append("") index += ["## Optional", ""] @@ -332,7 +332,7 @@ def generate(site_dir: Path) -> None: f" missing {sorted(generated - listed)}, stale {sorted(listed - generated)}" ) for src_uri, title, description in _OPTIONAL_PAGES: - index.append(f"- [{title}]({site_url}{_page_url(src_uri)}): {description}") + index.append(f"- [{title}]({site_url}{page_url(src_uri)}): {description}") index.append("") (site_dir / "llms.txt").write_text("\n".join(index), encoding="utf-8")