# Test Refactoring Optimization Strategy

## Refactoring Rules

### 1. Structure
- One `SCENARIO("description")` per logical test group, with `[tag1][tag2]` annotations
- Run `./build/orbit_test --list-tags` to see tags used by other tests. Original tags from the old test file are a useful starting point, but the implementor has discretion to choose appropriate tags.
- **Use SCENARIO as a shared fixture** for setup/initialization. SECTIONs represent **different test scenarios** that branch from that fixture with distinct operations. Avoid using SECTIONs as section headers to group assertions about the same result — group related assertions into fewer SECTIONs instead.
- Catch2 re-initializes the fixture before each SECTION, so shared constants, structs, and variables declared in the SCENARIO body run fresh per SECTION. This is intentional: each SECTION should test a different code path or mutation of the fixture.
- Example: one SECTION checks the initial state before modification; a separate SECTION applies a burn and checks all post-burn results.
- Embed expected values directly in `WithinAbs()` calls (see Section 4 for precalc script usage). No need to declare named constants unless the value is reused.
- **No decorative comments.** Do not add `// (Old: ...)` comments, `===` separators, `---` separators, or any other decorative annotations. The SECTION description string is the documentation.
- **Use `REQUIRE()` for integer comparisons**, `WithinAbs()` only for floating-point. E.g., `REQUIRE(sim->body_count == 2)` not `REQUIRE_THAT(sim->body_count, WithinAbs(2.0, 0.001))`.

### 2. Duplication Elimination
- Use lambdas that capture the fixture for repeated setup→call→assert patterns
- Reuse shared structs in-place (mutate fields rather than recreating)

### 3. Assertions
- Include `src/test_utilities.h` for tolerance constants and test utilities
- `#include <catch2/matchers/catch_matchers_floating_point.hpp>` (required for `WithinAbs` matcher)
- `using Catch::Matchers::WithinAbs;` after includes
- `REQUIRE_THAT(value, WithinAbs(expected, tolerance))` — never `Approx()`
- **Always use named tolerance constants** — never hardcode raw numbers for tolerance in `WithinAbs()`. Exception: relative error thresholds for continuous/low-thrust approximations (e.g., `WithinAbs(0.0, 0.01)` for 1% tolerance) when no named constant exists.

#### Tolerance Reference
All constants defined in `src/test_utilities.h` — use those, do not redefine locally.

| Constant | Value | Use for |
|----------|-------|---------|
| `D_TOL` | `1e-12` | Double-precision arithmetic (vec3, mat3 ops) |
| `A_TOL` | `1e-6` | Semi-major axis (meters) |
| `E_TOL` | `1e-12` | Eccentricity, round-trip conversion |
| `ANG_TOL` | `1e-12` | Angles in radians (nu, inc, Ω, ω) |
| `ANG_TOL_COARSE` | `1e-4` | Angles, degenerate cases (polar/retrograde) |
| `R_TOL` | `1e-6` | Radius / distance magnitudes (meters) |
| `V_TOL` | `1e-6` | Velocity magnitudes (m/s) |
| `M_TOL` | `1e-6` | Time / period values (seconds) |
| `REL_TOL` | `1e-8` | Relative / percentage errors (dimensionless) |
| `DRIFT_TOL` | `1e-12` | Energy drift percent (parabolic orbit) |

- Replace qualitative checks (`a > b`) with quantitative (`WithinAbs(expected, tol)`)
- `INFO("label: " << value)` for debugging context

### 4. Precalc Scripts
- For each test file, create `scripts/precalc_<test_name>.py` that computes expected values.
- **Always output local-frame values** (distances from parent, not global from origin).
  - C++ tests typically use local coordinates (e.g., `vec3_distance(craft->local_position, (Vec3){0,0,0})`).
  - Global distances are dominated by parent body positions (e.g., Earth-Sun distance swamps LEO orbit).
- **Always output SI units** (meters, m/s, seconds) — C++ tests use SI internally.
- Output C++-style comments with precalculated expected values for embedding in the test. Tolerances are chosen separately by the test writer using the Tolerance Reference table — the precalc script should not output tolerance values.
- **No decorative comments in precalc scripts.** Use simple blank lines between sections, no `# ====` or `# -----` separators.
- Run with: `python3 scripts/precalc_<test_name>.py`

## Test Refactoring Status

### Completed
- `test_barkers_equation` — Barker's equation unit tests + parabolic propagation
- `test_cartesian_to_elements_advanced` — Advanced conversion tests (eccentricity spectrum, inclination, true anomaly, 3D orientation)
- `test_cartesian_to_elements_basic` — Element round-trip conversion (semi-major axis, eccentricity, true anomaly, inclination, radius, velocity)
- `test_parabolic_orbit` — Parabolic orbit energy conservation + escape trajectory + initial conditions
- `test_extreme_eccentricity` — High-eccentricity orbits (single SCENARIO, precalculated values, REL_TOL)
- `test_extreme_orientation_mixed` — Extreme orientation conversions, rotation matrix properties, singularity handling
- `test_extreme_timescales` — 9 TEST_CASEs → 1 SCENARIO with 11 SECTIONs, all WithinAbs use named constants
- `test_analytical_propagation` — 5 SCENARIOs → 1 SCENARIO with 23 SECTIONs, precalculated values, all WithinAbs use named constants
- `test_moon_orbits` — Multi-body hierarchical propagation with local/global coordinate tracking
- `test_omega_debug` — Burn + element reconstruction + maneuver triggers
- `test_energy` — Energy calculations and conservation tests
- `test_inclined_orbits` — 3D inclined orbit conversions, Molniya orbits, rotation matrices
- `test_maneuvers` — Impulsive burn tests with precalculated values
- `test_maneuver_planning` — Maneuver trigger system (TIME + elliptical TRUE_ANOMALY triggers)
- `test_orbital_period` — Orbital period calculations, SCENARIO/SECTION pattern
- `test_true_anomaly_roundtrip` — True anomaly conversion round-trips, tight tolerances
- `test_physics_utilities` — Vector math, acceleration, matrix ops, rotation matrices, compare_vec3
- `test_periapsis_burn` — prograde burns
- `test_hybrid_burns` — 14 TEST_CASEs → 1 SCENARIO with 22 SECTIONs, impulse + continuous burns, precalculated values; converted 17 qualitative checks to quantitative, replaced hardcoded tolerances with named constants (A_TOL, E_TOL, D_TOL, M_TOL, ANG_TOL, R_TOL)

