rewrite phase 3
This commit is contained in:
@@ -0,0 +1,77 @@
|
||||
# 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.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 `CHECKLIST.md`.
|
||||
- For each item, cross-reference the implementation against `REWRITE_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 `REWRITE_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.
|
||||
|
||||
**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 ✔️:
|
||||
|
||||
```bash
|
||||
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.
|
||||
@@ -0,0 +1,114 @@
|
||||
# Rewrite Agent Rules
|
||||
|
||||
These rules apply when implementing the Next.js rewrite 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 is yours; the ✔️ column belongs to the review agent.
|
||||
- After completing each numbered item, mark its ✅ box by changing `[ ]` to `[x]`.
|
||||
- If you complete an optional item (`[~]`), change it to `[x]`. If you skip it, leave it as `[~]`.
|
||||
- Before starting any item in a new phase, read `CHECKLIST.md` and confirm that every **required** (`[ ]`/`[x]`) item in all preceding phases has `[x]` in both ✅ and ✔️. Items marked `[~]` in both columns do not need to be completed first.
|
||||
- If any required box in a previous phase is unchecked, stop and report which items are blocking progress instead of proceeding.
|
||||
|
||||
## Rewrite Context
|
||||
|
||||
- `REWRITE_PLAN.md` is the guiding plan for the migration from the current CRA + Express app to a Next.js App Router + TypeScript app.
|
||||
- When working on the rewrite, follow the migration phases in `REWRITE_PLAN.md` unless the user explicitly asks for a different order.
|
||||
- Treat each numbered migration item as a checkpoint: implement it, update its ✅ box in `CHECKLIST.md`, add or update tests, run the relevant verification, then continue.
|
||||
- Prefer building the new Next.js structure in parallel until feature parity is proven. Do not delete `server/`, `oebb-planner-app/`, or `oebb-planner.jsx` before equivalent Next.js behavior is implemented, tested, and the user has clearly asked for cleanup.
|
||||
- Preserve existing API contracts and user-visible behavior during migration unless the rewrite plan or user request explicitly changes them.
|
||||
- Use `npm` consistently because the existing project uses `package-lock.json`.
|
||||
|
||||
## Work Step By Step
|
||||
|
||||
- Start by reading the relevant files and identifying the smallest safe next step.
|
||||
- State the plan before making non-trivial changes.
|
||||
- Implement one coherent change at a time.
|
||||
- After each step, review the diff and check whether it still matches the intended behavior.
|
||||
- Do not move on to the next step while the current step has unresolved compile errors, failing tests, or obvious regressions.
|
||||
- Prefer small, targeted edits over broad rewrites.
|
||||
- Preserve existing behavior unless the user explicitly asks to change it.
|
||||
- When a task spans multiple rewrite phases, complete one vertical slice at a time where practical: type or library code, route or hook, UI integration, tests, then verification.
|
||||
- Keep reusable logic in `src/lib`, side effects in hooks or route handlers, and shared contracts in `src/types`.
|
||||
|
||||
## Testing Requirements
|
||||
|
||||
- Add or update tests for every new feature, bug fix, and behavior change.
|
||||
- Put tests near the code they cover and follow the existing test style.
|
||||
- Cover the main success path, important edge cases, and failure behavior.
|
||||
- Do not remove or weaken tests just to make the suite pass.
|
||||
- If a change cannot reasonably be tested, explain why and add the closest practical verification.
|
||||
- For the Next.js rewrite, prefer unit tests for `src/lib`, route tests for `src/app/api`, and component smoke or behavior tests for UI components.
|
||||
- Mock external services in automated tests, including ÖBB HAFAS, Nominatim, OSRM, geolocation, time, and calendar downloads. Do not make tests depend on live network availability.
|
||||
- Test TypeScript data shapes and boundary parsing where API responses are transformed into app types.
|
||||
|
||||
## Verification Before Moving On
|
||||
|
||||
- Run the narrowest relevant tests after each meaningful change.
|
||||
- Run the broader project checks before finishing.
|
||||
- For server changes, run:
|
||||
|
||||
```bash
|
||||
cd server
|
||||
npm test
|
||||
```
|
||||
|
||||
- For React app changes, run:
|
||||
|
||||
```bash
|
||||
cd oebb-planner-app
|
||||
CI=true npm test -- --watchAll=false
|
||||
npm run build
|
||||
```
|
||||
|
||||
- For the Next.js rewrite, once the root Next.js project exists, run the relevant root checks instead:
|
||||
|
||||
```bash
|
||||
npm test
|
||||
npm run build
|
||||
```
|
||||
|
||||
- If available, also run type-checking and linting scripts before finishing:
|
||||
|
||||
```bash
|
||||
npm run typecheck
|
||||
npm run lint
|
||||
```
|
||||
|
||||
- If a change touches both server and app behavior, run both sets of checks.
|
||||
- If a command fails, stop, inspect the failure, fix the cause, and rerun the command.
|
||||
- Do not claim the work is complete until the relevant checks pass, or until the remaining blocker is clearly reported.
|
||||
|
||||
## Quality Bar
|
||||
|
||||
- Make sure additions do not introduce compile errors, lint errors, runtime crashes, or broken imports.
|
||||
- Check that public APIs, endpoint contracts, props, and data shapes remain compatible with existing callers.
|
||||
- Keep error handling explicit and user-facing failures understandable.
|
||||
- Avoid hidden global state, timing assumptions, and network-dependent tests unless the project already uses that pattern.
|
||||
- Keep dependencies unchanged unless they are necessary for the task and justified.
|
||||
- Do not commit generated artifacts, caches, logs, or local environment files.
|
||||
- Keep TypeScript strictness intact once introduced. Do not use `any` as a shortcut around unclear domain types.
|
||||
- Keep server-only code out of client components. Route handlers and `src/lib` clients that use secrets, privileged headers, or upstream service details must not be imported into browser-only code.
|
||||
- Respect Nominatim usage requirements when implementing geocoding: configurable base URL, clear user agent, rate-limit-aware caching, and no direct browser calls to the public service.
|
||||
- Keep OSRM and HAFAS clients behind API routes or server-side utilities so failures can be normalized and tested.
|
||||
- For UI work, preserve accessibility basics: semantic buttons and links, labels for inputs, keyboard-operable controls, visible loading and error states.
|
||||
|
||||
## Completion Checklist
|
||||
|
||||
Before finishing a step, confirm:
|
||||
|
||||
- The requested behavior is implemented.
|
||||
- The ✅ box for the corresponding item in `CHECKLIST.md` is checked.
|
||||
- The change matches the relevant phase or numbered item in `REWRITE_PLAN.md`, when applicable.
|
||||
- Tests were added or updated where appropriate.
|
||||
- Relevant tests and build checks pass.
|
||||
- The change is scoped to the request.
|
||||
- No unrelated user changes were overwritten.
|
||||
- Old implementation files were not removed unless parity is tested and cleanup was requested.
|
||||
- Any limitations or skipped checks are reported clearly.
|
||||
Reference in New Issue
Block a user