Striff Engineering Architecture
An architecture review checklist for pull requests, done by hand
Seven steps for reviewing a pull request for structural risk: what your own docs already say, new dependency directions, reaches into module internals, blast radius and cycles. Plus the arithmetic on how long it takes, and which steps a machine can do for you.
On this page
Most review checklists cover correctness, tests and style. Almost none cover structure: which new dependencies a change creates, what they point at, and whether the system still matches what its own documentation says about it. Structure is what degrades a codebase over years, and it degrades one reviewed pull request at a time.
This is the checklist we wish every team had. It is fully manual. Everything below can be done with an IDE, a search box and patience. At the end we do the arithmetic on how much patience, and say which steps can be handed to a machine and which cannot.
The checklist
Work through this on any change that adds imports, moves code, or touches shared components. In practice, that is most non-trivial changes.
Structural review, step by step
1. List the components, not the files
From the diff, write down every class or module touched and every new import. You are building a mental mini-graph: nodes and new edges. Files are how the diff is displayed; components are what the architecture is made of.
2. Read what your docs already say about them
Open the README, the architecture notes, the ADRs and the agent instruction files (AGENTS.md, CLAUDE.md) that describe the components on your list. For every sentence that names one of them (where it lives, what it calls, what it must never depend on), ask whether it is still true after this change. This is the highest-value step on the list and the one skipped most reliably, because the sentence that just became false sits in a file the diff does not contain.
3. Check each new edge's direction
For every new import: which package depends on which, and is that direction consistent with your layering? Core importing from a plugin, domain importing from infrastructure, shared utilities importing from a feature: each is one line in the diff and a boundary inversion in the graph.
4. Ask whether the target was meant to be reachable
Even when the direction is right, is the thing being imported part of that module's public surface, or one of its internals? A reach past a module's front door is how two modules stop being two modules.
5. Measure the blast radius of modified contracts
For every changed public interface, base class, or widely used type: find usages and count. A three-line change to something with twelve dependents is a bigger event than a five-hundred-line change to a leaf. Say the number out loud in the review.
6. Hunt for cycles, including near-cycles
For each new edge A → B, ask: is there any existing path from B back to A? If yes, this change closes a cycle. If a path gets within one hop, it plants a near-cycle seed. Flag it now, while the fix is one comment.
7. Ask the trend question
Is this the second or third change nudging the same component in the same direction? One convenient import is an exception; three are a new architecture nobody decided on. This is the step that catches drift, and the one that needs memory rather than analysis.
Steps 1, 3 and 4 need only the diff and the repository. Steps 2, 5, 6 and 7 need the rest of the system: every document nobody opened, every file the change did not touch, and in step 7, every previous change. That is why they are the ones that get skipped under deadline.
What step 2 looks like on a real pull request
Step 2 sounds like diligence. In practice it is a search problem, and a real example shows why.
Ericsson’s ecChronos documents its core.impl module class by class. Line 136 of that module’s README says that NodeWorker “Calls RepairScheduler.putConfigurations() to keep jobs up to date.” Pull request #1786 touched 27 files and, along the way, handed that call to SchemaRefresher. After it, NodeWorker does not reference RepairScheduler at all.
A reviewer working from the diff sees NodeWorker.java lose one field and gain another, which is a clean refactor. Nothing in the diff says that a README in another directory now describes the old design. Finding that out means knowing the sentence exists, which means having read every document that mentions every class on your step 1 list. The pull request merged, and at the time of writing the README still says it. The full story, and how it was caught, is here.
The cheat sheet
The compressed version, for pinning next to your review queue:
Signal, question, red flag
| You see in the diff | You ask | Red flag |
|---|---|---|
| A change to a component your docs describe | Does any sentence about it now read false? | The doc and the code disagree, and the doc is what the next reader, or agent, will believe |
| A new import | Which way does this edge point? | Toward a plugin, a feature, or anything "above" the importer |
An import of something named internal, impl, or similar | Was this meant to be reachable from here? | A module's internals being consumed from outside it |
| A moved class, or work moved between classes | What do its dependents import now, and which docs still describe the old home? | Dependents reaching across a boundary to follow it, or a doc describing a design that is gone |
| A changed interface or base class | How many dependents? (Count them.) | A double-digit count on a "trivial" change |
| A new edge A → B | Does any path lead from B back to A? | Yes (cycle), or almost (near-cycle) |
| A tiny diff on a core component | What is the blast radius? | Small diffs on high fan-in nodes hide the biggest surprises |
Every row here is something that happens in ordinary, well-reviewed pull requests. The first is the one this post opened with: a README still crediting a class with work a refactor took away from it.
The arithmetic
Suppose a competent structural pass takes fifteen to thirty minutes on a non-trivial change. At twenty minutes average:
Manual structural review, minutes per day
Arithmetic, not a study: count × twenty minutes, and the twenty is our estimate, not a measurement. Three hundred minutes is five senior-engineer hours a day, and it lands on your most senior people, because they are the only ones holding enough of the documents and the graph in their heads to do steps 2 and 5 to 7 at all.
This is why “we will just review more carefully” fails as a strategy at current shipping volume. The checklist is sound, but the budget for it does not exist. Teams skip structural review because it is the only review activity whose cost scales with the size of the codebase rather than the size of the diff, and no amount of caring changes that.
Skipping step 2 has a second cost that the chart does not show. Everyone who reads the stale sentence afterward, a coding agent using it as context included, acts on wrong information. Someone pays for that a second time, later, in a form that is hard to trace back to the doc that caused it.
Which steps a machine can take, and which it cannot
Any tool that claims to automate all seven of these is either overstating what it does or has quietly redefined “the trend question” into something smaller. Here is the split, including the one row we don’t try to automate:
Mechanical, and not
| Step | Automatable? | Why |
|---|---|---|
| 1. List components and new edges | ✓ Fully | Parsing two revisions and diffing the graph is exactly a computer's job. |
| 2. Contradicting your own docs | ✓ Mostly | A sentence that names real components and says where they live or what they depend on can be turned into a query and run at both revisions. A sentence about intent, taste or process cannot, and should be left out rather than guessed at. |
| 3. Edge direction, first-ever crossings | ✓ Fully | "Has this direction existed before" is a lookup, not a judgment. |
| 4. Reaching into internals | ✓ Fully | Module layout is in the repository; the comparison is mechanical. |
| 5. Blast radius of a changed contract | ✓ Fully | Counting dependents is counting. Deciding whether twelve is acceptable is yours. |
| 6. Cycles and near-cycles | ✓ Fully | Path-finding over a graph. The only hard part is having the graph. |
| 7. The trend question | ✗ Not really | "Is this the third change pushing the same way" needs a judgment about whether three instances make a direction. Tools can show you history; deciding it is a pattern is a human call, and pretending otherwise generates noise. |
Steps 1 and 3 to 6 have a right answer a program can compute. Step 2 has one for every sentence that is about structure, and that is most of them: in a scan of 609 public pull requests, 5,674 of the 7,161 rules read out of their docs could be answered from the parsed code. Step 7 does not have a right answer.
Step 2 is the one we automate. Striff reads the sentences in your own documentation as rules and checks them at both revisions of every pull request, and every pull request gets a diagram of the components it touched and how they connect, which is most of step 1 done before you open the diff. If your layering matters, write it down (“domain does not depend on infrastructure”) and step 3 is checked for that rule on every change too. On most pull requests nothing breaks a rule, and the check lists what it looked at. Install the GitHub App and keep the rest of the checklist for the steps that need you.