From 2adc2eef158bcd86ccc2ee45c7f89d8f67d0d62a Mon Sep 17 00:00:00 2001 From: Baptiste LAFOURCADE Date: Wed, 1 Jul 2026 12:27:46 +0200 Subject: [PATCH 1/2] refactor(framework): strict anti-slop templates and review redesign Templates now state a qualify-if bar per section and are a closed set: an instance fills or omits the defined sections and never invents, adds, renames, or reorders one (R9). - spec: done-when is observable conditions, not commands or gates. - plan: decisions are architecture-magnitude only, resources are external sources only, phase wireframe is UI-only, the phase->plan redirect is dropped, and the architecture-projection emoji legend is visible. - review: phases as checkboxes, one shared Findings table (Kind column), and a Verification table; drop per-axis sections, prose, and Follow-up. - review verdict: an unchecked criterion tagged fix cannot yield approve; the Unplanned (scope-creep) line is restored. - decouple the checker and 00-sdlc review from 05-review's report file and section names; they name only the skill, which owns its report. Co-Authored-By: Claude Opus 4.8 --- README.md | 4 +- .../2026_07_01-sdlc-anti-slop/phase-1.md | 30 +++++++++ .../2026_07_01-sdlc-anti-slop/phase-2.md | 31 +++++++++ .../2026_07_01-sdlc-anti-slop/phase-3.md | 32 ++++++++++ .../2026_07_01-sdlc-anti-slop/phase-4.md | 43 +++++++++++++ .../2026_07_01-sdlc-anti-slop/phase-5.md | 41 ++++++++++++ .../plans/2026_07_01-sdlc-anti-slop/plan.md | 63 +++++++++++++++++++ .../references/skill-authoring.md | 2 +- plugins/aidd-dev/agents/checker.md | 3 +- plugins/aidd-dev/skills/00-sdlc/SKILL.md | 2 +- .../skills/00-sdlc/actions/04-review.md | 4 +- .../skills/01-plan/actions/04-plan.md | 2 +- .../skills/01-plan/assets/phase-template.md | 18 +++--- .../skills/01-plan/assets/plan-template.md | 18 ++++-- plugins/aidd-dev/skills/05-review/README.md | 23 +++---- plugins/aidd-dev/skills/05-review/SKILL.md | 4 +- .../05-review/actions/01-review-code.md | 12 ++-- .../05-review/actions/02-review-functional.md | 18 +++--- .../05-review/actions/03-review-relevancy.md | 20 +++--- .../05-review/assets/review-template.md | 46 ++++++-------- .../05-review/references/review-rubric.md | 8 ++- .../skills/04-spec/actions/01-build.md | 2 +- .../skills/04-spec/assets/spec-template.md | 8 ++- .../skills/04-spec/assets/spec-validator.yml | 2 +- 24 files changed, 339 insertions(+), 97 deletions(-) create mode 100644 aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-1.md create mode 100644 aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-2.md create mode 100644 aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-3.md create mode 100644 aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-4.md create mode 100644 aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-5.md create mode 100644 aidd_docs/plans/2026_07_01-sdlc-anti-slop/plan.md diff --git a/README.md b/README.md index f0a9cbbf..e971b8a4 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ [![CI](https://github.com/ai-driven-dev/framework/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/ai-driven-dev/framework/actions/workflows/ci.yml) [![Made in France](https://img.shields.io/badge/made%20in-France-0055A4?labelColor=EF4135)](https://www.ai-driven-dev.fr/) -

🗺️ Live roadmap — what's shipping Now / Next / Later

+

🗺️ Live roadmap — what's shipping Now / Next / Later

