How Jane Street Reviews Code with Iron

I want to explain how Jane Street reviews code, concentrating on the ideas behind our in-house tool, Iron. It combines aspects of a revision control system and a review tool. I use it as a programmer rather than as one of its designers, so I will focus on the experience and design ideas rather than present myself as an authority on its implementation.

Iron is open source, but its public repository is difficult to use as a practical installation: development happens internally in a Mercurial-based system, and public releases appear as large diffs with little installation guidance. Rather than pitch the software itself, I hope some of its ideas inspire other tools.

Many review tools I have used share a web-based, branch-centered model of looking at patches and commenting. Iron takes a different route. Some individual choices may seem worse on their own; the benefits become clearer when the pieces work together.

Reading code carefully is difficult. A superficial style check is not the same as understanding a change well enough to suggest improvements. Review gives back what you put into it, but rather than solve the semantic difficulty of understanding programs, I want to focus on the mechanical difficulty of keeping track of what you have read.

A sequence of commits is straightforward to review one diff at a time. The trouble begins when a branch diverges and someone merges the upstream branch into it. A merge commit contains unrelated upstream changes, but it may also contain a conflict resolution with real semantic changes that needs review.

Rebasing presents a different problem: the old commits are replaced by new ones. The changes may be equivalent, but the person rebasing can also change them. A reviewer could reread everything or trust the rebase, neither of which is attractive. At Jane Street we always merge, although we call the operation rebasing; our history is consequently messy, and Iron is designed around branches rather than neat individual commits.

Reviewing by file or by commit raises similar questions when changes arrive mid-review. I want a tool that shows all changes, including conflict resolutions, without making people review the same material twice. It should allow both new changes and upstream merges or rebases while a review is in progress.

Iron's answer is a piece of state called a brain, associated with a person and a branch. It starts as an empty diff. As I explicitly accept changes I have reviewed, those changes are applied to my brain. The important distinction is that Iron remembers the content I have reviewed, not just references to commits I have seen.

A branch can be thought of as a diff, and my brain as the diff I understand. Once the two match, I have reviewed the branch. This simple model lets the tool track my review independently of the order in which I read things and of the commits that originally contained the changes.

When a merge or rebase changes the branch, Iron compares my brain with the new branch diff. The result is a diff of diffs: it describes the difference between what I knew and what I now need to know. Reading that result updates my brain to match the branch without forcing me to start from scratch.

Consider a small change to a function that calls another function after a target time: the branch reverses the order of a comparison for clarity. Meanwhile, an upstream change renames the target variable and changes an anonymous function to a point-free form. The same lines change in both places, so the merge produces a conflict.

Iron's resulting diff of diffs shows both the old and new versions of the change. Some lines merely indicate that context has moved; the meaningful part shows the comparison being flipped with the new variable name. Even this simple example can look intimidating, which is one cost of the approach.

Most of the time Iron uses heuristics to render the comparison as an ordinary, more readable diff. When there is a real conflict, a reviewer may have to read the more complex diff of diffs. Conceptually this resembles a functional approach: the tool tracks the values actually reviewed rather than relying on references to commits.

One downside is that after a rebase it can be hard to review a branch one commit at a time: you may need to consider the entire branch diff. We compensate by making small branches. When a diff of diffs is difficult, a reviewer can also discard the relevant review state and review that part again.

The benefit is that a branch can be updated at any time while each reviewer retains a consistent view. A change introduced during a rebase, or a resolution introduced by a merge, appears as a difference from what that reviewer has already accepted. Unchanged work does not have to be reread just because the history changed.

Review also shares knowledge and reduces the bus factor, but a central purpose is to improve code through feedback. Jane Street's mechanism initially surprised me: a reviewer checks out the author's branch, adds a specially marked comment directly to the source, commits it, and pushes it back to that branch.

A conventional review comment might be attached to a commit, diff, branch, or particular file version. Those associations can be awkward when the code moves or a commit disappears during a rebase. A comment in the source travels with the code when the branch is updated.

A review comment identifies its author and intended recipient using a CR marker. The recipient sees it as a new change to the branch, can address it, and can mark a reply with XCR to send the discussion back. The comment is next to the code it concerns instead of depending on a tool to guess where it belongs.

There are costs. The history includes commits that add and remove review comments, and manually checking out a branch and locating the right line for each comment would be painful. By the end of review the comments are gone, so they do not affect blame in the same way, but the history still looks unusual.

