ADR 0006: the person drives review
Each item below carries its own state: done is in the tree, in progress sits on a branch, planned is decided and queued, proposed is an idea not yet decided, and not built was weighed and declined.
Context
Agents write code far faster than we can review it, and we are now the bottleneck. When you write code you understand it as you go. When an agent writes it, you meet it for the first time as a reviewer, a third party to code you did not write, and you start at a disadvantage.
The review process has not caught up, and it does not scale:
- Most of us first read a change in the GitHub UI, after the push, when colleagues or the public can already see it. That UI makes a large change hard to navigate.
- Teams answer the volume by shortening review cycles and thinning review, so fewer people read each change, faster. That trades review for speed.
- With one approval required, a change often merges while a second reviewer is halfway through reading it. Their time is gone, and nobody counts it.
Left to run on their own, agents creep scope and ship slop. A review of this repository's open pull requests, all agent-written, found exactly that:
- One reverted its own first approach a commit later and kept the test written for it.
- One claimed "five real races" it never named, and its gate change broke the merge queue.
- One widened a security guard with bypasses we found in minutes, and bundled an unrelated fix under a changelog file named for a third topic.
- One shipped a 485-line analyzer whose default mode cannot catch the problem that justifies it.
An earlier change shipped the Vale prose linter as a built-in spell, which I never
intended, and the port that followed briefly handed this repository's own prose taste to
every user as magus\prose(). An independent review measured about 57% false positives
for those rules on the tree they were written for.
Each of these came from a session nobody drove: scope crept, claims went unchecked, and a standard from one tree leaked into the product.
Decision
Principles
- A person holds both ends. I open the work by deciding what it is for and what it must not touch, and I close it by reading, questioning and cutting what came back. The agent does the middle, often the largest share, but never the start or the finish. I drive the session the whole way; an agent never decides the work is done.
- People answer people. A reply to a reviewer is typed by the person who sends it. An agent may show a short outline to think with, never text to copy, and it never posts a reply. Choosing the words is part of answering for the change.
- Write for people. The moment we start writing code with the intent of other agents reading that code instead of other humans is the moment we have lost our way as software engineers. Code and comments are for a human reader, and text padded for an agent's context window is a defect.
- People merge. An agent never enables auto-merge or queues its own pull request. Whether a change met its criteria is my call.
- Review before publication. The first careful read happens locally, before a push puts the change in front of anyone else.
- Enforce only what we know is right. magus refuses only what cannot be undone. Anything else it ships advises, and anything mined from or tuned to this repository stays this repository's policy.
1. What magus enforces on itself versus what it ships
| Item | State |
|---|---|
"Conventions" means this repository's policy (libs/conventions, hack/lint, hack/policy); "precedents" means norms mined from a user's own tree. Separate homes; no conventions/precedents |
done (decided) |
The root module never imports libs/conventions or libs/testlayout; a depguard rule enforces it |
done (#550) |
Vale is not a spell. Its rules run as native Go in libs/proofread, judged by a repo-only runner over symbol docs |
done (#550) |
magus ships symbol doc comments as data (magus\symbols()), never a judging call; a user's own magusfile decides what text must say |
done (#550) |
coldread folds into libs/conventions and judges doc comments from the symbol index in every language; its in-body checks are dropped |
done (#550) |
| A shipped rule must be justified without magus's own data, be a correctness property or documented tool norm, show near-zero false positives on three or more repositories other than magus, and only advise unless the action cannot be undone | done (decided) |
fieldwise stays repo policy with report-partial on, after its false positives are fixed |
done (#554) |
2. The guard denies only what cannot be undone
| Item | State |
|---|---|
| Built-in rules that deny a recoverable action (a whole read, a pipe, an in-place edit, staging everything, a raw tool) advise by default, once per session | not built (ADR 0013) |
Every built-in rule can be set by name in the root magusfile: magus\guard.builtins({name: "deny" | "advise" | "off" | {decision, lines}}); an unknown name is an error; the stricter of the committed and working-tree settings applies, so loosening waits for a commit |
done (#550) |
| No number measured on this repository ships in the binary; this repository sets read-navigation's line limit, with the evidence for every rule, in its own guard policy | done (#550, ADR 0013) |
| A rule that guards destroyed work, publication, provenance, credentials, the guard's own authority, or a boundary a job declared keeps denying by default | done (#550) |
A guard rule that denies an agent enabling auto-merge on its own pull request (gh pr merge --auto and equivalents); the denial hands the person the command to run, and it lives in this repository's guard policy |
done (hack/policy/gh.buzz) |
3. Local review in reading order
Research (empirical, prior art, and what magus already has) is in the plan this ADR came from. What it established:
- Position steers attention: reviewers comment less on later files and miss more defects there. Any order we pick matters, so the central change goes first.
- Grouping related parts helps more than any one reading direction. Studies support showing a definition before its use; nobody has tested top-down against bottom-up.
- Reordering cuts noise and false alarms. Nobody has shown it finds more bugs, so we promise comprehension and nothing more.
- No shipped tool orders a change by real symbol relationships, locally, before push, the same way every time. The closest product runs hosted, after the push, and lets a model pick the order.
| Item | State |
|---|---|
magus diff carries a reading order with no flag: hunks grouped by definition and use among the changed symbols, definitions before uses, interfaces before implementations, code before its tests, larger and wider-reaching groups first |
done |
| Each step names the relationship that placed it; a completeness line proves every hunk appears exactly once; the order is deterministic and never chosen by a model | done |
Consecutive hunks from one file merge into one step; generated output folds into a group of its own; hunks magus cannot place form a final "unranked" group; when the symbol index cannot be brought current the order is omitted and a note names magus graph build |
done |
The text report, the terminal viewer and -o json follow the order, and the console's focus mode walks it one step at a time with everything else put aside |
done |
The closing read keeps private read marks keyed by hunk content; magus diff --unread narrows the review to the hunks no mark covers under every -o and always exits 0; the managed pre-push section hands the check-drift job the refs git is pushing, and the job counts the unread hunks of exactly those ranges and raises a desktop notice for them; a commit never does; nothing blocks a push |
done |
| The reading order opens with the intent: the job's criteria, the brief, and the rationale the session recorded, since magus already holds what an agent was asked and what it ran | proposed |
| A Buzz prototype ordered four recent pull requests; it read better than file order where a change was one connected feature, and worse where it was many small unconnected hunks | done |
Prerequisites the prototype found: hunks with their symbols on magus\diff(), a file and line to symbols lookup, an index-freshness check, and an implements edge from SCIP (knowledge schema 17) |
done |
4. Review between people
| Item | State |
|---|---|
A reviewer can mark a review as being read now. The mark is local; magus prints the gh pr comment line that tells the pull request, and the person runs it. It is a notice, not a hold: it does not stop the merge, and a merge that lands while the mark is set is reported to the reviewer |
done (decided) |
| Review time lost to merges that landed during an active review is measured by two opt-in metrics: a count of such merges and the time from the mark to the merge | done |
| A person enables auto-merge; an agent, or tooling acting for one, never does | done (decided) |
| A review thread from the forge shows with its replies under the hunk of its first comment, in the terminal and the console, and a reply goes to a named thread | done |
The host's resolved state is not fetched. A thread carries only outdated, set when its commented line has left the head |
done (decided) |
magus diff --thread <id> narrows the review to one thread for the person reading it: the viewer opens on its hunk, and printed it is the conversation, the hunk, and what the change there reaches. -o json, the console's thread route and the diff MCP tool's read-only thread op return the same record |
done |
A plain magus diff report never reaches the network: it names thread ids only from a running server's session, and with no server it lists none and names the command that starts one |
done (decided) |
Every command a person types is for that person, and none of its flags exists to feed a model; magus diff --prompt and --print-hook are removed. The one exception is hook glue: magus shell, with --message and --observes-skill-loads. Agent affordances live on the MCP tools |
done (decided) |
| An agent's outline of a reply is at most five topics of 60 characters that cannot be copied or inserted, and a second outline of a thread replaces the first; the person types every reply | done |
5. Text a stranger can read
Reviewers read our pull requests and design docs as answers to a question nobody showed them. The fix is the principle to write for people, enforced.
| Item | State |
|---|---|
| Pull request text and hand-written Markdown keep to written-prose rules (reply voice, tense, hedges, attribution, filler), judged in Go by one judge that the guard, a CI check and the Markdown lint all run | done (#558) |
| A pull request description opens with the goal behind the change and why this code stands in its way; an area the diff touches that the text never names draws a question, never a refusal | done (#558) |
| No commit, pull request or page credits a tool or narrates how the work was made | done (#558) |
| Skills stay terse (25 words a sentence and 60 a paragraph in the short form) and say "rule" only for what magus enforces; the rest are instructions | done (#558) |
| Every ADR follows one template, with its status and date in front matter; prose uses plain punctuation and no spaced hyphen as a dash | done (#555, #557, #558) |
Alternatives
Weighed and declined:
- An order chosen by a language model. It changes between runs, and it anchors the reviewer on the model's account of the change instead of the code it leaves out.
- Review scores, completion ratios, or anything that rewards clearing a review unread.
- A judging host call that ships this repository's prose rules to users (
magus\prose()). - One-human-one-agent review as a speed target.
Consequences
- A workspace that relied on a built-in refusal of a recoverable action gets advice
instead, and declares the refusal with
magus\guard.builtinsif it wants it back. - This repository keeps its standards in its own policy files, and a depguard rule stops them from reaching the binary. Promoting one to something magus ships means meeting the bar in section 1 first.
- The reading order depends on the symbol index: per-hunk symbols, the uses of the changed
symbols, and an
implementsedge. The edge raised the knowledge schema to 17, so a graph built before it rebuilds. Where the index is missing or stale, the order is omitted and a note names the rebuild, because an order drawn without the uses would read as "nothing is related". - Marking a review as being read is a notice, and nothing holds a merge. A merge can still land under a reader. In exchange magus keeps no state on the forge, and the metrics show how often it happens.
- A thread the host calls resolved looks the same as an open one. Fetching that state
takes a query the comments listing does not make, and
outdatedalready says when the code moved.
Open questions
- Whether the reading order keeps working at the scale agents produce (thousands of lines), or whether the answer there is smaller changes.
- Whether the completeness line and the folded group are enough to show what the order leaves out, so a reviewer reads the code as well as the grouping.
Amendments
2026-10-20: proofread returns a grade with a required minimum
Alternatives above declines "review scores, completion ratios, or anything that rewards clearing a review unread". That row stays as written. It covers a reviewer's progress through a change. It does not cover the text an author writes, and proofread now grades that text. Eli decided this on 2026-10-20.
The forces, from the tree at 002e91670:
- An advise finding fails nothing. The
pr-descriptiontarget prints it withmagus\log.warnand passes (magusfile.buzz:735-758). That is the warning nobody reads in a CI log, which No silent defaults rules out. - The pass bar is a flag, not policy.
--fail-on deny|advise|neverdefaults tonever(libs/proofread/cmd/proofread/main.go:252), so a bare run exits 0 whatever it finds. - How far to trust a rule is already measured, then dropped.
evidenceCeiling(libs/proofread/evidence.go) records each rule's Wilson 95% lower bound on precision in a comment, and uses it only to cap a rule at off or advise. blame is 34/37 (0.79), subject-mood 37/40 (0.80), filler 32/40 (0.65), absolute 6/40 (0.07). A finding from absolute counts the same as one from subject-mood.
Decision
| Item | State |
|---|---|
A judged text gets a grade: 100 - 100 * sum(w) / (1 + min(words, cap) / k), floored at 0. w is the rule's Wilson 95% lower bound on precision, pinned in libs/proofread/evidence.go and held by a test to what internal/calibrate recomputes from the latest labeling round in testdata/rounds. A rule no round measured over 30 firings weighs 0. A rule that is off does not fire. k is 100 words and cap is 1,000, so padding a text past 1,000 words buys no headroom |
done |
| A deny finding fails the text whatever its grade. The grade replaces the advise tier and nothing else | done |
A grade under its kind's minimum is itself a deny finding, rule grade (PRF1100), whose message names the grade, the minimum and the rules that cost the most. Every caller that already fails on deny fails on it, and the findings contract (ADR 0008) gains only a weight per finding |
done |
Each kind's minimum sits beside the decisions table in hack/policy/proofread.buzz, with its reason, and the decisions file proofread reads carries it. A run over a kind with no minimum exits 2 and names the kind. It never falls back to 0 or 100 |
done |
The first minimums are the lowest grade each kind has in this tree at the commit that lands them, so the change fails no text that passes today. Raising one is a commit with its reason. Two kinds depart, each with its reason in MINIMUMS: commit-message sits at 20, under its lowest of 74, so the length test below holds; and the review kinds, agent-reply and stylesheet sit at 0, since this repository keeps no such text or this change could not read it |
done |
Deleted: --fail-on and reachesBar; the warn loop in the pr-description target; and every evidenceCeiling entry a weight replaces. The off entries for rules no round has measured over 30 firings stay, as unmeasured |
done |
Each check's per-kind advise list becomes one flag for the whole rule. A rule that advised on any kind is graded on every kind it judges, and its measured weight carries what the list encoded by hand. A rule that advised nowhere keeps denying. Rules that judged some kinds at deny and others at advise (reply-voice, wordy, signpost, leak, changelog-group) lose the deny. This repository puts each back in its decisions table, with a reason, since its own gates relied on them. subject-mood and reply-opener, which their checks declared deny and the ceiling held to advise and off, carry the flag too |
done |
| A test pins the length risk: a commit message of 12 words or fewer, with one finding from the strongest measured rule, grades at or above the commit-message minimum, and two findings grade below it | done |
The exit codes keep their meaning: 0 passes, 1 is a deny finding (a low grade among them), 2 is input proofread cannot judge.
Principles checked
- No silent defaults: a kind with no minimum is an error, and advice that changed nothing is gone from CI output.
- A refusal carries its reason: a low grade names the minimum and the rules that cost the most, not only a number.
- Agents propose, humans dispose: the minimums are this repository's policy, committed by a person with a reason. No number measured here ships as a default in the binary, as section 2 already requires.
- The outer harness stays outside: the guard reads the grade and denies or allows. It rewrites nothing the host sends.
Alternatives
- Keep pass/fail with advise. It lost because advice fails nothing and a reader cannot
tell 1 finding from 20. Evidence: the
pr-descriptionwarn loop and--fail-ondefault above. It would come back if a grade turned out to track only length (see the next item). - A plain count of findings, no weights. It lost because it treats a 0.07 rule like a
0.80 rule. Evidence: the bounds in
evidenceCeiling. It would come back if the weights moved between labeling rounds by more than the minimums allow. - An uncapped length normalizer. It lost because padding would buy headroom. It would come back if long reference pages hit the cap often and grade unfairly low.
- A weight for each kind as well as each rule. This is what the deleted
adviselists approximated by hand. It was not built because no labeling round has measured a rule per kind. It would come back with a round whose per-kind lower bounds differ by more than 0.10 for one rule. - A diff-level score. It stays declined. The 2026-10-10 diff-grade research found that size alone separated its classes at AUC 0.82, so a score over a whole change mostly measures how big it is.
Open questions
- Whether the grade tracks length on short texts too. The diff-grade result (AUC 0.82 from size alone) is the reason to doubt it, and the short-commit test pins only one point. The first labeling round after this lands should report the correlation between grade and word count per kind.
- Whether the Markdown, symbol-doc and help-text lints should grade each text or keep
reading deny findings alone. They pass deny findings through today, and the
gradefinding reaches them now that their kinds have minimums. - Whether a minimum of 100, which agent-brief, skill-template, cli-help, tool-description and review-reply hold, should stay: on those kinds any graded finding fails, as a deny would.
2026-10-21: the guard denies by default
Section 2 is reversed by ADR 0013. Eli decided on 2026-10-21 that advice an agent can route around is routed around: the recoverable rules, and every advisory whose text names a command, deny by default in the binary. A rule whose remedy does not clear it is a notice, which never refuses. A workspace still lowers any deny by name, and loosening still waits for a commit. The section's State rows above are updated to match; the reasoning stays as written.