docs: give the panel's filing rule a recovery path — withdraw, don't patch again - #230
Closed
topij wants to merge 1 commit into
Closed
docs: give the panel's filing rule a recovery path — withdraw, don't patch again#230topij wants to merge 1 commit into
topij wants to merge 1 commit into
Conversation
…patch again The doctrine already says a new mechanism gets filed "however squarely a finding prompted it". That is prevention, and PR #225 measured it failing: a mechanism a round-1 finding prompted was built rather than filed, and every later round found a HIGH inside that mechanism while the rest of the change drew nothing. Rounds 2-5 reviewed the fixes, not the work. What was missing is the recovery. Once the mechanism is in the diff and drawing HIGHs, the existing rule has nothing to say, and the default — patch it — is what turns two rounds into five. The rule added names a signal that is not a round count: a fix that itself draws a HIGH, twice, in the same mechanism. Past that, each patch is a fresh guess at a problem the previous guess misread. Withdraw the mechanism, file it with what defeated it, and let the removing round be the last — it deletes surface rather than adding more for the next round to find. The companion measurement is the counterfactual: filed at round 1 as the rule already required, #225 is two rounds instead of five. Second half is operational. The threshold has to be declared BEFORE 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. #225 declared it at round 4 and honoured it. Placement: the rule goes in the paragraph that already carries the filing rule rather than opening a section, and the measurement goes to the companion — #213 stays open on this file's length, so an addition here has to earn its lines. The executable half is 14 lines; the evidence is 40 and lands in the file #213's split created for exactly that. Both files are KIT_OWNED, so kit-manifest.json is regenerated. Verified: the addition is outside `## The contract every lens gets`, which is the only section panel_prompt.py parses — `contract()` still returns all 13 items, so no launch prompt changes and #167's stable numbering is untouched. make test -> 699 passed. kit_doctor -> 32 unchanged, 0 differ, 0 missing, 0 unknown, exit 0. #213, #228 and #163's missing cost measurement all stay open.
📝 WalkthroughWalkthroughThe fallback review panel adds a predefined withdrawal rule after repeated HIGH findings. The evidence document records five review rounds and scanner defects. The manifest hashes are updated for both modified doctrine documents. ChangesFallback review policy
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with 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.
Inline comments:
In `@docs/agentic-dev-kit/fallback-review-panel.md`:
- Around line 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.
- Around line 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.
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 55b7bc4e-7d4c-41e6-9e75-ff895633a7d7
📒 Files selected for processing (3)
docs/agentic-dev-kit/fallback-review-panel-evidence.mddocs/agentic-dev-kit/fallback-review-panel.mdkit-manifest.json
Comment on lines
+446
to
+454
| **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 |
There was a problem hiding this comment.
🎯 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.
Comment on lines
+451
to
+460
| 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 | ||
| cheapest round there is, deleting surface rather than adding more for the next | ||
| round to find. | ||
|
|
||
| **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. |
There was a problem hiding this comment.
🗄️ 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.
Comment on lines
+454
to
+456
| 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. |
There was a problem hiding this comment.
🎯 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.
Owner
Author
|
Closing unmerged, under this PR's own pre-declared withdrawal threshold. The PR body declared, before round 1: "if a round finds a HIGH in the rule Round 1 returned five HIGH findings across both lenses, plus three Major Redesign is filed as #231, carrying all three reviewers' findings, the design Why this is the right call and not just consistency. Two of the findings are
Fixing those means redesigning where the threshold lives and how it is disputed — Two errors of mine worth recording separately, both in newly-added record
And one finding about the mechanism itself. The threshold I declared here was No commit was made in response to these findings, so no receipt was invalidated |
topij
added a commit
that referenced
this pull request
Aug 2, 2026
…line Panel round 1, both lenses, converging on the same two. **The headline claim was wrong about one of its own three members (HIGH).** It said a mechanism kept drawing HIGH findings "round after round" three times, and that withdrawing it ended the loop each time. Two fit. `#230` does not: it drew HIGHs once, in its only round, before any fix existed — there was no loop for a withdrawal to end, and the block's own Learned bullet said so two paragraphs later. It was also a hand-written count of this session's own work, which `wrap-up.md` step 3 forbids in those words, for exactly the reason that applied here: a recount found it wrong. Cut rather than corrected, per the record-prose rule that a HIGH imprecision is deleted or shortened and never amended into a more careful figure. What is left is the invariant with the tally removed, and the two withdrawals named as pointers. **The demoted block lost its own `▶ Next:` line, against this file's own precedent (regression).** Every earlier session block still carries the starter it ended with; this was the first demotion to strip one, and `wrap-up.md` authorises adding a starter to the newest block, not removing one elsewhere. Restored. Left unfixed and logged on the PR, all sub-HIGH imprecision in record prose, which under this PR's declared criterion is logged rather than repaired: `#233` described as three withdrawn attempts when its third round shipped; CodeRabbit's "Major" findings folded into "HIGHs" in the `#230` sentence; and a loose `#143` adjacency in the friction-log bullet. `check_doc_budget.py` still reports the handoff inside budget.
topij
added a commit
that referenced
this pull request
Aug 2, 2026
…eat repair (#235) * chore: update handoff — Phase 2's blockers are done, and withdrawal beat repair Phase 2's named blockers `#41`, `#134` and `#226` all went out in `ee3371d` and `3e34fe5`, so the cs-toolkit vendoring is the next large piece. The block leads with what outlives both features rather than with either: three separate times this session a mechanism kept drawing HIGH findings round after round, and each time removing it ended the loop that patching was extending. The doctrine already forbids building those mechanisms — "a new mechanism gets filed, however squarely a finding prompted it" — and has nothing to say once one is already in the diff. `#231` is that gap, and it is the `▶ Next:`. Record prose written to the workflow's own rules: no hand-written counts of this session's work — the per-tree figures live in `3e34fe5`'s commits, where each is stated with the vendored subset that identifies it, and every live figure names the command that prints it. The superseded block's `▶ Next:` line was removed rather than left to compete with the new one. No friction-log entry: the inbox is over budget and the one workflow defect worth recording went to `#71`, which already owns the class — a hand-run closing-keyword scan is line-based, so a keyword and its reference split across a newline are invisible to it, and twice the scan and the publish went out in one invocation so the scan gated nothing. Both instances were caught and the published text corrected. Sweep ran: `archive_plan_sessions.py --target-lines 400` moved one block to `kit-handoff-history.md` (445 -> 383). `check_doc_budget.py` now reports the handoff inside budget; the friction log is still over and needs `triage-friction-log`, which wants tracker writes and operator approval. * fix: cut the round-count claim, and restore the demoted block's Next line Panel round 1, both lenses, converging on the same two. **The headline claim was wrong about one of its own three members (HIGH).** It said a mechanism kept drawing HIGH findings "round after round" three times, and that withdrawing it ended the loop each time. Two fit. `#230` does not: it drew HIGHs once, in its only round, before any fix existed — there was no loop for a withdrawal to end, and the block's own Learned bullet said so two paragraphs later. It was also a hand-written count of this session's own work, which `wrap-up.md` step 3 forbids in those words, for exactly the reason that applied here: a recount found it wrong. Cut rather than corrected, per the record-prose rule that a HIGH imprecision is deleted or shortened and never amended into a more careful figure. What is left is the invariant with the tally removed, and the two withdrawals named as pointers. **The demoted block lost its own `▶ Next:` line, against this file's own precedent (regression).** Every earlier session block still carries the starter it ended with; this was the first demotion to strip one, and `wrap-up.md` authorises adding a starter to the newest block, not removing one elsewhere. Restored. Left unfixed and logged on the PR, all sub-HIGH imprecision in record prose, which under this PR's declared criterion is logged rather than repaired: `#233` described as three withdrawn attempts when its third round shipped; CodeRabbit's "Major" findings folded into "HIGHs" in the `#230` sentence; and a loose `#143` adjacency in the friction-log bullet. `check_doc_budget.py` still reports the handoff inside budget. * docs: record the cs-toolkit upgrade dry run, and repoint Next at the adapter decision The `▶ Next:` argued for `#231` before the cs-toolkit vendoring. A dry run of `/upgrade` against a throwaway copy of cs-toolkit, run after that line was written, supersedes it. The upgrade **succeeds**: all 32 kit files install at `scripts/devkit/`, `init.sh` migrates in place without touching an in-use narrative doc, the pre-push hook installs, and `kit_doctor` reports 32 unchanged, 0 differ, 0 missing. That clean result is the finding rather than the reassurance — all six of cs-toolkit's `.claude/commands/` adapters are forks that reference no shared workflow doc, Step 4 instructs the operator to keep them, and no `.claude/` path is in `KIT_OWNED` so nothing can report the divergence. The engines and the doctrine are refreshed; the surface an agent actually invokes is not. Step 5's prescribed verification then reports `no tests ran`, because test files never reach an adopter. Filed as `#236`. Two known issues also got live occurrences from the same run rather than new tickets: `#51` (an older kit labelled "likely LOCAL EDITS" — cs-toolkit's copy is missing every feature the kit has added, so it is unambiguously older) and `#93` (the slug mismatch means the kit's `wrap-up` skill installs beside the 160-line fork instead of replacing it). `▶ Next:` now points at the adapter decision. `#231` is not dropped — it gates `#6`'s vendoring, which is novel engine work where this session's review costs actually landed. A file copy has no mechanism to invent, so it does not gate the upgrade. `check_doc_budget.py` reports the handoff inside budget. * docs: put the demoted block's starter where it actually belongs, and cut a claim to what was measured Panel round 2. **A fix I reported as landed had not (adversarial, HIGH).** Round 1 found the demoted session block had lost its own `▶ Next:` line, against this file's convention that every block keeps the starter it ended with. Round 1's commit message says "Restored." It was not: the line went into the *Latest* block beside that block's own starter, and round 2's commit then deleted both. So the defect survived two commits while a published commit message asserted it was repaired. The line is now inside the demoted block, verified by position between the two headings rather than by grep alone. The lenses split on this. Correctness read the collapse to a single `▶ Next:` as a correction toward the doctrine's "one obvious next thing". That rule governs adding a starter to the LATEST block; it says nothing about how many the file carries, and every session block in this file has kept its own. Adversarial is right and its finding is the one acted on. **A claim covered six adapters and the measurement covered four (both lenses).** Cut to what was measured rather than amended, per "shorten, don't correct". Correctness supplied the better reason: the kit ships shared workflow docs for only four workflows, so `triage-friction-log` and `post-merge-systemize` never had one to diverge from — "references no shared workflow" was never applicable to those two. A correction comment goes on `#236`, whose body generalises the same way. `check_doc_budget.py` reports the handoff inside budget.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
The doctrine already carries the prevention:
PR #225 measured that rule failing, and — more usefully — measured what happens
next, which the doctrine had nothing to say about. Once the mechanism is already
in the diff and drawing HIGHs, the default is to patch it. That is what turned a
two-round PR into a five-round one.
Only round 1 reviewed the change. Round 1's second HIGH was real — the derived
graph missed
lib/repo_root.sh, which two shell enginessource, sokit_doctorcalled a broken tree clean at exit 0. The fix was a bashsourcescanner: a new mechanism, prompted by a finding, which the filing rule says to
file rather than build. It was built, and every HIGH in rounds 2–4 was inside it,
each a defect in the previous round's fix for it.
What this adds
The recovery half. The signal is not a round count — it is a fix that itself
draws a HIGH, twice, in the same mechanism. Past that point, patching is
evidence-free: each patch is a fresh guess at a problem the previous guess
misread. Withdraw the mechanism, file it with what defeated it, and let the
removing round be the last. Removal is the cheapest round available because it
deletes surface instead of adding more for the next round to find — round 5 above
reviewed 90 insertions against 560 deletions and found nothing.
The threshold must be declared before the 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.
Placement, and why it is short
#213stays open on this file's length, so an addition has to earn its lines.The rule goes into the paragraph that already carries the filing rule rather
than opening a section — 14 lines. The 40-line measurement goes to
fallback-review-panel-evidence.md, which is the file#213's split created forexactly this. Net effect on what must be read before every panel: 14 lines.
Verification
## The contract every lens gets— the onlysection
panel_prompt.pyparses.contract()still returns all 13 items,so no launch prompt changes and
#167's stable numbering is untouched.make test→ 699 passeduv run scripts/kit_doctor.py→ 32 unchanged, 0 differ, 0 missing, 0 unknown,exit 0 (both files are
KIT_OWNED; the manifest is regenerated in this commit)Review cost
Declared before round 1: both configured lenses over the full diff,
--carry-forwardempty. This is executed prose but not a gate or send path —second class: act on HIGH and on any lens-marked regression; a sub-HIGH
imprecision in record prose is logged rather than fixed, which produces no commit
and leaves the receipt standing. Only-loggable findings ⇒ merges at round 1.
Withdrawal threshold, declared now — the rule applied to itself: if a round
finds a HIGH in the rule being added, the rule is filed as a tracker issue and
dropped from this PR rather than patched.
#213,#228, and#163's missing cost measurement all stay open.Summary by CodeRabbit