Splitting a Change into Units That Can Be Reviewed Independently

A change of six hundred lines touching fourteen files cannot be reviewed. That is not a claim about reviewer attention; it is a structural claim. Reviewing means holding a model of what the change is supposed to do and checking each line against that model. When a diff contains several intents at once — a bug fix, a rename, a formatting pass, and a new configuration path — the reader has no single model to check against, and the check degrades into reading code and hoping. The remedy is decomposition, and the useful unit of decomposition is smaller than a commit but larger than a hunk.

The unit is a reversible intent

The property that makes a change reviewable is that it can be described in one sentence without the word “and”, and that removing it leaves the system in a coherent state. Call this a reversible intent. A rename is one. A bug fix is one. A rename plus the bug fix that the rename made possible is two, because a reviewer cannot judge whether the bug fix is correct while the identifiers are also moving, and cannot judge whether the rename is complete while the logic is also changing.

Reversibility is what makes the intent a unit rather than a topic. A commit that adds a new function and also changes every call site to use it is one intent if the old function is removed in the same commit; it is two if the old function survives, because then the first commit is “add” and the second is “migrate”, and each is independently revertible. The test is mechanical: revert the commit in isolation and ask whether the tree builds and behaves as before. If not, the commit is not a unit, and whatever it depends on must either be included or landed first.

This test is stronger than it looks. It rules out the common practice of committing “the interface change” separately from “the implementation”, because the interface change alone does not build. It also rules out a commit that fixes a test and the code it tests, if the test change assumes the fix. What survives the test is a chain of commits each of which is green, and the chain order encodes the dependency structure of the work — which is exactly the information a reviewer needs to read them in order.

Mixed changes produce review deadlock

The practical failure of a mixed change is not that it is large. It is that different parts of it attract objections of different kinds, and the objections block each other.

Consider a change that renames a configuration key, updates the code that reads it, and adds a migration for existing files. The rename is correct but the migration has an edge case. A reviewer comments on the edge case; the author fixes it by changing the key again; now the rename portion has changed, so the reviewer must re-examine the parts already approved. Every round of review touches every intent. The change grows monotonically and the approval never converges.

Splitting avoids this because review state is per unit. Three units can be in three states: the rename approved, the config reader approved pending a question, the migration still under discussion. Fixes to the migration do not invalidate the rename’s approval, because the rename’s diff does not change. The reviewer’s mental model is discarded only for the unit that changed, and that is the whole economy of incremental review. It is also why approval has to be scoped to a revision rather than to a change: a per-unit state machine is only meaningful if the unit has an identity.

There is a second-order effect on defect detection. A reviewer who has approved part of a change tends to skim the rest, and the skimming is worse when the approved and unapproved parts are interleaved in one diff, because attention cannot be scoped to a hunk. A diff containing only the unapproved intent gets read at full attention; a diff containing both gets read at the average.

How the split is actually performed

Splitting is a mechanical operation on the index, not a rewrite of the work. The usual technique is to stage hunks selectively, deciding per hunk which intent it belongs to, and to commit the staged set before moving on. Hunks that contain two intents must be split at a finer grain, which means editing the staged copy by hand: stage the hunk, then unstage the lines belonging to the other intent, which in Git means editing the index directly rather than the working tree.

git add -p src/config/loader.py        # stage hunk by hunk
git diff --cached                      # inspect exactly what will land
git commit -m "Rename read_timeout to request_timeout"
git add -p src/config/migrate.py
git commit -m "Migrate legacy timeout key on load"

The step that is always skipped is the middle one. The staged set is a different object from the working tree and from the last commit; inspecting it before committing is the only way to know that the split is clean. A split commit that accidentally includes a stray line from the next intent is worse than no split, because the reviewer trusts the boundary and reads only what the boundary claims to contain.

When the intents genuinely interleave at the line level — a loop rewritten and a condition inverted in the same expression — the honest resolution is to rewrite the change as a sequence rather than to split the diff. Do the rename first as a pure rename, commit it, then make the semantic change on top. This produces two diffs that are each readable, at the cost of doing the work in a different order than it was discovered. That reordering is the actual price of decomposition, and it is usually small compared with the cost of reviewing a change that cannot be reviewed.

The cost side of the ledger

Splitting is not free, and the costs are real enough that a rule of “always split” is wrong.

The first cost is history noise. A chain of nine commits for what a reader will experience as one feature makes the log harder to read for anyone who wants the feature as a whole, and it complicates bisection: a bug introduced by the interaction of commit five and commit eight is attributed to whichever of them bisect lands on, which is not necessarily the commit whose author would recognise the defect.

The second cost is that intermediate states must be valid, and validity is expensive to arrange. A split that leaves the tree uncompilable between commits is not a split; it is a sequence of snapshots. Achieving compilable intermediate states sometimes requires writing temporary adapters, which then must be removed in a later commit — code that exists only to make a split possible, and that a reviewer must read and approve despite it being throwaway.

The third cost is that splitting can hide a defect at a boundary. A change that is correct as a whole and incorrect in each half is rare but not impossible: a rename that is only consistent once the second commit lands, or a migration that only works after the new reader is in place. Splitting such a change produces two commits that individually look wrong and jointly are right, and a reviewer reading them in isolation will object to both. The mitigation is to state the dependency in the commit message and to let the reviewer read the chain rather than the commits, which is exactly what stacked changes formalise.

What belongs in the message

If the unit is the intent, the commit message is the intent’s statement, and it has one job in review: to let a reader predict what the diff contains before opening it. A message that says “fix timeout handling” and a diff that also reformats a module makes the message a lie in the only sense that matters — it causes the reader to form a wrong model.

The useful shape is a subject line that names the single behaviour changed, and a body that states what the reader needs in order to judge it: the condition under which the old code was wrong, the input that demonstrates it, and any assumption the change relies on. That body is not documentation for the future; it is the reviewer’s brief. A body that explains why the change is correct is doing the reviewer’s job, and a body that explains only what files were touched is doing nothing.

Limits

Some changes resist decomposition. A dependency upgrade that changes an API across two hundred call sites is one intent, and splitting it per file produces commits that do not build. A mechanical reformat of a whole repository is one intent and cannot be split at all, which is why it should be landed alone and immediately followed by an entry in whatever file records ignored revisions, so that git blame and future diffs skip it. The general rule is that decomposition follows the dependency structure of the change, and where the dependency structure is one node, the change is one unit no matter how many lines it contains.

What decomposition cannot fix is a change whose intent is unclear to its author. Splitting such a change produces several unclear commits instead of one, and the reviewer’s problem is unchanged.