Reject a Backtick in a Backtick-Fence Info String - #904
Conversation
CodeRabbit, on PR #901: per CommonMark, a backtick-fenced opener's info string may not itself contain a backtick (the restriction exists so inline code spans are not misread as a new fence). _fence_step() accepted ```md` as a valid opener regardless, so a heading after it was hidden. Verified against the actual code before fixing. A tilde fence has no such restriction and is unaffected. 2 regression cases added to unfenced_text, 1 to undeclared_h2_headings. Also split the two over-25-word docstring sentences CodeRabbit flagged in the same round.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
PR Summary by QodoReject backticks in backtick-fence info strings per CommonMark
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
There was a problem hiding this comment.
🟢 Approval recommended
The change aligns _fence_step() with CommonMark, and the added selftests demonstrate the corrected behavior without introducing new risk.
Pull request overview
Tightens the Markdown fence parser used by spec/audit.py to match CommonMark’s restriction on backtick-fenced info strings, preventing invalid openers (e.g., `````md` ``) from hiding subsequent headings during scans.
Changes:
- Update
_fence_step()so a backtick-fence opener is rejected when its info string contains a backtick. - Add regression selftests covering the invalid-backtick opener case (and confirming tildes are unaffected).
- Split previously overlong docstring sentences while updating the fence-parsing explanation.
File summaries
| File | Description |
|---|---|
| spec/audit.py | Enforces the CommonMark backtick-in-info-string restriction in _fence_step() and adds regression coverage to prevent heading-hiding regressions. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code Review by Qodo
1. _fence_step docstring wraps sentences
|
| A mismatched or shorter marker does not close it, such as a ~~~ example inside a ``` block, or a | ||
| ``` inside a longer ````. Trailing text does not close it either, since an opening fence's language | ||
| tag has no closing counterpart. A backtick fence's info string may not itself contain a backtick, | ||
| per CommonMark, so such an opener is not a boundary either. |
There was a problem hiding this comment.
1. _fence_step docstring wraps sentences 📜 Skill insight ✧ Quality
The updated _fence_step() docstring wraps sentences across lines and also places multiple sentences on a single line. This violates the requirement that multi-line comments/docstrings use exactly one sentence per line without mid-sentence line wraps.
Agent Prompt
## Issue description
The `_fence_step()` docstring is multi-line prose that wraps mid-sentence and combines multiple sentences on one line, which violates the comment-structure rule.
## Issue Context
This docstring was edited in this PR and now contains several sentences that are split across lines and/or share a line with another sentence.
## Fix Focus Areas
- spec/audit.py[395-401]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
## Summary Follow-up to #904, addressing a suppressed Copilot finding raised on the #900 -> #901 promotion PR review. `undeclared_h2_headings()`'s docstring read as if a bare \`##\`-prefixed shell comment is never misread as a heading anywhere, when that only holds inside a fenced code sample, the same as the heading-syntax example right beside it. Reworded so both read as one example of fenced content rather than two independent claims. ## Verification - \`python3 spec/audit.py --selftest\` -> \`SELFTEST PASS\` - \`python3 spec/validate.py\` -> \`Spec validation OK\` - \`ruff check .\` / \`ruff format --check .\` -> clean - \`python3 scripts/prose_lint.py --diff origin/develop spec/audit.py\` -> clean
Summary
Follow-up to #903, addressing a finding raised on the #900 -> #901 promotion PR review (CodeRabbit).
Per CommonMark, a backtick-fenced opener's info string may not itself contain a backtick (the spec's own reasoning: otherwise inline code spans could be misread as a new fence). `_fence_step()` accepted an opener like
```md`regardless, so a heading right after it was hidden from the scan. Verified independently against the actual code before fixing. A tilde fence has no such restriction and is unaffected.Also split the two over-25-word docstring sentences flagged in the same review round.
Verification
python3 spec/audit.py --selftest->SELFTEST PASS, including 3 new regression cases and the full pre-existing suite unchangedpython3 spec/validate.py->Spec validation OKruff check ./ruff format --check .-> cleanpython3 scripts/prose_lint.py --diff origin/develop spec/audit.py-> cleanA note on scope
This is the fourth follow-up PR (#901 -> #902 -> #903 -> this one) chasing progressively deeper CommonMark fence-parsing edge cases that CodeRabbit's automated review keeps finding one round at a time against
_fence_step(). Each one has been real and independently verified, but I want to flag the pattern rather than silently keep going: CommonMark has more edge cases than these four (unterminated fences at EOF, tab expansion in indentation, and others), and a sufficiently persistent automated reviewer may keep surfacing them. Worth a decision on where "correct enough" is for a fleet-internal audit tool versus a full CommonMark implementation.