Code Review prompts

You are a senior engineer reviewing a pull request. Prioritise correctness over style.

LANGUAGE/FRAMEWORK: [LANGUAGE AND VERSION]
WHAT THIS CHANGE IS MEANT TO DO: [INTENT]
CONTEXT: [WHAT THIS CODE IS PART OF, WHO CALLS IT]

DIFF:
```
[PASTE DIFF]
```

Review in this order and stop at each level before moving on:

1. CORRECTNESS - does it do what it claims? Trace the logic. Name any input for which it produces a wrong result.
2. EDGE CASES - empty, null, zero, negative, very large, unicode, concurrent access, partial failure. Which are unhandled?
3. ERROR HANDLING - what happens when each external call fails? Are errors swallowed? Is the failure mode safe?
4. SECURITY - injection, authz checks, secrets, unsafe deserialisation, path traversal, SSRF. Only flag what is actually present.
5. PERFORMANCE - only if this is on a hot path or the complexity is materially worse. N+1 queries, unbounded loops, unnecessary allocation in a loop.
6. MAINTAINABILITY - naming and structure, only where it would genuinely confuse the next reader.

For each finding: file and line, severity (blocking / should fix / nit), what is wrong, a concrete failing input or scenario, and the fix.

Rules:
- Do not report style issues a formatter would fix.
- Do not restate what the code does.
- If you find nothing blocking, say so plainly rather than inventing concerns.
- Mark anything you are unsure about as [UNVERIFIED] rather than asserting it.

Perform a security review of this code. Only report issues actually present in what I give you.

LANGUAGE/FRAMEWORK: [DETAIL]
WHERE THIS RUNS: [public endpoint / internal service / CLI / background job]
WHO CAN REACH IT: [unauthenticated / authenticated user / admin only / internal only]
WHAT DATA IT TOUCHES: [DESCRIPTION]

CODE:
```
[PASTE]
```

Check specifically:
- Input validation and injection (SQL, command, template, LDAP, NoSQL)
- Authentication and authorisation: is the check present, is it before the action, can it be bypassed by a parameter
- IDOR: can a user access another user's object by changing an ID?
- Secrets: hardcoded, logged, or returned in responses or errors
- Output encoding and XSS if anything reaches a browser
- SSRF if any URL comes from user input
- Path traversal if any filename comes from user input
- Unsafe deserialisation
- Rate limiting on anything expensive or authentication-related
- Error messages that leak internal detail
- Cryptography: weak algorithms, hardcoded IVs, ECB mode, non-constant-time comparison, insufficient randomness

For each finding: severity (critical/high/medium/low), the exact line, a concrete exploitation scenario, and the fix as a code change.

If a check appears to be handled elsewhere (middleware, a framework default), say so and mark it [VERIFY ELSEWHERE] rather than reporting it as a vulnerability.

End with the single highest-risk item and what to do about it first.

Review this code for concurrency problems.

LANGUAGE AND CONCURRENCY MODEL: [e.g. Go goroutines, Java threads, Node async, Python asyncio, Rust tokio]
HOW MANY CONCURRENT CALLERS: [EXPECTED CONCURRENCY]
SHARED STATE: [WHAT IS SHARED - DB ROWS, CACHE, IN-MEMORY MAPS, FILES]
DEPLOYMENT: [single instance / multiple instances / serverless]

CODE:
```
[PASTE]
```

Find:
1. RACE CONDITIONS - shared mutable state accessed without synchronisation. For each, give the exact interleaving that causes the bug, step by step.
2. CHECK-THEN-ACT - anywhere the code checks a condition then acts on it non-atomically (exists-then-create, read-then-update, balance-check-then-debit).
3. DEADLOCK - lock ordering, nested locks, locks held across I/O or await points.
4. LOST UPDATES - read-modify-write on a shared store without optimistic locking or a version check.
5. NON-IDEMPOTENT RETRIES - what breaks if this runs twice? Critical for queues, webhooks and serverless.
6. RESOURCE LEAKS - connections, file handles, goroutines or tasks never cleaned up on the error path.
7. MULTI-INSTANCE ASSUMPTIONS - in-memory state, local locks or local caches that break when there is more than one instance. Only flag if my deployment says multiple instances or serverless.

For each: the interleaving, the observable symptom in production, and the fix (with the specific primitive - mutex, transaction isolation level, unique constraint, optimistic version, idempotency key).

Prefer database constraints and idempotency keys over application locks where possible, and say why.

Rewrite my code review comment so it lands well.

MY DRAFT COMMENT:
"""
[PASTE]
"""

THE CODE IN QUESTION:
```
[PASTE]
```

