diff --git a/brainstorm/SKILL.md b/brainstorm/SKILL.md index 975d935..b0ecec0 100644 --- a/brainstorm/SKILL.md +++ b/brainstorm/SKILL.md @@ -69,6 +69,12 @@ Before asking any clarifying questions: - If the project has a design ledger configured under `paths.design_ledger`, walk to the relevant contracts for the invariants the new cycle might touch. +- When reading any file under `paths.design_models` (or the + ledger's "model" rows), note its `status` / + `validated-against` frontmatter if present: `status: + aspirational` means its code is a target, not verified fact. + Track which content you may lift from such a source — it is + flagged for lift-time validation (Step 4). - `git log --oneline -20` for the chronological scan. - Identify scope: is this one cycle, or does it need to be decomposed into sub-cycles first? If multi-subsystem, @@ -146,6 +152,16 @@ code to show" is the rationalisation to refuse: an infra cycle that cannot be tied to concrete code is speculative infra. +**Code lifted from an aspirational source is a hypothesis, not +evidence.** If the concrete code here is lifted from a +`design_models` file (or any source marked `status: aspirational`, +per Step 1), it carries no validation by default — the marker +exists precisely because no one has run it through the live tool. +Its `design/` location is not ratification. Treat it as the spec's +most suspect bytes: it must clear the Step-7 parse-every-block +gate before PASS. The marker tells you *which* blocks to distrust +first. + ### Step 5 — Present design in sections Scale each section to its complexity (a few sentences if @@ -226,6 +242,24 @@ Inline checklist (not a subagent dispatch): change (plus the worked user-facing example for a surface cycle)? A load-bearing change described only in prose is a self-review failure to fix, exactly like a placeholder. +6. **Parse-every-block gate.** If the project profile declares + `spec_validation.parsers`, extract every fenced code block + from the spec. For each block whose fence label has a + `parsers` entry: write the block to a temp file with the + entry's `ext`, run the entry's `cmd` with `{file}` + substituted, and require exit 0. A non-zero exit is a + self-review failure — the spec inlines code that does not + parse against the live tool — fix the spec before proceeding; + do not pass the unparsed bytes downstream. Paste the + parse-trace (the Run block and its output) into the + brainstorm chat: a visible trace is what attests the gate + fired, and its absence means the gate was skipped. A block + whose fence label has no `parsers` entry is skipped and the + skip is noted ("no parser for fence label X"); never a silent + pass. A malformed entry (`cmd` without `{file}`, or `ext` / + `cmd` missing) is a profile error to surface, not a skip. If + the profile declares no `spec_validation`, this check is a + documented no-op. Fix issues inline. No need to re-review — fix and commit. @@ -356,6 +390,8 @@ Hand off carries: | "Approaches A, B, C are all bad — proceed with A" | This is exactly the moment to surface "the problem is mis-framed" rather than ratify a known-bad shape into the design ledger. Escalate to the user. | | "Just polishing a wording after PASS, no need to re-dispatch" | The grounding-check report attests to specific bytes. A polish edit changes the bytes; the previous attestation no longer covers them. Re-dispatch is cheap; the alternative is a commit with an attestation that doesn't match the file. | | "The shape is clear from the prose, I don't need to paste the code" | If it's clear, pasting it is free; if pasting it is hard, it wasn't clear. The feature-acceptance criterion is unjudgeable without the worked code; "a user reaches for it" with no shown code is the exact hand-wave the criterion exists to kill. | +| "The block obviously parses — I wrote it carefully" | Careful authoring is not a parse. A spec's code blocks are hypotheses until the live tool accepts them; treating them as truth is the compound-hallucination failure the parse-every-block gate exists to kill. If a parser is configured for the block's fence label, run it and paste the trace. | +| "It's in a model doc / the design ledger, so it's canonical" | A `models` / RFC / proposal doc holds aspirational code — a target, not verified fact; its `design/` location is not ratification. Code lifted from a `status: aspirational` source must clear the Step-7 parse gate like any other block, and is in fact the most suspect bytes in the spec. | ## Red Flags — STOP @@ -367,6 +403,12 @@ Hand off carries: - A load-bearing change described in prose with no before → after code block; a surface cycle with no worked user-facing example +- A spec carrying a code block whose fence label has a + configured `spec_validation` parser, committed without a + parse-trace in the brainstorm chat +- Lifting code verbatim from a `status: aspirational` source (a + `models` / RFC / proposal file) into the spec without flagging + it for the Step-7 parse gate - Editing the spec file after a Step 7.5 PASS without re-dispatching - Jumping straight from spec to `implement` (must go via diff --git a/brainstorm/agents/grounding-check.md b/brainstorm/agents/grounding-check.md index cd38e24..fc65668 100644 --- a/brainstorm/agents/grounding-check.md +++ b/brainstorm/agents/grounding-check.md @@ -1,6 +1,6 @@ --- name: grounding-check -description: Read-only grounding-check reviewer for spec drafts. Dispatched by the brainstorm skill in Step 7.5, between linguistic self-review and user-approval. Reads the draft with fresh context, extracts its load-bearing assumptions about current codebase behaviour, and for each one searches the workspace for a currently-green test that ratifies it. Reports PASS or BLOCK. Does NOT propose fixes, does NOT edit files. +description: Read-only grounding-check reviewer for spec drafts. Dispatched by the brainstorm skill in Step 7.5, between linguistic self-review and user-approval. Reads the draft with fresh context, extracts its load-bearing assumptions about current codebase behaviour, and for each one searches the workspace for a currently-green test that ratifies it; also validates the spec's own fenced code blocks against the project's configured parsers. Reports PASS or BLOCK. Does NOT propose fixes, does NOT edit files. tools: Read, Glob, Grep, Bash --- @@ -135,8 +135,9 @@ mechanism strongly. EXTRACT ASSUMPTIONS FROM THE SPEC, NOT FROM YOUR MEMORY. RATIFICATION REQUIRES A NAMED, CURRENTLY-GREEN TEST. NOT CODE PRESENCE. NOT RECALL. ONE UNRATIFIED LOAD-BEARING ASSUMPTION = BLOCK. NO PARTIAL CREDIT. +ONE SPEC CODE BLOCK THAT FAILS ITS CONFIGURED PARSER = BLOCK. YOU DO NOT EDIT FILES. YOU DO NOT PROPOSE FIXES. -YOU DO NOT RUN THE FULL TEST SUITE. (TEST LIST AND TYPE-CHECK ARE OK.) +YOU DO NOT RUN THE FULL TEST SUITE. (TEST LIST, TYPE-CHECK, AND PER-BLOCK PARSER RUNS ARE OK.) ``` ## The Process @@ -163,12 +164,29 @@ YOU DO NOT RUN THE FULL TEST SUITE. (TEST LIST AND TYPE-CHECK ARE OK.) 4. Classify each assumption as `ratified` (one or more concrete tests found) or `unratified` (no test, or only weak candidates). -5. Compute aggregate status: - - All assumptions ratified → `PASS`. - - One or more unratified → `BLOCK`. +5. **Code-block parse pass.** If the project profile declares + `spec_validation.parsers`, extract every fenced code block + from the spec. For each block whose fence label has a + `parsers` entry: write it to a temp file with the entry's + `ext`, run the entry's `cmd` with `{file}` substituted, and + require exit 0. A non-zero exit marks the block unparseable. + A block whose fence label has no entry is skipped and noted + ("no parser for fence label X"); never a silent pass. If the + profile declares no `spec_validation`, this pass is a + documented no-op. This pass is complementary to the + assumption search: it checks the spec's own bytes against the + live tool, not the codebase's behaviour — and it is + independent of the orchestrator's own Step-7 parse gate, the + fresh-context second line of the same defense. +6. Compute aggregate status: + - All assumptions ratified AND every configured-label block + parsed clean → `PASS`. + - One or more unratified assumptions, OR one or more + unparseable code blocks → `BLOCK`. - Any infra error (cannot read spec, type-check fails, - workspace does not build) → `INFRA_ERROR`. -6. Emit the report in the format below. + workspace does not build, a configured parser command is + missing from PATH) → `INFRA_ERROR`. +7. Emit the report in the format below. The process is bounded by your dispatch context. If extraction yields more than ~20 candidate assumptions, stop @@ -215,6 +233,15 @@ Unratified assumptions: would have> (Repeat per unratified assumption. Omit this block entirely on PASS.) + +Unparseable code blocks: +- Block: + Parser: + Failure: + +(Repeat per unparseable block. Omit this block entirely when none +failed. A "no parser for fence label X" skip is noted here as a +single line, not a failure.) ``` On `INFRA_ERROR`, only the first two lines are required, @@ -235,6 +262,7 @@ the assumption-extraction step yields an empty list, emit | "I extracted too many assumptions, let me trim the report" | Don't trim. If a spec has too many assumptions to check, that is the finding — report it as BLOCK with reason "spec too broad". | | "The orchestrator will override if I block, so I'll lean toward PASS" | The override is the orchestrator's job, not yours. Your job is to be the fresh-context check. Skewing toward PASS defeats the whole role. | | "I'll run the full test suite to see which tests are actually green" | You may NOT run the full test suite. Use the project's test-list command to enumerate, then read test bodies. Running tests would mutate workspace state and is out of scope for a read-only review. | +| "The code block is just illustrative, not a claim about behaviour" | A spec code block whose fence label has a configured parser is a hypothesis the downstream plan will lift verbatim. Illustrative or not, if it does not parse it is a defect. Run the parser and report the failure. | ## Red Flags — STOP @@ -246,6 +274,8 @@ the assumption-extraction step yields an empty list, emit it - About to mark an assumption ratified based on code presence rather than test presence +- About to PASS a spec carrying a code block whose fence label + has a configured parser without having run that parser - About to skip an assumption because "it would always be true" - Report exceeding ~500 tokens diff --git a/docs/profile-schema.md b/docs/profile-schema.md index d39021e..974d4a9 100644 --- a/docs/profile-schema.md +++ b/docs/profile-schema.md @@ -23,6 +23,37 @@ Omitted optional keys signal that the feature is unused in this project; skills that depend on them either short-circuit or skip the corresponding step. +### Aspirational-source marker (recommendation) + +Files under `design_models` (or an equivalent `RFCs` / +`proposals` directory) commonly carry aspirational code — +constructs written before the surface that would parse them +exists. To let a later brainstorm tell aspirational content from +validated contract, projects are encouraged (not required) to +give each such file a frontmatter marker: + +```yaml +--- +status: aspirational +validated-against: +--- +``` + +`status: aspirational` says "the code here is a target, not a +verified fact"; `validated-against` records the last commit at +which someone actually ran the code through the live tool (or +`"no validation"`). A design ledger (`design_ledger`) is likewise +encouraged to distinguish "model" rows from "contract" rows so a +reading agent can mechanically tell which carry verified +behaviour. + +When a file lacks the marker the signal is simply absent — the +brainstorm degrades to treating its content as unmarked, never +hard-failing. The marker is consumed by the brainstorm skill (see +its Step 1 and Step 4): content lifted from an aspirational source +is flagged and must clear the Step-7 parse-every-block gate before +it ships in a spec. + ## `naming` | Key | Type | Default | Description | @@ -51,6 +82,47 @@ take the next-higher number; deleted files retire their number. | `regression` | list | `[]` | Regression scripts run by the audit skill. Each entry is a shell command; non-zero exit is a regress. | | `architect_sweeps` | list | `[]` | Project-specific architect sweep commands run by the `architect` agent in addition to its universal checks. Each entry is a shell command; non-zero exit means at least one sweep matched and the matches are drift-suspicions to verify. Optional. | +## `spec_validation` + +Optional. A registry mapping each markdown fence label to the tool +that validates a spec code block of that kind. The brainstorm +parse-gate and the grounding-check code-block pass run these +parsers so that spec code blocks are treated as hypotheses to +verify, not as authoritative truth. + +```yaml +spec_validation: + parsers: + ail: + ext: ".ail" + cmd: "ail parse {file}" + ail-json: + ext: ".ail.json" + cmd: "ail check {file}" + ll: + ext: ".ll" + cmd: "llvm-as {file} -o /dev/null" +``` + +The key of each `parsers` entry is the fence info-string of a spec +code block (the token immediately after the opening ` ``` `). Only +blocks whose label has an entry are validated; a block whose label +is absent from the map is skipped and the skip is documented ("no +parser for fence label X") — never a silent pass. + +| Key | Type | Description | +|-------|--------|-------------| +| `ext` | string | Extension (including the leading dot) the harness gives the temp file it writes the block into, so tools that key off extension — `.ail` vs `.ail.json` — see the right one. | +| `cmd` | string | Validation command. MUST contain the `{file}` placeholder, which is substituted with the temp file's path. Exit 0 means a clean parse; any non-zero exit is a parse failure the consuming skill turns into a BLOCK. | + +A malformed entry — `cmd` missing the `{file}` placeholder, or +either `ext` or `cmd` absent — is a profile error the consuming +skill surfaces, not a silent skip; the gate fails closed. + +Omitting the whole `spec_validation` section disables the +block-validation gates: the consuming skills short-circuit, exactly +as with other omitted optional features. + ## `vocabulary` | Key | Type | Default | Description | diff --git a/planner/SKILL.md b/planner/SKILL.md index e3948d2..6ea8590 100644 --- a/planner/SKILL.md +++ b/planner/SKILL.md @@ -221,6 +221,26 @@ Before handing the plan off, run this checklist inline: so "nothing ran" cannot masquerade as "nothing regressed". This is a recurring defect family — scrub every Run step whose assertion lives in a filter string. +9. **Parse-the-bytes-you-inline gate.** If the project profile + declares `spec_validation.parsers`, every verbatim code body + the plan inlines into a task step whose fence label has a + `parsers` entry must be parsed clean against the live tool + before hand-off. For each such block: write it to a temp file + with the entry's `ext`, run the entry's `cmd` with `{file}` + substituted, require exit 0. A non-zero exit is a plan failure + — the plan would hand the implementer bytes that do not parse, + the last defensive line before dispatch — fix the plan, do not + hand off. The parse-trace goes into the planner's self-review + activity (the session) as the attestation the gate fired. The + target is the surface-language snippets the plan lifts verbatim + (example programs, fixtures); the project's source-language + test / implementation bodies are NOT the target — the + `implement` compile gate catches those. A block whose fence + label has no entry is skipped and the skip noted ("no parser + for fence label X"); never a silent pass. A malformed entry + (`cmd` without `{file}`, or `ext` / `cmd` missing) is a profile + error to surface, not a skip. If the profile declares no + `spec_validation`, this check is a documented no-op. Fix issues inline. @@ -252,6 +272,7 @@ The planner skill does not perform the commit itself. | "Step 5 'implement the parser' is fine, I'll detail it at execution time" | Then it's not a step, it's a wish. Steps are bite-sized OR the plan isn't done. | | "Task 7 is similar to Task 4, just say so" | The executor may read tasks out of order. Repeat the code. | | "The spec has a TBD too, I can pass it through" | Bounce back to `brainstorm`. Plans inherit spec gaps; spec gaps are not plan placeholders. | +| "The example program came straight from the spec, it must be valid" | The spec's code blocks are hypotheses, not verified bytes — brainstorm's parse gate can be skipped and a post-spec edit can break them. Re-parse every surface-language body you inline; this is the last line before the implementer hits it (issue #1 Fix 4). | ## Red Flags — STOP @@ -262,6 +283,9 @@ The planner skill does not perform the commit itself. - Step descriptions longer than the code they describe - Header missing parent spec reference - Self-review skipped because "the plan looks fine" +- A task step inlining a surface-language code body whose fence + label has a configured `spec_validation` parser, handed off + without a parse-trace in the planner session ## Cross-references diff --git a/planner/agents/plan-recon.md b/planner/agents/plan-recon.md index 647470b..3a2266f 100644 --- a/planner/agents/plan-recon.md +++ b/planner/agents/plan-recon.md @@ -126,6 +126,22 @@ never to fix or write. missing the second is a known failure mode the project's past audits have flagged. + **Content-pin-perturbing scopes.** A change need not touch a + signature, variant, or symbol to break a test: if + `iteration_scope` adds, removes, or renames anything that + shifts a content-addressed or hashed value a test pins — a + workspace / package listing, a content digest, a generated + manifest, a snapshot — the Non-compile-checked sweep above is + MANDATORY for this scope too, independent of any compile-driven + set. Enumerate EVERY test file under the project's test roots + that grep-matches the load-bearing constant, fixture name, or + assertion shape the change perturbs — not just the first hit. + The recurring failure here is a hash pinned in two test files + where the recon found one and missed the twin; a module + addition that shifts a workspace listing is the canonical case. + Start from the lockstep-invariant pairs in the project's + `CLAUDE.md` — a content-pin is a lockstep invariant. + 6. **Spec-named-path existence table.** Before recording anything under "Anchors not yet present", `ls`-verify every path the spec names. `git ls-files ` or @@ -210,9 +226,12 @@ answer. For iterations where `iteration_scope` does NOT involve a signature change / variant addition or removal / public-symbol -removal, the "Compile-driven site set" and -"Non-compile-checked sites" sections MAY be omitted; the -"Spec-named-path existence table" is always required. +removal AND does not perturb a content-addressed pin (see +"Content-pin-perturbing scopes" above), the "Compile-driven site +set" and "Non-compile-checked sites" sections MAY be omitted; the +"Spec-named-path existence table" is always required. A +content-pin-perturbing scope requires the "Non-compile-checked +sites" section even when no compile-driven set applies. ## Status protocol @@ -235,6 +254,7 @@ removal, the "Compile-driven site set" and | "Hand-listing every match arm gives me the site set — the compiler would say the same" | Multiple documented misses in this project's history say otherwise — each missed a different site type. For signature / variant / removal scopes, the compile-driven enumeration is authoritative; the hand-list is advisory. Run the type-checker after a stub, report the broken site set verbatim. | | "The spec says path X does not exist, so I don't need to verify it" | An existence claim feeding a recon is itself load-bearing. `git ls-files ` or `test -e ` is the authoritative check; an orchestrator-time grep of a sibling directory is not. The existence table makes this unmissable. | | "Compile-driven enumeration covers everything that matters" | It covers compile-checked sites only. Drift pins, fixtures, design-doc anchors, prose references — none of these break the build. Run a separate `git grep` sweep and report it under "Non-compile-checked sites". | +| "The change only adds a module, it touches no signature — no blast radius" | A pure addition can still shift a content-addressed value (a workspace listing, a hash, a snapshot) that tests pin. Enumerate every test that grep-matches the perturbed constant; the canonical miss is a hash pinned in two files where one was found and the twin missed. | ## Red Flags — STOP @@ -253,3 +273,8 @@ removal, the "Compile-driven site set" and - About to return only the compile-driven set without sweeping for non-compile-checked sites (drift pins, fixtures, doc references) +- About to treat a content-pin-perturbing scope (module + add / remove / rename, listing or hash change) as having no + blast radius because it changes no signature +- About to report one content-pin test site without grepping + for its twins under the project's test roots diff --git a/templates/project-profile.yml b/templates/project-profile.yml index 98cb385..736f52f 100644 --- a/templates/project-profile.yml +++ b/templates/project-profile.yml @@ -28,6 +28,15 @@ commands: regression: [] # list of shell commands run by audit; non-zero exit = regress architect_sweeps: [] # optional project-specific architect sweeps; non-zero exit = drift suspicion +# spec_validation: # optional — fence label -> validator for spec code blocks +# parsers: # key = fence info-string; labels with no entry are skipped + documented +# ail: +# ext: ".ail" # extension for the temp file the harness writes the block into +# cmd: "ail parse {file}" # {file} = temp-file path; exit 0 = clean parse, non-zero = BLOCK +# ail-json: +# ext: ".ail.json" +# cmd: "ail check {file}" + vocabulary: cycle: cycle # what a top-level work unit is called subcycle: iteration # what a sub-unit is called