Our Emacs review client reduces the friction. While reading a diff I can press a key to jump to the corresponding line in the branch's workspace, then write a comment in my editor. This makes the transition from reviewing to editing much smaller than it sounds.

The arrangement also makes it easy to fix the code and update the comment together. If I see a typo or an unclear variable name while reviewing, I can often just change it instead of opening a comment and waiting for the author to perform a trivial fix. Context remains attached to the code as the branch changes.

Letting reviewers push changes also changes the feeling of ownership. A feature may begin as my work, but it is ultimately Jane Street's code and other people can contribute to it early. Because I am reviewing my own branch as well, my brain shows me changes that other people have made.

The resulting history is undeniably messy, with commits for individual comments and small corrections. Jane Street accepts that tradeoff rather than treating a polished commit graph as the primary goal. This was an adjustment for me, but I came to appreciate the collaboration it enables.

Review obligations determine who must look at a change. The author reviews the branch first, then assigns whole-feature reviewers, who read and explicitly second the branch. After that, the review widens to reviewers with obligations for the files changed by the feature.

File reviewers know the relevant code and can judge whether a change fits its intent. Version-controlled metadata specifies owners, scrutiny, and review obligations for files or groups of files. More important files can require more attention, and a rule can require at least a specified number of reviewers from a group rather than every member.

Nested branches let one feature depend on another. For example, a testing DSL might depend on a new monad transformer library. Maintaining the hierarchy manually can be awkward, but it fits our process: the atomic unit of review is a branch, so a sequence of related changes becomes a sequence of small dependent branches.

In our monorepo, permanent project-specific branches can act as intermediate masters where changes are merged before reaching the rest of the repository. Iron calls branches features and implements them as Mercurial bookmarks with additional metadata, including an owner and a description that plays a role similar to a commit message.

The Iron client includes a to-do dashboard showing reviews, comments, branch status, and suggested next steps. It also lists unclean workspaces, which is useful when many branches have separate working directories and it is easy to forget uncommitted or unpushed work.

The dashboard makes it easier to move among multiple features without losing track of them. I also use review comments as personal reminders. From the Emacs dashboard I can request an asynchronous rebase; if it fails because of a conflict, the to-do list later tells me that I need to intervene.

A workspace is a separate working directory for a branch, similar in purpose to a Git worktree. It lets me switch tasks without replacing the contents of one checkout or stashing unfinished work. In our large monorepo, separate workspaces can also keep incremental OCaml build processes running rather than restarting a build every time I change branches.

Iron uses locks differently from a traditional exclusive file lock. I can lock a feature against release if it has a known critical bug, or lock a rebase if it needs careful handling. A branch for a production patch can also be kept on an older version so that an automatic update does not accidentally pull in newer changes.

The centralized server enforces rules: it will not release a branch that has not been fully reviewed or still has blocking CRs. This constrains structural operations and requires a tradeoff between safety and convenience. Mercurial still allows local work without contacting the server; the restrictions matter when changes are shared and released.

Once we had a good review tool, we used it for more than source code. Configuration changes and blog posts can go through the same process. Trading limits also have obligations so that people authorized to approve a change can review it before release. The same mechanism could even be used to review a talk written in Markdown.

In questions, I noted that nested branches are largely a formalized workflow rather than a unique primitive. A similar arrangement could be built with hierarchical branch names and scripts to update dependent branches. My comments on the influence of Mercurial's history and rebase conventions on Iron's design were explicitly speculative.

I used to polish commit history and write detailed commit messages before review. At Jane Street I stopped treating individual commits as the important narrative: the branch carries the review and description. Many people even configure a fixed commit message, allowing them to commit intermediate work freely without curating a pristine sequence.

When a production bug appears, we generally roll back the deployed executable to an archived version. Iron also has traits: a revision can be marked as containing a bug, and a later revision marked as fixing it. Because published history is merged rather than rewritten, the tool can track which revisions inherit the bug and whether a rebuild includes the fix.

We fix the problem with a new change or apply a reverse diff and commit it, rather than rewriting an old commit out of the history. The same reverse-diff approach can remove a change from a branch while preserving the historical record.

Bisection illustrates a real cost of this history. Normally, a binary search between a known good and bad revision can locate the commit that introduced a bug. With our many merge commits, following the relevant ancestry is painful. A branch-level bisection that examines feature releases could help, but we do not have it.

Finally, file ownership has a specific mechanical meaning. If a branch is released with a nonblocking CR-soon comment, responsibility for that follow-up passes to the file owner and appears in their to-do list. File review obligations are separate: they determine who gets to review changes, although the owner is often also a reviewer.