Skip to content

Migrate docs from Jupyter Book to zensical#737

Open
jackgdsf wants to merge 2 commits into
gdsfactory:mainfrom
jackgdsf:migrate-docs-to-zensical
Open

Migrate docs from Jupyter Book to zensical#737
jackgdsf wants to merge 2 commits into
gdsfactory:mainfrom
jackgdsf:migrate-docs-to-zensical

Conversation

@jackgdsf

@jackgdsf jackgdsf commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #736

  • Replace Jupyter Book/Sphinx with zensical (MkDocs Material), matching the gdsfactory migration
  • Convert RST API docs (api_design.rst, api_verification.rst) to mkdocstrings markdown directives
  • Add docs/hooks.py for MyST→Material admonition conversion and {cite:p}[@key] bibtex citation conversion
  • Add docs/zensical.yml with full nav structure matching the previous _toc.yml
  • Update Makefile: nbdocs target converts notebooks to markdown (with execution split — heavy-sim notebooks like meep/palace/lumerical convert without execution), docs target runs zensical build
  • Update CI workflow to use make nbdocs + zensical build
  • Replace jupyter-book dependency with zensical, mkdocstrings[python], mkdocs-bibtex
  • Remove old _config.yml, _toc.yml, RST API files

Test plan

  • CI docs build passes
  • Nav structure matches previous site
  • API reference pages render with mkdocstrings
  • Executed notebooks show output
  • Non-executed notebooks render code cells without errors
  • Citations render correctly (path_length_analysis)

Reminder: AI-created PRs still require human review of the actual code changes before merge. I'll open the PR, but a human must review and approve it.

🤖 Generated with Claude Code

Summary by Sourcery

Migrate the documentation site from Jupyter Book/Sphinx to a MkDocs Material-based zensical setup, including updated notebook handling and API reference generation.

New Features:

  • Introduce a zensical (MkDocs Material) configuration for building and serving the documentation site.
  • Add mkdocstrings-based API reference pages for design and verification modules.
  • Add a notebook post-processing hook to normalize MyST admonitions, citations, and brace usage for MkDocs.

Enhancements:

  • Update the Makefile to convert notebooks to markdown with selective execution and to build/serve docs via zensical.
  • Align the documentation navigation structure in zensical with the previous Jupyter Book table of contents.
  • Simplify several documentation pages by removing MyST table-of-contents directives and switching to snippet includes for README and changelog.

Build:

  • Update documentation extras to use zensical, mkdocstrings, mkdocs-bibtex, and nbconvert instead of Jupyter Book.
  • Adjust pre-commit YAML and Jinja linting configuration to exclude the new zensical configuration file from certain checks.

CI:

  • Modify the docs CI workflow to run notebook conversion and build the site with zensical instead of Jupyter Book.

Documentation:

  • Replace legacy Jupyter Book/Sphinx configuration and RST-based API docs with MkDocs/MkDocStrings markdown equivalents and a new navigation file.

Replace Jupyter Book/Sphinx docs with zensical, matching the gdsfactory
migration. Notebooks are converted to markdown via nbconvert with
execution split: simulatable notebooks run with --execute, heavy-sim
notebooks (meep, palace, lumerical, etc.) convert without execution.

- Add docs/zensical.yml with full nav matching previous _toc.yml
- Add docs/hooks.py for MyST→Material admonition and citation conversion
- Convert RST API docs to mkdocstrings markdown directives
- Update Makefile with nbdocs/docs/docs-serve targets
- Update CI workflow to use zensical build
- Replace jupyter-book dep with zensical + mkdocstrings + mkdocs-bibtex
- Remove _config.yml, _toc.yml, and RST API files

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Migrates the documentation system from Jupyter Book/Sphinx to zensical (MkDocs Material), including notebook conversion tooling, API reference migration to mkdocstrings, custom markdown post-processing hooks, and CI/build pipeline updates.

Sequence diagram for CI docs build with zensical and nbdocs

