Skip to content

Skip fenced code blocks in the link check, and report unclosed fences (#202) - #207

Merged
realmarcin merged 1 commit into
mainfrom
fix/pr-sanity-code-fences
Aug 2, 2026
Merged

Skip fenced code blocks in the link check, and report unclosed fences (#202)#207
realmarcin merged 1 commit into
mainfrom
fix/pr-sanity-code-fences

Conversation

@realmarcin

Copy link
Copy Markdown
Contributor

Closes #202.

The bug

check_markdown_links scanned line by line with no notion of fenced code, so a link written as an example inside a fence was treated as a real link:

```markdown
see [the docs](docs/example.md)
```

Nothing is affected today — the repo reports 0 findings. But pr-sanity now runs unfiltered on every PR (#201), so the first person to document a link pattern would have had an unrelated PR blocked by a link that was never broken.

Fence handling

Follows the CommonMark rules that actually matter here:

  • an opening fence is 3+ backticks or tildes, indented at most 3 spaces
  • a closing fence is the same character, at least as long, with no info string
  • therefore a ````-fence can contain a ```-fence — which is how one documents a fenced block at all, including in pr-sanity's link check does not skip fenced code blocks #202 itself
  • a tilde fence does not close a backtick fence

Inline code spans are blanked as well, since `[x](y.md)` is prose about a link rather than a link.

The real risk here was over-correcting

Skipping too much would silently stop verifying real links. That is worse than the false positive being removed, and it is exactly the failure this script exists to catch — a check that reports "all clear" because it stopped looking.

Two guards:

Measured coverage before and after against the live corpus, with an explicit diff of which links changed status:

in-repo links verified
before 26
after 26
no longer checked 0

Unterminated fences are now their own finding. Without that, a single stray opening fence would make every later line in the file invisible to all checks while the run still printed "all clear". Now it says so and fails.

Testing

8 new tests: nested ````/``` fences, tilde fences, fences indented under list items, backtick-vs-tilde non-closing, inline code spans, the unterminated case, and a broken link inside a fence correctly not firing.

25 tests for this script, 177 total, just qc green. Runtime still 0.27s — it has to stay cheap enough that nobody is ever tempted to put it behind a paths: filter, which is the failure it exists to prevent.

🤖 Generated with Claude Code

…#202)

check_markdown_links scanned line by line with no notion of fenced code, so a
link written as an *example* inside a fence was treated as a real link. Nothing
was affected today, but pr-sanity now runs unfiltered on every PR, so the first
person to document a link pattern would have had an unrelated PR blocked.

Fence handling follows the CommonMark rules that matter here:

- an opening fence is 3+ backticks or tildes, indented at most 3 spaces
- a closing fence is the SAME character, AT LEAST as long, with no info string
- so a ````-fence can contain a ```-fence, which is how one documents a fenced
  block at all — including in #202 itself
- a tilde fence does not close a backtick fence

Inline code spans are blanked too: `[x](y.md)` is prose about a link.

The risk in this change was the opposite of the bug: skipping too much would
silently stop verifying real links, which is worse than a false positive and is
the failure this script exists to prevent. Two guards against that.

First, measured coverage before and after against the live corpus — 26 in-repo
links verified both times, with an explicit diff of which links changed status
(none). A drop would have been visible rather than inferred.

Second, an unterminated fence is now its own finding. Without it, one stray
opening fence would make every later line in that file invisible to all checks
while the run still reported "all clear".

8 new tests covering nested fences, tilde fences, indented fences, inline code,
the unterminated case, and a broken link inside a fence not firing. 25 tests for
this script, 177 total. Still 0.27s.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@realmarcin

Copy link
Copy Markdown
Contributor Author

Review

Adversarial pass over my own diff. Nothing to fix; one limitation filed.

The over-correction risk was the real one, and it is measured

The bug being fixed is a false positive. The dangerous fix is one that skips too much and silently stops verifying real links — worse than the false positive, and the exact failure this script exists to catch.

So the load-bearing evidence here is not "tests pass", it is the before/after coverage diff against the live corpus: 26 in-repo links verified before, 26 after, 0 changed status. If fence handling had swallowed real links, that number would have dropped and the diff would have named them.

Probed the parser rather than trusting it

input behaviour correct?
`` (two backticks) not a fence, line still scanned yes — needs 3+
```js as a closing attempt does not close; info strings are opening-only yes
~~~ inside a ``` block does not close it yes
```` wrapping ``` inner does not close outer yes — this is how #202 itself is written
fence indented under a list item opens a block yes — up to 3 spaces
unmatched single ` left alone, link still checked yes — conservative, does not over-strip
two adjacent inline spans both blanked yes

Filed, not fixed — #208

4-space indented code blocks are still scanned. CommonMark has two code-block forms; this PR handles fences, not indentation. Zero live instances, so no impact today.

I stopped short deliberately. Indented code is ambiguous with list continuation content — four spaces under a list item is indentation, not code — so a naive indent rule would silently skip real links inside lists. That is precisely the over-correction this PR's coverage measurement exists to prevent, and it would have dropped the 26. Doing it properly needs block-level parsing that tracks list context, or a real Markdown parser.

A parser (markdown-it-py) would subsume both #202 and #208 and be correct by construction. The cost is a dependency in a script whose whole selling point is being stdlib-only and 0.27s, so nobody is ever tempted to put it behind a paths: filter. If this class of bug shows up a third time, that trade flips — noted in the issue.

State

All four checks pass (pr-sanity 19s, qc, pytest, vendored-sync). 25 tests for this script, 177 total. Runtime 0.27s, unchanged.

Not merging — your call.

Copilot AI 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.

Pull request overview

This PR updates scripts/pr_sanity.py’s Markdown link checker to understand fenced code blocks and inline code spans, preventing false-positive broken-link findings from links shown as examples. It also adds a new finding type for unterminated fences to avoid silently skipping the remainder of a file.

Changes:

  • Add fence-aware line filtering (prose_lines) so fenced code blocks are skipped during link checks.
  • Blank inline code spans before link extraction so prose about links isn’t treated as links.
  • Report UNTERMINATED_FENCE when a fence is opened but never closed, and add targeted tests for these behaviors.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
scripts/pr_sanity.py Introduces fenced-code/inline-code handling for link scanning and adds UNTERMINATED_FENCE reporting.
tests/test_pr_sanity.py Adds tests for nested/tilde/indented fences, inline code spans, and unterminated fence reporting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_pr_sanity.py
Comment on lines +251 to +253
def test_inline_code_span_is_not_a_link():
kept = _links_kept("use `[x](y.md)` here [real](z.md)\n")
assert kept == ["use here [real](z.md)"]
Comment thread scripts/pr_sanity.py
fence_len = len(m.group("fence"))
opened_at = lineno
continue
out.append((lineno, INLINE_CODE_RE.sub("", line)))
@realmarcin
realmarcin merged commit d7746d0 into main Aug 2, 2026
5 checks passed
@realmarcin
realmarcin deleted the fix/pr-sanity-code-fences branch August 2, 2026 07:09
realmarcin added a commit that referenced this pull request Aug 3, 2026
Two of the file's numbers did not reproduce, and a whole work thread was
missing.

- Header: no open PRs; merged through #210. Replaced the prose issue list
  with a table of all 11 open issues; recorded #184/#199/#200/#202/#204 as
  closed.
- Section 4: the "predicates 85% / nodes 62%" figures do not reproduce under
  any metric. The repo's own dry-run scripts give 63% and 35%; point at them
  as ground truth instead of restating a number here. The "quality floor"
  claim was also too generous — `positively regulates` (37 edges) and
  `negatively regulates` (16) are exact RO labels sitting ungrounded while
  their paraphrases `promotes`/`inhibits` are already mapped.
- Section 5: "1 of 220 traits done" was wrong arithmetic. 220 is what is
  still fragmented, re-measured today, across 220 of 477 files.
- Section 7 (new): the CI/agent-workflow thread — #194, #196, #201, #206,
  #207, #210 — plus the seven small review issues it left open.
- Section 8 (new): the paid Edison sweep's manifest says 353 ok, but
  research/ is gitignored and only 11 reports survive here. Resume detection
  is file-existence based, so a re-run would re-bill 342 completed calls.
- Section 9 (new): the grounding backfill that follows from the section 4
  correction, gated by the existing blocking label-correspondence check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
realmarcin added a commit that referenced this pull request Aug 3, 2026
Two of the file's numbers did not reproduce, and a whole work thread was
missing.

- Header: no open PRs; merged through #210. Replaced the prose issue list
  with a table of all 11 open issues; recorded #184/#199/#200/#202/#204 as
  closed.
- Section 4: the "predicates 85% / nodes 62%" figures do not reproduce under
  any metric. The repo's own dry-run scripts give 63% and 35%; point at them
  as ground truth instead of restating a number here. The "quality floor"
  claim was also too generous — `positively regulates` (37 edges) and
  `negatively regulates` (16) are exact RO labels sitting ungrounded while
  their paraphrases `promotes`/`inhibits` are already mapped.
- Section 5: "1 of 220 traits done" was wrong arithmetic. 220 is what is
  still fragmented, re-measured today, across 220 of 477 files.
- Section 7 (new): the CI/agent-workflow thread — #194, #196, #201, #206,
  #207, #210 — plus the seven small review issues it left open.
- Section 8 (new): the paid Edison sweep's manifest says 353 ok, but
  research/ is gitignored and only 11 reports survive here. Resume detection
  is file-existence based, so a re-run would re-bill 342 completed calls.
- Section 9 (new): the grounding backfill that follows from the section 4
  correction, gated by the existing blocking label-correspondence check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
realmarcin added a commit that referenced this pull request Aug 3, 2026
The backlog had drifted a day and carried numbers that did not reproduce. Docs-only;
the PR touches `NEXT_TASKS.md` and nothing else.

Corrected:
- Section 4's grounding coverage was recorded as "predicates 85% / nodes 62%". The
  repo's own scripts give 63% and 35%; the hardcoded percentages are replaced by the
  commands that produce them.
- Section 5 presented four different measurements — 220 fragmented graphs, 219
  baseline files, 1314 findings, 1264 stranded nodes — as though they were one. Now a
  table with a source per row.
- Section 8 read "353 ok, 8 fail" as 8 unfinished traits. Every `fail:1` row is a
  retry that later succeeded; `fail − ok` is empty, so zero traits are outstanding.
- Section 2 called the cross-Mech vendored-sync sweep pending. It landed:
  MediaIngredientMech#160 and CommunityMech#280/#278 are closed and both spokes carry
  the unfiltered workflow. Only the CultureMech `label-correspondence` half may remain.

Added: section 7 for the CI/agent-workflow thread (#194, #196, #201, #206, #207,
#210, #216), section 8 for the paid Edison sweep whose 353 reports are gitignored and
absent from a fresh clone, section 9 for the grounding backfill.

Filed along the way: #214 (residual reports drift with nothing to catch it), #217
(no workflow-conventions page), #218 (enforce the concurrency rule in pr-sanity),
#220 (`audit-graphs` cannot see a graph splitting into two trait-bearing components —
`morphology/dumbbell_shaped.yaml` is the live instance, and it is why 220 and 219
disagree). #215 was found and fixed the same way, in #216.

The open-issue table was cross-checked against `gh issue list` before merge: 15 rows,
exact match.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
realmarcin added a commit that referenced this pull request Aug 4, 2026
#207 handled fenced blocks; a block indented 4+ spaces was still scanned, so the first person to document a link pattern that way would have had an unrelated PR blocked by a link that was never broken.

Measured before writing it: across all 46 tracked .md files, zero lines indented 4+ contain a markdown link. So this is preventive in both directions, which is also why it can afford to be conservative.

Two conditions keep prose in scope, and both took a review round to get right:

Blank line required. CommonMark forbids an indented block from interrupting a paragraph, so wrapped prose stays scanned.

Indentation measured RELATIVE to the innermost open list item. Inside `- item` content begins at column 2, so code needs 6 spaces; a continuation paragraph indented 4 is prose. Measuring absolutely silently dropped every such paragraph — a coverage loss, which is worse than the false positive being fixed. The test that should have caught it was worthless: "- item\n  more [x](…)" is both under 4 columns and has no preceding blank line, so it passed for two independent reasons and pinned neither.

List tracking runs only on lines already known to be prose. Updating the threshold earlier let a bullet-shaped line inside a code block move it and un-skip the rest of the block, reopening the very false positive this closes.

Thirteen shapes verified, including the ones that cut the other way: 6 spaces under `- item` is code, code after a list closes is still code, a bullet inside a block stays code, and a list body resumes correctly after a code block inside the same item.

Documented limitations rather than left to be rediscovered: tabs are not treated as indentation, and list tracking is a single innermost column rather than a container stack.

Closes #208.

Co-Authored-By: Claude Opus 5 <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.

pr-sanity's link check does not skip fenced code blocks

2 participants