diff --git a/apps/predbat/compare.py b/apps/predbat/compare.py
index e15fb1d78..d9c242aa7 100644
--- a/apps/predbat/compare.py
+++ b/apps/predbat/compare.py
@@ -27,6 +27,65 @@
# 3. Consider Octopus API key to access current tariff info and switch links
+# An empty 'config:' line means no overrides; any other non-mapping value can't be applied
+EMPTY_COMPARE_CONFIG = (None, "", [], {})
+
+
+def check_compare_list(compare_list):
+ """Split compare_list into the entries that can be compared and a message per skipped entry (#5472).
+
+ Each kept entry is a copy with its id and name as stripped strings, and its config either a
+ mapping of overrides or absent, so run_all() and fetch_config() can rely on that shape.
+ """
+ if not compare_list:
+ return [], []
+ if not isinstance(compare_list, list):
+ return [], ["compare_list must be a list of tariffs, each with an id and a name"]
+
+ valid = []
+ problems = []
+ seen_ids = set()
+ for index, entry in enumerate(compare_list):
+ problem = None
+ entry_id = ""
+ if not isinstance(entry, dict):
+ problem = "must be a dictionary with an id and a name"
+ else:
+ # Results, sensor names and the web page are all keyed by the id as a string, so an
+ # unquoted numeric id in apps.yaml must not reach them as an int
+ entry_id = "" if entry.get("id") is None else str(entry["id"]).strip()
+ # A numeric name would likewise break publish_data() building the friendly name
+ entry_name = "" if entry.get("name") is None else str(entry["name"]).strip()
+ if not entry_id:
+ problem = "requires a non-empty 'id'"
+ elif entry_id in seen_ids:
+ problem = "has a duplicate id '{}'".format(entry_id)
+ elif not entry_name:
+ problem = "('{}') requires a non-empty 'name'".format(entry_id)
+ elif entry.get("config") not in EMPTY_COMPARE_CONFIG and not isinstance(entry["config"], dict):
+ # run_all() and fetch_config() look overrides up by key; a list of them raised TypeError
+ # there and ended the whole comparison, and any other value was never a usable override
+ problem = "('{}') has a 'config' that isn't a set of 'setting: value' overrides".format(entry_id)
+ if problem:
+ problems.append("compare_list entry {} {}".format(index + 1, problem))
+ continue
+ seen_ids.add(entry_id)
+ checked = dict(entry, id=entry_id, name=entry_name)
+ if checked.get("config") in EMPTY_COMPARE_CONFIG:
+ checked.pop("config", None)
+ valid.append(checked)
+ return valid, problems
+
+
+def valid_compare_list(compare_list, log=None):
+ """Return the compare_list entries that can be compared, logging a warning for each one skipped (#5472)."""
+ valid, problems = check_compare_list(compare_list)
+ if log:
+ for problem in problems:
+ log("Warn: Compare, {}, skipping it".format(problem))
+ return valid
+
+
class Compare:
"""Tariff comparison engine.
@@ -483,12 +542,14 @@ def load_yaml(self):
try:
data = yaml.safe_load(f)
if data:
- self.comparisons = data.get("comparisons", {})
+ # str() keys: a numeric id saved before ids were normalised (#5472) would
+ # otherwise break publish_data() and never match its compare_list entry again
+ self.comparisons = {str(tariff_id): result for tariff_id, result in (data.get("comparisons") or {}).items()}
except yaml.YAMLError as exc:
self.log("Error loading comparisons: {}".format(exc))
if self.comparisons:
- compare_list = self.pb.get_arg("compare_list", [])
+ compare_list = valid_compare_list(self.pb.get_arg("compare_list", []))
self.select_best(compare_list, self.comparisons)
self.publish_data()
@@ -512,7 +573,8 @@ def publish_data(self):
if result:
cost = result.get("cost", 0)
- name = result.get("name", "")
+ # str(): a numeric name saved before names were normalised (#5472) must not crash startup
+ name = str(result.get("name", ""))
attributes = {
"friendly_name": "Compare " + name,
"state_class": "measurement",
@@ -539,7 +601,7 @@ def publish_only(self):
"""
Update HA sensors only
"""
- compare_list = self.pb.get_arg("compare_list", [])
+ compare_list = valid_compare_list(self.pb.get_arg("compare_list", []))
if not compare_list:
return
@@ -597,7 +659,7 @@ def run_all(self, debug=False, fetch_sensor=True):
Compare a comparison in prices across multiple energy tariffs and report results
take care not to destroy the state of the system for the primary settings
"""
- compare_list = self.pb.get_arg("compare_list", [])
+ compare_list = valid_compare_list(self.pb.get_arg("compare_list", []), log=self.log)
if not compare_list:
return
@@ -672,7 +734,8 @@ def run_all(self, debug=False, fetch_sensor=True):
try:
for tariff in compare_list:
- prior = results.get(tariff["id"], {})
+ tariff_id = tariff["id"]
+ prior = results.get(tariff_id, {})
if prior:
prior_date = prior.get("date", "")[:10]
if prior_date == today_date:
@@ -684,11 +747,11 @@ def run_all(self, debug=False, fetch_sensor=True):
else:
# First ever run for this tariff: start from actual midnight SoC
start_soc = soc_midnight_fallback
- self.log("Compare tariff {} starting SoC: {:.2f} kWh".format(tariff.get("id", ""), start_soc))
+ self.log("Compare tariff {} starting SoC: {:.2f} kWh".format(tariff_id, start_soc))
try:
result_data = self.run_single(tariff, rate_import_base, rate_export_base, end_record, debug=debug, fetch_sensor=fetch_sensor, car_charging_slots=save_car_charging_slots, start_soc=start_soc, io_adjusted_base=io_adjusted_base)
if result_data is not None:
- results[tariff["id"]] = result_data
+ results[tariff_id] = result_data
finally:
# Restore hardware settings after each tariff so overrides don't bleed into the next tariff;
# in a finally so a raised run_single() can't leave compare values applied to the live instance
diff --git a/apps/predbat/predbat.py b/apps/predbat/predbat.py
index 93508ee50..529ac9431 100644
--- a/apps/predbat/predbat.py
+++ b/apps/predbat/predbat.py
@@ -1070,6 +1070,15 @@ def update_car_manual_soc(self):
soc_next = min(soc_next, self.car_charging_limit[car_n])
self.expose_config("car_charging_manual_soc_kwh" + car_postfix, dp3(soc_next))
+ def run_comparison(self):
+ """Run the tariff comparison, clearing compare_active afterwards even if it raises (#5472)."""
+ # Without the finally, a crashed comparison leaves compare_active on and re-triggers itself every cycle
+ self.expose_config("compare_active", True)
+ try:
+ self.comparison.run_all()
+ finally:
+ self.expose_config("compare_active", False)
+
def update_pred(self, scheduled=True):
"""
Update the prediction state, everything is called from here right now
@@ -1431,9 +1440,7 @@ def update_pred(self, scheduled=True):
if self.comparison:
if (scheduled and self.minutes_now < RUN_EVERY) or self.get_arg("compare_active", False):
# Compare tariffs either when triggered or daily at midnight
- self.expose_config("compare_active", True)
- self.comparison.run_all()
- self.expose_config("compare_active", False)
+ self.run_comparison()
else:
# Otherwise just update HA sensors to prevent then expiring
self.comparison.publish_only()
diff --git a/apps/predbat/tests/test_compare.py b/apps/predbat/tests/test_compare.py
index ac7629f31..99d8ca640 100644
--- a/apps/predbat/tests/test_compare.py
+++ b/apps/predbat/tests/test_compare.py
@@ -19,8 +19,11 @@
"""
import inspect
+import os
+import tempfile
+from unittest.mock import MagicMock, patch
-from compare import Compare
+from compare import Compare, check_compare_list, valid_compare_list
from const import MINUTE_WATT
@@ -717,6 +720,216 @@ def _mock_plan_iboost_smart():
else:
print("PASS T26: only a tariff reusing the live import rates keeps the live dispatch markers")
+ # ------------------------------------------------------------------
+ # T27: valid_compare_list() keeps only the entries run_all() can compare -
+ # skipping, with a warning naming the entry's position, one that isn't a
+ # dict, has no id, repeats an earlier id, has no name or has a config:
+ # that is neither a mapping nor empty (None, "", [] and {} mean no
+ # overrides) - and turns a numeric id into a string (#5472 - run_all()
+ # used to index tariff["id"] directly, raising KeyError every cycle, and a
+ # numeric id still crashed publish_data() building the sensor name)
+ # ------------------------------------------------------------------
+ logged = []
+ entries = [
+ "not_a_dict",
+ {"name": "No Id Tariff"},
+ {"id": " ", "name": "Blank Id Tariff"},
+ {"id": "agile", "name": "Agile"},
+ {"id": "agile", "name": "Agile Again"},
+ {"id": "no_name"},
+ {"id": 2024, "name": "Numeric Id Tariff"},
+ {"id": " agile ", "name": "Padded Agile"},
+ {"id": "numeric_name", "name": 1234},
+ {"id": "blank_name", "name": " "},
+ {"id": "bad_config", "name": "Bad Config", "config": ["metric_battery_cycle"]},
+ {"id": "empty_config", "name": "Empty Config", "config": None},
+ {"id": "empty_list_config", "name": " Empty List Config ", "config": []},
+ {"id": "empty_string_config", "name": "Empty String Config", "config": ""},
+ {"id": "empty_dict_config", "name": "Empty Dict Config", "config": {}},
+ {"id": "false_config", "name": "False Config", "config": False},
+ {"id": "string_config", "name": "String Config", "config": "best_soc_min: 2.0"},
+ {"id": "mapping_config", "name": "Mapping Config", "config": {"best_soc_min": 2.0}},
+ ]
+ kept = valid_compare_list(entries, log=logged.append)
+ kept_ids = [entry["id"] for entry in kept]
+ skipped_positions = sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in logged)
+
+ if kept_ids != ["agile", "2024", "numeric_name", "empty_config", "empty_list_config", "empty_string_config", "empty_dict_config", "mapping_config"]:
+ print("ERROR T27: valid_compare_list() should keep only 'agile', '2024', 'numeric_name', the four empty-config entries and 'mapping_config' (ids as strings), got {}".format(kept_ids))
+ failed += 1
+ elif skipped_positions != [1, 2, 3, 5, 6, 8, 10, 11, 16, 17]:
+ print("ERROR T27: valid_compare_list() should warn for entries 1, 2, 3, 5, 6, 8 (padded duplicate id), 10 (blank name), 11, 16 and 17 (unusable config), got {}".format(logged))
+ failed += 1
+ elif not all("'config'" in msg for msg in logged if msg.split("compare_list entry ")[1].split(" ")[0] in ("11", "16", "17")):
+ print("ERROR T27: entries 11, 16 and 17 should be skipped for their config, got {}".format(logged))
+ failed += 1
+ elif kept[7].get("config") != {"best_soc_min": 2.0}:
+ print("ERROR T27: a mapping config: should be kept as the tariff's overrides, got {}".format(kept[7]))
+ failed += 1
+ elif any("config" in entry for entry in kept[3:7]):
+ print("ERROR T27: an empty 'config:' (None, [], \"\" or {{}}) should be dropped as no overrides, got {}".format(kept[3:7]))
+ failed += 1
+ elif kept[4]["name"] != "Empty List Config":
+ print("ERROR T27: a kept entry's name should be stripped, got {!r}".format(kept[4]["name"]))
+ failed += 1
+ elif kept[2]["name"] != "1234":
+ print("ERROR T27: a numeric name should become a string, or publish_data() can't build the friendly name, got {!r}".format(kept[2]["name"]))
+ failed += 1
+ elif kept[0] is entries[3] or entries[6]["id"] != 2024:
+ print("ERROR T27: valid_compare_list() must return copies, not rewrite the user's compare_list in place")
+ failed += 1
+ elif valid_compare_list(None) != [] or valid_compare_list([{"id": 0, "name": "Zero"}]) != [{"id": "0", "name": "Zero"}]:
+ print("ERROR T27: valid_compare_list() should accept a missing list and an id of 0")
+ failed += 1
+ elif check_compare_list("agile") != ([], ["compare_list must be a list of tariffs, each with an id and a name"]) or check_compare_list(5)[0] != []:
+ print("ERROR T27: a compare_list that isn't a list should give no entries and one problem, got {} / {}".format(check_compare_list("agile"), check_compare_list(5)))
+ failed += 1
+ else:
+ print("PASS T27: valid_compare_list() skips malformed entries with a positioned warning and normalises ids to strings")
+
+ # ------------------------------------------------------------------
+ # T28: run_all() skips an id-less compare_list entry and still compares the
+ # valid one after it - run against the real fixture with the expensive
+ # collaborators stubbed, so only the loop's own handling is exercised (#5472)
+ # ------------------------------------------------------------------
+ cmp = Compare.__new__(Compare)
+ cmp.pb = my_predbat
+ run_all_logged = []
+ cmp.log = run_all_logged.append
+ cmp.comparisons = {}
+ cmp.live_saving_state = None
+ run_single_ids = []
+ saved_comparisons = []
+ cmp.run_single = lambda tariff, *args, **kwargs: run_single_ids.append(tariff.get("id")) or {"date": "2026-10-11", "soc": 1.0, "soc_start": 1.0}
+ cmp.select_best = lambda compare_list, results: None
+ cmp.save_yaml = lambda: saved_comparisons.append(dict(cmp.comparisons))
+ cmp.publish_data = lambda: None
+ # The config: entries go through run_all()'s own override snapshot, which runs before any try
+ compare_list = [{"name": "No Id Tariff"}, "not_a_dict", {"id": "valid", "name": "Valid Tariff", "config": None}, {"id": "valid", "name": "Duplicate Tariff"}, {"id": 2024, "name": "Numeric Id Tariff"}, {"id": "list_config", "name": "List Config", "config": [{"metric_battery_cycle": 1}]}]
+ real_get_arg = my_predbat.get_arg
+
+ def fake_get_arg(key, default=None, *args, **kwargs):
+ """Return the test compare_list, deferring every other key to the real get_arg."""
+ if key == "compare_list":
+ return compare_list
+ return real_get_arg(key, default, *args, **kwargs)
+
+ try:
+ with patch.object(my_predbat, "get_arg", side_effect=fake_get_arg):
+ cmp.run_all(fetch_sensor=False)
+ run_all_error = None
+ except Exception as exc:
+ run_all_error = exc
+
+ if run_all_error is not None:
+ print("ERROR T28: run_all() raised {!r} on a compare_list with malformed entries".format(run_all_error))
+ failed += 1
+ elif run_single_ids != ["valid", "2024"]:
+ print("ERROR T28: only the valid tariffs should reach run_single(), ids as strings, got {}".format(run_single_ids))
+ failed += 1
+ elif not saved_comparisons or set(saved_comparisons[-1].keys()) != {"valid", "2024"}:
+ print("ERROR T28: only the valid tariffs' results should be persisted, keyed by string id, got {}".format(saved_comparisons))
+ failed += 1
+ elif sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in run_all_logged if "skipping it" in msg) != [1, 2, 4, 6]:
+ print("ERROR T28: run_all() should log a warning for each skipped entry (1, 2, 4 and 6), got {}".format(run_all_logged))
+ failed += 1
+ else:
+ print("PASS T28: run_all() skips malformed entries and compares and persists the valid ones by string id")
+
+ # ------------------------------------------------------------------
+ # T29: run_comparison() clears compare_active even when run_all() raises -
+ # otherwise the flag stays on and the crashed comparison re-triggers
+ # itself every 5-minute cycle (#5472)
+ # ------------------------------------------------------------------
+ exposed = []
+ real_expose_config = my_predbat.expose_config
+
+ def record_expose_config(name, value, *args, **kwargs):
+ """Record compare_active writes, then apply them for real."""
+ if name == "compare_active":
+ exposed.append(value)
+ return real_expose_config(name, value, *args, **kwargs)
+
+ prior_compare_active = my_predbat.get_arg("compare_active", False)
+ raising_comparison = MagicMock()
+ raising_comparison.run_all.side_effect = KeyError("id")
+ raised = False
+ with patch.object(my_predbat, "comparison", raising_comparison), patch.object(my_predbat, "expose_config", side_effect=record_expose_config):
+ try:
+ my_predbat.run_comparison()
+ except KeyError:
+ raised = True
+ compare_active_after = my_predbat.get_arg("compare_active", False)
+ # Shared fixture: put the flag back as the suite had it, so test order can't matter (#5079)
+ my_predbat.expose_config("compare_active", prior_compare_active)
+
+ if not raised:
+ print("ERROR T29: run_comparison() should let run_all()'s exception propagate")
+ failed += 1
+ elif exposed != [True, False]:
+ print("ERROR T29: compare_active should be set then cleared despite the exception, got {}".format(exposed))
+ failed += 1
+ elif compare_active_after:
+ print("ERROR T29: compare_active is still on after run_all() raised")
+ failed += 1
+ else:
+ print("PASS T29: run_comparison() clears compare_active even when run_all() raises")
+
+ # ------------------------------------------------------------------
+ # T30: load_yaml() turns a numeric result key saved before ids were
+ # normalised back into a string, so publish_data() can build the
+ # sensor name and the result still matches its compare_list entry (#5472)
+ # ------------------------------------------------------------------
+ cmp, pb = _make_compare()
+ published = []
+ pb.get_arg = lambda key, default=None: [{"id": 2024, "name": "Numeric Id Tariff"}] if key == "compare_list" else default
+ cmp.dashboard_item = lambda entity_id, **kwargs: published.append(entity_id)
+ with tempfile.TemporaryDirectory() as temp_dir:
+ with open(os.path.join(temp_dir, "comparisons.yaml"), "w") as f:
+ f.write("comparisons:\n 2024:\n name: Numeric Id Tariff\n cost: 1.0\n metric: 1.0\n")
+ cmp.config_root = temp_dir
+ try:
+ cmp.load_yaml()
+ load_error = None
+ except Exception as exc:
+ load_error = exc
+
+ if load_error is not None:
+ print("ERROR T30: load_yaml() raised {!r} on a numeric result key".format(load_error))
+ failed += 1
+ elif list(cmp.comparisons.keys()) != ["2024"] or not cmp.comparisons["2024"].get("best"):
+ print("ERROR T30: the numeric key should load as '2024' and be matched as the best tariff, got {}".format(cmp.comparisons))
+ failed += 1
+ elif published != ["predbat.compare_tariff_2024"]:
+ print("ERROR T30: expected sensor predbat.compare_tariff_2024 to be published, got {}".format(published))
+ failed += 1
+ else:
+ print("PASS T30: load_yaml() normalises a numeric result key to a string and publishes it")
+
+ # ------------------------------------------------------------------
+ # T31: publish_data() copes with a numeric name saved to comparisons.yaml
+ # before names were normalised - otherwise "Compare " + name raised
+ # TypeError inside Compare.__init__ and Predbat failed to start (#5472)
+ # ------------------------------------------------------------------
+ cmp, pb = _make_compare()
+ published = {}
+ cmp.dashboard_item = lambda entity_id, **kwargs: published.update({entity_id: kwargs.get("attributes", {})})
+ cmp.comparisons = {"numeric_name": {"name": 2024, "cost": 1.0}}
+ try:
+ cmp.publish_data()
+ publish_error = None
+ except Exception as exc:
+ publish_error = exc
+
+ if publish_error is not None:
+ print("ERROR T31: publish_data() raised {!r} on a saved numeric name".format(publish_error))
+ failed += 1
+ elif published.get("predbat.compare_tariff_numeric_name", {}).get("friendly_name") != "Compare 2024":
+ print("ERROR T31: expected friendly name 'Compare 2024', got {}".format(published))
+ failed += 1
+ else:
+ print("PASS T31: publish_data() publishes a saved numeric name as text")
+
if failed:
print("**** compare tests FAILED: {} errors ****\n".format(failed))
else:
diff --git a/apps/predbat/tests/test_web_apps_edit.py b/apps/predbat/tests/test_web_apps_edit.py
index 4cce66edf..e867c86d2 100644
--- a/apps/predbat/tests/test_web_apps_edit.py
+++ b/apps/predbat/tests/test_web_apps_edit.py
@@ -380,6 +380,22 @@ def run_web_apps_edit_tests(my_predbat):
print(" ERROR: expected the duplicate id to be refused, got: {}".format(result))
failed += 1
+ # ---------------------------------------------------------------------
+ print("Test: a compare profile with a blank id is refused (#5472)")
+ web_interface = _reset_fixture(my_predbat)
+ result = _post_changes(web_interface, {"compare_list[]#1": _add("compare_list[]", 'name: Blank\nid: " "\n'), "compare_list[]#2": _add("compare_list[]", "name: Clash\nid: one\n")})
+ if result.get("success") or "entry 4 requires a non-empty 'id'" not in result.get("message", "") or "entry 5 has a duplicate id 'one'" not in result.get("message", ""):
+ print(" ERROR: expected the blank id and the duplicate to be refused, naming both at once, got: {}".format(result))
+ failed += 1
+
+ # ---------------------------------------------------------------------
+ print("Test: a numeric compare profile id clashing with the same id quoted is refused, since both publish one sensor (#5472)")
+ web_interface = _reset_fixture(my_predbat)
+ result = _post_changes(web_interface, {"compare_list[]#1": _add("compare_list[]", "name: Numeric\nid: 7\n"), "compare_list[]#2": _add("compare_list[]", 'name: Quoted\nid: "7"\n')})
+ if result.get("success") or "duplicate id '7'" not in result.get("message", ""):
+ print(" ERROR: expected the numeric/quoted id clash to be refused as a duplicate, got: {}".format(result))
+ failed += 1
+
# ---------------------------------------------------------------------
print("Test: deleting a compare profile's id is refused, since results are indexed by it")
web_interface = _reset_fixture(my_predbat)
diff --git a/apps/predbat/tests/test_web_functions.py b/apps/predbat/tests/test_web_functions.py
index 45c0a8392..f3553d40c 100644
--- a/apps/predbat/tests/test_web_functions.py
+++ b/apps/predbat/tests/test_web_functions.py
@@ -10,6 +10,7 @@
import asyncio
import os
+from unittest.mock import MagicMock, patch
from web import WebInterface, is_data_numerical
from web_helper import get_plan_renderer_js
@@ -339,6 +340,37 @@ def run_compare_empty_state_tests(my_predbat, web):
print(f" ERROR: expected the genuine loading message when compare_list is set but not yet computed")
failed += 1
+ # -------------------------------------------------------------------------
+ print("Test: a numeric compare id finds its string-keyed result, and a malformed entry doesn't break the page (#5472)")
+ my_predbat.args["compare_list"] = ["not_a_dict", {"name": "No Id"}, {"id": 2024, "name": "Numeric Id Tariff"}]
+ fake_comparison = MagicMock()
+ fake_comparison.get_comparison.side_effect = lambda tariff_id: {"entity_id": "predbat.compare_tariff_2024", "html": "NUMERIC_ID_PLAN"} if tariff_id == "2024" else {}
+ try:
+ with patch.object(my_predbat, "comparison", fake_comparison):
+ text = asyncio.run(web.html_compare(None)).text
+ except Exception as exc:
+ text = None
+ print(f" ERROR: html_compare() raised {exc!r} on a compare_list with malformed entries")
+ failed += 1
+ if text is not None:
+ if "NUMERIC_ID_PLAN" not in text:
+ print(f" ERROR: expected the plan stored under '2024' to be shown for the numeric id")
+ failed += 1
+ if "compare_list entry 1 must be a dictionary" not in text or "compare_list entry 2 requires a non-empty 'id'" not in text:
+ print(f" ERROR: expected the page to say why entries 1 and 2 were skipped")
+ failed += 1
+
+ # -------------------------------------------------------------------------
+ print("Test: a compare_list with only malformed entries explains them instead of 'not configured' or a stuck loading message (#5472)")
+ my_predbat.args["compare_list"] = [{"name": "No Id"}]
+ text = asyncio.run(web.html_compare(None)).text
+ if "No tariffs configured yet" in text or "Loading chart (please wait)" in text:
+ print(f" ERROR: should not say 'not configured' or 'loading' when the configured entries are all malformed")
+ failed += 1
+ if "compare_list entry 1 requires a non-empty" not in text:
+ print(f" ERROR: expected the page to say why the only entry was skipped")
+ failed += 1
+
my_predbat.args = original_args
print("**** Compare empty state tests completed ****")
diff --git a/apps/predbat/web.py b/apps/predbat/web.py
index 613c33289..d3ab5c45a 100644
--- a/apps/predbat/web.py
+++ b/apps/predbat/web.py
@@ -96,6 +96,7 @@
from predbat import THIS_VERSION_DISPLAY
from component_base import ComponentBase
from config import APPS_SCHEMA
+from compare import check_compare_list
import debug_history
from web_annual import AnnualPage
from web_chat import WebChat
@@ -4081,23 +4082,13 @@ def _update_nested_yaml_value(self, data, path, value):
def _validate_compare_list(self, compare_list):
"""
Check every compare_list profile still has the unique id and non-empty name that
- compare.py indexes results by, raising ValueError if a batch has left one without
- """
- if not compare_list:
- return
-
- seen_ids = set()
- for entry in compare_list:
- if not isinstance(entry, dict):
- raise ValueError("Each compare_list entry must be a dictionary with an id and a name")
- entry_id = entry.get("id")
- if not entry_id:
- raise ValueError("Each compare_list entry requires a non-empty 'id'")
- if entry_id in seen_ids:
- raise ValueError(f"Duplicate compare_list id '{entry_id}'")
- seen_ids.add(entry_id)
- if not entry.get("name"):
- raise ValueError(f"compare_list entry '{entry_id}' requires a non-empty 'name'")
+ compare.py indexes results by, raising ValueError if a batch has left one without.
+ Uses compare.py's own rules, so the editor refuses exactly what a comparison would skip,
+ and names every problem at once so they can all be fixed in one go.
+ """
+ _, problems = check_compare_list(compare_list)
+ if problems:
+ raise ValueError("; ".join(problems))
async def html_apps_post(self, request):
"""
@@ -4452,7 +4443,9 @@ async def html_compare(self, request):
else:
cost_yesterday_no_car = cost_yesterday_hist
- compare_list = self.get_arg("compare_list", [])
+ # Same entries, with the same string ids, as the comparison itself ran on (#5472)
+ configured_compare_list = self.get_arg("compare_list", [])
+ compare_list, compare_problems = check_compare_list(configured_compare_list)
compare_hist = {}
for item in compare_list:
id = item.get("id", None)
@@ -4465,8 +4458,6 @@ async def html_compare(self, request):
compare_hist[id]["metric"] = history_attribute(entity_hist, state_key="metric", attributes=True, daily=True, pounds=True)
compare_hist[id]["cost_1am"] = self.history_daily_at_hour(entity_hist)
- compare_list = self.get_arg("compare_list", [])
-
for compare in compare_list:
name = compare.get("name", "")
id = compare.get("id", "")
@@ -4526,6 +4517,10 @@ async def html_compare(self, request):
text += "\n"
+ # The comparison only logs skipped entries when it runs, so say why a configured tariff is missing here
+ for problem in compare_problems:
+ text += "Skipped: {} - fix it in apps.yaml
\n".format(html_module.escape(problem))
+
# Charts
text += ''
series_data = []
@@ -4541,9 +4536,9 @@ async def html_compare(self, request):
if compare_hist:
text += self.render_chart(series_data, self.currency_symbols[0], "Tariff Comparison - True cost", now_str, daily_chart=False)
- elif not compare_list:
+ elif not configured_compare_list:
text += '
No tariffs configured yet - see Comparing Energy Tariffs for how to add some to apps.yaml
'
- else:
+ elif compare_list:
text += "
Loading chart (please wait)...
"
# 7-day rolling average chart
@@ -4560,7 +4555,7 @@ async def html_compare(self, request):
if series_7d:
text += self.render_chart(series_7d, self.currency_symbols[0], "Tariff Comparison - 7 day rolling average", now_str, tagname="chart7d", daily_chart=False)
elif not compare_list:
- pass # Already explained by the "No tariffs configured" message above
+ pass # Already explained above, by "No tariffs configured" or the skipped entries
else:
text += "
7 day rolling average chart loading (please wait)...
"