From 3d224e5650471524c847e22cbbff9e4e3a694a1b Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 18:03:01 +0000 Subject: [PATCH 1/4] feat(#439): scale review detail to change complexity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add complexity-based calibration to the review agent so simple changes (deletion-only, docs/CI config edits affecting ≤2 files) receive a brief 2-3 sentence review instead of a full multi-dimension analysis table. Changes to pr-review skill (orchestrator): - Add step 3b-1: classify change complexity as simple or standard with explicit criteria and edge case guidance - Modify step 3c: simple changes dispatch only correctness and style-conventions sub-agents, skip the challenger pass - Add simple row to step 3e scope constraints table (trivial) - Add challenger skip note in step 6d for simple changes - Restructure step 7 into 7a (brief format) and 7b (standard format) so simple approvals produce concise prose output Changes to code-review skill (standalone): - Add complexity calibration subsection before dimension evaluation with matching simple/standard classification No changes to sub-agent definitions, output schemas, or post-scripts. Complex PRs retain the full multi-dimension analysis. The brief format still includes the hidden SHA comment for re-review anchoring. Closes #439 --- skills/code-review/SKILL.md | 15 ++++++ skills/pr-review/SKILL.md | 98 ++++++++++++++++++++++++++++++++----- 2 files changed, 100 insertions(+), 13 deletions(-) diff --git a/skills/code-review/SKILL.md b/skills/code-review/SKILL.md index 9a037ff6..4d46622d 100644 --- a/skills/code-review/SKILL.md +++ b/skills/code-review/SKILL.md @@ -61,6 +61,21 @@ git log --oneline -10 -- ### 3. Evaluate each dimension +#### Complexity calibration + +Before evaluating dimensions, classify the change complexity: + +**Simple** — ALL of the following hold: the diff is purely subtractive +(deletion-only) OR touches only CI/CD configuration or documentation +files; affects 2 or fewer files; does not modify production logic; does +not touch security-sensitive paths. For simple changes, evaluate only +**Correctness** and **Style/conventions** — skip the other four +dimensions. Produce a brief 2-3 sentence assessment rather than a +per-dimension breakdown. If a simple change reveals unexpected +concerns (e.g., a deletion removes a security guard), reclassify as +standard and evaluate all dimensions. + +**Standard** — any change that does not meet all simple criteria. Evaluate all six dimensions independently. Do not let confidence in one dimension carry over to another — each requires its own scrutiny. diff --git a/skills/pr-review/SKILL.md b/skills/pr-review/SKILL.md index c033791e..a4c6e81b 100644 --- a/skills/pr-review/SKILL.md +++ b/skills/pr-review/SKILL.md @@ -319,13 +319,54 @@ dimensions are relevant: - Repository has documentation files → `docs-currency` - Always included → `style-conventions` +#### 3b-1. Classify change complexity + +After classifying change domains, classify the overall change +complexity as **simple** or **standard**: + +**Simple** — ALL of the following must hold: + +- The diff is purely subtractive (deletion-only with no added lines), + OR the change touches only CI/CD configuration files (workflows, + pipeline definitions, `.pre-commit-config.yaml`) or documentation + files (`.md`, `.adoc`, `.rst`, `README`, `CHANGELOG`) +- The change affects 2 or fewer files +- The change does not modify production logic (application code, + library code, controllers, handlers, models) +- The change does not touch security-sensitive paths (auth, + permissions, secrets, RBAC, token handling) + +**Standard** — any change that does not meet ALL simple criteria. + +Record the classification for use in steps 3c, 3e, and 7. + +**Edge cases:** + +- A deletion that removes a security check or auth guard is NOT + simple — removing security controls requires full analysis +- A CI config change that modifies permissions blocks, secret + references, or token scoping is NOT simple — these are + security-sensitive +- Mostly deletions with even 1 line of new logic → standard +- 3+ files even if all are mechanical renames → standard + #### 3c. Select sub-agents Based on the domain classification, select sub-agents for dispatch. All selected sub-agents run in parallel (with the exception of the challenger, which runs by itself after all other sub-agents have finished). -**Dispatch sub-agents based on the classification — typically 3-6.** +**Simple change dispatch:** When the change is classified as simple +(step 3b-1), dispatch ONLY `correctness` and `style-conventions`. +Do not dispatch conditional sub-agents (`security`, +`intent-coherence`, `docs-currency`, `cross-repo-contracts`) +regardless of step 3b's domain classification. Skip the challenger +pass (step 6d) — there are not enough findings to warrant adversarial +challenge on a simple change. This prevents multi-dimension analysis +noise on deletion-only diffs, README edits, and CI config tweaks. + +**Standard change dispatch:** For standard changes, dispatch +sub-agents based on the classification — typically 3-6. The orchestrator should auto-select which sub-agents are relevant for the specific change rather than dispatching all agents by default. A complex PR that triggers all conditions legitimately needs all 6. @@ -398,17 +439,19 @@ normal scope (current behavior preserved). **Dispatch examples:** -| PR type | Agents dispatched | -|----------------------------------------------------------|----------------------------------------------------------------------------------| -| Implementation plan | correctness, style-conventions, intent-coherence, docs-currency | -| Typo fix in README | correctness, style-conventions | -| Bug fix in auth middleware | correctness, security, style-conventions, intent-coherence | -| New API endpoint with tests | correctness, security, style-conventions, cross-repo-contracts | -| Large refactor across packages | correctness, style-conventions, intent-coherence, docs-currency | -| CI/CD pipeline change | correctness, security, style-conventions, intent-coherence | -| DB migration + API change | correctness, security, style-conventions, cross-repo-contracts, docs-currency | -| Re-review after fix (prior findings in correctness only) | correctness (full scope), style-conventions (trivial scope), challenger | -| Re-review after fix (prior findings in security only) | correctness (full scope), security (normal scope), style-conventions (trivial scope), challenger | +| PR type | Complexity | Agents dispatched | +|----------------------------------------------------------|------------|----------------------------------------------------------------------------------| +| Deletion of CI config + README edit (2 files) | simple | correctness, style-conventions (trivial scope, no challenger) | +| Deletion-only removal of a build task | simple | correctness, style-conventions (trivial scope, no challenger) | +| Typo fix in README | simple | correctness, style-conventions (trivial scope, no challenger) | +| Implementation plan | standard | correctness, style-conventions, intent-coherence, docs-currency | +| Bug fix in auth middleware | standard | correctness, security, style-conventions, intent-coherence | +| New API endpoint with tests | standard | correctness, security, style-conventions, cross-repo-contracts | +| Large refactor across packages | standard | correctness, style-conventions, intent-coherence, docs-currency | +| CI/CD pipeline change | standard | correctness, security, style-conventions, intent-coherence | +| DB migration + API change | standard | correctness, security, style-conventions, cross-repo-contracts, docs-currency | +| Re-review after fix (prior findings in correctness only) | standard | correctness (full scope), style-conventions (trivial scope), challenger | +| Re-review after fix (prior findings in security only) | standard | correctness (full scope), security (normal scope), style-conventions (trivial scope), challenger | #### 3c-1. Security-critical file triage (large PRs) @@ -575,6 +618,7 @@ sub-agents must honor — it overrides their default exploration budget. | Change classification | `scope_constraint` | |------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------| +| Simple (step 3b-1) | `"trivial: ≤5 tool calls. Read ONLY the diff and linked issue. Do NOT read project docs, surrounding files, git history, or directory listings. Return findings immediately after scope verification."` | | Mechanical / value-only (digest bump, version bump, hash swap, URL update, feature flag toggle) | `"trivial: ≤5 tool calls. Read ONLY the diff and linked issue. Do NOT read project docs, surrounding files, git history, or directory listings. Return findings immediately after scope verification."` | | Small non-mechanical (under 20 changed lines, structural) | `"small: ≤15 tool calls. Read the diff, linked issue, and up to 3 context files directly relevant to the change."` | | Standard / large | `"none"` (sub-agent uses its own exploration budget) | @@ -844,6 +888,11 @@ and an auth bypass on the same line are two distinct findings. #### 6d. Challenger pass (dedicated sub-agent) +**Skip this step for simple changes** (step 3b-1). Simple changes +dispatch only `correctness` and `style-conventions` with trivial scope +— the finding set is too small to warrant adversarial challenge. +Proceed directly to step 6e. + After steps 6a–6c produce a merged finding set, dispatch the `challenger` sub-agent to adversarially challenge the findings with fresh context. The challenger has not seen the orchestrator's synthesis @@ -1130,7 +1179,30 @@ challenger-adjudicated finding set and evaluate: ### 7. Produce the review result -Compose the review comment using this structure: +#### 7a. Brief format (simple changes) + +When the change was classified as **simple** (step 3b-1) AND the +outcome is `approve` (no medium+ findings), produce a brief review +body: the hidden SHA comment followed by a 2-3 sentence summary of +what was checked and the result. Do not use the `## Review` header, +`### Findings` section, or severity sub-headings. Example: + +```markdown +Deletion of the Tekton task definition and corresponding README +reference. No logic or security implications — the removal is +clean with no remaining references to the deleted task. Looks +good. +``` + +If a simple change produces medium+ findings (unexpected for a +genuinely simple change — consider whether the complexity +classification was wrong), fall through to the standard format +below. + +#### 7b. Standard format + +For standard changes (or simple changes with medium+ findings), +compose the review comment using this structure: The first line must be an HTML comment embedding the head SHA. Construct it by concatenating: the HTML comment open delimiter, From d15e571545a23e53132c8c8e334be2baaaf3c70a Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 20:56:54 +0000 Subject: [PATCH 2/4] fix(pr-review): always include challenger agent, collapse info findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove all language making challenger optional for simple changes: step 3c dispatch, step 6d skip directive, dispatch examples table - Challenger now always runs after dimension sub-agents, regardless of change complexity classification - Place info-level findings in collapsed
section in the review comment template and formatting rules - Verified step 3c-1 (security-critical file triage) is not contradicted: simple changes (≤2 files) never reach per-file mode (≥50 files), so the two steps are mutually exclusive by definition Addresses review feedback on #441 --- skills/pr-review/SKILL.md | 41 ++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/skills/pr-review/SKILL.md b/skills/pr-review/SKILL.md index a4c6e81b..c614144c 100644 --- a/skills/pr-review/SKILL.md +++ b/skills/pr-review/SKILL.md @@ -360,10 +360,11 @@ challenger, which runs by itself after all other sub-agents have finished). (step 3b-1), dispatch ONLY `correctness` and `style-conventions`. Do not dispatch conditional sub-agents (`security`, `intent-coherence`, `docs-currency`, `cross-repo-contracts`) -regardless of step 3b's domain classification. Skip the challenger -pass (step 6d) — there are not enough findings to warrant adversarial -challenge on a simple change. This prevents multi-dimension analysis -noise on deletion-only diffs, README edits, and CI config tweaks. +regardless of step 3b's domain classification. The challenger pass +(step 6d) still runs after dimension sub-agents complete — it is +never skipped. This prevents multi-dimension analysis noise on +deletion-only diffs, README edits, and CI config tweaks while +preserving adversarial challenge of all findings. **Standard change dispatch:** For standard changes, dispatch sub-agents based on the classification — typically 3-6. @@ -441,9 +442,9 @@ normal scope (current behavior preserved). | PR type | Complexity | Agents dispatched | |----------------------------------------------------------|------------|----------------------------------------------------------------------------------| -| Deletion of CI config + README edit (2 files) | simple | correctness, style-conventions (trivial scope, no challenger) | -| Deletion-only removal of a build task | simple | correctness, style-conventions (trivial scope, no challenger) | -| Typo fix in README | simple | correctness, style-conventions (trivial scope, no challenger) | +| Deletion of CI config + README edit (2 files) | simple | correctness, style-conventions (trivial scope), challenger | +| Deletion-only removal of a build task | simple | correctness, style-conventions (trivial scope), challenger | +| Typo fix in README | simple | correctness, style-conventions (trivial scope), challenger | | Implementation plan | standard | correctness, style-conventions, intent-coherence, docs-currency | | Bug fix in auth middleware | standard | correctness, security, style-conventions, intent-coherence | | New API endpoint with tests | standard | correctness, security, style-conventions, cross-repo-contracts | @@ -888,11 +889,6 @@ and an auth bypass on the same line are two distinct findings. #### 6d. Challenger pass (dedicated sub-agent) -**Skip this step for simple changes** (step 3b-1). Simple changes -dispatch only `correctness` and `style-conventions` with trivial scope -— the finding set is too small to warrant adversarial challenge. -Proceed directly to step 6e. - After steps 6a–6c produce a merged finding set, dispatch the `challenger` sub-agent to adversarially challenge the findings with fresh context. The challenger has not seen the orchestrator's synthesis @@ -1230,9 +1226,21 @@ where `[open]` = `<` + `!--` and `[close]` = `--` + `>`. ... -#### Medium / Low / Info +#### Medium + +... + +#### Low ... + +
+Info findings + +- **[]** `:` — +- ... + +
``` **Formatting rules:** @@ -1252,6 +1260,13 @@ where `[open]` = `<` + `!--` and `[close]` = `--` + `>`. section. If there are no findings at all, set the body to the hidden SHA comment followed by a newline and "Looks good to me" — omit the `## Review` header and `### Findings` section entirely. +- **Info-level findings in collapsed section.** When info-level + findings are present, render them inside a collapsed `
` + block with `Info findings`. This keeps the + review focused on actionable findings while preserving info-level + observations for readers who expand the section. If info is the + only severity present, the `
` block appears directly + under `### Findings` (no other severity sub-headings). - **No footer.** Do not append any footer, action-hints block, or boilerplate after findings. The post-review pipeline appends action hints deterministically when appropriate. From 4f87c48a8b394d8657769e43a586fc7b3cb1b63e Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 21:37:12 +0000 Subject: [PATCH 3/4] fix(review): address review findings on PR #441 - Fix logic error in code-review constraint: "all six dimensions" hardcoded count contradicts the new complexity calibration which intentionally evaluates only two dimensions for simple changes. Changed to "all applicable dimensions" with a parenthetical clarifying what "applicable" means per classification. - Add hidden SHA comment placeholder to pr-review brief format example so the example matches the prose instruction requiring the SHA comment. - Add "(trivial scope)" annotation to correctness agent in simple change dispatch table rows, consistent with the step 3e scope constraint table which mandates trivial scope for ALL sub-agents on simple changes. Addresses review feedback on #441 --- skills/code-review/SKILL.md | 8 +++++--- skills/pr-review/SKILL.md | 7 ++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/skills/code-review/SKILL.md b/skills/code-review/SKILL.md index 4d46622d..216ffdee 100644 --- a/skills/code-review/SKILL.md +++ b/skills/code-review/SKILL.md @@ -321,6 +321,8 @@ wins. - **Never review from the diff alone.** Always read full source files to understand surrounding context. - **Report failure rather than producing a partial review.** If you - cannot complete all six dimensions (tool failure, missing context, - ambiguous findings), state that clearly rather than producing an - incomplete result. + cannot complete all applicable dimensions (tool failure, missing + context, ambiguous findings), state that clearly rather than + producing an incomplete result. ("Applicable" means the dimensions + required by the complexity classification — two for simple changes, + six for standard.) diff --git a/skills/pr-review/SKILL.md b/skills/pr-review/SKILL.md index c614144c..0337d38f 100644 --- a/skills/pr-review/SKILL.md +++ b/skills/pr-review/SKILL.md @@ -442,9 +442,9 @@ normal scope (current behavior preserved). | PR type | Complexity | Agents dispatched | |----------------------------------------------------------|------------|----------------------------------------------------------------------------------| -| Deletion of CI config + README edit (2 files) | simple | correctness, style-conventions (trivial scope), challenger | -| Deletion-only removal of a build task | simple | correctness, style-conventions (trivial scope), challenger | -| Typo fix in README | simple | correctness, style-conventions (trivial scope), challenger | +| Deletion of CI config + README edit (2 files) | simple | correctness (trivial scope), style-conventions (trivial scope), challenger | +| Deletion-only removal of a build task | simple | correctness (trivial scope), style-conventions (trivial scope), challenger | +| Typo fix in README | simple | correctness (trivial scope), style-conventions (trivial scope), challenger | | Implementation plan | standard | correctness, style-conventions, intent-coherence, docs-currency | | Bug fix in auth middleware | standard | correctness, security, style-conventions, intent-coherence | | New API endpoint with tests | standard | correctness, security, style-conventions, cross-repo-contracts | @@ -1184,6 +1184,7 @@ what was checked and the result. Do not use the `## Review` header, `### Findings` section, or severity sub-headings. Example: ```markdown + Deletion of the Tekton task definition and corresponding README reference. No logic or security implications — the removal is clean with no remaining references to the deleted task. Looks From 5c7a9c858c464783336613d9c80b1677e1fe27d8 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 21:45:46 +0000 Subject: [PATCH 4/4] fix(review): normalize challenger in dispatch examples table Remove 'challenger' from all rows in the dispatch examples table and add a footnote clarifying it always runs (step 6d). The selective inclusion in some rows but not others implied it was conditional, contradicting the text at lines 357 and 364. Addresses review feedback on #441 --- skills/pr-review/SKILL.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/skills/pr-review/SKILL.md b/skills/pr-review/SKILL.md index 0337d38f..84a5afa0 100644 --- a/skills/pr-review/SKILL.md +++ b/skills/pr-review/SKILL.md @@ -442,17 +442,19 @@ normal scope (current behavior preserved). | PR type | Complexity | Agents dispatched | |----------------------------------------------------------|------------|----------------------------------------------------------------------------------| -| Deletion of CI config + README edit (2 files) | simple | correctness (trivial scope), style-conventions (trivial scope), challenger | -| Deletion-only removal of a build task | simple | correctness (trivial scope), style-conventions (trivial scope), challenger | -| Typo fix in README | simple | correctness (trivial scope), style-conventions (trivial scope), challenger | +| Deletion of CI config + README edit (2 files) | simple | correctness (trivial scope), style-conventions (trivial scope) | +| Deletion-only removal of a build task | simple | correctness (trivial scope), style-conventions (trivial scope) | +| Typo fix in README | simple | correctness (trivial scope), style-conventions (trivial scope) | | Implementation plan | standard | correctness, style-conventions, intent-coherence, docs-currency | | Bug fix in auth middleware | standard | correctness, security, style-conventions, intent-coherence | | New API endpoint with tests | standard | correctness, security, style-conventions, cross-repo-contracts | | Large refactor across packages | standard | correctness, style-conventions, intent-coherence, docs-currency | | CI/CD pipeline change | standard | correctness, security, style-conventions, intent-coherence | | DB migration + API change | standard | correctness, security, style-conventions, cross-repo-contracts, docs-currency | -| Re-review after fix (prior findings in correctness only) | standard | correctness (full scope), style-conventions (trivial scope), challenger | -| Re-review after fix (prior findings in security only) | standard | correctness (full scope), security (normal scope), style-conventions (trivial scope), challenger | +| Re-review after fix (prior findings in correctness only) | standard | correctness (full scope), style-conventions (trivial scope) | +| Re-review after fix (prior findings in security only) | standard | correctness (full scope), security (normal scope), style-conventions (trivial scope) | + +The challenger always runs after all dimension sub-agents complete (step 6d) — it is not listed per row because it is never conditional. #### 3c-1. Security-critical file triage (large PRs)