Code quality in the age of AI
· 27 min read
A version of the same question comes up in every conversation I have about coding agents: what happens to code quality when most of the code isn't typed by a person? The question usually assumes that quality was a property of the typing. It never was.
AI didn't change what good code is. It changed where the chain breaks. Writing got cheap. Review, understanding and responsibility didn't – and the pressure that used to sit on the keyboard now sits on them.
The thesis: quality in 2026 comes from the same things it always did – a well-designed codebase whose rules are actually enforced, deterministic CI underneath it, and a person who understands the system in broad strokes. What AI changed is the weak link: it moved from writing to review, and most of the fixes below are about making review possible again.
What code quality means in 2026
The same thing it meant before: competent people designed the architecture, and then it was enforced – hard, consistently, for years. In the agent era that is worth more, not less, because nothing is as effective as a good codebase. No rules file, no skill, no hook comes close. The agent does what it sees in the surrounding code.
A concrete example. File storage sits behind one abstraction, and Azure Blob, S3, MinIO and SharePoint are each an adapter behind it. When a new adapter is needed, the agent takes an existing one and follows the same shape: the same port, the same validation, the same place for the secret. If the same logic is scattered across five controllers in five different ways, the agent invents a sixth. Keeping the common modules and the domain logic in one place is the real prompt.
// What the agent sees decides what the agent writes.
// Five controllers, five ways. The sixth upload will be written a sixth way.
public async Task<IActionResult> UploadInvoice(IFormFile file)
{
var blobs = new BlobServiceClient(_config["Storage:Azure"]); // controller 1: Azure, inline
// …
}
public async Task<IActionResult> UploadContract(IFormFile file)
{
using var s3 = new AmazonS3Client(StaticKeys.Access, StaticKeys.Secret); // controller 2: S3, its own retry
// …
}
// One port, four adapters. The fifth adapter will look like the other four,
// because that is the only shape in sight.
public interface IFileStorage
{
Task<IFileInfo> GetAsync(StoragePath path, CancellationToken ct);
Task PutAsync(StoragePath path, Stream contents, CancellationToken ct);
Task DeleteAsync(StoragePath path, CancellationToken ct);
}
internal sealed class AzureBlobFileStorage : IFileStorage { /* … */ }
internal sealed class S3FileStorage : IFileStorage { /* … */ }
internal sealed class MinioFileStorage : IFileStorage { /* … */ }
internal sealed class SharePointFileStorage : IFileStorage { /* … */ }The counterpoint is worth saying out loud, because it is the same mechanism running backwards. In legacy code where the pattern itself is wrong, the agent scales the wrong thing – faster than any junior ever could. There, the pattern has to be fixed first. The agent comes after.
Where does that leave the rules file, the CLAUDE.md or AGENTS.md? It is part of the same architecture work, not a substitute for it. Its job is to point: here are the common modules, here are the internal packages we already trust, here are the tools that have been proven on this app – use them, don't reinvent them. It is a footnote in the sense that it refers to the code. A footnote can't carry the argument on its own.
The best prompt is the codebase the agent sees around itself. The rules file is a footnote to it.
Why classic code review is falling apart
The bottleneck moved to review. An agent writes two thousand lines in minutes. A person can't review that properly, so they start skimming, and in the end they approve it because the pipeline is green. The pull requests are too big – and that is not the reviewer's fault. It is the process's.
The second problem is the noise from review bots. The GitHub apps and the assistants chime in, and what they say is generally valid – and impossible in your context. An example that keeps coming back: the bot suggests caching the settings in memory, because it is faster. True. Except the app runs on several nodes, and the settings are synced from the database by a background job precisely so that every node sees the same thing. The "fix" gives each node its own state.
// The bot's suggestion, with a "fix it" button next to it. Generally valid. Here, wrong.
public sealed class SettingsProvider(ISettingsRepository repo, IMemoryCache cache)
{
- public Task<Settings> GetAsync(CancellationToken ct) => repo.LoadAsync(ct);
+ // Avoid a database round-trip on every request: cache the settings for ten minutes.
+ public Task<Settings> GetAsync(CancellationToken ct) =>
+ cache.GetOrCreateAsync("settings", entry =>
+ {
+ entry.AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes(10);
+ return repo.LoadAsync(ct);
+ })!;
}
// What the bot can't see: the app runs on three nodes, and a background job syncs settings into the
// database so that an edit on one node applies everywhere within seconds. With this change each node
// answers from its own copy – three nodes, three states, for up to ten minutes.
// The test suite runs on one node. Every test is green.This is the most dangerous point in the whole flow. A fix-it button puts it into the backend, every test is green because the tests run on one node, and production falls apart. I described the single-node version of this in the engineering mindset post; the bot version is worse, because the suggestion arrives with authority and a button. You have to know this before you press it. Nothing in the bot does.
Green on GitHub, red in production. The difference is context, and the context isn't in the bot.
What code review is actually for
Not for finding bugs – tests and CI do that better. Review is a check after the fact: that the conventions hold, that the domain grows in the right direction, and that the edge cases that came up reach the rest of the team.
That last one is the most underrated. Say the agent worked out a new edge case on a migration – what happens to half-migrated rows if the deployment is rolled back. Through the review, the other developers learn it. Without the review, the agent knows it, and tomorrow the agent doesn't remember.
The practical rule follows from that: small pull requests. A reviewer should have to understand one thing per PR. With an agent this is enforceable, because the task is sliced that way before the agent starts – the same reason the steps are small in the loop engineering post.
Review isn't a bug filter. It is knowledge transfer. If you only approve, the knowledge stays with the agent, and the agent won't remember tomorrow.
What to automate, and what not
Classic CI/CD is the base, and it is excellent: build, formatter, linter, type check, unit and integration tests, security scan. These are deterministic – the same code gets the same answer every time. If the AI parts are built on top of this and have to pass these gates, they are fine.
Wire the agent's review in, but not as scripture. It should flag, not fix. The fix-it button stays a human decision. The pattern that works: a scheduled pipeline runs the agent, the output is always a small PR, and the reviewer sends it back with an /iterate comment – the loop I built step by step in the loop engineering post. Every line of code passes through a person that way, which is also the version you can defend later.
The ideal agent job: a runtime upgrade
Moving from one long-term-support runtime to the next – .NET 8 to .NET 10, say – is where agents are close to perfect, and the reason is the sensor. The compiler is deterministic: zero errors and zero warnings is a condition you can state, check, and refuse to merge without. Then CI/CD puts the build into the QA environment, the automated tests run against it, and the runtime errors that only show up on deployment show up there. The whole thing is a pipeline with a well-defined pass condition at every step.
# A runtime upgrade is the ideal agent job: the compiler is the sensor, and it is deterministic.
# Scheduled, not triggered by a person. The output is always a small PR – never a merge.
on:
schedule: [{ cron: '0 2 * * 1' }] # Monday night, on the upgrade branch
jobs:
upgrade:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-dotnet@v4
with: { dotnet-version: '10.0.x' }
- run: agent run upgrade --target net10.0 --max-files 20 # one unit of work per run
- run: dotnet build -c Release -warnaserror # 0 errors, 0 warnings – or no PR
- run: dotnet test -c Release --no-build
- run: gh pr create --label agent-upgrade --fill # a person merges; /iterate sends it back
env: { GH_TOKEN: ${{ github.token }} }There is a career note hiding in that pipeline. The manual tester who used to click through the app after an upgrade is the person best placed to write the automated tests the pipeline needs – and to build the framework they run on. That is work with more value in it, not less.
What you don't automate: the decision whether a change fits the domain, and whether a bot's suggestion makes sense in your context. Those are the two questions a pipeline can't answer, and they are the two questions review exists for.
An agent review is a loud colleague in the meeting. Worth hearing. Not the one who decides.
New guardrails, and judges
The principle: whatever you caught twice in review becomes a rule that runs by itself. The agent produces mistakes faster than a person can filter them, so the conventions have to come out of people's heads and into the pipeline.
Concretely. Architecture rules as tests: the domain module doesn't reference an adapter, and adapters don't call each other. These used to be review comments; now they are red in CI.
// Architecture rules as a test: the things I used to type into review comments, now red in CI.
[Fact]
public void Domain_does_not_depend_on_adapters()
{
var result = Types.InAssembly(typeof(Invoice).Assembly)
.That().ResideInNamespace("Billing.Domain")
.ShouldNot().HaveDependencyOnAny("Billing.Adapters", "Npgsql", "Azure.Storage.Blobs")
.GetResult();
Assert.True(result.IsSuccessful, string.Join("\n", result.FailingTypeNames ?? []));
}
[Fact]
public void Adapters_do_not_call_each_other()
{
var result = Types.InAssembly(typeof(MinioFileStorage).Assembly)
.That().ResideInNamespace("Billing.Adapters.Storage")
.ShouldNot().HaveDependencyOn("Billing.Adapters.Email")
.GetResult();
Assert.True(result.IsSuccessful);
}Banned patterns, such as creating your own HttpClient instead of using the shared one that carries the proxy, the timeout and the retry policy. The analyzer turns the habit into a build error, with the reason attached:
BannedSymbols.txt
; BannedSymbols.txt – read by the BannedApiAnalyzers package. A hit is a build error (RS0030), not a comment.
M:System.Net.Http.HttpClient.#ctor;Inject IHttpClientFactory – the shared client carries the proxy, the timeout and the retry policy
M:System.DateTime.get_Now;Use TimeProvider – a test can't move the wall clock
P:System.Threading.Tasks.Task`1.Result;Await it – .Result deadlocks under a synchronisation contextA migration check that catches the operations that lock a Postgres table for the duration of the build. And a PR-size limit: above a line count the pipeline is red, and there is no discussion.
-- Caught twice in review. Now a CI step reads every new migration and fails on the first form.
-- Holds a lock that blocks writes for the whole build. On a two-million-row table that is the night shift's problem.
CREATE INDEX ix_invoices_customer ON invoices (customer_id);
-- Builds without blocking writes. Can't run inside a transaction – the migration tool has to know that.
CREATE INDEX CONCURRENTLY ix_invoices_customer ON invoices (customer_id);
-- The seatbelt for everything else: wait two seconds for the lock, then fail. Don't queue behind a
-- long report and block every write that arrives after you.
SET lock_timeout = '2s';
ALTER TABLE invoices ADD COLUMN archived_at timestamptz; -- metadata only: fast, whatever the row count# Above the limit the pipeline is red – no discussion, no "just this once". Slice the task instead.
- name: PR size
run: |
changed=$(gh pr view ${{ github.event.pull_request.number }} \
--json additions,deletions --jq '.additions + .deletions')
if [ "$changed" -gt 400 ]; then
echo "::error::$changed changed lines – the limit is 400. Split the PR."
exit 1
fi
env:
GH_TOKEN: ${{ github.token }}The judge, for what is left
The LLM judge is for the remainder – what can't be poured into a rule, such as whether the change fits the domain. And with a narrow rubric: not "is this code good?", but concrete yes/no questions about the diff. Does the new adapter implement the existing interface? Is there state in here that can differ between nodes? The judge is non-deterministic, so it is a signal, not a gate. When it is wrong twice on the same question, the question gets fixed.
# The judge gets the diff and yes/no questions – never "is this code good?". It is non-deterministic,
# so its answer is a comment on the PR, not a required check. Wrong twice on a question: fix the question.
judge:
output: pr-comment # a signal, never a gate
questions:
- id: port-shape
ask: Does the new adapter implement the existing IFileStorage interface without adding members to it?
- id: node-state
ask: Does the change introduce state that can differ between nodes – static fields, in-memory caches, local files?
- id: domain-fit
ask: Does the change put domain rules into a controller or an adapter instead of the domain module?
- id: migration-safety
ask: Does any migration hold a lock for longer than a metadata-only change would?What you said twice in review, the third time the pipeline says.
Is testing enough?
No. Tests are needed, and more than before: automated tests, your own test environment, canary builds, A/B tests. But a green test only tells you what the test checks. The agent writes the tests for its own code, so it proves what it implemented – not what you wanted.
The classic example is a list endpoint that queries the database once per item instead of once for all of them – the N+1. With fifty rows of test data it takes a tenth of a second, the test is green, and the UI works beautifully. In production, at two million rows, the database lies down. At three in the morning.
// Green with fifty rows of test data. With two million open invoices this is two million and one queries.
var invoices = await db.Invoices.Where(i => i.Status == InvoiceStatus.Open).ToListAsync(ct);
foreach (var invoice in invoices)
invoice.CustomerName = (await db.Customers.FindAsync([invoice.CustomerId], ct))!.Name; // one query per row
// One query, one page at a time, only the columns the endpoint returns.
var page = await db.Invoices
.Where(i => i.Status == InvoiceStatus.Open)
.OrderBy(i => i.Id)
.Select(i => new OpenInvoiceRow(i.Id, i.Customer.Name, i.Total))
.Skip(offset).Take(100)
.ToListAsync(ct);The same family: a parallel loop with no limit on how many tasks it starts, a query that loads the whole table into memory, the index nobody added, which you only feel at a hundred times the data. What works on the UI can have a bomb ticking behind it, set to go off at the worst possible time.
What to do about it: a staging environment with production-like data volume, a load test on the critical path, a canary that sends a small share of traffic to the new version with metrics on it. And a two-minute look at the diff, in broad strokes: is there a query inside a loop? An unbounded list? A call without a timeout or a cancellation token? Does the migration lock? You don't have to understand everything. Your eye has to catch on something.
A test tells you it works. Not whether it holds.
Readability and maintainability
Code is written for the other developers, not for the machine. The machine reads anything; your colleague doesn't, and in two months neither do you. No one-line miracles that take an afternoon to unpick later. The agent, left to itself, likes the clever abstraction, the generic solution, the layer on top of the layer. The measure is simple: does the next developer understand it in half a minute?
Half a sentence that matters: readability doesn't require a slower tool, least of all in syntax. Sixty lines of nested SQL and the same query as named steps give the same plan in most cases – one of them can be read top down.
-- The same query twice. The first one has to be read from the inside out.
SELECT c.name, t.total
FROM customers c
JOIN (SELECT customer_id, sum(amount) AS total
FROM invoices
WHERE status = 'open' AND due_date < now() - interval '30 days'
GROUP BY customer_id) t ON t.customer_id = c.id
WHERE t.total > (SELECT avg(x.total)
FROM (SELECT sum(amount) AS total FROM invoices
WHERE status = 'open' GROUP BY customer_id) x);
-- Named steps, read top down. The planner inlines these, so the plan is the same – only the reader is faster.
WITH overdue AS (
SELECT customer_id, sum(amount) AS total
FROM invoices
WHERE status = 'open' AND due_date < now() - interval '30 days'
GROUP BY customer_id
),
typical AS (
SELECT avg(total) AS total
FROM (SELECT sum(amount) AS total FROM invoices WHERE status = 'open' GROUP BY customer_id) per_customer
)
SELECT c.name, overdue.total
FROM overdue
JOIN customers c ON c.id = overdue.customer_id
WHERE overdue.total > (SELECT total FROM typical);Maintainability is following the patterns laid down by people who understood the system and then enforced them. Delete the branch, that's wrong, again – until everyone has learned what is allowed and what isn't. There is no other way. In the AI era this is cheaper than it has ever been: a rejected agent branch is ten minutes of regeneration, not a wasted human day, and nobody's feelings are hurt.
One thing to add, though: don't only throw away the output. Fix the context. If the agent got it wrong, either the pattern wasn't clear enough in the code, or a lint rule was missing. Regenerating from the same context gives you the same mistake.
Deleting a branch used to be expensive. Now it's free – so there is no excuse for being lenient.
Is the spec the new source of truth?
I don't think so. The running code is the truth: it does what it does. The description next to it can be stale. Don't hang enormous wikis on the AI that have to be kept fresh, because the agent follows a stale document exactly as faithfully as a fresh one – and that is worse than having nothing.
What works: the code documents the what – names, structure, tests as runnable examples – and the documentation supplies the why. A short decision record, for instance: why do we sync the settings from the database with a background job, and why don't we cache them in memory? That is the one thing you can't read out of the code, and it is exactly the thing the review bot from earlier didn't know.
docs/adr/0007-settings-sync.md
# ADR 0007 – settings are synced from the database by a background job, not cached in memory
**Status:** accepted · 2025-03
**Context.** The app runs on several nodes. Settings are edited in the admin UI on one node
and have to apply on all of them within seconds.
**Decision.** A background job on every node reloads the settings from the database every
five seconds. No per-request cache, no in-memory expiry.
**Why not an in-memory cache.** Each node would answer from its own copy for the lifetime of
the cache entry. Two nodes, two states – and a customer sees the old value from one of them.
**Consequences.** One cheap query per node every five seconds. Any "cache the settings"
suggestion – from a reviewer or from a bot – is answered by this file.The architecture diagram matters, but keep it small, keep it in the repository next to the code, and update it in the same PR. A spec for one task is fine: a short statement of intent in the PR description, short-lived. When the spec and the code disagree, the code wins – and the disagreement is a signal that either the spec needs updating or there is a bug.
The code says what it does. The docs say why.
Who writes the spec: the decision lives where the code is
A precise specification never arrives from the customer. It is assembled from words and follow-up questions: what do they actually want, and how does a blurry picture become a production-ready feature – with limits, with fallbacks, and with the things the customer never thought of but without which the feature isn't complete. This is also where it gets decided whether the thing is generic and useful to every customer, or built for one customer behind a feature flag.
The person who can decide that well is the one who knows the product's architecture, knows what the change touches, knows the use cases, and sees from the codebase who is working on what – not from a "I'm on this" status in a call. That is the technical lead, and it stays the technical lead even when they run a team. What a product needs is strong technical people, and above them a leader who is also technical, is inside the domain and the product's direction, and decides when a decision is needed.
A layer in between doesn't take work off that chain. It adds a round trip. If the go-between doesn't get proper information, doesn't pass proper information on to the technical people, or doesn't understand the request, the technical person ends up in the call anyway – or writes the answer to the customer themselves. Meanwhile the request has been distorted twice, on the way in and on the way out. The commercial context sits with sales, and sales gets a faster and more precise answer from the technical lead directly, at scale too, than through anyone in the middle.
The calls get faster too, and that is not a side effect – it is most of the point. With the technical lead on the call, the customer's question is answered on the call: whether it can be done, what it touches, roughly how much work it is, and what to do instead when the answer is no. Through a relay the same question turns into "I'll check with the team", a note that loses half the context, and a second meeting a week later to deliver an answer that may already be wrong. One call with the person who decides beats three calls with the person who forwards.
If the technical person has to join the call in the end anyway, the layer in between didn't take work off them. It added a round trip and two distortions.
The product role doesn't disappear. It gets distributed.
Saying "no product manager" is not saying "no product work". The work is real; the question is who holds each piece of it, and the answer is that it splits along the lines where the knowledge already is:
- Discovery and scope – the technical lead, who knows the product and the customer.
- Pricing, packaging, go-to-market – sales.
- Contracts and SLAs – legal.
- Scheduling and deadlines – delivery, worked out together with the technical lead.
There is one condition, and it is not a small one: the technical lead needs both the right and the spine to say no to sales – and the same weight with delivery. If a big deal lets sales overrule them, they aren't deciding about the product any more; the pipeline is. If a date set without them decides the scope, the calendar is. The schedule gets worked out together with delivery, with the technical lead at the table, not handed to the team afterwards. A technical lead who runs discovery but can't refuse a feature or move a date is the relay again, with a better title.
At scale: staff and principal engineers
With twenty teams you still need coordination; I am not arguing against that. I am arguing that the coordinator has to understand what they are coordinating. That is what staff and principal engineers are for: technical people who make decisions across teams, and who can read the code the decision lands in. Information isn't lost because of a title. It is lost with every step between the decision and the code, and a decision made far from the code is a bad decision whether a manager makes it or an architect does.
What AI changes
I worked this way before AI. In the AI era I think it is the only good direction. The reason a go-between used to be needed between the technical person and the customer was that the technical person's time went into writing code. Now the agent writes most of the code, and the freed-up time belongs where the decisions are made: next to the customer. The technical side can't outsource the calls to a management layer because it doesn't feel like taking them. Then it decides on second-hand information, and that is where the bad features come from.
Most of what the coordinating layer lived on is also exactly what an agent does well now:
| What the layer did | Who does it now |
|---|---|
| Status – walking around asking where things stand | An agent summarises the PRs, the tickets and the commits. Nobody has to collect it by hand. |
| Translation – between the customer and the engineers | The technical person talks to the customer, with the time the agent freed up. |
| Handoffs – between the teams a feature crosses | A smaller team ships the same amount, so there are fewer dependencies to manage in the first place. |
| People – career, feedback, hiring, conflict, burnout | Still a person, and more important than before. An agent doesn’t do this, and a technical lead can’t do it well on the side. |
That last row is the one I want to be clear about. People management matters more in the AI era, not less. Careers, feedback, hiring, conflicts, burnout – the AI does none of it, and a technical lead who is also running discovery can't do it well as a sideline. What becomes redundant is the other kind of manager: the one whose job was moving information.
In the AI era the people-manager gets more important and the relay-manager less. People need leading. Information only needs delivering.
What I mean when I say the role isn't needed
I have said before that product and project managers aren't needed, and the sentence travels further than what I mean by it. So let me be precise about the claim. What I am against is the relay: a position whose content is passing a request from one side to the other without understanding it and without deciding on it. Plenty of people with those titles do exactly the work I described above – they know the domain, they know what the change touches, they sit with the customer and they decide. Then they are the technical lead by another name, and I have no quarrel with the name. The thing to name is the distance from the code, not the title.
I'll also be honest about where this comes from. It is experience, not a law: I have not yet worked with a product or project manager where understanding what was going on wasn't extra work for everyone. I work on technical products, where the customer is technical too, and in that setting I haven't seen the layer in between add anything. A consumer product with a research-driven roadmap is a different setting, and I am not writing about it. It is also an opinion, not a measurement: the flattening of middle management is happening, but how much of it is AI and how much is cost-cutting can't be separated yet.
And I know this position will sting for people in those roles, because it questions whether the role is needed. That is the price of holding the opinion, and I am not going to soften it below what I actually think. What I can do is say exactly what it is – a claim about distance from the code and the customer, and about who decides – so that it can be argued with on those terms.
My problem isn't with managers. It is with the relay: whoever forwards a request without understanding it, and doesn't decide on it either.
Letting go of control, one rung at a time
A ladder, not a switch.
- The agent proposes, a person writes.
- The agent writes, a person reviews everything.
- In a narrow, low-risk class – dependency updates, added tests, mechanical refactors with good coverage – lighter review, behind deterministic gates.
- In the narrowest class, auto-merge, with instant rollback.
The condition for the next rung isn't a feeling. It is a measured track record: revert rate, defects that escaped to production, and the scope – a small blast radius, a change you can take back. What you never let go of: migrations, permissions, money movement, data deletion. One class, one step, measure, then the next. Patiently.
There is a sensible asymmetry inside this. Where a change isn't core business logic, and a mistake doesn't cost you face with a customer, the PR can be accepted with a lighter look: read the PR message, see which files changed, and if you know the domain and the product you know roughly that nothing critical can be wrong there. The business logic itself – the part that carries the real value – gets split into small PRs and read properly. The rung belongs to the class of change, never to the repository as a whole.
review-policy.yaml
# Which rung each class of change stands on, and what it takes to move up. The rung is per class, never per repo.
classes:
dependency-bump: { rung: 4, gates: [build, tests, security-scan], rollback: automatic }
add-tests-only: { rung: 3, review: light }
mechanical-refactor: { rung: 3, review: light, requires: "coverage >= 80% on the touched files" }
feature-non-core: { rung: 2, review: full }
business-logic: { rung: 2, review: full, max-lines: 200 }
never-delegated: # rung 1 for good – a person writes or owns every line
- migrations
- permissions
- money-movement
- data-deletion
promotion: # measured over a quarter, per class, before a class moves up
revert-rate: "< 2%"
escaped-defects: 0
blast-radius: small, and reversibleDon't ask whether you trust the agent. Ask: for which kind of change, at what stakes, and with what evidence.
How do we know it is better?
Measure, against a baseline. Before you introduce anything, know today's numbers. The four DORA metrics are a good start, and they are four simple questions: how long from idea to production (lead time), how often do we deploy, what share of changes causes a failure, and how long until the system is restored. Next to them: revert rate, defects that escaped to production, PR size and review time, and rework – code that gets rewritten within two or three weeks.
-- The baseline, before anything changes. Lead time, change failure rate and time to restore, per quarter,
-- from tables you already have: pull requests, deployments, and incidents linked to the deployment that caused them.
SELECT date_trunc('quarter', d.deployed_at) AS quarter,
percentile_cont(0.5) WITHIN GROUP (ORDER BY d.deployed_at - pr.first_commit_at) AS lead_time_p50,
count(DISTINCT d.id) AS deployments,
count(DISTINCT i.caused_by_deployment_id)::numeric / count(DISTINCT d.id) AS change_failure_rate,
avg(i.resolved_at - i.opened_at) AS time_to_restore
FROM deployments d
JOIN pull_requests pr ON pr.id = d.pull_request_id
LEFT JOIN incidents i ON i.caused_by_deployment_id = d.id
GROUP BY 1
ORDER BY 1;
-- Not in this query, on purpose: lines written, number of PRs, the share of code an agent wrote.| Measure | Don’t measure |
|---|---|
| Lead time from first commit to production, and deployment frequency | Lines of code written |
| Change failure rate, and time to restore | Number of pull requests |
| Revert rate and defects that escaped to production | The share of code written by AI |
| PR size and review time – the cost side of the ledger | How fast it feels |
| Rework: code rewritten within two or three weeks | Anything measured over one or two sprints |
The right-hand column measures activity, not quality. And don't trust the feeling either: with AI it is easy to feel faster, because the writing really is fast – and the review, the fixing and the debugging slip exactly where you aren't looking. One or two sprints is too short; it takes a quarter to see. Finally, the other side of the cost counts: what faster writing gains, review time can easily eat.
If you only feel faster, you don't know yet.
Who still understands the system?
This is where the arc comes together. The basic knowledge has to be there, so that the AI's beautiful explanations don't lead you by the nose: a convincing explanation is not evidence. Yes, it is slower than not reviewing. But responsibility can't be delegated, and laziness collects its debt with interest.
You don't need every detail, but you need the broad strokes, so that at review time it clicks: yes, this is how it works here. Two quick tests. Can you draw the architecture without opening the code? Can you say what happens when a node dies in the middle of a sync? If not, the system isn't yours any more. It is the agent's – and the agent forgets it by tomorrow. Call it comprehension debt: the code grows faster than the understanding of it, and someone will pay the difference.
In practice: module owners, small PRs, the why written by a person, onboarding as a test – is the new developer in the picture after a week? – and writing code by hand now and then, so the muscle stays. For juniors this is especially sharp: whoever never fought a bug to the end doesn't build a mental model, and that is hard to make up later. I wrote about that loss in the engineering mindset post; here it is the whole team's problem, not only the junior's.
Machines can write the code. Understanding and responsibility can't be outsourced.
Objections, and the answers
| They say | You say |
|---|---|
| "The agent writes the tests too, so the review is redundant." | The agent’s tests prove what it implemented, not what you wanted – and they pass at the size of the test data. Review is for the domain fit and for the team learning the edge cases. Neither is in a test. |
| "The review bot is more thorough than a tired human." | More thorough, and without the context. Its advice is generally valid and locally wrong, and the fix-it button makes the locally wrong part cheap to apply. Let it flag. Let a person decide. |
| "A proper spec up front would fix all of this." | The running code is the truth; a spec is a description that can go stale, and the agent follows a stale one just as faithfully. Keep the why in a short decision record and the intent in the PR. Keep the what in the code. |
| "So product managers are useless?" | No. The relay is: a position that forwards requests without understanding or deciding. The product work gets distributed – discovery to the technical lead, pricing to sales, contracts to legal, dates to delivery – and the people-manager matters more than before, not less. |
| "Twenty teams can’t coordinate themselves." | They can’t, and nobody said so. The coordinator has to understand what they coordinate – that is what staff and principal engineers are for. Information is lost per step between the decision and the code, not per title. |
| "A PR-size limit is bureaucracy." | It is the one rule that makes every other one possible. A reviewer can read four hundred lines. Nobody can read two thousand, and an agent can slice a task any way you tell it to. |
| "We measured: forty percent more code shipped." | That measures activity. Measure lead time, change failure rate, reverts, escaped defects and rework over a quarter – and put review time on the cost side, because that is where the gain goes to die. |
| "Just never let the agent merge." | That is rung two, forever, and it leaves the cheap wins on the table. Climb per class of change, on evidence, and keep migrations, permissions, money and deletes on the bottom rung for good. |
Takeaways
- Quality didn't change – the weak link moved. Writing got cheap; review, understanding and responsibility cost what they did.
- The codebase is the prompt. One port with adapters gets copied; five controllers five ways gets a sixth. Fix a wrong pattern before you let an agent scale it.
- Small PRs, or no review at all. A person can read four hundred lines. Slice the task before the agent starts.
- Review is knowledge transfer, not a bug filter. If you only approve, the edge case stays with the agent.
- Deterministic gates underneath, the agent's opinion in the middle, a person at the end. The agent flags; the fix-it button is a human decision.
- Caught twice in review becomes a rule – an architecture test, a banned API, a migration check, a size limit. What can't be a rule gets a judge with a narrow rubric, as a signal.
- A green test proves the size of the test data. Staging with real volume, a load test on the critical path, a canary – and a two-minute look for the query in the loop.
- Delete the branch, fix the context. Regenerating from the same context gives the same mistake.
- The code says what, the docs say why. A short decision record beats a big wiki the agent trusts blindly.
- The spec is assembled by whoever understands and decides – close to the code and the customer at once. A relay in between adds a round trip and two distortions; the product work gets distributed, and the people-manager matters more.
- Let go per class of change, on evidence. A ladder, not a switch, and some classes never leave the bottom rung.
- Measure against a baseline, over a quarter. DORA, reverts, escaped defects, rework – and review time on the cost side.
- Someone has to understand the system in broad strokes. Comprehension debt is the one debt that compounds silently.
Back to the thesis: the chain is the same as it ever was, only now you can see where it snaps. A good codebase the agent follows. A small PR a person can take in. Deterministic gates that don't ask for permission. Trust that is measured and given one rung at a time. And a person who understands, in broad strokes, what is running. Writing code got cheap. Understanding didn't. That is where to spend.
Sources
- DORA's four key metrics – lead time, deployment frequency, change failure rate, time to restore.
- Building indexes concurrently and lock_timeout – the Postgres documentation on the two migration guardrails.
- WITH queries – when Postgres inlines a CTE and when it materialises one.
- BannedApiAnalyzers – the
BannedSymbols.txtformat and ruleRS0030. - NetArchTest – architecture rules as unit tests in .NET.
- Efficient querying in EF Core – projections, paging and the N+1 problem.
- Architecture decision records – the short form of writing down the why.
- .NET releases and support – the LTS cadence behind the upgrade example.