Reviewing Rails an agent wrote

On 23 September 2026 DHH opened Rails World in Austin with "It's pencils down, people. Writing code by hand is no longer an economically viable skill for most programmers at most companies." He posted the line himself, and the @rails account summarised the keynote as being about why 37signals has gone pencils down on handwritten code. Take that at face value and the skill that replaces typing is reading, at a volume nobody on a Rails team has had to read at before.

One thing belongs at the top rather than buried. The same keynote announced HEY 2.0 as native apps on every platform over a Rust backend, which DHH said is efficient enough to serve HEY's peak traffic on a Raspberry Pi. The creator of Rails is moving his flagship product's server off Rails. What DHH actually said at Rails World 2026 takes that announcement on its own terms rather than as a footnote to a boilerplate pitch.

Line by line is the wrong instrument

Reviewing agent written code the way you review a colleague's pull request fails for a boring reason: the arrival rate. A person who writes 300 lines in a morning has spent the morning deciding things, and the diff is a record of those decisions, so reading it in order recovers the reasoning. An agent's 900 lines contain no decisions of that kind. It produced a plausible implementation of the thing you described, and the plausible implementation is correct in most places and wrong in a small number of specific ones.

Reading top to bottom spends attention uniformly on a defect distribution that is not uniform. By line 400 the attention is gone, and line 400 is usually where the migration is.

What works instead is inspecting by category rather than by position. Open the diff five times, each time looking for exactly one thing, in an order fixed in advance. The five passes below are that order. Each one is a grep or a command rather than a read, which means each one finishes, and the part of the review that requires actual judgement is left until only the tests are in front of you.

The cost of working this way is real and worth stating. You will not build the mental model of the change that a linear read gives you, and for a change that alters how the application is shaped, that model is the point. Save the linear read for those, and know which you are doing.

The order, and why it is not the diff order

Migration, strong parameters, callbacks, queries, tests. The sequence is sorted by how expensive the mistake is once it is merged, not by how likely it is.

A bad migration is first because it is the only item on the list that can be unrecoverable. Once remove_column has run against production, the review that would have caught it is a week late and the data is gone. Strong parameters are second because a widened permit list is a privilege escalation that no test fails on, and because it takes eleven seconds to check. Callbacks are third: they are recoverable but they compound, and a callback merged in March is load bearing by June. Queries are fourth because the tooling can be made to catch them. Tests are last because they require the most attention and there is no point spending it before the four cheap passes have finished.

The order has one property that matters more than its exact contents: it is fixed, so it survives a tired afternoon. A checklist that gets reordered by what the diff looks like is not a checklist.

Pass one: the migration, then the schema diff

The migration file is not the artifact to review. db/schema.rb is, because that is what a fresh database will be built from and what every future migration will be applied on top of. An agent that writes a migration and does not run it leaves a schema file that does not match, and the diff shows this instantly.

Reversibility is where the Rails specifics live. The Rails Guides state flatly that "the change_column command is irreversible", and remove_column is reversible only when it is given the original type:

# rolls back
remove_column :products, :part_number, :string

# raises ActiveRecord::IrreversibleMigration on rollback
remove_column :products, :part_number

An agent writes the second form regularly, because it is what the column removal needs in order to run forward and the forward direction is the one being tested. The check is two commands:

bin/rails db:rollback STEP=1
bin/rails db:migrate
git diff db/schema.rb    # must be empty

On PostgreSQL there is a second class of problem the diff will not show you: a plain add_index blocks writes for the duration, which is invisible on a development database with 40 rows and an outage on a table with 40 million. strong_migrations exists to fail the build on exactly that family, and it wants disable_ddl_transaction! with algorithm: :concurrently in its place. If agents are writing your migrations, that gem stops being optional.

Pass two: what the permit list widened to

Rails 8 added params.expect, which requires and permits in one call:

params.expect(post: [ :title, comments: [[ :message ]] ])

The documented reason is that require followed by permit could be made to raise NoMethodError by sending a string where a hash was expected, which is a 500 the user chose. expect raises ActionController::ParameterMissing on the same input, which is a 400. The double bracket in comments: [[ :message ]] declares that an array of hashes is expected, and a single bracket does not mean the same thing.

What to look for in agent written code is narrower than "is this correct". Diff the permitted key list against the previous version and read the keys that were added. An agent that was asked to make a form save one more field will sometimes add that field and sometimes widen the list until the spec passes, and those two look identical in a diff unless you are comparing key by key. The names worth stopping on are the ones no form should ever submit: :role, :admin, :confirmed_at, :user_id, anything ending in _id that identifies the owner of the record rather than a choice the user made.

expect! is the stricter sibling, raising ActionController::ExpectedParameterMissing, and the docs describe it as being for internal APIs where a malformed shape indicates a bug rather than a tampering attempt. Reserve it for that; used on a public form it converts hostile input into an exception report.

Pass three: the callback that was a service

