fallow-rs/fallow-skills/fallow-review

Fallow Review: graph-grounded code review

fallow review turns a changeset into a deterministic, graph-derived review brief and an agent-contract loop. It answers "where do I look, and what calls actually need human taste?" rather than "will CI block this?" (that is fallow audit, which gates). review always exits 0, so it is safe to run regardless of verdict.

The four jobs it does, in order:

• subtract: dead code, complexity, duplication, and styling for the changed files are reported and kept OUT of the judgment loop, so attention is not spent on what a deterministic check already owns.
• focus: changed-file units are ranked by a composite attention score (fan-in/out, risk zone, change shape) with a review-here / not-prioritized label and a full deprioritized escape-hatch list, so nothing is hidden.
• structure: the decision surface lifts the handful of consequential STRUCTURAL decisions out of the diff and frames each as a judgment question, capped to a working-memory-sized set, each anchored to a signal_id fallow emitted.
• direct: the walkthrough guide hands an agent a graph-derived digest, the review direction, a graph-snapshot pin, and the exact judgment schema to return.

When to use

• Reviewing a PR, branch, or diff (AI-generated or human-written) before merge.
• After an agent has done work and removed the fallow findings it could; this surfaces what is left for human/agent taste.
• Producing inline-reviewable judgments that can flow back to the agent that wrote the code.

When NOT to use

• Gating CI on a pass/fail verdict: use fallow audit (it exits non-zero on a fail verdict).
• Whole-project health, cleanup, or dead-code reports: use the fallow skill.

Quick human brief

# Auto-detect the base (merge-base against the upstream / remote default):
fallow review
 
# Pin the base, or scope to a precise diff:
fallow review --base origin/main
git diff --find-renames origin/main...HEAD | fallow review --base origin/main --diff-stdin

The human brief prints the orientation facts, the focus map, and the decision surface ("Decisions to make"). --format json emits the full structured envelope (decisions, focus, deltas, impact_closure, partition, graph_facts). --max-decisions N tunes the cap (clamped to a small band). --show-deprioritized expands what the focus map collapsed.

The decision surface

Each decision is a framed question anchored to a signal_id fallow deterministically derived from the graph (a delta key or a coordination-gap key). There are exactly three shippable categories:

• coupling-boundary: a new cross-zone dependency edge.
• public-api-contract: a new exported public-API surface, or a changed contract consumed by modules OUTSIDE this diff (a coordinate-or-confirm signal).
• dependency: a changed package.json that adds third-party entries or moves a declared entry across a major version (or a 0.x minor). One batched decision per manifest per kind, weighted by the graph's in-repo importers of the affected packages: blast is the importer count, internal_consumer_count the importers outside the diff. Both counts are a union over the batched packages. An entry outside dependencies shows a (dev), (optional), or (peer) tag in the question. digest.deltas carries the keys as dependency_added (<manifest>::<name>) and dependency_major_bumped (<manifest>::<name>@<from>-><to>); both lists are always present, possibly empty. Minor and patch bumps and non-numeric ranges (workspace, file, git, tags) are never candidates. A dependency decision has no suppress action: it anchors on package.json, which cannot carry a comment, so never paste a // fallow-ignore line into a manifest. The decision_surface MCP tool surfaces the same dependency decisions as the CLI. Framing rule: cite the changelog question ("which documented change in <from> to <to> reaches these N importers?"); do not guess the behavior change.

A decision may carry previous_signal_id when its anchor file was renamed in the change: that is the signal_id the same decision would have had at the old path, so a review surface can re-attach a prior reviewer comment across a git mv.

Eliciting the broader trade-offs (taste ownership)

The decision surface above is the DETERMINISTIC slice: only the trade-offs fallow can prove from the graph (the three categories). Real architectural trade-offs are broader (abstraction level, error-handling strategy, data-model shape, eager-vs-lazy, state ownership, extensibility-vs-YAGNI, testability, trust boundaries), and none of those are graph-detectable. Surfacing them needs a model reading the diff, not a static pass.

Run the trade-off elicitation prompt in references/tradeoff-elicitation.md over the diff plus the guide. It applies TASTE OWNERSHIP: the model makes each choice legible and frames a genuinely open question; the human decides. The prompt carries the honesty contract (diff anchoring, the observed / tradeoff / question split, deterministic: false fencing, provenance, the top-five cap with honest abstention, no duplication of the deterministic surface, options that never pick). fallow owns what it can prove, the prompt covers the rest, and the fencing keeps the two from being confused.

Running it as a review step

When you want the trade-off surface alongside the deterministic decisions:

1. Fetch the guide and read the diff:

