From ed84548f0a8d2a0a981e0c2b9849a7bbe25c732d Mon Sep 17 00:00:00 2001 From: fredericrous Date: Sun, 25 Oct 2026 12:10:07 +0100 Subject: [PATCH 1/4] docs(plan): code rules reach the reviewer, phase 4 Co-Authored-By: Claude Opus 5.5 --- .../2026-10-24-code-rules-reach-the-reviewer.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/plans/2026-10-24-code-rules-reach-the-reviewer.md b/docs/plans/2026-10-24-code-rules-reach-the-reviewer.md index 7882cc4..385c7f2 100644 --- a/docs/plans/2026-10-24-code-rules-reach-the-reviewer.md +++ b/docs/plans/2026-10-24-code-rules-reach-the-reviewer.md @@ -1,7 +1,7 @@ --- canonical: decisions:docs/plans/2026-10-24-code-rules-reach-the-reviewer.md -phases: [2] -status: done +phases: [2, 4] +status: active --- # Code rules reach the reviewer (pointer) @@ -26,3 +26,13 @@ The `Candidates:` block (phase 4) comes in a later PR. - approve-with-changes, then approve-with-changes on the Delta. Fixed: the reviewer contract's no-corpus case, exit 1 named, the repeated lines kept, the Delta's fresh RULES line. - deliberate: the Delta step does not yet say "none in a repository with no `.adr.yaml`"; phase 4's dotfiles PR edits this same paragraph for the `Candidates:` block and adds it. - 41k + 44k tokens. + +## Phase 4 + +This repository carries Phase 4's skill and agent edits: +- F4b's brief lists the `<<>>` block that amont-agent 3.3.0 prints; +- step 3 and the Delta paste that block verbatim into the reviewer's prompt, after the RULES line; +- the implementation-review contract adds a `Candidates:` block ruling on every listed id, which the push check reads; the output word cap leaves that block out; +- the no-corpus exception goes, since `review-brief` now handles a repository with no `.adr.yaml`. + +The Delta's "none in a repository with no `.adr.yaml`" note (Phase 2's deliberate) lands here. From beb27ae6779374ab9e50ec2ed43fcd1ca25e6fab Mon Sep 17 00:00:00 2001 From: fredericrous Date: Sun, 25 Oct 2026 12:10:09 +0100 Subject: [PATCH 2/4] docs(review): candidates in F4b and the contract Co-Authored-By: Claude Opus 5.5 --- dot_claude/agents/implementation-review.md | 15 ++++++++++++-- dot_claude/skills/worktree-task/SKILL.md | 24 ++++++++++++++-------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/dot_claude/agents/implementation-review.md b/dot_claude/agents/implementation-review.md index c5a1079..1ff464f 100644 --- a/dot_claude/agents/implementation-review.md +++ b/dot_claude/agents/implementation-review.md @@ -13,7 +13,8 @@ never waive a rule, and never approve on the author's behalf. Your prompt carries, in order: the review block (`<< sha=<64 hex>>>>` — the tree you are reading, which the hook will check against the push), `Round 1.` or `Delta.`, the -`<<>>` line, the brief `amont-agent review-brief` built (inline, +`<<>>` line, the `<<>>` block, the brief +`amont-agent review-brief` built (inline, or its path to read in full), and this contract. In a repository with no `.adr.yaml` there is no RULES line and the brief is hand-built: that is not a defect. Read the plan at the path the brief @@ -65,7 +66,7 @@ Every finding needs evidence: a `file:line` in the diff, or a line of the plan. A finding without one is a question, and belongs under "Would still check by hand". -## Output (at most 350 words) +## Output (at most 350 words, not counting the `Candidates:` block) The first line is the verdict, exactly as below, because the hook reads it. @@ -74,8 +75,18 @@ Verdict: approve | approve-with-changes | rework Findings: 1. [blocking|high|medium|low] . Evidence: . Rule: . Edit: . Would still check by hand: +Candidates: + violation: + fine: ``` +The `Candidates:` block rules on every id the prompt's CANDIDATES block +lists, and only those: `violation:` when the line breaks the rule (and the +same problem is a finding above), `fine:` with the clause of the rule body +that makes it fine. The push check reads this block and journals any id left +without a ruling. With `<<>>`, or `n=0`, there is no +block to write. + `rework` only when the implementation cannot ship as written: a ticked phase that is not there, a recorded verification that cannot be true, or a blocking constraint violation whose fix changes the design. A blocking diff --git a/dot_claude/skills/worktree-task/SKILL.md b/dot_claude/skills/worktree-task/SKILL.md index 6a516bd..c828fbe 100644 --- a/dot_claude/skills/worktree-task/SKILL.md +++ b/dot_claude/skills/worktree-task/SKILL.md @@ -182,11 +182,16 @@ below names one of them. It prints, in order: - the `<<>>` block (the same as `tree-sha --block`: `HEAD` without `docs/plans/`) and a `<<>>` line; + - the `<<>>` block (amont-agent 3.3.0 and + adr-lint 0.3.0): one line per candidate line the diff adds, ending + `<<>>`, or `<<>>` when adr-lint + could not run; - the plan's Verification section; - the diff stat, and the full diff under 60 KB; - the code rules in full; - the constraint index; - the decisions that apply to the changed paths, each resolved; + - a `## Candidates` section with each candidate's pattern and line; - the plan's Non-goals. For a pointer plan, pass the canonical plan's path. The merge-base is @@ -194,10 +199,8 @@ below names one of them. Exit 1 means the repository or its base could not be read: pass `--base ` (a stacked or local-only branch). Exit 3 means aval failed and nothing was printed; fix aval, never hand-build the brief - instead. One exception, until review-brief - handles it: a repository with no `.adr.yaml` has no decided code - rules, and aval exits 3 there. Build that brief by hand from the - parts above, with no rules section and no RULES line. + instead. A repository with no `.adr.yaml` has no decided code rules: + the brief says so and carries no RULES line (amont-agent 3.3.0). 2b. **Interface review** (advisory; skipped unless the diff touches the repository's UI). Trigger: the `ui` areas from `aval traits`, matched against `git diff --name-only ..HEAD`; no match, no step. Build @@ -216,8 +219,12 @@ below names one of them. 3. Launch the `implementation-review` agent once, in the foreground, with, in order: the TREE line verbatim; `Round 1.`; the RULES line verbatim (the push check journals a review whose prompt lacks it, or - carries a stale one); the brief, inline or as its path with "read it - in full" (an inline brief repeats both lines at its top; leave them); + carries a stale one); the CANDIDATES block verbatim, from its first + line to `<<>>` (the push check reads the ids from the + prompt, not from a file); the brief, inline or as its path with "read + it in full" (an inline brief repeats these lines at its top; leave + them); "Your result must carry a `Candidates:` block ruling on every + listed id"; "Read the plan and the diff. Open at most 10 more files or 1,500 lines. Follow your output contract." A background launch counts only after its completion notification. @@ -236,8 +243,9 @@ below names one of them. 6. `approve`, or `approve-with-changes` with every finding fixed or named deliberate: continue to F5. A fix that changes code returns to F1, and the **second** pass through F4b is the `Delta`: a fresh block - for the new tree, `Delta.`, the RULES line from a fresh - `review-brief`, the round-1 findings, the diff since, + for the new tree, `Delta.`, the RULES line and the CANDIDATES block + from a fresh `review-brief` (none in a repository with no + `.adr.yaml`), the round-1 findings, the diff since, "answer resolved / not resolved / new blocker for each". The Delta may resume the same reviewer with `SendMessage` (the block first in the message) instead of a fresh launch: the hook counts a resumed round on From 6924c20822da2dacc4a62e263c6733acab44a3bc Mon Sep 17 00:00:00 2001 From: fredericrous Date: Sun, 25 Oct 2026 12:12:00 +0100 Subject: [PATCH 3/4] docs(review): candidates block always present Co-Authored-By: Claude Opus 5.5 --- dot_claude/agents/implementation-review.md | 4 ++-- dot_claude/skills/worktree-task/SKILL.md | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dot_claude/agents/implementation-review.md b/dot_claude/agents/implementation-review.md index 1ff464f..678dfc0 100644 --- a/dot_claude/agents/implementation-review.md +++ b/dot_claude/agents/implementation-review.md @@ -16,8 +16,8 @@ the hook will check against the push), `Round 1.` or `Delta.`, the `<<>>` line, the `<<>>` block, the brief `amont-agent review-brief` built (inline, or its path to read in full), and this contract. In a repository with no -`.adr.yaml` there is no RULES line and the brief is hand-built: that is not a -defect. Read the plan at the path the brief +`.adr.yaml` there is no RULES line and the brief says it has no decided code +rules: that is not a defect. Read the plan at the path the brief names first, then the diff the brief carries (or the files its stat names, when the diff was too large to inline). Open at most 10 more files or 1,500 lines: the files the diff touches, then their tests. diff --git a/dot_claude/skills/worktree-task/SKILL.md b/dot_claude/skills/worktree-task/SKILL.md index c828fbe..22a6f13 100644 --- a/dot_claude/skills/worktree-task/SKILL.md +++ b/dot_claude/skills/worktree-task/SKILL.md @@ -223,8 +223,8 @@ below names one of them. line to `<<>>` (the push check reads the ids from the prompt, not from a file); the brief, inline or as its path with "read it in full" (an inline brief repeats these lines at its top; leave - them); "Your result must carry a `Candidates:` block ruling on every - listed id"; + them); when the block lists any id, "Your result must carry a + `Candidates:` block ruling on every listed id"; "Read the plan and the diff. Open at most 10 more files or 1,500 lines. Follow your output contract." A background launch counts only after its completion notification. @@ -243,9 +243,9 @@ below names one of them. 6. `approve`, or `approve-with-changes` with every finding fixed or named deliberate: continue to F5. A fix that changes code returns to F1, and the **second** pass through F4b is the `Delta`: a fresh block - for the new tree, `Delta.`, the RULES line and the CANDIDATES block - from a fresh `review-brief` (none in a repository with no - `.adr.yaml`), the round-1 findings, the diff since, + for the new tree, `Delta.`, the RULES line (absent in a repository + with no `.adr.yaml`) and the CANDIDATES block from a fresh + `review-brief`, the round-1 findings, the diff since, "answer resolved / not resolved / new blocker for each". The Delta may resume the same reviewer with `SendMessage` (the block first in the message) instead of a fresh launch: the hook counts a resumed round on From f96b1372c06792ae31c6ecbcd1357e16b1df3f71 Mon Sep 17 00:00:00 2001 From: fredericrous Date: Sun, 25 Oct 2026 12:13:19 +0100 Subject: [PATCH 4/4] docs(plan): code rules reach the reviewer, phase 4 Co-Authored-By: Claude Opus 5.5 --- ...026-10-24-code-rules-reach-the-reviewer.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/docs/plans/2026-10-24-code-rules-reach-the-reviewer.md b/docs/plans/2026-10-24-code-rules-reach-the-reviewer.md index 385c7f2..72bd34e 100644 --- a/docs/plans/2026-10-24-code-rules-reach-the-reviewer.md +++ b/docs/plans/2026-10-24-code-rules-reach-the-reviewer.md @@ -1,7 +1,7 @@ --- canonical: decisions:docs/plans/2026-10-24-code-rules-reach-the-reviewer.md phases: [2, 4] -status: active +status: done --- # Code rules reach the reviewer (pointer) @@ -36,3 +36,20 @@ This repository carries Phase 4's skill and agent edits: - the no-corpus exception goes, since `review-brief` now handles a repository with no `.adr.yaml`. The Delta's "none in a repository with no `.adr.yaml`" note (Phase 2's deliberate) lands here. + +### Verification (phase 4) + +Input → expected → actual: +- amont-agent#144's implementation reviewer, given the inline `<<>>` block → a `Candidates:` block ruling on every listed id → 18/18 in round 1 and 18/18 in the delta (including the one new id), each `fine` with a quoted rule-body clause. +- `amont-agent review-brief` 3.3.0 in this repository, which has no `.adr.yaml` → exit 0, no RULES line, a "no aval corpus" section, a CANDIDATES block → exit 0, as expected, `n=0`. + +A deployed Round-1 review whose `Candidates:` block the push check reads without a "left without a ruling" entry can only run after `chezmoi apply`, on the next code branch. It is recorded in the decisions PR that records phase 4. + +### Implementation review (phase 4) + +- approve-with-changes (4 findings), then approve on the Delta. +- Fixed: + - the Delta always carries the CANDIDATES block, and only the RULES line is absent without a corpus; + - the contract no longer says the brief is "hand-built"; + - step 3 asks for `Candidates:` only when the block lists an id. +- 34k + 38k tokens.