Repository navigation
Document Review: hold a spec or ticket set to its Parent and to the checkout, on the same Bridge #43
Description
Problem Statement
After /to-spec writes a spec, and especially after /to-tickets (or /route) cuts that spec into tickets, the documents go straight to implementation with no independent check. A ticket-cutting session hallucinates decisions the spec never made, quietly drops decisions the spec did make, and writes acceptance criteria that do not verify the goal their own ticket states. The technical decisions in those documents are also never held against the checkout they will land in — whether they add a Module an existing one already owns, a Seam the checkout already has, or contradict an ADR. Review-Switch today reviews only code: a Review Scope held to standards and to a spec. There is no way to run the same protocol — independent Lane, report file, Rounds Contract, Response — on the documents themselves.
Solution
A Document Review: the same Bridge, the same Lanes, the same Rounds Contract and result contract, run on documents before any code exists. The caller names the documents (a spec, a ticket set, an issue's sub-issues, a crewtask run) and, where there is one, their Parent. Two axes run in parallel and report separately:
- requirements — do the documents carry their Parent and nothing the Parent never asked for, with acceptance criteria that verify what their own document claims? With no Parent, the documents are held to each other.
- design — do the technical decisions deepen the checkout's existing Modules, Seams, and ADRs? The reviewer loads the
codebase-designskill and judges in its vocabulary.
Each axis carries a template baseline — the rules to-spec and to-tickets wrote the documents under, drawn from mattpocock-skills v1.2.3 and condensed to what it is → how to fix, the way the Standards brief condenses Fowler's smells — and reports in matt's Spec-axis shape: quoted line per finding, under 400 words. Every finding is one the caller can act on by editing the documents; the Dispatcher then earns one re-review per axis against a Response, exactly as a Code Review does.
Code Review is not changed: its briefs, receipt, rounds, and every existing test stay as they are. Design decisions and their grounding: docs/design/document-review.md; the research behind the two axes, docs/research/spec-review-options.md, is superseded by that record where they differ.
User Stories
- As a developer who just ran
/to-tickets, I want an independent reviewer to hold the ticket set against its spec, so that decisions the cutter dropped or invented are found before an agent implements them. - As a developer who just ran
/to-spec, I want the same review to run on a lone spec, so that its sections are checked against each other when there is no Parent to hold it to. - As a developer, I want to name a Parent when there is one and omit it when there is not, so that one rule covers both objects and the receipt says which case I was in.
- As a developer, I want acceptance criteria that do not verify what their own ticket claims reported as findings, so that a ticket cannot pass its own criteria and still miss its goal.
- As a developer, I want scope or decisions the Parent never asked for reported, so that hallucinated work is cut before it is scheduled.
- As a developer, I want requirements the Parent asked for that no document carries, or carries only in part, reported, so that lazy coverage is visible.
- As a developer, I want the design axis to report a Module the documents add that an existing Module already owns, so that a parallel implementation is stopped on paper.
- As a developer, I want the design axis to report a Seam or Interface the documents add where the checkout already has one, so that the seam count does not grow without a reason.
- As a developer, I want the design axis to report a decision that contradicts an ADR or the glossary, so that recorded decisions are not silently bypassed.
- As a developer, I want the design reviewer to load the
codebase-designskill rather than read a paraphrase of it, so that the judgement uses the skill's own vocabulary and probes and the brief does not duplicate it; and when it cannot load the skill, I want that said in the first line of its report. - As a developer, I want each axis brief to carry the
to-spec/to-ticketsrules as a baseline, so that "executed the template well" is judged against the template's own words and not the reviewer's taste. - As a developer, I want only findings that would change what gets built — not wording, format, or hypothetical future needs — so that the review stays short and does not pile up edge cases.
- As a developer, I want the two axes reported separately and never merged or reranked, so that one axis cannot mask the other, as in Code Review.
- As a developer, I want each axis's report written to a file and named in the result, so that I open it where it was written, as in Code Review.
- As a developer, I want each Document Review axis to earn one re-review against a Response, so that a finding I declined is a disagreement and not a message that never arrived.
- As a developer, I want the result to carry the Next Call for that re-review with every document argument echoed, so that I run it as-is.
- As a developer, I want a Parent that cannot be fetched carried as an unfetched reference with its failure detail in the brief and the receipt, so that the review runs and says what it lacked.
- As a developer, I want a document that cannot be fetched to fail preparation before any Lane opens, so that no round is spent reviewing nothing.
- As a developer, I want the Bridge to refuse a call that mixes document arguments with a fixed point or a spec, or names an axis of the wrong kind, so that a wrong call fails before any Lane opens.
- As a developer, I want
--axis bothto mean both axes of whichever kind I asked for, so that the default call shape is the same for both reviews. - As a developer using either Lane, I want a Document Review delivered to
codexin tmux panes or toclaudeheadless exactly as a Code Review is, so that the Lane changes who reads the brief and never what it says. - As a developer whose driver died, I want
--recover-sessionto return the Document Review result with its receipt, so that recovery covers both kinds. - As a developer, I want the design brief to name the checkout's tracked standards documents, so that the reviewer finds this repository's pointer to its glossary and ADRs the way the Standards axis does.
- As a caller inside a Claude session, I want to describe in prose what to review — a parent issue, a run directory, some paths — and have the Dispatcher assemble the Bridge call, so that I never write
--documentflags by hand. - As a caller inside a Claude session, I want the Dispatcher to list a parent issue's open sub-issues as the documents, so that "review the tickets of Spec: one review protocol behind a process boundary — Review-Switch owns the review, callers own the reviewer #11" is one sentence.
- As a caller inside a Claude session, I want a
crewtaskrun directory expanded into its spec as Parent and its ticket files as documents, so that a staged run is reviewable as a unit. - As a caller running a Code Review, I want the Dispatcher to read no Document Review prose beyond a short route, so that the Code Review agent's context holds nothing it does not need.
- As a maintainer, I want every change to the delivery half — Lanes, rounds, Response, Next Call, recovery, hooks — to reach both kinds without a second edit, so that there is one protocol and not two.
- As a maintainer, I want the Bridge to stay one file installed by one symlink, so that the install and the test harness are untouched.
- As a maintainer, I want the Dispatcher to remain the only skill this project installs, so that the Adjudicator and the Glossary stay true.
- As a maintainer, I want the Document Review preparation testable at the command seam through the existing fake Lanes, so that no test crosses into its Implementation.
- As a maintainer, I want the Dispatcher's route lines and reference file asserted by the existing prose-contract test, so that a stale copy of the protocol is caught the same way it is today.
- As a maintainer, I want a receipt for a Document Review that recovery returns as written — without the Code Review legacy fields filled into it — so that a recovered result carries the same facts a delivered one did.
Implementation Decisions
Two review kinds, one table. The Bridge learns a review kind: code (axes standards, spec) and documents (axes requirements, design). One table holds, per kind, its axes, the rounds each axis earns, and the preparation that fills its briefs. The five places that today hard-code Code Review — scope resolution and preparation in the main flow, the expansion of --axis both, rounds per lineage, the accepted axis names, and the per-axis brief dispatch — each become a lookup in that table. Rounds: spec 2, standards 1, requirements 2, design 2.
Kind is inferred from the arguments. --document present means documents; absent means code — the same inference the Bridge already makes when --spec is absent. Argument validation refuses, before any Lane opens: --document together with --base or --spec; a code axis name with --document; a documents axis name without --document.
Bridge Interface additions. --parent <ref> (optional; issue reference or file path, the --spec syntax), --document <ref> (repeatable; required for the documents kind), and requirements / design as axis names. No Review Scope is resolved for the documents kind. All new options are caller-owned, so the existing Next Call mechanism echoes them without change.
A second adapter at the existing preparation seam. A Lane already takes one Axis Brief and reads the preparation's receipt; nothing below that point knows what a diff is. ReviewPreparation is the Code Review adapter and is not edited. DocumentReviewPreparation is the second adapter, with the same Interface — a brief per axis, briefs for a set of axes, and a receipt. It holds the Parent slot, one slot per document, the standards sources, and the two Document Review brief templates. It reuses the Bridge's spec reader for the Parent and for every document — an issue fetched with gh and written beside the report files (ADR-0007), a path read where it lies, a failure recorded — and reuses the standards-source reader for the design axis.
The Bridge stays one file. The single-file symlink install and the by-path test harness are the reasons. The Document Review preparation and its two templates are one delimited section beside the Code Review preparation. This is the decision, not a deferral.
Failure rules. A Parent that cannot be fetched is carried the way an unfetched spec is: the brief names the reference and the failure, the receipt records parentSource as not fetched: <ref> with parentFailure, and the review runs. A document that cannot be fetched fails preparation, before any Lane opens; the review-end Lifecycle Hook still fires.
The receipt for the documents kind: parentSource (<path>, not provided, or not fetched: <ref>), parentFile, parentFailure, documents (a list of source and file), standardsFiles, standardsCondition, codeGraphUsed (always false), responseFile. The Code Review receipt is not changed.
Recovery gets one condition. Recovery today fills the Code Review legacy fields (specFile, specFailure, responseFile) into whatever receipt it reads back. That fill becomes conditional on the receipt being a Code Review one (it carries specSource); a Document Review receipt is returned as written. This is the one edit to the delivery half.
Load what can be loaded, quote what cannot (ADR-0010). to-spec and to-tickets are user-only skills, so their rules reach the reviewer only as the template baseline in the brief. codebase-design is model-invocable and installed for both Lanes, so the design brief asks the reviewer to load it and judge in its vocabulary; a reviewer that cannot says so in the first line of its report, the way an unfetched spec is reported, and the Dispatcher relays that line. This amends one consequence of ADR-0003 and the Glossary's Axis Brief entry says which brief requires what. There is no path fallback, no argument naming the skill's directory, and no resolve script.
The two Axis Briefs. Both open with the environment line every brief carries and close with matt's report sentence shape; neither mentions rounds. Each carries a template baseline whose items are drawn from to-spec and to-tickets v1.2.3 and condensed, plus one item that is Document Review's own rule and says so, and one gate sentence in place of a finding taxonomy.
requirements:
Read-only review: report findings; leave the working tree untouched. Review it yourself in this session.
Parent: {parent_slot}
Documents:
{document_list}
Template baseline (the rules these documents were written under; every item is a judgement call; report only what would change what gets built — not wording, format, or hypothetical future needs):
- Vertical slice: a ticket cuts one complete path through every layer and is demoable or verifiable on its own. → a ticket that is one layer of several is recut.
- Blocking edges: a ticket is blocked only by the tickets that genuinely gate it. → an edge that does not gate is removed; a gate that is missing is added.
- Acceptance criteria: written from the user's perspective, each verifying the behaviour "What to build" claims. → a criterion that checks something else, or nothing observable, is rewritten.
- One context window: a ticket is sized to finish in a single fresh session. → split it.
- No parent (Document Review's own rule): a spec's Problem Statement, Solution, User Stories, Implementation Decisions, Testing Decisions, and Out of Scope are held to one another. → the section that contradicts the others is the finding.
Report: (a) requirements in the parent that the documents miss or carry only in part; (b) scope or decisions the parent never asked for; (c) acceptance criteria that do not verify what their own document claims. With no parent, hold the documents to each other. Quote the line for each finding. Under 400 words.
design:
Read-only review: report findings; leave the working tree untouched. Review it yourself in this session.
Load the codebase-design skill from the mattpocock-skills plugin before reviewing and judge in its vocabulary; if you cannot load it, say so in the first line of your report.
Parent: {parent_slot}
Documents:
{document_list}
Standards sources: {standards_files}
Template baseline (the rules these documents were written under; every item is a judgement call; report only what would change what gets built — not wording, format, or hypothetical future needs):
- Seams: existing seams are preferred to new ones, the highest possible, and as few as possible. → a new seam says why no existing one serves.
- Testing decisions test external behaviour through the interface, not implementation. → a test plan that must cross the interface is a finding about the module's shape.
- The glossary's vocabulary and the ADRs are followed. → name the term or the ADR.
- No file paths or code snippets, except a prototype's snippet that carries a decision. → cut it.
Report: (a) a Module the design adds that an existing one already owns; (b) a Seam or Interface the design adds where the checkout already has one; (c) a decision that contradicts an ADR or the glossary. Name the code or ADR beside the quoted line for each finding. Under 400 words.
Slot fillings: {parent_slot} is the Spec-slot shape (<path> — <summary>. Read it before reviewing.), or not provided; hold the documents to each other., or the unfetched shape with its failure detail. {document_list} is one - <path> line per document, with an issue's summary where the Bridge wrote it out. Standards sources: is the Code Review line reused, including its none documented wording.
The Dispatcher stays the only skill. Its prose gains a three-line route before the section that completes the call: when the caller asks to review documents — a spec, a ticket set, an issue's sub-issues, a crewtask run — rather than a change, read the Document Review reference to complete the call; the Result section and everything after it apply unchanged. The reference holds only what is Document Review's: assembling references from prose (a parent issue → --parent plus one --document per open sub-issue listed with gh; a crewtask run directory → its spec as --parent and each ticket file as a --document; explicit paths or numbers passed as given; no Parent named → no --parent), and the preparation line: Parent: <parentSource> · Documents: <n> · Standards: <condition>, then one line per axis as today; a design report whose first line says the skill could not be loaded is relayed as such. The Dispatcher locates references and never opens a document, as it never opens a spec.
Not changed: Lanes and their dependencies, SessionStore and its files, cost harvest, Lifecycle Hooks and their variables, the Rounds Contract mechanism, Response, Next Call, the Adjudicator, both Code Review briefs, the Code Review receipt, the install.
Testing Decisions
A good test here crosses the seam a caller crosses: arguments in, the brief a fake Lane received and the JSON result out. Nothing asserts on the preparation's internal state; every Document Review behaviour is observable in a brief's text or a receipt's field, and is asserted there.
Bridge tests use the existing harness — the argument builder, the fake Codex and Claude sessions, the pane test case — exactly as the existing preparation, rounds, recovery, and arguments suites do, and live beside them:
--documentwith a parent issue, a parent file, and no parent → the Parent slot in each brief andparentSourcein the receipt.- A parent that cannot be fetched → brief names reference and failure,
parentFailureset, the review proceeds. - A document that cannot be fetched → preparation fails before any Lane opens; the review-end hook still fires.
--documentwith--base/--spec; a code axis with--document;requirementswithout--document→ refused before any Lane opens.--axis bothunder the documents kind →requirements,design, in that order.- Rounds:
designearns one re-review; its Next Callargvechoes--parentand every--document. - A resume rebuilds the Document Review preparation and appends the Response, as a Code Review resume does.
- Recovery returns the documents receipt as written — no
specFile/specFailurekeys added — and a Code Review receipt with its legacy fields filled, as today. - No tracked standards documents → the design brief carries the existing
none documentedwording. - Every existing Code Review test unchanged and green — the proof the kind table changed nothing for the
codekind.
Dispatcher tests use the existing black-box shell seam: the prose-contract test gains assertions for the route lines and the reference file.
Out of Scope
- A third "consistency" axis; a
ready / needs-decision / limitedstatus; basis fields; a numeric cap on findings; a finding format of its own. - Editing documents automatically, or publishing review findings to the tracker.
- A code graph navigation block for Document Review.
- Splitting the Bridge into modules, or a second command.
- Reviewing documents that are still only in a session's context and not yet published or written to disk.
- Reviewing the conversation that produced a spec; a spec's Parent is a document the caller names, or nothing.
- Deciding an authority order between conflicting sources; a conflict between Parent and document is reported, and the caller decides.
Further Notes
- Glossary entries for Code Review, Document Review, and Parent, the amended Axis Brief entry, and ADR-0010 are in the private brainstorming repository.
- Both Lanes on this machine have the mattpocock-skills plugin installed, so the design brief's load sentence works on either.
- The two-axis split and the baseline-in-brief pattern are inherited from matt's
code-review; the axis contents come from the research indocs/research/spec-review-options.md, narrowed to whatto-specandto-ticketsthemselves require.
Activity
Simon-AI-coding commentedon Aug 30, 2026
/crew crewtask/3
docs: record the Document Review design and the research behind it (#43)