Prohibit agent-originated product behavior in code review - #201
mikeangstadt wants to merge 2 commits into
Conversation
Adds "Agent-originated product behavior — never ship it" to the four reviewer prompts that operate on symphony-alpha. The rule: an agent must not originate user-visible behavior. Refusing to show something is product behavior, as is a default, a threshold, a fallback, an ordering, or an empty-state sentence. Every such rule must cite a human source; if it cannot, that is the finding, reported at High. The review heuristic is the prose itself. A behavior a human specified cites that human. A behavior an agent invented argues for itself — and length correlates with invention. A thorough test suite around an uncited product rule is evidence of the defect, not of diligence, because it makes the invention permanent and makes the next reader assume intent. Includes the measured instance that motivated it: a Sessions LOC/$ card that refused to publish a ratio while both of its operands rendered beside it on the same row, behind a rule no human specified, carrying a justification paragraph, a canonical reason map, a feature flag and a nine-case suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Plugin Version Bump check requires any plugin whose files changed to carry a version bump. This change edits reviewer prompts in both plugins/code and plugins/code-review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Beginning a code-review. Please allow time for me to finish before merging the PR |
|
|
||
| **Never refuse a derivation you have the inputs for.** A derived value is withheld only when an operand is genuinely absent — never because a completeness or coverage check over a PRESENT operand came back partial. Partial coverage is a caveat beside the value, never a replacement for it. Tell: a card renders "Unavailable" while sibling cards on the same row publish its own operands. | ||
|
|
||
| **How to review for it.** For each user-visible decision in the diff, name the human source. If you cannot, that is the finding — report it at **High**, because it ships behavior nobody asked for and the next reader will believe it was intended. Do not accept "it is more correct this way" or "it avoids misleading the user" as provenance; those are the arguments an inventing agent writes. |
There was a problem hiding this comment.
P1: This GitHub-only prompt is read by the orchestrator, not the reviewers that inspect the patch. Both local and GitHub review workers receive shared_prompt.txt, while local mode skips this file entirely, so the new rule never runs where it needs to catch anything.
Put the reviewer-facing provenance rule in tools/prompts/shared_prompt.txt (keep GitHub-only output instructions here if needed).
|
|
||
| **A test does not confer provenance.** Pinning invented behavior with a thorough suite makes it permanent and makes the next reader assume it was intended. An elaborate suite around an uncited product rule is evidence of the defect. | ||
|
|
||
| **Never refuse a derivation you have the inputs for.** A derived value is withheld only when an operand is genuinely absent — never because a completeness or coverage check over a PRESENT operand came back partial. Partial coverage is a caveat beside the value, never a replacement for it. Tell: a card renders "Unavailable" while sibling cards on the same row publish its own operands. |
There was a problem hiding this comment.
P1: This turns the ISS-10354 LOC/$ decision into the same kind of uncited global product rule we’re trying to catch. That ruling says this card must publish its value despite partial coverage; it does not say every derived metric with present inputs must always be shown.
A different product requirement may deliberately withhold a value below a specified confidence or coverage threshold, and this will tell reviewers to reject it anyway. Keep the provenance check, but scope the partial-coverage guidance to cases where the ticket/PRD/operator decision says to publish the value.
| "name": "code-review", | ||
| "description": "Code review plugin", | ||
| "version": "1.5.5", | ||
| "version": "1.5.6", |
There was a problem hiding this comment.
P2: The version bumps need their generated changelog entries too. .githooks/pre-push rejects plugin changes without a root or plugin-local changelog diff, but the current CI version check only verifies the manifest bump, so this passes CI and then fails the repository’s release guardrail.
Run /update-documentation and include the generated entries for both code-review and code.
2 participants
Summary
Adds "Agent-originated product behavior — never ship it" to the four reviewer prompts that operate on
symphony-alpha:plugins/code/agents/code-review-guidelines.mdplugins/code/agents/code-reviewer.mdplugins/code-review/prompts/github-review.md.claude/agents/pr-reviewer.mdThe rule. An agent must not originate user-visible product behavior. Every rule deciding what a user sees — shown, withheld, refused, defaulted, thresholded, ranked, reworded — must cite a human source: ticket AC, PRD, or an operator instruction. If it cannot, that is the finding, reported at High.
Why it needs a reviewer rule and not just a repo doc. This class ships looking correct. The implementer reasons their way to a defensible position, writes a persuasive comment, and pins it with tests. Every downstream reader then treats it as intended. Nothing in the existing severity table catches it, because the code is well-formed, well-tested, and internally consistent — it is simply behavior nobody asked for.
The review heuristic is the prose itself. A behavior a human specified CITES that human. A behavior an agent invented ARGUES FOR ITSELF. Length correlates with invention. A thorough suite around an uncited product rule is evidence of the defect, not of diligence.
Sits beside "Blind scope drop — never an option" in
code-review-guidelines.mdand follows its shape: named prohibition, concrete tells, and a measured instance.The measured instance
symphony-alphaISS-10354, 2026-09-15. A Sessions LOC/$ card refused to publish its ratio whenever a coverage probe returned anything butComplete. No human specified that rule. It shipped with a paragraph of justification, a canonical state-to-copy map, a feature flag, and a nine-case suite pinning each refusal.In production it rendered
LOC / $ —on a row that simultaneously displayedTotal estimated cost $37,927andPRs shipped 362— both of its own operands, present and rendering. One refused state (MigrationPending, "still being rebuilt") described a rebuild that no job performs, so the blackout was permanent rather than transient.The code's own comment admitted what it was: "the coverage verdict is a refusal to publish a number that exists." It was caught by a human looking at a screenshot, after code-side analysis of the coverage state machine missed it twice.
Companion change in symphony-alpha: closedloop-ai/symphony-alpha#7356, which deletes the refusal and adds the matching sections to
AGENTS.md,.claude/review/soul.mdand.claude/logical/discipline-core.md.Test plan
Prose-only change to four reviewer prompt files; no code paths, no runtime behavior. Verified each file received the section exactly once and that the insert is idempotent (re-running skips a file that already carries it).
🤖 Generated with Claude Code