``bash fallow review --base <ref> --walkthrough-guide --format json > guide.json git diff <ref>...HEAD # or: git diff --cached for staged work ``

1. Run the references/tradeoff-elicitation.md prompt over the diff plus guide.json.
2. Keep the envelope (the { graph_snapshot_hash, abstained, tradeoffs[] } shape). The composed review below uses it.

This step is deliberately NOT part of the post-validate agent-contract loop below. The trade-off envelope is never round-tripped through fallow review --walkthrough-file (which only validates emitted signal_ids), so it carries NO fallow-grade guarantee: every item stays deterministic: false, agent-self-checked against the diff, not graph-validated. Do not present it as fallow-validated.

The agent-contract loop

The loop lets an agent produce judgments that fallow post-validates against the live graph. The verifier is the graph, not a second model.

1. Fetch the guide:

``bash fallow review --base origin/main --walkthrough-guide --format json > guide.json ``

The guide contains: digest (the brief + decision surface), direction (where to look), graph_snapshot_hash (the staleness pin), agent_schema (the exact shape to return), and injection_note. The digest is built from the graph ONLY; PR prose is never folded in, so the guide is injection-resistant by construction.

1. Read the decisions in digest. Each carries a signal_id, a category, the framed question, and an anchor_file / anchor_line.

1. Return a judgment matching agent_schema:

``json { "graph_snapshot_hash": "<echo the value from the guide>", "judgments": [ { "signal_id": "<one signal_id fallow emitted>", "framing": "<your reasoning for the human reviewer>", "action": "<block | address | consider | fyi>", "concern": "<optional: one lens from agent_schema.concern_vocabulary>" } ] } ``

Every signal_id MUST be one fallow emitted in the guide (emitted_signal_ids). An unanchored id is rejected. Echo the graph_snapshot_hash verbatim.

For a changed region without a finding, a judgment can cite an emitted change_anchor (a chg: id from the guide's change_anchors) in place of a signal_id. Each anchor carries file, start_line and line_count. An unknown id is rejected as unknown-change-anchor.

action tells the author what the judgment asks of them: block and address are required actions, consider is optional, fyi needs nothing. concern names the lens; prefer the guide's agent_schema.concern_vocabulary, thirteen kebab-case lenses: abstraction, coupling, data-model, error-handling, control-flow, performance, dependencies, api-ergonomics, compatibility, state-ownership, extensibility, testability, trust-boundary. The guide publishes both lists as agent_schema.action_vocabulary and agent_schema.concern_vocabulary; read them from the guide rather than from this file. action_vocabulary is enforced; concern_vocabulary is advisory, any string is accepted.

1. Post-validate:

``bash fallow review --base origin/main --walkthrough-file judgment.json --format json ``

The response sorts each judgment into:

• accepted: the signal_id was emitted and the snapshot matches; the agent's framing is fenced as non-deterministic (deterministic: false) and never gates. The action is echoed next to agent_framing, fenced the same way: it is an instruction to the author, never a fallow fact.
• rejected with reason: "unanchored-signal-id": the signal_id was never emitted (a hallucination). Drop or correct it.
• rejected with reason: "invalid-action" and invalid_value (the label fallow refused): the action is outside the vocabulary. This is reported only after the anchor resolved; a hallucinated anchor plus a bad label rejects as unanchored-signal-id / unknown-change-anchor instead. Fix the anchor first, then the label.
• rejected with reason: "stale-snapshot" and stale: true: the tree moved since the guide was fetched. Re-fetch the guide and redo the judgments.

Production runtime data (Fallow Cloud)

When the project sends production coverage to Fallow Cloud, check the changed functions against it. Use the scoped reads, not the full runtime-context pull:

• Before an edit: fallow coverage review-packet --repo <owner/repo> --base <ref> (MCP: get_cloud_review_packet). Read hit_count, covered_by_test, and the callers when present. A hot function with covered_by_test: null is a high-risk edit.
• Before a delete: require period_tracking_state: "never_called" over the period, not only tracking_state, and read evidence_window.observed_hours. Low observed hours or a low-traffic surface mean "not visited", not "dead".
• After a deploy: fallow coverage deployment-changes --repo <owner/repo> (MCP: get_cloud_deployment_changes). When comparable is false, report reason and claim no change.
• Open files by repo_path, not file_path.

The data is context: cite it in a framing, never as a gate. Field meanings and the full recipe: [references/production-runtime-data.md](references/production-runtime-data.md).

Compose the review

Validation is not the review. Once the judgments are accepted, render them for a human in this fixed order, so the reader lands on what changes the outcome first and the deterministic remainder last: