Skip to content

fix(plan): value left-over battery on the base tariff, not saving-session prices - #4488

Merged
springfall2008 merged 1 commit into
mainfrom
fix/battery-value-base-rates
Aug 12, 2026
Merged

fix(plan): value left-over battery on the base tariff, not saving-session prices#4488
springfall2008 merged 1 commit into
mainfrom
fix/battery-value-base-rates

Conversation

@springfall2008

Copy link
Copy Markdown
Owner

The problem

battery_value_rate() credits the SoC left at end_record. It ceilings that credit on rate_max and takes its export-recovery ratio from rate_export_max — both whole-horizon figures that include saving sessions. On a flat tariff running a session, that puts the credit above what discharging a stored kWh can actually realise, and the planner spends the difference: holding charge and importing instead scores as profit, so it freeze charges every window up to end_record while the battery sits nearly full.

A reported case (predbat_debug_bad_freeze_charge) had both halves at once:

rate_max        = 31.2   <- 29.2p tariff + 2.0p session bonus tomorrow 18:00-20:00
rate_max_base   = 29.2   <- the real tariff, perfectly flat
rate_export_max = 100    <- session expiring 19 minutes after minutes_now
                            (export pays 0p for the entire rest of the horizon)

The session bonus raised the ceiling; the expiring export event switched off the discount that would have caught it. Net: freeze charge worth +2.00p per kWh of load — exactly the session bonus — and 5 of 6 charge windows frozen.

Marginal arithmetic from the last window before end_record, straight out of the plan log:

cost battery_value metric
off 277.88 283.80 37.77
freeze 292.76 299.70 36.74

Freeze imports 0.51 kWh extra at 29.2p (+14.88p) and retains 0.547 kWh credited at 29.05p (+15.90p) — a scored gain of 1.03p, against an honest discharge worth of 27.19p/kWh.

The fix

Read the base tariff for both terms.

  • rate_max_base is already captured in rate_scan before sessions inflate rate_max
  • rate_export_max_forward is new: built by fetch from rate_export_base at the point that copy is taken, so sessions, axle slots and overrides applied afterwards stay out of it. Forward-looking like rate_min_forward, so an export price that has already passed stops counting

Both fall back to their whole-horizon equivalents when the base data is absent, so replaying an older debug file behaves exactly as before.

On the reported case the credit drops 29.05p → 21.75p per kWh, freeze charge goes to −5.84p per kWh of load, and no window freezes.

Why the base tariff is the right input

The discount asks "can I sell surplus?" — a property of the tariff, not of a 2-hour bonus event. A saving session doesn't make surplus generally sellable, so it shouldn't answer that question. rate_max_base / rate_min_base already exist on exactly this reasoning; this applies the same idea to export.

Scoping forward over the live export rates alone is not enough, and I tested that rather than assumed it: with a 100p session placed after end_record but inside the forecast, the recovery ratio saturates again and 4 of 5 windows re-freeze. Base-scoping holds in both placements.

Tests

  • test_rate_export_max_forward_calc.py — 7 cases mirroring the existing min-forward suite, including that a spike stops counting once passed, and that fetch feeds it base rather than session-inflated rates
  • test_compute_metric.py — the ceiling must come from rate_max_base under a session-inflated rate_max, and the recovery ratio must ignore a session sitting in rate_export_max; both cover the absent-base-data fallback. Existing tests updated to pin the new fields rather than inherit them from whatever ran before

Full suite green, pre-commit green across all files.

Note on test_single_debug.py

debug_cases failed on the first full-suite run while passing standalone. Not the fix — rate_max_base and rate_export_max_forward leak between tests on the shared fixture, and the planner consumes them now, so the agile1 replay picked up an unrelated test's rates. Both are reset alongside the dynamic_load_baseline resets already there for the same reason. Harness-only; fetch rebuilds both every cycle in the product.

Scope

