lemoncrow-lab/lemoncrow/perf-review

Performance review

Gates a code change against its performance bar by running it — five measured signals: latency & throughput (tail incl.), hot-path truth, memory & resource leaks, I/O & wire budgets, measured scaling. Any language/project: discover repo's own benchmark/profiler/load/query-log/browser-perf tooling; never assume stack. Weighs every cost in context (Guardrails). Not general code review (use /code-review); no inline auto-fixes — engineer owns fix. Opt-in remediation on request: one solver per blocker, re-measured before merge (step 11).

On invoke: state measurement plan; fixes opt-in — per-blocker solvers, re-measured before merge. Then gather inputs. Whatever the target's phrasing — a change to check, or a question about perf-review itself — it's the surface to measure; never substitute a hand-written explanation for actually running the gates below.

Operating loop

1. Ground the target and baseline. First discover project's own perf tooling + invocation (CLAUDE.md/AGENTS.md, README, CI config, dependency manifest): bench runner, profiler, load generator, query log, (UI) browser-perf. Examples — bench: pytest-benchmark, go test -bench, cargo bench/criterion, JMH, hyperfine, own suite; profiler: cProfile/py-spy, pprof, perf, async-profiler, Instruments, browser devtools; load/soak: wrk, k6, locust, ab; memory/leak: tracemalloc, memray, valgrind, heaptrack, /proc + fd counts. Page/frontend: Core Web Vitals (LCP/INP/CLS/TBT) via project browser tooling (Lighthouse, Chrome DevTools, Playwright traces); host web-performance skill present → delegate page-load measurement. Surface defaults to the invocation argument (/perf-review <target>) if given — never re-ask it. Remaining gaps → one AskUserQuestion call, minimum: surface (only if not given via argument); baseline (default: pre-change code via VCS — working-tree stash or parent commit); thresholds (defaults: 5% wall-time on p50 _and_ p99, 0 new allocations on the hot path, 0 net heap/handle growth across the soak, 0 new queries/round-trips); representative workload; input sizes for scaling curve; soak iteration count. Confirm exact commands per side-effect guardrail before any run.
2. Establish the baseline. Measure unchanged code first (stash diff or checkout baseline via VCS). Capture full latency distribution (p50/p95/p99), not mean only.
3. Measure the change. Same bench + workload on changed code — same machine, inputs, iterations beating noise. Record variance + distribution, not one number.
4. Gate — latency & throughput. Change-vs-baseline wall-time/throughput delta beyond threshold _and_ outside noise band = Blocker. Gate tail (p95/p99), not p50 only — median flat, p99 blown = fail. Include cold-start/warmup/first-call cost, time-to-first-byte (streaming), latency under representative concurrency (profiler-surfaced lock contention, queueing count). Page surfaces: regressed Core Web Vital (LCP/INP/CLS) = Blocker.
5. Gate — hot path. Profile change under workload (stack's profiler); confirm real top cost centers incl. off-CPU/lock-wait on concurrent loads. Uncorroborated hot-path claim, or fix aimed at non-hot path = Blocker. Usual CPU-burn shapes: busy-wait/hot-poll loops; retry storms (no backoff/attempt cap) burning idle cores; per-call re-creation of expensive resources (client/connection/pool/compiled artifact rebuilt per call → connect/TLS/setup churn under sustained load); redundant recompute, O(n²) inner loops; lock contention from unbounded concurrency. Each = Blocker only if profiler-material on critical path. Before labeling waste, name feature served — feature-paid work ≠ waste even ranked high (Guardrails: product guarantee).
6. Gate — memory & resource leaks. Stack's memory/allocation profiler under workload: peak memory/allocations + GC pressure (allocation rate, pause time/frequency). Then soak many iterations: heap/RSS and open fds, sockets, DB connections, threads must return flat. Monotonic growth = leak = Blocker — single peak reading can't catch. RAM shapes to target: unbounded caches/maps/sets, no eviction/TTL; per-session/per-turn accumulators, history, buffers never trimmed (worse if deep-copied each call); large payloads pulled wholesale into memory (whole file, base64 blob, full embedding matrix/result set, no size cap) — spike on big input, leak if never released on refresh/reindex. Soak must exceed any eviction cap — below-cap run proves nothing above it. Growth beyond budget = Blocker.
7. Gate — I/O & wire. Under workload measure: query count and N+1; query plan/index usage (EXPLAIN — silent full-scan = regression even at unchanged count); cache hit-rate (caching changes); syscall/disk-IO volume; network round-trips; response payload size/bytes over wire (agent/LLM surfaces: tokens per call/response); shipped artifact/bundle/binary size. Growth beyond budget = Blocker.
8. Gate — scaling & worst-case. Never infer complexity from one point: measure growing input sizes, fit curve. Superlinear where linear claimed (or worse than stated) = Blocker. Probe worst-case/adversarial inputs — data skew, pathological regex/catastrophic backtracking, degenerate/deeply-nested structures; collapse on realistic worst case = Blocker. Probe fan-out ceilings: work scaling with caller-controllable count (threads, processes, subprocesses, tasks, connections, worktrees) — drive count up, watch process/thread/fd/memory totals. Unbounded fan-out, no ceiling = resource-exhaustion Blocker even with cheap units — one call → thousands of units → OS process/fd/memory exhaustion.
9. Critique (advisory only). Theoretical complexity, micro-optimizations, unmeasured "could be faster" = Warnings, never blockers — speculation not gate-able. Measured but immaterial on critical path = also Warning; Blocker = material costs only (Guardrails: out of budget).
10. Verdict. End with exactly one fenced JSON block (final element of review), caller-parseable:

{
  "verdict": "NEEDS_FIX",
  "gates": {
    "latency": "fail",
    "hot_path": "pass",
    "memory": "pass",
    "io_wire": "pass",
    "scaling": "pass"
  },
  "baseline": "parent commit (HEAD~1) vs working tree",
  "measurements": {
    "latency": "search() p50 4.1ms -> 6.8ms (+66%), p99 9.0ms -> 31ms (+244%); noise band +/-3%",
    "soak": "1000 iters: RSS flat, fds flat (no leak)",
    "scaling": "fit exponent 1.0 over 4 input sizes (linear, as claimed)",
    "iterations": 200
  },
  "blockers": [
    "search() +66% p50 / +244% p99 wall-time vs baseline (threshold 5%) — the index is re-walked in full on every call instead of reused"
  ],
  "warnings": [
    "a hot-loop allocation in the parser is wasteful but off the measured hot path (0.4% of samples)"
  ],
  "not_checked": [
    "production-scale dataset",
    "sustained concurrent load",
    "cold OS page cache",
    "GC behavior under memory pressure"
  ]
}

1. Remediate (optional, user-gated — never automatic). NEEDS_FIX → engineer owns fix by default. User opt-in only (confirm via AskUserQuestion after verdict); reviewer never hand-edits product code. You stay the orchestrator: spawn solvers via host sub-agent capability — create worktrees, dispatch, re-measure, open PRs. Never hand remediation to a workflow/swarm engine running end-to-end without you — you own the loop. Per blocker, own pipeline, independently:
2. Isolate. One git worktree per blocker (host worktree/swarm/sub-agent capability; else git worktree add). One finding, one worktree — no collisions, masking, or all-or-nothing bundle merges.
3. Spawn one sub-agent per blocker, yourself. Host sub-agent tool, one solver per finding/worktree, orchestrated directly. Hand each only its finding: measured evidence (numbers, profiler/soak output, exact cost center) + minimal-fix hint. No solver takes two findings; no refactor scope-creep.
4. Re-measure _and_ re-verify the feature — don't trust the diff. Solver done → (a) re-run finding's failed gate(s) in its worktree — identical harness, workload, inputs, warm/cold state, iteration count; gate must pass, no previously-passing gate regressed. Measure the right thing — change-blind metric (net-growth on file rewrite) or fix-missing workload (soak below eviction cap) proves nothing; (b) re-verify product guarantee holds — clearing number via degraded freshness, durability, ordering, or accuracy = false solution. Gate not pass, or feature broken → not done — send back or report unresolved; never merge.
5. Review. Show user per-finding before → after numbers, not just "fixed".
6. Merge gate. Merge to main (repo convention — PR or direct) only when (a) re-run proves gate cleared on same workload, (b) fix preserves product guarantee (no false solution), (c) user approves finding's numbers. Rejected → discard worktree. Merge per-finding — each fix judged on own evidence.

Guardrails