diff --git a/docs/config_validation_refactoring_plan.md b/docs/config_validation_refactoring_plan.md new file mode 100644 index 0000000..3a42f8b --- /dev/null +++ b/docs/config_validation_refactoring_plan.md @@ -0,0 +1,162 @@ +# Config Validation Refactoring Plan + +## Overview + +Refactor config validation logic to separate it from parsing logic in `config_loader.cpp`, creating a dedicated `config_validator` module. + +## Phase 1: Create Config Validator Module + +**Goal**: Separate validation logic from parsing logic + +**Tasks**: +1. Create `src/config_validator.h` with function declarations: + - `bool validate_initial_positions(SimulationState* sim)` - move from simulation + - `bool validate_mass_ratios(SimulationState* sim)` - new + - `bool validate_soi_overlap(SimulationState* sim)` - new + - `bool validate_nested_orbits(SimulationState* sim)` - new + - `bool run_all_config_validations(SimulationState* sim)` - wrapper + +2. Create `src/config_validator.cpp` with implementations: + - Move `validate_initial_positions()` from `simulation.cpp` + - Implement `validate_mass_ratios()` - check parent mass / child mass >= MIN_MASS_RATIO (1000) + - Implement `validate_soi_overlap()` - for bodies sharing same parent, check if `distance < (soi_a + soi_b)`, ERROR if true + - Implement `validate_nested_orbits()` - for chains like Sun→Earth→Moon, ensure child's orbital radius is < (fraction) of parent's SOI + - Implement `run_all_config_validations()` - calls all validators and returns false if any fail + +**Design Decisions**: +- SOI overlap: **ERROR** (reject configs with overlapping SOIs) +- Mass ratio threshold: **MACRO/DEFINE** for easy adjustment (default 1000) +- Nested orbit check: Use SOI (not Hill sphere), check `child_semi_major_axis < (0.5 * parent_soi_radius)` + +--- + +## Phase 2: Update Existing Modules + +**Tasks**: +1. Update `src/config_loader.h`: + - Add `#include "config_validator.h"` + +2. Update `src/config_loader.cpp`: + - Replace call to `validate_initial_positions(sim)` (line 281) with `run_all_config_validations(sim)` + - Move inline validation blocks into separate functions in `config_validator.cpp`: + - `validate_parent_index_ordering()` + - `validate_orbital_elements()` + - Keep parsing functions (`parse_toml_body`, `parse_toml_spacecraft`, etc.) focused on TOML parsing only + +3. Update `src/simulation.h`: + - Remove declaration of `validate_initial_positions()` + +4. Update `src/simulation.cpp`: + - Remove implementation of `validate_initial_positions()` (lines 181-219) + +--- + +## Phase 3: Update Tests + +**Tasks**: +1. Update `tests/test_invalid_parent_assignment.cpp`: + - **Test 1** ("Earth should not become child of spacecraft"): **DELETE** + - Spacecraft are now in separate array + - Cannot become parent to bodies + + - **Test 2** ("Massive bodies never become children of small bodies"): **UPDATE** + - Create new test config with small comet orbiting Earth/Mars + - Test should now call `load_system_config()` which will run `validate_mass_ratios()` + - Config should fail if mass ratio < 1000 + + - **Test 3** ("Detect placeholder config values"): **DELETE** + - This checked distance validation which is now in `validate_initial_positions()` + - Validation happens at load time, not needed as separate test + + - **Test 4** ("Mutual SOI"): **UPDATE** to expect load failure + - Keep `mutual_soi_close.toml` as-is (invalid config) + - Test should expect `load_system_config()` to fail + - Verify error message about SOI overlap + +2. Update `tests/configs/`: + - Create new config for Test 2 (small comet around Earth/Mars with mass ratio < 1000) + - Keep `mutual_soi_close.toml` as-is (invalid for Test 4) + +--- + +## Phase 4: Update Build System + +**Tasks**: +1. Update `Makefile`: + - Add `config_validator.o` to build target + - Update dependencies for `orbit_sim` and `orbit_test` + +--- + +## File Changes Summary + +**New Files**: +- `src/config_validator.h` +- `src/config_validator.cpp` + +**Modified Files**: +- `src/config_loader.h` +- `src/config_loader.cpp` +- `src/simulation.h` +- `src/simulation.cpp` +- `Makefile` +- `tests/test_invalid_parent_assignment.cpp` + +**New Test Configs**: +- `tests/configs/small_comet_bad_mass_ratio.toml` (invalid config for Test 2) + +**Existing Test Configs** (keep as-is): +- `tests/configs/mutual_soi_close.toml` (invalid config for Test 4) + +--- + +## Implementation Order + +1. Create config_validator module (Phase 1) +2. Update simulation and config_loader (Phase 2) +3. Update build system (Phase 4 - test compile) +4. Update tests and configs (Phase 3) +5. Run full test suite to verify changes + +--- + +## Validation Logic Details + +### `validate_mass_ratios(SimulationState* sim)` +- For all bodies with parent_index >= 0: + - Calculate ratio = parent_mass / child_mass + - If ratio < MIN_MASS_RATIO (1000), print error and return false + - Skip check for spacecraft (they have their own validation) + +### `validate_soi_overlap(SimulationState* sim)` +- For all pairs of bodies sharing the same parent: + - Calculate distance between their orbital positions + - Calculate sum of their SOI radii + - If distance < (soi_a + soi_b), print error and return false + +### `validate_nested_orbits(SimulationState* sim)` +- For all bodies with parent that is not root (parent_index != 0): + - Calculate child's orbital radius (semi_major_axis) + - Get parent's SOI radius + - If child_semi_major_axis > (0.5 * parent_soi_radius), print error and return false + - Ensures child is well within parent's gravitational dominance region + +### `run_all_config_validations(SimulationState* sim)` +- Calls all validators in sequence: + 1. validate_parent_index_ordering() + 2. validate_orbital_elements() + 3. validate_mass_ratios() + 4. validate_soi_overlap() + 5. validate_nested_orbits() + 6. validate_initial_positions() +- Returns false if any validator fails +- Stops on first failure for efficiency + +--- + +## Constants and Thresholds + +```cpp +#define MIN_MASS_RATIO 1000.0 +#define NESTED_ORBIT_FRACTION 0.5 // Child orbit must be < 50% of parent SOI +```