Rename the send-back state from reject to needs-work — label through verb through docs - #231
Merged
Merged
Conversation
"needs work" is a more standard tag than rejected, and it is the accurate
name: the state is a transient send-back, not a verdict against the work.
A send-back carries a work order, the producer reworks that same PR and
branch, and the push is the transition — the head moves, the verdict stops
being current, and the next vet clears the label. "Rejected" named a
terminal judgement the FSM does not have.
The spelling changes together, because a verb that writes a differently
named label is the drift this repo keeps paying for:
- labels `ai:reject` -> `ai:needs-work`, `human:reject` -> `human:needs-work`
- the ruling verb on `human-rule` / `human-rule-issue`, which follows from
the label sets: `ruling_verb` derives the verb from the label's namespace
suffix, so the vocabulary moves with the constant rather than beside it
- the vetter verdict `record-verdict … needs-work` and the `Reviewed <sha>:
needs-work — …` comment it writes
- the slash command, `plugins/human-fsm/commands/needs-work.md`, with the
plugin manifest and marketplace listing bumped to 0.15.0 in lockstep
- `counts.reject` -> `counts.needsWork`, the vetter-verdicts lane state, and
the `migrate-reject` one-shot -> `migrate-needs-work`, which after this
rename moves `human:needs-work` to `ai:needs-work` and has to say so
- README FSM diagram and text, CLAUDE.md, campaign-prompt.txt,
review-prompt.txt, plugin READMEs
HISTORY CONTINUITY. `STATE_NEEDS_WORK` carries `hist: Some("needsWork")` and
`hist_fold: &["reject", "humanReject", "relink"]`. `reject` is this state's
own pre-rename key, and it is the load-bearing entry: the committed
`human-queue-history.jsonl` holds 216 samples under it, the newest measured
2026-08-07T09:31:05Z, and ZERO under `needsWork`. Without the fold the
series would not merely lose its past — it would start empty and render the
state as newly created, which is the #130 defect `hist_fold` exists for.
`human-queue-history.jsonl` is NOT rewritten; the old lines are real
measurements of the machine as it then was.
`the_needs_work_series_draws_every_spelling_the_state_has_had` pins it,
structurally off the descriptor table (so it cannot pass vacuously in the
flake sandbox where the repo-root read is filtered out) and then against the
real committed history.
Deliberately NOT renamed: the close-candidate flag verdicts (`CC_VERDICTS`,
`CcGate::RejectedStillFlagged`, `clear-stranded-rejected`,
`record-close-candidate-verdict … reject`). Those judge a FLAG, write no
label and create no state, so they are not the send-back state. Nor is the
"refuse" sense (`ResultRejected`, `iso_to_epoch_ms_rejects_malformed`, the
`rejected` rate-limit status, a non-fast-forward push), nor `reworked-reject`
and `humanReject`, which name things that are gone or historical and would
become lies under the new spelling.
Closes #230
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Main is 31 commits ahead of this branch's base and every one of the five PRs that landed in between touches what this renames, so main wins on structure and this branch wins on the spelling. Where a hunk carried both, the merged code takes both: 224's verb->target tables and typed worklist row, 223's state descriptors, 225's deleted `ai:blocked-deploy`, and 226/227's `/nd` and `/nm`, all spelled `needs-work`. The rename had to reach code that did not exist when this branch was written: - `NextAction::ReworkReject` -> `ReworkNeedsWork`, string `rework-reject` -> `rework-needs-work`. The action vocabulary is DERIVED from the enum (#224), so `ALL_ACTIONS`, `ACTIONABLE_ACTIONS` and `action_rank` follow from the variant. - `RejectState` -> `NeedsWorkState` with `NotRejected` -> `NotNeedsWork`, `reject_state` -> `needs_work_state`, `reject_instruction` -> `needs_work_instruction`, and the `PrSignals.reject` field -> `needs_work` — 224's typed row that replaced the producer prompt's raw label search. - The producer prompt's step 3, which 224 rewrote around that typed row. - review-prompt's #219 sentence, `design.md`, and 226/227's `nd.md` / `nm.md`. Deliberately NOT renamed, because a blind sweep would have corrupted them: the close-candidate FLAG verdicts (`CC_VERDICTS`, `cc_verdict_plan`, `cc_gate`, `CcGate::RejectedStillFlagged`, `/close-candidate <n> reject`) judge a flag and write no state; the "refuse" sense (`ResultRejected`, the rate-limit `rejected`, a non-fast-forward push); `reworked-reject`, a subcommand that is GONE; and `counts.humanReject`, which is both a kept series and a `histFold` key. History continuity is measured, not assumed. `STATE_NEEDS_WORK.hist_fold` keeps `"reject"` alongside `humanReject`/`relink`, and against the committed `human-queue-history.jsonl` (232 rollup lines) those three keys carry 218 samples each, newest 2026-08-07T11:31:03Z, while `needsWork` has none — so the whole drawn series comes through the fold. The 14 lines that fold nothing are the contiguous oldest prefix, predating the state itself. The history file is not rewritten; `the_needs_work_series_draws_every_spelling_the_state_has_had` pins this structurally first so it cannot pass vacuously in the flake sandbox. Three things the merge markers did not cover and the suite did: - `every_shipped_command_carries_its_frontmatter` — the command set is compared SORTED, and `needs-work` sorts between `nd` and `nm` where `reject` sorted first. Carrying the old order across would have been green-looking nonsense. - `a_cc_verdict_carries_the_machines_citation_read_under_the_vetters_note` — the close-candidate comment's expected string had been swept into `needs-work`; the CC verb is not this state. - `the_vetter_prompt_makes_the_note_say_what_it_read` — same, on a prompt bullet about rejecting a FLAG. Counts re-derived from merged reality rather than carried across: 22 tools declared, the shipped command set is 8 files, and no count assertion was retyped. Gates at this commit: `cargo test` 1052+3+9+47+8+7+4 passed 0 failed; CI-exact `rainix-rs-static` at rainix 53e96a7 with the changed file touched first; `pre-commit run --all-files` all hooks passed; `nix build .#pr-review-report`. Note for the record: this branch renames `migrate-reject` to `migrate-needs-work` as a permanent subcommand. The live labels were already migrated before this PR reached that step, so the verb is spent — under the standing ruling that a migration is an execution vehicle, it should come out, which is a separate change from this merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WalkthroughThe PR renames reject-oriented states, commands, labels, and workflow terminology to needs-work. It updates FSM documentation, the human-fsm plugin, campaign and review prompts, and plugin version metadata. ChangesNeeds-work terminology migration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.45.0)pr-review-report-rs/src/main.rsast-grep timed out on this file 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 `@campaign-prompt.txt`:
- Line 58: Update the producer instructions in step 3d and lines 78–82 to route
every GitHub input/output and pipeline transition through typed pr-review-report
subcommands, including fetching baseRefName, resolving conflicts, pushing
screenshot branches, posting comments, and creating or reading migration issues.
Enumerate these subcommands in the producer profile before referencing them, and
remove raw gh/git commands so all state changes are recorded and guarded by FSM
transitions.
In `@plugins/human-fsm/README.md`:
- Line 19: Update the /needs-work documentation row to distinguish the issue
path from the pull-request path: retain ai:needs-work for pull requests and
explicitly state that human-rule-issue writes human:needs-work, matching the
behavior documented in needs-work.md.
In `@review-prompt.txt`:
- Around line 15-16: Update the requirement governing needs-work note contents
near the review verdict rules so it explicitly permits review-blocking evidence
failures, including missing or false QA evidence and missing screenshots,
alongside code defects. Preserve the existing mandatory evidence gates and their
specified notes, ensuring valid evidence failures can produce honest needs-work
notes without requiring a misleading defect description.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: 9a7385e1-523b-4aed-8630-19310427da87
📒 Files selected for processing (15)
.claude-plugin/marketplace.jsonCLAUDE.mdREADME.mdcampaign-prompt.txtplugins/human-fsm/.claude-plugin/plugin.jsonplugins/human-fsm/README.mdplugins/human-fsm/commands/close-candidate.mdplugins/human-fsm/commands/design.mdplugins/human-fsm/commands/nd.mdplugins/human-fsm/commands/needs-work.mdplugins/human-fsm/commands/nm.mdplugins/human-fsm/commands/nr.mdpr-review-report-rs/src/main.rsreview-prompt.txtreview-run.sh
| (iv) MIGRATION — the deploy-shaped case, triggered by EITHER (a) a red prod-pin / `testProdDeploy*` / deploy-pinned-constant check (REPLACES (ii)/(iii) for it: there is no code fix to push), OR (b) a GREEN PR carrying a legacy `REQUIRES redeploy at land` marker in its body or a trusted note. Under the split deploy/abstract release lifecycle NO merge waits on a deploy (#162): deploy repos freeze per-tag records and publish on `rainix-tag-release`, the on-chain deploy is a separate manual workflow dispatch decoupled from publishing, and library repos never deploy — so either trigger firing is evidence of exactly ONE thing: THIS REPO HAS NOT MIGRATED to the split lifecycle. The PR is blocked on that migration, not on any deploy, and you deploy NOTHING, EVER (there is no producer deploy path; deploys are a human's decoupled manual dispatch). Route it: (1) find the repo's lifecycle-migration issue/PR (search the repo's open issues/PRs for the split-lifecycle / release-workflow migration); (2) if none exists, FILE it — `gh issue create` in that repo, assignee {{ASSIGNEE}}, shaped as the audit skill's lifecycle-split finding (the repo keeps a single "current" deployment pin that premerge CI tests against, so every bytecode-changing PR goes red until an out-of-band deploy; the fix is the split lifecycle: per-tag frozen deploy records + `rainix-tag-release` publishing, with the on-chain deploy a decoupled manual dispatch) — this is the ONE sanctioned `gh issue create` and the only exception to step 4's no-new-issues posture; (3) TRANSITION the PR: `pr-review-report flag-blocked-on <owner/repo> <n> "repo not migrated to the split release lifecycle: <the red pin check / the legacy marker>" --blocked-by <owner/repo#n of the migration issue/PR>` — the typed dep makes clearance automatic: the vetter clears the flag the run after the migration lands and the PR re-enters vetting fresh. An APPROVED PR is human-sacred and cannot be flagged — report it as blocked-on-migration in the run summary instead (still do (1)/(2) so the dep exists). DEDUP: if the PR already carries `ai:blocked-on` naming the migration, it is parked — do not re-flag. There is no blocked-deploy state and no producer deploy any more (#162, deleted outright by #221): `flag-blocked-deploy` refuses with exactly this replacement, and `ai:blocked-deploy` is not a state the machine models — a PR still wearing the string is an unmodelled label, so treat it as any other PR and route it by the rules above. Still NEVER merge; landing stays interactive on the human's explicit word. | ||
| 3c. BACKFILL MISSING SCREENSHOTS on your OWN open UI PRs (still existing-work — after 3b's red fixes, before step 4 opens anything new): a UI PR with no screenshot is, like a red PR, in-flight work that is NOT review-ready (step 5), so finish it before starting new work. YOUR LIST IS STEP 2a's `worklist` ROWS WHOSE `nextAction` IS `screenshot-3c`, never a path list written here: `worklist`'s `is_ui_path` is the ONE definition of which paths fall under this requirement — raindex's `packages/webapp` and `packages/ui-components`, everything under a `site/` dashboard tree INCLUDING the `health.json` its panels draw, and every `.svelte`/`.css`/`.html` file wherever a repo keeps it (`cyclo.site` keeps its components in `src/lib/components/`, which is where both of #140's PRs lived) — and a second enumeration in this step is a second definition, free to drift from it while reading as though it agrees. The row's `markers.uiTouch` tells you which answer routed it: `yes` a path under the requirement is in the changed list, `unknown` the changed list could not be ruled out — BOTH route here, because `unknown` means nothing ruled UI out, not that there is none. For each, judge FROM THE DIFF whether it changes RENDERED OUTPUT a user sees (per step 5: `.svelte` markup, a visible component/element, user-visible styling, a non-`.svelte` service whose output is directly displayed — chart data / labels / formatters — OR a `site/` dashboard page's visible markup/section). A change that ONLY touches `<script>`/store/load/pure-logic with NO visible effect does NOT need a screenshot — skip it. If it needs one, render + post it via step 5's harness — a row that reached this step has neither a shot nor a waiver on it already, which is exactly what routed it here (`markers.screenshotSettled` is `false`). BACK-OFF: at most ONE screenshot attempt per PR per run; the screenshot comment IS your cross-run marker, and a PR carrying one never routes here again — a trusted comment embedding ANY `pr-screenshots/…png` on the PR settles it, whatever step 5 named the file, so do not go looking for one particular filename. If after a focused attempt the component genuinely cannot be rendered in the harness, ASK THE HUMAN FOR THE HARNESS: `pr-review-report flag-design <owner/repo> <n> "unrenderable-render: <the stack, e.g. cyclo.site SvelteKit/Tailwind> — needs a harness that can mount it"`. A stack the harness has NEVER been able to render is not a passing outage, it is a standing gap only a person can close, so it is a design question. Its `ai:design` label surfaces it in `human-queue` and is the cross-run marker; skip that PR on future runs (never re-attempt a PR already carrying its screenshot or that state). | ||
| 3d. RESOLVE MERGE CONFLICTS on your OWN open PRs so they stay landable for the human (still existing-work — after 3b's reds and 3c's screenshots, before step 4 opens anything new). The merge cron does NOT resolve conflicts: a PR whose `mergeStateStatus` is `DIRTY`/`CONFLICTING` (or one the merge cron logged as NEEDS-REBASE / un-mergeable, e.g. BEHIND in a repo that requires up-to-date branches) is STUCK until its conflicts with the base are resolved, and that is YOUR job — overlapping PRs (e.g. many test PRs touching the same file) routinely go conflicting as siblings merge ahead of them. YOUR LIST IS A TYPED ROW, NOT A SEARCH: step 2 already computed it — every fleet row whose `nextAction` is `conflict-3d` is one of your open PRs whose `mergeState` is `DIRTY` or `BEHIND`, and the row carries that `mergeState` so you never re-ask which it is. Do NOT re-derive the set with `gh search prs`+`gh pr view`: that is one search plus a fetch PER PR to rebuild a classification `state-load` already handed you, and a second read of the fleet is a second answer free to disagree with the first — the same defect as step 3's old reject query. The ONE detail this step needs that the row does not carry is `baseRefName`, the branch to merge in; fetch it with `gh pr view <n> -R <owner>/<repo> --json baseRefName` for the PRs you ACTUALLY work, never for the fleet (step 2a's rule: fetch a single PR only for a detail the row does not carry). For each `conflict-3d` row, in order: | ||
| 3d. RESOLVE MERGE CONFLICTS on your OWN open PRs so they stay landable for the human (still existing-work — after 3b's reds and 3c's screenshots, before step 4 opens anything new). The merge cron does NOT resolve conflicts: a PR whose `mergeStateStatus` is `DIRTY`/`CONFLICTING` (or one the merge cron logged as NEEDS-REBASE / un-mergeable, e.g. BEHIND in a repo that requires up-to-date branches) is STUCK until its conflicts with the base are resolved, and that is YOUR job — overlapping PRs (e.g. many test PRs touching the same file) routinely go conflicting as siblings merge ahead of them. YOUR LIST IS A TYPED ROW, NOT A SEARCH: step 2 already computed it — every fleet row whose `nextAction` is `conflict-3d` is one of your open PRs whose `mergeState` is `DIRTY` or `BEHIND`, and the row carries that `mergeState` so you never re-ask which it is. Do NOT re-derive the set with `gh search prs`+`gh pr view`: that is one search plus a fetch PER PR to rebuild a classification `state-load` already handed you, and a second read of the fleet is a second answer free to disagree with the first — the same defect as step 3's old send-back query. The ONE detail this step needs that the row does not carry is `baseRefName`, the branch to merge in; fetch it with `gh pr view <n> -R <owner>/<repo> --json baseRefName` for the PRs you ACTUALLY work, never for the fleet (step 2a's rule: fetch a single PR only for a detail the row does not carry). For each `conflict-3d` row, in order: |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Route every producer GitHub operation through a tool.
Line 58 still instructs the producer to use gh pr view for GitHub input. Lines 78-82 also instruct the producer to push a screenshot branch, post comments, create a migration issue, and read issue state with raw GitHub commands.
These operations bypass tested FSM transitions. They can create state that pr-review-report does not record or guard. Add typed subcommands and enumerate them in the producer profile before keeping these instructions.
As per coding guidelines: producer prompts must perform all GitHub input and output through pr-review-report subcommands and must not use raw gh or git as pipeline transitions.
Also applies to: 78-82
🤖 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 `@campaign-prompt.txt` at line 58, Update the producer instructions in step 3d
and lines 78–82 to route every GitHub input/output and pipeline transition
through typed pr-review-report subcommands, including fetching baseRefName,
resolving conflicts, pushing screenshot branches, posting comments, and creating
or reading migration issues. Enumerate these subcommands in the producer profile
before referencing them, and remove raw gh/git commands so all state changes are
recorded and guarded by FSM transitions.
Source: Coding guidelines
| | `/close-candidate <owner/repo#n> reject "…"` | `record-close-candidate-verdict … reject` — drop the flag, back to the producer (issue-only) | | ||
| | `/reject <owner/repo#n> "…"` | `human-rule … reject --rework` / `human-rule-issue … reject --rework` — the send-back: `ai:reject` (PR) plus the trusted `Rework note` work order, one call, pinned to the head sha or the issue. `--rework` is REQUIRED on either subject; there is no parked reject | | ||
| | `/design <owner/repo#n> "…"` | `human-rule … design --rework` / `human-rule-issue … design --rework` — the answer, delegated as a work order: `ai:reject` on a PR (the same send-back a rejection is, #219), comment-only on an issue. `--rework` is REQUIRED; there is no parked spelling | | ||
| | `/needs-work <owner/repo#n> "…"` | `human-rule … needs-work --rework` / `human-rule-issue … needs-work --rework` — the send-back: `ai:needs-work` (PR) plus the trusted `Rework note` work order, one call, pinned to the head sha or the issue. `--rework` is REQUIRED on either subject; there is no parked needs-work | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Document the issue-side state explicitly.
The /needs-work row names human-rule-issue but documents only ai:needs-work, which applies to pull requests. State that the issue path writes human:needs-work, consistent with plugins/human-fsm/commands/needs-work.md.
🤖 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 `@plugins/human-fsm/README.md` at line 19, Update the /needs-work documentation
row to distinguish the issue path from the pull-request path: retain
ai:needs-work for pull requests and explicitly state that human-rule-issue
writes human:needs-work, matching the behavior documented in needs-work.md.
Comment on lines
+15
to
+16
| - **QA GATE (mandatory):** a PR whose body lacks the QA-GUIDE.md section-8 evidence block, or whose claims in it do not hold, is `needs-work` with note "missing/false QA evidence" — regardless of how correct the diff looks. Correct-looking and unpinned is the failure mode this gate exists for. | ||
| - **SCREENSHOT GATE (mandatory):** a PR that changes RENDERED OUTPUT a user sees (`.svelte` markup, a visible component, user-visible styling in packages/webapp or packages/ui-components, a non-`.svelte` service whose output is displayed, or a `site/` dashboard page's visible markup) is NOT review-ready without VISUAL evidence: `needs-work` with note "UI change, no screenshot — not review-ready", UNLESS a trusted producer comment in `pr_context` embeds a `pr-screenshots/shots/…png` OR carries a VALID `screenshot pending (manual): <reason>`. **A WAIVER IS VALID ONLY WHEN ITS REASON IS A RENDER THAT WAS ATTEMPTED AND FAILED**, and the reason has to NAME THE ATTEMPT: WHAT WAS RUN (the component or page it mounted, the command) AND WHERE IT STOPPED (the throw, the gate, the fixture that was missing) — "mounted `RewardsCard.svelte` in the Vite harness, it throws without a `@tanstack/svelte-query` context", "loaded `site/index.html` in headless chromium, the changed section renders empty without a live `health.json`", "the harness reaches the connect-wallet screen and the signature dialog is unreachable headless". **A STATEMENT ABOUT THE WORLD IS NOT AN ATTEMPT:** "the stack has never rendered in it", "this stack is unsupported", "there is no harness for this repo" are true whether or not anybody TRIED, so they name no attempt and waive nothing — `needs-work` on the same ground as a claim about the output. And where the attempt shows a WHOLE STACK has no render path at all, the waiver is not the end of it: that is a standing gap only a person can close, so the verdict is `design` with that gap as the note (the producer's side raises `flag-design "unrenderable-render: …"` for the same reason) — never a `ready` that banks the waiver and leaves the gap unrecorded. **A JUDGEMENT ABOUT THE RENDERED OUTPUT IS NEVER A WHY-NOT:** "pixel-identical", "no visible effect", "cosmetic only", "the browser ignores this attribute anyway" are the CONCLUSIONS a render exists to support or refute, so they waive the evidence with the claim it was supposed to produce — and a change nobody TRIED to render is not pending, it is MISSING. That waiver is `needs-work` with note "screenshot waived on a claim about the render, not a failed attempt". Both of 2026-07-29's human send-backs are this shape: cyclo.site#431 waived on "rendered output is pixel-identical" while removing attributes from an input carrying a long conditional Tailwind class list (`[appearance:textfield]`, webkit spin-button resets) — identical is precisely what an unintended shift hides behind; cyclo.site#408 carried a screenshot-pending marker through THREE vetter passes, and the render nobody made would have shown in one look that its rewritten rewards card lists four epochs that have ALL ENDED — the defect all three verdicts missed. Judge whether a USER sees the change, not the file extension. The gate fires on a GUI CLAIM, not only a changed UI file: if the PR body or its QA evidence ASSERTS a rendered outcome ("renders/displays/shows X", "the toast now reads Y", "the modal shows both balances") — a claim about what a user SEES — that claim needs a screenshot backing it (or a waiver meeting the bar above); an unbacked GUI claim is `needs-work` with note "GUI claim, no visual proof". A grepped code path is not evidence a user sees the result — hold every GUI claim (producer close-candidate flags included, when you evaluate them) to a screenshot or a FAILED RENDER ATTEMPT, symmetric with the producer's step-7a GUI-claim rule. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Allow evidence failures to produce honest needs-work notes.
Lines 15-16 make missing or false QA evidence and missing screenshots valid needs-work grounds. Line 24 then requires every needs-work note to name a code defect. This conflict can force a misleading note or make a valid evidence repair unusable.
Change the requirement to cover review-blocking evidence failures as well as code defects.
Proposed wording
- - The `needs-work` note MUST name a specific code defect that would fail review EVEN IF CI WERE GREEN
+ - The `needs-work` note MUST name a specific review-blocking defect or evidence failure; do not use CI status or merge conflicts as the groundAlso applies to: 24-24
🧰 Tools
🪛 LanguageTool
[style] ~16-~16: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...e is no harness for this repo" are true whether or not anybody TRIED, so they name no attempt ...
(WHETHER)
🤖 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 `@review-prompt.txt` around lines 15 - 16, Update the requirement governing
needs-work note contents near the review verdict rules so it explicitly permits
review-blocking evidence failures, including missing or false QA evidence and
missing screenshots, alongside code defects. Preserve the existing mandatory
evidence gates and their specified notes, ensuring valid evidence failures can
produce honest needs-work notes without requiring a misleading defect
description.
… drain `migrate-needs-work` was the #133 one-shot: move every open PR still carrying the RETIRED PR-side `human:needs-work` onto `ai:needs-work`. It has run. Across `rainlanguage` + `cyclofinance` + `S01-Issuer` the open-PR population on that label is now ZERO (the two remaining subjects are closed PRs), which is the same residue-zero bar #225 held `migrate-blocked-deploy` to. Per the standing ruling on #221 a migration is an execution vehicle rather than permanent machinery, so the verb comes out with the state it drained — exactly as `migrate-blocked-deploy` (#225) and `migrate-design` (#224) did, each leaving only a test that the subcommand no longer parses. There is no separate `migrate-reject`: this branch had renamed that verb to `migrate-needs-work`, so both spellings go with this one deletion. Done compiler-first — the `Cmd` variant, its dispatch arm, the plan/mode machinery and its tests, then `RETIRED_HUMAN_NEEDS_WORK_LABEL` itself, following every rustc error rather than re-literalising anything to keep the tree compiling. That cascade is larger than the subcommand, because the retired label was the last thing holding the PR-side sacred-label machinery up: - `PR_SACRED_LABELS` had one entry, so it and `has_human_override` are gone. A PR's human-sacredness is now exactly what #133/#219 said it should be: a native review, or a `👤 human` ruling pinned to the current head. No label. - `PrSignals.human_parked` and `next_action`'s parking arm — #225's list names this one explicitly. Parking is now a modeled `ai:*` state, not a namespace. - `STATE_HUMAN_NEEDS_WORK`, its `classify_lane` arm, its daily-review section, and `Lane::HumanDecisions` — the lane held that one state, so it drained to nothing and came out with it. - `DesignHit::HumanRuled` and `/nd`'s `humanRuled` count, whose only producer was `has_human_override`. `aiDesign`'s stated partition loses that term. - The worklist row's `humanOverride` key, for the same reason. Nothing outside the binary read it. `human:needs-work` is NOT added to `DELETED_STATE_LABELS`, deliberately. That registry claims no transition can write the string again, and on an ISSUE that is false: `human-rule-issue needs-work` still writes it and 21 open issues carry it today. Only the PR-side state was removed, so the lane-arithmetic test names the string locally and says why. What the compiler could NOT see, and the suite caught: fourteen tests built their human-sacred witness from the string literal `"human:needs-work"` rather than the constant, so they compiled clean and then failed — the verdict/scope/ lens/convention gates, the MCP refusal, `producer_state_plan`, the state-load fixture's 36-PR cohort, and the golden descriptor and lane rows. Each now uses a form that is still a human decision (a native review), or asserts the new truth. The last grep sweep then found what neither reached: prose in README.md, CLAUDE.md, `keep-open.md` and `nm.md`, plus the subcommand's row in CLAUDE.md's table — seven documentation sites with no compiler or test coverage at all. Counts re-derived from the merged table rather than carried across: `STATE_DESCRIPTORS` 12 -> 11, `REVIEW_LANE_SECTIONS` 8 -> 7, `Lane::ALL` 6 -> 5. Gates: `cargo test` 1047+3+9+47+8+7+4 passed 0 failed; CI-exact `rainix-rs-static` at rainix 53e96a7 with the changed file touched first; `pre-commit run --all-files`; `nix build .#pr-review-report`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
224-225: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse one spelling for “modeled”.
Use the repository’s selected spelling consistently. The document currently mixes
modeledandmodelled.🤖 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 `@README.md` around lines 224 - 225, Update README.md to use the repository’s selected spelling consistently, replacing the alternate “modeled”/“modelled” variant wherever it appears in the document.Source: Linters/SAST tools
🤖 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 `@README.md`:
- Around line 1457-1464: Update the README passage describing deleted labels so
it preserves the namespace-specific behavior: deleted human:* labels must remain
leaks, while deleted ai:* labels fall through due to the ai-scoped vetter sweep.
Remove or revise the statement equating deleted labels across both namespaces,
keeping the surrounding human-decisions explanation intact.
---
Outside diff comments:
In `@README.md`:
- Around line 224-225: Update README.md to use the repository’s selected
spelling consistently, replacing the alternate “modeled”/“modelled” variant
wherever it appears in the document.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: eaf8840b-8e9e-4bfc-b0cb-250276c48313
📒 Files selected for processing (5)
CLAUDE.mdREADME.mdplugins/human-fsm/commands/keep-open.mdplugins/human-fsm/commands/nm.mdpr-review-report-rs/src/main.rs
Comment on lines
+1457
to
+1464
| `ai:*` label.) (There is no **human-decisions** lane. It held exactly one | ||
| state — the RETIRED PR-side `human:needs-work` (#133) — and once | ||
| `migrate-needs-work` had moved that population to `ai:needs-work` the state, | ||
| the lane and the migration verb all came out together. A live human decision | ||
| on a PR is a comment or a native review, never a label; `human:design` is | ||
| DELETED (#219) too. A deleted label behaves the same way in either namespace: | ||
| like the deploy string above, residue wearing it models nothing and falls | ||
| through. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve namespace-specific leak behavior.
This text says deleted human:* labels fall through like deleted ai:* labels. Earlier in README.md, Lines 664-670 state that deleted human:* labels remain leaks because the vetter sweep is ai:*-scoped. Keep these behaviors separate.
Proposed correction
- A deleted label behaves the same way in either namespace: ... residue wearing it models nothing and falls through.
+ Deleted `ai:*` labels fall through to `un-vetted` and are absorbed by the next vetter verdict.
+ Deleted `human:*` labels remain leak candidates because the vetter sweep is `ai:*`-scoped.📝 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
| `ai:*` label.) (There is no **human-decisions** lane. It held exactly one | |
| state — the RETIRED PR-side `human:needs-work` (#133) — and once | |
| `migrate-needs-work` had moved that population to `ai:needs-work` the state, | |
| the lane and the migration verb all came out together. A live human decision | |
| on a PR is a comment or a native review, never a label; `human:design` is | |
| DELETED (#219) too. A deleted label behaves the same way in either namespace: | |
| like the deploy string above, residue wearing it models nothing and falls | |
| through. | |
| `ai:*` label.) (There is no **human-decisions** lane. It held exactly one | |
| state — the RETIRED PR-side `human:needs-work` (`#133`) — and once | |
| `migrate-needs-work` had moved that population to `ai:needs-work` the state, | |
| the lane and the migration verb all came out together. A live human decision | |
| on a PR is a comment or a native review, never a label; `human:design` is | |
| DELETED (`#219`) too. Deleted `ai:*` labels fall through to `un-vetted` and are absorbed by the next vetter verdict. | |
| Deleted `human:*` labels remain leak candidates because the vetter sweep is | |
| `ai:*`-scoped. |
🤖 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 `@README.md` around lines 1457 - 1464, Update the README passage describing
deleted labels so it preserves the namespace-specific behavior: deleted human:*
labels must remain leaks, while deleted ai:* labels fall through due to the
ai-scoped vetter sweep. Remove or revise the statement equating deleted labels
across both namespaces, keeping the surrounding human-decisions explanation
intact.
Contributor
Author
|
Reviewed 4125ec3: approve — read the diff and verified the merge independently. This renames the send-back state from reject to needs-work across the label, the ruling verb, the vetter verdict, the slash command, the counts key, the lane state and the prose, closing the divergence that has been live since the labels were migrated this morning: GitHub carries ai:needs-work on 91 subjects while main's classifier still spelled ai:reject in 49 places, so the producer's rework queue read empty and the dashboard showed a drained lane that was not drained. The merge was the hard part — based on 71ac743 with main 31 commits ahead, 8 files and 67 hunks in main.rs. Resolution took main's file whole (main was structurally right in nearly every hunk; this branch contributed spelling) and rebuilt the rename compiler-first on merged reality, then ported back the one thing the branch genuinely added: the histFold "reject" entry and its continuity test. The rename reached ten symbols that post-date the branch, all from #224 — NextAction::ReworkReject and its rework-reject string, and the whole RejectState typed row with its predicates and PrSignals field. Verified by hand at this head: HUMAN_PR_RULINGS maps both verbs to ai:needs-work, so no PR-side transition can write a human:* label — which is why deleting the spent migrate-needs-work cascaded correctly into the sacred-label machinery, the human-decisions lane and /nd's humanRuled count; each guarded a state nothing could create. The issue side is untouched and still writes human:needs-work and human:keep-open. Live confirms: 0 open PRs on human:needs-work, 21 open issues. History continuity measured rather than assumed: reject/humanReject/relink carry 218 samples each in the rollup and needsWork has zero, so the entire drawn series arrives through the fold; human-queue-history.jsonl is not rewritten. 1047 tests green, CI-exact static, pre-commit and nix build all 0, mutants killed on the deletion targets, 19 checks green. |
thedavidmeister
added a commit
that referenced
this pull request
Aug 8, 2026
The gate asserted that no committed rollup line carries the post-rename `needsWork` key. That was the same fact as "the fold is load-bearing" only until the first rollup after #231 was committed; sixteen such lines are on main today, so the gate fires on the date rather than on a defect. It now asserts the property that is actually invariant over an append-only history: each folded spelling was measured on committed lines where `needsWork` is absent, which is exactly the span a series folding nothing would lose. Those lines exist and no later rollup can un-write them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Closes #230
Renames the send-back state everywhere it is spelled: label, ruling verb, vetter
verdict, slash command, counts key, lane state, descriptor, docs and prompts.
The name is the accurate one — the state is a transient send-back that carries a
work order, the producer reworks that same PR and branch, and the push is the
transition. "Rejected" named a terminal judgement the FSM does not have.
READ THIS FIRST — the live labels are ALREADY renamed, so
mainis currently blindThis is not a hypothetical migration. As of writing, across
rainlanguage+cyclofinance+S01-Issuer:ai:needs-workhuman:needs-workai:rejecthuman:rejectmain's code still readsai:reject/human:reject. Those constants matchone archived-repo PR and nothing else, so until this lands the pipeline
does not see 79 open PRs and 21 open issues that are sitting in the send-back
state. That is the argument for landing it promptly once the stack ahead of it
clears — it is not a cosmetic rename any more, it is the code catching up to the
labels.
Contains PR 223 — and deliberately not 224
histFoldand thestateDescriptorstable arrive with#223, and the history-continuity
half of #230 is unimplementable without them, so 223's branch is merged into this
one (commit
b5038bb). Once 223 lands onmainthis PR's diff reduces to therename alone. To review only my work:
#224 is deliberately NOT merged
in. It carries
NextAction::ReworkReject, but it also deletes thehuman:designstate that 223 adds a descriptor for, and the two conflictsemantically in
human_queue_doc's counts block and inHUMAN_DECISION_LABELS.Arbitrating that inside a rename PR would be the wrong place to settle it, so
ReworkRejectis left for a merge-main once 224 lands. Judgment call statedup front: when it arrives it should become
ReworkNeedsWork/"rework-needs-work",because exact 1:1 correspondence between the variant and the state key is the
property #230 is buying; the mild stutter is a cosmetic cost worth paying. (Note
NextAction::ReworkRuling/"rework-ruling"already onmainis NOT renamed —that is #111's delegated-design work order, a different state.)
What changed
ai:reject→ai:needs-work,human:reject→human:needs-work.them:
ruling_verbderives the verb from the label's namespace suffix, soHUMAN_PR_RULING_LABELS/HUMAN_RULING_LABELSmoving is the whole change andthe verb list cannot drift from the labels it writes.
VETTER_VERDICTS,verdict_label, and theReviewed <sha>: needs-work — …comment.plugins/human-fsm/commands/reject.md→needs-work.md(git-tracked rename), plugin manifest and marketplace listing both to
0.15.0, distinct from every open sibling's claim (224 → 0.12.0,
227 → 0.13.0, 226 → 0.14.0).
counts.reject→counts.needsWork, thevetter-verdictslane state, andSTATE_REJECT→STATE_NEEDS_WORK.migrate-reject→migrate-needs-work(withMigrateRejectPlan,migrate_reject_plan,migrate_reject_mode). After this rename it moveshuman:needs-workontoai:needs-work, so the verb has to say so.review-prompt.txt, plugin READMEs.
History continuity
"reject"is this state's OWN pre-rename key and it is the load-bearing entry.Measured against the committed
human-queue-history.jsonl(230 rollup lines):rejecthumanRejectrelinkneedsWorkSo the new series has no committed samples of its own: without the fold it
would not merely lose its past, it would start empty and render the state as
newly created — the exact #130 defect
hist_foldexists for.human-queue-history.jsonlis not rewritten.New test
the_needs_work_series_draws_every_spelling_the_state_has_hadpins it,asserting structurally off the descriptor table first (so it cannot pass
vacuously in the flake sandbox, where the repo-root read is filtered out and
repo_root_textreturnsNone), then corroborating against the real committedhistory — that the three fold keys each have real samples, and that
needsWorkhas none, which is what makes the fold load-bearing rather than decorative.
Live label migration — already performed; verified, not re-run
The
gh label edit <old> --name <new>migration had already been carried outbefore this PR reached that step. Re-running it would be pointless and
delete-and-recreate would be destructive, so this PR made no label writes at
all — it verified the outcome instead.
Full read-only census across the three orgs:
(38 archived).
9 define
ai:needs-workonly, 2 definehuman:needs-workonly.rainlanguage/rain.webapp,archived: true.Population reconciliation, which is the check that matters:
human-queue.json)counts.reject= 79ai:needs-workopen PRs = 79human:needs-workopen issues = 21Nothing was lost. The counts match exactly, which is what distinguishes a
rename from a delete-and-recreate.
The single straggler is
rainlanguage/rain.webapp#354(open PR, stillai:reject), and it is unmigratable by construction:rain.webappisarchived, therefore read-only, so
gh label editcannot rename a label there.That is also why exactly one subject was left behind rather than an arbitrary
number — it is the one repo the migration could not write to. No write was
attempted against it. The repo already models this: an archived-repo PR is
inventoried in
archivedRepoPrsrather than in a lane(#206), so it is accounted
for without being actionable.
Deliberately NOT renamed
A blind
grep -ri rejectsweep would have corrupted all of these, which is whythis was done constants-first and compiler-led, with the grep as the last pass:
CC_VERDICTS = ["uphold", "reject"],CcGate::RejectedStillFlagged,CC_CLEAR_REJECTED,record-close-candidate-verdict … reject,/close-candidate <n> reject.These judge a flag, write no label and create no state, so they are not
the send-back state this ruling renames.
ResultRejected,iso_to_epoch_ms_rejects_malformed,blocked_by_ref_rejects_garbage, therejectedrate-limit status, anon-fast-forward push,
forge fmt --checkrejecting a diff.reworked-reject— a subcommand that is GONE; renaming it would invent ahistory in which it was called something it never was. A test asserts it stays
unparseable.
counts.humanReject— the retired-but-kept series the dashboard reads ANDa
histFoldkey. Renaming it would break both.Sequencing
Merges LAST, after
#223,
#224,
#226 and
#227 — it touches the same
regions as all four. Based on
main; I will merge main in as each siblinglands (never rebase over review history), and re-apply the rename to whatever
prose they bring with them. 226 and 227 both edit the marketplace description
line and the
/rejectcommand reference, so expect a conflict there.The dashboard follows on its own once this emits the new vocabulary, per
#130 descriptors, but
rain-org-health's FROZEN fallback table still spells the old name and needs the
same rename for descriptor-less snapshots.
QA
the_needs_work_series_draws_every_spelling_the_state_has_had(new),state_descriptors_emit_the_ratified_shape,verdict_label_maps_the_four_verdicts,human_ruling_vocabulary_is_derived_from_the_label_sets,the_vetter_prompt_teaches_four_verdicts_and_routes_linkage_to_needs_work,every_descriptor_occupancy_source_resolves_in_the_emitted_document— each verified to fail on base by mutation rather than by assertion: M1–M5 below re-introduce the pre-rename spelling at one site each (which IS the base state for that site) and every one of these tests goes red, then green again on restore. The new continuity test additionally fails ifhuman-queue-history.jsonlis ever rewritten to carryneedsWorksamples.hist_fold: &["reject",…]-> drop"reject"->the_needs_work_series_draws_every_spelling_the_state_has_had+state_descriptors_emit_the_ratified_shape;verdict_label("needs-work")->Some("ai:reject")->verdict_label_maps_the_four_verdicts;VETTER_VERDICTS->"reject"->a_valid_verdict_reaches_the_write_exactly_as_given+tools_list_is_exactly_the_vetter_fsm_surface+the_vetter_prompt_teaches_four_verdicts_and_routes_linkage_to_needs_work;HUMAN_PR_RULING_LABELS->"ai:reject"-> 9 tests incl.every_pr_ruling_label_lands_in_a_modeled_lane;STATE_NEEDS_WORK.occupancy.counts->"reject"->every_descriptor_occupancy_source_resolves_in_the_emitted_document. No survivors; all restored byte-identical (table below).human-queue-history.jsonlmeasured independently of this code (230 rollup lines:reject/humanReject/relink= 216 samples each, newest 2026-08-07T09:31:05Z;needsWork= 0), and the LIVE GitHub label populations read viagh api search/issues(ai:needs-work= 79 open PRs,human:needs-work= 21 open issues) checked against the repo's own pre-rename snapshothuman-queue.jsoncounts.reject= 79. Expected values come from those measurements, not from the implementation's own emission.HUMAN_*_RULINGS, derived from the label sets), vetter verdict (VETTER_VERDICTS+ theReviewed <sha>:comment), slash command (reject.md->needs-work.md, manifest + marketplace to 0.15.0),counts.reject->counts.needsWork+ vetter-verdicts lane state +histFold, and all six prose surfaces.NextAction::ReworkRejectis Refs-not-Closes because it does not exist on this base — it arrives with PR 224, which is deliberately not merged in (semantic conflict with 223 overhuman:design); the chosen spellingReworkNeedsWorkis stated above and applied at merge-main. Close-candidate flag verdicts are out of category by design, reasoned above.Toolchain via nix throughout. Gates run at commit
5931e04,pr-review-report-rs/src/main.rssha2562f9bee0109be83322e3b4fffd144b8ab5101267e17d7cfc5e15843f66dca7df8, working treeclean. Exit codes captured unpiped.
1. Full test suite —
nix develop github:rainlanguage/rainix/53e96a7d0a97d7c7c75c3b2412521324776fdac6#rust-shell -c cargo test --manifest-path pr-review-report-rs/Cargo.toml2. CI-exact static, changed file touched first so a warm target cannot serve
a false pass —
touch pr-review-report-rs/src/main.rs && nix develop github:rainlanguage/rainix/53e96a7d0a97d7c7c75c3b2412521324776fdac6#rust-shell -c rainix-rs-staticCaught a real failure first time round:
needs-workis four characters longerthan
reject, which pushed several lines past the width limit. Fixed withcargo fmt, not by shortening anything.3. rainix pre-commit bundle —
nix develop …#rust-shell -c pre-commit run --all-files4. Flake package —
nix build .#pr-review-report --print-build-logsAdversarial mutation pass
Run after committing the real work, so
git checkout --cleanup could noteat uncommitted edits. Each mutant: apply, run the whole suite, restore, verify
the restored file's sha256 matches the pre-mutation sha.
"reject"fromSTATE_NEEDS_WORK.hist_foldthe_needs_work_series_draws_every_spelling_the_state_has_had,state_descriptors_emit_the_ratified_shapeverdict_label("needs-work")→Some("ai:reject")verdict_label_maps_the_four_verdictsVETTER_VERDICTSback to"reject"a_valid_verdict_reaches_the_write_exactly_as_given,tools_list_is_exactly_the_vetter_fsm_surface,the_vetter_prompt_teaches_four_verdicts_and_routes_linkage_to_needs_workHUMAN_PR_RULING_LABELShuman_ruling_vocabulary_is_derived_from_the_label_sets,every_pr_ruling_label_lands_in_a_modeled_lane,the_human_rule_schemas_pin_the_sacred_label_setshistisneedsWorkevery_descriptor_occupancy_source_resolves_in_the_emitted_documentNo survivors. All five restored byte-identical to
2f9bee0109be83322e3b4fffd144b8ab5101267e17d7cfc5e15843f66dca7df8.M1 is the one the issue asks for specifically, and it behaves as required: the
history truncates and a test catches it. M3 is the drift the issue names — a
verb and a label that stop agreeing — and the prompt-conformance test fires,
which means the vocabulary the vetter is taught is pinned to the vocabulary the
guard enforces. M5 is worth noting because its failure message is exactly
`ai:needs-work` names history key `needsWork` but `counts` has no such key— the descriptor and the emitted document disagreeing, caught structurally.
🤖 Generated with Claude Code
Summary by CodeRabbit
/needs-workcommand for returning pull requests and issues with required rework notes.migrate-needs-work.