Rework the PR-feedback loop's triage: move the decision to the implementer #141

New issue

Closed

opened 2026-08-13 12:49:04 +00:00 by root · 0 comments

root commented

2026-08-13 12:49:04 +00:00

(Migrated from gitlab.core-project.space)

Imported from GitHub amaurycoudr/chess-project#141, opened by @amaurycoudr on 2026-08-13 12:49 UTC.
Closed on GitHub on 2026-10-04 14:13 UTC as not planned.

What to build

pnpm agent:feedback <pr> classifies every unresolved review thread and then implements the todo ones. The classification and the deciding are both done by the triage phase, and the quality of that decision-making is the problem.

The mechanism is visible in .sandcastle/feedback-triage-prompt.md, which requires each expectedFix to be specific about "which file and location, what to change, and why". Triage therefore pre-decides the edit down to line numbers, and the implement phase becomes a typist with no standing to disagree with a reviewer who was wrong. On PR #118, six of seven threads were "delete this comment"; triage never asked whether the reviewer was right, only how to comply.

The likely fix is not a better triage prompt — it is moving the decision: triage classifies and orders, the implementer judges whether the reviewer was right and says so if not. That is the same principle ADR 0010 applies in the other direction, where the runner's implement phase is told to treat review findings as evidence rather than instructions (.sandcastle/implement-prompt.md).

Three decisions this can now inherit rather than re-decide, all landed in claude/sandcastle-runner-contract:

the canonical definition of done is CONTRIBUTING.md's five commands
CODING_STANDARDS.md lives at the repo root and is the shared review input
the typed host↔agent verdict contract (<review-output> + Zod), which this loop's <triage-output> predates and could align with

Acceptance criteria

Triage no longer prescribes the edit; it classifies, orders into waves, and states what the reviewer asked for
The implement phase decides whether each finding is correct, and records disagreement rather than complying silently
Commit granularity is deliberate rather than one-per-thread (PR #118 produced six commits deleting six comments)
Whether feedback.ts's four statuses stay separate from /triage's five canonical roles is settled explicitly

Out of scope

Merging the two vocabularies for their own sake — they were deliberately kept separate because a triage role answers "who should do this issue" and a feedback status answers "what do I do with this reviewer's comment".

> _Imported from GitHub [amaurycoudr/chess-project#141](https://github.com/amaurycoudr/chess-project/issues/141), opened by [@amaurycoudr](https://github.com/amaurycoudr) on 2026-08-13 12:49 UTC._ > _Closed on GitHub on 2026-10-04 14:13 UTC as not planned._  --- ## What to build `pnpm agent:feedback <pr>` classifies every unresolved review thread and then implements the `todo` ones. The classification and the deciding are both done by the triage phase, and the quality of that decision-making is the problem. The mechanism is visible in `.sandcastle/feedback-triage-prompt.md`, which requires each `expectedFix` to be specific about "which file and location, what to change, and why". Triage therefore pre-decides the edit down to line numbers, and the implement phase becomes a typist with no standing to disagree with a reviewer who was wrong. On PR #118, six of seven threads were "delete this comment"; triage never asked whether the reviewer was right, only how to comply. The likely fix is not a better triage prompt — it is **moving the decision**: triage classifies and orders, the implementer judges whether the reviewer was right and says so if not. That is the same principle ADR 0010 applies in the other direction, where the runner's implement phase is told to treat review findings as evidence rather than instructions (`.sandcastle/implement-prompt.md`). Three decisions this can now inherit rather than re-decide, all landed in `claude/sandcastle-runner-contract`: - the canonical definition of done is `CONTRIBUTING.md`'s five commands - `CODING_STANDARDS.md` lives at the repo root and is the shared review input - the typed host↔agent verdict contract (`<review-output>` + Zod), which this loop's `<triage-output>` predates and could align with ## Acceptance criteria - [ ] Triage no longer prescribes the edit; it classifies, orders into waves, and states what the reviewer asked for - [ ] The implement phase decides whether each finding is correct, and records disagreement rather than complying silently - [ ] Commit granularity is deliberate rather than one-per-thread (PR #118 produced six commits deleting six comments) - [ ] Whether `feedback.ts`'s four statuses stay separate from `/triage`'s five canonical roles is settled explicitly ## Out of scope Merging the two vocabularies for their own sake — they were deliberately kept separate because a triage role answers "who should do this issue" and a feedback status answers "what do I do with this reviewer's comment".

root (Migrated from gitlab.core-project.space) closed this issue

2026-10-04 14:13:18 +00:00