sequenceDiagram
    actor CI
    participant Make as make
    participant Nb as jupyter_nbconvert
    participant Hooks as python_docs_hooks
    participant Z as zensical

    CI->>Make: make nbdocs
    Make->>Nb: jupyter nbconvert --to markdown [exec/no-exec]
    Nb-->>Make: .md notebooks with images
    Make->>Hooks: python docs/hooks.py docs/notebooks/*.md
    Hooks-->>Make: processed markdown
    CI->>Z: zensical build --strict -f docs/zensical.yml
    Z-->>CI: static site in _build/html
Loading

Flow diagram for notebook and markdown processing pipeline

flowchart LR
    A[.ipynb notebooks in docs] --> B[make nbdocs]
    B --> C{matches NOEXEC_PATTERNS?}
    C -->|yes| D[jupyter nbconvert --to markdown --embed-images]
    C -->|no| E[jupyter nbconvert --to markdown --execute --embed-images]
    D --> F[.md in docs/notebooks]
    E --> F
    F --> G[python docs/hooks.py docs/notebooks/*.md]
    G --> H[Processed markdown for zensical]
    H --> I[zensical build --strict -f docs/zensical.yml]
Loading

File-Level Changes

Change Details Files
Switch docs build from Jupyter Book/Sphinx to zensical (MkDocs Material) and adjust Makefile targets accordingly.
  • Replace jb build docs invocation with zensical build using a new docs/zensical.yml configuration.
  • Introduce nbdocs Makefile target to convert all notebooks to markdown, executing most notebooks while skipping heavy simulations based on NOEXEC_PATTERNS.
  • Add docs-serve Makefile target for local docs preview via zensical.
  • Update PHONY targets to include nbdocs and docs-serve.
Makefile
docs/zensical.yml
Update documentation dependencies from Jupyter Book to MkDocs/zensical and supporting plugins.
  • Remove jupyter-book from docs extra dependencies.
  • Add jupyter, nbconvert, mkdocstrings[python], mkdocs-bibtex, and zensical to docs extra dependencies.
  • Keep existing plotting and visualization dependencies intact.
pyproject.toml
Adjust GitHub Actions workflow to use the new nbdocs + zensical build pipeline for docs publication.
  • Rename docs build job to a generic name and keep existing setup steps.
  • Replace jb build docs command with make nbdocs followed by zensical build using docs/zensical.yml.
  • Ensure artifacts are still uploaded for GitHub Pages deployment.
.github/workflows/pages.yml
Exclude the zensical MkDocs configuration file from YAML-related pre-commit hooks that may be incompatible with its syntax.
  • Update check-yaml hook to exclude docs/zensical.yml.
  • Update j2lint hook exclusions to include docs/zensical.yml.
.pre-commit-config.yaml
Convert MyST-style includes and table-of-contents blocks to MkDocs Material/zensical idioms in existing markdown docs.
  • Replace MyST include directives in index and changelog docs with mkdocs-material snippets syntax using --8<-- includes.
  • Remove MyST {tableofcontents} blocks from plugin and workflow markdown pages, relying on MkDocs nav instead.
docs/index.md
docs/changelog.md
docs/plugins_circuits.md
docs/plugins_fdtd.md
docs/plugins_mode_solver.md
docs/plugins_optimization.md
docs/plugins_process.md
docs/workflow.md
Add a zensical/MkDocs configuration file defining theme, navigation, plugins, markdown extensions, and snippet handling.
  • Define site metadata, theme (Material), logo/favicons, and navigation structure mirroring the previous Jupyter Book TOC.
  • Configure mkdocstrings and bibtex plugins for API docs and citations.
  • Enable various pymdownx extensions and math rendering via external JS.
docs/zensical.yml
Migrate API reference pages from RST to mkdocstrings-based markdown for design and verification APIs.
  • Add api_design.md with mkdocstrings directives referencing gplugins design-related APIs.
  • Add api_verification.md with mkdocstrings directives for KLayout DRC and dataprep APIs.
  • Remove legacy RST API files that were used by Sphinx/Jupyter Book.
docs/api_design.md
docs/api_verification.md
docs/api_design.rst
docs/api_verification.rst
Introduce a post-processing hook script for notebook-generated markdown to align MyST content with MkDocs Material and mkdocstrings.
  • Add docs/hooks.py implementing transformations for MyST admonitions into Material-style !!! blocks.
  • Convert MyST citation syntax {cite:p}key into mkdocs-bibtex citation form [@key].
  • Escape {WORD} patterns outside code/math to avoid mkdocstrings resolution issues, and wire the hook into nbdocs processing.
docs/hooks.py
Makefile
Remove obsolete Jupyter Book configuration files now superseded by zensical.yml.
  • Delete _config.yml and _toc.yml from docs, as their roles are taken over by zensical.yml.
  • Ensure navigation and build configuration are fully driven by MkDocs/zensical.
docs/_config.yml
docs/_toc.yml

Assessment against linked issues

Issue Objective Addressed Explanation
#736 Replace Jupyter Book/Sphinx configuration files (_config.yml, _toc.yml) with a docs/zensical.yml MkDocs Material configuration, and update existing markdown docs to be compatible.
#736 Convert RST-based API documentation (api_design.rst, api_verification.rst) to mkdocstrings-based markdown files.
#736 Update documentation tooling to use zensical: add hooks.py for MyST→Material and citation conversion, switch Makefile and CI workflow from jb build to nbconvert + zensical build, and replace the Jupyter Book dependency with zensical + mkdocstrings + mkdocs-bibtex.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 4 issues, and left some high level feedback:

  • The nbdocs Makefile target currently swallows all nbconvert stderr and success/failure via 2>/dev/null || true, which makes it hard to notice broken notebook conversions; consider at least logging failures or failing the docs build when nbconvert exits non‑zero.
  • The hard-coded NOEXEC_PATTERNS regex in the Makefile will be difficult to maintain as notebook names evolve; consider moving this list to a separate config file or a clearly documented variable so changes are easier and less error-prone.
  • The regex-based transformations in docs/hooks.py (especially _escape_curly_braces and _myst_to_material_admonitions) are fairly complex and could affect non-MyST content; it would help to constrain their scope (e.g., to known notebook directories or patterns) or add comments explaining the assumptions behind the patterns to reduce future breakage when markdown formats change.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The nbdocs Makefile target currently swallows all nbconvert stderr and success/failure via `2>/dev/null || true`, which makes it hard to notice broken notebook conversions; consider at least logging failures or failing the docs build when nbconvert exits non‑zero.
- The hard-coded NOEXEC_PATTERNS regex in the Makefile will be difficult to maintain as notebook names evolve; consider moving this list to a separate config file or a clearly documented variable so changes are easier and less error-prone.
- The regex-based transformations in docs/hooks.py (especially _escape_curly_braces and _myst_to_material_admonitions) are fairly complex and could affect non-MyST content; it would help to constrain their scope (e.g., to known notebook directories or patterns) or add comments explaining the assumptions behind the patterns to reduce future breakage when markdown formats change.

## Individual Comments

### Comment 1
<location path="Makefile" line_range="80" />
<code_context>
+	nbdocs:
</code_context>
<issue_to_address>
**issue:** Post-processing only `docs/notebooks/*.md` may miss other converted notebooks under `docs/`.

`nbdocs` converts all `docs/**/*.ipynb` to markdown, but the hook only processes `docs/notebooks/*.md`. Notebooks placed elsewhere under `docs/` will produce unprocessed `.md` files, which can cause inconsistent rendering or mkdocstrings issues. Either limit the `find` in `nbdocs` to `docs/notebooks` or expand the hook to cover all generated `.md` files under `docs/` (e.g., via a matching `find` or a pattern tied to the conversion output).
</issue_to_address>

### Comment 2
<location path="docs/hooks.py" line_range="16-34" />
<code_context>
+    return markdown
+
+
+def _escape_curly_braces(markdown: str) -> str:
+    r"""Escape {WORD} patterns that mkdocstrings would try to resolve.
+
+    Skips content inside code blocks, inline code, and math delimiters.
+    """
+    _IGNORE = re.compile(
+        r"```.*?```|`[^`\n]+`|\$\$.*?\$\$|\$(?!\s)[^$]+?(?<!\s)\$",
+        re.DOTALL,
+    )
+    parts = _IGNORE.split(markdown)
+    ignored = _IGNORE.findall(markdown)
+    for i, part in enumerate(parts):
+        parts[i] = re.sub(r"\{([A-Z_]+)\}", r"\1", part)
+    result: list[str] = []
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Curly-brace handling removes braces rather than escaping, which may alter legitimate content.

This helper currently turns `{WORD}` into `WORD`, so any intentional `{FOO}` literals (e.g., config examples, placeholders, or math-like notation outside `$...$`) will be altered in the rendered docs. Consider preserving the visual text by escaping the braces (e.g. `\{WORD\}`) or wrapping these patterns in inline code so mkdocstrings ignores them without changing what users see.

```suggestion
def _escape_curly_braces(markdown: str) -> str:
    r"""Escape {WORD} patterns that mkdocstrings would try to resolve.

    Skips content inside code blocks, inline code, and math delimiters.
    Escapes braces so the rendered text remains visually unchanged.
    """
    _IGNORE = re.compile(
        r"```.*?```|`[^`\n]+`|\$\$.*?\$\$|\$(?!\s)[^$]+?(?<!\s)\$",
        re.DOTALL,
    )
    parts = _IGNORE.split(markdown)
    ignored = _IGNORE.findall(markdown)
    for i, part in enumerate(parts):
        # Turn `{WORD}` into `\{WORD\}` so mkdocstrings does not resolve it,
        # while preserving the literal text in rendered documentation.
        parts[i] = re.sub(r"\{([A-Z_]+)\}", r"\\{\1\\}", part)
    result: list[str] = []
    for i, part in enumerate(parts):
        result.append(part)
        if i < len(ignored):
            result.append(ignored[i])
    return "".join(result)
```
</issue_to_address>

### Comment 3
<location path="docs/hooks.py" line_range="66" />
<code_context>
+    )
+
+
+def _myst_citations_to_bibtex(markdown: str) -> str:
+    r"""Convert MyST {cite:p}`key` to mkdocs-bibtex [@key] syntax."""
+    return re.sub(r"\{cite:[a-z]+\}`([^`]+)`", r"[@\1]", markdown)
</code_context>
<issue_to_address>
**issue:** Citation conversion doesn’t handle multiple keys in a single `{cite:...}` block correctly.

Right now the regex turns `{cite:p}`foo; bar`` into `[@foo; bar]`, but mkdocs-bibtex expects separate keys like `[@foo; @bar]`. This means grouped citations won’t resolve correctly. Consider splitting the captured text on `[,;]`, trimming, and rebuilding it as `[@k1; @k2]` before substitution.
</issue_to_address>

### Comment 4
<location path="docs/plugins_fdtd.md" line_range="15" />
<code_context>

 Gdsfactory follows the Sparameters syntax `o1@0,o2@0` where `o1` is the input port `@0` mode 0 (usually fundamental TE mode) and `o2@0` refers to output port `o2` mode 0.

-```{tableofcontents}
</code_context>
<issue_to_address>
**suggestion (typo):** Consider correcting "Sparameters" to the standard term "S-parameters".

Using the standard term "S-parameters" (or "S-parameter" in singular) here will better match common technical usage.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread Makefile
--ExecutePreprocessor.timeout=600 "$$nb" 2>/dev/null || true; \
fi; \
done
python docs/hooks.py docs/notebooks/*.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: Post-processing only docs/notebooks/*.md may miss other converted notebooks under docs/.

nbdocs converts all docs/**/*.ipynb to markdown, but the hook only processes docs/notebooks/*.md. Notebooks placed elsewhere under docs/ will produce unprocessed .md files, which can cause inconsistent rendering or mkdocstrings issues. Either limit the find in nbdocs to docs/notebooks or expand the hook to cover all generated .md files under docs/ (e.g., via a matching find or a pattern tied to the conversion output).

Comment thread docs/hooks.py
Comment on lines +16 to +34
def _escape_curly_braces(markdown: str) -> str:
r"""Escape {WORD} patterns that mkdocstrings would try to resolve.

Skips content inside code blocks, inline code, and math delimiters.
"""
_IGNORE = re.compile(
r"```.*?```|`[^`\n]+`|\$\$.*?\$\$|\$(?!\s)[^$]+?(?<!\s)\$",
re.DOTALL,
)
parts = _IGNORE.split(markdown)
ignored = _IGNORE.findall(markdown)
for i, part in enumerate(parts):
parts[i] = re.sub(r"\{([A-Z_]+)\}", r"\1", part)
result: list[str] = []
for i, part in enumerate(parts):
result.append(part)
if i < len(ignored):
result.append(ignored[i])
return "".join(result)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Curly-brace handling removes braces rather than escaping, which may alter legitimate content.

This helper currently turns {WORD} into WORD, so any intentional {FOO} literals (e.g., config examples, placeholders, or math-like notation outside $...$) will be altered in the rendered docs. Consider preserving the visual text by escaping the braces (e.g. \{WORD\}) or wrapping these patterns in inline code so mkdocstrings ignores them without changing what users see.

Suggested change
def _escape_curly_braces(markdown: str) -> str:
r"""Escape {WORD} patterns that mkdocstrings would try to resolve.
Skips content inside code blocks, inline code, and math delimiters.
"""
_IGNORE = re.compile(
r"```.*?```|`[^`\n]+`|\$\$.*?\$\$|\$(?!\s)[^$]+?(?<!\s)\$",
re.DOTALL,
)
parts = _IGNORE.split(markdown)
ignored = _IGNORE.findall(markdown)
for i, part in enumerate(parts):
parts[i] = re.sub(r"\{([A-Z_]+)\}", r"\1", part)
result: list[str] = []
for i, part in enumerate(parts):
result.append(part)
if i < len(ignored):
result.append(ignored[i])
return "".join(result)
def _escape_curly_braces(markdown: str) -> str:
r"""Escape {WORD} patterns that mkdocstrings would try to resolve.
Skips content inside code blocks, inline code, and math delimiters.
Escapes braces so the rendered text remains visually unchanged.
"""
_IGNORE = re.compile(
r"```.*?```|`[^`\n]+`|\$\$.*?\$\$|\$(?!\s)[^$]+?(?<!\s)\$",
re.DOTALL,
)
parts = _IGNORE.split(markdown)
ignored = _IGNORE.findall(markdown)
for i, part in enumerate(parts):
# Turn `{WORD}` into `\{WORD\}` so mkdocstrings does not resolve it,
# while preserving the literal text in rendered documentation.
parts[i] = re.sub(r"\{([A-Z_]+)\}", r"\\{\1\\}", part)
result: list[str] = []
for i, part in enumerate(parts):
result.append(part)
if i < len(ignored):
result.append(ignored[i])
return "".join(result)

Comment thread docs/hooks.py
)


def _myst_citations_to_bibtex(markdown: str) -> str:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: Citation conversion doesn’t handle multiple keys in a single {cite:...} block correctly.

Right now the regex turns {cite:p}foo; bar`` into [@foo; bar], but mkdocs-bibtex expects separate keys like `[@foo; @bar]`. This means grouped citations won’t resolve correctly. Consider splitting the captured text on `[,;]`, trimming, and rebuilding it as `[@k1; @K2]` before substitution.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean buddy?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m talking about how grouped citations are rewritten.

Right now your function is:

def _myst_citations_to_bibtex(markdown: str) -> str:
    r"""Convert MyST {cite:p}`key` to mkdocs-bibtex [@key] syntax."""
    return re.sub(r"\{cite:[a-z]+\}`([^`]+)`", r"[@\1]", markdown)

This works fine for a single key:

  • Input: {cite:p}smith2020`
  • Output: [@smith2020]

But if the source has multiple keys in one cite, which is common in MyST:

  • Input: {cite:p}smith2020; jones2021`
  • Current output: [@smith2020; jones2021]

mkdocs-bibtex expects each key to be individually prefixed with @, like:

  • Expected: [@smith2020; @jones2021]

So the issue is: in the multiple-key case the second (and later) keys don’t get an @, and those citations won’t resolve.

One way to fix it is to post-process the captured group and rebuild it:

def _myst_citations_to_bibtex(markdown: str) -> str:
    """Convert MyST {cite:...}`k1; k2` to mkdocs-bibtex [@k1; @k2]."""

    def _replace(match: re.Match[str]) -> str:
        raw = match.group(1)
        # split on comma or semicolon, keep the original separators
        parts = re.split(r"([;,])", raw)
        out: list[str] = []
        for part in parts:
            if part in {",", ";"}:
                out.append(part)
            else:
                key = part.strip()
                if not key:
                    continue
                out.append(f" @{key}")  # leading space to separate from previous
        # join and normalize spacing around separators
        text = "".join(out)
        text = re.sub(r"\s*([;,])\s*", r"; ", text)
        text = text.strip()
        return f"[@{text.lstrip('@')}]"  # ensure first key has @

    return re.sub(r"\{cite:[a-z]+\}`([^`]+)`", _replace, markdown)

Not saying you must use exactly this implementation, just that the current regex only really handles the single-key case. If you’re sure your notebooks only ever use single-key cite blocks, then it’s fine as-is; otherwise multi-key cites will partially break.

Comment thread docs/plugins_fdtd.md
@@ -14,5 +14,3 @@ gdsfactory provides you a similar python API to drive 3 different FDTD simulator

Gdsfactory follows the Sparameters syntax `o1@0,o2@0` where `o1` is the input port `@0` mode 0 (usually fundamental TE mode) and `o2@0` refers to output port `o2` mode 0.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (typo): Consider correcting "Sparameters" to the standard term "S-parameters".

Using the standard term "S-parameters" (or "S-parameter" in singular) here will better match common technical usage.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the documentation build system from Jupyter Book to MkDocs (using Zensical and the Material theme). It introduces a Python hook script to post-process notebook markdown, updates the Makefile to handle notebook conversion and building, and adjusts dependencies in pyproject.toml. The review feedback highlights a potential bug in the MyST admonition regex parser, a critical security vulnerability regarding the use of the compromised polyfill.io domain, and opportunities to improve directory handling and error visibility during the build process.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread docs/hooks.py
Comment on lines +37 to +63
def _myst_to_material_admonitions(markdown: str) -> str:
r"""Convert MyST ```{type}\n...\n``` to Material !!! type\n\n ..."""

def _replace(match: re.Match[str]) -> str:
kind = match.group("kind")
body = match.group("body")
lines = body.splitlines()
title = ""
while lines and not lines[0].strip():
lines.pop(0)
if lines and (m := re.match(r"^#+ +(.+)", lines[0])):
title = m.group(1)
lines.pop(0)
non_empty = [line for line in lines if line.strip()]
min_indent = min(
(len(line) - len(line.lstrip()) for line in non_empty), default=0
)
dedented = [line[min_indent:] for line in lines]
indented = [" " + line if line.strip() else "" for line in dedented]
header = f'!!! {kind} "{title}"' if title else f"!!! {kind}"
return header + "\n\n" + "\n".join(indented)

return re.sub(
r"(?ms)^(?P<fence>`{3,})\{(?P<kind>\w+)\}\s*\n(?P<body>.*?)\n(?P=fence)$",
_replace,
markdown,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current regular expression-based conversion of MyST admonitions uses a lazy match (?P<body>.*?) which will prematurely match the closing fence of any nested code block (e.g., a python code block inside a note) if they use the same number of backticks. This will corrupt the generated markdown.

Additionally, MyST allows titles on the same line as the directive (e.g., ````{note} Title`). The current regex does not match these.

Using a robust line-by-line parser resolves both issues.

def _myst_to_material_admonitions(markdown: str) -> str:
    """Convert MyST admonitions to Material style."""
    lines = markdown.splitlines()
    result = []
    i = 0
    n = len(lines)
    while i < n:
        line = lines[i]
        m = re.match(r"^(?P<fence>\x60{3,})\{(?P<kind>\w+)\}(?P<title_part>.*)$", line)
        if m:
            fence = m.group("fence")
            kind = m.group("kind")
            title = m.group("title_part").strip()
            
            body_lines = []
            i += 1
            found_closing = False
            while i < n:
                if lines[i].strip() == fence:
                    found_closing = True
                    break
                body_lines.append(lines[i])
                i += 1
            
            if found_closing:
                if not title:
                    while body_lines and not body_lines[0].strip():
                        body_lines.pop(0)
                    if body_lines and (tm := re.match(r"^#+ +(.+)", body_lines[0])):
                        title = tm.group(1)
                        body_lines.pop(0)
                
                non_empty = [bl for bl in body_lines if bl.strip()]
                min_indent = min(
                    (len(bl) - len(bl.lstrip()) for bl in non_empty), default=0
                )
                dedented = [bl[min_indent:] for bl in body_lines]
                indented = ["    " + bl if bl.strip() else "" for bl in dedented]
                header = f'!!! {kind} "{title}"' if title else f"!!! {kind}"
                result.append(header + "\n" + "\n".join(indented))
            else:
                result.append(line)
                result.extend(body_lines)
        else:
            result.append(line)
        i += 1
    
    res = "\n".join(result)
    if markdown.endswith("\n") and not res.endswith("\n"):
        res += "\n"
    return res

Comment thread docs/zensical.yml
Comment on lines +113 to +115
extra_javascript:
- https://polyfill.io/v3/polyfill.min.js?features=es6
- https://cdn.jsdelivr.net/npm/mathjax@3/es5/tex-mml-chtml.js

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The polyfill.io domain was sold to a third party in early 2024 and has been actively used to inject malware and malicious redirects into websites using it. It is highly recommended to remove or replace any references to polyfill.io.

Since MathJax 3 has excellent native support in all modern browsers, the ES6 polyfill is no longer necessary and can be safely removed.

extra_javascript:
  - https://cdn.jsdelivr.net/npm/mathjax@3/es5/tex-mml-chtml.js

Comment thread docs/hooks.py
Comment on lines +71 to +83
def main() -> None:
for path_str in sys.argv[1:]:
path = Path(path_str)
if not path.exists():
continue
text = path.read_text(encoding="utf-8")
processed = process(text)
if processed != text:
path.write_text(processed, encoding="utf-8")


if __name__ == "__main__":
main()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Enhance main() to support directory paths recursively. This avoids relying on shell globbing (which can hit command-line length limits or fail in certain shell environments) and allows processing nested directories of notebooks if they are added in the future.

Suggested change
def main() -> None:
for path_str in sys.argv[1:]:
path = Path(path_str)
if not path.exists():
continue
text = path.read_text(encoding="utf-8")
processed = process(text)
if processed != text:
path.write_text(processed, encoding="utf-8")
if __name__ == "__main__":
main()
def main() -> None:
for path_str in sys.argv[1:]:
path = Path(path_str)
if not path.exists():
continue
if path.is_dir():
paths = path.rglob("*.md")
else:
paths = [path]
for p in paths:
text = p.read_text(encoding="utf-8")
processed = process(text)
if processed != text:
p.write_text(processed, encoding="utf-8")

Comment thread Makefile
--ExecutePreprocessor.timeout=600 "$$nb" 2>/dev/null || true; \
fi; \
done
python docs/hooks.py docs/notebooks/*.md

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since docs/hooks.py now supports directory paths recursively, we can pass the directory directly instead of relying on shell glob expansion.

	python docs/hooks.py docs/notebooks

Comment thread Makefile
Comment on lines +68 to +79
@find docs -name "*.ipynb" | while read nb; do \
if echo "$$nb" | grep -qE '$(NOEXEC_PATTERNS)'; then \
echo " [no-exec] $$nb"; \
jupyter nbconvert --to markdown --embed-images "$$nb" 2>/dev/null || true; \
else \
echo " [exec] $$nb"; \
PYVISTA_OFF_SCREEN=0 PYVISTA_JUPYTER_BACKEND=html \
jupyter nbconvert --to markdown --execute --embed-images \
--ExecutePreprocessor.allow_errors=True \
--ExecutePreprocessor.timeout=600 "$$nb" 2>/dev/null || true; \
fi; \
done

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Silencing all stderr output with 2>/dev/null makes it extremely difficult to debug notebook conversion or execution failures in CI or local builds. Removing 2>/dev/null allows errors to be printed to the console while still allowing the build to proceed via || true.

	@find docs -name "*.ipynb" | while read nb; do \
		if echo "$$nb" | grep -qE '$(NOEXEC_PATTERNS)'; then \
			echo "  [no-exec] $$nb"; \
			jupyter nbconvert --to markdown --embed-images "$$nb" || true; \
		else \
			echo "  [exec] $$nb"; \
			PYVISTA_OFF_SCREEN=0 PYVISTA_JUPYTER_BACKEND=html \
			jupyter nbconvert --to markdown --execute --embed-images \
				--ExecutePreprocessor.allow_errors=True \
				--ExecutePreprocessor.timeout=600 "$$nb" || true; \
		fi; \
	done

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

Migrate docs from Jupyter Book to zensical

2 participants