From b568c8fd5e5487fadd99e17c165e594f4afaf748 Mon Sep 17 00:00:00 2001 From: Sammy Dabbas Date: Fri, 3 Jul 2026 18:58:32 -0400 Subject: [PATCH 1/2] [#205] Keep opportunities sorted when merging windows The merge path in AccessSatellite._add_window extended a window's close time in place, leaving self.opportunities out of order at generation seams. upcoming_opportunities bisects on close time and requires sorted order. Re-insert the extended opportunity with bisect.insort using the same key as the normal insert path. --- src/bsk_rl/sats/access_satellite.py | 9 +++++++ tests/unittest/sats/test_access_satellite.py | 25 ++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/bsk_rl/sats/access_satellite.py b/src/bsk_rl/sats/access_satellite.py index 6231e348..ca7966bd 100644 --- a/src/bsk_rl/sats/access_satellite.py +++ b/src/bsk_rl/sats/access_satellite.py @@ -294,7 +294,16 @@ def _add_window( 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). + self.opportunities.remove(opportunity) 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..56335065 100644 --- a/tests/unittest/sats/test_access_satellite.py +++ b/tests/unittest/sats/test_access_satellite.py @@ -237,6 +237,31 @@ 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). + sat.opportunities = [ + dict(object=self.tgt0, window=(5.0, 30.0), type="target"), + ] + # 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"), From 1e6d067d043ade082f16ebfe9e59711190415565 Mon Sep 17 00:00:00 2001 From: Sammy Dabbas Date: Wed, 8 Jul 2026 21:30:27 -0400 Subject: [PATCH 2/2] [#205] Address review: remove by index, add release note, seed r_LP_P --- docs/source/release_notes.rst | 4 ++++ src/bsk_rl/sats/access_satellite.py | 9 ++++++--- tests/unittest/sats/test_access_satellite.py | 10 ++++++++-- 3 files changed, 18 insertions(+), 5 deletions(-) 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 ca7966bd..e4972d9f 100644 --- a/src/bsk_rl/sats/access_satellite.py +++ b/src/bsk_rl/sats/access_satellite.py @@ -288,7 +288,7 @@ 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 @@ -296,8 +296,11 @@ def _add_window( ): # Extending the close time changes the opportunity's sort # position, so re-insert it to keep opportunities ordered by - # close time (see issue #205). - self.opportunities.remove(opportunity) + # 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, diff --git a/tests/unittest/sats/test_access_satellite.py b/tests/unittest/sats/test_access_satellite.py index 56335065..78e82ee6 100644 --- a/tests/unittest/sats/test_access_satellite.py +++ b/tests/unittest/sats/test_access_satellite.py @@ -241,9 +241,15 @@ 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). + # 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"), + 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