[#205] Keep opportunities sorted when merging windows

[Sammy-Dabbas]

Fixes #205.

Root cause

AccessSatellite._add_window has two paths: the normal bisect.insort insert (keeps order) and a merge path that extends a window truncated at a generation seam. The merge path mutated the opportunity's close time in place, so the extended window kept its old (smaller-key) position and any windows closing between the seam and the true end were left out of order -- exactly the behavior described in #205. upcoming_opportunities runs bisect.bisect_left on self.opportunities, which is only valid on a list sorted by close time, so the broken invariant silently corrupts opportunity lookups.

Fix

In the merge branch: remove the opportunity, extend its close time, and re-insert with bisect.insort(..., key=lambda x: x["window"][1]) -- the same primitive and key the normal insert path already uses. No new imports; nothing else touched.

Test

Added test_add_window_merge_keeps_sorted to the existing TestAccessSatellite class (reusing its fixtures/mocks). It plays the two-segment seam scenario: without the fix the close-time list comes out [50.0, 35.0, 40.0]; with it, [35.0, 40.0, 50.0], and the merge still produces a single extended (5.0, 50.0) window rather than a duplicate.

ruff/isort clean on the touched files.

[Copilot]

Pull request overview

Fixes an ordering invariant bug in AccessSatellite._add_window where merging (extending) an existing opportunity window could leave self.opportunities out of close-time order, breaking bisect-based lookups (Issue #205).

Changes:

• Re-inserts an extended (merged) opportunity via bisect.insort(..., key=close_time) to preserve sorted order.
• Adds a regression unit test covering the “generation seam” merge scenario to ensure opportunities remain sorted and merges don’t duplicate windows.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/bsk_rl/sats/access_satellite.py	Updates the merge/extend path in _add_window to maintain the sorted-by-close-time invariant.
tests/unittest/sats/test_access_satellite.py	Adds a regression test for Issue #205 verifying sorted order and correct merge behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[LorenzzoQM]

Thanks for addressing the issue, the fix looks good and clean! Please, update the release notes under docs/source/release_notes.rst with a brief description of the issue and fix.

[Sammy-Dabbas]

Thanks. Added the release notes entry under Development. Switched the merge path to index-based removal with del self.opportunities[i], so it no longer compares opportunity dicts by value; that avoids the numpy ambiguous-truth-value error and removes the exact matched opportunity. The regression test now seeds r_LP_P as a numpy array to match real contents.

[LorenzzoQM]

Looks good, thanks for addressing the comments!