# Review instructions

Used by local `/code-review` and by the local-LLM reviewer (`tools/ai-review/review_pr.php`).
A reviewer **comments; it never approves**. Approval is a human code owner in CodeCommit.

## Passes
1. **Bugs and logic**: wrong behaviour, unhandled errors, broken contracts between services.
2. **Security**: auth, injection, secrets, PII (see `.claude/skills/secure-api-review/SKILL.md`).
3. **Compliance**: the diff against `intent/<folder>/spec.md` and `plan.md`. Flag anything built
   that the plan didn't call for, and anything the plan called for that is missing.

## What Important means
It breaks behaviour, leaks data, breaches policy, or blocks a rollback. Everything else is a Nit.

## Limits
At most 5 Nits per review; summarise the rest as a count. Lead with Important findings.

## Do not report
- Anything under `vendor/`, `node_modules/`, `dist/`, `b2g_filems_fe/app/`
- `composer.lock`, `package-lock.json`, `*.lock`
- `buildTimestamp` / `buildNumber` churn in `environment.*.ts` from `update-version.js`
- Formatting and style that no tool enforces here

## Always check
- The PR description has **Explain-back** and **How to debug in production** sections
  (`docs/ai-sdlc/PULL_REQUEST_TEMPLATE.md`).
- The diff still matches `plan.md`.
- **Outbound curl** sets `CURLOPT_CONNECTTIMEOUT` or `CURLOPT_TIMEOUT`. Service Connect allows an
  inbound request 15s; an unbounded curl hangs about 133s and try/catch cannot save it.
- **PHP namespace case** matches the file path exactly. Windows/macOS hide this; Linux 500s.
- **Logging**: no Yii `FileTarget` pointed at `/proc/self/fd/2`. Use the StderrLogTarget component.
- **Frontends**: no absolute platform host (`blu2green.earth`, `192.168.*`, `172.16.*`,
  `localhost`) outside `environment.*.ts`; `environment.docker.ts` keeps `const KONG = ''`;
  `angular.json` keeps its `docker` configuration and `baseHref`.
- **Build output** is not committed (`dist/`, `app/`, `demo/`).
- **`package.json` and `package-lock.json`** change together, from the same side.
- **`ci/curl-timeout-baseline.txt`**: counts may only go down. A regenerated baseline is Important.
- **`docker/php-nginx/php.ini`**: `opcache.revalidate_freq` stays at `2`.
- **Data**: a `_fk` may point at a table in another service's database. Queries are scoped to the
  caller's company/user, with no cross-tenant reads.
- No PII (names, emails, phone numbers, tokens) in logs or error responses.
- No credentials, `uac=` tokens or secrets in source or config.