# Review Agent Rules

These rules apply when reviewing completed implementation steps on the `main` branch.

## Checklist Tracking

`FEATURES_CHECKLIST.md` uses three checkbox states:
- `[ ]` — pending and **required**; blocks the next phase
- `[x]` — done
- `[~]` — optional or deferred; **never blocks phase advancement**

Rules:
- The ✅ column belongs to the implementation agent; the ✔️ column is yours.
- Only review items whose ✅ box is already `[x]`. Do not attempt to review unimplemented items.
- If a ✅ box is `[~]` (optional, skipped), mark the ✔️ box `[~]` as well — no review needed for skipped items.
- After reviewing each required item and confirming it meets the quality bar below, mark its ✔️ box by changing `[ ]` to `[x]`.
- Before reviewing any item in a new phase, read `FEATURES_CHECKLIST.md` and confirm that every **required** item in all preceding phases has `[x]` in both ✅ and ✔️. Items where both columns are `[~]` do not need review and do not block advancement.
- If any required box in a previous phase is unchecked, stop and report which items are blocking progress instead of proceeding.

## Review Scope

- Review one phase at a time. Within a phase, review items in the order they appear in `FEATURES_CHECKLIST.md`.
- For each item, cross-reference the implementation against `FEATURES_PLAN.md` and the quality criteria below.
- Report concrete issues with file paths and line numbers. Do not flag style nitpicks that are not covered by a project guideline.

## What to Check

**Correctness**
- The behavior matches the intent described in `FEATURES_PLAN.md` and the checklist item.
- API contracts, endpoint shapes, and TypeScript types are compatible with existing callers.
- No regressions are introduced in previously working behavior.

**Tests**
- Tests exist for the new code and cover the main success path, edge cases, and failure behavior.
- Tests are not weakened or removed just to make the suite pass.
- External services (HAFAS, Nominatim, OSRM, geolocation, time, calendar downloads) are mocked; tests do not depend on live network availability.

**Quality**
- No compile errors, lint errors, runtime crashes, or broken imports.
- TypeScript strictness is intact — no `any` used as a shortcut.
- Server-only code is not imported into client components.
- Nominatim usage follows the project requirements: configurable base URL, clear user agent, rate-limit-aware caching, no direct browser calls.
- Error handling is explicit and user-facing failures are understandable.
- No generated artifacts, caches, logs, or local environment files are committed.
- Dependencies are unchanged unless necessary and justified.
- Package boundaries are respected — shared types in `packages/core`, API wrappers in `packages/api-client`, app code in `apps/web`.

**Scope**
- The change is scoped to the checklist item — no unrelated modifications.
- Existing implementation files were not removed unless parity is tested and cleanup was explicitly requested.

**Accessibility (UI items only)**
- Semantic buttons and links, labels for inputs, keyboard-operable controls, visible loading and error states.

## Verification

- Run the relevant test and build checks to confirm the implementation passes before marking ✔️:

```bash
npm run typecheck -w packages/core
npm run typecheck -w packages/api-client
npm run typecheck -w apps/web
npm run lint -w apps/web
npm run build -w apps/web
npm test
```

- If a check fails, do not mark the ✔️ box. Report the failure with the exact output and leave the item for the implementation agent to fix.

## Completion Checklist

Before marking a ✔️ box, confirm:

- The ✅ box for this item is already checked by the implementation agent.
- All preceding phase items have both ✅ and ✔️ checked.
- The implementation matches the intent in `FEATURES_PLAN.md`.
- Tests exist, are meaningful, and pass.
- Build and type checks pass.
- No quality issues from the criteria above remain unresolved.
- Any limitations or known gaps are reported clearly to the user.