feat(plan): split the Import p cell when the car's own rate diverges - #4647
Merged
Conversation
On Octopus Intelligent Go, a car's own low-rate dispatch allowance is capped independently of the house rate - once used up, the car falls back to the peak rate while the house keeps its real (possibly still cheap) rate for the same clock-time. The plan previously only ever showed the house rate, with no way to see that the car was actually paying more for the same slot. Adds car_charge_slot_rate() (kWh-weighted average rate across any car charging windows active in a slot) and splits the Import p cell - house on the left, car on the right, each independently coloured and with its own tooltip - whenever the two diverge. Server HTML and the client-side JS renderer both updated; editable/override mode is left untouched since overrides apply to the single household rate. Fixes #4646. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved HTML escaping, misleading tooltip, and historical rate fallback issues remain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds separate household and car import-rate display when charging rates diverge.
Changes:
- Calculates weighted car charging rates and adds related JSON fields.
- Splits server/client Import cells with separate tooltips.
- Adds regression coverage and fixes test isolation.
File summaries
| File | Review summary |
|---|---|
apps/predbat/web_helper.py |
Moderate: avoid implying an IOG cap whenever rates merely differ. |
apps/predbat/tests/test_plan_json_rate_adjust.py |
Adds coverage for divergent rates and split rendering. |
apps/predbat/tests/test_octopus_slots.py |
Makes car-count setup explicit for test isolation. |
apps/predbat/plan.py |
Moderate: historical reconstructed slots need a household-rate fallback instead of zero. |
apps/predbat/output.py |
Critical: escape user-configurable tooltip content; moderate: avoid misleading cap wording. |
Review details
Suppressed comments (2)
apps/predbat/tests/test_plan_json_rate_adjust.py:150
- With the default
plan_interval_minutesof 30,minutes_nowis only aligned to 5 minutes, whilepublish_html_plan()emitsslot_minutevalues aligned to 30. Unless the test runs exactly on a half-hour, this lookup returnsNone, prints only a warning, and skips all the new assertions, so the test can pass without covering the feature. Aligncar_minuteto the plan interval (or select the row containing it) and fail if the row is missing.
car_minute = my_predbat.minutes_now
my_predbat.car_charging_slots[0] = [{"start": car_minute, "end": car_minute + 30, "kwh": 3.0, "average": 28.0, "cost": 84.0, "soc": 0.0, "octopus": True}]
html_plan, raw_plan = my_predbat.publish_html_plan(pv_step, pv_step, load_step, load_step, my_predbat.end_record, publish=False)
car_row = next((row for row in raw_plan["rows"] if row.get("slot_minute") == car_minute), None)
apps/predbat/web_helper.py:6533
- The added test checks the server-generated
html_plan, but it never validates this read-onlyrenderPlanTable()branch. A mismatch in the newrate_split/car_ratefields or a client-sidetoFixed()failure would therefore pass the test suite while the History/Baseline renderer is broken. Add a renderer-level assertion or browser test for both a split row and a non-split row.
} else if (row.rate_split) {
// Car's own rate has diverged from the house rate (IOG dispatch cap reached) -
// split the cell, house on the left, car on the right, each with its own tooltip.
const houseTitle = escapeAttr(`House rate: ${row.import_rate.toFixed(2)}${currencyMinor}/kWh`);
const carTitle = escapeAttr(`Car rate: ${row.car_rate.toFixed(2)}${currencyMinor}/kWh (IOG dispatch cap reached)`);
- Files reviewed: 5/5 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- HTML-escape the split cell's title attributes (currency_symbols is user-configurable free text; unescaped it could break out of the attribute in the server-rendered plan - the client JS path already used escapeAttr()). - Reword the car tooltip from "IOG dispatch cap reached" to "differs from house rate" - any car window with its own average can diverge, not just an IOG cap (e.g. combined dynamic-rate windows), so the original wording asserted a cause the data doesn't actually confirm. - car_charge_slot_rate() now skips windows with no "average" key instead of treating them as 0p/kWh. Non-Octopus historical reconstruction (Yesterday view) appends car-energy-sensor slots with no rate at all - defaulting to 0 dragged the weighted average down and falsely flagged ordinary charging as diverging from house rate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
Author
|
Addressed in efb0e35:
|
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
car_charge_slot_rate()(kWh-weighted average rate across any car charging windows active in a slot) and splits the Import p cell - house on the left, car on the right, each independently coloured and with its own hover tooltip - whenever the two diverge by more than 0.01. Server HTML and the client-side JS renderer are both updated; editable/override mode is left untouched since overrides apply to the single household rate.Relationship to #4644
Independent of #4644 (the IOG daily-cap slot-splitting fix) - verified no file/line overlap and no shared commit history, so either can merge first or standalone. This one just displays whatever rate is already present in
car_charging_slots, regardless of which version of that calculation produced it.Test plan
./run_all --quickpassestest_plan_json_rate_adjust.pycovers both the JSON fields (car_rate,rate_split,car_rate_color) and the rendered split-cell HTML with both tooltipsrun_pre_commitcleanrun_load_octopus_slots_tests()never setnum_carsitself, silently depending on whatever a prior test in the same run left it as🤖 Generated with Claude Code