review: skip redundant approvals, drop skipped-dimensions note, persist sub-agent outputs by jeresig · Pull Request #194 · Khan/actions

jeresig · 2026-07-02T16:13:06Z

What changed

Eight related refinements to the PR review workflow (workflows/review/review.md):

Skip redundant no-comment approvals. Before submitting, the reviewer checks whether the review would be a no-op repeat of the PR's current state — an APPROVE with no inline comments and no skipped-dimension notes — and the most recent github-actions[bot] review was already APPROVED. When all hold, it does not re-submit an identical approval. Steps 7 and 8 still run on the Step 4 verdict; only the redundant submit-pull-request-review call is skipped.
Persist sub-agent outputs as an artifact. The orchestrator writes each sub-agent's raw JSON to /tmp/gh-aw/review/out/<agent>.json and uploads that directory via a new upload-artifact safe output (30-day retention, scoped via allowed-paths) — a structured record of what each reviewer produced, for diagnosing and tuning.
Optional repo-specific correctness checks. The correctness-reviewer pulls in extra guidance via an optional {{#runtime-import? .github/aw/review/correctness-checks.md}}. Repos that provide the file get its checks; repos that don't are unaffected (the ? variant is silently skipped).
skip-ai-review label opt-out. Adding the skip-ai-review label skips automated review entirely via the job-level if: condition — a labeled PR never starts the agent (zero AI credits) and posts nothing. Gates each trigger going forward; does not retroactively dismiss a prior review.
Scope inline comments to newly-changed code. Each run records a content-based hunk signature in cache memory — one SHA-256 per hunk over its added lines only — and scopes new inline comments to hunks whose content is new since the last review. Keying on added-line content (not commit SHAs or line numbers) makes it survive force-pushes, rebases, squashes, and base-branch merges. Risk, patterns, and routing still use the whole PR. Narrow exception: genuine issue (blocking) correctness findings survive even on unchanged code; nits, suggestions, and all skill violations are scoped strictly to new code.
Only request changes when a posted comment is actually blocking. The Step 4 verdict is a mechanical function of the final comment set: REQUEST_CHANGES iff at least one posted comment carries a blocking label (issue (blocking), issue (blocking, best-practice), or todo (blocking)); APPROVE otherwise, including when only non-blocking comments are posted.
Pin model versions instead of floating tier aliases. The workflow used gh-aw tier aliases (opus/large/small) that route through a model gateway matching anthropic/*opus* / *sonnet* / *haiku* — i.e. any version. Now pinned: orchestrator engine and opus sub-agents → claude-opus-4-8; pattern-triage (was Sonnet-tier large) → claude-sonnet-4-6; reviewer-mapper (was Haiku-tier small) → claude-haiku-4-5.
Severity-based skill / best-practice flagging. Previously every skill violation was blocking. The skill-auditor now assigns each violation a severity of blocking or advisory — from the skill file's own declaration (skill-level default or per-rule annotation) when present, otherwise judged by impact (hard-requirement phrasing or correctness/security/data/compat risk → blocking; stylistic/preference → advisory; ties → advisory). blocking keeps issue (blocking, best-practice) and drives REQUEST_CHANGES; advisory gets a new suggestion (non-blocking, best-practice) label and rides along with an APPROVE. Builds on Add major version tags, to allow implicit upgrades for patch & minor versions #6 (verdict is label-driven) so no verdict-logic change was needed; claim-validator can correct an overstated severity.

Why

Try referencing one of our monorepo'd actions! #1 keeps the every-push workflow from re-posting identical approvals.
Readme, package.json, and changesets #2 captures each reviewer's output as structured data for diagnosing and tuning.
Bring over our composite actions from Khan/perseus #3 lets each consuming repo extend the correctness pass without forking the shared workflow.
Add scripts to build & publish our github actions, to 'bare tags' in the Khan/actions repo #4 gives humans a cheap, explicit opt-out.
Add eslint & prettier #5 addresses user reports of the reviewer re-reviewing already-covered code.
Add major version tags, to allow implicit upgrades for patch & minor versions #6 addresses a user report of a blocking review whose comments were all non-blocking.
Add a shared action for gerald, and use it! #7 stops the reviewer's behavior from drifting when a new model ships.
Use the local versions of our actions in our workflows, and add jest #8 stops minor best-practice nits from blocking a PR while keeping genuine convention breaches blocking.

The reviewer still notes which dimensions weren't assessed when a sub-agent's output is unavailable (a Note: <dimension> not assessed this run… line in the review body); that behavior is unchanged from main.

Reviewer notes

Why content-based scoping for Add eslint & prettier #5, not commit ranges: the github MCP has no compare-commits tool, and commit-range compares break on exactly the force-push/rebase case users hit. Hashing added-line content sidesteps that and needs no checkout/fetch-depth change or new tools.
Add a shared action for gerald, and use it! #7 verification: gh aw compile (v0.81.6) shows the engine pin resolving to ANTHROPIC_MODEL: claude-opus-4-8; full versioned IDs are accepted for sub-agents (0 errors). Orchestrator pinned to Opus 4.8 (applying the Opus rule to the main agent); Haiku-tier small pinned to the current claude-haiku-4-5.
Use the local versions of our actions in our workflows, and add jest #8 severity source: hybrid — skill files can declare severity (the authoritative source), with an auditor impact-judgment fallback so it works before any skill file is annotated. Skill violations stay scoped to newly-changed code (Add eslint & prettier #5) regardless of severity.
This workflow runs on every push, so Readme, package.json, and changesets #2 uploads one artifact per push. 30-day retention is deliberate; we can later gate uploads to REQUEST_CHANGES/error runs if noisy.
Validated with gh aw compile: compiles with 0 errors once the consumer-supplied .github/aw/review/config.md import is stubbed. The bare compile's only error is that missing consumer import, which fails identically on main.
Eight changesets included, all review: patch.

🤖 Generated with Claude Code

…rop skipped-dimensions note, persist sub-agent outputs Three related refinements to the PR review workflow: - Skip submitting a review when it would be a no-op repeat: an APPROVE with no inline comments where the PR's most recent github-actions[bot] review was already APPROVED. Steps 7 and 8 still run; only the redundant review submission is skipped. - Remove the "skipped dimensions" note that was appended to the review body when a sub-agent's output was unavailable, and all references to it. - Persist each sub-agent's structured JSON to /tmp/gh-aw/review/out/ and upload that directory as a run-scoped artifact (30-day retention) so a human can inspect exactly what each reviewer produced when diagnosing or tuning it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

changeset-bot · 2026-07-02T16:13:10Z

🦋 Changeset detected

Latest commit: eea90db

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
review	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

…port optional repo-specific checks Add an optional `{{#runtime-import? .github/aw/review/correctness-checks.md}}` to the correctness-reviewer sub-agent so a host repo can supply extra correctness things to validate. The `?` variant is silently skipped when the file is absent, so repos that don't provide it are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…view Add a `skip-ai-review` label check to the workflow's job-level `if:` condition so a human can opt a specific PR out of automated review. A labeled PR never starts the agent (zero AI credits) and posts nothing. The label gates each trigger event going forward; it does not retroactively dismiss a review already left before the label was added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…wly-changed code Users reported the reviewer re-commenting on code that a previous review already covered. The workflow reviews the full PR diff on every push, so it would regenerate comments on unchanged hunks. Fix it with content-based incremental scoping rather than commit ranges (the github MCP has no compare-commits tool, and commit-range compares break on force-push/rebase anyway): - Step 1 computes a per-file "hunk signature" — one SHA-256 per hunk over its added lines only (context/removed lines and line numbers excluded), so the hash is stable across force-pushes, rebases, squashes, and base merges. - It recalls the previous run's signature from cache and derives the set of hunks that are new since the last review, written to new-scope.json. - Step 3 drops candidate correctness findings / skill violations that aren't on newly-changed lines, so already-reviewed code is not re-flagged. Narrow exception: genuine `issue (blocking)` correctness findings survive even on unchanged code. Risk, patterns, and reviewer routing still use the whole PR. - Step 9 persists reviewedHunks each run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

… posted comment is blocking A blocking review was observed even though every comment it left was non-blocking. Step 4's verdict was a category judgment ("any blocking issues") rather than a check on the labels of the comments actually posted. Make the verdict a mechanical function of the final comment set's labels: REQUEST_CHANGES if and only if at least one posted comment carries a blocking label (`issue (blocking)`, `issue (blocking, best-practice)`, or `todo (blocking)`); APPROVE otherwise, including when only non-blocking comments are posted. Never request changes on a purely non-blocking comment set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…of floating tier aliases The workflow used gh-aw tier aliases (opus/large/small) that route through a model gateway matching "anthropic/*opus*", "*sonnet*", "*haiku*" — i.e. any version, so the reviewer could silently change behavior when a new model ships. Pin explicit versions: - Orchestrator engine → claude-opus-4-8 (engine object form) - opus sub-agents (correctness-reviewer, skill-auditor, thread-reconciler, claim-validator) → claude-opus-4-8 - pattern-triage (was `large`, Sonnet-tier) → claude-sonnet-4-6 - reviewer-mapper (was `small`, Haiku-tier) → claude-haiku-4-5 Verified with `gh aw compile`: the engine pin resolves to ANTHROPIC_MODEL: claude-opus-4-8 in the lock, and full versioned IDs are accepted for sub-agents (0 errors). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…agging severity-based Previously every skill violation was blocking. Now the skill-auditor assigns each violation a severity of `blocking` or `advisory`: - Source: the skill file's declaration (skill-level default or per-rule annotation) when present; otherwise the auditor judges by impact (hard- requirement phrasing or correctness/security/data/compat risk -> blocking; stylistic/preference -> advisory; ties -> advisory). - `blocking` -> `issue (blocking, best-practice)` (drives REQUEST_CHANGES). - `advisory` -> new `suggestion (non-blocking, best-practice)` label (rides along with an APPROVE). The Step 4 verdict is already a mechanical function of posted-comment labels (#7), so advisory violations no longer force REQUEST_CHANGES; only blocking ones do. claim-validator can correct an over-/under-stated severity. Skill violations remain scoped to newly-changed code (#6) regardless of severity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

jwbron

This mostly looks great! One issue though, removing the skipped-dimension note turns a degraded run into a silent one. If the correctness or skills pass dies, the workflow can still approve (or, with the new redundant-approval skip, post nothing at all) with no human-visible signal that a core dimension was never assessed, since the run artifact is somewhere nobody looks on an approved PR. Keep the artifact, but a missing correctness/skills output should stay visible in the review body (or better, hold the approval for a human rather than auto-approving on a partial review).

…side from claim-validator note Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…on note Bring back the note that tells the author which review dimensions were not assessed when a sub-agent's output was unavailable (or claim validation was skipped). Step 3 tracks the skipped dimension and Step 6 appends `Note: <dimension> not assessed this run (<sub-agent> output unavailable).` to the review body for either verdict. Reconcile with the redundant no-comment approval skip: a review carrying a skipped-dimension note is not a bare approval, so it is submitted rather than skipped. Drops the now-inaccurate "removes the note" line from the skip-redundant-approval changeset (net-vs-main the note behavior is unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

jeresig · 2026-07-02T20:34:46Z

@jwbron fair! I've added the note back in.

jwbron

Great stuff!

kevinb-khan · 2026-07-02T20:23:18Z

  !startsWith(github.event.pull_request.head.ref, 'deploy/') &&
-  github.event.pull_request.head.ref != 'changeset-release/main'
+  github.event.pull_request.head.ref != 'changeset-release/main' &&
+  !contains(github.event.pull_request.labels.*.name, 'skip-ai-review')


Out of curiosity, what is the .*. represent?

kevinb-khan · 2026-07-02T20:24:10Z

+  # Persist each sub-agent's structured JSON output as a run-scoped artifact so a
+  # human can inspect exactly what each reviewer produced when diagnosing or tuning
+  # the reviewer after the fact — this is the only place that reasoning is captured
+  # as clean structured data (the Actions logs and OTLP traces are harder to mine).
+  # The orchestrator writes each result to `/tmp/gh-aw/review/out/` (Step 3) and
+  # uploads only that directory (`allowed-paths`); 30-day retention gives a useful
+  # window for post-hoc review.
+  upload-artifact:
+    max-uploads: 1
+    retention-days: 30
+    allowed-paths:
+      - "/tmp/gh-aw/review/out/**"


This sounds like it'll be really useful in debugging odd/unwanted behaviours.

kevinb-khan · 2026-07-02T20:27:26Z

+**Compute the newly-changed-code scope.** So that Step 3 only comments on code this
+workflow has not already reviewed, work out which parts of the diff are *new since the
+last review* — by **content**, not by commit, so it survives force-pushes and rebases.
+For every changed file, split its `patch` into hunks and compute one hash per hunk: the
+SHA-256 of just that hunk's **added (`+`) lines**, each with the leading `+` stripped and
+trailing whitespace trimmed, concatenated in order. Deliberately ignore context lines,
+removed lines, and line numbers — a rebase, squash, or base-branch merge rewrites commit
+SHAs and shifts line numbers but does **not** change the text the author added, so a
+content hash of the added lines stays stable across all of those. Call this map
+`path → [hunkHash, …]` the **hunk signature**; you always compute it and save it as
+`reviewedHunks` in Step 9.
+
+Then recall `reviewedHunks` from cache memory (the hunk signature the previous review
+saved) and derive the scope:
+- **No prior review** of this PR (no `reviewedHunks` in cache) → the whole diff is new.
+  Do not scope anything this run; Step 3 reviews everything.
+- **Otherwise** a hunk is **in scope** (newly-changed) when its hash is **not** present
+  in `reviewedHunks[path]`. A file absent from `reviewedHunks` is entirely in scope
+  (newly touched). A hunk whose hash matches one the previous run already saw is **out of
+  scope** — already reviewed and unchanged since, even if a force-push or rebase rewrote
+  the commits around it.
+
+Write the result to `/tmp/gh-aw/review/new-scope.json` as
+`{"priorReview": true|false, "inScope": {path: [line, …]}}`, where the lines are the
+RIGHT-side line numbers of the added lines inside in-scope hunks. Step 3 uses this to
+filter candidate comments.


This seems like fairly complex logic. I wonder if it would make sense it to live in an external script that the workflow could reference?

kevinb-khan · 2026-07-02T20:30:02Z

+  **One exception:** keep a dropped candidate when it is a `correctness-reviewer` finding
+  whose `label` is `issue (blocking)` — a genuine blocking bug is worth surfacing even if
+  a change elsewhere introduced it on previously-reviewed lines. Nits, suggestions,


Will this allow such a finding to be reposted or do we guard against this elsewhere?

kevinb-khan · 2026-07-02T20:30:47Z

+acts on. (The existing `thread-reconciler` dedup remains a second layer: even an in-scope
+line that duplicates a still-open thread must not open a duplicate comment, Step 5.)


I think this addresses my previous comment.

kevinb-khan · 2026-07-02T20:35:36Z

+post. The verdict is a **mechanical function of the labels on the comments you will
+actually post** — the `correctness-reviewer` findings and `skill-auditor` violations that
+survived validation (Step 3 Phase 3), after any corrections and after the
+newly-changed-code scope filter. A claim the validator dropped or downgraded to


I'm having a hard time parsing the second sentenc in this paragraph.

task-1-1: workflows/review/lib/finding-schema.ts - FINDING_SCHEMA_VERSION exported constant; each finding carries schema_version and the validator rejects an unrecognized version (breaking-shape guard). - Finding shape (snake_case wire keys, matching #194 per-run JSON artifacts): id, lens, anchor, severity, confidence, evidence_trace, optional suggested_patch, optional pre_merge_obligation, producing_hunt, model_authored_prose. - Anchor is a discriminated union with the required PR-level type (pr | file | line, line supports side + multi-line start_line..line range). - KNOWN_LENSES (11 specialist + always-on/triage) and SEVERITIES exported for reuse by the slice-3 router and slice-2 verdict; lens/severity validated against them. - validateFinding collects ALL violations (diagnosable per-lens drop rate), plus isValidFinding narrowing wrapper and throwing assertFinding. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

… rendering (slice-2) task-2-1/2-2 verdict.ts: pure computeVerdict(labels + dimension availability + policy conflicts) -> APPROVE | REQUEST_CHANGES | HOLD_FOR_HUMAN. Consumes #194's mechanical label rule (isBlockingLabel); does not re-implement it. R2 gate: a missing core dimension (correctness/skill-severity) or a policy-named conflict holds for a human; a lost pattern-triage is note-and-continue. Documented default blocking threshold = 1 (tunable, not a HITL gate). task-2-3 render-comment.ts: code-owned Conventional-Comment label taxonomy + deterministic label-wrapping/templating from the finding schema; models own all prose. Review-body rendering mirrors review.md Step 6 + skipped-dimension notes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…lice-4) E1: correctness-reviewer names the high-risk trigger + one-line judgment in riskReason. E3: untrusted-input rule broadened to all PR content; injection attempts to steer the reviewer are themselves findings (issue (blocking)). E5: deletions are findings -- flag removals that drop a guard/check/test. R3b: pre-existing bugs on touched lines are fair to flag, scoped to touched lines, building on #194 severity (does not reopen it). E6: stage the full reply chain per bot thread; reconciler weighs author reasoning and never re-raises a conceded point. E7: stage open human-thread lines; reconciler emits skipLines and the orchestrator defers there (verdict + Step 5 kept consistent). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…, model+effort defaults, kept gates - task-6-1: define 5 always-on reviewers (holistic, completeness, test-adequacy, first-principles, conventions) as sub-agents with trust/advisory constraints. completeness reads Jira/Confluence read-only inside the non-posting sub-agent, fetched text = untrusted data (interface §4.4). first-principles advisory-only, never blocks, Fable 5 day one. conventions advisory, router-gated. - task-6-2: prompt edit 14 named mandates for holistic/completeness/first-principles. - task-6-3: R12 model launch defaults + effort table (Opus 4.8 workhorse; medium triage/reconciliation; high lenses/whole-change; xhigh claim-validator/refuters; first-principles=Fable 5). Effort recorded per role (annotation + authoritative table); gh-aw has no per-agent effort field. Does not re-pin #194's model pins. - task-6-4: always-on reviewers feed verdict/scope/claim-validator via the same findings path (no new gate); pattern-triage exclusions surfaced in guidance comment folded into risksPatternsKey; refuter panel noted as wave-2; dedup + verdict bookends + thread-reconciler preserved; advisory reviewers can never block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Pure, deterministic (no-model) thumbs sweep in workflows/review/lib/ thumbs-sweep.ts: - Two-grain collection: inline per-line review comments + the standalone summary PR comment, each keyed (grain, id) since the two id spaces can collide. - One follow-up per NEW thumbs-down offering the fixed reason vocabulary (incorrect / unimportant / unclear / duplicate) plus free text. - Idempotent / never re-ping: follow-ups carry a hidden HTML marker (#194-style) scanned on each poll; per-(grain,id) guard also dedups within a single sweep. No external state store needed. - Config-driven for either consumer repo via ThumbsSweepConfig (owner/repo/botLogin) with a full-error validator; all GitHub side effects sit behind an injected ThumbsSweepPort so the sweep is a pure, unit-testable function and deployable against Khan/webapp and Khan/frontend with no consumer commit (interface guarantee 4.3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…+ staged CI) R5 smoke benchmark as a tagged subset of the shared eval corpus, one harness: - workflows/review/eval/corpus/loader.ts (task-9-1): the single dataset format + loader shared with the slice-11 full suite. Reads corpus JSON recursively, validates recorded findings against the finding schema, and filters the smoke subset by the `smoke` tag (loadSmokeCorpus). fs is injectable for tests. - workflows/review/eval/corpus/smoke/*.json (task-9-1): ~a dozen cases — incident repros (sql/auth/money/race/cache), adversarial-injection PRs (untrusted-instruction handling, E3), known-clean PRs, plus R2 hold and scope-filter cases. All carry the `smoke` tag. - workflows/review/eval/runner.ts (task-9-2): a no-post run mode that replays the REAL deterministic review path over a case using the production lib modules (router.route -> labelForFinding -> newly-changed-code scope filter -> computeVerdict -> renderComment/renderReviewBody) and returns findings + verdict + the review it WOULD submit. Imports no GitHub client and performs no GitHub write (posted: false). A live producer can be injected for a full-eval arm while every downstream stage stays identical. - .github-staging/review-smoke.yml (task-9-4): dedicated CI entry point for the smoke set, staged because no producer role may push .github/ directly (#2508); the PR body must note the human move to .github/workflows/review-smoke.yml. Consumes (does not re-implement) #194's mechanical label model and the slice-1/2/3 determinism-boundary code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…e, gates, version stamp) task-11-1: four datasets loading via the shared loader — golden (human-comment + revert/follow-up ground truth), synthetic-mutations mapped to lenses, clean, and a non-smoke incident repro; holdout/fresh/adversarial tags for the gates. task-11-2: metrics.ts — five metrics (must-catch recall, golden precision, clean false-block, noise, calibration/ECE) computed over a corpus run, pure. task-11-3: judge.ts — Opus-4.8 LLM-judge with a pure request/aggregate core and an injected model seam; deterministic human-audit sample; thumbs (slice-8) calibration pass. task-11-4: gates.ts — golden holdout split + fresh mutations (overfitting report) and the adversarial set as a hard gate for automatic mode. task-11-5: version-stamp.ts — SHA-256 of prompt+config+schema rendered into the existing #194 pr-reviewer HTML marker; the single drift-guard surface (parse + hasDrifted for a consumer sync check). run-types.ts holds the shared EvalRun shape the three eval modules consume. No consumer-repo writes; no benchmark-gated construction. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…nditional-approval task-12-3 (R15): counters.ts — pure aggregation over #194 per-run artifacts + run summary: validator drop rate per source, comments/PR, verdict mix, thumbs agree rate, cost/run. No new logging mechanism; best-effort fs loader over the conventional artifact layout, defensive normalization from loosely-typed JSON. task-12-4 (R16): dismissal-learning.ts — mines resolved-without-fix threads, 👎-with-reply, and correct-pushback signals into candidate do-not-flag rules, emitted as a unified diff against a committed config for human approval. Never auto-applied; min-occurrences noise guard; deterministic ordering + diff. task-12-5 (R17): render-comment.ts — renderObligationsComment() builds the prominent pre-merge obligations comment from the schema's pre_merge_obligation field (verbatim), for the existing add-comment safe output; APPROVE body reflects the obligation count. Determinism boundary preserved (code owns structure only). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

… rendering (slice-2) task-2-1/2-2 verdict.ts: pure computeVerdict(labels + dimension availability + policy conflicts) -> APPROVE | REQUEST_CHANGES | HOLD_FOR_HUMAN. Consumes #194's mechanical label rule (isBlockingLabel); does not re-implement it. R2 gate: a missing core dimension (correctness/skill-severity) or a policy-named conflict holds for a human; a lost pattern-triage is note-and-continue. Documented default blocking threshold = 1 (tunable, not a HITL gate). task-2-3 render-comment.ts: code-owned Conventional-Comment label taxonomy + deterministic label-wrapping/templating from the finding schema; models own all prose. Review-body rendering mirrors review.md Step 6 + skipped-dimension notes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…lice-4) E1: correctness-reviewer names the high-risk trigger + one-line judgment in riskReason. E3: untrusted-input rule broadened to all PR content; injection attempts to steer the reviewer are themselves findings (issue (blocking)). E5: deletions are findings -- flag removals that drop a guard/check/test. R3b: pre-existing bugs on touched lines are fair to flag, scoped to touched lines, building on #194 severity (does not reopen it). E6: stage the full reply chain per bot thread; reconciler weighs author reasoning and never re-raises a conceded point. E7: stage open human-thread lines; reconciler emits skipLines and the orchestrator defers there (verdict + Step 5 kept consistent). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…, model+effort defaults, kept gates - task-6-1: define 5 always-on reviewers (holistic, completeness, test-adequacy, first-principles, conventions) as sub-agents with trust/advisory constraints. completeness reads Jira/Confluence read-only inside the non-posting sub-agent, fetched text = untrusted data (interface §4.4). first-principles advisory-only, never blocks, Fable 5 day one. conventions advisory, router-gated. - task-6-2: prompt edit 14 named mandates for holistic/completeness/first-principles. - task-6-3: R12 model launch defaults + effort table (Opus 4.8 workhorse; medium triage/reconciliation; high lenses/whole-change; xhigh claim-validator/refuters; first-principles=Fable 5). Effort recorded per role (annotation + authoritative table); gh-aw has no per-agent effort field. Does not re-pin #194's model pins. - task-6-4: always-on reviewers feed verdict/scope/claim-validator via the same findings path (no new gate); pattern-triage exclusions surfaced in guidance comment folded into risksPatternsKey; refuter panel noted as wave-2; dedup + verdict bookends + thread-reconciler preserved; advisory reviewers can never block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Per reviewer feedback on the roster strategy: the default roster stays exactly the #194 set (correctness, skill-auditor, pattern-triage, thread-reconciler, claim-validator) — zero new per-PR cost. The whole-change reviewers (holistic, completeness, test-adequacy, first-principles, conventions) land as capability only: - New 'enable <reviewer>' directive in the consumer ROUTING file; the router validates names against ENABLEABLE_REVIEWERS and surfaces the set as routing.json's enabledReviewers (empty by default). - review.md dispatches an opt-in reviewer only when enabledReviewers names it, and states the policy: a reviewer earns its enable line through the eval suite, never by shipping. - Model/effort table marks the opt-in rows and carries the cost note. - Align the completeness reviewer's embedded-instruction handling with the slice-4 steering-text rule (non-blocking note, not auto-blocking). - Strip remaining plan identifiers (R12, E3, R9, prompt-edit/wave numbers).