Skip to content

refactor(rpo-nyx): NYX audit cleanup + greenfield follow-ups#11

Merged
sakobu merged 13 commits into
mainfrom
nyx-audit-cleanup
Apr 14, 2026
Merged

refactor(rpo-nyx): NYX audit cleanup + greenfield follow-ups#11
sakobu merged 13 commits into
mainfrom
nyx-audit-cleanup

Conversation

@sakobu
Copy link
Copy Markdown
Owner

@sakobu sakobu commented Apr 14, 2026

No description provided.

sakobu added 13 commits April 14, 2026 10:48
replan_mission had no callers outside its own tests. The WASM-eligible
primitive replan_from_transfer in rpo-core remains live.

Audit reference: NYX_AUDIT.md Findings 1 and 9.
The deletion of `replan_mission` in the previous commit removed the
`use rpo_core::pipeline::replan_from_transfer` import, which broke the
`[replan_from_transfer]` rustdoc link in the module doc. Fully-qualify
both intra-doc links so they survive future import cleanups.
Shrinks the rustdoc surface of nyx_bridge from 17 items to 9. Demoted:
apply_impulse, build_nyx_safety_states, query_anise_eclipse, state_to_orbit,
TimedState, ChiefDeputySnapshot, EARTH_J2000. No external consumers anywhere
in the workspace.

Also dropped config_to_spacecraft and spacecraft_to_state from the crate-root
re-export block entirely: their only internal consumer (propagate.rs) imports
them directly via super::conversions::, so a pub(crate) re-export was
unused. They remain pub fn in conversions.rs, unchanged.

Collateral fix: dynamics.rs had an intra-doc link
[config_to_spacecraft](crate::nyx_bridge::config_to_spacecraft) that relied
on the now-removed re-export. Replaced with a plain code span since
conversions is a private module.

Audit reference: NYX_AUDIT.md Findings 2 and 8.
Only caller outside tests is pipeline::compute_validation_burns in the
same crate; external consumers reach it through that wrapper.

Collateral: downgrade the intra-doc link to `convert_cola_to_burns` in
pipeline::compute_validation_burns to a plain code span, since rustdoc
warns when public docs link to private items.

Audit reference: NYX_AUDIT.md Finding 3.
Three sites in monte_carlo were missing the mandatory inline comment
required by CLAUDE.md §Casting Policy. No behavioral change.

Also normalized two sites in validation/trajectory.rs that used
ASCII-form comments ("u32 -> usize: safe (usize >= 32 bits)") to the
canonical Unicode form ("u32 → usize: always safe (usize ≥ 32 bits)")
so every prod u32→usize cast in rpo-nyx now matches the exact string.

Audit reference: NYX_AUDIT.md Finding 7.
Brings monte_carlo in line with nyx_bridge, pipeline, and validation
(each of which hosts its error type in a dedicated errors.rs sibling).
Also adds a comment explaining the asymmetric Box<NyxBridgeError>
size optimization.

Audit reference: NYX_AUDIT.md Findings 5 and 8 (boxing comment).
Covers disperse_deputy_state and disperse_spacecraft — pure sampling
functions with no nyx dependency. Six tests total: pass-through,
Gaussian perturbation / physical-bounds clamping, and deterministic
seeding for each function.

Audit reference: NYX_AUDIT.md Finding 6.
Splits the 867-prod-line module into trajectory/{cola,leg,pipeline}.rs,
mirroring the structure already used by monte_carlo/ and nyx_bridge/.
No behavioral change; public path of validate_mission_nyx, ColaBurn,
ColaValidationInput, ValidationConfig, ValidationPipelineCtx is preserved.

Audit reference: NYX_AUDIT.md Finding 4 (decomposition).
The pub(crate) helper at monte_carlo/statistics.rs:23 shadowed the core
function at rpo-nyx/src/statistics.rs:110 -- call sites were ambiguous
on casual read. Renaming to require_percentile_stats captures the
semantic difference (Result<_, EmptyEnsemble> vs Option) and removes
the shadow.

5 non-test + 5 test call sites updated in monte_carlo/execution.rs and
monte_carlo/statistics.rs. The sampling.rs tests were incidentally
pointing at the wrapper via super::super::statistics; repointed them
at crate::statistics (the core Option-returning function) since they
don't need the Result semantics. Zero behavioral change.

Audit reference: NYX_AUDIT.md Finding 3.4.
Previously rpo-nyx::pipeline::plan_mission was re-exported from a
top-level pub(crate) mod planning, so the code location didn't match
the public path. Moving the file under pipeline/ removes the cross-
module re-export indirection and puts plan_mission physically where
consumers find it.

The only non-test caller of crate::planning:: was pipeline/mod.rs
itself, so the move has zero cascading churn across the workspace.

Audit reference: NYX_AUDIT.md Finding 8 (cohesion).
Previous order was alphabetical. Layered ordering (primitives ->
utilities -> orchestration) encodes the dependency arrow and matches
how contributors navigate the crate. Section comments make the
layering explicit.

nyx_bridge precedes lambert because lambert imports state_to_orbit
from nyx_bridge. monte_carlo / validation / pipeline are ordered by
increasing composition scope.

Audit reference: NYX_AUDIT.md Finding 8 (ordering).
…dation tests

23 validation tests in rpo-nyx are #[ignore]d because they need SPICE
ephemeris data from MetaAlmanac. Without a note in CONTRIBUTING, a new
contributor running --include-ignored hits an opaque network error.

Audit reference: NYX_AUDIT.md Finding 3.4 (ignored-test runbook).
Two stale refs introduced earlier in this branch surfaced during the
final --include-ignored sanity check:

1. Test counts: 7f0ed86 added 6 inline dispersion tests to rpo-nyx,
   bringing rpo-nyx unit+integration from 132 to 136 and workspace
   total from 621 to 625. Ignored count unchanged (32).

2. trajectory.rs path: ccf8017 split validation/trajectory.rs into
   validation/trajectory/{cola,leg,pipeline}.rs. The full-physics tol
   constants (FULL_PHYSICS_SINGLE_LEG_POS_TOL_KM, DRAG_STM_VS_NYX_
   POS_TOL_KM, FULL_PHYSICS_MULTI_LEG_POS_TOL_KM) now live in
   trajectory/pipeline.rs.

Verified: cargo test --workspace -- --include-ignored passes 625/625
(0 failed, 0 ignored in --include-ignored mode) across all 5 crates.
@sakobu sakobu merged commit 90421e1 into main Apr 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant