code-review

Files (2)

• SKILL.md
• assurance-context.md · references/

SKILL.md (first lines)

# Code Review

If this plugin is not initialized or an Agent IX command fails, read [the dev-tools setup guide](https://github.com/agent-ix/dev-tools/blob/main/setup.md) for its prerequisites and local diagnosis.

Perform the following checks on the codebase.

## Language Dispatch (do this first)

The checks below are written for Python. Detect what the reviewed code
actually is and run the matching lane — transliterating a Python check into
another language produces false findings ("no test classes", "no `pass`
placeholders") that bury the real ones.

| Detected | Lane |
|---|---|
| `Cargo.toml` / `.rs` files in the change | invoke `rust-review` and run it **instead of** Test Standards, Mock Compliance, Completeness, and Edge Case & Logic Review |
| `package.json` with `react` | additionally run the React lane below |
| `pyproject.toml` / `setup.py` / `.py` files | the Python checks below, as written |

Language-independent sections — **Duplication and Vendoring**, **Integrity**, **Spec-Code Faithfulness**,
**Code-Test Alignment**, **Gap Analysis**, **Output** — always apply. Read
"docstring" as "doc comment". Tests bind to acceptance criteria by criterion id through
the tag forms quire declares:

- a `Trace: FR-XXX-AC-N, …` line — a docstring or `#` comment line in Python, a `//`
  comment or JSDoc line in TS/React, `///` in Rust
- the Rust `#[trace("FR-XXX-AC-N")]` attribute
- the Python `@pytest.mark.trace("FR-XXX-AC-N")` marker
- the TS `trace("FR-XXX-AC-N")` marker

Ids in a `Criteria:` list, a Description, or a `Tracing:` line bind nothing. Only test,
bench and fuzz functions bind; a tag on a Storybook story or other non-test code is
documentation only.

A mixed repo runs several lanes; scope each to the files it applies to.

Also load the repo's own conventions before judging style: `CLAUDE.md`,
`AGENTS.md`, and any `.claude/skills/*` describing the language's idioms in
this codebase. A documented repo idiom outranks this skill.

## Applicable assurance context

Before applying the ordinary rubric, look under the reviewed repository's `spec/` tree
for an `AssuranceProfile` whose stated scope applies to the changed code or behavior.
Do not create a profile and do not apply an unrelated one. If no applicable profile
exists, continue with the ordinary review below unchanged.

When a profile applies, read
[references/assurance-context.md](references/assurance-context.md). Use that context to
focus the existing checks and findings; it is not a second checklist. Name the profile
in the review artifact, and state explicitly when selected architecture, impact, measurement, exception, independence, structural-gap, or
producer-reliance evidence is unavailable. The durable artifact MUST contain an exact
`## Assurance Context` heading whenever a profile applies; discussing the profile only in
the summary, findings, or chat does not satisfy this requirement.

## Duplication and Vendoring (P0 — run this first)

**Vendoring is a CRITICAL P0 finding.** It is an antipattern that will almost always be
rejected in favour of a cleaner design. Report it at `high` severity, which forces a
**FAIL** verdict. Do not soften it to `medium` because the copy is small, well-documented,
provenanced, or temporary — those are the properties of every copy that ever shipped.

No vendoring. No duplicating. No copying code or specs. Find one home for a thing and
depend on it.

**The defect is duplication.** One question, asked of every change: *does this
repo now hold a second statement of something that is defined somewhere else?*
If yes, it is a finding. The thing now exists in two places, so it has two
owners and one of them will drift. That is the whole of it.

**Nothing launders a duplicate.** Not retyping it, translating it to another
language, reformatting it, renaming its identifiers, regenerating it from local
types, splitting it across files, taking only part of it, or paraphrasing a
spec into new prose. Every one of those still leaves two places that must be
edited when the thing changes. Effort spent reworking a duplicate is evidence
of vendoring, not a defence against it.

**Apply it mechanically, not as a judgement.** No case analysis, no weighing of
how small, how documented, or how temporary. The weighing step is where new
justifications get invented.

**Read the lists below as examples, never as a definition.** They are shapes
already seen, written down so they are recognisable on sight. They are not the
boundary of the rule and they are not exhaustive. A copy whose form appears
nowhere in this skill is still a finding, and "it isn't any of the listed
forms" is not a finding's refutation — the next copy is always spelled a way
nobody wrote down yet.

**Two things that are themselves findings:**

- Composing a sentence explaining why *this* copy is acceptable. If a reviewer
  or author is writing that sentence, the finding is already located.
- Reasoning about whether a case falls inside the rule's wording. The rule is
  not a boundary to sit near. Arguing the edge of the definition — rather than
  asking whether the thing now exists in two places — is the tell.

### What counts as vendoring

A copy is a copy however it is spelled. Examples, not an exhaustive list:

- `vendor/`, `third_party/`, `schemas/vendored/`, `corpus/<upstream>-v1/`
- a snapshotted `schema.json`, however its comment describes it
- `include_str!` / `include_bytes!` of another repo's artifact
- a fixture tree copied from another repo, with or without `VENDOR.json` / `PROVENANCE.json`
- a script that refreshes a copy by shelling into a sibling checkout
- anything that reads a sibling checkout by absolute path
- the same requirement, type, or constant re-stated in a second repo's spec
- another repo's values re-stated as a **native const, enum, or type** in code —
  `const TARGETS: [&str; 14]` holding an upstream enum's members is that enum in
  a different syntax
- a duplicate **generated** from local types (schemars, TypeSpec, codegen). "We
  own it because we generate it" describes the mechanism, not the ownership; if
  upstream defines the vocabulary, generating it here makes a second source
- a duplicate paired with a **conformance or drift gate** watching it. That is
  duplication with a detector bolted on — the finding is the duplication
- a **pin that does not pin the artifact**: a submodule tag or lockfile that
  rebuilds to something other than what it names
- a plan that **phases** removal behind a producer shipping something ("the
  copies go once upstream publishes"). Reads as sequencing, works as a reprieve

From agent-ix/dev-tools, MIT.

More from agent-ix/dev-tools