fallow-review

Review AI-generated or human-written code changes with fallow's graph-grounded review brief. Subtracts deterministic concerns (unused code, complexity, duplication, styling) from the loop, ranks what to look at by blast radius and risk, and surfaces the few consequential structural decisions (new pu

By fallow-rs · 481 installs

npx skills add fallow-rs/fallow-skills --skill fallow-review

Source repository · Upstream listing

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 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: 2. Run the references/tradeoff elicitation.md prompt over the diff plus guide.json . 3. 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 id s), 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: 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. 2. Read the decisions in digest . Each carries a signal id , a category , the framed question , and an anchor file / anchor line . 3. Return a judgment matching agent schema : 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. 4. Post validate: 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: false or 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 . Without a local fallow install or checkout, use the hosted Fallow Cloud MCP server. It also has get file context , verify deployment , list errors , get branch usage and get runtime dependencies . In a checkout, prefer the local server. 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: 1. Accepted decision judgments , in direction.order . Each one carries the digest question and tradeoff verbatim, then your framing , then the number that gives it weight: internal consumer count , the out of diff paths, or the unit's scoring budget . 2. Trade offs from the elicitation step, ranked by consequence , at most five. When the envelope is abstained: true , print one line saying so; do not fill the slot. 3. Subtract as ONE line with the counts from the brief: "handled deterministically: N dead code, N duplication, N complexity, N styling; not in the discussion". Never re derive any of these from the diff; the brief already owns them. 4. Deprioritized as one line with the count and show deprioritized as the escape hatch, so nothing is hidden and nothing is padded. 5. A review with zero decisions and an abstained trade off envelope is a complete review: "nothing consequential; deterministic findings: N". It is not a failure and never a reason to invent items. Three graph facts feed the composition. Only a test reach note can add an item, and it anchors on a change anchor : Each direction unit carries test adjacency : none (no test file imports this unit), untouched (a test importer exists but is not in the diff), or changed (a test importer is in the diff). It is absent for test files and when the graph was not retained. For a review here unit with none , ask the author for the verification story in the framing. Never claim coverage; the value says whether a test imports the unit, not whether it exercises the change. digest.test reach gives the reach of each changed value export. status is measured or no tests . counts holds every class. exports lists rows only for the classes that are not di