NEON-25: add code review for isometric follow camera

docs/reviews/2026-04-07-NEON-25.md

@@ -0,0 +1,43 @@
+# Code review: NEON-25 (isometric follow camera)
+
+**Date:** 2026-04-07  
+**Scope:** Branch `NEON-25-isometric-follow-camera` vs `main`; issue [NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25). Commits on branch include implementation plan, locked defaults, **NEON-29** backlog row in `E1_M2_IsometricCameraController.md`, and camera implementation + tests + docs. Manual testing reported OK by author.  
+**Base:** `main` (compare: `main...NEON-25-isometric-follow-camera`).
+
+## Verdict
+
+**Approve with nits**
+
+## Summary
+
+The change replaces a static `Camera3D` with an `IsometricFollowCamera` rig that follows the player using a documented spherical offset (`desired_eye_world`), exponential smoothing, teleport snap beyond `snap_distance`, and per-frame `look_at` toward a configurable focus. A `RefCounted` `CameraState` is owned by the rig and refreshed each `_process`, with orbit yaw held at zero while `allow_yaw` is false—matching prototype policy. `main.gd` stays a thin composer; `ground_pick` still receives the active `Camera3D`. Automated tests cover `CameraState` fields and the static eye-position helper; manual verification (movement, bumps, picking) is appropriate per plan. No server or authority surface is added. Risk is low: presentation-only, well-scoped, aligns with E1.M2 and the implementation plan.
+
+## Documentation checked
+
+| Document | Result |
+|----------|--------|
+| `docs/plans/NEON-25-implementation-plan.md` | **Matches** — locked defaults (`_process`, damped follow + snap, `NodePath` follow, per-frame `CameraState`), file list, tests, and README/module snapshot are reflected in the diff. |
+| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — implementation snapshot documents rig scripts, yaw seam, single-band distance; Jira table includes NEON-29 (separate ticket; see nits). |
+| `docs/decomposition/modules/client_server_authority.md` (E1.M2) | **Matches** — camera and `CameraState` remain client-local; no server use of camera pose. |
+| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E1.M2 row still **Planned** with full contract set; NEON-25 delivers a slice (follow + `CameraState` seam), not zoom/occlusion. Updating register or module **Status** after merge is optional hygiene, not required for this PR’s correctness. |
+| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **N/A** — no E1.M2-specific row found; no conflict. |
+
+## Blocking issues
+
+None.
+
+## Suggestions
+
+1. **Null follow target:** If `follow_target_path` breaks or resolves empty, `_process` returns without updating `_smoothed_eye` or syncing state—acceptable for the current scene, but a one-line comment or dev warning could save a future scene refactor.
+2. **Register / status (optional):** After merge, consider whether `module_dependency_register.md` or the E1.M2 summary **Status** should reflect partial delivery (e.g. follow baseline landed; zoom/occlusion still open)—only if the team tracks that table at story granularity.
+
+## Nits
+
+- **Nit:** `camera_state.gd` header references `[member presentation_yaw_deg]`; that property lives on `isometric_follow_camera.gd`, not on `CameraState`—minor doc confusion for Godot help tooltips.
+- **Nit:** Same branch carries **NEON-29** (prototype district) backlog registration; harmless, but reviewers merging a “NEON-25 only” PR should expect that small doc delta.
+- **Nit:** Plan wording “set state yaw to `0` when `allow_yaw` is false” is satisfied via `_orbit_yaw_rad = 0` and `_state.yaw = _orbit_yaw_rad`; presentation yaw remains in exports—consistent with module intent but worth remembering for readers who expect a single “total yaw” on state.
+
+## Verification
+
+- **Automated:** From `client/`, run GdUnit headless per `client/README.md` (Godot 4.6 import + `GdUnitCmdTool` on `res://test`).
+- **Manual:** (Author confirmed OK.) Re-run: server + F5, click-move floor and NEON-7 bumps, ground pick from moving camera, idle observation; optional inspector tweak of `follow_smoothness` / `snap_distance` if feel regresses on different hardware.