Skip to content

feat(review): adversarial code-review workflow + findings convention (ADR 0077)#1851

Merged
protoquinn[bot] merged 1 commit into
mainfrom
feat/adversarial-review-workflow
Jul 6, 2026
Merged

feat(review): adversarial code-review workflow + findings convention (ADR 0077)#1851
protoquinn[bot] merged 1 commit into
mainfrom
feat/adversarial-review-workflow

Conversation

@mabry1985

Copy link
Copy Markdown
Contributor

M1 of the codified-delivery-loop plan (#1850). The review step of the delivery loop as a first-class, headlessly-callable workflow.

What ships

Piece Where
Findings convention — schema, FINDINGS_CONTRACT prompt snippet, tolerant parse_findings, render_findings_markdown graph/review/findings.py
review-finder (one angle per dispatch, github-diff tools) + review-synthesizer (dedup/rank, never adds) roles graph/subagents/config.py
verifier grows a code-findings mode + github read tools (verdicts from re-reading the diff, not vibes) graph/subagents/config.py
code-review recipe: 4 finders ∥ → dedup → one verify pass → report ending in canonical fenced findings JSON plugins/workflows/recipes/code-review.yaml
craft /code-review becomes a thin workflow driver (old two-axis review = fallback) plugins/craft/skills/code-review/SKILL.md
ADR 0077 + index row + nav regen docs/adr/

Design notes

  • The engine stays string-based (ADR 0002); the findings contract is a convention layered on prose, tolerant by construction — that's what lets the M5 board gate parse the same output the human skill presents.
  • Per-finding verify fan-out is deliberately parked: it needs a data-driven engine primitive. One verify pass over the merged list is the honest compromise; extend the engine only if it demonstrably misses.
  • New roles set allow_skill_emission=False — per-invocation verdicts must not pollute the skills index.
  • Missing github tool names resolve to nothing when the plugin is off, so research-only deployments are unaffected.

Verification

Acceptance (/code-review <PR#> on a real PR) exercised after the github-plugin PR merges and the working-tree plugin is synced.

🤖 Generated with Claude Code

@protoquinn

protoquinn Bot commented Jul 6, 2026

Copy link
Copy Markdown

👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 4 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7ba2ca5b-99ba-4dd6-867f-d6597d5d9e0a

📥 Commits

Reviewing files that changed from the base of the PR and between 1359dd0 and 9e20987.

📒 Files selected for processing (10)
  • docs/adr/0077-adversarial-code-review-workflow.md
  • docs/adr/index.md
  • graph/review/__init__.py
  • graph/review/findings.py
  • graph/subagents/config.py
  • plugins/craft/skills/code-review/SKILL.md
  • plugins/docs/nav.json
  • plugins/workflows/recipes/code-review.yaml
  • tests/test_code_review_workflow.py
  • tests/test_review_findings.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/adversarial-review-workflow

Comment @coderabbitai help to get the list of available commands.

@protoquinn protoquinn Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA Audit — PR #1851 | feat(review): adversarial code-review workflow + findings convention (ADR 0077)

VERDICT: WARN (pending CI — approve-on-green policy promotes to APPROVED once all checks settle terminal-green)


CI Status

  • All 8 checks in_progress: build, Lint, Python tests, A2A live smoke, Fleet integration, Web E2E smoke, Verify workspace config, gitleaks

Diff Review

  • 10 files, +755/−43: ADR 0077 doc, graph/review/findings.py (findings convention module), new review-finder/review-synthesizer roles, code-review.yaml recipe, craft skill update
  • Design is sound: the findings contract (schema → prompt snippet → tolerant parser → renderer) is exactly the right shape for an LLM-facing string-based engine
  • Recipe mirrors the proven deep-research pattern (parallel gather → dedup → verify → report); per-finding verify fan-out deliberately parked per ADR text

Observations

  • LOW: Clawpatch structural review timed out — diff-based review only
  • Gap: Companion PR protoLabsAI/github-plugin#20 (provides github_pr_diff — the finders' diff source) not verified; finders depend on it
  • Gap: findings.py truncated at ~209 lines in the diff preview — parse_findings / render_findings_markdown implementations not visible to this review pass
  • Gap: Tests referenced (15 new, 3304 passed) but not visible in the diff
  • Note: The visible findings.py code is clean — _VERDICT_ALIASES mapping, SEVERITIES tuple, FINDINGS_CONTRACT prompt snippet, and dataclass shape are all well-structured

— Quinn, QA Engineer

@protoquinn

protoquinn Bot commented Jul 6, 2026

Copy link
Copy Markdown

Submitted COMMENT review on #1851.

…(ADR 0077)

The delivery loop's review step, codified (plan M1):

- graph/review/findings.py — THE findings convention: schema [{file, line,
  severity, category, claim, evidence, verdict?}], FINDINGS_CONTRACT (the
  prompt snippet producers embed — schema written down once), tolerant
  parse_findings (JSON-in-prose: fenced preferred, bare arrays, coerced
  fields, verifier-vocabulary normalization), render_findings_markdown.
- review-finder / review-synthesizer subagent roles — one diff-reading lens
  per finder (angle passed per-step), synthesizer dedups/ranks and never adds
  findings; both allow_skill_emission=False. verifier gains a code-findings
  mode + github read tools so verdicts come from re-reading the diff.
- code-review.yaml — 4 finder angles ∥ (correctness / removed-behavior /
  cross-file / conventions) → dedup → ONE verify pass over the merged list
  (per-finding fan-out parked; needs a data-driven engine primitive) →
  report ending in the canonical fenced findings JSON.
- craft /code-review skill — now a thin driver for the workflow; the old
  two-axis inline review survives as the no-plugin fallback.
- ADR 0077 + index row + nav regen; 15 new tests.

Companion: github-plugin#20 adds github_pr_diff (the finders' diff source).
Consumed headlessly by the projectBoard review gate next (plan M5).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mabry1985 mabry1985 force-pushed the feat/adversarial-review-workflow branch from 31a1bef to 9e20987 Compare July 6, 2026 07:12

@protoquinn protoquinn Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA Audit — PR #1851 | feat(review): adversarial code-review workflow + findings convention (ADR 0077)

VERDICT: WARN (CI still settling — approve-on-green policy would promote to APPROVED once all checks are terminal-green, but build was observed as failure in the last snapshot. If build remains red the policy will not promote; the author must fix the build before merge.)


CI Status (last snapshot)

  • build: failure ⬅️
  • Lint (ruff + import contracts): success
  • Python tests: success
  • A2A live smoke: success
  • Fleet integration: success
  • Web E2E smoke: success
  • Verify workspace config: success
  • gitleaks: success
  • deploy: skipped

Diff Review

  • 10 files, +755/−43: ADR 0077, graph/review/findings.py, review-finder/review-synthesizer roles, code-review.yaml recipe, craft skill update
  • Design sound: tolerant JSON-in-prose parser, schema-in-prompt-snippet pattern, parallel-finder recipe mirrors proven deep-research shape
  • Clawpatch structural: no features touched (docs + config + new module — expected)

Observations

  • HIGH: build CI check observed as failure. Likely an import/packaging issue in the new graph/review/ package or recipe config — check build logs. A red build blocks the approve-on-green promotion even if all other checks pass.
  • Gap: Companion PR protoLabsAI/github-plugin#20 (github_pr_diff) not verified; finders depend on it
  • Gap: findings.py truncated at ~209 lines in diff preview — full file not inspected

— Quinn, QA Engineer

@protoquinn

protoquinn Bot commented Jul 6, 2026

Copy link
Copy Markdown

Submitted COMMENT review on #1851.

@protoquinn

protoquinn Bot commented Jul 6, 2026

Copy link
Copy Markdown

✅ CI went terminal-green with no blockers on the prior review — promoting it to APPROVED per the approve-on-green policy (#748).

@protoquinn protoquinn Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI terminal-green, no blockers on prior review — auto-approving on green (#748).

@protoquinn protoquinn Bot merged commit 22f8df9 into main Jul 6, 2026
10 checks passed
@protoquinn protoquinn Bot deleted the feat/adversarial-review-workflow branch July 6, 2026 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant