Pre-Commit Intelligent Code Review: Severity Tiers and Practice

Why Run a Separate Machine Review Before Commit

When implementation outpaces careful reading, review often degrades into “I glanced at the diff and nothing obvious failed, so it passes.” A clean compile only shows that syntax and type constraints are mostly satisfied. Unit tests usually cover only the paths the author already imagined. Null references, off-by-one errors at boundaries, treating value equality as object identity, wrong pagination offsets, and swallowed errors that still return success all belong to the class of bugs that compile, may not turn tests red, and only blow up after release.

That is why a fixed, structured review should run locally before commit or before opening a merge request. The input is the uncommitted or pending change. The job is only to find defects that change behavior, and to report each one by location, symptom, trigger, and impact—not as a vague style comment. People still own architecture, business rules, and tradeoffs. The machine filters repetitive, mechanical errors that are fatal to the result.

What This Review Should Answer

An effective pre-commit review is not a rewrite of the requirements. It is a differential reading of the change. Inputs should include at least the changed files and line-level diff, relevant call context, and the project’s existing error handling and data constraints. Output must be structured, not a prose verdict. Each issue should include four items:

1. Location: file and line number, plus the symbol name when needed.
2. Problem: which checkable correctness condition is violated.
3. How it fails: the smallest trigger, not “there may be a risk.”
4. Impact: wrong result, data corruption, privilege expansion, or silent failure.

Opinions without a trigger should default to suggestions and must not block the commit. That keeps formatting and naming preferences from drowning the review.

A sample issue record (a team convention, not a product API) can look like this:

[Defect] src/order/page.ts:42
Condition: page is 1-based and limit is a positive integer
Symptom: offset is computed as page * limit, so the first page is skipped and the second page overlaps the first
Impact: the list misses the first screen of data and repeats rows on the next page
Suggestion: offset = (page - 1) * limit, and reject the request when page < 1

Defects Worth Intercepting by Machine First

Narrow the scope so recall stays stable. Prefer classes that depend little on business vocabulary but directly change runtime results.

1. Nulls and uninitialized values. External input, query results, and optional configuration are used without a branch, so later property access or arithmetic fails at runtime. Follow the data source: on which branches can the value be null, and should the function then return an error, a default, or continue computing?

2. Boundaries and counts. Pagination, slices, loop bounds, timezone day cuts, and the smallest currency unit are where off-by-one errors hide. The test is not “the code looks symmetric.” Walk the minimum, maximum, empty collection, and single-element inputs in your head or in a comment.

3. Equality versus identity. Strings, enums, amounts, and timestamps should not rely on unreliable reference comparison. Floating-point money should not use equality checks that are sensitive to rounding error. Distinguish “the same object” from “equal in the business sense.”

4. Error handling. Catching an exception and returning success, ignoring a failed write, retrying without a cap, or returning the raw internal error to the caller all push the failure to a place that is harder to diagnose.

5. Concurrency and duplicate submission. A check-then-write with no constraint inserts twice when the same request is replayed. Ask whether the operation has an idempotency key and whether retry after failure is safe.

6. Authorization and data scope. Filtering only by primary key and omitting tenant or owner checks enables horizontal privilege escalation. Mark these high priority even when current callers “all look internal.”

Style, comment density, and variable length stay off the blocking list unless the project has a hard rule that auto-formatters cannot cover and that is already written into the standard.

Grade by Depth Instead of Loading Full Context Every Time

Review cost comes from context length and reasoning depth. In practice, split intensity into three tiers and pick by change risk. Do not use the deepest tier every time.

1. Fast. Read the diff and a few dozen neighboring lines. Target nulls, obvious comparison mistakes, log leaks, and leftover debug code. Fits copy changes, config tweaks, and pure renames. Keep the output to a handful of items that can be read in a few minutes.

2. Standard. Extend to callers and callees of the modified symbols. Check that parameter meaning, error codes, and return values match existing conventions. Fits everyday feature commits. Every defect must state its trigger.

3. Deep. Follow the data flow through persistence, cache, messaging, and external calls. Watch transaction boundaries, duplicate consumption, and whether compensation is paired. Fits payments, authorization, migrations, and batch jobs. A longer read is acceptable, but still report only reproducible behavior bugs—not refactoring ideas.

A workable team default: new APIs and data changes use Standard; money, identity, deletion, and cross-service calls escalate to Deep; pure formatting stays Fast. The tier is a process convention, not a hidden tool parameter. Map it onto the intelligent coding environment the team actually uses.

Boundaries of Automatic Fixes

After the issue list is confirmed, a tool may edit the code directly. Automatic fixes shorten the loop, but they must be constrained or the “review” becomes an uncontrolled second generation pass.

Suggested gates:

1. Fix only listed defects. Do not restyle or rename unrelated code along the way.
2. Handle one logical point at a time, then stop, so the patch stays reviewable.
3. After the fix, rerun tests or a minimal reproduction for that path. “It looks correct” is not enough.
4. Authorization, transactions, and migration scripts are not auto-edited by default. Provide a patch suggestion for a person to merge.
5. Keep the review notes next to the corresponding diff so mistaken fixes can be sampled later.

An acceptable loop is: produce the issue list, have a person check the items allowed to auto-fix, apply the patch, run tests, then run a Fast pass on the remaining diff. If that pass finds new issues, decide whether they are regressions from the fix or earlier misses. If two rounds in a row keep widening the change, stop automatic repair and hand the work to a person.

Illustrative flow (command names are placeholders; replace them with the equivalent steps in your environment):

1. Collect the working-tree diff
2. Choose Fast, Standard, or Deep by risk
3. Emit a structured issue list
4. A person confirms which items may be auto-fixed
5. Apply the smallest patch and run the targeted tests
6. Run one Fast pass on the remaining diff
7. Allow commit or a merge request only after that passes

What People Still Have to Read

An empty machine list does not mean design review can be skipped. Leave the following to someone who knows the domain:

1. Whether the change solves the right problem, not merely whether it fails to crash.
2. Whether the API leaks internal models, and whether the compatibility strategy is acceptable.
3. What the user sees on failure, and whether that matches the product promise.
4. Whether a data migration can be rolled back, and whether the signals are enough to judge a release.

One sentence covers the split: the machine supplies locatable evidence for “will this compute wrong, crash, or exceed authority”; the person owns “should we do it this way.” Neither replaces the other.

How to Fit the Flow into Daily Development

For individuals: before commit, always run a review matched to the risk, and keep the issue list and test results in the commit notes. Do not accept a whole-file rewrite without reading the diff.

For the team: add four checkboxes to the merge-request template—review tier, whether an automatic fix was included, the test command, and open questions that still need human judgment. Blocking conditions stay limited to correctness and security. Style belongs to the formatter.

Do not measure success by “more findings are better.” More useful is to sample two sets: among post-release rollbacks and hotfixes, how many were nulls, boundaries, or error-handling bugs that a pre-commit review could have caught; and what share of automatic-fix patches were later reverted. The first falling means the review works. The second rising means the gates are too loose.

Common Failure Modes

1. Treating review as chat. With no diff as input, the model can only repeat generic advice and has no hold on the current patch.
2. Context too narrow. Looking only at changed lines and missing call conventions turns a correct defensive check into a false “redundant” report.
3. Context too wide with no tier. Stuffing the whole repository in makes the conclusion longer and the point thinner, so authors stop checking item by item.
4. Automatic fixes without tests. The patch introduces a new boundary bug while the original issue is already checked off.
5. Using review as a substitute for coding standards. Problems that static checks and formatters can solve should not consume behavioral-review attention.

Summary

The value of intelligent pre-commit review is a fixed structure that catches defects which compile but produce the wrong result, depth tiers that control cost, and automatic fixes constrained by minimal patches plus regression tests. It is a first pass, not a replacement for design review. Once input, output, tier, and fix gates are written as conventions the team can execute, the speed of generative implementation does not turn directly into the speed of production incidents.