8a4239a9d9
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
159 lines
7.9 KiB
Markdown
159 lines
7.9 KiB
Markdown
# 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 1–2, 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 1–2. 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
|