Files
pragent/docs/research/2026-08-04-prior-art-ai-code-review.md
T
Marcos 8a4239a9d9 docs: rework design after prior-art review
Red Hat's MIT ai-code-review already implements phases 1-2 (four forge clients,
six providers, CI integration, repo context file). Adds a research writeup,
inserts Phase 0 (evaluate it before building), and folds in seven requirements
the original design missed — chiefly prior-comment synthesis, without which our
own 1.7-runs-per-PR assumption means every push re-posts dismissed findings.

Amends implementation tasks 4, 5, 6, 9, 10 and gates the subagent briefs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Ye1KNFMkkUtmzTypHXkoK
2026-08-04 17:27:15 +00:00

159 lines
7.9 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Prior art review — Red Hat `ai-code-review`
**Date:** 2026-08-04
**Subject:** https://gitlab.com/redhat/edge/ci-cd/ai-code-review (MIT, Python, 249 commits, 40 tags, created 2025-08-29)
**Also published as:** `ai-code-review-cli` on PyPI
**Why it matters:** it is the same shape as pragent's Phases 12, already shipped and maintained.
## What it is
An MIT-licensed CLI that reviews local changes, GitLab MRs, GitHub PRs, and Forgejo PRs.
Runs as a CI job or locally in a container. Python, LangChain for provider abstraction,
Pydantic for config and structured output, Jinja2 for rendering.
Repository structure (via GitLab API):
```
src/ai_code_review/
cli.py
core/
base_platform_client.py ← our ForgeAdapter port, same idea
gitlab_client.py github_client.py forgejo_client.py local_git_client.py
review_engine.py ← one engine, not a plugin bus
providers/
anthropic.py anthropic_vertex.py gemini.py gemini_vertex.py
ollama.py openai.py ← our ModelClient port, already six impls
models/ config.py, platform.py, review.py, settings_sources.py
utils/ prompts.py, review_templates/*.md.j2
.ai_review/
config.yml.example
project.md ← our repo profile, same idea, hand/agent-written
```
The convergence is not a coincidence — CLI-in-CI, an adapter per forge, a provider port,
and a committed repo-context file are what this problem shape pushes you toward. That is
mild evidence our architecture is right, and strong evidence we should not spend weeks
rebuilding the parts they have already debugged.
## What they have that our design missed
These are real gaps, ordered by how much they matter.
### 1. Review-context synthesis (the big one)
`enable_review_context` fetches **all** prior comments and reviews on the MR — including
resolved ones — and `enable_review_synthesis` runs a **cheap model first** (Haiku / Flash /
`gpt-4o-mini`) to compress them before the main review, so the reviewer does not repeat
suggestions that were already made, addressed, or explicitly rejected by a human.
Our design has nothing here, and the omission is worse than it looks. Our own cost model
assumes **1.7 review runs per PR** — every push re-reviews. Without prior-comment context,
run 2 repeats run 1's findings and argues with the human who dismissed them. That is the
single fastest way for an AI reviewer to get muted, and we designed it in by accident.
Their two-phase structure is also the cheap fix: a small model compresses the comment
thread, the expensive model sees the summary.
### 2. Team/org context file, loadable from a URL
`team_context_file` accepts a local path **or a remote URL**, and outranks the project
context. One company-standards document, fetched by every repo, no copying.
We have org config layering (thresholds, locked keys) but no shared *review guidance*
document. For the "roll out across many projects" goal, this is the missing half.
### 3. Skip conditions we did not consider
Draft/WIP MRs, "WIP" in the commit message, `wip/` branch prefixes, bot commits, tagged
MRs. All deterministic, all free. Our tier engine only looks at paths and sizes — it would
happily spend $2 reviewing a draft.
### 4. Enterprise self-hosting details
`gitlab_url` / `github_url` / `forgejo_url`, `ssl_verify`, `ssl_cert_path`. Obvious in
hindsight and completely absent from our design. A self-hosted GitLab behind a corporate
CA is the *normal* case for the company deployment we are targeting.
### 5. Provider breadth as a hard requirement
Six providers including Ollama (local) and both Vertex variants. For regulated repos
"the diff never leaves our network" is a procurement requirement, not a preference. Our
`ModelClient` port allows this, but our plan pins Anthropic and never states the matrix.
### 6. Adaptive input clamping
`max_chars` defaults per provider (Gemini 200k, Anthropic 150k, Ollama 50k, OpenAI 100k),
plus `max_files: 100` and `exclude_patterns`. Our `oversized` tier caps files and lines but
never clamps characters, and our exclusion list lives only in the tier rules.
### 7. Smaller things worth stealing
- **MR summary generation** alongside findings (`include_mr_summary`) — users like it
- **`dry_run`** with mock responses — lets a team wire the pipeline before buying keys
- **Context7 integration** — pulls official library docs into the review; a good argument
for our profile-enricher extension point
- **Forgejo support** — Forgejo is a Gitea fork with a compatible API, so our Gitea adapter
should target both and say so
## What we have that they do not
This is the honest differentiation list. It is shorter than the gap list, but it is real.
| Capability | Them | pragent |
|---|---|---|
| Attention control | Binary skip / review | Four tiers with a recorded `tier_reason` per decision |
| Review dimensions | One engine, one prompt template | Analyzer plugin bus: per-analyzer model, effort, tool budget |
| Cost engineering | Char clamps per provider | Shared cached prompt prefix across analyzers, per-PR spend ceiling |
| Measurement | None | JSONL/OTel run records, `explain`, `replay`, finding-outcome feedback |
| Governance | Priority order (repo can override anything) | Org-locked config keys a repo cannot downgrade |
The measurement column is the one that matters. They cannot answer "what is the
false-positive rate of our security review, and did last week's prompt change improve it?"
Neither can CodeRabbit or Greptile. That is a real gap in the category, not just in this tool.
## Verdict
**Do not start Phase 1 as written.** Insert an evaluation phase first.
The plan currently spends 11 tasks rebuilding a local git adapter, a provider client, a
CLI, and config loading — all of which this project already has, tested, in six provider
variants, across four forges. Building that from scratch to *then* discover it behaves like
theirs is the expensive way to learn something a week of use would tell us.
Three paths, in order of my preference:
**A. Evaluate first, then decide (recommended).** Run their tool on real repos in the Gitea
setup for a week. Two outcomes, both useful:
- It covers ~80% of the need → pragent shrinks to what is genuinely missing (tiering,
analyzer bus, analytics), possibly built *on top of* or *contributed to* their CLI.
- It falls short in ways we can name → we build, with requirements informed by a working
baseline instead of by speculation.
**B. Fork and extend.** Take their platform clients and provider layer, add the tier engine,
analyzer bus, and analytics. Saves most of Phases 12. Costs: Python instead of TypeScript
(fine — the reviewed repos are polyglot either way), a plugin layer retrofitted into
someone else's architecture, and ongoing divergence from an actively developed upstream
(249 commits since August 2025).
**C. Build as planned, steal the ideas.** Keep our architecture, fold in the seven gaps
above. Cleanest design, most work, and it means maintaining a forge and provider matrix
that someone else maintains for free.
I recommend **A**, because it is cheap and it makes the choice between B and C on evidence
rather than on taste. The design and plan in this repo are not wasted either way — the
tiering model, analytics schema, and analyzer contract are what we would add to any base.
## Requirement changes regardless of path
Fold these into the design now, since they apply to all three options:
1. Prior-comment context + cheap-model synthesis before the main review
2. Team/org context document, local path or URL, outranking the repo profile
3. Skip conditions: draft, WIP commit/branch, bot author, tagged MR
4. Self-hosted forge URLs, `ssl_verify`, custom CA path
5. Provider matrix as an explicit requirement, Ollama included, with the diff-never-leaves
the-network case called out
6. Per-provider character clamp alongside the existing file/line caps
7. `--dry-run` with mock responses
8. Gitea adapter targets Forgejo too