perf-leak-review

Files (4)

• SKILL.md
• repo-wide-sweep.md · references/
• reviewer-brief.md · references/
• validate_findings.py

SKILL.md (first lines)

# perf-leak-review

## Overview

A session writes code and the next thing that reads it is a reviewer looking for
correctness. Performance defects and leaks survive that pass: they are correct
code, they pass the tests, and the cost shows up in production instead. praxis
has no axis that looks for them — [`retrospect`](../retrospect/SKILL.md) grades
workflow friction, and its one "performance" word means a slow *tool call*, not
slow code.

This skill hands a unified diff and the repository around it to a **read-only
LLM reviewer** and asks one question: does this change introduce a candidate in
one of five defect classes? It is **report-only** — it runs none of the
project's code, writes nothing into the repository, and grades every finding
`candidate` rather than defect, the same read-oracle lineage
[`codex-review-wrap`](../codex-review-wrap/SKILL.md) sits in.

**Core principle:** a reader can point at a candidate; only an execution can
prove a defect — so the output says `candidate` and never more.

**References:** [`references/reviewer-brief.md`](references/reviewer-brief.md)
is the delegated reviewer's prompt (Step 3);
[`references/repo-wide-sweep.md`](references/repo-wide-sweep.md) is the
area fan-out for a request with no diff. The skill ends at the candidate list;
proving a candidate by execution is outside it.

## When to Use

- A change touches loops, I/O, handles, subscriptions, caches, or query paths
  and you want the leak axis read before it merges.
- You have a diff (any repository, any language) and want its perf/leak
  candidates listed with file and line.
- Do **not** use it to measure anything. No profiler, no benchmark, no
  before/after comparison — those need execution, which this skill refuses.
  Proving or refuting a candidate by running it is separate work that starts
  from this skill's report.
- No diff — the user wants the whole repository swept. That changes this
  skill's contract (Step 1 requires `--diff`); follow
  [`references/repo-wide-sweep.md`](references/repo-wide-sweep.md), which
  reuses the reviewer brief per area and reports area sampling, not
  exhaustive coverage.

## The closed list

Five classes, and nothing outside them is reported. The list is the spec:
widening it is a separate change to
[`references/reviewer-brief.md`](references/reviewer-brief.md) and
`validate_findings.py` together.

| #  | Class                                                      |
|----|------------------------------------------------------------|
| C1 | N+1 / remote call inside a loop                            |
| C2 | Unreleased resource (file, socket, connection, subprocess) |
| C3 | Unbounded global container or cache                        |
| C4 | Load-everything-then-filter-in-memory                      |
| C5 | Unreleased listener, callback, or timer                    |

## Caller-agnostic by contract

The skill takes `--diff <path> --repo <root>` and returns a finding envelope. It
does not decide **when** a review runs or **where** the findings go — those are
the caller's, and they are deliberately unsettled. So no step here reads a
session transcript, posts a comment, opens an issue, or blocks anything; a
trigger surface added later attaches by passing the two arguments.

## Process

### Step 1: Resolve the two inputs

```bash
git rev-parse --show-toplevel
```

`--diff <path>` is a unified diff file and `--repo <root>` is the root the diff
applies to. **`--diff` is required.** Step 3 hands the reviewer a path, and this
skill cannot produce one from `git diff`: that writes to stdout, and the
`allowed-tools` above carry no `Write` and no redirect, deliberately — a skill
whose whole claim is that it does not write to the repository does not get a
file-creating step. A caller with no diff file on hand makes one itself
(`git diff > <path outside the repository>`) and passes it.

With `--diff` given and `--repo` omitted, the root comes from
`git rev-parse --show-toplevel`. Both paths are read-only for the rest of the
run.

Stop if `--diff` is absent and report
`perf-leak-review: --diff <path> is required — this skill does not create one`.

Stop if the diff is empty: there is nothing to review, and an empty envelope
from an empty diff says nothing about the reviewer.

### Step 2: Check the host can delegate

This step needs the `Agent` tool. On a host that does not provide it — the Codex
host is **UNKNOWN** here, never verified — **stop** and report
`perf-leak-review: this host cannot delegate a read-only reviewer — unverified
here`. Do not review the diff inline instead: the session that wrote the diff is
the one context that cannot read it independently (CLAUDE.md, *the author-exempt
trap*), and an inline fallback would silently trade that away.

### Step 3: Delegate the review

Read [`references/reviewer-brief.md`](references/reviewer-brief.md) and pass it
as the prompt, verbatim, followed by the two resolved paths:

```text
Agent(
  subagent_type="general-purpose",
  prompt=<the full text of references/reviewer-brief.md>
         + "\n\n--diff <resolved diff path>\n--repo <resolved repo root>\n"
)
```

One delegation per diff. The brief, not this file, is the source of truth for
what the reviewer is told — the class definitions, the read budget, the tool
restriction, and the output schema all live there, so they cannot drift from
what the validator enforces.

From devseunggwan/praxis, MIT.

More from devseunggwan/praxis