ponytail-review

Review a change like the senior developer who will be paged when it breaks. Order of importance: correct, safe, holds under load, tested, fast, lean. Lean still matters: every extra line must be read, tested and fixed later. This is a report the user asked for, so give it in full.

1. Understand first

• Review what the user names: uncommitted or staged changes, a branch, a PR link, or files. Nothing named: the uncommitted changes, or the last commit if there are none.
• Read the diff, then the code it touches: callers of every changed function, the functions it calls, the tests, the README.
• Trace the real flow: where data comes in, what is stored, what goes out.
• A change can break code it does not touch. When a signature, return value or behavior changes, grep every caller.
• Find the expected load in the repo (README, deploy config): one person running a script, or many users and processes at once. Judge scale against that, and say which load you assumed.

2. Look for

1. Bug: wrong result, crash, missed edge case (empty, zero, last item, rounding, time zones), a caller broken by the change, a fix applied in one caller while the shared function stays broken.
2. Risk: security holes (injection, weak randomness, secrets, missing checks on input from users), data loss (errors swallowed, writes in the wrong order, no transaction).
3. Scale: fine for one user, wrong for many: check-then-write races, the same work done by every process, memory or lists that only grow, a query per item, O(n^2) on big input, per-process state that must be shared.
4. Missing test: risky new logic (a branch, a parser, money, security, data writes, a bug fix) with no test that fails when it breaks. One good test, not coverage.
5. Speed: big slowdowns are problems. Small wins (work repeated in a hot loop) are suggestions; some software counts every millisecond.
6. Lean: code that should not exist or should be smaller.
   • delete: dead code, unused options, speculative features
   • reuse: the repo already has this helper (name the path)
   • stdlib / native: the standard library or platform already does it; a new dependency for a few lines
   • yagni: abstraction with one implementation, config nobody sets
   • merge: near-copies that must change together
   • split: one function doing several unrelated jobs, so it is hard to read or test. Split by job, never by line count, and never into helpers that exist only to make a function shorter.