Estimator handoff contract is asymmetric: no on-removal hook to deregister departing satellites

[zsoerenm]

Severity: minor–major — decide alongside the #130 contract before v2

The gap

The estimator gets two hooks when a satellite enters the track set:

• init_estimator_state(estimator, sat) — per-sat state (src/sat_state.jl)
• update_estimator_on_handoff(estimator, new_sats) — cross-satellite/cross-system shared state (src/sat_state.jl)

When a satellite leaves via remove_satellite! / remove_satellite (src/tracking_state.jl), it gets nothing. Removal just deletes the entry from the group dictionary:

function remove_satellite!(track_state::TrackState; prn, group = :default)
    delete!(_dict_for_group(track_state, group), prn)
    track_state
end

So an estimator with cross-satellite shared state — a joint-channel covariance block, a PRN registry, per-batch normalization terms — has no way to deregister the departing satellite. After remove_satellite!, that shared state holds a stale entry for a PRN that is no longer tracked, and there is no documented point at which it can clean up.

This is harmless for the conventional estimators (ConventionalPLLAndDLL / ConventionalAssistedPLLAndDLL), which carry no cross-sat shared state. But it directly affects the class of estimator the update_estimator_on_handoff docstring was written to support, so it is the mirror image of the handoff-contract gap fixed in #130 / #141.

Options

1. Add a symmetric hook, e.g. update_estimator_on_removal(estimator, removed_sats), defaulting to returning estimator unchanged, called by both remove_satellite! and remove_satellite. To stay consistent with the add_satellite! discards the return value of update_estimator_on_handoff — decide the estimator-handoff contract before v2 #130 resolution (which honors a rebuilt estimator through the return value), the hook should be allowed to return a rebuilt estimator of the same concrete type — and remove_satellite! would then need to thread the return value through a TrackState{G,DE}(...) rebuild, exactly as add_satellite! now does. The "always bind the result" convention adopted in fix(tracking): honor rebuilt estimator from add_satellite! handoff #141 already covers the call sites.

2. Document the asymmetry as intentional — state that estimators with shared state must not assume a deregistration callback exists, and must tolerate stale per-PRN entries (or reconcile lazily against the live satellite set on the next update_estimator_on_handoff / tracking step).

Coupling with #130

Whatever contract is chosen for entry should mirror on exit:

• If add_satellite! discards the return value of update_estimator_on_handoff — decide the estimator-handoff contract before v2 #130 lands on Option 1 (return a possibly-rebuilt estimator — current state of fix(tracking): honor rebuilt estimator from add_satellite! handoff #141), the removal hook should also return a possibly-rebuilt estimator, and remove_satellite! threads it through.
• If add_satellite! discards the return value of update_estimator_on_handoff — decide the estimator-handoff contract before v2 #130 is revisited toward Option 2 (estimators must mutate shared state in place and return the same object), the removal hook can be in-place too, and neither ! function needs to return a swapped estimator.

Found in review of #141 (fix for #130).