Repository navigation
fix(plan): shrink, not grow, the source window in the export swap's partial combine (#5423) - #5447
Merged
Merged
Conversation
…artial combine (#5423) When optimise_swap_export partially combined an export window into a later, trimmed one, it lengthened the earlier window by the amount moved instead of shortening it, with no clamp at the previous window's end. The earlier window's start then slid back into the slot before it. With a manual demand override in that slot, the guards (which only check a window's start minute) missed it, and the plan exported through the override - in #5423 a 19:30 window was pushed back to 19:05-19:15 inside a 19:00-19:30 demand slot, and the inverter was put into timed export at 19:05. The earlier window now shrinks by what the target gains, so the move conserves export time as the full combine already does. The random golden results change for 3 of 20 scenarios accordingly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused arithmetic correction is consistent with adjacent combine logic and has targeted regression coverage.
0 open findings
What changed in this PR
Fixes export-window partial combining so manual demand overrides remain respected.
Changes:
- Conserves export duration by shrinking the source window.
- Adds a regression test for issue #5423.
- Updates affected random-scenario baselines.
| File | Description |
|---|---|
apps/predbat/plan.py |
Corrects partial-combine window arithmetic. |
apps/predbat/tests/test_optimise_swap_export.py |
Tests manual-slot protection. |
coverage/cases/random_results.json |
Updates expected planning results. |
🧠 Review effort: Balanced
💡 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.
(Posted by Claude on Rik's behalf.)
Closes #5423
Problem
A manual demand override for 19:00-19:30 was not respected: the plan exported inside it, and at 19:05 the inverter was put into timed export until a later recompute moved the export to 19:30.
The log attached to #5423 shows the cause.
find_charge_windowsplit 19:00-19:30 out as its own window correctly, but each fresh plan then started the export window inside it:Cause
optimise_swap_export's Partial combine branch moves export time from an earlier window into a later, trimmed one. It lengthened the earlier window by the amount moved (amount_to_move + window_length) instead of shortening it, and unlike the plain Swap branch it has noprevious_endclamp. The earlier window's start therefore slid back into the slot before it. The manual-override guards only check whether a window's start minute is a manual time, so the stretched 19:30 window passed them and exported through the demand slot.Fix
The earlier window now shrinks by what the target gains (
window_length - amount_to_move), so the move conserves export time, as the Full combine branch already does. The new length is always at least one minute in this branch, because Full combine is taken whenever the two windows fit together.Tests
tests/test_optimise_swap_export.py: a manual slot, then an export window, then a trimmed later export window. It fails on main (the middle window is pushed back into the manual slot) and passes with the fix../run_all --quick: only therandomgolden test changes, for 3 of its 20 scenarios (metric 1164.54 → 1164.47, 628.40 → 628.36, 477.07 → 477.35).randompasses on main without the fix.cases/random_results.jsonis updated with those values only; runtimes are left as they were.Not in this PR
The plain Swap branch (around
plan.py:4135) can still stretch a window back to the previous window's end, past its own original start, into an empty gap. It stops at an adjacent manual window, so it can't reproduce #5423, and I've left it alone.🤖 Generated with Claude Code