An invoice-export pull request passed 47 tests, produced correct CSV files, and still created a second authorization rule that would let future exports bypass the company’s retention policy. This rubric is designed to catch that kind of maintainability failure before merge: code that works locally but makes the next change more expensive and less safe.

The problem is not that AI-written code is uniquely broken. A rushed engineer or an inexperienced engineer can make the same mistakes. The AI-specific risk is the combination of plausible local code and incomplete repository context: a model can confidently reproduce a stale pattern, invent an internal utility that does not exist, or solve a request in three files while missing the one service that already owns the policy.

That distinction matters as generated-code volume rises. InfoQ’s coverage of Ox Security’s Army of Juniors: The AI Code Security Crisis report describes AI-generated code as highly functional while lacking architectural judgment, and reports recurring architecture and security anti-patterns. Treat that as a warning about observed patterns, not proof that every AI-authored line is worse than human code. The practical response is a review process that examines evidence beyond “the diff is readable” and “CI is green.”

Why passing tests are not enough

Most pull request review already asks whether the change behaves correctly. That is necessary, but it checks only the shortest path from request to output. Architectural debt usually appears in the second or third change: when legal retention rules change, when another export format is added, or when an on-call engineer needs to identify why a job ran twice.

AI assistants are especially likely to optimize for the immediate acceptance criteria supplied in a prompt. If the task says “add invoice CSV export,” a model may add a route, query the database, serialize rows, and add happy-path tests. It may not know that exports must go through an existing asynchronous job system, that authorization belongs in a policy object, or that audit events must use a shared event schema.

  • Correctness debt: generated tests assert the same flawed assumptions as the implementation.
  • Policy debt: authorization, retention, redaction, or validation is copied into a new path instead of reused.
  • Ownership debt: a controller, UI component, or worker gains business logic owned by another layer.
  • Operational debt: a new path skips metrics, audit events, idempotency controls, or retry conventions.
  • Dependency debt: one generated import adds a package that overlaps with an existing capability.

The review target is therefore not “is this AI code?” It is “what new capability, policy, and maintenance obligation does this diff introduce?” The five gates below score those questions separately so a reviewer does not accidentally count “matches a nearby file” as evidence for everything.

Use five independent gates and a 10-point merge rule

Score each gate from 0 to 2. A score of 2 means the PR includes direct evidence; 1 means the design is plausible but incomplete; 0 means missing, contradicted, or unreviewable. Do not award points for confident prose in an AI-generated PR description. Award them for tests, repository references, diagrams, search results, and named owners.

Gate 0 points 1 point 2 points
Tests No new relevant test, or tests only snapshot implementation details. Happy path and one failure case exist, but an important boundary is untested. Tests cover behavior, failure or denial, and the external contract; generated tests do not merely mirror private helpers.
Dependencies New package, service, or SDK has no necessity or ownership explanation. Need is explained, but alternatives, license/security review, or lifecycle ownership is unresolved. Existing capability was checked; the addition has an owner, purpose, removal/lifecycle plan, and required approval.
Security-sensitive paths Auth, tenant scope, secrets, PII, payments, or destructive actions are changed without traced controls. Controls are present but one boundary—such as audit logging, rate limiting, or object-level authorization—is unproven. Reviewer can trace identity, authorization, input handling, data classification, audit behavior, and failure behavior end to end.
Duplicated policy or capability The diff reimplements an existing rule, serializer, client, validation rule, or integration. Some reuse exists, but a policy is copied or two paths can drift. The change extends a shared capability, or documents why separate behavior is intentionally required.
Architectural fit Wrong ownership, layer, contract, or operational convention; no design decision exists. Placement is reasonable, but ownership or migration implications need a named follow-up. Responsibilities, layer boundaries, contracts, observability, and deployment/runtime conventions match the intended design.

Merge rule: merge at 9 or 10 with no blocker. One score of 1 is acceptable only when the PR author creates a linked follow-up with an owner, acceptance criteria, and a due release or date. Two scores of 1 require staff or tech-lead approval. Any 0 blocks merge.

Some changes also require a blocking reviewer regardless of score:

  • A new production dependency requires the designated dependency owner or platform maintainer.
  • A change touching authorization, secrets, payment flows, personal data, tenant isolation, or destructive actions requires the security or domain owner.
  • A new public API, database migration, background-processing model, or cross-service contract requires the owning architecture or service team.

Copy this one-page PR review artifact

Put this in the pull request template or paste it into the review summary. It turns a vague “looks good” into an auditable decision. The author should fill it in before requesting review; the reviewer changes scores only when the linked evidence does not support them.

## AI-assisted change review

Change map
- User-visible behavior:
- Files/components changed:
- Existing capability searched for:
- Source of truth for policy:
- New dependency/service/API:
- Security-sensitive data or action:
- Operational signals added or reused:

Scores (0-2)
- Tests: __ / 2
  Evidence: tests, cases, and contract asserted
- Dependencies: __ / 2
  Evidence: alternatives checked, owner, approval
- Security-sensitive paths: __ / 2
  Evidence: auth/data/audit trace
- Duplicated policy or capability: __ / 2
  Evidence: reused component or intentional exception
- Architectural fit: __ / 2
  Evidence: owner, layer, contract, operations

Total: __ / 10
Blocking reviewer required? __
Follow-up issue, owner, and due release: __
Novel-pattern decision record, if applicable: __

