Migrate docs from Jupyter Book to zensical#737
Conversation
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>
Reviewer's GuideMigrates 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 nbdocssequenceDiagram
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
Flow diagram for notebook and markdown processing pipelineflowchart 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]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
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>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| --ExecutePreprocessor.timeout=600 "$$nb" 2>/dev/null || true; \ | ||
| fi; \ | ||
| done | ||
| python docs/hooks.py docs/notebooks/*.md |
There was a problem hiding this comment.
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).
| 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) |
There was a problem hiding this comment.
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.
| 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) |
| ) | ||
|
|
||
|
|
||
| def _myst_citations_to_bibtex(markdown: str) -> str: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| @@ -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. | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| 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, | ||
| ) |
There was a problem hiding this comment.
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| extra_javascript: | ||
| - https://polyfill.io/v3/polyfill.min.js?features=es6 | ||
| - https://cdn.jsdelivr.net/npm/mathjax@3/es5/tex-mml-chtml.js |
There was a problem hiding this comment.
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| 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() |
There was a problem hiding this comment.
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.
| 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") |
| --ExecutePreprocessor.timeout=600 "$$nb" 2>/dev/null || true; \ | ||
| fi; \ | ||
| done | ||
| python docs/hooks.py docs/notebooks/*.md |
| @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 |
There was a problem hiding this comment.
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>
Summary
Closes #736
api_design.rst,api_verification.rst) to mkdocstrings markdown directivesdocs/hooks.pyfor MyST→Material admonition conversion and{cite:p}→[@key]bibtex citation conversiondocs/zensical.ymlwith full nav structure matching the previous_toc.ymlnbdocstarget converts notebooks to markdown (with execution split — heavy-sim notebooks like meep/palace/lumerical convert without execution),docstarget runs zensical buildmake nbdocs+zensical buildjupyter-bookdependency withzensical,mkdocstrings[python],mkdocs-bibtex_config.yml,_toc.yml, RST API filesTest plan
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:
Enhancements:
Build:
CI:
Documentation: