From 72a0ca4fe746de490ecfb757ac17f8f8750b2bfc Mon Sep 17 00:00:00 2001 From: Siddharth Kapoor Date: Wed, 8 Jul 2026 23:10:53 -0400 Subject: [PATCH] feat(pr): review evaluates changes against stated intent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A review should judge whether a change delivers its purpose, not just whether the code is clean. Add an INTENT ALIGNMENT dimension as the first thing review checks: read the PR/MR title and description as the stated intent, trace the headline use case end-to-end, and flag anything that undercuts it — a main path that stalls, an abstraction that never connects, a "reusable" thing that isn't — as BLOCKING even when the code is well-formed. The summary now leads with an intent verdict. --- docs/skills/pr.md | 2 ++ skills/pr/references/new-review-template.md | 4 ++-- skills/pr/references/workflows/help.md | 6 +++++- skills/pr/references/workflows/review.md | 14 ++++++++++++-- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/docs/skills/pr.md b/docs/skills/pr.md index 152394a..7cc50a6 100644 --- a/docs/skills/pr.md +++ b/docs/skills/pr.md @@ -32,6 +32,8 @@ The body is written in a **terse, human, junior-dev-friendly voice** — short s Read a PR/MR diff and generate AI-authored inline comments. Comments are terse (see the voice guide): one or two sentences — problem, then ask — with a code suggestion where it's clearer. Severities are `BLOCKING`, `CONSIDER`, `NOTE`. +Reviews evaluate the change against its **stated intent** first — does the diff actually deliver the PR/MR's purpose end-to-end (tracing the headline use case), not just whether the code is clean? A well-formed change that fails its intent is flagged `BLOCKING`. + Runs a **dedicated slop-detector pass** in parallel: a subagent whose only job is catching AI slop and unwanted agent-introduced change — unnecessary/out-of-scope edits, stochastic churn (random renames, reordering, reformatting), verbose boilerplate, dead/debug leftovers, dependency creep. A prevalent problem with agentic coding. Its findings are prefixed `Slop:`. ```bash diff --git a/skills/pr/references/new-review-template.md b/skills/pr/references/new-review-template.md index 9dc64e8..2fc5645 100644 --- a/skills/pr/references/new-review-template.md +++ b/skills/pr/references/new-review-template.md @@ -12,9 +12,9 @@ Write this structure to `.codevoyant/review/{slug}/new-review.md`. - **Stats**: +{additions} -{deletions} across {changedFiles} files - **Reviewed**: {timestamp} -## Summary +## Summary — does this deliver its intent? -{One-paragraph overall impression. State the main concern first. No filler phrases.} +{Lead with an intent verdict: does the change deliver its stated purpose end-to-end? Trace the headline use case. Then the main concern. No filler phrases.} ## Inline Comments diff --git a/skills/pr/references/workflows/help.md b/skills/pr/references/workflows/help.md index 797ecfc..974f64d 100644 --- a/skills/pr/references/workflows/help.md +++ b/skills/pr/references/workflows/help.md @@ -21,7 +21,11 @@ Adjust whatever `open` / `review` / `address` produced last, from `` (minor) / `` (major) annotations you add inline to its local file, or a plain-language request (`/pr update "tighten the summary"`). Re-syncs to the platform if the artifact was already pushed. -## review — slop / scope pass +## review — intent + slop passes + +Reviews evaluate the change against its **stated intent** first — does the diff actually deliver the PR/MR's +purpose end-to-end, tracing the headline use case — not just line-level code quality. A clean diff that fails +its intent is flagged BLOCKING. Alongside the correctness review, a dedicated **slop-detector** agent audits the diff for AI slop and unwanted agent-introduced change: unnecessary/out-of-scope edits, stochastic churn (random renames, reordering, reformatting), diff --git a/skills/pr/references/workflows/review.md b/skills/pr/references/workflows/review.md index 99b42fd..7387a53 100644 --- a/skills/pr/references/workflows/review.md +++ b/skills/pr/references/workflows/review.md @@ -127,6 +127,13 @@ Diff: Write a thorough inline code review. Be thorough in what you CATCH, terse in what you WRITE. +INTENT ALIGNMENT (evaluate this FIRST — it usually matters more than line-level nits): +- Treat the PR/MR title and description above as the **stated intent** — the goal and its implicit acceptance criteria. If the description is thin, infer the intent from the branch name and the shape of the diff. +- Judge whether the diff actually **delivers that intent end-to-end**, not just whether the code is clean. **Trace the headline use case concretely** — walk the main path step by step and check it would really work as written. +- Flag anything that undercuts the stated purpose, even if the code is well-formed: a feature whose main path stalls or needs manual intervention, an abstraction that never connects to its consumer, something billed as "reusable/global/automatic" that isn't, a fix that doesn't actually cover the reported case, a config/flag that has no effect. +- These are **BLOCKING** when they mean the change doesn't do what it claims. A clean diff that fails its intent is still a failing change — say so, and name the exact scenario where it breaks. +- Do NOT rubber-stamp scope you didn't verify. If part of the intent can't be confirmed from the diff, say what you couldn't verify rather than assuming it works. + TONE (follow references/voice.md — terse, human, junior-dev friendly): - Each comment is usually ONE or TWO short sentences: name the problem, then the ask. Skip the mechanism walk-through and the list of every consequence — the author can read the code. - Human and respectful. No sarcasm, no faint praise ("nice work but…"), no rhetorical questions, no hype. @@ -134,12 +141,15 @@ TONE (follow references/voice.md — terse, human, junior-dev friendly): - Example — instead of: "Any logged-in user can POST any pathname and it'll be saved to the DB, so someone could record a path to another user's file or a made-up URL and it shows up as a legitimate upload…" write: "This accepts any pathname, even fake ones. Worth validating." Then a code suggestion if it helps. CONTENT: +- **Intent gaps** (does the change deliver its stated purpose?) come first — see INTENT ALIGNMENT above. - Flag bugs, logic errors, and security issues as **BLOCKING** - Flag style deviations, naming, and structure as non-blocking (CONSIDER) - For each non-trivial issue, prefer a concrete code suggestion (diff block or replacement snippet) over prose — it's usually clearer and shorter. - Cite external documentation, an RFC, or prior art (URL) only when it saves the author a search or justifies the point — not as decoration. - Skip comments on style that matches project conventions — do not nitpick conforming code -- Focus on correctness, security, and design. **Unnecessary/out-of-scope changes and AI "slop" are handled by a dedicated pass (Step 6b) — don't duplicate that here.** +- Focus on correctness, security, design, and intent. **Unnecessary/out-of-scope changes and AI "slop" are handled by a dedicated pass (Step 6b) — don't duplicate that here.** + +For an intent-gap finding, anchor the comment on the most relevant line of the change (the assumption that doesn't hold, the flag that does nothing, the seam that doesn't connect) and describe the concrete failure scenario in the body. OUTPUT FORMAT — always produce a valid JSON array (empty array if no comments): [ @@ -155,7 +165,7 @@ OUTPUT FORMAT — always produce a valid JSON array (empty array if no comments) Return `[]` if the code has no issues. Do not include an overall summary in this JSON — that goes in a separate field. ``` -Also produce a one-paragraph overall summary as a separate string. +Also produce a one-paragraph overall summary as a separate string. **Lead the summary with an intent verdict** — does the change deliver its stated purpose end-to-end? — then note the most important findings. If the headline use case wouldn't work as written, say so up front, not buried under line nits. ## Step 6b: Dedicated slop pass (run in parallel with Step 6)