Keep the change map short. For a 300-line PR, four bullets are usually enough. The useful part is not the form itself; it is the requirement to name the policy source and existing capability before a reviewer spends 30 minutes commenting on naming and formatting.

Recognize AI-specific failure signals before they become debt

These signals are not exclusive to AI, but they are common when code is generated from a narrow prompt rather than repository-wide understanding. Each has a fast reviewer check and a concrete correction.

Failure mode Reviewer signal Corrective action
Repository-context hallucination The PR imports TenantExportService, mentions a package, or calls a method not found in the repository. Run git grep -n "TenantExportService"; replace invented abstractions with real interfaces or add a deliberately designed one.
Confident internal-utility invention A helper has a generic name such as safeFetch or validateUser, but bypasses the project’s established client or validator. Search for the existing capability and consolidate; if none exists, assign an owner and define the new utility’s supported contract.
Stale-pattern refactor The model copies an old controller or deprecated client because it was prominent in local context. Check recent commits, deprecation notes, and current call sites; narrow the diff rather than spreading an obsolete pattern.
Implementation-mirroring tests Tests mock every helper and assert that private methods were called, while no test checks an unauthorized or malformed request. Replace one or more mocks with contract-level tests covering input, observable output, and a rejected path.
Policy reproduction across files The same role check, field-redaction list, or retry rule appears in a route, worker, and UI. Move the rule to its policy owner; add a test that proves all callers receive the same decision.

A useful command sequence is git grep -n "export", git log -S "retention" --all, and a search for the relevant domain type. This does not prove the architecture is right, but it quickly exposes whether the generated PR invented a parallel system.

Walk through the invoice-export PR

Assume the product requirement is: account administrators can export invoices as CSV. The generated PR appears compact and clean:

+ POST /api/invoices/export
+ InvoiceExportController.export()
+ CsvWriter.ts
+ exportInvoices.test.ts
+
+ if (user.role === "admin") {
+   const invoices = db.invoice.findMany({ where: { accountId } })
+   return csv(invoices)
+ }

CI passes. The generated test creates an admin user, three invoices, calls the endpoint, and asserts that the response contains three CSV rows. A conventional review may approve it because the endpoint is authenticated and output is correct.

The reviewer instead starts with the change map and finds an existing ExportJob pipeline. That pipeline applies retention rules, emits an audit event, stores generated files for a limited period, and uses the shared canExportFinancialData() policy. The direct endpoint skips all four behaviors.

The review comments should be precise:

  • Comment on controller: “This duplicates financial-data authorization. Please call canExportFinancialData(user, account); user.role === "admin" does not cover account membership or suspended accounts.”
  • Comment on query: “Why is CSV generation synchronous here? Existing exports use ExportJob so audit events, expiration, and large-account behavior are consistent. Please extend that flow or document an intentional exception.”
  • Comment on test: “This test repeats the implementation’s admin assumption. Add a denied cross-account request and assert the audit event and job payload contract.”

The initial score is Tests 1, Dependencies 2, Security-sensitive paths 0, Duplicated policy or capability 0, Architectural fit 0: 3/10, blocked. The fact that no dependency was added is good, but it cannot compensate for bypassing policy and operations.

The revised design changes POST /api/invoices/export to enqueue ExportJob.create({ type: "invoice-csv", accountId, requestedBy }). The job invokes the shared policy, uses the existing export storage lifecycle, and emits the standard audit event. Tests cover an authorized request, cross-account denial, the queued job payload, and expiration metadata. The revised score is 10/10.

Do not preserve a bad pattern just because it already exists

“Follow existing conventions” is a dangerous shortcut when the existing convention is known to be weak. For example, if five services each implement their own retry loop, adding a sixth matching loop earns local consistency but worsens operational ownership. The duplication gate should score that as 0, not 2.

Introduce a new pattern when all three conditions are true:

  • The existing pattern cannot meet a current requirement such as tenant isolation, observability, security, or reliability.
  • The PR names the intended owner of the new shared pattern.
  • The change includes a migration or deprecation decision, even if migration happens later.

Document the exception in a lightweight architecture decision record: the old approach, the failure it creates, the new contract, affected callers, migration owner, and retirement condition. A good PR note is specific: “New ExportPolicy replaces route-local role checks. Billing owns migration of three existing export routes by release 2026.4; route-local checks will then be removed.”

Novel work is not a review failure when no comparable implementation exists. It is a design decision. In that case, require an explicit rationale, an accountable owner, and a statement of what future code should reuse. Score architectural fit as 1 until that decision exists; score it 2 once the new boundary, operational behavior, and adoption path are clear.

Adopt the rubric in one week

Start with only AI-assisted PRs larger than 100 changed lines or touching authentication, data export, payment, storage, or external APIs. Ask authors to add the review artifact and calculate a score before requesting review. This creates a small amount of upfront work, but it replaces long comment threads with a shared decision rule.

  1. Day 1: add the scorecard to your pull request template and define your security and dependency approvers.
  2. Day 2: review three recently merged AI-assisted PRs retrospectively. Score them without changing code; use disagreements to calibrate the team.
  3. Day 3: identify two repository searches reviewers should run for common domains, such as git grep -n "authorization" and git grep -n "audit".
  4. Days 4–5: enforce blockers for new dependencies and security-sensitive paths, then collect examples of scores that changed the implementation.

The long-term benefit is not stricter review for its own sake. It is keeping AI’s speed at the code-writing layer while preserving human judgment at the boundary where code becomes a system: policy ownership, operational behavior, and the next engineer’s ability to change it safely.