add planning doc for improvements to simulation propagation

docs/TODO

@@ -18,6 +18,7 @@ If you see modifications to this file in git status, IGNORE them and do not comm
     - work on maneuver interpolation. see docs/planning/hohmann-rendezvous-quantization-fix.md
     - write out/fix inline code docs with the documentation generation script, and review
         - can then work on a better method for commenting/detecting changes via script
+    - simulation propagation refactor: see docs/planning/propagation_refactor.md
 
 
 

docs/planning/propagation_refactor.md

@@ -0,0 +1,164 @@
+# Propagation Call Chain Analysis
+
+## Session Date
+2026-04-20
+
+## Objective
+Audit all call sites of `propagate_orbital_elements()` for spacecraft and trace the `update_simulation()` call chain through `execute_pending_maneuvers()` and `update_spacecraft_physics()` to identify inefficiencies and confusing branching.
+
+## Call Chain: `update_simulation()`
+
+```
+update_simulation()
+│
+├── reset_spacecraft_tracking()
+│
+├── update_bodies_physics()
+│   └── for each body:
+│       ├── find_dominant_body()
+│       ├── orbital_elements_to_cartesian() → expected_vel
+│       ├── velocity drift check (every frame)
+│       ├── cartesian_to_orbital_elements() (if drift > 1e-6)
+│       └── propagate_orbital_elements()
+│
+├── compute_global_coordinates()
+│
+├── execute_pending_maneuvers()
+│   └── for each unexecuted maneuver:
+│       ├── check_maneuver_trigger()
+│       │   ├── TRIGGER_TIME: sim->time >= trigger_value
+│       │   └── TRIGGER_TRUE_ANOMALY:
+│       │       ├── propagate_orbital_elements() ← per-frame probe
+│       │       └── Kepler equation solving
+│       └── if triggered:
+│           ├── propagate_orbital_elements(dt_to_burn)
+│           ├── execute_maneuver()
+│           │   ├── apply_impulsive_burn()
+│           │   └── cartesian_to_orbital_elements()
+│           └── propagate_orbital_elements(remaining_dt)
+│
+├── update_spacecraft_physics()
+│   └── for each craft NOT handled:
+│       ├── orbital_elements_to_cartesian() → expected_vel
+│       ├── velocity drift check
+│       ├── cartesian_to_orbital_elements() (if drift)
+│       └── propagate_orbital_elements()
+│
+├── compute_spacecraft_globals()
+└── sim->time += sim->dt
+```
+
+## Issues Found
+
+### Issue 1: Redundant Propagation for True-Anomaly Triggers
+
+**Location:** `src/maneuver.cpp`, line 147 in `check_maneuver_trigger()`
+
+For every frame that a true-anomaly maneuver is pending, `propagate_orbital_elements()` is called as a "look-ahead" probe to determine if the target angle is approaching.
+
+```cpp
+// maneuver.cpp:147
+OrbitalElements future_elements = propagate_orbital_elements(craft->orbit, sim->dt, parent->mass);
+```
+
+**Impact:** For a Hohmann transfer with a ~244,000s wait time and DT=10s, this results in ~24,400 redundant propagations — each one solving Kepler's equation — before the maneuver even fires. The spacecraft's orbit state is not modified, but the computational cost is paid every single frame.
+
+### Issue 2: Mixed Concerns in `execute_pending_maneuvers()`
+
+**Location:** `src/simulation.cpp`, `execute_pending_maneuvers()`
+
+The function performs two distinct responsibilities:
+1. **Checking** trigger conditions (calls `check_maneuver_trigger()`)
+2. **Executing** the burn (propagation → burn → propagation)
+
+```cpp
+void execute_pending_maneuvers(SimulationState* sim) {
+    for (...) {
+        if (check_maneuver_trigger(maneuver, craft, sim)) {  // CHECKING
+            // ... propagation, burn, more propagation         // EXECUTING
+        }
+    }
+}
+```
+
+The trigger-checking logic (especially the true-anomaly branch with its Kepler equation solving) is interleaved with the execution logic. This makes it difficult to reason about what happens in each phase and complicates testing.
+
+### Issue 3: Ambiguous `scheduled_dt` Semantics
+
+The `scheduled_dt` field in `Maneuver` has different meanings depending on trigger type:
+
+| Trigger Type | `scheduled_dt` meaning | Set by |
+|-------------|----------------------|--------|
+| `TRIGGER_TIME` | Always 0.0 (never set) | Never |
+| `TRIGGER_TRUE_ANOMALY` | Seconds until exact burn position | `check_maneuver_trigger()` |
+
+In `execute_pending_maneuvers()`:
+
+```cpp
+double dt_to_burn = maneuver->scheduled_dt;
+if (dt_to_burn > 0.0) {
+    craft->orbit = propagate_orbital_elements(craft->orbit, dt_to_burn, ...);
+}
+// ... burn ...
+double remaining_dt = sim->dt - dt_to_burn;
+craft->orbit = propagate_orbital_elements(craft->orbit, remaining_dt, ...);
+```
+
+For `TRIGGER_TIME`, `scheduled_dt == 0` is a coincidence — the field is never set. The code works, but the reason is opaque. The branching (`if dt_to_burn > 0`) exists only for true-anomaly triggers, but a reader cannot tell this from the code alone.
+
+### Issue 4: Time-Triggered Burns Propagate from Wrong State
+
+For time triggers, the sequence is:
+
+```
+Frame N:  sim->time = 310.0  (trigger_value = 305.0)
+          check: 310 >= 305 → true
+          dt_to_burn = 0
+          remaining_dt = 10
+          propagate craft by 10s starting from sim->time=310
+```
+
+The craft's orbit state is at `sim->time=310`, not at the trigger time `305`. The burn fires 5s late and the post-burn propagation starts from the wrong orbital position. This is the root cause of the burn timing quantization problem documented in `docs/planning/hohmann-rendezvous-quantization-fix.md`.
+
+### Issue 5: Hardcoded Array Size
+
+**Location:** `src/simulation.cpp`, static declaration
+
+```cpp
+static bool spacecraft_handled_this_frame[256];
+```
+
+A global array with a hardcoded size of 256. If more than 256 spacecraft are added, the tracking silently truncates. It should be part of `SimulationState` or dynamically sized. There is already a FIXME comment on this line.
+
+### Issue 6: Duplicated Propagation Logic
+
+The "normal" propagation path in `update_spacecraft_physics()` and the maneuver path in `execute_pending_maneuvers()` are essentially duplicates:
+
+**Normal path** (`update_spacecraft_physics`):
+```cpp
+propagate_orbital_elements(craft->orbit, sim->dt, ...);
+orbital_elements_to_cartesian(...);
+```
+
+**Maneuver path** (`execute_pending_maneuvers`, when `dt_to_burn == 0`):
+```cpp
+propagate_orbital_elements(craft->orbit, 0, ...);     // no-op
+orbital_elements_to_cartesian(...);
+execute_maneuver(...);
+propagate_orbital_elements(craft->orbit, sim->dt, ...);
+orbital_elements_to_cartesian(...);
+```
+
+When no sub-step is needed, the maneuver path is: propagate → convert → burn → propagate → convert. The propagation and coordinate conversion logic is duplicated across both code paths.
+
+## Call Sites Summary
+
+| Location | Function | Context | dt value |
+|----------|----------|---------|----------|
+| `simulation.cpp:287` | `update_bodies_physics()` | Normal body propagation | `sim->dt` |
+| `simulation.cpp:315` | `update_spacecraft_physics()` | Normal craft propagation | `sim->dt` |
+| `simulation.cpp:343` | `execute_pending_maneuvers()` | Pre-burn sub-step propagation | `dt_to_burn` (0 to `sim->dt`) |
+| `simulation.cpp:350` | `execute_pending_maneuvers()` | Post-burn remaining propagation | `sim->dt - dt_to_burn` |
+| `maneuver.cpp:147` | `check_maneuver_trigger()` | True-anomaly look-ahead probe | `sim->dt` |
+
+Total: 5 call sites, 3 distinct contexts (normal propagation, sub-step execution, probe).