feat(gatorwalk-factory): example review prompts define severity and scale to the change (swamp-club #2781) #392

Merged

seth merged 1 commit from cue/2781-gatorwalk-factory-review into main 2026-09-30 21:30:20 +00:00

Conversation 0 Commits 1 Files changed 6 +133 -19

seth commented 2026-09-30 21:27:32 +00:00

Owner

Copy link

Fixes swamp-club #2781.

In the 2026-09-29 trial, plan review ran five rounds on a one-character change. Rounds 2 to 4 each raised one high finding about manual-verification logistics, and each high finding sent the plan round again with no person involved. The review prompt said "do not soften them" and gave no bar.

What changed

• Rubric in the example factory definitions, not the engine. The plan-review and code-review prompts in build-swamp-extension.yaml and starter.yaml now say:
  • critical or high only if the plan or change would ship a defect, lose data, break a stated rule, or make the declared checks meaningless;
  • gaps in process, logistics or manual verification that the automated checks already cover are medium at most;
  • judge against the size of the change, and give the smallest adequate fix;
  • do not soften the severity of a real defect.
• Approving carries findings. starter.yaml's implement stage now injects plan-review, as build-swamp-extension.yaml already did. Both examples' plan-review and revise descriptions, the walkthrough's approval question, and a new paragraph in driving.md (Human stops) say that approving with nothing blocking takes the open medium and low findings into implement. They also say that revise is for changing the plan itself.
• examples_test.ts pins both: every findings stage prompt in the two examples carries the rubric phrases and no bare "do not soften them", and implement injects plan-review. starters.ts is regenerated.
• Not touched: the engine, the definition schema, swamp-club-swamp-extensions.yaml, testdata/factories, and agent-constraints/adversarial-dimensions.md.

Manual replay (done-when item 3)

I took plan v2 of trial work item cue-er7koww5 from ~/src/factory and reviewed it against the trial's plan-review prompt, with the new rubric paragraph in place of "Record findings with severities; do not soften them". A dispatched subagent did the review over cue's source at 7fbcda4, the commit round 2 reviewed.

Result: the hand-check-cannot-run issue (trial PR2-1, rated high) came back as R-1, rated medium. There were no critical or high findings, so the plan would have stopped for the person's approval instead of reworking automatically. This is one sample from a reviewer that can vary between runs.

Prompt sent

You are an adversarial reviewer. Try to refute this plan for cue
(<scratch>/cue-7fbcda4):
Add a keyboard shortcut to reach the Board from anywhere: pressing `b` (when not typing in a box, including a select) navigates to /board. It rides on the rail's existing key machinery (CueWeb.Rail's `.Keys` colocated hook and its `rail_key` handle_event hook), so it works on every page that has the rail, follows the same gmail/github plain-letter arrangement as j/k/n/p/?, and is listed in the `?` help panel. The place rows (Queue, Board, Done) get a leading fixed-width key slot like the session rows' numbers, holding a faint `b` on Board and empty on the other two so their icons stay aligned. Pressing `b` while already on the Board, with or without filter query params, does nothing, so URL-held filters survive. Revised for plan-review PR-1..PR-6: work happens in a worktree from main, checks compile with MIX_ENV=test, SELECT counts as typing, help tests assert real text, the hint's placement is pinned and tested, and the no-op is tested with filters.
Check its steps against the code and cue's CLAUDE.md rules, and its
testing strategy against the risks.
Rate a finding critical or high only if the plan would ship a
defect, lose data, break a stated rule, or make the declared checks
meaningless. Gaps in process, logistics or manual verification that
the automated checks already cover are medium at most. Judge the
plan against the size of the change: do not ask for verification
machinery bigger than the change, and give the smallest adequate fix
for each finding. Report findings with severities, and do not soften
the severity of a real defect.

---

Read this product:
- plan: <scratch>/replay/plan-v2.json

Write your result as JSON, one file per product, holding the
payload and nothing else. These files are the only thing you may
write.
- artifact plan-review: write it to <scratch>/replay/plan-review.json
  Its schema: {"findings": [{"id": string, "severity": "critical"|"high"|"medium"|"low", "description": string}]}

Findings returned

• R-1 (medium): The manual check of the JS half (the /^[1-9bjknp?]$/ filter and the SELECT guard, which ExUnit cannot reach) says to run it 'after restarting the daemon'. But step 1 puts the work in a worktree, and the running daemon serves /home/seth/src/cue (the main checkout). Restarting that daemon loads main's code, not the branch, so the check would run against code without the change. The only verification of the JS changes would then exercise the wrong code: pressing b does nothing, and the SELECT test passes whether or not the change is there. Smallest fix: say that the manual check runs after the branch lands in the checkout the daemon serves, and restart it then. Do not run a second server from the worktree, because a second BEAM would run the coordinator and substrate against the same rmux panes.
• R-2 (low): The plan does not keep the key list in docs/status.md current. The 'Moving between sessions' bullet (around lines 346-354) lists 1-9, j/k and n/p as the plain keys, and CLAUDE.md sends readers to docs/status.md first for where the build actually is. Smallest fix: add one clause there saying b reaches the board, or add a step that edits the file.
• R-3 (low): The 'filter is still shown' assertion for /board?project= is underspecified. The board draws a project chip only for projects that Board.projects() returns, which means pooled Lab issues carrying that project must exist. The rail_test setup creates none, and the seen/2 fixture lives in board_live_test.exs. Without such an issue the assertion either cannot be written or matches the wrong text. The part that carries the weight is already sound: render_hook returns HTML instead of {:error, {:live_redirect, _}}, which proves nothing navigated, and so proves the query survived. Smallest fix: either seed one pooled issue with that project, or assert on the aria-pressed chip, or drop the filter assertion and rely on the no-redirect assertion.
• R-4 (low): The <.number>-style slot: the existing number/1 component declares attr :n, :integer. If the implementation reuses <.number n={key}> with the string "b", Phoenix does not check the attr type of a dynamic value, so it compiles quietly against a mistyped attr. A literal n="b" would fail --warnings-as-errors. Smallest fix: either change the attr to :any and update its comment ('a number or a key'), or render a separate span with the same classes, as the plan's wording suggests.
• R-5 (low): Counting SELECT in typing() also changes Escape and Ctrl-[. Before the change, Escape on a focused select pushed 'Escape' to the server (closing help or focusing the composer). After it, Escape only blurs the select. This is probably the desired behaviour, but the plan's summary describes the SELECT change only for 'every rail key' and says nothing about Escape. Smallest fix: one line in the hook comment or plan so the change is deliberate. No test is needed.

Verification

The attestation is e40300a6-b6e9-47e2-b29c-e4694b9afabb, for commit 1833f14736. verify-build 29d79025 and verify-reviews ccbb158f both passed; code-review and adversarial-review both returned pass.

🤖 Generated with Claude Code

Fixes swamp-club #2781. In the 2026-09-29 trial, plan review ran five rounds on a one-character change. Rounds 2 to 4 each raised one high finding about manual-verification logistics, and each high finding sent the plan round again with no person involved. The review prompt said "do not soften them" and gave no bar. ## What changed - **Rubric in the example factory definitions, not the engine.** The plan-review and code-review prompts in `build-swamp-extension.yaml` and `starter.yaml` now say: - critical or high only if the plan or change would ship a defect, lose data, break a stated rule, or make the declared checks meaningless; - gaps in process, logistics or manual verification that the automated checks already cover are medium at most; - judge against the size of the change, and give the smallest adequate fix; - do not soften the severity of a real defect. - **Approving carries findings.** `starter.yaml`'s implement stage now injects `plan-review`, as `build-swamp-extension.yaml` already did. Both examples' plan-review and `revise` descriptions, the walkthrough's approval question, and a new paragraph in `driving.md` (Human stops) say that approving with nothing blocking takes the open medium and low findings into implement. They also say that `revise` is for changing the plan itself. - `examples_test.ts` pins both: every findings stage prompt in the two examples carries the rubric phrases and no bare "do not soften them", and implement injects plan-review. `starters.ts` is regenerated. - Not touched: the engine, the definition schema, `swamp-club-swamp-extensions.yaml`, `testdata/factories`, and `agent-constraints/adversarial-dimensions.md`. ## Manual replay (done-when item 3) I took plan v2 of trial work item `cue-er7koww5` from `~/src/factory` and reviewed it against the trial's plan-review prompt, with the new rubric paragraph in place of "Record findings with severities; do not soften them". A dispatched subagent did the review over cue's source at 7fbcda4, the commit round 2 reviewed. **Result:** the hand-check-cannot-run issue (trial PR2-1, rated **high**) came back as R-1, rated **medium**. There were no critical or high findings, so the plan would have stopped for the person's approval instead of reworking automatically. This is one sample from a reviewer that can vary between runs. <details><summary>Prompt sent</summary> ```text You are an adversarial reviewer. Try to refute this plan for cue (<scratch>/cue-7fbcda4): Add a keyboard shortcut to reach the Board from anywhere: pressing `b` (when not typing in a box, including a select) navigates to /board. It rides on the rail's existing key machinery (CueWeb.Rail's `.Keys` colocated hook and its `rail_key` handle_event hook), so it works on every page that has the rail, follows the same gmail/github plain-letter arrangement as j/k/n/p/?, and is listed in the `?` help panel. The place rows (Queue, Board, Done) get a leading fixed-width key slot like the session rows' numbers, holding a faint `b` on Board and empty on the other two so their icons stay aligned. Pressing `b` while already on the Board, with or without filter query params, does nothing, so URL-held filters survive. Revised for plan-review PR-1..PR-6: work happens in a worktree from main, checks compile with MIX_ENV=test, SELECT counts as typing, help tests assert real text, the hint's placement is pinned and tested, and the no-op is tested with filters. Check its steps against the code and cue's CLAUDE.md rules, and its testing strategy against the risks. Rate a finding critical or high only if the plan would ship a defect, lose data, break a stated rule, or make the declared checks meaningless. Gaps in process, logistics or manual verification that the automated checks already cover are medium at most. Judge the plan against the size of the change: do not ask for verification machinery bigger than the change, and give the smallest adequate fix for each finding. Report findings with severities, and do not soften the severity of a real defect. --- Read this product: - plan: <scratch>/replay/plan-v2.json Write your result as JSON, one file per product, holding the payload and nothing else. These files are the only thing you may write. - artifact plan-review: write it to <scratch>/replay/plan-review.json Its schema: {"findings": [{"id": string, "severity": "critical"|"high"|"medium"|"low", "description": string}]} ``` </details> <details><summary>Findings returned</summary> - **R-1 (medium)**: The manual check of the JS half (the /^[1-9bjknp?]$/ filter and the SELECT guard, which ExUnit cannot reach) says to run it 'after restarting the daemon'. But step 1 puts the work in a worktree, and the running daemon serves /home/seth/src/cue (the main checkout). Restarting that daemon loads main's code, not the branch, so the check would run against code without the change. The only verification of the JS changes would then exercise the wrong code: pressing b does nothing, and the SELECT test passes whether or not the change is there. Smallest fix: say that the manual check runs after the branch lands in the checkout the daemon serves, and restart it then. Do not run a second server from the worktree, because a second BEAM would run the coordinator and substrate against the same rmux panes. - **R-2 (low)**: The plan does not keep the key list in docs/status.md current. The 'Moving between sessions' bullet (around lines 346-354) lists 1-9, j/k and n/p as the plain keys, and CLAUDE.md sends readers to docs/status.md first for where the build actually is. Smallest fix: add one clause there saying b reaches the board, or add a step that edits the file. - **R-3 (low)**: The 'filter is still shown' assertion for /board?project=<x> is underspecified. The board draws a project chip only for projects that Board.projects() returns, which means pooled Lab issues carrying that project must exist. The rail_test setup creates none, and the seen/2 fixture lives in board_live_test.exs. Without such an issue the assertion either cannot be written or matches the wrong text. The part that carries the weight is already sound: render_hook returns HTML instead of {:error, {:live_redirect, _}}, which proves nothing navigated, and so proves the query survived. Smallest fix: either seed one pooled issue with that project, or assert on the aria-pressed chip, or drop the filter assertion and rely on the no-redirect assertion. - **R-4 (low)**: The `<.number>`-style slot: the existing number/1 component declares attr :n, :integer. If the implementation reuses <.number n={key}> with the string "b", Phoenix does not check the attr type of a dynamic value, so it compiles quietly against a mistyped attr. A literal n="b" would fail --warnings-as-errors. Smallest fix: either change the attr to :any and update its comment ('a number or a key'), or render a separate span with the same classes, as the plan's wording suggests. - **R-5 (low)**: Counting SELECT in typing() also changes Escape and Ctrl-[. Before the change, Escape on a focused select pushed 'Escape' to the server (closing help or focusing the composer). After it, Escape only blurs the select. This is probably the desired behaviour, but the plan's summary describes the SELECT change only for 'every rail key' and says nothing about Escape. Smallest fix: one line in the hook comment or plan so the change is deliberate. No test is needed. </details> ## Verification The attestation is `e40300a6-b6e9-47e2-b29c-e4694b9afabb`, for commit 1833f147361f01caf4604f35dfbffb96590abbaf. verify-build `29d79025` and verify-reviews `ccbb158f` both passed; code-review and adversarial-review both returned pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

seth added 1 commit 2026-09-30 21:27:32 +00:00

feat(gatorwalk-factory): example review prompts define severity and scale to the change (swamp-club #2781) ... 1833f14736

In a trial, plan review ran five rounds on a one-character change: a reviewer
told not to soften findings, with no bar, rated manual-verification logistics
as high, and each high finding sent the plan round again with no person
involved.

The review prompts in build-swamp-extension.yaml and starter.yaml now carry a
severity bar (critical or high only for a shipped defect, lost data, a broken
rule or meaningless checks; logistics the automated checks cover are medium at
most) and ask for the smallest adequate fix in proportion to the change. "Do
not soften" is kept for the severity of real defects. The rubric lives in the
factory definitions, not the engine.

starter's implement stage now injects plan-review, so approving carries the
open medium and low findings into implement, as build-swamp-extension already
did. Both examples, the walkthrough and the skill's human-stop guidance say so,
and that revise is for changing the plan itself. examples_test.ts pins the
rubric and the inject; starters.ts is regenerated.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

seth merged commit 0d7bca40cd into main 2026-09-30 21:30:20 +00:00

seth deleted branch cue/2781-gatorwalk-factory-review 2026-09-30 21:30:21 +00:00

seth referenced this pull request from a commit 2026-09-30 21:30:21 +00:00

feat(gatorwalk-factory): example review prompts define severity and scale to the change (swamp-club #2781) (#392)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

swamp-club/swamp-extensions!392

No description provided.