Reduce metric_battery_value_scaling step to 0.01 and document the export risk - #4847
Merged
Merged
Conversation
…ort risk The setting's step was 0.1, but the margin that stops Predbat exporting the battery at a loss is the ratio of what exporting earns to what replacing the energy costs - only about 1% on a flat export tariff near the cheapest import rate (11.174p / 11.319p on the 10.54p import, 12p export case in #4840). One step below the 1.0 default therefore landed at 0.9, well past the ~0.987 flip point, so the planner force-exported the battery for a real loss of about 0.2p/kWh with no way for the user to nudge the setting gently. 0.02 is not enough either - 0.98 is still below the flip point - so the step becomes 0.01. Every value reachable on the old 0.1 step remains a valid multiple of the new one, so existing settings are unaffected, and all step consumers branch only on whether the step is integral, which is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is low-risk (config step + documentation) and is backed by a focused regression test that protects both the reported margin behavior and backward compatibility of existing user values.
Pull request overview
This PR addresses issue #4840 by making metric_battery_value_scaling adjustable at a finer granularity (so users can make small reductions without accidentally crossing the export/hold “flip point” on near-parity import/export tariffs) and by documenting the associated risk of “exporting at a loss” when the setting is reduced.
Changes:
- Reduce
metric_battery_value_scalingconfig step from0.1to0.01to allow safe incremental adjustments below the1.0default. - Document the near-parity tariff risk in the customisation docs (why lowering this setting can trigger loss-making exports).
- Add a regression/schema self-check ensuring (a) the first step below default stays above the worked flip point and (b) all legacy
0.1-step values remain representable.
File summaries
| File | Description |
|---|---|
apps/predbat/config.py |
Changes metric_battery_value_scaling step to 0.01 to improve configurability without altering planner logic. |
docs/customisation.md |
Adds a warning explaining why lowering the scaling below ~0.99 can cause apparent “random” exports on near-parity tariffs. |
apps/predbat/tests/test_integer_config.py |
Adds a regression test asserting the step granularity avoids overshooting the export margin and preserves legacy step values. |
apps/predbat/unit_test.py |
Registers the new regression test in the unit test harness. |
.claude/skills/issue-triage/references/debug-journal.md |
Documents the triage finding and adds a symptom→setting lookup hint for future investigations. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 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.
This is an automated draft PR generated from issue #4840 — a maintainer should review it before merging.
Relates to #4840
Summary
#4840 reported Predbat force-exporting the battery at peak import times on a flat 12p export tariff. Triage confirmed the cause was the reporter's
metric_battery_value_scaling: 0.9(default 1.0), not a planner defect — but the setting's granularity made that outcome hard to avoid.compute_metric()credits leftover SoC atbattery_value_rate(), the replacement cost of a stored kWh. Exporting wins as soon as that credit drops below what exporting earns, so the entire protective margin is:The step was 0.1, so the smallest possible reduction below the default landed at 0.9 — 7.8x past the flip point — and the planner churned the battery for a real loss of ~0.2p/kWh. 0.02 is not sufficient either, since 0.98 is still below 0.9872; this PR uses 0.01, where the first nudge (0.99) stays clear.
Changes:
apps/predbat/config.py—metric_battery_value_scalingstep0.1->0.01.docs/customisation.md— warn that reducing this below 1.0 on a tariff whose export rate is close to the cheapest import rate makes Predbat export at a loss, and to check it first when exports look unexplained.apps/predbat/tests/test_integer_config.py— new schema self-check asserting one step below the default stays clear of the flip point, and that every value reachable on the old 0.1 step is still a valid multiple of the new one..claude/skills/issue-triage/references/debug-journal.md— record the finding under "Check configuration before code" plus a symptom-table row.Compatibility: every 0.1-multiple is a 0.01-multiple, so existing user values stay valid. All
stepconsumers (ha.py:987,userinterface.py:460,userinterface.py:1050,agent_tools.py:1133) branch only on whether the step is integral, which is unchanged — it was fractional before and remains so.Testing
tools/triage_test.sh metric_battery_value_scaling_step— passes. It initially failed against 0.02, which is what surfaced that 0.02 was insufficient.integer_config,expose_config_integer,config_item_range_clamp,config_item_step_min_max_types,get_ha_config_fractional_default,metric_battery_cycle_fractional../run_pre_commit— passes (exit 0), including the--quicksuite: All tests passed (4 slow tests skipped).Notes
The behaviour was confirmed by replaying the reporter's debug snapshot against
main, one setting at a time. Baseline reproduces the export exactly (force export to 51% in the 17:00–17:30 window, plan cost-937.5024):metric_battery_value_scaling= 1.0 (default)metric_cloud_enable= true (default)metric_battery_cycle= 1.0metric_pv_calibration_enable= true (default)export_more_solar= false (default)Worth noting the export plan's cost looks ~52p better only because it ends the horizon with 4.7kWh less in the battery — 11.12p/kWh realised against an 11.32p/kWh replacement cost.
This PR does not change any planner behaviour; it only makes the setting adjustable at a useful granularity and documents the trap. The separate observation that the reporter also had
metric_cloud_enabledisabled (default on), which independently removed the export, is left on the issue rather than changed here.