Validate documentation snapshots with mdast JSON#43
Closed
lodyai[bot] wants to merge 4 commits into
Closed
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 1 minute Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideAdds a detailed ExecPlan document outlining how to migrate documentation snapshot tests to use normalized mdast JSON semantics instead of raw Markdown formatting, including constraints, risks, implementation steps, and validation strategy for both the parent template and generated Rust projects. Sequence diagram for semantic mdast JSON snapshot test flowsequenceDiagram
actor Developer
participant Makefile
participant Pytest as pytest
participant MdastHelper as parse_markdown_semantics
participant MarkdownParser as parse_markdown
participant Syrupy as syrupy
Developer->>Makefile: make test
Makefile->>Pytest: pytest tests/test_template -k snapshot
Pytest->>MdastHelper: parse_markdown_semantics(markdown)
MdastHelper->>MarkdownParser: parse_markdown(markdown)
MarkdownParser-->>MdastHelper: parser_ast
MdastHelper-->>Pytest: normalized_mdast_json
Pytest->>Syrupy: snapshot.compare(normalized_mdast_json)
Syrupy-->>Pytest: assertion result
Pytest-->>Makefile: test pass/fail
Makefile-->>Developer: gate result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
lodyai
Bot
changed the title
Describe the approved path for replacing documentation snapshot checks with normalized mdast JSON assertions in both the parent template tests and generated Rust projects.
Parse generated Markdown documentation into normalized mdast JSON before snapshotting so wrapping and table alignment changes do not churn semantic snapshots.
Add generated Rust tests that parse documentation with markdown-rs and compare normalized mdast JSON with insta snapshots. Document the snapshot workflow for template and generated-project contributors.
Mark the execplan complete and capture the shipped parser choices, validation evidence, commits, and CodeRabbit review results.
pandalump
force-pushed
the
mdast-snapshot-assertions
branch
from
July 6, 2026 16:05
f9fe6d5 to
8910cf7
Compare
pandalump
marked this pull request as ready for review
July 6, 2026 16:05
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/developers-guide.md`:
- Around line 25-30: Update the documentation wording in the affected markdown
sections to use en-GB spellings: replace “normalized” with “normalised” and
“normalization” with “normalisation” in the prose that describes the Markdown
snapshot process, including the reference to
tests/helpers/markdown_semantics.py. Keep the rest of the content unchanged and
ensure the terminology is consistent across both updated sections in
docs/developers-guide.md.
In `@docs/execplans/mdast-snapshot-assertions.md`:
- Line 137: The markdown snapshot text contains an inline code span with a
trailing space in the `Plan: ` fragment, which triggers the MD038 syntax check.
Update the affected entry in the mdast snapshot assertions so the inline code
uses the trimmed form `Plan:` or rephrase the sentence to avoid spacing inside
backticks; make sure the fix is applied in the snapshot content itself where the
`Removed the` note is defined.
In `@template/docs/developers-guide.md.jinja`:
- Around line 20-28: The documentation guidance incorrectly refers contributors
to the nonexistent Make target `make fmt`; update the text in the developers
guide to point to the actual formatting target exposed by the generated
Makefile, `check-fmt`, and keep the surrounding workflow references in
`tests/documentation_snapshots.rs` and `markdownlint` unchanged.
In `@tests/helpers/markdown_semantics.py`:
- Line 32: Refactor _normalise_node to use structural pattern matching instead
of the current isinstance-based decomposition for dict and list values. Update
the branching in _normalise_node to match on the value shape directly with
match/case, covering both the dict handling and the list handling referenced in
the comment, while preserving the existing normalization behavior.
- Around line 14-17: Add numpy-style docstrings in the helper module: expand
parse_markdown_semantics with full structured documentation including Parameters
and Returns, and add at least a one-line summary docstring to the private
helpers _gfm_parse_options, _normalise_node, and _normalise_soft_text. Keep the
wording aligned with their actual behavior so the public API and internal
utilities all satisfy the path docstring requirements.
In `@tests/test_template/test_documentation_snapshots.py`:
- Line 41: The tests in the markdown snapshot checks use bare asserts, which
should be updated to include failure messages. In the
`test_documentation_snapshots` test functions, replace each bare `assert` with
the same condition plus a clear message so failures are easier to diagnose,
including the semantic comparison in the `parse_markdown_semantics` checks and
the other asserts covered in the same block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: af1a7efb-e5ad-4d16-a410-d50b6f4f8b73
📒 Files selected for processing (13)
Makefiledocs/developers-guide.mddocs/execplans/mdast-snapshot-assertions.mdtemplate/Cargo.toml.jinjatemplate/docs/developers-guide.md.jinjatemplate/tests/documentation_snapshots.rstemplate/tests/snapshots/documentation_snapshots__docs_contents.snap.jinjatemplate/tests/snapshots/documentation_snapshots__docs_repository_layout_app.snap.jinjatemplate/tests/snapshots/documentation_snapshots__docs_repository_layout_lib.snap.jinjatests/helpers/markdown_semantics.pytests/test_helpers.pytests/test_template/__snapshots__/test_documentation_snapshots.ambrtests/test_template/test_documentation_snapshots.py
Comment on lines
+25
to
+30
| Documentation snapshot tests parse Markdown files into normalized mdast JSON | ||
| before comparing snapshots. The normalization removes parser source positions | ||
| and treats soft line wrapping as formatting noise, while preserving document | ||
| structure such as headings, links, lists, tables, inline code, and code blocks. | ||
| Markdown formatting remains covered separately by `markdownlint-cli2` and | ||
| `mdtablefix`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use en-GB spellings here.
Replace normalized/normalization with normalised/normalisation in both updated sections.
Triage: [type:spelling]
As per path instructions, use en-GB-oxendict spelling in docs/**.
♻️ Proposed fix
-Documentation snapshot tests parse Markdown files into normalized mdast JSON
-before comparing snapshots. The normalization removes parser source positions
+Documentation snapshot tests parse Markdown files into normalised mdast JSON
+before comparing snapshots. The normalisation removes parser source positions
@@
-- `tests/helpers/markdown_semantics.py` parses Markdown as normalized mdast JSON
+- `tests/helpers/markdown_semantics.py` parses Markdown as normalised mdast JSONAlso applies to: 98-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/developers-guide.md` around lines 25 - 30, Update the documentation
wording in the affected markdown sections to use en-GB spellings: replace
“normalized” with “normalised” and “normalization” with “normalisation” in the
prose that describes the Markdown snapshot process, including the reference to
tests/helpers/markdown_semantics.py. Keep the rest of the content unchanged and
ensure the terminology is consistent across both updated sections in
docs/developers-guide.md.
Source: Path instructions
| 50 passed and 1 skipped. | ||
| - [x] 2026-06-28: Received explicit user approval to implement the planned | ||
| functionality and began execution. | ||
| - [x] 2026-06-28: Removed the `Plan: ` prefix from PR #43 and renamed the |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Trailing space inside inline code span.
`Plan: ` includes a trailing space inside the backticks (MD038). Trim it to `Plan:` or reword.
Triage: [type:syntax/md]
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 137-137: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/execplans/mdast-snapshot-assertions.md` at line 137, The markdown
snapshot text contains an inline code span with a trailing space in the `Plan: `
fragment, which triggers the MD038 syntax check. Update the affected entry in
the mdast snapshot assertions so the inline code uses the trimmed form `Plan:`
or rephrase the sentence to avoid spacing inside backticks; make sure the fix is
applied in the snapshot content itself where the `Removed the` note is defined.
Source: Linters/SAST tools
Comment on lines
+20
to
+28
| Documentation snapshot tests live in `tests/documentation_snapshots.rs`. They | ||
| parse selected Markdown files into normalized mdast JSON before comparing | ||
| `insta` snapshots, so line wrapping and table alignment changes do not churn | ||
| semantic snapshots. Markdown formatting is still checked by `make markdownlint` | ||
| and formatted by `make fmt`. When a documentation meaning or structure change | ||
| is intentional, update the affected `tests/snapshots/*.snap` files with | ||
| `INSTA_UPDATE=always cargo test --test documentation_snapshots`, then review | ||
| the JSON diff before committing it. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Replace make fmt with make check-fmt.
The generated Makefile exposes check-fmt, not fmt, so this instruction points contributors at a dead target.
Proposed fix
- Markdown formatting is still checked by `make markdownlint` and formatted by `make fmt`.
+ Markdown formatting is still checked by `make markdownlint` and validated by `make check-fmt`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Documentation snapshot tests live in `tests/documentation_snapshots.rs`. They | |
| parse selected Markdown files into normalized mdast JSON before comparing | |
| `insta` snapshots, so line wrapping and table alignment changes do not churn | |
| semantic snapshots. Markdown formatting is still checked by `make markdownlint` | |
| and formatted by `make fmt`. When a documentation meaning or structure change | |
| is intentional, update the affected `tests/snapshots/*.snap` files with | |
| `INSTA_UPDATE=always cargo test --test documentation_snapshots`, then review | |
| the JSON diff before committing it. | |
| Documentation snapshot tests live in `tests/documentation_snapshots.rs`. They | |
| parse selected Markdown files into normalized mdast JSON before comparing | |
| `insta` snapshots, so line wrapping and table alignment changes do not churn | |
| semantic snapshots. Markdown formatting is still checked by `make markdownlint` | |
| and validated by `make check-fmt`. When a documentation meaning or structure change | |
| is intentional, update the affected `tests/snapshots/*.snap` files with | |
| `INSTA_UPDATE=always cargo test --test documentation_snapshots`, then review | |
| the JSON diff before committing it. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@template/docs/developers-guide.md.jinja` around lines 20 - 28, The
documentation guidance incorrectly refers contributors to the nonexistent Make
target `make fmt`; update the text in the developers guide to point to the
actual formatting target exposed by the generated Makefile, `check-fmt`, and
keep the surrounding workflow references in `tests/documentation_snapshots.rs`
and `markdownlint` unchanged.
Comment on lines
+14
to
+17
| def parse_markdown_semantics(markdown: str) -> object: | ||
| """Parse Markdown into normalized mdast JSON.""" | ||
| tree = mdast.md_to_ast(markdown, config=_gfm_parse_options()) | ||
| return _normalise_node(tree) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add numpy-style docstrings per path instructions.
parse_markdown_semantics is a public interface but only carries a one-line summary; as per path instructions, public interfaces need full structured (Parameters/Returns) numpy-style docs. _gfm_parse_options, _normalise_node, and _normalise_soft_text are private and currently have no docstring at all, but the guideline requires at least a single-line summary for private functions/methods too.
Also applies to: 20-28, 50-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/helpers/markdown_semantics.py` around lines 14 - 17, Add numpy-style
docstrings in the helper module: expand parse_markdown_semantics with full
structured documentation including Parameters and Returns, and add at least a
one-line summary docstring to the private helpers _gfm_parse_options,
_normalise_node, and _normalise_soft_text. Keep the wording aligned with their
actual behavior so the public API and internal utilities all satisfy the path
docstring requirements.
Source: Path instructions
|
|
||
|
|
||
| def _normalise_node(value: Any) -> object: | ||
| if isinstance(value, dict): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Prefer structural pattern matching over isinstance() chains.
_normalise_node decomposes with isinstance(value, dict) / isinstance(value, list). As per path instructions, "Prefer structural pattern matching over isinstance() or imperative decomposition" for **/*.py.
♻️ Proposed refactor using match/case
def _normalise_node(value: Any) -> object:
- if isinstance(value, dict):
+ match value:
+ case dict():
normalised = {}
node_type = value.get("type")
for key, child in value.items():
if key in _POSITION_KEYS:
continue
if key == "value" and node_type == "text" and isinstance(child, str):
normalised[key] = _normalise_soft_text(child)
continue
normalised[key] = _normalise_node(child)
return normalised
-
- if isinstance(value, list):
+ case list():
return [_normalise_node(child) for child in value]
-
- return value
+ case _:
+ return valueAlso applies to: 44-46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/helpers/markdown_semantics.py` at line 32, Refactor _normalise_node to
use structural pattern matching instead of the current isinstance-based
decomposition for dict and list values. Update the branching in _normalise_node
to match on the value shape directly with match/case, covering both the dict
handling and the list handling referenced in the comment, while preserving the
existing normalization behavior.
Source: Path instructions
| - [x] Keep snapshots semantic | ||
| """ | ||
|
|
||
| assert parse_markdown_semantics(compact) == parse_markdown_semantics(reflowed) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add failure messages to asserts.
Both asserts are bare. As per path instructions, "Use assert …, message over bare asserts" for **/*.py.
✅ Proposed fix
- assert parse_markdown_semantics(compact) == parse_markdown_semantics(reflowed)
+ assert parse_markdown_semantics(compact) == parse_markdown_semantics(reflowed), (
+ "expected formatting-only Markdown differences to normalise identically"
+ )- assert {
+ assert (
+ {
"docs/contents.md": parse_markdown_semantics(
(project / "docs/contents.md").read_text(encoding="utf-8")
),
"docs/repository-layout.md": parse_markdown_semantics(
(project / "docs/repository-layout.md").read_text(encoding="utf-8")
),
- } == snapshot
+ }
+ == snapshot
+ ), "expected generated documentation to match the reviewed semantic snapshot"Also applies to: 56-63
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_template/test_documentation_snapshots.py` at line 41, The tests in
the markdown snapshot checks use bare asserts, which should be updated to
include failure messages. In the `test_documentation_snapshots` test functions,
replace each bare `assert` with the same condition plus a clear message so
failures are easier to diagnose, including the semantic comparison in the
`parse_markdown_semantics` checks and the other asserts covered in the same
block.
Source: Path instructions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement normalized mdast JSON documentation snapshots for both the parent Copier template tests and the generated Rust projects.
mdast, removes source-position metadata, normalizes soft text wrapping, and snapshots semantic JSON with syrupy.markdown-rs, normalize throughserde_json, and compareinstaJSON snapshots..github/workflows/release.ymlandsrc/main.rsremain part of the contract.Review Walkthrough
docs/execplans/mdast-snapshot-assertions.mdfor scope, decisions, red/green evidence, validation logs, and outcomes.tests/helpers/markdown_semantics.pyandtests/test_template/test_documentation_snapshots.pyfor the parent semantic snapshot contract.template/tests/documentation_snapshots.rsandtemplate/tests/snapshots/*.snap.jinjafor generated-project snapshot behaviour.Validation
make test 2>&1 | tee /tmp/test-agent-template-rust-mdast-snapshot-assertions-full.out: 52 passed, 1 skipped.coderabbit review --agentafter the parent snapshot milestone: 0 findings.coderabbit review --agentafter the generated snapshot milestone: 0 findings.References