Skip to main content

The PR review loop

The strongest thing ovecc does: tell you, deterministically, what a change introduced — not what's wrong with the codebase in general, but what this diff made worse. New findings with file:line, new dependency cycles with their concrete import witness edges, and added duplication, in one call.

The loop​

# 1. snapshot the base
git checkout main
ovecc index .

# 2. snapshot the head
git checkout my-feature
ovecc index .

# 3. what did the change introduce?
ovecc review

review compares previous → latest by default; pass explicit snapshot ids or Git refs for other pairs.

Reading a review​

Real output from a change that introduced a cycle, a hardcoded key, two injection sinks, an overweight function, and copy-pasted logic:

$ ovecc review
Review: fail (risk Critical) snapshot:85ee2372… -> snapshot:5ab78093…
  - 1 new dependency cycle(s): core ↔ services
  - 6 new security finding(s)
  - 1 new high-complexity function(s)
  - 2 new dead-code finding(s)
  - 1 new duplication(s)
New cycles:
  core <-> services
    src/core/db.ts:2 imports ../services/billing -> src/services/billing.ts
    src/services/billing.ts:1 imports ../core/db -> src/core/db.ts
New findings:
  [Critical] HardcodedSecret (src/services/billing.ts:5 (Stripe secret key)) Hardcoded secret: Stripe secret key
  [Critical] TaintedFlow (src/routes/api.ts:60 (sink: command)) Potential tainted flow: GET /admin/ping -> command (command)
  [Critical] TaintedFlow (src/routes/api.ts:55 (sink: eval)) Potential tainted flow: POST /admin/eval -> eval (eval)
  [High] InsecurePattern (src/routes/api.ts:55 (eval)) Dynamic code execution: eval
  [High] HighComplexity (src/services/orders.ts:8 (cyclomatic 24, cognitive 32)) High complexity: validateOrder (cyclomatic 24, cognitive 32)
  [High] CrossDomainDependency (src/core/db.ts:2 (../services/billing)) core -> services
  [Medium] TaintedFlow (src/services/reports.ts:4 (sink: reads)) Potential tainted flow: GET /billing/summary/:id -> invoices (reads)
  [Low] UnusedExport (src/services/reports.ts:16) Unused export: refundSummary in src/services/reports.ts
New duplications:
  89 tokens / 12 lines / 2 copies
    src/services/reports.ts:3-14
    src/services/reports.ts:16-27

Three properties worth noticing:

  • Cycles come with witnesses. Not "there is a cycle" but the two import lines that create it. You can fix it without an investigation.
  • Findings are matched by content identity (enclosing symbol / pattern), not by line number — a pre-existing finding that merely moved down the file is not blamed on your change.
  • A clean change passes quietly, exit code 0:
$ ovecc review
Review: pass (risk Low) snapshot:b3fb2f1b… -> snapshot:95c00fe4…
  - no new defects introduced by this change

A ready-to-post PR comment​

--format markdown renders the same result as a PR-comment-ready document, including per-finding remediation actions:

ovecc review --format markdown
# Change review: FAIL

- Risk: **Critical**
- Files: +2 added, ~3 modified
- 1 new dependency cycle(s): core ↔ services
...
## New findings (10)
- **[Critical] HardcodedSecret** (src/services/billing.ts:5 (Stripe secret key)) —
  Hardcoded secret: Stripe secret key _(rule `security/secret`)_ · action: `rotate_and_externalize_secret`
...

gate: the pass/fail verdict​

gate is the counts-only companion — the thing your CI branches on:

$ ovecc gate
Gate: fail (snapshot:85ee2372… -> snapshot:5ab78093…)
New cycles: 1, new modules: 0, new deps: 4, risk: Medium
  - 1 new circular-dependency component(s)
  - 1 new boundary violation(s)
  - 6 new security finding(s)
  - 1 new high-complexity function(s)
  - 2 new unused export(s)

Both review and gate accept --fail-on any|medium|high and exit 1 when the threshold is crossed — with the full result still printed, so the failing check carries its own explanation. Use review for humans and agents, gate when you only need the verdict, and diff when you want the raw added/removed modules and dependency edges behind it.

Progressive adoption on a legacy codebase​

Two tools keep the noise down while you dig out:

The baseline ratchet. Accept the existing backlog once; fail only on new findings:

ovecc violations --write-baseline     # once, on main
ovecc violations --baseline --fail-on any    # in CI

Change-scoped findings. Only report findings touching files changed since a ref:

ovecc violations --changed-since origin/main
ovecc deadcode  --changed-since origin/main

Gate the diff, not the backlog.

Wiring it into CI​

See CI integration for the GitHub Action (indexes base + head, posts the review markdown as a PR comment, gates on severity), the GitLab Code Quality recipe, and a generic shell pipeline for any CI system.