fix(octopus): split a slot around a fully-contained overlap instead of losing its remainder - #4504
Merged
Merged
Conversation
…f losing its remainder (#4497) fetch.py merges the HA Octopus Energy integration's completed_dispatches ahead of planned_dispatches unconditionally. load_octopus_slots()'s overlap dedup processes slots in that order, but could only ever trim one edge of a slot against an already-decoded one, or drop it entirely if fully contained - it had no path to split a slot that fully CONTAINS an already-decoded one. A short completed historical interval sitting inside a much longer still-active planned interval therefore truncated the planned interval down to whatever sliver preceded the completed interval, silently discarding its entire future remainder (e.g. a planned 07:59-16:00 dispatch containing a 08:00-09:00 completed interval collapsed to 07:59-08:00, already in the past). Reported with an exact repro and code trace by @cparmar. The overlap resolution now subtracts each already-decoded slot's span from the new slot's remaining segments, which can produce zero, one, or two pieces - the two-piece case is the one that was missing. kwh is only rescaled proportionally when a slot is actually split into multiple pieces; the existing single-edge-trim behaviour (and its energy value) is unchanged, preserving the existing hard-coded regression expectations in test_octopus_slots.py.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes overlap-deduplication in load_octopus_slots() so that when a new Octopus dispatch slot fully contains an already-decoded slot, the new slot is correctly split into “before” and “after” segments (instead of being truncated and losing its future remainder). This directly addresses Issue #4497’s reported case where completed historical dispatches (merged ahead of planned dispatches) could collapse an otherwise-valid future charging window.
Changes:
- Update
apps/predbat/octopus.pyoverlap handling to subtract all previously-decoded slot spans from the incoming slot, allowing 0/1/2 resulting segments and proportionally scaling kWh only when an actual split occurs. - Add a regression test in
apps/predbat/tests/test_octopus_slots.pycovering “completed slots fully contained within a planned slot” and asserting the future remainder survives with correctly-scaled kWh.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/predbat/octopus.py | Fix overlap deduplication to support the containment/split case and preserve future remainder segments. |
| apps/predbat/tests/test_octopus_slots.py | Add regression coverage reproducing the containment overlap scenario from #4497. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #4497 -
fetch.pymerges the HA Octopus Energy integration'scompleted_dispatchesahead ofplanned_dispatchesunconditionally.load_octopus_slots()'s overlap dedup loop processes slots in that order, but could only ever trim one edge of a slot against an already-decoded slot, or drop it entirely if it was fully contained within an already-decoded one - it had no path for the reverse case, where a new slot fully contains an already-decoded one.A short completed historical interval sitting inside a much longer still-active planned interval therefore truncated the planned interval down to whatever sliver preceded the completed interval, silently discarding its entire future remainder. Example from the report: a planned 07:59-16:00 dispatch containing a 08:00-09:00 completed interval collapsed to 07:59-08:00 - already in the past by the time it's evaluated, so the car plan lost its whole future charging window despite a valid ready-by time and SoC gap.
Reported by @cparmar with an exact repro payload and precise code line references - I traced it through the actual code before touching anything and it checks out exactly as described (see verification note below).
The fix
The overlap resolution now subtracts each already-decoded slot's span from the new slot's remaining segments, which can produce zero, one, or two pieces - the two-piece case is the one that was missing:
kwhis only rescaled proportionally when a slot is actually split into multiple pieces, mirroring the proportional scalingdecode_octopus_slot()already does a few lines above for forecast-window capping. The pre-existing single-edge-trim behaviour (and its energy value) is left exactly as before, so the existing hard-coded regression expectations intest_octopus_slots.py'ssample_badtest are unaffected.Verification
Hand-traced the reporter's exact example against the pre-fix code and reproduced the failure precisely (planned slot's end trimmed to the completed interval's start, future remainder never restored). Added a regression test reproducing it exactly; confirmed it fails without the fix (
got 1080- only the unrelated later dispatch survived, the contained one's future remainder was gone) and passes with it.Test plan
test_octopus_slots.py: completed dispatches fully contained within a planned dispatch - future remainder survives with correctly-scaled kwh, non-overlapping dispatches untouchedoctopus.pyonly, re-ran)test_octopus_slots,multi_car_iog,add_now_to_octopus_slot,octopus_slots_changetests pass unchanged./run_all --quickpasses./run_pre_commitpasses🤖 Generated with Claude Code