This fixes the saving-session class, not the underlying invariant. Residual exposure is narrower than before: with the ceiling on rate_max_base, a genuinely flat tariff gives a freeze margin of R·(haircut − 1) ≤ 0 regardless of export rate — safe by construction rather than by luck. A spurious freeze now needs a real peak in the tariff and the plan's forward-minimum landing at or near it. The invariant fix — bounding the credit at max(forward import, forward export) × inverter_loss × battery_loss_discharge, what a stored kWh can actually realise — re-bases the credit for every user and wants a benchmark run behind it, so it is left as a follow-up.

🤖 Generated with Claude Code

…sion prices

battery_value_rate ceilinged the end-of-plan battery credit on rate_max and took its
export-recovery ratio from rate_export_max, both whole-horizon figures that include
saving sessions. On a flat tariff running a session that put the credit above what
discharging a stored kWh can realise, so the planner scored freeze charge as profit
and froze every window up to end_record while the battery sat nearly full.

A reported case had a flat 29.2p import with a 2p session bonus the next evening
(rate_max 31.2 vs rate_max_base 29.2) and a 100p export session expiring 19 minutes
after minutes_now (rate_export_max 100, export 0p for the rest of the horizon). The
session bonus raised the ceiling while the expiring export event switched the discount
off, leaving freeze charge worth +2.00p per kWh of load - exactly the session bonus -
and 5 of 6 charge windows frozen.

Read the base tariff for both terms instead. rate_max_base is already captured before
sessions inflate rate_max; rate_export_max_forward is new, built by fetch from
rate_export_base at the point that copy is taken, and forward-looking so an export
price that has passed stops counting. On the reported case the credit drops from
29.05p to 21.75p per kWh, freeze charge goes to -5.84p per kWh of load, and no window
freezes. Both terms fall back to their whole-horizon equivalents when the base data is
absent, so replaying an older debug file is unchanged.

run_single_debug now resets both fields alongside the existing dynamic_load_baseline
resets: the planner consumes them now, debug files written before they existed carry
neither, and inside the full suite they leaked from earlier tests into the replay.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 11, 2026 20:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an optimisation artifact where battery_value_rate() could over-credit end-of-plan battery SoC when saving sessions inflate the horizon-wide max import/export rates, making “freeze charge” appear falsely profitable on otherwise flat tariffs.

Changes:

  • Update battery_value_rate() to cap leftover SoC value using the session-free rate_max_base, and to compute the export-recovery ratio from a new forward-looking, base-tariff export max (rate_export_max_forward).
  • Compute and store rate_export_max_forward during fetch from base export rates (before saving sessions / overrides are applied), and reset it in core state resets.
  • Add focused unit tests for the new forward-export-max calculation and the new battery-value behaviour; harden debug replay harness to avoid cross-test leakage.

Reviewed changes

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

Show a summary per file
File Description
docs/customisation.md Documents that export recovery uses the forward-looking base tariff rate, not session spikes.
apps/predbat/unit_test.py Registers the new forward-export-max unit test in the test runner.
apps/predbat/tests/test_single_debug.py Resets new base-tariff fields to prevent fixture leakage across debug-case replays.
apps/predbat/tests/test_rate_export_max_forward_calc.py Adds a new test suite for rate_export_max_forward_calc() mirroring min-forward coverage.
apps/predbat/tests/test_compute_metric.py Pins and tests the new rate_max_base ceiling and forward base export recovery behaviour (plus fallbacks).
apps/predbat/predbat.py Initializes rate_export_max_forward during reset() to a safe default.
apps/predbat/plan.py Applies rate_max_base and rate_export_max_forward in battery_value_rate() to avoid session-driven overvaluation.
apps/predbat/fetch.py Computes rate_export_max_forward from rate_export_base before sessions/overrides are layered on; adds rate_export_max_forward_calc().

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

@springfall2008
springfall2008 merged commit 2adaa76 into main Aug 12, 2026
3 checks passed
@springfall2008
springfall2008 deleted the fix/battery-value-base-rates branch August 12, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants