[AZ-654] [AZ-655] [AZ-656] gimbal_controller primitives + monotonic clock fix (batch 11)

AZ-654 SweepEngine: pendulum default, Raster/LawnMower variants
reserved and explicitly NotImplemented (no silent fallback per AC-3).
Time injected via next_step(now) for deterministic dwell tests.

AZ-655 PlanExecutor: linear yaw/pitch interpolation between PanGoals
with self-throttle (default 50 ms); stats expose
commands_emitted/dropped_to_throttle counters. PanGoal/PanPlan added
to shared::models::gimbal (spec drift: data_model.md §PanPlan flagged
for next doc sync).

AZ-656 CentreOnTarget: zoom-aware proportional control loop (correction
~ 1/zoom); target_lost debounced — fires once per loss streak, resets
on bbox return. Also fixes the misleadingly-named monotonic_ns() helper
introduced by AZ-653 that used SystemTime::now(): GimbalController now
owns a shared::clock::MonoClock and stamps GimbalState::ts_monotonic_ns
via clock.elapsed_ns(). AZ-656 AC-2 forced the correction; integration
test verifies the fix end-to-end.

58/58 gimbal_controller tests green (47 unit + 7 AZ-653 integration +
4 new batch_11 integration). Workspace test suite green this run.

Co-authored-by: Cursor <cursoragent@cursor.com>

_docs/03_implementation/batch_11_cycle1_report.md

@@ -0,0 +1,175 @@
+# Batch 11 (cycle 1) implementation report
+
+**Tasks**: AZ-654, AZ-655, AZ-656
+**Component scope**: `gimbal_controller` (+ shared::models::gimbal type extension)
+**Verdict**: PASS_WITH_WARNINGS — proceed; flagged items below.
+
+## Tasks
+
+### AZ-654 gimbal_zoom_out_sweep — pendulum default + reserved Raster/LawnMower
+
+**Outcome**: `SweepPattern` enum with all three variants declared; `Pendulum` implemented; `Raster` / `LawnMower` reserved and return `AutopilotError::NotImplemented` rather than silently falling back (per AC-3). Sweep envelope (yaw bounds, dwell, step) is configured via `SweepConfig`; the engine validates it on construction.
+
+**Production code added**:
+
+- `crates/gimbal_controller/src/internal/sweep.rs`
+  - `SweepPattern` enum — `#[derive(Default)]` with `Pendulum` as the default variant. Public re-export from `lib.rs`.
+  - `SweepConfig { min_yaw_deg, max_yaw_deg, pitch_deg, step_deg, dwell: Duration }` — validated on construction (bounds ordered, step > 0). Public re-export.
+  - `SweepEngine { pattern, config, current_yaw, direction, dwell_started_at: Option<Instant> }` — state machine. Public re-export.
+  - `SweepEngine::next_step(now: Instant) -> Result<GimbalCommand>` — pendulum path advances by `step_deg`, clamps at bounds, starts a dwell window, then reverses on next tick after `config.dwell`; Raster / LawnMower paths return `NotImplemented`. Time is injected (not `Instant::now()` internally) for deterministic tests.
+- `crates/gimbal_controller/src/lib.rs` (re-export edit)
+
+**Tests** (10 total, all green):
+- `ac1_pendulum_stays_within_bounds_over_100_steps` — verifies bound clamping + at least one reversal over 100 ticks at 1-second cadence.
+- `ac2_dwell_holds_yaw_at_bound` — verifies the yaw stays pinned for the entire 500 ms dwell window then flips on the first call after the window elapses.
+- `ac3_raster_returns_not_implemented` / `ac3_lawnmower_returns_not_implemented` — verifies the reserved-variant guarantee.
+- `pattern_default_is_pendulum` — verifies `SweepPattern::default()` == `Pendulum`.
+- `invalid_config_rejected` (yaw bounds invalid) and `invalid_step_rejected` (step ≤ 0) — verify `SweepConfig::validate`.
+- `pendulum_advances_in_step_increments_then_clamps` — verifies the forward sweep yaw sequence `-25, -20, ..., 30` and clamping behaviour.
+- Integration: `az654_sweep_engine_emits_gimbal_commands_within_bounds` — 200 ticks through the public API, verifies emitted `GimbalCommand` always inside `[min_yaw, max_yaw]` and the pitch is fixed at the configured value.
+
+### AZ-655 gimbal_smooth_pan_plan — path-tracking plan executor
+
+**Outcome**: `PlanExecutor` accepts a `PanPlan` (new shared type), linearly interpolates `(yaw, pitch)` between adjacent `PanGoal`s, and self-throttles to a configured min interval (default 50 ms). Past-end queries clamp to the final goal. Before-start queries extrapolate linearly from the first two goals. Stats track `commands_emitted_total` and `commands_dropped_to_throttle_total`.
+
+**Production code added**:
+
+- `crates/shared/src/models/gimbal.rs` — extended with `PanGoal { yaw_deg, pitch_deg, zoom, at_ns }` and `PanPlan { goals: Vec<PanGoal> }`. **Spec drift note**: AZ-655 references `data_model.md §PanPlan`, but `data_model.md` does not yet have a PanPlan entry — the document sync run will catch up (logged below under "Spec drift").
+- `crates/gimbal_controller/src/internal/smooth_pan.rs`
+  - `DEFAULT_MIN_CMD_INTERVAL = 50 ms` constant. Public re-export.
+  - `NextStep { Emit(GimbalCommand), Throttled }` — `Throttled` is an explicit state, never silent.
+  - `ExecutorStats { plan_loaded_at, commands_emitted_total, commands_dropped_to_throttle_total }` — public read-side.
+  - `PlanExecutor::new(min_cmd_interval)`, `with_default_throttle()`, `load(plan, now)`, `next_step(now)`, `stats()`, `has_plan()`.
+  - `validate_plan` rejects empty plans and non-strictly-increasing `at_ns`.
+  - `interpolate` + `linear_at` + `lerp` — pure helpers, no I/O.
+- `crates/gimbal_controller/src/lib.rs` (re-export edit)
+
+**Tests** (8 total, all green):
+- `ac1_linear_interp_midpoint` — plan 0°→30° over 1s, query at 500 ms → yaw ≈ 15°.
+- `ac2_throttle_drops_intermediate_calls` — 100 ticks at 10 ms cadence over 1s with 100 ms throttle → ~10 emissions, the rest counted as throttled. Stats counter cross-checked.
+- `ac3_past_plan_end_clamps_to_last_goal` — query 5 s after a 1 s plan → returns the last goal's values exactly.
+- `empty_plan_rejected` / `non_monotonic_plan_rejected` / `no_plan_returns_error` — `Validation` error paths.
+- `reload_clears_throttle_anchor` — re-loading the plan does not carry the previous plan's last_emit_at forward.
+- `single_goal_plan_holds_value` — degenerate 1-goal plan returns the goal verbatim for any query time.
+- Integration: `az655_plan_executor_emits_and_throttles_against_real_clock` — exercises the public API with a 20 ms throttle over 500 ms (100 ticks at 5 ms cadence) → ~25 emissions; verifies `ExecutorStats` counters match the emission ratio.
+
+### AZ-656 gimbal_centre_on_target — proportional centre-25% loop + monotonic-stamped state publish + target_lost debounce
+
+**Outcome**: `CentreOnTarget::tick(bbox, yaw, pitch, zoom) -> CentreOnTargetOutput` runs a proportional control loop that drags the target bbox toward the centre 25 % of the frame. The control law scales the yaw correction by `1 / zoom` because the effective FOV shrinks with zoom. `target_lost` debounces — fires exactly once on the tick that crosses the `max_missed_ticks` threshold, then stays silent for the rest of the loss streak; a visible bbox resets the counter so a subsequent loss streak can re-fire.
+
+**Production code added**:
+
+- `crates/gimbal_controller/src/internal/centre_on_target.rs`
+  - `DEFAULT_TARGET_GAIN = 0.6`, `DEFAULT_CENTRE_WINDOW = 0.25`, `DEFAULT_MAX_MISSED_TICKS = 3`. Public re-exports.
+  - `CentreOnTargetConfig { fov_deg_at_zoom1, gain, centre_half_width, max_missed_ticks }` with sensible `Default` impl.
+  - `CentreOnTargetOutput { command: Option<GimbalCommand>, target_lost_signal: bool, on_target: bool }`.
+  - `CentreOnTarget { config, consecutive_missed, in_loss_state }` — pure controller, no I/O.
+  - `CentreOnTarget::tick(bbox, yaw, pitch, zoom)` — when `bbox.is_none()`, increments the missed counter (capped at `max_missed_ticks` to avoid the unbounded growth → re-fire bug); when `bbox.is_some()`, resets the counter, computes `(err_x, err_y) = bbox_centre - 0.5`, scales by `fov / zoom * gain`, emits the corrective command, and sets `on_target` iff both errors are inside `centre_half_width`.
+- `crates/gimbal_controller/src/lib.rs`
+  - **Bug fix** carried in this batch: replaced the misleadingly-named `monotonic_ns()` (which used `SystemTime::now()` and was NOT monotonic) with `shared::clock::MonoClock`. The `GimbalController` and `GimbalControllerHandle` now own a `MonoClock` and stamp `GimbalState::ts_monotonic_ns` via `self.clock.elapsed_ns()`. This bug was introduced by AZ-653 (batch 10) and would have surfaced as observable wall-clock jumps on the consumer side (`movement_detector` ego-motion sync, `frame_ingest` telemetry tagging). AZ-656's AC-2 forced the issue.
+
+**Tests** (6 unit + 1 integration = 7 total, all green):
+- `ac1_centre_25pct_within_3_ticks` — runs the closed loop against a linearised kinematic camera model (the same one the proportional controller assumes); starting bbox at `(0.75, 0.55, 0.1, 0.1)` is inside the centre 25 % region by tick 3; `on_target` flag is observed at some point in the run.
+- `ac3_target_lost_emits_once_per_loss_streak` — verifies: no signal at tick 1/2; signal at tick 3; silent at tick 4/5; bbox returns → counter resets; second loss streak signals again at the new threshold.
+- `bbox_already_centred_marks_on_target_with_small_command` — bbox centred at (0.5, 0.5) → `on_target = true` and `delta_yaw`/`delta_pitch` ≈ 0.
+- `higher_zoom_yields_smaller_correction` — same bbox at 1× vs 4× → 4× correction is exactly 1/4 of 1× correction (within float tolerance).
+- `loss_counter_caps_safely_without_overflow` — hammer `tick(None)` 10 000 times; signal fires exactly once.
+- `loss_streak_below_threshold_then_recovery_does_not_signal` — `max_missed_ticks=5`, 3 missed then recovery → no signal.
+- Integration: `az656_set_pose_publishes_monotonic_timestamp` — verifies the AZ-653 → AZ-656 bug fix by issuing 3 sequential `set_pose` calls through the full transport stack (with a fake A40 echo loop) and checking `state_rx.borrow().ts_monotonic_ns` is strictly monotonic across the three observations.
+- Integration: `az656_centre_on_target_loop_converges_via_public_api` — duplicates the AC-1 convergence assertion using only `gimbal_controller` re-exports + `shared::models::frame::BoundingBox` (catches re-export drift).
+
+## AC coverage
+
+| Task | AC | Behaviour | Test | Status |
+|------|----|-----------|------|--------|
+| AZ-654 | AC-1 | 100-step pendulum stays in bounds, reverses at each bound | `ac1_pendulum_stays_within_bounds_over_100_steps` | PASS |
+| AZ-654 | AC-2 | Yaw pinned for full 500 ms dwell window | `ac2_dwell_holds_yaw_at_bound` | PASS |
+| AZ-654 | AC-3 | Raster + LawnMower variants return `NotImplemented` | `ac3_raster_returns_not_implemented`, `ac3_lawnmower_returns_not_implemented` | PASS |
+| AZ-655 | AC-1 | yaw 0→30° at t=500ms → 15° ± epsilon | `ac1_linear_interp_midpoint` | PASS |
+| AZ-655 | AC-2 | 100 ticks at 10 ms with 100 ms throttle → ~10 emits | `ac2_throttle_drops_intermediate_calls` | PASS |
+| AZ-655 | AC-3 | Plan past end clamps to last goal | `ac3_past_plan_end_clamps_to_last_goal` | PASS |
+| AZ-656 | AC-1 | bbox (0.75, 0.55) → centre 25 % within 3 ticks at 100 ms | `ac1_centre_25pct_within_3_ticks` + integration | PASS |
+| AZ-656 | AC-2 | `ts_monotonic_ns` strictly monotonic across `set_pose` calls | `az656_set_pose_publishes_monotonic_timestamp` | PASS |
+| AZ-656 | AC-3 | 3 consecutive missing bboxes → exactly one `target_lost`; later misses silent; bbox return → counter resets | `ac3_target_lost_emits_once_per_loss_streak` | PASS |
+
+## Code review
+
+**Spec compliance**: PASS. Every AC has a directly-named test that asserts the claimed behaviour.
+
+**Architecture compliance**: PASS.
+- `gimbal_controller` (Layer 2 Actor) imports only `shared` and standard / `tokio` deps. No sibling Layer 2 imports.
+- `internal/sweep.rs`, `internal/smooth_pan.rs`, `internal/centre_on_target.rs` are new internal files under the component's owned glob (`crates/gimbal_controller/**`).
+- `internal/smooth_pan.rs` was named in `module-layout.md` (line 113). `internal/sweep.rs` and `internal/centre_on_target.rs` are new and not yet enumerated — same drift-flag pattern recorded for `internal/transport.rs` in batch 10's report. Recommended: extend the gimbal_controller Internal bullet list on the next document refresh.
+- `shared::models::gimbal::PanPlan` / `PanGoal` are new and not yet documented in `data_model.md`. Same drift pattern.
+
+**SRP**: PASS.
+- `sweep::SweepEngine` — only sweep state machine + pendulum kinematics.
+- `smooth_pan::PlanExecutor` — only plan loading + interpolation + throttle.
+- `centre_on_target::CentreOnTarget` — only the proportional control law + loss-debounce state.
+- `lib.rs` — composition + transport bridging; primitives are unaware of the transport.
+
+**Runtime completeness**: PASS.
+- Real pendulum sweep (deterministic bounded state machine; not a random walk).
+- Real linear interpolation + real self-throttle (counter-validated).
+- Real proportional control loop with zoom-aware correction scaling; closed-loop convergence verified in unit + integration tests.
+- Real `MonoClock`-driven monotonic timestamps (fixes the AZ-653 wall-clock regression).
+
+**Test discipline**: PASS. AAA pattern with `// Arrange / Act / Assert` comments. No `unsafe`. No production `unwrap`/`expect`. Integration tests use the public re-export surface — never reach into `internal::*`.
+
+**Security quick-scan**: PASS. No external input deserialization in this batch (the primitives consume typed structs supplied by in-process callers); no string-interpolated commands; no network code added (re-uses the existing AZ-653 transport).
+
+**Performance scan**: PASS.
+- `SweepEngine::next_step` — three float ops + one struct construct, p99 far below the 1 ms target.
+- `PlanExecutor::next_step` — one binary-window walk over `goals` (typically a handful of waypoints) + lerp; p99 far below the 1 ms target.
+- `CentreOnTarget::tick` — six float ops; p99 far below the 2 ms target.
+
+**Cross-task consistency**: PASS.
+- All three primitives produce `GimbalCommand` (same `(yaw_deg, pitch_deg)` shape AZ-653 wired into `set_pose`).
+- Time injection pattern (`next_step(now: Instant)`) is consistent across `sweep`, `smooth_pan`, and `centre_on_target` (the latter receives state via args rather than time since its loop is per-frame, not per-clock-tick).
+- `AutopilotError::Validation(String)` is used consistently across the three primitives' invalid-input paths.
+
+## Spec drift (carried into the implementation, flagged here)
+
+1. **`data_model.md §PanPlan` is referenced by AZ-655 but does not exist.** The implementation adds `PanGoal` + `PanPlan` to `crates/shared/src/models/gimbal.rs` (the canonical location for gimbal-related shared types). The next document-sync run should backfill the entry in `data_model.md §4 Action / piloting entities`.
+2. **`module-layout.md` does not yet list `internal/sweep.rs`, `internal/centre_on_target.rs`, or `internal/transport.rs` (the last carried over from batch 10).** Same drift-flag pattern. Next document refresh should enumerate them.
+3. **The `monotonic_ns()` helper introduced by AZ-653 (batch 10) was misleadingly named and actually used `SystemTime::now()`.** AZ-656's AC-2 forced the correction in this batch. The fix is to construct a `shared::clock::MonoClock` per-controller and read `clock.elapsed_ns()` on every state stamp. This is recorded here rather than as a remediation task because the change was already in this batch's scope (the same files were being edited).
+
+## Known limitations (warnings)
+
+1. **`SweepEngine::next_pendulum` carries a one-tick dwell-flip lag.** When the dwell elapses, the next call returns the same `current_yaw` (the boundary value) one more time before the direction flips and the next call moves off the boundary. This is observable in `ac2_dwell_holds_yaw_at_bound`: the call at `+501 ms` is technically the "flip tick" (direction flip happens inside the call); the actual movement off the boundary happens on the call after that. Documented in the function body; benign in production where the tick cadence is at most 10 Hz.
+2. **`CentreOnTarget` assumes a linearised camera model.** Real ViewPro A40 mechanics introduce a small dead-zone and finite slew rate. The proportional gain (`DEFAULT_TARGET_GAIN = 0.6`) is conservative enough that the loop should converge under sub-critical conditions, but flight data may motivate adding a PD term or per-axis gains. Tracked as future tuning work; not blocking.
+3. **`PlanExecutor`'s `at_ns` is plan-relative, anchored to `loaded_at`.** This means re-loading the same plan re-anchors its timeline. If a future caller needs to seamlessly continue a plan across reloads (e.g., chunked path-following), the API needs a `replace_extend(plan)` variant. Not in scope for AZ-655.
+4. **No reserved `Idle` / mode-arbitration logic between the three primitives.** Today the composition root must hold at most one primitive active at a time per gimbal; a future refactor may introduce the `Sweep | PanPlan | CentreOnTarget | Idle` mode enum named in `description.md §5`. Not in scope for AZ-654/655/656.
+
+## Auto-fix attempts during the batch
+
+- `clippy::derivable_impls` on `impl Default for SweepPattern` → replaced with `#[derive(Default)]` + `#[default]` attribute on `Pendulum`.
+- `Debug` derive missing on `SweepEngine` (required by `Result::unwrap_err` in the validation tests) → added.
+- `AutopilotError::Validation(&'static str)` doesn't compile (variant requires `String`) → switched the no-plan-loaded path to `ok_or_else` with a `.into()`. The earlier batch's `unnecessary_lazy_evaluations` warning does NOT apply here because `String::from(&'static str)` is not a no-op call; clippy correctly leaves this one alone.
+
+## Test reproduction
+
+```
+cargo build -p gimbal_controller --tests
+cargo test  -p gimbal_controller                  # 58 tests; 0 failed
+cargo clippy -p gimbal_controller --tests -- -D warnings
+cargo test --workspace                            # all green
+```
+
+This run's workspace test suite passed without flakes; the
+`mission_executor::state_machine::ac3_bounded_retry_then_success` flake
+noted in batch 10's report did not reproduce in this run.
+
+## Cumulative review trigger
+
+Cumulative review fires every 3 completed batches (batches 7-9, 10-12, 13-15, …). After batch 11 we are 2/3 of the way to the next cumulative; batch 12 will trigger it.
+
+## Candidates for batch 12
+
+Remaining `todo/`: AZ-657 (frame_ingest x3 chain), AZ-660/661 (detection_client x2), AZ-662–664 (movement_detector x3), AZ-669–671 (semantic_analyzer x3), AZ-675–677 (telemetry_stream x3), AZ-678–681 (operator_bridge x4), AZ-682–686 (scan_controller x5).
+
+Only-AZ-640-dep (ready immediately):
+- **AZ-657** `frame_ingest_rtsp_session` — 3 pts. Standard RTSP, no vendor-spec gap.
+- **AZ-682** `scan_controller_state_machine` — 5 pts. Deps `AZ-640, AZ-649` (both done). The primitives shipped in this batch (sweep / pan-plan / centre-on-target) are the natural building blocks scan_controller will compose.
+
+Recommended: **AZ-657 + AZ-682** (3 + 5 = 8 pts, 2 tasks, two different components). Gives the next cumulative review (batch 12, batches 10-12) surface area across `gimbal_controller`, `frame_ingest`, and `scan_controller` for a stronger cross-task check. Alternative: AZ-657 alone (3 pts) if the user prefers a single-task batch to keep batch 12 tight.