3.8 KiB
3.8 KiB
Review Agent Rules
These rules apply when reviewing completed implementation steps on the rewrite/next branch.
Checklist Tracking
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 rewrite 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
CHECKLIST.mdand 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
CHECKLIST.md. - For each item, cross-reference the implementation against
REWRITE_PLAN.mdand 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
REWRITE_PLAN.mdand 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
anyused 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.
Scope
- The change is scoped to the checklist item — no unrelated modifications.
- Old implementation files (
server/,oebb-planner-app/,oebb-planner.jsx) 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 ✔️:
npm test
npm run build
npm run typecheck
npm run lint
- If a check fails, do not mark the ✔️ box. Report the failure with the exact output and leave the item for the rewrite agent to fix.
Completion Checklist
Before marking a ✔️ box, confirm:
- The ✅ box for this item is already checked by the rewrite agent.
- All preceding phase items have both ✅ and ✔️ checked.
- The implementation matches the intent in
REWRITE_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.