fix(fetch): exclude saving-session/Axle boosted minutes from automatic rate thresholds (#5050)

[chalfontchubby]

🤖 This PR was written by Claude.

Fixes #5050

Replaces #5052 (closed). This is a clean reimplementation against current main, not a rebase.

Problem

A saving session or Axle VPP event raises the import and/or export rate by the event reward, and tags those minutes "saving". set_rate_thresholds() worked its thresholds out from rate_max/rate_min/rate_average, which include those boosted minutes. A two-rate tariff plus a large reward could then push the low-rate threshold above every real tariff rate, so the whole day read as "low rate". In #5050 that left binary_sensor.predbat_low_rate_slot on for about 24 hours.

Design

1. Thresholds use the tariff's own prices. rate_minmax_excluding_saving() caps each session minute down to the rate it would have without the session. User overrides on session minutes are kept. Capping only goes down: an event reward is removed, but a free or discounted session still counts as cheap. Both thresholds use these stats, in automatic and manual modes. rate_max/rate_min/rate_average themselves are unchanged, because dashboards, graph scaling and plan pricing want the real event price.

2. The plan can still charge ahead of an event (your option 2). In automatic mode, find_event_pre_charge() treats each run of consecutive session minutes (import or export) as one event. The event starts at its first minute whose export or import price is above the tariff's highest import rate. find_low_rate_windows() then gives the plan:

• every slot at a tariff import price before the last such event still to come. The scan covers only the minutes before the 5-minute step the event starts in, at the tariff's maximum + 0.1p, which is the rate_export_max > rate_max rule from Add automatic Octopus saving session support (beta) #249 limited to before the event;
• the tariff's own cheap windows from that step on.

Windows end at the event by construction. An earlier event whose import is raised is above the tariff maximum, so it is never offered as a charge window. Import-only sessions qualify, so a 0p export tariff still gets a pre-charge to cover the house through the session. A free session that runs straight into a saving session stays a cheap window before it. An event already running has nothing before it to charge in, so it doesn't count.

3. The sensors and car plans keep the tariff's own cheap windows. These go in a separate list, low_rates_tariff. It feeds binary_sensor.predbat_low_rate_slot, the predbat.low_rate_* sensors and car charging plans, because a car cannot export. With no qualifying event it is the same list object as low_rates, so ordinary days scan once, exactly as on main. The automatic threshold tightening also follows the tariff's own windows, so on event days the published threshold and the dispatch timeline do not read the day rate as cheap.

4. A manual rate_low_threshold is a cap. It is not widened for events, so with a manual threshold Predbat will not charge above it ahead of an event, even when that would pay. customisation.md and energy-rates.md say so.

compare runs the same window logic. Per side, it keeps the live session minutes when it reuses the live rates and clears them when the tariff replaces them. annual always installs a different tariff, so it clears them.

On your review case (7p/30p import, 15p export, +300p session at 17:30-18:30 on both sides, time 10:00):

	Plan windows before the session	Plan windows in total	Sensor windows
main	15	46	46
46f2828 (what you reviewed)	0	10	10
this head	15	27	12 (7p only)

The 19 windows main has and this head does not are day-rate slots after the session. The same case with 0p export, so the session is on import only, gives the same numbers.

Behaviour changes worth checking

• Manual threshold mode loses the pre-charge on event days. On main, the event inflated the average and so admitted day-rate slots. This is deliberate, see design point 4.
• Day-rate slots after an event are no longer charge candidates. On main the inflated threshold admitted the whole day.
• Car charging plans use the tariff's own cheap windows, not the windows widened for the event.
• Saving session distorts import-side slot selection, causing peak-rate car/battery charging #5237 (peak-rate top-up next to a session) is not addressed. Its session raised import above the tariff maximum, so the slots before it are offered again, as on main. The optimiser decides whether they pay.

Changes since your review of 46f2828

• Blocking point: option 2, as above.
• run_single_debug() resets the saving-session fields before reading a dump, and --redo uses find_low_rate_windows() as fetch does.
• The ordering test notes that it matches source text, so reformatting those lines fails it.
• The rate_minmax_excluding_saving() docstring is down to what it does. The review-history references are gone from the code comments and test docstrings.
• The pre-session snapshots now keep only their session minutes, so debug dumps no longer carry two extra full rate tables. Overrides are applied to the snapshot only when there are overrides to apply, which stops a log line on every cycle of a session. Fetch and compare apply them through one helper, override_session_rates().

The PV10 "dispatch gone" pricing change that was on this branch locally (the Axle reward inflating it, from #5392) has moved to its own PR, #5411, stacked on this one.

Testing

• test_set_rate_thresholds.py: 32 tests. The new ones cover:
  • your review case, with 15p and 0p export;
  • an export event, including a window that runs into the event and is cut at its start;
  • an event below the import price;
  • two events, where the earlier one is not offered as a window;
  • an event whose price dips, a free session running into a saving session, and an event starting off the 5-minute grid;
  • an event already in progress;
  • manual mode;
  • car planning, the sensor publishing, the session-minute snapshot, and a compare override applied to it.
• Mutation-tested: reverting each new piece of behaviour fails its test.
• ./run_all --quick and --test debug_cases green.
• ./run_pre_commit green.
• Reviewed in several rounds by independent fresh-context reviewers and /code-review high. Their correctness findings are fixed. Not done:
  • annual.py keeps its own window scan. It never has session minutes, so it behaves identically, and _apply_rates has a large blast radius.
  • compare.run_all() does not restore the new fields afterwards, the same as the existing low_rates.

Scope and future work

This covers tagged events only. The general case, where a price threshold hides a whole untagged period (e.g. a 20p morning / 90p afternoon export tariff), is #5221. The pre-event step here could extend to "export before a forecast clipping period" in the same way.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved issues remain around empty-rate handling, final automatic rescanning, and saving-event rate-base mapping; the reset test is also source-only.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Preserve IOG discounts when normalizing saving rates · New
• Test threshold state reset through actual execution paths · New

What changed in this PR

This PR updates automatic rate-threshold calculations to exclude saving-session and Axle reward boosts while preserving genuine discounts.

Changes:

• Adds saving-minute tracking and base-rate normalization.
• Resets stale saving metadata in simulated tariffs.
• Adds threshold regression tests and registers the suite.

File	Description
apps/​predbat/​unit_test.py	Registers the new test suite.
apps/​predbat/​tests/​test_set_rate_thresholds.py	Adds threshold and state-reset regression tests.
apps/​predbat/​predbat.py	Initializes saving-minute tracking state.
apps/​predbat/​fetch.py	Implements saving-aware threshold calculations.
apps/​predbat/​compare.py	Clears stale saving metadata.
apps/​predbat/​annual.py	Clears stale saving metadata.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Address the identified export regression coverage, test fixture isolation, and snapshot ordering gaps.

Review effort: Lite
Findings: 3

Open (3)

• Missing automatic export test for boosted tariff thresholds · New
• Missing automatic export saving-boost regression coverage · New
• Source-order test omits load_free_slot placement · New

Resolved since last review (2)

• Preserve IOG discounts when normalizing saving rates
• Test threshold state reset through actual execution paths

[chalfontchubby]

Posted by Claude on behalf of @chalfontchubby.

Summary: this PR stops saving-session / Axle event boosts from distorting the automatic rate thresholds. Those thresholds decide which slots count as low-rate, which export windows the planner is offered, and when non-smart car charging runs, so one root cause shows up as several different symptoms. It fixes #5050 and #5237, and for the Axle clipping case reported on #4036 it restores the export windows that were being hidden.

How it helps, issue by issue

Issue	Symptom	What this PR changes	Verified how
#5050	A big saving-session reward pushed the low-rate threshold above every real tariff rate, so binary_sensor.predbat_low_rate_slot stayed ON for about 24h	set_rate_thresholds() reads stats with event-tagged minutes capped back to their pre-event price	test_set_rate_thresholds.py, mutation-tested
#5237	Non-smart car charging started at the Go peak rate on a session day	The import threshold no longer inflates on event days (stays at 34.1p in the reporter's data), so the peak is not treated as a cheap slot	Code reading only: the attached dump was taken after the session ended
#4036 (Axle clipping report) and #5221	With an Axle export event on a flat 20p export tariff, only the two 120p event windows were offered to the planner, so the battery could not export ahead of the event and clipped	The event no longer widens the export min/max, so the tariff reads as flat again and the automatic export threshold becomes 19.9p instead of 20.5p. The ordinary 20p windows are offered as well, and the ratchet settles on 20p instead of latching on the event rate	Replay of the 4 Sep debug file and a new test, both below

Evidence for the Axle case

Replay of the 4 Sep debug file (v8.54.3 dump, --redo). The dump predates this PR, so it lacks the saving-minute state, and replay does not run fetch_sensor_data(). I rebuilt that state from the dump's own rate_export_replicated == "saving" tags and rate_export_base:

	main	this PR
Export threshold	20.5p	19.9p
Export windows offered	99-120p (event only)	20-120p
Import threshold	123.3p	34.5p
Plan metric	-1659.39	-1715.66

The metric is a cost, so lower is better: about 56p better for that day. Caveats: it is one file, the rebuild assumes rate_export_base holds the pre-event rates, and I did not check clipped kWh.

New test test_axle_event_on_flat_export_tariff_admits_ordinary_windows. It builds a +100p Axle export event on a flat 20p tariff through the real load_axle_slot() and runs the same sequence as fetch_sensor_data() (thresholds, window scan, ratchet). With this PR the threshold is 19.9p, the windows before the event are offered and the ratchet settles on 20p. A control run without the saving state reproduces main's 20.5p / event-only / ratchet-to-99 result, so the test would fail on the unfixed code. It mirrors the fetch.py sequence rather than calling fetch_sensor_data() itself; the existing source-order test guards the real ordering.

A correction to our earlier comment on #5221

On 24 Sep we said this PR gave a "correct" 20.5p threshold and that the ratchet then undid it. That was wrong: 20.5p is main's value. The check had called set_rate_thresholds() by hand on the old debug dump, which has none of the state this PR adds, so the fix never ran. Run properly, the threshold is 19.9p and the ratchet is not a separate problem for this case.

Behaviour change to be aware of

On an event day the automatic import threshold now drops, for example from 329.5p to 29.5p in the 7p/30p tariff with a +300p session. Ordinary day-rate slots are no longer charge candidates on those days, so the planner will not charge at the day rate just to export into the event. The cheap night slots stay candidates. This is described in the PR body under "Trade-off worth a maintainer's view".

Not covered

• Export-slot visibility that has nothing to do with events, for example a 20p morning / 90p afternoon tariff (Export threshold can hide whole periods from the planner: slot visibility, not cost #5221). A separate proposal to admit more export candidates ahead of predicted clipping is on Export threshold can hide whole periods from the planner: slot visibility, not cost #5221.
• The clipping headroom report PV clipping protection: proactively create battery headroom using PV forecast/PV90 #4865 is a different case (combine_export_slots, spiky cloud) and is not addressed here.
• rate_max / rate_min / rate_average are deliberately left as the real boosted prices, because dashboards, graphs and plan pricing want those.

The latest commit on the branch adds the test above and makes a session minute that has no rate in the underlying tariff (load_axle_slot() can create one) be ignored, since it would otherwise count the reward as a tariff rate. The branch is also rebased onto current main.

[chalfontchubby]

Posted by Claude on behalf of @chalfontchubby.

Another real-world case to add to the table in the summary comment above: #5344.

Issue	Symptom	What this PR changes	Verified how
#5344	Export threshold sat above the real tariff rate (12.18p vs a flat 12.0p) while an 18:00-19:00 saving session was still ahead in the forecast, dropping to exactly 12.0p the moment the session rolled past at 19:00	rate_export_average/rate_export_min would no longer include the session's uncapped reward, so the threshold would track the real tariff rate throughout	Log-verified against the reporter's attached debug/logs: confirmed rate_export reads 20.5p (12.0p tariff + 8.5p reward) for the session hour, and the logged "Rate thresholds" line tracks rate_export_average 1:1 (their rate_high_threshold is 1.0, manual mode)

This looks like a contributing factor to the "battery doesn't charge enough overnight while the car's on an IOG dispatch" shape reported there and on #5330 - not the whole story (there's a separate day-boundary and sort-tie-break issue we're tracking for those), but a clean, independently-confirmed hit on exactly the bug this PR describes.

Given #5050, #5237, #4036/#5221, and now #5344 all trace back to the same unfixed threshold-vs-session-reward interaction, this seems worth prioritising for review/merge.

[chalfontchubby]

Posted by Claude on behalf of @chalfontchubby.

Another real-world case to add to the table in the summary comment above: #5392.

Issue	Symptom	What this PR changes	Verified how
#5392	An Axle export event (+100p, 5 Oct 20:00-21:00) on IOG pushed rate_max to 132.25p and the automatic import threshold to 131.75p, so every 32.26p day slot was offered as a charge candidate	set_rate_thresholds() would read the pre-event 32.26p maximum, so the import threshold drops back to about 31.8p and the day-rate slots stop being charge candidates	Log-verified against the reporter's attached log: Rate thresholds (for charge/export) are import 131.75p at 20:35, with the tariff peaking at 32.26p (rate_import_no_io). Replayed with only the threshold corrected to 31.76p (everything else unchanged): the plan is identical, so in this case the inflated threshold had no effect on the outcome

The main #5392 symptom (the house battery idle during the car's overnight slots) is a separate rounding bug, fixed in #5396. But the event-inflated rate_max is what made that bug costly: the PV10 "dispatch goes away" scenario (prediction.py, dispatch_gone) prices a vanished dispatch at self.rate_max, which was 132.25p, so charging there looked prohibitively risky. A replay with only that worst-case price corrected to 32.26p (rounding bug still present) reaches 100% by 05:30 instead of 92%, though it still idles for half an hour of the car's slot.

This PR deliberately leaves self.rate_max as the real boosted price, so it won't change that worst-case pricing. That needs a follow-up: price a vanished dispatch at the minute's own rate without the dispatch (rate_import_no_io) rather than the day's maximum. It's another place where an event's reward leaks into a "what does a normal slot cost" calculation, alongside #5050, #5237, #4036/#5221 and #5344.

[springfall2008]

Posted by Claude on behalf of @springfall2008.

Thanks for the rework. I reviewed head 46f2828. The mechanism is sound and I have one concern that needs settling before merge, plus a few small points.

Blocking: no pre-charge before a same-day session

The PR changes which slots reach the optimiser's candidate lists, and on a session day that removes every day-rate charge window. Measured on this branch with a 7p/30p import tariff, 15p export, a +300p session at 17:30-18:30 and the time set to 10:00:

	Import threshold	Charge windows offered	Of which before the session
main	329.5p	46	15
this PR	29.5p	10	0

So when a session is announced after the night rate has passed, the planner has no charge window before it, and charging at 30p to export at 315p is no longer possible. A session known the day before is unaffected, because the night slot is still a candidate.

You raised this under "Trade-off worth a maintainer's view" and described it as a side effect of the inflated threshold. I read it as documented behaviour: docs/energy-rates.md says "If necessary, a pre-charge may happen at some point during the day to maintain the battery right level for the session."

A single threshold cannot express "cheap slots, plus anything before the event", so I see two ways forward:

1. Accept the loss and change the saving-session docs to say a pre-charge only happens from slots that are cheap on the tariff's own terms.
2. Also admit import windows that end before an export event whose rate beats the tariff's own maximum import rate. That keeps the Saving session / Axle VPP rate boost breaks automatic low-rate threshold - binary_sensor.predbat_low_rate_slot stuck ON, day rate reported as "low rate" #5050 and Saving session distorts import-side slot selection, causing peak-rate car/battery charging #5237 fixes for the rest of the day and keeps the pre-charge.

My preference is option 2, at the cost of a second mechanism beside the threshold. If you think it brings #5050 back in some form, or is not worth the complexity, say so and we can go with option 1.

What I checked and found sound

• Ordinary days are unchanged: with no tagged minutes the new scan covers the same window and rounding as rate_minmax().
• The cap only goes down, so saving sessions and Axle export events are capped back while free sessions and Axle import events keep their discount.
• Overrides on session minutes survive, and applying them to the snapshot a second time is idempotent for load_scaling.
• Compare and annual clear or keep the state per side as described. The explicit import_replaced / export_replaced flags resolve my earlier identity-check comment.
• The new set fields round-trip through a debug dump (!!set), so replaying a new session-day dump reproduces the live thresholds.
• ./run_all --quick passes locally on this head: 734 passed, 4 slow tests skipped. CI on the PR only ran pre-commit and kernel-binaries, so that local run is the only unit-test evidence.

Minor

• run_single_debug() resets rate_max_base and rate_export_max_forward because older dumps lack them. The four new fields need the same treatment, otherwise an old dump replayed with --redo on a reused fixture can pick up stale saving minutes.
• The ordering test finds exact source strings in fetch.py, so reformatting those lines fails it with no behaviour change. Worth a comment in the test saying so, if there is no better way to pin the order.
• The rate_minmax_excluding_saving() docstring is about 40 lines and cites review history ("fix(fetch): exclude saving-session/Axle boosted minutes from automatic rate thresholds (#5050) #5163 review", "fix(fetch): exclude saving-session/Axle boosted minutes from automatic rate thresholds (#5050) #5052 review"). Please cut it to what the function does and why; the history belongs here in the PR.
• The Saving session distorts import-side slot selection, causing peak-rate car/battery charging #5237 claim rests on code reading only, as you say. Fine to leave, but the PR should not be the thing that closes Saving session distorts import-side slot selection, causing peak-rate car/battery charging #5237 until someone confirms it.
• The branch is 44 commits behind main. It still merges cleanly.

[chalfontchubby]

Posted by Claude on behalf of @chalfontchubby.

Thanks for the review. You were right about the pre-charge. The rate_export_max > rate_max clause came in deliberately with #249, so calling it accidental in the PR body was wrong.

Blocking point: I've gone with your option 2. In automatic mode the plan gets every tariff slot before the last qualifying session or event still to come, and the tariff's own cheap windows after it. An event qualifies when its export or import price beats the tariff's highest import rate, so import-only sessions on 0p export count too. The event minutes themselves are never offered as charge windows. The low-rate sensors and car plans keep the tariff-only windows, so #5050 stays fixed.

On your case (7p/30p, 15p export, +300p at 17:30, time 10:00) the plan now has 15 windows before the session, the same as main (46f2828 had 0). In total it has 27, against main's 46: the difference is the day-rate slots after the session. A full plan replay of cases/predbat_debug_pre_saving1.yaml, with a +300p session injected at 18:00 and the battery nearly empty, gives these plan metrics (lower is better):

Session	main	46f2828	this head
Import and export	263.69	818.57	263.60
Import only (0p export)	590.36	818.64	592.67

One choice I'd like you to check: a manual rate_low_threshold is treated as a cap, so with a manual threshold there is no pre-charge above it, even when it would pay. The docs say so. If you'd rather manual mode also got the pre-event windows, it's a one-line gate.

Minor points:

• run_single_debug() now resets the saving-session fields before reading a dump.
• The ordering test has a note that it matches source text.
• The docstring is cut down, and the review history is gone from comments and test docstrings.
• Saving session distorts import-side slot selection, causing peak-rate car/battery charging #5237 is no longer claimed. Its session raised import above the tariff maximum, so the slots before it are offered again, as on main.
• The branch is still behind main and still merges cleanly. I haven't rebased, so the history you reviewed stays intact.

The PV10 "dispatch gone" price was inflated by the Axle reward in #5392. That fix has moved to its own PR, #5411, stacked on this one.

The PR description is rewritten to lead with the design.