diff --git a/docs/source/release_notes.rst b/docs/source/release_notes.rst index d8936f6c..cf4d867a 100644 --- a/docs/source/release_notes.rst +++ b/docs/source/release_notes.rst @@ -5,6 +5,10 @@ Development - |version| ----------------------- .. *Release Date: MMM. DD, YYYY* +* Fix a bug where extending an access opportunity window across a generation seam + could leave the satellite's ``opportunities`` list out of close-time order, which + corrupted ``bisect``-based opportunity lookups. See issue #205. + Version 1.3.0 ------------- *Release Date: May. 19, 2026* diff --git a/src/bsk_rl/sats/access_satellite.py b/src/bsk_rl/sats/access_satellite.py index 6231e348..e4972d9f 100644 --- a/src/bsk_rl/sats/access_satellite.py +++ b/src/bsk_rl/sats/access_satellite.py @@ -288,13 +288,25 @@ def _add_window( check all windows for merges. """ if new_window[0] == merge_time or merge_time is None: - for opportunity in self.opportunities: + for i, opportunity in enumerate(self.opportunities): if ( opportunity["type"] == type and opportunity["object"] == object and opportunity["window"][1] == new_window[0] ): + # Extending the close time changes the opportunity's sort + # position, so re-insert it to keep opportunities ordered by + # close time (see issue #205). Remove by index rather than + # value: opportunity dicts hold r_LP_P as a numpy array, so + # list.remove would compare arrays and raise on their + # ambiguous truth value. + del self.opportunities[i] opportunity["window"] = (opportunity["window"][0], new_window[1]) + bisect.insort( + self.opportunities, + opportunity, + key=lambda x: x["window"][1], + ) return bisect.insort( self.opportunities, diff --git a/tests/unittest/sats/test_access_satellite.py b/tests/unittest/sats/test_access_satellite.py index d20f004a..78e82ee6 100644 --- a/tests/unittest/sats/test_access_satellite.py +++ b/tests/unittest/sats/test_access_satellite.py @@ -237,6 +237,37 @@ def test_add_window(self, merge_time, tgt, window, expected_window): ) assert expected_window in sat.opportunities_dict()[tgt] + def test_add_window_merge_keeps_sorted(self): + # Regression test for issue #205: extending a window across a generation + # seam via the merge path must keep opportunities sorted by close time. + sat = self.make_sat() + # Segment 1 left tgt0 truncated at the seam (close time 30). Seed with + # r_LP_P as a numpy array to match how _add_window stores opportunities. + sat.opportunities = [ + dict( + object=self.tgt0, + window=(5.0, 30.0), + type="target", + r_LP_P=np.zeros(3), + ), + ] + # Segment 2 generates windows starting at the seam (merge_time=30). + # Insert opportunities that close after the seam but before tgt0's + # true close time, then extend tgt0 across the seam. + sat._add_window( + self.tgt1, (31.0, 35.0), merge_time=30.0, type="target", r_LP_P=np.zeros(3) + ) + sat._add_window( + self.tgt2, (33.0, 40.0), merge_time=30.0, type="target", r_LP_P=np.zeros(3) + ) + sat._add_window( + self.tgt0, (30.0, 50.0), merge_time=30.0, type="target", r_LP_P=np.zeros(3) + ) + close_times = [opp["window"][1] for opp in sat.opportunities] + assert close_times == sorted(close_times) + # The merge still extends the original tgt0 window rather than duplicating it. + assert sat.opportunities_dict()[self.tgt0] == [(5.0, 50.0)] + opportunities = [ dict(object="downObj1", window=(10, 20), type="downlink"), dict(object="tgtObj1", window=(20, 30), type="target"),