Skip to content

docs: add MkDocs Material documentation site - #110

Merged
pdlourenco merged 4 commits into
mainfrom
claude/plan-sid-docs-website-vFMRn
Jul 30, 2026
Merged

pdlourenco merged 4 commits into
mainfrom
claude/plan-sid-docs-website-vFMRn

Conversation

@pdlourenco

@pdlourenco pdlourenco commented May 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

Adds a unified MkDocs Material documentation site covering Python and MATLAB/Octave, the algorithm specification, and the example notebooks. API reference is generated for both languages — Python via mkdocstrings (NumPy docstrings), MATLAB via a gen-files script that parses each sid*.m H1 header. The 11 example notebooks execute at build time and ship with rendered outputs plus a Binder launch badge.

Originally opened 2026-05-15 and adapted in place (not rebuilt) after review — see the maintainer decision and the adaptation commits 194768f + 70bfc8b. Because almost everything regenerates from source, the PR's content survived ~170 commits of drift on main.

Layout: docsite/ is the site, docs/ stays internal

The site source lives in its own top-level docsite/ directory, not in docs/. docs/ is the home of internal engineering documents (ADRs, analyses, plans, DESIGN.md, REVIEW_CONTEXT.md, the function catalogue) and keeps growing; sharing a directory meant those documents were swept into the public site and their relative links broke the strict build.

With the split, a new internal document can neither leak onto the site nor break its build. Publishing anything from docs/ is a deliberate nav change, never a side effect of the build config — recorded in mkdocs.yml and in a new CONTRIBUTING.md section that also documents the local build command.

What's in this PR

  • mkdocs.yml, requirements-docs.txt
  • docsite/ — landing, getting-started, concepts, Python API stubs, examples, spec includes, about, hooks, stylesheets, MathJax config
  • scripts/build_matlab_api.py — parses every matlab/sid/sid*.m H1 header into a page (standalone + gen-files dual-mode)
  • scripts/build_matlab_examples.py, scripts/link_notebook_examples.py
  • scripts/check_python_api_pages.py — new: fails the build if sid.__all__ and the Python API reference disagree in either direction, or if a stub is missing from the nav or the index tables
  • .github/workflows/docs.yml — strict build on every PR and push; deploy to gh-pages gated on main
  • CONTRIBUTING.md — the docs/ ↔ docsite/ split and how to build locally

Build results

Reproduced locally on a clean tree at head, and green in CI on this PR.

Metric Value
mkdocs build --strict exit 0, zero warnings (was 39 link warnings before the restructure)
Python API pages 19 functions + result types + index
MATLAB API pages 20, generated from H1 headers
Notebook pages 11, executed at build time, Binder badge on each
MATLAB example pages 11 scripts
Spec pages SPEC + EXAMPLES + 4 COSMIC notes (live-included from spec/)
Rendered HTML 89 pages (incl. 404.html)
Internal docs leaked into site/ 0 (verified by scan)

CI behaviour