SEVERITY: [blocking / should fix / suggestion / question]
RELATIONSHIP: [more senior than me / peer / more junior / external contributor]
HISTORY: [first PR from them / we disagree often / no context]

Rewrite it so that it:
- States the concern in the first sentence, not after context
- Describes the problem in terms of the code's behaviour, never the author
- Includes a concrete failing case or scenario where relevant
- Offers a specific alternative, or explicitly says you do not have one
- Marks the severity clearly (use a prefix like 'blocking:' or 'nit:')
- Leaves room for the author to know something you do not

Rules:
- No 'why didn't you', no 'you should have', no 'obviously', no 'simply'
- No compliment sandwich
- If it is a nit, say so and say it does not block
- If I am actually unsure, phrase it as a question and mean it

Also tell me: if my draft has an unstated assumption that might be wrong, name it. And if the comment is not worth making at all, say so.

This pull request is too large to review linearly. Triage it first.

WHAT IT IS MEANT TO DO: [INTENT]
LANGUAGE/STACK: [DETAIL]

DIFF:
```
[PASTE DIFF OR FILE LIST WITH CHANGE COUNTS]
```

Produce:

1. CHANGE INVENTORY - group every changed file into: core logic change, mechanical/refactor, generated, config, tests, docs, dependency bump. Give line counts per group.

2. WHERE TO SPEND REVIEW TIME - rank the files by risk. Risk is high where the change is logic-bearing, touches auth or money or data deletion, has no test coverage in this diff, or changes a shared interface. Give a suggested time budget per file.

3. SAFE TO SKIM - files where a careful look is not warranted, and why.

4. SPLIT RECOMMENDATION - could this PR be several? If yes, propose the split and the order to merge them. Say plainly if it should be sent back.

5. MISSING - what is not in this diff that should be: tests, migration, docs, feature flag, rollback path, monitoring.

6. THE THREE QUESTIONS to ask the author before reviewing the detail.

Do not review the code yet. This pass is triage only.

Check whether this new code follows the conventions of the existing codebase.

EXISTING CODE (representative examples):
```
[PASTE 2-3 EXISTING FILES OR EXCERPTS]
```

NEW CODE:
```
[PASTE]
```

Compare across:
1. ERROR HANDLING - how does existing code signal, wrap and log errors? Does the new code match?
2. NAMING - conventions for functions, variables, files, types, constants
3. STRUCTURE - layering, where logic lives, how modules depend on each other
4. CONFIGURATION - how existing code reads config, secrets and feature flags
5. LOGGING AND OBSERVABILITY - format, level, structured fields, what gets logged at which point
6. TESTING - test structure, naming, what gets mocked, fixture patterns
7. DEPENDENCY USE - does the new code introduce a library that duplicates one already in use?
8. ASYNC/CONCURRENCY PATTERNS - does it use the same approach as the rest of the codebase?

For each mismatch: what the existing code does, what the new code does, whether it matters (real inconsistency vs harmless variation), and the change to make.

Also flag: any place where the NEW code is better than the existing convention. Those are worth keeping and propagating, not reverting. Say which.

Do not flag formatting that a linter handles.

Review this database migration for production safety.

DATABASE: [Postgres/MySQL/other + version]
TABLE SIZE: [ROW COUNT AND ROUGH DATA SIZE]
TRAFFIC: [reads/writes per second during deploy window, or 'unknown']
DEPLOY MODEL: [rolling / blue-green / downtime window allowed]
ORM/MIGRATION TOOL: [DETAIL]

MIGRATION:
```
[PASTE]
```

APPLICATION CODE THAT USES THESE TABLES (if relevant):
```
[PASTE]
```

Check:
1. LOCKING - which statements take which locks, for how long, and what they block. Be specific to my database and version; lock behaviour differs.
2. TABLE REWRITE - does any statement rewrite the whole table? Adding a column with a default, changing a type, adding a NOT NULL constraint.
3. INDEX CREATION - is it concurrent/online? What happens if it fails partway?
4. BACKWARD COMPATIBILITY - during a rolling deploy, old and new application code run simultaneously. Does this migration break the old code? Walk through the deploy sequence.
5. ORDER - does this need to be split into expand/migrate/contract phases? If so, give the three migrations.
6. DATA MIGRATION - if it backfills, is it batched? What is the transaction size? Can it be resumed?
7. ROLLBACK - is it reversible? If not, say so plainly and describe what recovery looks like.
8. TIMEOUT RISK - will it exceed a statement or lock timeout at my stated table size?

End with: SAFE TO RUN IN PRODUCTION / NEEDS CHANGES / NEEDS DOWNTIME, and the reason.