### Can Refactor Now (sim_engine.py supports all features needed)
- `test_orbit_rendering` — rendering tests (check if sim_engine needed)
- `test_precision_boundaries` — boundary condition tests
- `test_invalid_parent_assignment` — validation/error handling tests
- `test_newton_raphson_convergence` — numerical convergence tests

### Blocked on Missing Features
- `test_soi_transition` — needs SOI transitions
- `test_root_body_transitions` — needs SOI transitions
- `test_hybrid_energy_conservation` — needs energy functions (KE, PE, total)
- `test_hyperbolic_orbit` — needs hyperbolic propagation
- `test_rendezvous` — needs Hohmann transfer calculations

## Tooling & Sim Engine Capabilities

### Tooling
- `scripts/sim_engine.py` — Generic orbital mechanics simulator (Python, TOML 1.0 configs)
  - Replicates C++ physics: Kepler propagation, orbital↔Cartesian transforms, drift detection
  - Multi-body hierarchical propagation with global/local coordinate tracking
  - Use for precalculating expected values (transition times, final states, energy conservation)
- TOML configs in `tests/` must use TOML 1.0 inline table syntax (single-line `{}`)
  - Old configs in `old_tests/` use multiline inline tables (toml-c17 style) — keep for reference
  - Python's `tomllib` requires single-line inline tables

### Sim Engine Capabilities
#### Implemented
- Maneuver trigger system (TIME and TRUE_ANOMALY triggers)
- BurnResult capture (position, velocity, true anomaly at exact burn time)
- Body propagation (elliptical + parabolic via Barker's equation)
- Orbital↔Cartesian transforms (full z-x-z Euler rotation)
- Velocity drift detection and element reconstruction
- Global coordinate computation (hierarchical parent→child)
- Spacecraft struct, loading, propagation
- Impulsive burns (prograde, retrograde, normal, antinormal, radial_in, radial_out, custom)
- TOML 1.0 config parsing

#### NOT Implemented (notify the user before beginning to refactor)
- SOI transitions
- Maneuver TRUE_ANOMALY triggers only work with elliptical orbits (parabolic/hyperbolic branches not implemented)
- Hohmann transfer calculations
- Rendezvous planning
- OrbitTracker
- Energy functions (KE, PE, total)
- Hyperbolic propagation

## Refactoring Procedure

### Step 1: Refactor
- **Pre-flight check:** Before starting, verify the python ./scripts/sim_engine.py supports all features the test needs (SOI, Hohmann transfers, rendezvous, hyperbolic propagation, energy functions). If any feature is missing, stop and report it to the user — do not begin refactoring until unblocked.
- Check if the test is already in `tests/` (already refactored). Skip if so.
- Process **one test file at a time**.
- Create `scripts/precalc_<test_name>.py` and run it to get expected values. Tests that use the simulation engine will need a precalc script regardless of whether a TOML config exists.
- Copy from `old_tests/` to `tests/`, rewrite using the pattern from `test_true_anomaly_roundtrip.cpp`.
- Verify the test file has a TOML config in `old_tests/`. If it doesn't, the test is likely hardcoded — still refactor the C++ code but skip the TOML rewrite step.
- Rewrite TOML configs to TOML 1.0 inline table syntax (single-line `{}`).
- Follow all rules in sections 1-4 above.

### Step 2: Tighten Tolerances
- Build and verify: `make test-build` then `./build/orbit_test -s '[tag]'`.
- Run full suite: `./build/orbit_test | tail`.
- Review every tolerance against actual observed errors from `-s` output.
- **If a test fails due to a tolerance being too tight, report the observed error to the user and ask whether to loosen the constant or investigate the root cause. Never silently widen a tolerance.**
- Refer to the tolerance reference table in Section 3 for constant names.

### Step 3: Code Review
- Remove unused includes (only include what's actually used).
- Remove unused variables (e.g., `const double mu = G * M_sun;` if never referenced).
- Look for repeated initialization patterns — extract into helper lambdas (`make_elements`, `convert_and_recover`).
- Use `const` for all fixture data and recovered results.
- Replace C-style arrays with `std::array` where appropriate.
- Ensure consistent tolerance usage (no hardcoded `1e-4` when `ANG_TOL_COARSE` exists).
- Check for `compare_vec3` availability in `test_utilities` instead of 6 individual `REQUIRE_THAT` calls.
- Run full suite again: `make test`.

#### Final Systematic REQUIRE Statement Review
After Step 3, review every `REQUIRE` in the test file:

1. `grep -Rn 'REQUIRE' tests/<test_name>.cpp`
2. Categorize each:
   - **Hardcoded tolerances** → replace with named constant
   - **Qualitative** (`a > b`, `x < 0.1`, `fabs(x) > 0`) → convert to quantitative via precalc
   - **OK as-is** → booleans, integers, strings, enums
4. Verify precalc outputs all needed values
5. `make test-build` → `./build/orbit_test -s '[tag]'`

### Step 4: User interaction
- **Always ask for review** before moving to the next file.
- **Only commit when asked.**