bmad-architecture: address PR review (augment + coderabbit)
- REF-03: use canonical "invoke the `skill` skill" language for all cross-skill references in SKILL.md (was "route to / offer / hand to / Next:") - lint_spine: scan frontmatter for unfilled template tokens / TBD (paradigm, scope, date were uncatchable in the body-only pass) - lint_spine: guard read_text() so a bad-encoding/unreadable spine returns error JSON and exit 0, honoring the documented contract - reviewer-gate: resolve the "always run" vs "may skip" ambiguity — the gate scales/skips by stakes, but finalize_reviewers always run once it does - tests: +3 (frontmatter token, frontmatter TBD, unreadable spine); use next()
This commit is contained in:
Brian Madison
parent
b3ea262589
commit
f675bdaf22
|
|
@ -22,13 +22,13 @@ You're a coach, and the **Coaching path is the default** — this runs against t
|
|||
|
||||
Elicit, don't quiz: open-ended "how are you thinking about X?" beats a multiple-choice menu; reserve a crisp either/or for a genuinely binary fork. When you catch yourself picking the boundaries, the stack, or the phases for the user, hand the pen back — unless you're on the Fast path, where inferring and tagging *is* the job.
|
||||
|
||||
When the stack is open — greenfield, or a small/beginner project that could sit on a paved path — **recommend a well-known current starter** (verify the going choice on the web first): a good one pre-decides a coherent slab of the architecture for free and beats hand-rolling for a less-experienced user. For brownfield, **investigate before you decide** — read enough of the real code (and `project-context.md`; offer `bmad-document-project` if there is none) to ratify the conventions already there rather than invent new ones.
|
||||
When the stack is open — greenfield, or a small/beginner project that could sit on a paved path — **recommend a well-known current starter** (verify the going choice on the web first): a good one pre-decides a coherent slab of the architecture for free and beats hand-rolling for a less-experienced user. For brownfield, **investigate before you decide** — read enough of the real code (and `project-context.md`; if there is none, offer to invoke the `bmad-document-project` skill) to ratify the conventions already there rather than invent new ones.
|
||||
|
||||
## Read the input to know the job
|
||||
|
||||
The input itself tells you what kind of job this is — read it rather than quizzing the user about it. A spec package (`SPEC.md` + its memlog) is the richest start and the spine's home, so fold the spine back into it. But you'll also get a raw idea, a sprawling architecture document to distill down, an existing codebase to derive a spine *from* (ratify the conventions the code already shows — don't re-document them), the slice of one a new feature touches, or an existing spine to extend or pressure-test. Prefer a `.memlog.md` over re-reading the source it came from. Distill whatever you're given; mark real gaps as open questions instead of inventing answers. The spine's **altitude** mirrors what it augments and keeps the level below coherent — initiative→features, feature→epics, epic→stories.
|
||||
|
||||
**Inheriting a parent spine** (e.g. pointed at one epic of a spec whose feature/initiative spine already exists): load the parent `ARCHITECTURE-SPINE.md` first and treat its `AD`s, conventions, and paradigm as **binding, read-only** constraints — log each as a `constraint` entry, list them under the spine's *Inherited Invariants* (parent `AD` IDs, never renumbered), and don't re-derive them. Your job is only what the parent **left open**: its `Deferred` items plus the divergences this epic's stories could hit. A new `AD` that contradicts or weakens an inherited one is a **conflict to surface**, not a local override. An epic spine fixes the invariants the epic's stories must share — it does **not** expand per-story detail; that's deferred to `bmad-create-story` at story time.
|
||||
**Inheriting a parent spine** (e.g. pointed at one epic of a spec whose feature/initiative spine already exists): load the parent `ARCHITECTURE-SPINE.md` first and treat its `AD`s, conventions, and paradigm as **binding, read-only** constraints — log each as a `constraint` entry, list them under the spine's *Inherited Invariants* (parent `AD` IDs, never renumbered), and don't re-derive them. Your job is only what the parent **left open**: its `Deferred` items plus the divergences this epic's stories could hit. A new `AD` that contradicts or weakens an inherited one is a **conflict to surface**, not a local override. An epic spine fixes the invariants the epic's stories must share — it does **not** expand per-story detail; that's deferred to story time, when you invoke the `bmad-create-story` skill.
|
||||
|
||||
## How a run works
|
||||
|
||||
|
|
@ -53,7 +53,7 @@ Writes go through the shared script (don't read the file back except on resume):
|
|||
|
||||
1. Resolve customization: `python3 {project-root}/_bmad/scripts/resolve_customization.py --skill {skill-root} --key workflow` (on failure read `{skill-root}/customize.toml`, use defaults). Run `{workflow.activation_steps_prepend}`, then `{workflow.activation_steps_append}`. Hold `{workflow.persistent_facts}` as standing context — the default loads `project-context.md`, load-bearing for brownfield — and consult `{workflow.external_sources}` on demand.
|
||||
2. Load `{project-root}/_bmad/bmm/config.yaml` (+ `config.user.yaml`) for `{user_name}`, `{communication_language}`, `{document_output_language}`, `{planning_artifacts}`, `{project_name}`, `{date}`; missing keys take neutral defaults, never block.
|
||||
3. Headless (no interactive user) → follow `references/headless.md` for the whole run. Otherwise greet `{user_name}` in `{communication_language}`. Detect the intent from the conversation and input — **create** (the default), **update** an existing spine, or **validate** one (see those sections). If the real ask is requirements / UX / a capability contract / epic breakdown / an agent, route to `bmad-prd` / `bmad-ux` / `bmad-spec` / `bmad-create-epics-and-stories` / `bmad-workflow-builder` (if the BMad Builder module is installed) instead.
|
||||
3. Headless (no interactive user) → follow `references/headless.md` for the whole run. Otherwise greet `{user_name}` in `{communication_language}`. Detect the intent from the conversation and input — **create** (the default), **update** an existing spine, or **validate** one (see those sections). If the real ask is requirements / UX / a capability contract / epic breakdown / an agent, invoke the `bmad-prd`, `bmad-ux`, `bmad-spec`, `bmad-create-epics-and-stories`, or `bmad-workflow-builder` (if the BMad Builder module is installed) skill instead.
|
||||
4. If a run folder for this target already exists under `{workflow.spine_output_path}`, offer to resume from its memlog rather than restart.
|
||||
5. Interactive create: offer the working mode in `{communication_language}` — **Coaching path** (default) or **Fast path** (see *How you work*) — before any drafting; default to Coaching unless the user asks for speed.
|
||||
|
||||
|
|
@ -72,8 +72,8 @@ Walk the sequence; reviewer fixes land before polish.
|
|||
3. **Reviewer pass.** Run the Reviewer Gate (`references/reviewer-gate.md`). Resolve before polish.
|
||||
4. **Triage.** Open questions and `[ASSUMPTION]` tags: blockers (unsafe for what's next) resolved one at a time; the rest deferred with a revisit condition in the memlog.
|
||||
5. **Renderings & polish.** The terse spine is the deliverable when the consumer is a downstream build agent. If the user needs a human-facing artifact instead, offer a fuller rendering (html or md solution design, deck, C4 set) and apply `{workflow.doc_standards}` polish only to that prose, never to the spine.
|
||||
6. **External handoffs.** Run `{workflow.external_handoffs}`; surface returned URLs/IDs. Offer to hand the spine to `bmad-spec` as a companion, keeping `AD` IDs stable so downstream can cite them.
|
||||
7. **Close.** Set the spine's own frontmatter `status: final`, `updated: {date}`; log a `memlog.py append --type event --text "spine finalized"` (the memlog has no status field). Share paths. Next: `bmad-spec`, `bmad-create-epics-and-stories`, or — epic altitude — `bmad-create-story`; `bmad-help` to route.
|
||||
6. **External handoffs.** Run `{workflow.external_handoffs}`; surface returned URLs/IDs. Offer to invoke the `bmad-spec` skill to adopt the spine as a companion, keeping `AD` IDs stable so downstream can cite them.
|
||||
7. **Close.** Set the spine's own frontmatter `status: final`, `updated: {date}`; log a `memlog.py append --type event --text "spine finalized"` (the memlog has no status field). Share paths. Next: invoke the `bmad-spec`, `bmad-create-epics-and-stories`, or — epic altitude — `bmad-create-story` skill; or invoke the `bmad-help` skill to route.
|
||||
8. Run `{workflow.on_complete}`.
|
||||
|
||||
## Update
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ The spine's pre-handoff review. Runs at Finalize (after distill + reconcile) and
|
|||
|
||||
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.
|
||||
|
||||
Assemble the menu: a **rubric walker** that judges the spine against the good-spine checklist below, **+ every entry in `{workflow.finalize_reviewers}` (always run)**, + 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 the set to the stakes: a throwaway prototype may run it quietly or skip; a high-criticality or platform-altitude spine earns more lenses and the explicit all / subset / skip menu.
|
||||
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.)
|
||||
|
||||
Dispatch every entry as a **parallel subagent against `ARCHITECTURE-SPINE.md`** (prefix convention: `skill:` / `file:` / plain text). Each writes its full review to `{doc_workspace}/review-{slug}.md` and returns ONLY a compact summary (verdict, top 2–5 findings, file path) — the parent never holds full review text. An inline self-check does not count: the independent context is the point, because a fresh reviewer finds the divergences the author talks past. If subagents are unavailable, run sequentially — write the file first, then flush it from context.
|
||||
|
||||
|
|
|
|||
|
|
@ -88,6 +88,24 @@ def find_placeholders(body: str, offset: int) -> list[dict]:
|
|||
return findings
|
||||
|
||||
|
||||
def find_frontmatter_placeholders(frontmatter: str) -> list[dict]:
|
||||
"""Catch unfilled tokens left in frontmatter (e.g. paradigm/scope/date) — part of the
|
||||
spine contract, but outside the body that find_placeholders scans."""
|
||||
findings: list[dict] = []
|
||||
for rx, label, severity in (
|
||||
(PLACEHOLDER_WORD, "placeholder marker", "high"),
|
||||
(TEMPLATE_TOKEN, "possible unfilled template token (verify)", "low"),
|
||||
):
|
||||
for m in rx.finditer(frontmatter):
|
||||
findings.append({
|
||||
"category": "placeholder",
|
||||
"severity": severity,
|
||||
"detail": f"frontmatter {label}: {m.group(0)!r}",
|
||||
"location": f"{SPINE} frontmatter (line {1 + line_of(frontmatter, m.start())})",
|
||||
})
|
||||
return findings
|
||||
|
||||
|
||||
def find_ad_issues(body: str, offset: int) -> list[dict]:
|
||||
findings: list[dict] = []
|
||||
scan = blank_fences(body) # AD headings shown inside a code fence are not live ADs
|
||||
|
|
@ -196,6 +214,7 @@ def _check_dep(item: str, findings: list[dict]) -> None:
|
|||
def lint(text: str) -> dict:
|
||||
frontmatter, body, offset = split_frontmatter(text)
|
||||
findings: list[dict] = []
|
||||
findings += find_frontmatter_placeholders(frontmatter)
|
||||
findings += find_placeholders(body, offset)
|
||||
findings += find_ad_issues(body, offset)
|
||||
findings += find_unpinned_deps(frontmatter)
|
||||
|
|
@ -221,7 +240,13 @@ def main(argv: list[str] | None = None) -> int:
|
|||
if not spine_path.exists():
|
||||
result = {"ok": False, "error": f"{spine_path} not found", "findings": [], "total_findings": 0}
|
||||
else:
|
||||
result = lint(spine_path.read_text(encoding="utf-8"))
|
||||
try:
|
||||
text = spine_path.read_text(encoding="utf-8")
|
||||
except (OSError, UnicodeDecodeError) as e:
|
||||
# honor the "exit code is always 0" contract: a read/decode failure travels in JSON
|
||||
result = {"ok": False, "error": f"could not read {spine_path}: {e}", "findings": [], "total_findings": 0}
|
||||
else:
|
||||
result = lint(text)
|
||||
|
||||
out = json.dumps(result, indent=2)
|
||||
if args.output:
|
||||
|
|
|
|||
|
|
@ -190,7 +190,7 @@ def test_placeholder_line_number_is_absolute():
|
|||
"TBD here\n"
|
||||
)
|
||||
result = lint_spine.lint(text)
|
||||
ph = [f for f in result["findings"] if "TBD" in f["detail"]][0]
|
||||
ph = next(f for f in result["findings"] if "TBD" in f["detail"])
|
||||
n = int(re.search(r"line (\d+)", ph["location"]).group(1))
|
||||
assert n == 13
|
||||
|
||||
|
|
@ -201,5 +201,28 @@ def test_missing_spine_file_reports_error(tmp_path, capsys):
|
|||
assert rc == 0 and out["ok"] is False and "not found" in out["error"]
|
||||
|
||||
|
||||
def test_frontmatter_unfilled_token_caught():
|
||||
# an unfilled {scope}/{paradigm}/{date} in frontmatter is part of the contract and must lint
|
||||
text = "---\nname: 'x'\nscope: '{what this spine governs}'\n---\n\n## Invariants\n"
|
||||
result = lint_spine.lint(text)
|
||||
fm = [f for f in result["findings"] if f["category"] == "placeholder" and "frontmatter" in f["detail"]]
|
||||
assert fm and any("template token" in f["detail"] for f in fm)
|
||||
|
||||
|
||||
def test_frontmatter_tbd_caught():
|
||||
text = "---\nname: 'x'\nstatus: TBD\n---\n\n## Invariants\n"
|
||||
result = lint_spine.lint(text)
|
||||
assert any(f["category"] == "placeholder" and "frontmatter" in f["detail"] and "TBD" in f["detail"]
|
||||
for f in result["findings"])
|
||||
|
||||
|
||||
def test_unreadable_spine_returns_error_not_crash(tmp_path, capsys):
|
||||
# a spine that exists but can't be UTF-8 decoded must yield error JSON + exit 0, not a traceback
|
||||
(tmp_path / lint_spine.SPINE).write_bytes(b"\xff\xfe bad bytes not utf-8")
|
||||
rc = lint_spine.main(["--workspace", str(tmp_path)])
|
||||
out = json.loads(capsys.readouterr().out)
|
||||
assert rc == 0 and out["ok"] is False and "could not read" in out["error"]
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(pytest.main([__file__, "-q"]))
|
||||
|
|
|
|||
Loading…
Reference in New Issue