From 9026a61203f53575531232bc1d8ef1b6fb3147ae Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 21 Aug 2026 19:26:27 -0700 Subject: [PATCH 1/3] Fix fence-matching, dedupe scan lists, tighten prose Review findings from PR #900 (qodo-code-review, CodeRabbit): - unfenced_text() toggled on any fence marker regardless of family or length, so a ~~~ line nested inside a ``` block, or a shorter ``` inside a longer ````, closed the wrong fence. Track the opening marker and require a same-family closing fence at least as long, per CommonMark. Added 5 regression cases. - TEMPLATE_REF_SCANNED and UNDECLARED_HEADING_SCANNED were two identical tuples that could silently drift apart; the second is now an alias of the first. - Split two over-length sentences in section-model.md and rewrote one for present tense (was 'has accumulated undetected before'). - Split three over-25-word docstring/comment sentences in audit.py. --- spec/audit.py | 66 ++++++++++++++++++++++++++++++++++--------- spec/section-model.md | 2 +- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/spec/audit.py b/spec/audit.py index e477b99b..a9e60511 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -419,8 +419,8 @@ def extract_section(text, heading): # Carried files scanned for a coordination reference (GOVERNANCE.md "Documentation Style Conventions"). TEMPLATE_REF_SCANNED = ("AGENTS.md", "GOVERNANCE.md", ".github/copilot-instructions.md") -# Carried files scanned for an undeclared H2 heading (spec/section-model.md). -UNDECLARED_HEADING_SCANNED = ("AGENTS.md", "GOVERNANCE.md", ".github/copilot-instructions.md") +# The undeclared-H2 scan reads the same set. +UNDECLARED_HEADING_SCANNED = TEMPLATE_REF_SCANNED def strip_sections(text, names): @@ -450,12 +450,12 @@ def strip_sections(text, names): def undeclared_h2_headings(text, declared): """Level-two headings in `text` that `declared` does not name, sorted. - `declared` is normalized here (stripped, lowercased) rather than trusted pre-normalized, so the - contract holds for any caller regardless of how its own section names are cased or spaced. - Scoped to `## ` only: the section model's unit is the H2, and an H1 title or a nested H3 is not itself a - section this check judges. Fence-aware via unfenced_text, so a `## ` line inside a fenced code sample - (documenting the heading syntax itself) or a `##`-prefixed shell comment is not misread as a real - heading. Per unfenced_text's own docstring, a checker left fence-blind is a document read two ways. + `declared` is normalized here (stripped, lowercased), not trusted pre-normalized. The contract + then holds for any caller, regardless of how its own names are cased or spaced. + Scoped to `## ` only, the section model's unit. An H1 title or a nested H3 is not itself a + section this check judges. + Fence-aware via unfenced_text. A `## ` line inside a fenced code sample, or a `##`-prefixed + shell comment, is not misread as a real heading. """ h2s = {ln[3:].strip().lower() for ln in unfenced_text(text).split("\n") if ln.startswith("## ")} return sorted(h2s - {d.strip().lower() for d in declared}) @@ -553,14 +553,22 @@ def unfenced_text(text): `` or an `![alt][ref]` in one is not a definition, a group, or a rendered badge. Kept as one helper because the checkers were fence-aware in some places and blind in others, which is the state that lets a document be read two ways by one audit. + A fence closes only on the same marker character at least as long as the one that opened it, per + CommonMark, so a mismatched or shorter marker nested inside (a ~~~ example inside a ``` block, or a + ``` inside a longer ````) is content, not a boundary. """ - out, fenced = [], False + out, marker, marker_len = [], None, 0 for ln in normalize(text).split("\n"): s = ln.strip() - if s.startswith(("```", "~~~")): - fenced = not fenced + run = len(s) - len(s.lstrip(s[0])) if s[:1] in ("`", "~") else 0 + if marker is None: + if run >= 3: + marker, marker_len = s[0], run + continue + elif s[:1] == marker and run >= marker_len: + marker = None continue - if not fenced: + if marker is None: out.append(ln) return "\n".join(out) @@ -2879,7 +2887,8 @@ def _selftest(): f" ok template-ref: {len(tref)} cases, verbatim regions excised before the hub-name scan" ) - # Undeclared-heading advisory: an H2 the manifest does not declare, scoped to AGENTS.md, GOVERNANCE.md, and .github/copilot-instructions.md, fence-aware so a documented heading syntax or a shell comment inside a code sample is not misread as a real section. + # An H2 the manifest does not declare, in AGENTS.md, GOVERNANCE.md, or .github/copilot-instructions.md. + # Fence-aware, so a heading syntax example or a shell comment inside a code sample is not misread as a real section. uh = [ ( "a declared H2 is not flagged", @@ -2937,6 +2946,37 @@ def _selftest(): if uh_ok: print(f" ok undeclared-heading: {len(uh)} cases, H2-only, case-insensitive, fence-aware") + # unfenced_text: a fence closes only on a same-family marker at least as long as the opener. + uf = [ + ("a simple ``` fence excludes its content", "```\n## Phantom\n```\n## Real\n", "## Real\n"), + ("a simple ~~~ fence excludes its content", "~~~\n## Phantom\n~~~\n## Real\n", "## Real\n"), + ( + "a ~~~ line nested inside a ``` fence does not close it", + "```md\n~~~\n## Phantom\n```\n## Real\n", + "## Real\n", + ), + ( + "a shorter ``` cannot close a longer ```` fence", + "````md\n## Phantom\n```\n## Real\n````\n", + "", + ), + ( + "a longer closing fence than the opener still closes", + "```\n## Phantom\n````\n## Real\n", + "## Real\n", + ), + ] + uf_ok = True + for label, doc, want in uf: + got = unfenced_text(doc) + if got != want: + ok = uf_ok = False + print(f" FAIL unfenced-text: {label} (expected {want!r}, got {got!r})") + if uf_ok: + print( + f" ok unfenced-text: {len(uf)} cases, closing fence matches the opener's marker and length" + ) + # Issue generator: findings land in the right buckets and the title carries the count. fe = {"name": "Widget", "types": ["python"]} it, ib = render_issue( diff --git a/spec/section-model.md b/spec/section-model.md index cebcb7a7..8a03ac4d 100644 --- a/spec/section-model.md +++ b/spec/section-model.md @@ -91,7 +91,7 @@ A repo that carried its governance inside `AGENTS.md` before the router split ho `files.json` declares each section's fidelity. [validate.py][validate] proves every declared section resolves to a real level-two heading in the hub's own copy of the file that declares it, so a renamed or mistyped section cannot silently stop being checked. [audit.py][audit] checks each repo's copy (presence for `intent`, byte-match for `verbatim`) and classifies a mismatch as stale (re-vendor) or modified (review). -The undeclared-heading advisory above also runs against `.github/copilot-instructions.md`, not only `AGENTS.md` and `GOVERNANCE.md`, since that file has its own declared sections in `files.json` and is where repo-specific content has accumulated undetected before. It names the heading as undeclared and points at this doc's destinations, and it does not attempt to name which destination a given heading belongs in, since neither `OPERATIONS.md`'s six headings nor `ARCHITECTURE.md`'s are declared anywhere as data, and matching by heading name would miss content filed under a differently worded heading regardless. +The undeclared-heading advisory also runs against `.github/copilot-instructions.md`, not only `AGENTS.md` and `GOVERNANCE.md`. That file carries its own declared sections in `files.json`, and repo-specific content can accumulate there unseen. The advisory names the heading as undeclared and points at this doc's destinations. It does not name which destination a heading belongs in. Neither `OPERATIONS.md`'s six headings nor `ARCHITECTURE.md`'s are declared anywhere as data, and matching by heading name misses content filed under a differently worded heading. From acf1be9e9279642865c21c64190ffa4f7971cd18 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 21 Aug 2026 19:35:39 -0700 Subject: [PATCH 2/3] Require blank suffix on a closing fence qodo-code-review: unfenced_text() closed a fence on any line whose marker run matched character and length, even with trailing text after it (```not-a-fence). Per CommonMark a closing fence has nothing but whitespace after the marker run, so that line is content of a still-open block, not a boundary. Verified the bug against the actual code before fixing. Added 2 regression cases. --- spec/audit.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/spec/audit.py b/spec/audit.py index a9e60511..e94a7c5d 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -553,9 +553,11 @@ def unfenced_text(text): `` or an `![alt][ref]` in one is not a definition, a group, or a rendered badge. Kept as one helper because the checkers were fence-aware in some places and blind in others, which is the state that lets a document be read two ways by one audit. - A fence closes only on the same marker character at least as long as the one that opened it, per - CommonMark, so a mismatched or shorter marker nested inside (a ~~~ example inside a ``` block, or a - ``` inside a longer ````) is content, not a boundary. + A fence closes only on the same marker character at least as long as the one that opened it, with + nothing but whitespace after the marker run, per CommonMark. A mismatched or shorter marker nested + inside (a ~~~ example inside a ``` block, or a ``` inside a longer ````) is content, not a boundary, + and so is a marker run followed by trailing text (an opening fence's language tag has no closing + counterpart). """ out, marker, marker_len = [], None, 0 for ln in normalize(text).split("\n"): @@ -565,7 +567,7 @@ def unfenced_text(text): if run >= 3: marker, marker_len = s[0], run continue - elif s[:1] == marker and run >= marker_len: + elif s[:1] == marker and run >= marker_len and not s[run:].strip(): marker = None continue if marker is None: @@ -2946,7 +2948,7 @@ def _selftest(): if uh_ok: print(f" ok undeclared-heading: {len(uh)} cases, H2-only, case-insensitive, fence-aware") - # unfenced_text: a fence closes only on a same-family marker at least as long as the opener. + # A fence closes only on a same-family marker at least as long as the opener. uf = [ ("a simple ``` fence excludes its content", "```\n## Phantom\n```\n## Real\n", "## Real\n"), ("a simple ~~~ fence excludes its content", "~~~\n## Phantom\n~~~\n## Real\n", "## Real\n"), @@ -2965,6 +2967,16 @@ def _selftest(): "```\n## Phantom\n````\n## Real\n", "## Real\n", ), + ( + "a marker run followed by trailing text does not close", + "```\n## Phantom\n```not-a-fence\n## Real\n```\n## Real2\n", + "## Real2\n", + ), + ( + "trailing whitespace after the marker run still closes", + "```\n## Phantom\n``` \n## Real\n", + "## Real\n", + ), ] uf_ok = True for label, doc, want in uf: From ea75007a99e13fa5ac87c18d2a2c9df5b7c2a5c7 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 21 Aug 2026 19:41:57 -0700 Subject: [PATCH 3/3] Honor CommonMark's 3-space fence indentation limit CodeRabbit: ln.strip() removed all leading whitespace before checking the marker, so a fence indented 4+ spaces (past CommonMark's 3-space limit) still opened or closed a block. A 4-space-indented '```' meant as ordinary indented content inside a fence could prematurely close it, exposing what followed as unfenced. Verified the bug against the actual code before fixing. Track leading-space count separately from the marker run: a fence line needs at most 3 leading spaces to count as a boundary at all, open or close. 3 regression cases added. Also split the fence-rule docstring sentence CodeRabbit flagged as over 25 words. --- spec/audit.py | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/spec/audit.py b/spec/audit.py index e94a7c5d..e144050f 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -553,21 +553,25 @@ def unfenced_text(text): `` or an `![alt][ref]` in one is not a definition, a group, or a rendered badge. Kept as one helper because the checkers were fence-aware in some places and blind in others, which is the state that lets a document be read two ways by one audit. - A fence closes only on the same marker character at least as long as the one that opened it, with - nothing but whitespace after the marker run, per CommonMark. A mismatched or shorter marker nested - inside (a ~~~ example inside a ``` block, or a ``` inside a longer ````) is content, not a boundary, - and so is a marker run followed by trailing text (an opening fence's language tag has no closing - counterpart). + Fence matching follows CommonMark. A fence marker needs at most 3 leading spaces, more reads as + content, not a boundary. A closing fence needs the same character, a run at least as long as the + opener's, and nothing but whitespace after that run - a mismatched or shorter marker (a ~~~ example + inside a ``` block, a ``` inside a longer ````) or trailing text (an opening fence's language tag has + no closing counterpart) does not close it. """ out, marker, marker_len = [], None, 0 for ln in normalize(text).split("\n"): - s = ln.strip() - run = len(s) - len(s.lstrip(s[0])) if s[:1] in ("`", "~") else 0 + stripped = ln.lstrip(" ") + indent = len(ln) - len(stripped) + char = stripped[:1] + run = ( + len(stripped) - len(stripped.lstrip(char)) if indent <= 3 and char in ("`", "~") else 0 + ) if marker is None: if run >= 3: - marker, marker_len = s[0], run + marker, marker_len = char, run continue - elif s[:1] == marker and run >= marker_len and not s[run:].strip(): + elif char == marker and run >= marker_len and not stripped[run:].strip(): marker = None continue if marker is None: @@ -2977,6 +2981,21 @@ def _selftest(): "```\n## Phantom\n``` \n## Real\n", "## Real\n", ), + ( + "a 3-space-indented closing fence still closes", + "```\n## Phantom\n ```\n## Real\n", + "## Real\n", + ), + ( + "a 4-space-indented closing fence remains content, not a boundary", + "```\n## Phantom\n ```\n## Still Phantom\n```\n## Real\n", + "## Real\n", + ), + ( + "a 4-space-indented opener never opens a fence", + " ```\n## Real\n", + " ```\n## Real\n", + ), ] uf_ok = True for label, doc, want in uf: