# Plan: Add Molniya Orbit Test Case

## Overview
Add test cases for highly inclined orbits, specifically focusing on Molniya orbits and generic inclined orbit behavior.

## Background - Molniya Orbit Properties
From Wikipedia research, Molniya orbits are designed for high-latitude coverage:
- **Orbital Period**: ~718 minutes (~12 hours, half a sidereal day)
- **Eccentricity**: ~0.74
- **Inclination**: 63.4° (critical value that prevents perigee precession)
- **Argument of Perigee**: 270° (apogee at northernmost point)
- **Perigee altitude**: ~600 km
- **Apogee altitude**: ~39,700 km
- **Semi-major axis**: ~26,600 km

## Current Codebase Status

### Earth as Root Body
✅ **SUPPORTED**
- Validator checks: `parent_index < body_index or -1`
- Earth can be root if it's first body (index 0)
- No special validation prevents this

### 3D Orientation Support
⚠️ **DEFINED BUT NOT APPLIED**
- `inclination`, `longitude_of_ascending_node`, `argument_of_periapsis` exist in `OrbitalElements` struct
- `orbital_elements_to_cartesian()` in `src/orbital_mechanics.cpp` only produces 2D orbits (x, y, z=0)
- Documented as "deferred implementation" in technical_reference.md line 114
- Config parser supports loading these parameters (src/config_loader.cpp)
- Test approach: Tests will expect 3D behavior and fail with `[!mayfail]` tag

## Implementation Plan

### Step 1: Create Test Configuration File
**File**: `tests/test_molniya.toml`

```toml
# Test Configuration: Molniya Orbit
# Earth as root body with highly elliptical, highly inclined satellite orbit

[[bodies]]
name = "Earth"
mass = 5.972e24
radius = 6.371e6
parent_index = -1
color = { r = 0.0, g = 0.5, b = 1.0 }
orbit = {
    semi_major_axis = 0.0,
    eccentricity = 0.0,
    true_anomaly = 0.0
}

[[spacecraft]]
name = "Molniya_Satellite"
mass = 1000.0
parent_index = 0
orbit = {
    semi_major_axis = 26540000.0,
    eccentricity = 0.74,
    true_anomaly = 0.0,
    inclination = 1.107,           # 63.4° in radians
    longitude_of_ascending_node = 0.0,
    argument_of_periapsis = 4.71    # 270° in radians
}
```

**Key points**:
- Earth is root body (index 0, parent_index = -1)
- Spacecraft uses Molniya orbital parameters
- 3D orientation parameters included even though not applied yet

### Step 2: Create Test File
**File**: `tests/test_inclined_orbits.cpp`

**Test Cases**:

1. **Molniya Position Verification** (tagged `[!mayfail]`)
   - Test satellite position at true_anomaly = 0 (perigee)
   - Test satellite position at true_anomaly = π/2
   - Test satellite position at true_anomaly = π (apogee)
   - Test satellite position at true_anomaly = 3π/2
   - Verify radii match polar equation: r = a(1-e²)/(1+e·cos(ν))
   - Check z-coordinate ≠ 0 for inclined orbits (will fail until 3D implemented)
   - Tolerance: ±10 km (10,000 meters)

2. **Molniya Orbital Period** (tagged `[!mayfail]`)
   - Verify period matches theoretical calculation from Kepler's 3rd law
   - Theoretical period: T = 2π√(a³/μ)
   - Use `OrbitTracker` with minimum time to prevent false completion
   - Expected: ~11.96 hours
   - Tolerance: ±600 seconds (10 minutes)

3. **Generic Inclined Orbit** (tagged `[!mayfail]`)
   - Test moderate inclination (45°) with e=0.5, a=10,000 km
   - Verify z-coordinate is non-zero (will fail until 3D implemented)
   - Verify position magnitude matches orbital radius
   - Simpler case to test 3D orientation without Molniya complexity

4. **Inclination Parameter Preservation**
   - Verify config loader correctly reads and preserves inclination value
   - This should PASS (just checking config loading, not physics)
   - Ensures 3D parameters are being stored correctly

**Constants**:
- `POSITION_TOLERANCE_METERS = 10000.0` (±10 km per user request)
- `PERIOD_TOLERANCE_SECONDS = 600.0` (±10 minutes)

### Step 3: Update Build System
Add test file to existing test build (Catch2 automatically includes all `test_*.cpp` files)

### Step 4: Expected Behavior

#### Before 3D Implementation
- All tests with `[!mayfail]` tag will run but failure is acceptable
- Position tests: Pass radius check, fail on z-coordinate check
- Period test: May pass (period independent of inclination for same orbital energy)
- Generic inclined test: Fail on z-coordinate check
- Config preservation test: **PASS** (just checks config loading)

#### After 3D Implementation
- All position tests should **PASS**
- Period test should **PASS**
- Generic inclined test should **PASS**
- Config preservation test should **PASS**
- Remove `[!mayfail]` tags from passing tests

 ## Implementation Findings

### Bug Discovered in Spacecraft Initialization

**Location**: `src/config_loader.cpp` lines 257-259

**Code**:
```cpp
if (!is_parabolic && craft->parent_index >= 0 && craft->parent_index < sim->body_count) {
    CelestialBody* parent = &sim->bodies[craft->parent_index];
    craft->orbit.semi_major_axis += parent->radius;  // BUG: Always adds parent radius
}
```

**Problem**:
- For **spacecraft**, code unconditionally adds parent radius to `semi_major_axis`
- For **bodies**, this logic does NOT exist (bodies use semi_major_axis as-is)
- This creates an inconsistency between body and spacecraft orbital definitions
- The logic was intended for `altitude` parameter convenience but is applied even when `semi_major_axis` is specified directly

**Impact on Molniya Tests**:
- Config specifies: `semi_major_axis = 26,540,000 m`
- Actual used: `26,540,000 + 6,371,000 = 32,911,000 m`
- Position errors: 1.7M m at perigee, 11.1M m at apogee
- Orbital period: ~16.5 hours instead of expected ~12 hours
- All radius-based tests fail with errors far exceeding ±10 km tolerance

**Root Cause**:
The config loader has these paths (simplified):
```cpp
// For bodies:
if (semi_major_axis.type == TOML_FP64) {
    body->orbit.semi_major_axis = semi_major.u.fp64;  // Used directly
} else if (altitude.type == TOML_FP64) {
    body->orbit.semi_major_axis = altitude.u.fp64;  // Stored as altitude
}

// For spacecraft:
if (semi_major_axis.type == TOML_FP64) {
    craft->orbit.semi_major_axis = semi_major.u.fp64;  // Stored as SMA
} else if (altitude.type == TOML_FP64) {
    craft->orbit.semi_major_axis = altitude.u.fp64;  // Stored as altitude
}

// AFTER loading all spacecraft (line 253-260):
for (int i = 0; i < sim->craft_count; i++) {
    Spacecraft* craft = &sim->spacecraft[i];
    bool is_parabolic = (fabs(craft->orbit.eccentricity - 1.0) < 0.005);
    if (!is_parabolic && craft->parent_index >= 0) {
        CelestialBody* parent = &sim->bodies[craft->parent_index];
        craft->orbit.semi_major_axis += parent->radius;  // ALWAYS adds!
    }
}
```

**Required Fix**:
The logic should only add parent radius when `altitude` was specified:
- Option A: Track which parameter was used during parsing
- Option B: Check if semi_major_axis < parent radius (altitude would be small)
- Option C: Only apply this transformation for altitude-based initialization

### Additional Issues Found

#### OrbitTracker Not Completing
- **Symptom**: Molniya period test never completes orbit (tracker->orbit_completed = false)
- **Possible causes**:
  1. Minimum time threshold too restrictive (currently 0.01 days = 14.4 minutes)
  2. Quadrant transition logic failing for highly elliptical orbits
  3. Angle comparison tolerance too tight for large orbital variations
- **Status**: Not investigated yet, secondary priority to config bug

#### Test File Name Mismatch
- Original plan: `tests/test_molniya.toml`
- Actual created: `tests/test_inclined_orbits.toml` (matches test file name)
- Reason: User preference to have config filename match test filename
- **Status**: ✅ RESOLVED (updated config in final implementation)

## Test Verification Formula

### Orbital Radius at True Anomaly
```
r = a(1-e²) / (1 + e·cos(ν))
```
Where:
- a = semi-major_axis
- e = eccentricity
- ν = true_anomaly

### Orbital Period (Kepler's 3rd Law)
```
T = 2π√(a³/μ)
```
Where:
- μ = G·M (standard gravitational parameter)
- M = parent body mass

## Questions Resolved

1. **Tolerance**: ±10 km (10,000 meters) - chosen as moderate tolerance for RK4 integration
2. **Test Scope**: Both Molniya + generic inclined orbit tests for completeness
3. **3D Status**: Tests expect 3D and fail with `[!mayfail]` tag, documenting expected behavior

## Next Steps After This Plan

1. Implement 3D orientation in `src/orbital_mechanics.cpp`
   - Apply rotation matrices for inclination, RAAN, and argument of periapsis
   - Update `orbital_elements_to_cartesian()` to include 3D transformations

2. Remove `[!mayfail]` tags from tests that now pass

3. Add additional 3D-specific tests:
   - Verify correct orientation of orbital plane
   - Test ascending/descending node crossings
   - Verify argument of periapsis positioning

## Files to Create
- `tests/test_molniya.toml` (new test config)
- `tests/test_inclined_orbits.cpp` (new test file)

## Files to Potentially Modify (future 3D implementation)
- `src/orbital_mechanics.cpp` - add 3D rotation logic
- `src/orbital_mechanics.h` - potentially expose rotation helper functions

## Bug Fix Implementation

### Decision Made
**Remove altitude parameter entirely** rather than fix it, based on:
- Minimal usage: Only 3 test configs, 0 production configs
- High complexity cost: Requires tracking parameter usage, post-processing loop
- Confusing semantics: Bodies say they'll add radius but don't
- Clean alternative: Explicit semi_major_axis is unambiguous

### Changes Implemented

#### 1. Updated Test Configs
Replaced `altitude` with explicit `semi_major_axis` in:
- `tests/test_maneuver_planning.toml`
- `tests/test_maneuvers.toml`
- `tests/test_orbit_rendering.toml`

**Change pattern:**
```toml
# OLD:
orbit = { altitude = 400000.0, ... }

# NEW:
orbit = { semi_major_axis = 6.771e6, ... }
# LEO orbit: 400 km altitude (Earth radius 6.371e6 m + 400e3 m)
```

#### 2. Removed Altitude from Body Parsing
**File**: `src/config_loader.cpp` (lines 70-73)
- Removed `altitude` variable declaration
- Removed altitude handling block (lines 99-101)
- Updated error messages to only mention `semi_major_axis`

#### 3. Removed Altitude from Spacecraft Parsing
**File**: `src/config_loader.cpp` (lines 296-299)
- Removed `altitude` variable declaration
- Removed altitude handling block (lines 325-327)
- Updated error messages to only mention `semi_major_axis`

#### 4. Removed Buggy Post-Processing Loop
**File**: `src/config_loader.cpp` (lines 249-257)
- Deleted entire loop that unconditionally added parent radius to spacecraft semi_major_axis
- This was the root cause of the bug

#### 5. Updated Technical Reference
**File**: `docs/technical_reference.md` (line 317)
- Removed documentation of altitude convenience feature
- Simplified to only mention `semi_major_axis` as required parameter

### Test Results After Fix

**Molniya Tests:**
- Position verification: ✅ PASS (radius error: 0 m, was 1.7M m)
- Z-coordinate checks: ⚠️ FAIL (expected, 3D deferred)
- Orbital period: ⚠️ INCOMPLETE (separate OrbitTracker issue, not related to bug)

**All Tests:**
- 36 tests passed
- 3 tests failed as expected (Molniya 3D tests)
- 0 regressions

### Remaining Issues

1. **OrbitTracker not completing orbits** for Molniya period test
   - May need to increase `MAX_SIMULATION_HOURS` or fix tracking logic
   - Separate from altitude parameter bug

2. **3D orientation not implemented** (documented as deferred)
   - Molniya tests will fail z-coordinate checks until implemented