A callback is the cheapest line that makes a failing spec pass. The agent has a spec saying that creating an Order sends an email, and after_create :send_email is one line, sits next to the association it reads, and turns the spec green. A service object called from the controller is four files. Both satisfy the request. Only one of them is still a good idea when the order is created from a Stripe webhook, a rake task, a console session and a factory.

The rule that survives review pressure is about reach rather than taste. A callback that writes to the record's own columns is fine. A callback that reaches outside the row, to a mailer, an HTTP call, another table's rows, or a background job, is a service that has been installed somewhere it cannot be turned off.

The Rails app this site sells has two callbacks in its entire app/models tree: before_create :assign_referral_code on User, which generates a code and writes it to the row it is on, and before_update :snapshot_previous on AiTemplate. Both are before_, both touch only their own record. Every side effect that leaves the row lives under app/services, in eight namespaces. That is not asceticism, it is what makes the codebase greppable: the answer to "what happens when an order is created" is in one place rather than distributed across a callback chain. Conventions are the context is the argument for why that shape matters more once an agent is the one writing into it.

Pass four: the association that made the view convenient

Do not review for N+1 queries by reading. Make the test suite fail on them.

prosopite detects the pattern rather than the symptom: more than one query with the same call stack and the same fingerprint. Its README is direct about why it exists, which is false positives and false negatives from bullet. With pg_query alongside it for fingerprinting on PostgreSQL, the wiring is three lines in the test environment:

require "prosopite/middleware/rack"
config.middleware.use(Prosopite::Middleware::Rack)
config.after_initialize { Prosopite.raise = true }

A request spec that triggers an N+1 now fails, in CI, without anybody having looked at the diff.

Two cases it will not save you from, and both appear in agent written code. The first is .count inside a loop. size on a loaded association returns the size of what is loaded; count issues a SELECT COUNT(*) every time regardless, so posts.each { |p| p.comments.count } stays an N+1 after someone has added includes(:comments) and satisfied themselves it is fixed. Grep the diff for .count and read each one. The second is the query that is fine at fixture scale and not at production scale, which no detector catches because the fixtures are the thing it runs against.

Rails 8 has config.active_record.strict_loading_mode = :n_plus_one_only as a narrower default than :all, which is worth knowing before you conclude strict_loading_by_default is too blunt to turn on.

Pass five: the test that asserts the implementation

Test review is the pass that cannot be automated, which is the reason it is last.

An agent asked to write tests for code it just wrote has read that code very recently, and the cheapest passing test is one that describes it. That produces specs that mock the class under test, assert that a private method was called, stub find_by and then assert find_by was called, or name the method in the description rather than the behaviour:

# asserts the implementation
it "calls create_charge" do
  expect(Billing::Charge).to receive(:create_charge)
  subject.call
end

# asserts the behaviour
it "leaves the order unpaid when the card is declined" do
  subject.call
  expect(order.reload).not_to be_paid
end

The first passes if the method is renamed to something that does nothing. It also fails the moment anybody refactors, which is the trap: a suite full of these makes every future change look dangerous, so the next agent works around the tests instead of through them.

The test for a test is a thought experiment that takes ten seconds. Delete the implementation and write it a different way. Does the spec still express something true? If the answer is no, the spec is a transcript. Testing what an agent wrote goes further into what to ask of a suite you did not write.

Green is not the signal here. A suite of implementation transcripts is green by construction, and a 100 percent pass rate on tests written by the author of the code is evidence of nothing at all.

Where this order would be wrong

The position above is that fixed category passes beat a linear read, and it is worth naming what would change it.

Two things would. If the change is architectural, the passes miss the entire point, because the defect is the shape rather than any line in it and no grep finds a shape. Read those linearly and slowly, and prefer not to have an agent produce them unreviewed in the first place. The other is scale in the opposite direction: below roughly fifty lines the five passes cost more than reading the diff, and running a checklist over a nine line change is ritual.

The order is also a review of the diff, which means it is the last line of defence rather than the first. Most of what it catches would not have been written if the repository had told the agent what it expects. An AGENTS.md for a Rails app and Cursor rules for Rails cover the upstream half, and Claude Code with Rails covers the loop the diff comes out of. A codebase whose conventions are written down produces diffs that fail these passes less often, which is the same claim from the other end as Token efficient Rails makes about cost.

What this page does not cover

Security review as a discipline. brakeman and bundler-audit belong in the pipeline and catch a different class of thing than the passes above; the strong parameters pass is the one place the two overlap and it is deliberately narrow.

Nor does it cover whether the change should exist. Reviewing agent written code is downstream of deciding what to build, and a correct implementation of the wrong feature passes all five passes. When Rails is still the answer is the page for the decision before this one.

Performance beyond query counts is absent too. An N+1 is detectable by pattern, and a slow query, a missing index at scale or a memory profile are not, so nothing here pretends to cover them.

José Valim published on how programming languages should evolve in the AI era the same week as the keynote, and it is worth reading on its own; this page makes no claim about what it argues.