Build site (strict) runs on every pull request — a PR that breaks the docs now fails before merge instead of after. Deploy to GitHub Pages is a separate job, gated on main and skipped for PRs (visible in this PR's checks), and publishes the exact artefact the build job tested. Permissions are {} at workflow level with contents: read for build and contents: write only for deploy.

Portability to Sphinx

Authored pages stick to plain CommonMark, the MATLAB H1 parser is standalone, notebooks/docstrings are untouched. A future Sphinx migration should be mechanical (config + nav rewrite + admonition/tab syntax pass) rather than a content rewrite.

Test plan

  • CI green on this branch (strict build, deploy correctly skipped)
  • After merge to main, CI deploys to gh-pages
  • One-time: repo Settings → Pages → source gh-pages / root
  • Verify https://pdlourenco.github.io/sid/ against the CI artefact

Notes

  • The build emits a "switch to ProperDocs" banner from mkdocs-gen-files's opportunistic import properdocs.replacement_warning. Suppressed in CI via DISABLE_MKDOCS_2_WARNING=true.
  • mkdocstrings logs an INFO about Black/Ruff for signature formatting — cosmetic; can add ruff to docs deps later.
  • Deliberately out of scope, from the review's follow-up list: adding the docs build to required checks (an ADR-0005 amendment), mike versioning, a scheduled external-link checker, and publishing ADRs under a Development section.

Builds a unified documentation site covering the Python and MATLAB
implementations, the algorithm specification, and the example notebooks.
Auto-generated API reference for both languages: Python via mkdocstrings
(NumPy docstrings), MATLAB via a custom gen-files script that parses
each `sid*.m` H1 header. The 11 example notebooks are executed at build
time and shipped with rendered outputs plus a Binder launch badge.

Site is built and (on `main`) deployed to GitHub Pages by
`.github/workflows/docs.yml`.

Copy link
Copy Markdown
Owner Author

Maintainer decision: adapt this PR — do not close it, do not restructure it from scratch

This PR predates a lot of movement on main (the branch is ~169 commits behind), so before reading the review below, here is the decision on how to proceed, made after a full assessment of the drift. Context you don't have: a test-merge of current main into this branch produces exactly one conflict (.gitignore, trivial union), and a full mkdocs build --strict on the merged tree runs end to end — notebooks execute, both API references generate — failing only on 39 link warnings, all caused by files main has since added under docs/ (docs/decisions/ ADRs, docs/analyses/, docs/plans/, docs/DESIGN.md, docs/REVIEW_CONTEXT.md). Because this PR sets docs_dir: docs, those internal engineering documents get swept into the public site and their relative links to ../CONTRIBUTING.md, ../spec/SPEC.md etc. break the strict build. The PR's own content is essentially intact.

Required restructure: move the site source out of docs/

docs/ on main is now the home of internal engineering docs (ADRs, analyses, plans, review context), and it will keep growing. The site source must live in its own directory so future internal docs can never break or leak into the public site. Concretely:

  1. Merge origin/main into this branch (resolve .gitignore as the union of both sides).
  2. Move all site source added by this PR from docs/ to a new top-level docsite/ (index.md, getting-started/, concepts/, api/, examples/, spec/, about/, hooks/, javascripts/, stylesheets/).
  3. Revert the file moves this PR made: docs/roadmap.md, docs/roadmap_python.md, and docs/TODO.md must stay at their original paths. They are internal dev documents referenced by path from CLAUDE.md, docs/REVIEW_CONTEXT.md, and workflow comments on today's main; moving them breaks those references.
  4. Update every path that encodes the source dir: mkdocs.yml (docs_dir: docsite, edit_uri: edit/main/docsite/, hooks: paths, pymdownx.snippets base_path), the three scripts/*.py gen-files scripts' REPO_ROOT-relative assumptions where applicable, the include-markdown relative paths (../../spec/SPEC.md etc. — recheck each), and .github/workflows/docs.yml.
  5. Do not add docs/** content to the site. If ADRs/analyses should ever be published, that is a separate, deliberate decision — not a side effect of the build config.
  6. Verify with mkdocs build --strict from a clean checkout of the merged branch before pushing.

Review

Per docs/REVIEW_CONTEXT.md (read it on main — it landed after this PR was cut, and it now defines the review discipline here). Mechanical checks ran: full strict build on a test-merge of main into this branch, plus a diff of the documented API surface between the PR base and today's main. Principle numbers below refer to REVIEW_CONTEXT's "Core principles".

Summary

The PR builds a unified MkDocs Material site: auto-generated API reference for both languages (mkdocstrings for Python, a custom H1 parser for MATLAB), spec pages live-included from spec/, and the 11 notebooks executed at build time. The direction is right and the architecture is genuinely good — because almost everything regenerates from source, the site self-heals against the ~3,900 lines of content churn main accumulated since May (the documented file sets — python/sid/, matlab/sid/, notebooks, spec/ — are unchanged, verified). Not mergeable as-is: it needs the restructure above plus the CI and content fixes below.

What works well

  • Generation-over-authoring throughout: live-includes for the spec, gen-files for MATLAB API and examples, executed notebooks. This is the single biggest reason the PR survived two months of drift.
  • The MATLAB H1 parser (scripts/build_matlab_api.py) is standalone-runnable, and the conservative MATH_RE whitelist is the right call versus over-eager math wrapping.
  • mkdocs-jupyter with execute: true, allow_errors: false makes the notebook pages a de-facto smoke test of the Python port.
  • The Binder badge is backed by the real binder/ config on main (postBuild installs ./python[plot]), so it actually works.
  • Verified: the 19 Python stub pages still map 1:1 onto the 19 public functions in python/sid/__init__.py on today's main; result types and SidError are covered by results.md.

Issues to address before merge

  1. Blocker — site source shares docs/ with internal docs. Covered by the restructure above. This is what currently breaks mkdocs build --strict against main.
  2. Blocker — docs/about/todo.md publishes the internal TODO on the public site (principle 9: user-facing docs are state-based and release-relative; REVIEW_CONTEXT red flag "dev-tracking leakage into user docs"). The TODO is a dev-tracking artifact full of struck-through work items. Remove it (and the two roadmaps) from the public nav entirely; they stay in docs/ as internal documents per restructure step 3.
  3. Blocker — CI builds without --strict (.github/workflows/docs.yml:43, run: mkdocs build). The PR description claims a strict zero-warning build, but CI doesn't enforce it, so link rot ships silently. Change to mkdocs build --strict.
  4. Blocker — the workflow never runs on pull requests (docs.yml:3-8: push to main + one hardcoded feature branch only). A PR that breaks the docs build is invisible until after merge. Add a pull_request trigger for the build job (deploy stays gated on github.ref == 'refs/heads/main'), and delete the hardcoded claude/plan-sid-docs-website-vFMRn branch entry — it's a development leftover.
  5. Non-blocker — the Changelog page mirrors only python/RELEASE_NOTES.md (docs/about/changelog.md). Since release 0.2.0, main has a root CHANGELOG.md plus per-language release notes. A page titled "Changelog" should include the root CHANGELOG.md (linking to the per-language notes), otherwise the site under-reports what changed.
  6. Non-blocker — the Python API reference is hand-maintained while the MATLAB one is generated (docs/api/python/*.md stubs + hand-written SUMMARY.md, vs. the MATLAB gen-files script). A new Python function silently misses the site — the same failure mode principle 7 (auto-discovery, no hardcoded manifests) exists to prevent for tests/examples. Acceptable for this PR since the mapping is currently exact (verified), but either generate the stubs from sid.__all__ or add a build-time assertion that every __all__ entry has a page. Same remark, smaller stakes, for the NOTEBOOK_TITLES dict in scripts/link_notebook_examples.py (auto-discovery has a fallback, so new notebooks appear with ugly titles rather than not at all) and the INCLUDE_SOURCES map in docs/hooks/rewrite_external_links.py (a forgotten entry becomes a strict-build failure once Fix DFT source bug and test failures #3 lands, so it fails loud — fine).
  7. Non-blocker — https://github.com/pdlourenco/sid/blob/main is hardcoded in three places (both hooks and build_matlab_examples.py) while mkdocs.yml already declares repo_url. Derive it from config where the hook API allows.
  8. Non-blocker — permissions: contents: write is workflow-wide in docs.yml; scope it to the deploy job.

Follow-up suggestions (not for this PR)

  • Once the docs workflow is strict and PR-triggered, propose adding it to the required checks — that set is governed by ADR-0005, so it goes through an ADR amendment, not a quiet settings change.
  • The concept pages (concepts/*.md) restate spec material in prose — terminology is currently consistent with the REVIEW_CONTEXT glossary (lag window, frozen transfer function, trajectory vs segment), but they are a drift surface with no verifier. Keep them thin and link-heavy; consider a docs note that the spec, not these pages, is normative.
  • Docs versioning (mike) once there's more than one release worth documenting; align with the release workflow.
  • An external-link checker (e.g. lychee) on a schedule, separate from the strict build (which only validates internal links).
  • Publishing ADRs under a "Development" section is worth considering someday — deliberately, with their cross-links fixed.
  • One-time after merge: repo Settings → Pages → source gh-pages, then verify the deployed site.

Verdict

Request changes — adapt in place per the restructure at the top plus items 1–4; items 5–8 at the implementer's discretion in this PR or a follow-up. The core of the PR is sound and worth keeping.


Generated by Claude Code

Adapts the docs-site PR in place per the maintainer decision on #110, rather
than closing or rebuilding it. The PR's content was essentially intact -- almost
everything regenerates from source -- so this is a restructure plus the four
blockers, not a rewrite.

Merge resolution:
- .gitignore: union of both sides.
- docs/TODO.md and docs/roadmap_python.md: the instruction said to keep them at
  their original paths, but #197 (merged ~3h after the review was posted) had
  deleted TODO.md outright and archived roadmap_python.md to
  docs/plans/2026-07-29-python-port-phase-log.md. Accepted main's side for both:
  nothing to restore, nothing to publish. This satisfies the "no dev-tracking on
  the public site" blocker more completely than restoring them would have.
- docs/roadmap.md: restored to its canonical path (the PR had moved it to
  docs/about/roadmap.md), carrying main's rewritten language-neutral catalogue.
  CLAUDE.md, docs/REVIEW_CONTEXT.md and CONTRIBUTING.md reference it by path.

Restructure -- site source out of docs/:
- All site source moved docs/ -> docsite/ (index.md, about/, api/, concepts/,
  examples/, getting-started/, hooks/, javascripts/, spec/, stylesheets/).
  docs/ now holds only internal engineering docs: DESIGN.md, REVIEW_CONTEXT.md,
  roadmap.md, decisions/, analyses/, plans/. Future internal docs can neither
  break the site build nor leak into the site.
- mkdocs.yml: docs_dir, edit_uri, snippets base_path, hooks paths retargeted.
  Include paths needed no change -- docsite/ sits at the same depth docs/ did.
  Generator scripts needed no change -- they use REPO_ROOT plus virtual
  mkdocs_gen_files paths, so they are source-dir agnostic.
- Removed roadmap/python-roadmap/TODO from the public nav, with a comment
  recording that publishing any internal doc is a separate deliberate decision.

Blockers:
- docs.yml now builds with --strict, so link rot fails the build instead of
  shipping silently (this is what the 39 pre-existing link warnings were).
- docs.yml runs on pull_request, so a PR that breaks the docs is caught before
  merge; deploy is split into its own job gated on main and never runs for a PR.
  Deleted the hardcoded claude/plan-sid-docs-website-vFMRn trigger.
- permissions scoped per job (workflow default {}, contents:read to build,
  contents:write only to deploy) instead of workflow-wide contents:write.

Non-blockers taken:
- Changelog page now mirrors the root CHANGELOG.md (added in 0.2.0) and links
  both per-language release notes, instead of only python/RELEASE_NOTES.md. The
  hook's INCLUDE_SOURCES map had to follow, or the included links would break
  the now-strict build.
- New scripts/check_python_api_pages.py runs as a gen-files script and fails the
  build if sid.__all__ and docsite/api/python/ disagree in either direction, or
  if a stub is missing from SUMMARY.md. Closes the hole where a newly exported
  function silently misses the site. Verified it catches both a missing page and
  an orphan page.
- Repo URL derived from mkdocs.yml repo_url in both the link-rewrite hook (via
  the config it already receives) and build_matlab_examples.py (via
  mkdocs_gen_files.config, falling back when standalone), instead of hardcoded
  in three places.
- exclude_docs: hooks/ -- mkdocs was copying the hook .py files AND their
  __pycache__/*.pyc into the published site as static assets.

Verified: mkdocs build --strict exits 0 from a clean tree (was 39 warnings), all
11 notebooks execute, both API references generate, and no internal document
(decisions/analyses/plans/DESIGN/REVIEW_CONTEXT/roadmap/todo) appears anywhere
under site/. ruff clean on the changed scripts and hooks.

Refs #110
… gate

Self-review of the adaptation (fresh-context subagent) found two bugs I had
introduced plus five improvements. All addressed:

Bugs (mine):
- rewrite_external_links.py interpolated a pathlib.Path into the GitHub URL, so
  on Windows it emitted ".../blob/main/docs\DESIGN.md" -- real 404s. Confirmed in
  the built site (docs\decisions\ADR-*, .github\workflows\*, spec\SPEC.md), then
  fixed with as_posix() and confirmed zero backslash URLs remain. CI builds on
  Linux so the deployed site was unaffected; local previews were broken. My
  INCLUDE_SOURCES switch newly surfaced it on the changelog page.
- build_matlab_examples.py: narrowing the config lookup to
  (ImportError, AttributeError) -- which I did to satisfy ruff BLE001 -- broke
  standalone runs from any cwd but the repo root, because
  mkdocs_gen_files.config loads mkdocs.yml and raises ConfigurationError. The
  broad catch is correct here and is now justified in a comment with an explicit
  noqa; the script is meant to run standalone from anywhere.

Improvements:
- check_python_api_pages.py no longer keeps a hand-listed NON_FUNCTION_EXPORTS
  set -- that just relocated the hardcoded manifest the gate exists to remove.
  It now partitions sid.__all__ by inspect.isfunction, so a new result type
  routes to the results.md check instead of demanding a stub (previously it
  would have failed every docs build with the wrong remedy, or been silently
  exempted). Verified both scenarios behave correctly.
- Same gate now also checks the two hand-written index manifests
  (docsite/api/index.md, docsite/api/python/index.md) and results.md coverage --
  a missing table row is invisible to the strict build.
- inject_binder_badge.py derives owner/repo from repo_url; this was the third of
  the three hardcoded-URL sites the review named, so item 7 is now complete
  rather than partly done.
- Dropped the duplicate "# Changelog" H1: the included CHANGELOG.md supplies its
  own, which produced two top-level ToC entries (about/contributing.md already
  avoids this by having no local H1).
- CONTRIBUTING.md documents the docs//docsite/ split, why it exists, that
  publishing an internal doc is a deliberate nav change, and how to build the
  site locally. The restructure's whole point is that a future contributor or
  agent cannot break or leak into the site -- that rule needs to live where they
  actually read it, not only in a mkdocs.yml comment.

Left alone deliberately: two ruff nits (FURB188, SIM114) in
scripts/build_matlab_api.py, which is untouched original PR content, flagged by
rules outside the project's pinned set (E4,E7,E9,F,I) and outside CI's lint
scope. Fixing them would be unrelated churn.

Verified: mkdocs build --strict exits 0; no backslash URLs in the built site;
single H1 on the changelog page; ruff clean under the project's rule set on
every file I touched; 30 relative links resolve.

Refs #110
@pdlourenco

Copy link
Copy Markdown
Owner Author

Adapted in place per the decision — restructure + all four blockers + all four non-blockers, in 194768f and 70bfc8b. mkdocs build --strict now exits 0 (from 39 warnings), and CI proves it: Build site (strict) ✅ with Deploy to GitHub Pages correctly skipped on a PR.

One instruction was stale — please confirm my call

Step 3 said docs/roadmap.md, docs/roadmap_python.md, and docs/TODO.md must stay at their original paths. Your review posted 18:35Z; #197 merged 21:41Z — three hours later — and it deleted docs/TODO.md outright and archived docs/roadmap_python.md to docs/plans/2026-07-29-python-port-phase-log.md.

So the merge surfaced a rename/delete and a rename/rename conflict on exactly those two. I read the instruction's intent as "internal dev docs stay internal, at their canonical paths, off the public site" and applied it to today's main:

  • docs/roadmap.md — restored to its canonical path, carrying main's rewritten language-neutral catalogue (verified byte-identical to main). CLAUDE.md, REVIEW_CONTEXT.md and CONTRIBUTING.md reference it by path, so this mattered.
  • docs/roadmap_python.md / docs/TODO.md — accepted main's side. Nothing to restore, nothing to publish. This satisfies blocker 2 more completely than restoring them would have.

Verified the merge is purely additive vs main (51 additions, zero deletions), so nothing from #197/#198 was clobbered, and no dead links to the removed paths remain — the only mentions are inline code spans in historical records, which the analyses/ convention says not to edit.

Restructure + blockers

docsite/ holds all 44 site-source files; docs/ holds only DESIGN.md, REVIEW_CONTEXT.md, roadmap.md, decisions/, analyses/, plans/. docs_dir, edit_uri, snippets base_path and both hooks: paths retargeted. Include paths needed no change (same depth); the generator scripts needed none either (REPO_ROOT + virtual gen-files paths). Roadmap/TODO removed from nav with a comment recording that publishing an internal doc is a separate deliberate decision. --strict in CI; pull_request trigger with deploy split into its own main-gated job; hardcoded claude/plan-… branch deleted; permissions scoped per job ({} default → contents:read build, contents:write deploy only).

Two extras worth flagging: mkdocs was publishing the hook .py files and their __pycache__/*.pyc as static site assets (exclude_docs: hooks/ now prevents it), and my changelog switch required updating the hook's INCLUDE_SOURCES map or the included links would have failed the now-strict build.

Self-review found two bugs I introduced — fixed in 70bfc8b

  • Windows path separators in GitHub URLs. rewrite_external_links.py interpolated a pathlib.Path, emitting .../blob/main/docs\DESIGN.md — real 404s. Confirmed in the built site, fixed with as_posix(), re-verified zero backslash URLs. CI builds on Linux so the deployed site was fine; local previews were broken. My INCLUDE_SOURCES switch newly surfaced it on the changelog page.
  • Standalone mode regression. Narrowing the config lookup to (ImportError, AttributeError) — which I did to satisfy ruff BLE001 — broke build_matlab_examples.py from any cwd but the repo root, because mkdocs_gen_files.config loads mkdocs.yml and raises ConfigurationError. The broad catch was right; restored with an explicit justification.

Also from that review: the new API gate no longer keeps a hand-listed export set (it partitions sid.__all__ by inspect.isfunction, so a new result type routes to the results.md check instead of wrongly demanding a stub — both scenarios tested); it now also checks the two hand-written index manifests; the Binder hook derives owner/repo from repo_url, completing item 7; and the duplicate # Changelog H1 is gone.

One thing added beyond the list

CONTRIBUTING.md now documents the docs/ ↔ docsite/ split, why it exists, that publishing an internal doc is a deliberate nav change, and how to build locally (pip install -r requirements-docs.txt && mkdocs build --strict). The restructure's whole point is that a future contributor or agent can't break or leak into the site — that rule shouldn't live only in a mkdocs.yml comment. Say the word if you'd rather it were shorter or elsewhere.

Deliberately not done

  • Two ruff nits (FURB188, SIM114) in scripts/build_matlab_api.py — untouched original content, rules outside the project's pinned set and outside CI's lint scope.
  • Your follow-up list (required-check via ADR-0005 amendment, mike versioning, external-link checker, publishing ADRs) — all still open, none in scope here.
  • One-time after merge, still yours: Settings → Pages → source gh-pages, then verify the first deploy. Worth watching that the split deploy job resolves publish_dir correctly without a checkout step — it should, given force_orphan: true, but the first run is the proof.

Copy link
Copy Markdown
Owner Author

Re-review of the adaptation (commits 194768f + 70bfc8b)

Follow-up to the review above. Mechanical checks ran against head 70bfc8b: a full mkdocs build --strict from a clean worktree, a leak scan of the built site/, and both failure modes of the new API gate exercised — standalone and through the real gen-files build path. All 12 files changed since the original review were read in full.

Verdict: approve — all four blockers closed, all four discretionary items taken, plus fixes beyond what was asked.

Verified (all pass)

  • Restructure: docs_dir: docsite, docs/ now holds only internal documents, and the PR is pure additions against main (+1871, zero modifications to internal docs). edit_uri, snippets base_path, and hook paths all retargeted; docs/roadmap.md restored to its canonical path carrying main's docs(195): restructure roadmaps along the contract/implementation layers #197 rewrite.
  • Strict build: mkdocs build --strict exits 0 on a clean checkout (independently reproduced, not just CI). The only log noise is Jupyter kernel notices — zero mkdocs warnings. All 11 notebooks execute; both API references generate.
  • No leakage: nothing from docs/ (roadmap, decisions, analyses, plans, DESIGN, REVIEW_CONTEXT) appears anywhere under site/; no hook .py/.pyc files ship (the exclude_docs: hooks/ catch was a real find); zero backslash URLs in the built HTML, confirming the as_posix() fix.
  • Workflow: pull_request trigger present and CI's "Build site (strict)" ran green on this PR with deploy correctly skipped; hardcoded feature branch gone; permissions: {} at workflow level with contents: read/write scoped per job; deploy publishes the exact artefact the build job tested — nicer than rebuilding.
  • API gate (check_python_api_pages.py): verified it fails the actual mkdocs build on an orphan page (BUILD_EXIT=1, clear message) and reports missing pages, SUMMARY omissions, and index-table omissions. The inspect.isfunction partition instead of a hand-kept exempt list is the right instinct — it removes the manifest rather than relocating it.
  • Changelog: now mirrors root CHANGELOG.md with links to both per-language release notes; single H1 confirmed.
  • Merge-resolution judgment call endorsed: the instruction to keep TODO.md/roadmap_python.md at their original paths was written before docs(195): restructure roadmaps along the contract/implementation layers #197 deleted/archived them on main; taking main's side was correct and satisfies the no-dev-tracking blocker more completely. Well documented in the commit message.
  • The CONTRIBUTING.md section explaining the docs//docsite/ split — with the "publishing an internal doc is a deliberate nav change" rule — closes the loop properly: the invariant now lives where contributors read.

Non-blockers (author's discretion, no re-review needed)

  1. Gate's index-table check matches bare substrings (if name not in text): spectrogram is satisfied by spectrogram_plot appearing, so a dropped spectrogram row in api/index.md would go undetected. Matching the link form ((python/{name}.md)) instead would make this leg as tight as the SUMMARY leg.
  2. The PR description is now stale: it still says the site source lives in docs/, and the build-metrics table predates the adaptation. Worth refreshing before merge so the merged record matches the diff.
  3. Noting for the record, no action: util_msd.py ships into site/examples/python/ as a static asset (side effect of copying it beside the notebooks for build-time execution) — arguably useful for anyone downloading a notebook; NOTEBOOK_TITLES remains a hand-kept titles map, previously agreed as acceptable (auto-discovery fallback exists).

After merge (from the original test plan, still open)

One-time: repo Settings → Pages → source gh-pages / root, then verify https://pdlourenco.github.io/sid/ against the CI artefact.


Generated by Claude Code

Re-review non-blocker 1. The index-table leg used `if name not in text`, which a
longer sibling satisfies: `spectrogram` is present in `spectrogram_plot`, so a
dropped `(python/spectrogram.md)` row passed the check. The gate was weaker than
its SUMMARY leg, which already matched the link form.

Each index page is now paired with its own link template -- `(python/{name}.md)`
for docsite/api/index.md, `({name}.md)` for docsite/api/python/index.md, since
the two link with different prefixes -- and the failure message names the exact
link it expected.

Verified against the reviewer's own example: deleting the `(python/spectrogram.md)`
row now fails with a precise message, where before it passed.

Refs #110
@pdlourenco

Copy link
Copy Markdown
Owner Author

Both non-blockers taken — 3f3db33 plus a refreshed PR description.

1. Index-table check tightened. You were right that it was the loose leg. Reproduced your exact example: deleting the (python/spectrogram.md) row from api/index.md passed the old check, because spectrogram is a substring of spectrogram_plot. Each index page is now paired with its own link template — (python/{name}.md) for api/index.md, ({name}.md) for api/python/index.md, since the two link with different prefixes — and the failure message names the exact link it expected. Re-tested: that deletion now fails precisely; baseline still passes; strict build still exits 0.

That leg is now as tight as the SUMMARY one, which is what made the gap visible.

2. PR description refreshed. It described the pre-adaptation state throughout — docs/ as the site source, a stale metrics table, and a test plan whose first item was already done. Rewritten against head, with every number re-measured rather than carried over:

  • mkdocs build --strict → exit 0, zero warnings (noted against the 39 it started at)
  • 20 MATLAB API pages, 19 Python functions, 11 notebooks, 11 MATLAB examples, 89 rendered HTML pages
  • 0 internal docs leaked into site/, stated as a verified metric
  • A new "Layout" section explaining why docsite/ exists and that publishing an internal doc is a deliberate nav change
  • CI behaviour documented (strict on PRs, deploy gated and skipped here, per-job permissions)
  • Test plan first box ticked; your follow-up list recorded as explicitly out of scope

3. Noted, no action — agreed on both: util_msd.py shipping beside the notebooks is a useful side effect for anyone downloading one, and NOTEBOOK_TITLES keeps its auto-discovery fallback.

Ready to merge whenever you are. The one-time Pages setup (Settings → Pages → source gh-pages / root) and verifying the first real deploy against the CI artefact remain yours — worth watching that first run, since the split deploy job resolves publish_dir without a checkout step.

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