From 237087ea03895a45a6b683191429769caf38c2d1 Mon Sep 17 00:00:00 2001 From: Brian Madison Date: Wed, 17 Jun 2026 01:43:34 -0500 Subject: [PATCH] Harden find_unpinned_stack: blank fences, locate columns, looser heading Address PR review (CodeRabbit + Augment): find_unpinned_stack scanned raw body, so pipe-rows or ## headings inside a fenced block could be misread as live Stack content and misreport version_pin. Now blanks fences first, like find_placeholders and find_ad_issues, honoring the linter's fences-are-non-live contract. Also locate both Name and Version columns from the header (a reordered table now pairs name to version correctly) and match the heading on a word boundary (## Stack & Versions still counts). Add regression tests for fenced rows, fenced headings, renamed heading, and reordered columns (28 passing). Reword stale 'unpinned deps' / 'unpinned dependency versions' to 'unpinned Stack versions' to match the body-table model. --- .../references/reviewer-gate.md | 2 +- .../bmad-architecture/scripts/lint_spine.py | 23 ++++++++---- .../scripts/tests/test_lint_spine.py | 36 ++++++++++++++++++- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/src/bmm-skills/3-solutioning/bmad-architecture/references/reviewer-gate.md b/src/bmm-skills/3-solutioning/bmad-architecture/references/reviewer-gate.md index 8c3f480c1..175676413 100644 --- a/src/bmm-skills/3-solutioning/bmad-architecture/references/reviewer-gate.md +++ b/src/bmm-skills/3-solutioning/bmad-architecture/references/reviewer-gate.md @@ -2,7 +2,7 @@ The spine's pre-handoff review. Runs at Finalize (after distill + reconcile) and *is* the Validate intent. The difference is the ending: at Finalize you apply the clear fixes yourself; under Validate you report and don't change the spine. -Cheap deterministic pass first: `python3 {skill-root}/scripts/lint_spine.py --workspace {doc_workspace}` settles the mechanical misses (placeholders, duplicate `AD` IDs, missing Binds/Prevents/Rule, unpinned deps), so reviewers spend judgment on the semantic half. +Cheap deterministic pass first: `python3 {skill-root}/scripts/lint_spine.py --workspace {doc_workspace}` settles the mechanical misses (placeholders, duplicate `AD` IDs, missing Binds/Prevents/Rule, unpinned Stack versions), so reviewers spend judgment on the semantic half. Assemble the menu: a **rubric walker** that judges the spine against the good-spine checklist below, **+ every entry in `{workflow.finalize_reviewers}`**, + ad-hoc lenses you invent or offer as the spine's rigor, altitude, and criticality warrant — a security/compliance lens for regulated stakes, a seam reviewer cross-team, a data-integrity lens for a heavy data model. Scale *whether and how heavily the gate runs* to the stakes: a throwaway prototype may run it quietly or skip the gate entirely; a high-criticality or platform-altitude spine earns more lenses and the explicit all / subset / skip menu. But once the gate runs, the `{workflow.finalize_reviewers}` always run — they are the configured floor, never cherry-picked out; only the ad-hoc lenses are optional. (Headless never skips the gate.) diff --git a/src/bmm-skills/3-solutioning/bmad-architecture/scripts/lint_spine.py b/src/bmm-skills/3-solutioning/bmad-architecture/scripts/lint_spine.py index 0836a90a3..d583a528d 100644 --- a/src/bmm-skills/3-solutioning/bmad-architecture/scripts/lint_spine.py +++ b/src/bmm-skills/3-solutioning/bmad-architecture/scripts/lint_spine.py @@ -153,16 +153,23 @@ def find_ad_issues(body: str, offset: int) -> list[dict]: def find_unpinned_stack(body: str, offset: int) -> list[dict]: """Flag a `## Stack` table row that names something but leaves its version blank or a placeholder. Pinning lives in the body table now, not frontmatter. A row whose name is - still a `{token}` skeleton is left to the placeholder pass, not double-reported here.""" + still a `{token}` skeleton is left to the placeholder pass, not double-reported here. + + Fences are blanked first (like find_placeholders / find_ad_issues), so a pipe-row or + heading inside a code block is never read as live Stack content. The heading match is + `## Stack` with a word boundary, so a renamed heading (`## Stack & Versions`) still + counts. Name and Version columns are located from the header row, so a reordered table + pairs name to version correctly; both default to the canonical positions (0, 1).""" findings: list[dict] = [] in_stack = False header_seen = False - ver_idx = 1 - for i, raw in enumerate(body.splitlines()): + name_idx, ver_idx = 0, 1 + scan = blank_fences(body) + for i, raw in enumerate(scan.splitlines()): if HEADING.match(raw): - in_stack = re.match(r"^##\s+Stack\s*$", raw) is not None + in_stack = re.match(r"^##\s+Stack\b", raw) is not None header_seen = False - ver_idx = 1 + name_idx, ver_idx = 0, 1 continue if not in_stack or not raw.lstrip().startswith("|"): continue @@ -172,10 +179,12 @@ def find_unpinned_stack(body: str, offset: int) -> list[dict]: if not header_seen: header_seen = True for j, c in enumerate(cells): - if c.lower() == "version": + if c.lower() == "name": + name_idx = j + elif c.lower() == "version": ver_idx = j continue - name = cells[0] if cells else "" + name = cells[name_idx] if len(cells) > name_idx else "" version = cells[ver_idx] if len(cells) > ver_idx else "" if not name or TEMPLATE_TOKEN.search(name): continue diff --git a/src/bmm-skills/3-solutioning/bmad-architecture/scripts/tests/test_lint_spine.py b/src/bmm-skills/3-solutioning/bmad-architecture/scripts/tests/test_lint_spine.py index 9f69ffc19..55cf7482d 100644 --- a/src/bmm-skills/3-solutioning/bmad-architecture/scripts/tests/test_lint_spine.py +++ b/src/bmm-skills/3-solutioning/bmad-architecture/scripts/tests/test_lint_spine.py @@ -6,7 +6,7 @@ The spine under test: a clean spine lints empty; the linter catches exactly the mechanical defects a prompt is unreliable at — literal placeholders, AD-n id breakage, -AD-n blocks missing required fields, and unpinned dependency versions. +AD-n blocks missing required fields, and unpinned Stack versions. """ import importlib.util import json @@ -189,6 +189,40 @@ def test_stack_table_all_pinned_ok(): assert "version_pin" not in cats(result) +def test_fenced_stack_rows_not_parsed(): + # an illustrative fenced table under ## Stack must not be read as live rows (fences are + # blanked first, like every other pass) — a blank-version row inside a fence is not a finding + text = ("---\nname: 'x'\n---\n\n## Stack\n\n| Name | Version |\n| --- | --- |\n" + "| fastapi | 0.115 |\n\n```text\n| example | |\n```\n") + result = lint_spine.lint(text) + assert "version_pin" not in cats(result) + + +def test_fenced_stack_heading_not_live(): + # a `## Stack` heading shown inside a code fence is not the live Stack section + text = ("---\nname: 'x'\n---\n\n## Docs\n\n```md\n## Stack\n\n| foo | |\n```\n") + result = lint_spine.lint(text) + assert "version_pin" not in cats(result) + + +def test_renamed_stack_heading_still_scanned(): + # the heading match is word-boundary, so a varied `## Stack` heading still counts + text = ("---\nname: 'x'\n---\n\n## Stack & Versions\n\n| Name | Version |\n| --- | --- |\n" + "| redis | |\n") + result = lint_spine.lint(text) + pins = [f for f in result["findings"] if f["category"] == "version_pin"] + assert len(pins) == 1 and "redis" in pins[0]["detail"] + + +def test_reordered_columns_pair_name_to_version(): + # Version-then-Name header: the unpinned row must still be flagged by its real name + text = ("---\nname: 'x'\n---\n\n## Stack\n\n| Version | Name |\n| --- | --- |\n" + "| 0.115 | fastapi |\n| | redis |\n") + result = lint_spine.lint(text) + pins = [f for f in result["findings"] if f["category"] == "version_pin"] + assert len(pins) == 1 and "redis" in pins[0]["detail"] + + def test_placeholder_line_number_is_absolute(): # a TBD after a multi-line fence reports its real file line (fence blanked, not collapsed) text = (