ReviewOS

stacks/buddy

AI: PR review engine (summaries, walkthroughs, line comments, incremental re-review)

#1375
Closed chris opened this 8 days ago ·
8 days ago

Part of #1367 (Phase 2 — AI Review Parity). The flagship parity feature vs CodeRabbit (PR reviews) and Pullfrog (PR reviews).

Goal

buddy-bot review + workflow triggers that produce CodeRabbit-class reviews on any PR (not just dependency PRs — this is the product expansion):

  1. PR summary + walkthrough: high-level summary, per-file change table, optional sequence diagram (mermaid), review-effort estimate
  2. Line-level comments: posted as a single PR review with inline comments; each finding has severity (critical/major/minor/nit), rationale, and a committable suggestion block when the fix is mechanical
  3. Incremental re-review: on new commits, review only the delta since the last reviewed commit (store last-reviewed SHA in a review-comment marker); never repeat resolved findings
  4. Review profiles: chill (fewer, high-confidence findings) / assertive (comprehensive) — CodeRabbit's profile concept
  5. Dependency-PR intelligence (our twist): on Buddy Bot's own update PRs, the review automatically assesses breaking-change risk from the fetched release notes (src/services/release-notes-fetcher.ts) and flags API changes that appear in the repo's usage — a review neither competitor can do with our context

Design

  • src/review/engine.ts: orchestrates — collect diff (via provider), chunk large diffs per-file with context windows, run agent in review mode (#1374), aggregate findings, dedupe, post
  • src/review/findings.ts: typed ReviewFinding { path, line, severity, category, message, suggestion? }; JSON-schema-validated model output (#1373 structured mode)
  • src/review/poster.ts: maps findings to GitHub review API (POST /pulls/{n}/reviews with comments[]); falls back to a single comment when line-anchoring fails (renamed/moved lines)
  • Posting state marker <!-- buddy-bot:review v1 {json} --> (same pattern as #1368) storing last-reviewed SHA + open finding fingerprints
  • Triggers in the unified workflow (src/setup.ts:generateUnifiedWorkflow): pull_request: [opened, ready_for_review, synchronize] gated by config; draft PRs skipped unless reviewDrafts: true
  • Severity gating: request_changes only at critical (configurable); otherwise COMMENT review

Config

ai.review?: {
  enabled?: boolean          // default false until GA
  profile?: 'chill' | 'assertive'
  drafts?: boolean
  autoReview?: boolean       // on PR open/sync
  ignoreTitleKeywords?: string[]   // e.g. ['wip']
  baseBranches?: string[]    // regex, default [baseBranch]
  ignoreUsernames?: string[]
  requestChangesOn?: 'never' | 'critical'
  summaryOnly?: boolean
}

Tasks

  • src/review/ per design; CLI command buddy-bot review <pr-number> (+ --summary-only, --profile)
  • Diff chunking that stays within model context (test with 100+ file fixture diffs)
  • Incremental re-review with SHA marker + finding fingerprints (no duplicate comments on re-push)
  • Committable suggestions using GitHub ````suggestion` blocks with exact line ranges
  • Dependency-PR breaking-change assessment path (reads the #1368 manifest + release notes)
  • Workflow trigger generation + loop guard (never review own bot commits reflexively)
  • Fixture-driven tests: findings schema validation, anchoring edge cases (file renames, deletions), profile behavior difference

Acceptance criteria

  • A test PR receives: one summary comment (walkthrough + effort estimate) and one review with ≥1 correctly-anchored inline comment with a committable suggestion
  • Pushing a new commit re-reviews only the delta; resolved findings are not re-posted
  • On a Buddy Bot dependency PR, the review includes a breaking-change risk section
  • With no API key: command exits gracefully, workflow step no-ops

Dependencies

8 days ago

Implemented in ca7a0d0.

src/review/ plus a buddy-bot review [pr-number] command that reviews a PR, or local changes against a base branch when no number is given.

Three decisions worth recording:

Findings are validated against the diff's own line map, not trusted. A finding on a line the diff does not touch is discarded rather than relocated — the model has misread the diff, and moving the comment somewhere plausible would present a guess as a located defect. Same rule applies to analyzer findings (#1378).

Fingerprints exclude the line number. A finding that survives an unrelated edit above it moves line but is the same finding; re-posting it on every push is the fastest way to make a review bot ignorable.

Lock files and build output are excluded by default, which is what makes reviewing a dependency PR affordable at all — and the summary names what it skipped rather than implying full coverage.

Review state (last-reviewed SHA plus reported fingerprints) rides in a marker on the PR, since a CI job has nowhere else to keep it. Also adds getPullRequestDiff and createReview to the provider; a review whose inline anchors GitHub rejects retries once as a summary-only comment, so one bad line number costs its own comment rather than the whole review.

Not yet wired: the pull_request workflow triggers for automatic review. That belongs with the comment-command plumbing in #1376, which shares the same trigger block.

Sign in to comment on this issue.