Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions docs/agentic-dev-kit/fallback-review-panel-evidence.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,47 @@ clause is `safety-critical-changes.md` rule 3's own wording, quoted from the
sibling doctrine file rather than from a tracker issue — a review lens went
looking for it on the tracker and could not source it.)

### The session that measured withdrawal

PR `#225` (`#41`, the required/optional manifest axis). Five rounds:

| Round | What it reviewed | HIGH |
|---|---|---|
| 1 | the change | 2 |
| 2 | the round-1 fix | 2 |
| 3 | the round-2 fix | 2 |
| 4 | the round-3 fix | 1 |
| 5 | the withdrawal | 0 |

Only round 1 reviewed the change. Its second HIGH was a real fail-open — the
derived graph missed `lib/repo_root.sh`, which two shell engines `source`, so
`kit_doctor` called a broken tree clean at exit 0 — and the fix was a bash
`source` scanner: a new mechanism, prompted by a finding, which the filing rule
says to file rather than build. It was built.

Every HIGH in rounds 2–4 was inside that scanner, and each was a defect in the
previous round's fix for it:

- **round 2** — the scan missed guarded `source` (`[ -f "$L" ] && source "$L"`),
and its exit-code test was confounded by its own fixture, so deleting the
mechanism under test left the suite green
- **round 3** — the widened anchor matched `source` inside a quoted string, so
`echo "run this; source lib/dep.sh"` became a real edge; and the heredoc
tracker read `<<<` and arithmetic `<<` as openers, blinding whole files
- **round 4** — the heredoc tracker recognised fewer opener forms than bash
accepts (`cmd <<A <<B`, `<<123`, `<<'MULTI WORD'`), so a real heredoc body was
scanned as code and manufactured an edge

Both failure directions are tokenizer problems — command position, and heredoc
boundaries — and no regex approximates either safely. The threshold was declared
before round 4's findings ("a HIGH means the scanner comes out"), a HIGH arrived,
and round 5 reviewed a delta of 90 insertions against 560 deletions and found
none. The scanner is `#228`, filed with the four constructs that defeated it and
an executable acceptance bar.

The counterfactual is the whole point: filed at round 1, as the rule already
required, that PR is two rounds rather than five.

### The cost measurement that does not exist

`#163` Sink 2 asked for a cost measurement before "aim the re-run at the delta"
Expand Down
16 changes: 16 additions & 0 deletions docs/agentic-dev-kit/fallback-review-panel.md
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,22 @@ the trap inside them. So make each round *smaller*, not
fewer: `safety-critical-changes.md` rule 3 ("a fix round addresses only what the
review found") — a new mechanism gets filed, however squarely a finding prompted it.

**When that rule was not followed, withdraw rather than patch.** The filing rule
is prevention; this is its recovery, and it exists because the prevention was
measured failing. A mechanism a round-1 finding prompted was built rather than
filed, and every later round then found a HIGH *inside that mechanism* while the
rest of the change drew nothing — rounds 2–5 reviewed the fixes, not the work.
The signal is not a round count but **a fix that itself draws a HIGH, twice, in
the same mechanism**: patching further is evidence-free, each patch a fresh guess
at a problem the last guess misread. Take the mechanism out, file it with what
defeated it, and let the round that removes it be the last — removal is the
Comment on lines +446 to +454

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Apply the withdrawal rule to the scope promised by the PR.

This paragraph makes withdrawal conditional on violating the filing rule and on a mechanism prompted by a round-1 finding. The PR objective states a broader trigger: withdraw a mechanism when a fix draws HIGH twice within that mechanism. If the narrower scope is intentional, state it explicitly. Otherwise, remove the dependency on “that rule” and “round-1 finding”. A pre-existing mechanism that fails twice could otherwise continue to receive patches.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/agentic-dev-kit/fallback-review-panel.md` around lines 446 - 454, Update
the withdrawal rule paragraph to apply to any mechanism whose fixes draw HIGH
findings twice, including pre-existing mechanisms. Remove the dependencies on
violating the filing rule and on a round-1 finding, unless the broader PR
objective is intentionally being narrowed; ensure the stated trigger matches
that objective.

cheapest round there is, deleting surface rather than adding more for the next
round to find.
Comment on lines +454 to +456

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Treat withdrawal as the last fix round, not the last review round.

The phrase “let the round that removes it be the last” can be read as skipping the required re-run. Existing Step 6 requires a re-run when the delta contains executable code or executed prose, and the evidence records a review of the withdrawal in round 5. State that removal is the last fix round, while the required verification and review still run afterward.

Proposed wording
- let the round that removes it be the last
+ make the removal the last fix round; still run the required verification and review
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
defeated it, and let the round that removes it be the last — removal is the
cheapest round there is, deleting surface rather than adding more for the next
round to find.
defeated it, and make the removal the last fix round; still run the required verification and review — removal is the
cheapest round there is, deleting surface rather than adding more for the next
round to find.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/agentic-dev-kit/fallback-review-panel.md` around lines 454 - 456, Update
the withdrawal guidance in the surrounding fallback-review process to state that
removal is the last fix round, not the last review round. Preserve Step 6’s
required re-run for executable code or executed prose and the subsequent
review/evidence recording after the withdrawal.


**Declare the withdrawal threshold before seeing a round's findings**, beside the
stopping criterion. Named in advance it is a decision; reached afterwards it is
indistinguishable from fatigue, and no reviewer can tell which it was.
Comment on lines +451 to +460

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Define the exact withdrawal threshold.

“Draws a HIGH, twice” does not define the counting unit. It can mean two HIGH findings in one fix or HIGH findings in two successive fix rounds. It also does not define whether the initial finding counts or how “same mechanism” is identified. docs/agentic-dev-kit/fallback-review-panel-evidence.md lists HIGH counts in rounds 2, 3, and 4, declares the threshold before round 4, and withdraws in round 5. State the intended trigger and explain that timeline. Otherwise reviewers can choose different withdrawal points.

🧰 Tools
🪛 LanguageTool

[locale-violation] ~459-~459: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...ed in advance it is a decision; reached afterwards it is indistinguishable from fatigue, a...

(AFTERWARDS_US)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/agentic-dev-kit/fallback-review-panel.md` around lines 451 - 460,
Clarify the withdrawal rule in the stopping-criterion section by defining the
counting unit, whether the initial finding counts, and how HIGH findings are
matched to the same mechanism. Align the wording with
fallback-review-panel-evidence.md: count the relevant HIGH findings from rounds
2, 3, and 4, declare the threshold before round 4, and withdraw the mechanism in
round 5.


**Batch the fix round into one commit and one push, and aim the re-run at the
delta.** Each push invalidates the current-head receipt, so each new head
costs another required review — a fix round landed as four pushes buys four
Expand Down
4 changes: 2 additions & 2 deletions kit-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
},
"docs/agentic-dev-kit/fallback-review-panel-evidence.md": {
"role": "doctrine",
"sha256": "74548e09fc06caae09556e11b7501a0f29171188bc033ce20ddcdce2dccd2eb6"
"sha256": "599fc0c1995cd0510cf6d7fa1b46b19a7c84acd77bf56cf8f8db91441b9e1d26"
},
"docs/agentic-dev-kit/fallback-review-panel.md": {
"role": "doctrine",
"sha256": "d397bacce89892cc425232668c91cee33b6b41a87ff6cbd7e72d27055ab402bd"
"sha256": "0ce06ccb47a2130fb30e1796aaae9ea747757493e77f2a11c568ab75d948f408"
},
"docs/agentic-dev-kit/safety-critical-changes.md": {
"role": "doctrine",
Expand Down
Loading