@@ -238,7 +238,7 @@ Task-oriented how-to sheets. **[Browse all recipes →](recipes/)** > ⭐ **Free & open-source (MIT), built by the AIDD community.** If AIDD saves you time, [**a star**](https://github.com/ai-driven-dev/framework/stargazers) genuinely helps the project grow and helps other developers find it. -> Actively maintained — the [roadmap](https://github.com/orgs/ai-driven-dev/projects/8/views/1) is public; help shape what comes after. +> Actively maintained — the [roadmap](https://github.com/orgs/ai-driven-dev/projects/8) is public; help shape what comes after. Got an idea or hit a bug? **[Open an issue](https://github.com/ai-driven-dev/framework/issues)** or **[start a discussion](https://github.com/ai-driven-dev/framework/discussions)**. For everything else, **[join the Discord](https://discord.gg/EWySJSpjWs)**. diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-1.md b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-1.md new file mode 100644 index 00000000..20943ce9 --- /dev/null +++ b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-1.md @@ -0,0 +1,30 @@ +--- +status: pending +--- + +# Instruction: Root rule, behavior over instructions + +Part of [`plan.md`](./plan.md). + +## Architecture projection + +```txt +plugins/aidd-context/skills/04-skill-generate/references/ + ✏️ skill-authoring.md # add the Steinberger rule (new R) +``` + +## Tasks to do + +### `1)` Add the behavior-over-instructions rule + +> Codify the pivot as an authoring rule every generator reads. + +1. Add a new `R` to `skill-authoring.md`: an action describes the behavior and the qualify-if bar, never a slot-fill checklist. A template section states what qualifies it and omits when nothing does. +2. State the anti-slop consequence in one line: an empty slot is omitted, never invented. +3. Keep it inside the existing R-list numbering and voice; no new file. + +## Test acceptance criteria + +| Task | Acceptance criteria | +| ---- | ---------------------------------------------------------------------------------- | +| 1 | `skill-authoring.md` carries a rule that says describe behavior + qualify-if/omit, and forbids inventing content for an empty section. | diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-2.md b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-2.md new file mode 100644 index 00000000..a0523d21 --- /dev/null +++ b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-2.md @@ -0,0 +1,31 @@ +--- +status: pending +--- + +# Instruction: Gates single-home + SDLC assert step + +Part of [`plan.md`](./plan.md). + +## Architecture projection + +```txt +plugins/aidd-dev/skills/00-sdlc/ + ✏️ SKILL.md # insert an assert step between implement and review +``` + +## Tasks to do + +### `1)` Chain assert in the orchestrator + +> The gate must run once gates leave the plan. + +1. Add an assert step to the `00-sdlc` action table, delegating to `aidd-dev:03-assert`, between `implement` and `review`. +2. Update the run order and the iterate loop so a failed assert loops back to implement. +3. Confirm no SDLC template or action restates build/test/typecheck commands; those stay in `coding-assertions.md`. + +## Test acceptance criteria + +| Task | Acceptance criteria | +| ---- | ------------------------------------------------------------------------- | +| 1 | `00-sdlc` runs implement → assert → review; a failed assert returns to implement. | +| 1 | No SDLC skill file lists a build or test command; the gate reads `coding-assertions.md`. | diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-3.md b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-3.md new file mode 100644 index 00000000..45a45dbc --- /dev/null +++ b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-3.md @@ -0,0 +1,32 @@ +--- +status: pending +--- + +# Instruction: Spec done-when as observable conditions + +Part of [`plan.md`](./plan.md). + +## Architecture projection + +```txt +plugins/aidd-pm/skills/04-spec/ + ✏️ assets/spec-template.md # done-when = observable condition, qualify-if guidance + ✏️ actions/01-build.md # behavior, not slot-fill +``` + +## Tasks to do + +### `1)` Discipline done-when + +> One observable condition per line, no user-story scaffolding. + +1. Rewrite the Done-when block guidance: each item is an observable condition (what a user or system can be seen doing), not a command and not a feeling. +2. Confirm the plan references the spec done-when rather than recopying it (contract owns it). +3. Update `01-build.md` so it describes the qualify bar per section and omits an empty one, per the phase-1 rule. + +## Test acceptance criteria + +| Task | Acceptance criteria | +| ---- | ----------------------------------------------------------------------------- | +| 1 | Done-when items read as observable conditions; the template shows no command as a done-when. | +| 1 | `01-build.md` instructs qualify-if/omit, carrying no fill-every-slot wording. | diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-4.md b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-4.md new file mode 100644 index 00000000..1050d3a0 --- /dev/null +++ b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-4.md @@ -0,0 +1,43 @@ +--- +status: pending +--- + +# Instruction: Plan + phases anti-slop + +Part of [`plan.md`](./plan.md). + +## Architecture projection + +```txt +plugins/aidd-dev/skills/01-plan/ + ✏️ assets/plan-template.md # decisions = magnitude, resources = URLs/files + ✏️ assets/phase-template.md # wireframe omit-guard, acceptance = observable, drop header redirect + ✏️ actions/04-plan.md # qualify-if/omit behavior + ✏️ actions/03-wireframe.md # only when the phase has UI +``` + +## Tasks to do + +### `1)` Qualify each plan section + +> Every table earns its rows or omits. + +1. Decisions: qualify bar = "you would regret reversing it" (architecture magnitude). Small choices are omitted, not recorded. +2. Resources: external URLs consulted, in the plan; modified files belong to the phase, not the plan resources table. +3. Phase acceptance criteria: observable behavior only; a build/test command is a gate, never an acceptance criterion. + +### `2)` Trim the phase + +> Cut what no reader uses. + +1. Wireframe section: keep the `omit when no UI` guard and make `03-wireframe` run only for a UI phase. +2. Drop the redundant "Part of plan.md" redirect if it carries no navigation value, or justify keeping it in one line. +3. Rewrite `04-plan.md` to the qualify-if/omit behavior from phase 1. + +## Test acceptance criteria + +| Task | Acceptance criteria | +| ---- | ------------------------------------------------------------------------------- | +| 1 | Templates state the qualify bar for decisions, resources, and acceptance criteria. | +| 1 | Acceptance-criteria guidance forbids a bare command as a criterion. | +| 2 | A no-UI phase produces no Wireframe section; `03-wireframe` self-skips without UI. | diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-5.md b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-5.md new file mode 100644 index 00000000..c05957c4 --- /dev/null +++ b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-5.md @@ -0,0 +1,41 @@ +--- +status: pending +--- + +# Instruction: Review redesign, trace phases as checkboxes + +Part of [`plan.md`](./plan.md). + +## Architecture projection + +```txt +plugins/aidd-dev/skills/05-review/ + ✏️ assets/review-template.md # phases as checkboxes, verification line, no prose/follow-up + ✏️ actions/02-review-functional.md # trace the plan's phases, not a free criterion table + ✏️ SKILL.md # point sections at the new template shape +``` + +## Tasks to do + +### `1)` Rebuild the review template + +> A skimmable gate a human reads in one pass. + +1. Structure the report around the plan's phases: one category per phase, each acceptance criterion a checked or unchecked box. +2. End with a verification line: percent verified, the files checked, and a fix / not-applicable / fixed status per unchecked item. +3. Delete the prose blocks and the Follow-up section; the verdict line and the boxes carry the signal. + +### `2)` Point the actions at it + +> The functional axis follows the plan. + +1. Rewrite `02-review-functional.md` to trace the plan's phases and their acceptance criteria, not an ad-hoc criterion list. +2. Update `SKILL.md` section wording to match the new template. + +## Test acceptance criteria + +| Task | Acceptance criteria | +| ---- | -------------------------------------------------------------------------------------- | +| 1 | `review.md` shows one category per plan phase with checked/unchecked boxes and a final verification line. | +| 1 | The template carries no Follow-up section and no prose paragraph. | +| 2 | `02-review-functional.md` traces the plan's phases; a review of a phased plan lists every phase. | diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/plan.md b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/plan.md new file mode 100644 index 00000000..82a0c934 --- /dev/null +++ b/aidd_docs/plans/2026_07_01-sdlc-anti-slop/plan.md @@ -0,0 +1,63 @@ +--- +objective: "SDLC artifacts stop emitting slop: every template section states what qualifies it and omits when nothing does, gates live only in /assert, the review traces the plan's phases as checkboxes." +status: pending +--- + +# Plan: SDLC anti-slop hardening + +## Overview + +| Field | Value | +| ---------- | -------------------------------------------------------------------------------------- | +| **Goal** | Cut the telling-what-to-do, tighten the output contract, so artifacts stay skimmable. | +| **Source** | Brainstorm 2026-06-30 (Alex's SDLC run: weak decisions, fake acceptance criteria, useless wireframe, unreadable review). | + +## Pivot + +Two axes were conflated. Separate them: + +- **How the agent works** (process, steps) => loosen, cut. This is Steinberger: describe the behavior, not the steps. +- **What it emits** (templates, assets) => constrain hard. + +"Cut more" means cut the instructions, not the structure. The root cause of every slop item: a template with empty slots that the agent fills anyway. Fix = each section states its qualify-if bar and omits when nothing meets it. + +## Phases + +| # | Phase | File | +| --- | -------------------------------------------- | ---------------------------- | +| 1 | Root rule: behavior over instructions | [`phase-1.md`](./phase-1.md) | +| 2 | Gates single-home + SDLC assert step | [`phase-2.md`](./phase-2.md) | +| 3 | Spec: done-when as observable conditions | [`phase-3.md`](./phase-3.md) | +| 4 | Plan + phases anti-slop | [`phase-4.md`](./phase-4.md) | +| 5 | Review redesign: trace phases, checkboxes | [`phase-5.md`](./phase-5.md) | + +## Resources + +| Source | Verified | +| ------------- | --------------------------- | +| `plugins/aidd-context/skills/04-skill-generate/references/skill-authoring.md` | Rules live as R1..Rn here; every generator reads it. Home for the Steinberger rule. | +| `plugins/aidd-dev/skills/03-assert/SKILL.md` | Already the gate: "returns a pass or fail verdict", fix-loop until green. Runs the project's coding assertions. | +| `plugins/aidd-dev/skills/00-sdlc/SKILL.md` | Chains spec→plan→implement→review→ship. No assert step: gates would drop if stripped from the plan. | +| `plugins/aidd-dev/skills/01-plan/assets/{plan,phase}-template.md` | Slot templates. `04-plan.md:17` already carries an omit idiom; phase wireframe carries `omit when no UI`. Inconsistent. | +| `plugins/aidd-pm/skills/04-spec/assets/spec-template.md` | Done-when + optional-section suffix `(optional)` already present. | +| `plugins/aidd-dev/skills/05-review/assets/review-template.md` | Prose + Follow-up section; functional axis traces criteria in a table, not the plan's phases. | +| `aidd_docs/memory/coding-assertions.md` | Project's gate commands (build/test/typecheck) — the single home gates belong in. | + +## Decisions + +| Decision | Why | +| ---------- | -------------- | +| Steinberger rule lands in `skill-authoring.md`, not a new central "golden rules" doc | No golden-rules surface exists; every generator already reads this file, so the rule propagates for free. | +| Gates leave the plan for `/assert` + `coding-assertions.md` | One home per fact; the plan becomes 100% behavioral, gates stay mechanical and single-sourced. | +| `00-sdlc` gains an assert step between implement and review | Stripping gates from the plan drops them unless the orchestrator runs `/assert` explicitly. | +| Reuse the existing `` idiom for optional sections | The pattern already exists (phase wireframe); inventing HTML-template machinery would be churn. | + +## Spec done-when → phase coverage + +| Done-when (observable) | Phase | +| ---------------------- | ----- | +| A rule in `skill-authoring.md` states "describe behavior + qualify-if/omit, never slot-fill" | 1 | +| `00-sdlc` runs `/assert` before review; no template restates build/test commands | 2 | +| Spec done-when items are observable conditions, no user-story scaffolding required | 3 | +| Plan decisions are magnitude-only; resources are URLs/files; wireframe omits without UI; no phase-header redirect | 4 | +| `review.md` mirrors the plan's phases as checked/unchecked boxes, ends with a verification line, carries no prose dump or Follow-up | 5 | diff --git a/plugins/aidd-context/skills/04-skill-generate/references/skill-authoring.md b/plugins/aidd-context/skills/04-skill-generate/references/skill-authoring.md index c267cf4f..1919a548 100644 --- a/plugins/aidd-context/skills/04-skill-generate/references/skill-authoring.md +++ b/plugins/aidd-context/skills/04-skill-generate/references/skill-authoring.md @@ -19,7 +19,7 @@ The contract every generated skill must satisfy. These rules govern the CLIENT s - **R6.** Zero duplication. One fact, one home. Actions cite a shared file via `@` instead of restating it. - **R7.** `references/` = documents to READ and apply in place. `assets/` = files to COPY, INJECT, or fill in per run. A template, checklist, or form is an asset, not a reference, because each run instantiates it. - **R8.** Every action follows the action anatomy (below) and carries a `## Test`. -- **R9.** Omit any optional section that would be empty. Never write a placeholder like `## External data` + `None.`. +- **R9.** A section earns its content or is omitted. State each section's qualify bar and describe the behavior it captures, not a slot to fill. An entry that fails the bar is dropped, never invented to occupy the row. Never write `## External data` + `None.`, nor mint a decision, criterion, or resource just because the template offers it. A template's section set is closed: an instance fills or omits the defined sections and never adds, renames, or reorders one. - **R10.** Generated skills are English only (frontmatter, body, actions, references, assets), regardless of conversation language. - **R11.** One idea per sentence. Split a sentence that runs past one line. Exceptions: the single-line `description` and table rows. - **R12.** One file = one artifact. A reference or asset holds a single coherent thing: one checklist, one template, one criteria set. When a file accumulates several independently reusable artifacts, split them so each is cited and reused alone. Prefer this split over bundling, even when the combined file is short. diff --git a/plugins/aidd-dev/agents/checker.md b/plugins/aidd-dev/agents/checker.md index 6ed4cf32..cd71e924 100644 --- a/plugins/aidd-dev/agents/checker.md +++ b/plugins/aidd-dev/agents/checker.md @@ -15,7 +15,8 @@ You are the checker. Your job is to judge finished work against its validator an - Run the checklist on every code or diff, leaving no item unchecked. - Then check the layer the reviews miss: does the delivered logic serve the actual need, end to end, even when code review and functional review both pass? Name any gap between intent and result. - Demand command output or file evidence, never bare claims. Lean strict: a false alarm costs less than a missed defect. -- Return your verdict, findings, and score to whoever invoked you. Hold yourself accountable for whatever you pass. +- When a review skill fits the work, run it and let it write its report; that report is your deliverable and your judgment is what fills it. Never hand-write a parallel prose review beside it. +- Return your verdict, findings, and score on top. Hold yourself accountable for whatever you pass. # Checklist diff --git a/plugins/aidd-dev/skills/00-sdlc/SKILL.md b/plugins/aidd-dev/skills/00-sdlc/SKILL.md index e7265a02..4b387cc3 100644 --- a/plugins/aidd-dev/skills/00-sdlc/SKILL.md +++ b/plugins/aidd-dev/skills/00-sdlc/SKILL.md @@ -14,7 +14,7 @@ Take a request from idea to shipped code, delegating every step. Interactive by | --- | ----------- | ------------------------------------- | --------------------------------------- | | 01 | `spec` | Consolidate sources into the contract | a spec capability | | 02 | `plan` | Produce the plan file | self, via `aidd-dev:01-plan` | -| 03 | `implement` | Build the plan's code | `executor`, via `aidd-dev:02-implement` | +| 03 | `implement` | Build the plan's code, gating on the assertions | `executor`, via `aidd-dev:02-implement` | | 04 | `review` | Verdict `ship` or `iterate` | `checker`, via `aidd-dev:05-review` | | 05 | `ship` | Open the change request | a commit and change-request capability | diff --git a/plugins/aidd-dev/skills/00-sdlc/actions/04-review.md b/plugins/aidd-dev/skills/00-sdlc/actions/04-review.md index d85e1f71..ec01fd66 100644 --- a/plugins/aidd-dev/skills/00-sdlc/actions/04-review.md +++ b/plugins/aidd-dev/skills/00-sdlc/actions/04-review.md @@ -8,12 +8,12 @@ The working diff or paths produced by `03`, the validator (the plan path and acc ## Output -A `ship` or `iterate` verdict with the reviewed items, the findings, the completion and quality scores, and the reviewed `HEAD` SHA (the commit the checker actually saw). The plan reaches `status: reviewed` on ship, and stays `implemented` on iterate. +A `ship` or `iterate` verdict with the reviewed items, findings, completion and quality scores, and the reviewed `HEAD` SHA (the commit the checker saw), from `05-review`. The plan reaches `status: reviewed` on ship, and stays `implemented` on iterate. ## Process 1. **Capture.** Record the current `HEAD` sha as the reviewed SHA. This is the exact code the checker judges, and the anchor `05-ship` checks against. -2. **Spawn.** Spawn the `checker` agent with the inputs above. Brief it to run `aidd-dev:05-review`, code, functional, and relevancy, on that diff, and return its verdict. +2. **Spawn.** Spawn the `checker` agent with the inputs above. Brief it to run `aidd-dev:05-review` on that diff, let that skill write and own its report, and return the verdict and findings. 3. **Map.** When every check passes, the verdict is `ship`. On any blocking finding, the verdict is `iterate`. 4. **Mark.** On `ship`, set the plan frontmatter `status: reviewed` and commit it. Carry the reviewed SHA in the verdict. On `iterate`, leave the plan `implemented`: the loop fixes the diff, not the plan. 5. **Iterate.** On `iterate`, return the findings as the fix list for `03`. The next `04` re-captures the SHA on the fixed diff; ship is reached only when a review of the current diff passes. diff --git a/plugins/aidd-dev/skills/01-plan/actions/04-plan.md b/plugins/aidd-dev/skills/01-plan/actions/04-plan.md index b3b7aa6f..627681e3 100644 --- a/plugins/aidd-dev/skills/01-plan/actions/04-plan.md +++ b/plugins/aidd-dev/skills/01-plan/actions/04-plan.md @@ -14,7 +14,7 @@ A feature folder, always at `aidd_docs/tasks//_ -Part of [`plan.md`](./plan.md). +# Instruction: {title} ## Architecture projection - +> Tree of the final files. ✅ create · ✏️ modify · ❌ delete ```txt . @@ -23,8 +23,10 @@ flowchart TD ## Wireframe + + ```txt -{the confirmed wireframe, or omit this section when the phase has no UI} +{the confirmed wireframe} ``` ## Tasks to do @@ -38,6 +40,8 @@ flowchart TD ## Test acceptance criteria -| Task | Acceptance criteria | -| ---- | ------------------------------------ | -| 1... | {focused deterministic verification} | + + +| Task | Acceptance criteria | +| ---- | -------------------------------- | +| 1... | {observable behavior of the task} | diff --git a/plugins/aidd-dev/skills/01-plan/assets/plan-template.md b/plugins/aidd-dev/skills/01-plan/assets/plan-template.md index b1080cb1..79301f70 100644 --- a/plugins/aidd-dev/skills/01-plan/assets/plan-template.md +++ b/plugins/aidd-dev/skills/01-plan/assets/plan-template.md @@ -3,6 +3,8 @@ objective: "{What must be true when done. One sentence.}" status: pending --- + + # Plan: {title} ## Overview @@ -21,12 +23,16 @@ status: pending ## Resources -| Source | Verified | -| ------------- | --------------------------- | -| {url or path} | {what it settled, one line} | + + +| Source | Verified | +| ------ | ----------------- | +| {url} | {what it settled} | ## Decisions -| Decision | Why | -| ---------- | -------------- | -| {decision} | {one-line why} | + + +| Decision | Why | +| ---------- | ----- | +| {decision} | {why} | diff --git a/plugins/aidd-dev/skills/05-review/README.md b/plugins/aidd-dev/skills/05-review/README.md index e258791a..96962343 100644 --- a/plugins/aidd-dev/skills/05-review/README.md +++ b/plugins/aidd-dev/skills/05-review/README.md @@ -35,8 +35,8 @@ The skill exposes 3 axes, run together by default or one when named: 1. `review-code` - grade the diff against clean-code principles; surface violations with file and line. -2. `review-functional` - verify the feature against the plan's acceptance - criteria; per-criterion trace plus missing / unplanned / edge-case gaps. +2. `review-functional` - trace the feature against the plan's phases; each + acceptance criterion a checked or unchecked box, plus a verification summary. 3. `review-relevancy` - judge whether the change belongs: fit to the need, conformance to the project's declared rules, and no duplication or over-engineering. @@ -45,15 +45,16 @@ The skill exposes 3 axes, run together by default or one when named: - One read-only `review.md` in the reviewed work's feature folder, beside `plan.md`, never patches the code: - - Header: the overall verdict (`approve` / `changes-requested` / `blocked`, - the strictest across the axes run), scope, axes run, findings count. - - `Code` section: severity-rated findings (🔴 / 🟡 / 🟢) with `file:line`; - fixes hand off to [07-refactor](../07-refactor/README.md). - - `Functional` section: per-criterion matrix plus the gap lists; fixes hand - off to [02-implement](../02-implement/README.md) / [08-debug](../08-debug/README.md). - - `Relevancy` section: misfits by lens (`fit` / `conform` / `rot`), each - tied to a rule, a duplication site, a smell, or a named need-gap. - - An axis not run is marked "Not run". + - Header: the overall verdict (`approve` / `changes-requested` / `blocked`), + scope, and findings count. + - `Phases`: plan phases with a checked or unchecked box per acceptance + criterion (functional axis). + - `Findings`: one table for every axis (🔴 / 🟡 / 🟢), Kind (`code` / `fit` / + `conform` / `rot`), a Phase column, and `file:line`; code and relevancy + append their rows here. + - `Verification`: percent verified, files checked, a tag on each unchecked + criterion, and the unplanned changes. + - Each axis runs independently, appending to the shared report. ## Prerequisites diff --git a/plugins/aidd-dev/skills/05-review/SKILL.md b/plugins/aidd-dev/skills/05-review/SKILL.md index f2eb2820..fa974ab9 100644 --- a/plugins/aidd-dev/skills/05-review/SKILL.md +++ b/plugins/aidd-dev/skills/05-review/SKILL.md @@ -14,7 +14,7 @@ Read-only review of a diff along three axes, code quality, feature behavior, and | # | Action | Axis | | --- | ------------------- | ---------------------------------------------------------------- | | 01 | `review-code` | Clean-code quality on the changed lines | -| 02 | `review-functional` | The diff against the plan's acceptance criteria | +| 02 | `review-functional` | The diff against the plan's phases and their acceptance criteria | | 03 | `review-relevancy` | Does the change belong: fit to the need, rule conformance, no rot | Run all three by default, composing one report. Run a single axis only when the caller names it; if it is unclear whether they want all or one, ask. @@ -24,7 +24,7 @@ Run all three by default, composing one report. Run a single axis only when the - Read-only: surface each finding with its fix described, never patch. - Output: always write `review.md` to disk; the file is the deliverable, never an inline-only verdict. - Folder: write into the reviewed work's feature folder (`aidd_docs/tasks//_/`, beside `plan.md`), or one resolved from the change when it has none. -- Sections: fill `review.md` from `assets/review-template.md`, each axis its own section, an unrun axis marked "Not run". +- Report: fill `review.md` from `assets/review-template.md`. One shared `Findings` table for every axis: functional writes the `Phases` boxes and the `Verification` table, code and relevancy append rows to `Findings` under their `Kind`. Tables and boxes, no prose, no per-axis sections. The Phase column ties a finding to the plan when one is in scope, `-` otherwise. - Re-run: overwrite `review.md` with the current review. It is a snapshot of the current diff, not a history; a later review of the same work replaces the earlier one. - Verdict: one overall verdict, the strictest across the axes run, per `references/review-rubric.md`. diff --git a/plugins/aidd-dev/skills/05-review/actions/01-review-code.md b/plugins/aidd-dev/skills/05-review/actions/01-review-code.md index b4154a51..0d9d59d4 100644 --- a/plugins/aidd-dev/skills/05-review/actions/01-review-code.md +++ b/plugins/aidd-dev/skills/05-review/actions/01-review-code.md @@ -4,20 +4,20 @@ Grade the diff against clean-code principles and record the findings in the revi ## Input -The diff to review, a git ref range or path, from the arguments; defaults to the diff against the repository default branch. +The diff to review, a git ref range or path, from the arguments; defaults to the diff against the repository default branch. The plan, when in scope, to tag each finding's phase. ## Output -The `Code` section of the feature folder's `review.md`, filled with severity-rated findings, each citing a changed `file:line`. +Rows in the `Findings` table of the feature folder's `review.md`, kind `code`, each rated and citing a changed `file:line`. ## Process 1. **Resolve.** Take the diff from the arguments, otherwise the diff against the repository default branch. -2. **Review.** Read every changed line for clean-code: naming, structure, complexity, smells, error handling. No runtime checks. Declared-rule conformance belongs to the relevancy axis, not this one. -3. **Rate.** One finding per issue on the changed lines, rated and categorized per `@../references/review-rubric.md`, citing a `file:line`. -4. **Record.** Write the findings into the `Code` section of `review.md`, each with its fix described. +2. **Review.** Read every changed line for clean-code: naming, structure, complexity, smells, error handling. No runtime checks. Rule conformance is the relevancy lens, not this one. +3. **Rate.** One row per issue, rated per `@../references/review-rubric.md`, citing a `file:line`. +4. **Record.** Append one row per finding to the `Findings` table: kind `code`, Phase the plan phase the file falls in or `-`. Write "None." when the run found nothing. ## Test -- The `Code` section of `review.md` is filled, every finding rated and citing a changed `file:line`. +- Each code finding is a `Findings` row, kind `code`, rated and citing a changed `file:line`. - No code is patched. diff --git a/plugins/aidd-dev/skills/05-review/actions/02-review-functional.md b/plugins/aidd-dev/skills/05-review/actions/02-review-functional.md index 253a9308..62e133ae 100644 --- a/plugins/aidd-dev/skills/05-review/actions/02-review-functional.md +++ b/plugins/aidd-dev/skills/05-review/actions/02-review-functional.md @@ -1,24 +1,24 @@ # 02 - Review Functional -Verify the diff matches the plan's acceptance criteria, flows, and edge cases, and record the result in the review report. +Trace the diff against the plan's phases and their acceptance criteria, recording each as a checked or unchecked box. ## Input -The plan path holding the acceptance criteria, from the arguments or the prompt, and the diff to trace (a git ref range; defaults to the diff against the repository default branch). +The plan path holding the phases and their acceptance criteria, from the arguments or the prompt, and the diff to trace (a git ref range; defaults to the diff against the repository default branch). ## Output -The `Functional` section of the feature folder's `review.md`: one row per acceptance criterion traced to the diff, plus the missing, unplanned, and edge-case gaps. +The `Phases` and `Verification` sections of the feature folder's `review.md`: one block per plan phase with a box per acceptance criterion, and the verification summary. ## Process -1. **Read.** Take the plan from the arguments; if absent, ask for the acceptance criteria, and mark this axis "Not run" when none are available. Static review only, no app execution or browser. -2. **Trace.** Fetch the diff, then trace each acceptance criterion to it, one matrix row per criterion: met, partial, or unmet, with evidence or the gap. -3. **Gaps.** List missing behaviors (criteria with no trace), unplanned behaviors (diff changes tracing to no criterion), and flow or edge-case gaps. An empty list reads "none", never omitted. -4. **Record.** Write the matrix and the gaps into the `Functional` section of `review.md`, each with the missing or broken behavior named. +1. **Read.** Take the plan from the arguments; if absent, ask for the acceptance criteria, and mark the `Phases` section "Not run" when none are available. Static review only, no app execution or browser. +2. **Trace.** Fetch the diff, then walk each phase in plan order. For every acceptance criterion, check the box `[x]` only when the diff shows evidence, citing the `file:line`; leave it `[ ]` when unmet or partial, naming the gap. +3. **Summarize.** Fill `Verification`: the percent and count of checked criteria, the files checked, every unchecked criterion tagged `fix`, `not-applicable`, or `fixed`, and any unplanned change in the diff that traces to no criterion. Write "none" for an empty list. +4. **Record.** Write the phase blocks and the verification summary into `review.md`. No prose paragraphs, boxes and the summary only. ## Test -- The `Functional` section of `review.md` holds exactly one row per acceptance criterion. -- The missing, unplanned, and edge-case lists are present, each reading "none" when empty. +- The `Phases` section holds one block per plan phase, every acceptance criterion a checked or unchecked box citing evidence or a gap. +- The `Verification` section reports the verified percent, the files checked, a tag on each unchecked criterion, and the unplanned changes (or "none"). - No code is patched. diff --git a/plugins/aidd-dev/skills/05-review/actions/03-review-relevancy.md b/plugins/aidd-dev/skills/05-review/actions/03-review-relevancy.md index 34b37b5f..97789901 100644 --- a/plugins/aidd-dev/skills/05-review/actions/03-review-relevancy.md +++ b/plugins/aidd-dev/skills/05-review/actions/03-review-relevancy.md @@ -1,24 +1,20 @@ # 03 - Review Relevancy -Judge whether the diff belongs, coherent with the codebase, its conventions and declared rules, serving the real need, with nothing duplicated or over-built, and record the misfits in the review report. +Judge whether the diff belongs (serves the need, conforms to the rules, no rot) and record the misfits in the review report. ## Input -The diff to review (a git ref range or path; defaults to the diff against the repository default branch), the need it serves (the plan objective or the ticket), and the project's declared rules and conventions, discovered at runtime. - -## Output - -The `Relevancy` section of the feature folder's `review.md`, filled with misfit findings under the lenses `fit`, `conform`, and `rot`, each tied to evidence. +The diff to review (a git ref range or path; defaults to the diff against the repository default branch), the need it serves (the plan objective or the ticket), the project's declared rules discovered at runtime, and the plan when in scope. ## Process -1. **Gather.** Resolve the diff from the arguments, otherwise the diff against the repository default branch. Capture the need from the plan objective or the ticket. Discover the project's declared rules and conventions at runtime, never hardcoded. Fall back cleanly when a source is absent. -2. **Fit.** Check the change against the need: does it serve the actual intent end to end, or only the literal criteria? Flag any drift between intent and result. -3. **Conform.** Check the change against the declared rules and the surrounding conventions. Flag each violation with the rule or pattern it breaks. -4. **Rot.** Scan for duplication (an existing helper reinvented), over-engineering (speculative generality, unused abstraction), and incoherence (naming, docs versus code). Cite the site. -5. **Record.** Write each finding into the `Relevancy` section of `review.md`, rated and placed under its lens per `@../references/review-rubric.md`. A bare opinion is not a finding: tie each to a declared rule, a duplication site, an over-engineering smell, or a named need-gap. +1. **Gather.** Resolve the diff, otherwise the diff against the repository default branch. Capture the need from the plan objective or the ticket. Discover the declared rules at runtime, never hardcoded. Fall back cleanly when a source is absent. +2. **Fit.** Does the change serve the real intent end to end, not only the literal criteria? Flag drift. Lens `fit`. +3. **Conform.** Does it hold to the declared rules and conventions? Flag each violation with the rule it breaks. Lens `conform`. +4. **Rot.** Scan for duplication, over-engineering, and incoherence. Cite the site. Lens `rot`. +5. **Record.** Append each misfit to the `Findings` table: kind the lens (`fit`, `conform`, `rot`), Phase the plan phase the file falls in or `-`. A bare opinion is not a finding: tie each to a rule, a duplication site, a smell, or a named need-gap. Write "None." when clean. ## Test -- The `Relevancy` section of `review.md` is filled, every finding under a lens and tied to a `file:line`, a declared rule, a duplication site, or a named need-gap. +- Each misfit is a `Findings` row, kind `fit`, `conform`, or `rot`, tied to a `file:line`, a rule, a duplication site, or a need-gap. - No finding is a bare opinion, and no code is patched. diff --git a/plugins/aidd-dev/skills/05-review/assets/review-template.md b/plugins/aidd-dev/skills/05-review/assets/review-template.md index 8c6df3ad..d2073fe8 100644 --- a/plugins/aidd-dev/skills/05-review/assets/review-template.md +++ b/plugins/aidd-dev/skills/05-review/assets/review-template.md @@ -1,43 +1,33 @@ - + # Review: {{feature}} -{{one_line_summary}} - - **Verdict**: {{approve | changes-requested | blocked}} -- **Diff scope**: `{{base}}...{{head}}` -- **Axes run**: {{code, functional, relevancy}} +- **Diff**: `{{base}}...{{head}}` - **Date**: {{yyyy_mm_dd}} - **Findings**: {{n_critical}} critical, {{n_warning}} warning, {{n_minor}} minor -Verdict: `approve` = ship it; `changes-requested` = fix the warnings or fixable criticals first; `blocked` = a critical that must not merge. The overall verdict is the strictest across the axes run. - -## Code - -Clean-code findings on the changed lines (or "Not run"). - -| Sev | Category | Location | Issue | Suggested fix | -| --- | -------- | -------- | ----- | ------------- | - -## Functional +## Phases -Each acceptance criterion traced to the diff (or "Not run"). + -| Criterion | Met | Evidence / gap | -| --------- | --- | -------------- | +### Phase {{n}} — {{phase-name}} -- **Missing behaviors**: {{criteria with no trace, or "none"}} -- **Unplanned behaviors**: {{diff changes tracing to no criterion, or "none"}} -- **Edge-case gaps**: {{gaps, or "none"}} +- [x] {{criterion met}} — {{file:line}} +- [ ] {{criterion unmet}} — {{gap}} -## Relevancy +## Findings -Does the change belong (or "Not run"). Every finding ties to evidence, never an opinion. + -| Sev | Lens | Location / rule | Misfit | Suggested fix | -| --- | ---- | --------------- | ------ | ------------- | +| Sev | Kind | Phase | Location | Issue | Fix | +| --- | ---- | ----- | -------- | ----- | --- | -## Follow-up +## Verification -- **Top fixes** (ranked): {{top_fixes}} -- **Notes**: {{additional_notes}} +| Metric | Value | +| ------------- | ------------------------------------------------- | +| Verified | {{pct}}% ({{n_checked}}/{{n_total}}) | +| Files checked | {{files, comma-separated}} | +| Unchecked | {{criterion — fix / not-applicable / fixed}}, or none | +| Unplanned | {{change tracing to no criterion}}, or none | diff --git a/plugins/aidd-dev/skills/05-review/references/review-rubric.md b/plugins/aidd-dev/skills/05-review/references/review-rubric.md index 759ee612..dae68188 100644 --- a/plugins/aidd-dev/skills/05-review/references/review-rubric.md +++ b/plugins/aidd-dev/skills/05-review/references/review-rubric.md @@ -12,9 +12,11 @@ Shared definitions for the three review axes. The actions and the report templat One overall verdict, the strictest across the axes run: -- `approve`: no critical finding, ship it. -- `changes-requested`: warnings, or a fixable critical to address first. -- `blocked`: a critical that must not merge. +- `approve`: no critical finding, and every acceptance criterion checked, ship it. +- `changes-requested`: warnings, a fixable critical, or any unchecked criterion tagged `fix`. +- `blocked`: a critical that must not merge, or an unchecked critical criterion. + +An unchecked criterion in the `Phases` section is a functional finding: one tagged `fix` cannot yield `approve`. ## Code categories diff --git a/plugins/aidd-pm/skills/04-spec/actions/01-build.md b/plugins/aidd-pm/skills/04-spec/actions/01-build.md index dd7a521f..5f08d7fd 100644 --- a/plugins/aidd-pm/skills/04-spec/actions/01-build.md +++ b/plugins/aidd-pm/skills/04-spec/actions/01-build.md @@ -14,7 +14,7 @@ The path to `spec.md` in the feature folder, drafted from the template, with the 1. **Source.** From a PRD path, lift its target, hard constraints, non-goals, and done-when into the template, dropping any implementation detail. From a request, map it onto the template sections directly. Do not explore the codebase. 2. **Gaps.** Replace any missing required field with `TBD: `. Never guess. -3. **Check.** Confirm every section the validator requires is present. +3. **Check.** Confirm every section the validator requires is present. Omit an optional section (stakeholders, context) that has nothing to say rather than emit a placeholder. 4. **Write.** Resolve the feature folder, reusing it when it exists, and save the spec there as `spec.md`. 5. **Return.** Surface the spec path and the notes. diff --git a/plugins/aidd-pm/skills/04-spec/assets/spec-template.md b/plugins/aidd-pm/skills/04-spec/assets/spec-template.md index b2f504f4..c669e755 100644 --- a/plugins/aidd-pm/skills/04-spec/assets/spec-template.md +++ b/plugins/aidd-pm/skills/04-spec/assets/spec-template.md @@ -1,3 +1,5 @@ + + # ## Target @@ -22,10 +24,10 @@ ## Done-when - + -- -- +- +- ## Stakeholders (optional) diff --git a/plugins/aidd-pm/skills/04-spec/assets/spec-validator.yml b/plugins/aidd-pm/skills/04-spec/assets/spec-validator.yml index 1da8edd5..5b1f55d8 100644 --- a/plugins/aidd-pm/skills/04-spec/assets/spec-validator.yml +++ b/plugins/aidd-pm/skills/04-spec/assets/spec-validator.yml @@ -18,7 +18,7 @@ criteria: weight: 20 - id: done_when - description: Conditions for completion listed and each is verifiable (defined check, test, or metric - not a feeling or vague heuristic). + description: Completion conditions listed, each an observable outcome a user or the system can be seen producing. Verifiable by observation, not a command and not a feeling. required: true weight: 25 From e51df8123fb0ea6e2668fbb160acb260c085f9bc Mon Sep 17 00:00:00 2001 From: Baptiste LAFOURCADE Date: Wed, 1 Jul 2026 13:24:37 +0200 Subject: [PATCH 2/2] feat(dev): enforce the review report section set with a validator review.md sections are now fixed by assets/review-validator.yml (Phases, Findings, Verification). 05-review verifies its output against it before returning and removes any section the model added, replacing the passive template comment with a deterministic contract. Co-Authored-By: Claude Opus 4.8 --- plugins/aidd-dev/skills/05-review/SKILL.md | 2 ++ plugins/aidd-dev/skills/05-review/assets/review-template.md | 2 +- .../aidd-dev/skills/05-review/assets/review-validator.yml | 5 +++++ 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 plugins/aidd-dev/skills/05-review/assets/review-validator.yml diff --git a/plugins/aidd-dev/skills/05-review/SKILL.md b/plugins/aidd-dev/skills/05-review/SKILL.md index fa974ab9..f653f9be 100644 --- a/plugins/aidd-dev/skills/05-review/SKILL.md +++ b/plugins/aidd-dev/skills/05-review/SKILL.md @@ -25,6 +25,7 @@ Run all three by default, composing one report. Run a single axis only when the - Output: always write `review.md` to disk; the file is the deliverable, never an inline-only verdict. - Folder: write into the reviewed work's feature folder (`aidd_docs/tasks//_/`, beside `plan.md`), or one resolved from the change when it has none. - Report: fill `review.md` from `assets/review-template.md`. One shared `Findings` table for every axis: functional writes the `Phases` boxes and the `Verification` table, code and relevancy append rows to `Findings` under their `Kind`. Tables and boxes, no prose, no per-axis sections. The Phase column ties a finding to the plan when one is in scope, `-` otherwise. +- Sections: the report has exactly the sections in `assets/review-validator.yml`. Before returning, verify against it and remove any section not listed. - Re-run: overwrite `review.md` with the current review. It is a snapshot of the current diff, not a history; a later review of the same work replaces the earlier one. - Verdict: one overall verdict, the strictest across the axes run, per `references/review-rubric.md`. @@ -35,3 +36,4 @@ Run all three by default, composing one report. Run a single axis only when the ## Assets - `assets/review-template.md`: the single report the three axes fill. +- `assets/review-validator.yml`: the closed set of report sections. diff --git a/plugins/aidd-dev/skills/05-review/assets/review-template.md b/plugins/aidd-dev/skills/05-review/assets/review-template.md index d2073fe8..4a2a7fb6 100644 --- a/plugins/aidd-dev/skills/05-review/assets/review-template.md +++ b/plugins/aidd-dev/skills/05-review/assets/review-template.md @@ -1,4 +1,4 @@ - + # Review: {{feature}} diff --git a/plugins/aidd-dev/skills/05-review/assets/review-validator.yml b/plugins/aidd-dev/skills/05-review/assets/review-validator.yml new file mode 100644 index 00000000..0f4489ea --- /dev/null +++ b/plugins/aidd-dev/skills/05-review/assets/review-validator.yml @@ -0,0 +1,5 @@ +# review.md has exactly these sections. Any other is invalid: remove it, never add or rename. +sections: + - Phases # required: one block per plan phase, a box per criterion + - Findings # required: one table, all axes, by Kind + - Verification # required: the summary table