Skill library

Skill library
Developerskills/developer/code-review/SKILL.md

Code Review

How we review SPYN code — what to look for, what to ignore, how to disagree well. Lead reviewer is Ahmed Mahmood Khan.

Code Review

Code review at Media Tech exists to catch real problems before they reach SPYN customers, to spread knowledge across a small team, and to keep the Laravel + React Native codebase coherent over years. It does not exist to demonstrate seniority, to enforce stylistic preferences, or to gate merges on questions of taste. Ahmed Mahmood Khan is the de facto Lead Reviewer for the backend; Frontend leads will pick that up for the mobile app once the role is filled.

What reviewers should look for

In rough priority order:

  1. Correctness. Does this code do what the PR description says it does? Are there edge cases the author missed? Does it handle errors in a way that the rest of the system expects?
  2. Security. Does this introduce an injection vector, expose data that shouldn't be exposed, or weaken an existing boundary? For SPYN, treat anything touching auth, diary content, image uploads, OpenAI prompt assembly, or user PII with extra care. Prompt injection on AI Diary endpoints is a real threat — review for it explicitly.
  3. Operability. Will we be able to debug this in production at 3am? Are logs present where they need to be? Are metrics emitted for the parts that matter?
  4. Tests. Do the tests actually test the thing? A passing test that never asserts the right behavior is worse than no test at all.
  5. Readability. Will a future engineer be able to understand what this code does and why? Names, comments where the why is non-obvious, and structure.

Style and formatting are not in the reviewer's purview — those are enforced by the linter. If the linter doesn't catch it, it isn't worth a review comment.

What reviewers should ignore

Personal preference. If two patterns are both reasonable and the author chose one, the reviewer does not get to demand the other. Bikeshedding is the most common failure mode of slow review cycles.

Authoring a reviewable PR

Keep PRs small. The single best predictor of review quality is PR size: under 400 lines of diff, review quality is high; over 1000 lines, reviewers skim. If you can't get under 400 lines, split it.

Write a PR description that says what changed, why, and how to verify. Link the ticket. Call out anything reviewers should pay extra attention to.

AI-assisted review

We allow and encourage AI-assisted review as a first pass for catching obvious issues — typos, missing null checks, dead code, missed test cases. The AI's comments are a starting point; a human reviewer still owns the approve decision. Do not approve a PR you have not read yourself.

Disagreeing well

When you and a reviewer disagree, default to discussing in the PR with concrete reasoning. If you remain stuck after a couple of back-and-forths, take it to a sync call — fifteen minutes of conversation beats fifteen comments. The author has the final call on judgment matters; the reviewer has a veto on correctness, security, and operability.

Triggered automatically

Code Review is event-driven: when a PR opens against any SPYN repo (spyn-backend, spyn-mobile, spyn-admin-panel), Vera generates a review checklist for that PR keyed off the specific files changed and the spec it references. The checklist is advisory — the human reviewer still owns the approve decision.

Owned by

Ahmed Mahmood Khan (Lead Developer), with Engineering Leadership as co-owner.