diff --git a/.cspell/custom-dictionary-workspace.txt b/.cspell/custom-dictionary-workspace.txt index 97d1da7a4..946759ba6 100644 --- a/.cspell/custom-dictionary-workspace.txt +++ b/.cspell/custom-dictionary-workspace.txt @@ -321,6 +321,7 @@ OCPP octo octoplus octopoints +Ofgem Ohme ohmepy Okabe diff --git a/apps/predbat/annual.py b/apps/predbat/annual.py index 01ad144f2..8dbdce9f9 100644 --- a/apps/predbat/annual.py +++ b/apps/predbat/annual.py @@ -25,6 +25,7 @@ from annual_weather import AnnualWeather, resolve_postcode from const import MINUTE_WATT, PREDICT_STEP from prediction import Prediction +from tariff_catalogue import PRICE_CAP_IMPORT_P, SEG_EXPORT_P VALID_SHAPES = ["night", "day", "flat"] @@ -215,24 +216,36 @@ def _validate_load(raw): } -def _validate_tariff(raw): - """Normalise the tariff block, requiring at least one import rate source. +# What a household with no PV and no battery actually pays: the Ofgem price cap for +# import, and a typical fixed Smart Export Guarantee rate for the export they cannot +# have without generation. Rates come from tariff_catalogue so the cap figure is stated +# in exactly one place. +DEFAULT_BASELINE_TARIFF = {"rates_import": [{"rate": PRICE_CAP_IMPORT_P}], "rates_export": [{"rate": SEG_EXPORT_P}]} + + +def _validate_tariff(raw, path="annual.tariff"): + """Normalise a tariff block, requiring at least one import rate source. A URL containing {dno_region} with no dno_region supplied is rejected here rather than left to 404 at fetch time, where it would surface as an unavailable month and read like an Octopus outage. + + ``path`` names the block being validated, because two of them come through here: + the main tariff and ``baseline_tariff``. Hard-coded messages sent every baseline + problem to "annual.tariff", telling the user to fix a block that was perfectly + valid - the one thing an error message must never do. """ if not isinstance(raw, dict): - raise AnnualConfigError("annual.tariff is required and must be a mapping") + raise AnnualConfigError("{} is required and must be a mapping".format(path)) if not raw.get("import_octopus_url") and not raw.get("rates_import"): - raise AnnualConfigError("annual.tariff requires either import_octopus_url or rates_import") + raise AnnualConfigError("{} requires either import_octopus_url or rates_import".format(path)) templated = [name for name in ["import_octopus_url", "export_octopus_url"] if raw.get(name) and "{dno_region}" in raw[name]] if templated and not raw.get("dno_region"): - raise AnnualConfigError("annual.tariff.{} uses {{dno_region}} but annual.tariff.dno_region is not set; supply your Octopus region letter, for example 'A' for Eastern England".format(", ".join(templated))) + raise AnnualConfigError("{path}.{fields} uses {{dno_region}} but {path}.dno_region is not set; supply your Octopus region letter, for example 'A' for Eastern England".format(path=path, fields=", ".join(templated))) tariff = dict(raw) - tariff["standing_charge_p_per_day"] = _require_number(raw.get("standing_charge_p_per_day", 0.0), "annual.tariff.standing_charge_p_per_day", minimum=0) + tariff["standing_charge_p_per_day"] = _require_number(raw.get("standing_charge_p_per_day", 0.0), "{}.standing_charge_p_per_day".format(path), minimum=0) return tariff @@ -297,6 +310,12 @@ def validate_config(config, today=None): "battery": battery, "load": _validate_load(raw.get("load")), "tariff": _validate_tariff(raw.get("tariff")), + # The counterfactual bill is what the household would pay with no system at all, + # and such a household is not on a battery tariff: the smart import tariffs are + # only worth having once you have something to shift load into. Pricing the + # no-PV/battery scenario on the same tariff as the battery scenarios therefore + # understates what the system is worth. Defaults to the Ofgem price cap. + "baseline_tariff": _validate_tariff(raw.get("baseline_tariff") or DEFAULT_BASELINE_TARIFF, path="annual.baseline_tariff"), "samples_per_month": samples_per_month, "costs": _validated_costs(raw.get("costs")), "debug": _coerce_bool(raw.get("debug", False)), @@ -1079,7 +1098,7 @@ def _capture_plan(predbat, pv_step, pv_step10, load_step, load_step10, end_recor return raw_plan -def _run_scenarios(predbat, config, weather, tariff, load_source, day, midnight_utc, car_kwh, car_rate_kw, plans=None): +def _run_scenarios(predbat, config, weather, tariff, load_source, day, midnight_utc, car_kwh, car_rate_kw, plans=None, baseline_tariff=None): """Run all four scenarios against one sampled day at a fixed car charging energy. ``car_kwh`` is the actual energy this leg charges - either a full weekly charging @@ -1119,11 +1138,32 @@ def _run_scenarios(predbat, config, weather, tariff, load_source, day, midnight_ predbat.charge_window_best = [] predbat.export_window_best = [] predbat.export_limits_best = [] + # Price the counterfactual on its OWN tariff. A household with no PV and no battery is + # not on a smart import tariff - those only pay off once there is something to shift + # load into - so charging the baseline at the battery tariff's overnight rate credits + # it with a saving it could never have had, and understates the system. + # + # The rates are restored immediately afterwards, from the tariff's own output rather + # than from predbat.rate_import: _apply_rates REPLICATES what it is given, so reusing + # the installed dict would re-replicate an already-replicated series. Every later + # scenario in this leg therefore runs on exactly what prepare_sample() installed. + if baseline_tariff is not None: + baseline_import, baseline_export = baseline_tariff.rates_for(midnight_utc, PLAN_MINUTES) + _apply_rates(predbat, baseline_import, baseline_export) + + # Constructed AFTER the rate swap, never before: Prediction snapshots the rates off + # predbat at construction time (prediction.py, `self.rate_import = base.rate_import`), + # so a Prediction built first would be billed at the main tariff no matter what was + # installed afterwards - the swap would appear to work and change nothing. predbat.prediction = Prediction(predbat, zero_step, zero_step, load_step, load_step, soc_kw=0, soc_max=0) results["no_pvbat"] = _billed_result(predbat, DAY_MINUTES, zero_step) if plans is not None: plans["no_pvbat"] = _capture_plan(predbat, zero_step, zero_step, load_step, load_step, DAY_MINUTES) + if baseline_tariff is not None: + main_import, main_export = tariff.rates_for(midnight_utc, PLAN_MINUTES) + _apply_rates(predbat, main_import, main_export) + # Scenario 1b: PV but no battery. apply_hardware gives the array its real inverter # and export limits - a PV-only system still has an inverter, and clipping matters - # while soc_max=0 leaves it with nowhere to store surplus, so everything the house @@ -1223,7 +1263,7 @@ def _blend_results(with_car, without_car, fraction): return {key: {field: fraction * with_car[key][field] + (1 - fraction) * without_car[key][field] for field in SCENARIO_FIELDS} for key in SCENARIO_KEYS} -def run_day(predbat, config, weather, tariff, load_source, day, midnight_utc, plans=None): +def run_day(predbat, config, weather, tariff, load_source, day, midnight_utc, plans=None, baseline_tariff=None): """Run all four scenarios against one sampled day and return their billed figures. A configured car charges in weekly sessions, not a daily smear (see @@ -1245,7 +1285,7 @@ def run_day(predbat, config, weather, tariff, load_source, day, midnight_utc, pl if car_charging_kwh <= 0: leg_plans = {} if plans is not None else None - result = _run_scenarios(predbat, config, weather, tariff, load_source, day, midnight_utc, car_kwh=0.0, car_rate_kw=car_rate_kw, plans=leg_plans) + result = _run_scenarios(predbat, config, weather, tariff, load_source, day, midnight_utc, car_kwh=0.0, car_rate_kw=car_rate_kw, plans=leg_plans, baseline_tariff=baseline_tariff) if plans is not None: plans.append({"leg": "single", "scenarios": leg_plans}) return result @@ -1257,12 +1297,12 @@ def run_day(predbat, config, weather, tariff, load_source, day, midnight_utc, pl sessions_per_week, session_kwh = car_charging_schedule(car_charging_kwh, car_rate_kw) with_car_plans = {} if plans is not None else None - with_car = _run_scenarios(predbat, config, weather, tariff, load_source, day, midnight_utc, car_kwh=session_kwh, car_rate_kw=car_rate_kw, plans=with_car_plans) + with_car = _run_scenarios(predbat, config, weather, tariff, load_source, day, midnight_utc, car_kwh=session_kwh, car_rate_kw=car_rate_kw, plans=with_car_plans, baseline_tariff=baseline_tariff) if plans is not None: plans.append({"leg": "with_car", "scenarios": with_car_plans}) without_car_plans = {} if plans is not None else None - without_car = _run_scenarios(predbat, config, weather, tariff, load_source, day, midnight_utc, car_kwh=0.0, car_rate_kw=car_rate_kw, plans=without_car_plans) + without_car = _run_scenarios(predbat, config, weather, tariff, load_source, day, midnight_utc, car_kwh=0.0, car_rate_kw=car_rate_kw, plans=without_car_plans, baseline_tariff=baseline_tariff) if plans is not None: plans.append({"leg": "without_car", "scenarios": without_car_plans}) @@ -1423,6 +1463,9 @@ async def run(self, progress=None): self.caveats.append("Months {} had too few forecast/actual day pairs, so their P10 used the flat {} derate.".format(sorted(self.weather.fallback_months), self.config["pv10_derate_fallback"])) if has_solar: self.caveats.append("The forecast-versus-ERA5 gap includes systematic model bias as well as forecast error, so measured solar uncertainty is slightly overstated.") + self.caveats.append( + "The no_pvbat counterfactual is priced on its own baseline tariff, since a household with no PV or battery would not be on a battery tariff. Both scenarios still use ONE standing charge - the main tariff's - so any difference in standing charge between the two tariffs is NOT included in the reported savings or payback." + ) self.caveats.append("export_credit_p_estimate is money ALREADY included inside cost_p (which prices every export minute at its real rate); it is informational only - adding it to cost_p double-counts export income.") self.caveats.append( "The without_predbat baseline charges in the single cheapest contiguous band of each day, mirroring Predbat's own savings baseline. On a half-hourly tariff such as Agile the cheapest band is often one 30 minute slot, so the baseline is a more pessimistic comparator there than on a banded tariff (Economy 7, Cosy, Flux) where it covers the whole cheap period. Compare predbat_vs_baseline_p across tariffs with that in mind." @@ -1439,7 +1482,11 @@ async def run(self, progress=None): self.predbat = create_headless_predbat(self.work_dir, self.config["timezone"], self.log) self.load_source = await self._build_load_source() self.tariff = AnnualTariff(self.config["tariff"], log=self.log, predbat=self.predbat, storage=self.storage, timezone=self.config["timezone"]) + # Prices the no-PV/battery counterfactual only. Its own AnnualTariff because it + # may be a completely different product with its own rate downloads and cache. + self.baseline_tariff = AnnualTariff(self.config["baseline_tariff"], log=self.log, predbat=self.predbat, storage=self.storage, timezone=self.config["timezone"]) + baseline_fallback_months = [] zone = pytz.timezone(self.config["timezone"]) months = [] total_units = 12 @@ -1452,6 +1499,11 @@ async def run(self, progress=None): days_in_month = calendar.monthrange(year, month)[1] standing_charge_p = self.tariff.standing_charge_p_per_day * days_in_month + baseline_ready = await self.baseline_tariff.fetch_month(year, month) + if not baseline_ready: + # Falls back to the main tariff for this month rather than losing it, but + # that silently changes what no_pvbat means, so it is recorded. + baseline_fallback_months.append(month) if not await self.tariff.fetch_month(year, month): months.append({"month": month, "status": "unavailable", "reason": "no rate data available", "days": days_in_month, "standing_charge_p": standing_charge_p}) completed += 1 @@ -1478,7 +1530,7 @@ async def run(self, progress=None): midnight_utc = zone.localize(datetime(day.year, day.month, day.day)).astimezone(pytz.utc) day_plans = [] if self.config["debug"] else None try: - result = run_day(self.predbat, self.config, self.weather, self.tariff, self.load_source, day, midnight_utc, plans=day_plans) + result = run_day(self.predbat, self.config, self.weather, self.tariff, self.load_source, day, midnight_utc, plans=day_plans, baseline_tariff=self.baseline_tariff if baseline_ready else None) except Exception as exc: # noqa: BLE001 - one bad sample must not abort the whole year self.log("Warn: Annual: {} in month {} failed to plan/cost ({}: {}); excluding it from this month's total".format(day.isoformat(), month, type(exc).__name__, exc)) failed_days.append(day.isoformat()) @@ -1535,6 +1587,13 @@ async def run(self, progress=None): self.caveats.extend(self._tariff_fallback_caveats(self.tariff.fallback_months, self.tariff.unpaid_export_months, year)) + if baseline_fallback_months: + # Falling back to the main tariff keeps the month rather than losing it, but it + # silently changes what no_pvbat means there, so it has to be said out loud. + self.caveats.append( + "No baseline-tariff rates were available for month(s) {}, so the no-PV/battery counterfactual there was priced on the main tariff instead, which understates what the system is worth in those months.".format(sorted(baseline_fallback_months)) + ) + if progress: progress(total_units, total_units, "Complete") diff --git a/apps/predbat/annual_store.py b/apps/predbat/annual_store.py index 6e2560cc2..8da298a6b 100644 --- a/apps/predbat/annual_store.py +++ b/apps/predbat/annual_store.py @@ -109,7 +109,19 @@ def build_summary(results, config): # it is repaying. None rather than 0 when the run predates costs entirely. "total_gbp": costs.get("total_gbp"), "quoted": bool(costs.get("pv_quoted") or costs.get("battery_quoted")), - "tariff": _describe_tariff((config or {}).get("tariff") or {}), + # The tariff dicts themselves, not a rendered name. Naming one needs the merged + # catalogue, which includes the user's own compare_list entries, and only the web + # layer can read those - a name baked in here would be blind to them and would + # also freeze at whatever the catalogue said on the day the run was saved. Both + # are small: a URL, or a handful of {"rate": n} entries. + # + # Copied, so a later edit to the live configuration cannot reach back and rewrite + # what a stored run says it used. + "tariff": copy.deepcopy((config or {}).get("tariff") or {}), + # What the no-PV/battery scenario was priced on. Carried separately because it is + # a genuinely different tariff to the one the modelled system runs on, and a + # saving quoted against an unnamed baseline cannot be checked. + "baseline_tariff": copy.deepcopy((config or {}).get("baseline_tariff") or {}), "cost_with_predbat_p": predbat, "saving_vs_none_p": (baseline - predbat) if (baseline is not None and predbat is not None) else None, "payback_years": payback_years, @@ -276,22 +288,37 @@ async def load_plan(storage, run_id, month, index, scenario): return None +def _summary_is_current(summary): + """Return True when a summary carries the fields the compare table reads today. + + Presence alone is not enough. A run stored before the compare table named three + tariffs has a summary, but it holds a single pre-rendered import name and neither + tariff dict - so those rows would show dashes forever despite the run's own document + holding the answer. ``baseline_tariff`` is the marker because ``build_summary`` + always writes it, even as an empty dict, so it is present exactly when the summary + was written by the current code. + """ + return isinstance(summary, dict) and "baseline_tariff" in summary + + async def backfill_summaries(storage, runs): - """Return ``runs`` with any missing summary filled in, writing the index back once. + """Return ``runs`` with any missing or outdated summary rebuilt, writing the index back once. - A run stored before summaries existed has none. Rather than re-reading its document - on every visit to the compare page - several megabytes for a debug run - its summary - is computed once and persisted, so every later visit is index-only. + A run stored before summaries existed has none; one stored before the tariff fields + has a summary that predates them. Both are rebuilt from the run's own document. + Rather than re-reading that document on every visit to the compare page - several + megabytes for a debug run - the result is persisted, so every later visit is + index-only. The index is written at most once per call, and only when something was actually - filled in: a compare page that changes nothing must not write storage at all. + rebuilt: a compare page that changes nothing must not write storage at all. """ if not storage or not runs: return runs or [] filled = False for entry in runs: - if entry.get("summary") or not entry.get("id"): + if _summary_is_current(entry.get("summary")) or not entry.get("id"): continue results = await load_run(storage, entry["id"]) if not isinstance(results, dict): @@ -309,3 +336,28 @@ async def backfill_summaries(storage, runs): if filled: await storage.save(STORAGE_MODULE, INDEX_NAME, runs, format="json") return runs + + +async def delete_run(storage, run_id): + """Remove one run entirely - its document, its captured plans and its index entry. + + Reuses the same discard path eviction uses, so a run the user deletes leaves no more + behind than one that aged out of the ring: the document and every plan key are + expired, not merely unlinked from the index. Storage has no ``delete`` method on the + real component, which is why discarding is overwrite-with-expiry rather than removal. + + Returns True when a run was found and removed, False when the id matched nothing - + so the caller can tell "deleted" from "already gone" rather than reporting success + for a run that was never there. + """ + if not storage or not run_id: + return False + + index = await list_runs(storage) + entry = next((existing for existing in index if existing.get("id") == run_id), None) + if entry is None: + return False + + await _discard_run(storage, run_id, entry.get("plan_keys")) + await storage.save(STORAGE_MODULE, INDEX_NAME, [existing for existing in index if existing.get("id") != run_id], format="json") + return True diff --git a/apps/predbat/annual_tariff.py b/apps/predbat/annual_tariff.py index dc98a63ed..692c6d2bd 100644 --- a/apps/predbat/annual_tariff.py +++ b/apps/predbat/annual_tariff.py @@ -408,10 +408,12 @@ async def fetch_month(self, year, month): self.log("Warn: Annual: no export rates available for {}-{:02d}, treating export as unpaid".format(year, month)) export_unpaid = True - # Only import failing makes the month unusable: rates_for's gate is - # `self.import_url or self.export_url`, so an export-only configuration - # (no import_url) must not be marked unavailable just because - # `import_rates` is the empty dict it was initialised to above. + # Only a failed import download makes the month unusable: there is then + # nothing to price import with, whereas a missing export merely means export + # earns nothing. A tariff with no import URL at all is fine here - it either + # has a basic import pattern, which rates_for now reads independently of the + # export side, or no import source whatsoever, which _validate_tariff rejects + # long before this point. if self.import_url and not import_rates: return False # Recorded only now that the month is confirmed available: a month excluded @@ -471,30 +473,35 @@ def _basic_window(self, info, name, minutes, cache_attr): rates = self._basic_table(info, name, cache_attr) return {minute: rates.get(minute % MINUTES_PER_DAY, 0.0) for minute in range(minutes)} - def rates_for(self, midnight_utc, minutes): - """Return (import, export) rate dicts keyed by absolute minute from ``midnight_utc``.""" + def _side_window(self, midnight_utc, minutes, url, stamped_by_month, basic, name, cache_attr): + """Return one side's rates, keyed by absolute minute from ``midnight_utc``. + + Each side picks its own source: a downloaded URL if it has one, otherwise its + basic repeating pattern. This used to be one either/or gate over BOTH sides + (``if self.import_url or self.export_url``), which silently mispriced any + mixed tariff - and since import and export became separately selectable, a + mixed tariff is an ordinary choice rather than a hypothetical. "Price cap + import with Octopus Outgoing Prime export" took the URL branch and then found + no import rates to stamp, pricing the entire year's import at zero. + """ + if not url: + return self._basic_window(basic or [], name, minutes, cache_attr) + key = (midnight_utc.year, midnight_utc.month) + # A 48 hour window starting late in a month spills into the next month's download + next_key = (midnight_utc.year, midnight_utc.month + 1) if midnight_utc.month < 12 else (midnight_utc.year + 1, 1) + stamped = dict(stamped_by_month.get(key, {})) + stamped.update(stamped_by_month.get(next_key, {})) + + rates = {} + for minute in range(minutes): + stamp = midnight_utc + timedelta(minutes=minute) + if stamp in stamped: + rates[minute] = stamped[stamp] + return rates - if self.import_url or self.export_url: - import_stamped = self.import_rates.get(key, {}) - export_stamped = self.export_rates.get(key, {}) - # A 48 hour window starting late in a month spills into the next month's download - next_key = (midnight_utc.year, midnight_utc.month + 1) if midnight_utc.month < 12 else (midnight_utc.year + 1, 1) - import_stamped = dict(import_stamped) - import_stamped.update(self.import_rates.get(next_key, {})) - export_stamped = dict(export_stamped) - export_stamped.update(self.export_rates.get(next_key, {})) - - rate_import = {} - rate_export = {} - for minute in range(minutes): - stamp = midnight_utc + timedelta(minutes=minute) - if stamp in import_stamped: - rate_import[minute] = import_stamped[stamp] - if stamp in export_stamped: - rate_export[minute] = export_stamped[stamp] - return rate_import, rate_export - - rate_import = self._basic_window(self.basic_import or [], "rates_import", minutes, "_basic_import_table") - rate_export = self._basic_window(self.basic_export or [], "rates_export", minutes, "_basic_export_table") + def rates_for(self, midnight_utc, minutes): + """Return (import, export) rate dicts keyed by absolute minute from ``midnight_utc``.""" + rate_import = self._side_window(midnight_utc, minutes, self.import_url, self.import_rates, self.basic_import, "rates_import", "_basic_import_table") + rate_export = self._side_window(midnight_utc, minutes, self.export_url, self.export_rates, self.basic_export, "rates_export", "_basic_export_table") return rate_import, rate_export diff --git a/apps/predbat/tariff_catalogue.py b/apps/predbat/tariff_catalogue.py index d111c467d..102abb17d 100644 --- a/apps/predbat/tariff_catalogue.py +++ b/apps/predbat/tariff_catalogue.py @@ -21,9 +21,37 @@ # Keys that mean the same thing in both and need no translation PASSTHROUGH_KEYS = ["rates_import", "rates_export"] -# The dropdown's escape hatch: leaves the URL fields blank for a hand-entered tariff +# The dropdown's escape hatch: leaves the URL fields blank for a hand-entered tariff. +# Import and export ids live in separate namespaces, so one constant serves both lists. CUSTOM_ID = "custom" +# "I am not paid for what I export." Modelled as a flat 0p export rate rather than as a +# missing export tariff: an absent rate leaves the minutes unstamped, which reads to the +# rest of the engine as "unknown", whereas 0p is the actual fact being stated. Cost-wise +# this is also exactly right for a physical zero-export (G99) limitation - curtailed +# solar and unpaid exported solar both earn nothing - though the reported export kWh +# will still show the surplus leaving the house. +NO_EXPORT_ID = "no_export" + +# What a household with no PV and no battery is assumed to be on. Named here so the form +# and the engine's own default cannot drift apart. The export side is inert for that +# scenario - no PV means nothing to export - but it is carried anyway so the baseline +# tariff has the same shape as any other. +BASELINE_DEFAULT_IMPORT_ID = "price_cap" +BASELINE_DEFAULT_EXPORT_ID = "seg" + +# Ofgem price cap, 1 July - 30 September 2026, direct debit, England/Scotland/Wales, +# including VAT. Named rather than repeated so the next cap change is a one-line edit. +# Source: https://www.ofgem.gov.uk/information-consumers/energy-advice-households/energy-price-cap-unit-rates-and-standing-charges +PRICE_CAP_IMPORT_P = 26.11 +PRICE_CAP_STANDING_CHARGE_P = 57.19 + +# A typical FIXED Smart Export Guarantee rate. Fixed SEG offers sit around 3-8p/kWh in +# mid-2026, so this is deliberately at the conservative end - the smart export tariffs +# that pay far more are separate entries in this catalogue, and a household on the price +# cap is not on one of them. +SEG_EXPORT_P = 4.1 + _OCTOPUS = "https://api.octopus.energy/v1/products" # The two Octopus flat-rate export products offered against each import tariff below. @@ -41,113 +69,41 @@ _OUTGOING_FIXED = "{}/OUTGOING-VAR-24-10-26/electricity-tariffs/E-1R-OUTGOING-VAR-24-10-26-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS) _OUTGOING_PRIME = "{}/OUTGOING-PRIME-FIX-12M-26-06-23/electricity-tariffs/E-1R-OUTGOING-PRIME-FIX-12M-26-06-23-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS) -BUILTIN_TARIFFS = [ - {"id": "cap_seg", "name": "Price cap import / SEG export", "rates_import": [{"rate": 24.86}], "rates_export": [{"rate": 4.1}]}, +# There is no INTELLI-FLUX-EXPORT product - Octopus publishes both the import and the +# export rates for Intelligent Flux under the one import product code. Do not "fix" the +# export entry below to a distinct export code; that product does not exist and 404s. +_INTELLI_FLUX = "{}/INTELLI-FLUX-IMPORT-23-07-14/electricity-tariffs/E-1R-INTELLI-FLUX-IMPORT-23-07-14-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS) + +# Import and export are chosen independently, so they are listed independently. They used +# to be one list of pre-paired combinations, which could not express a pairing nobody had +# thought to enumerate - and, because the pairs were matched back to the dropdown on their +# import URL alone, two pairs sharing an import (Agile/Fixed and Agile/Prime) were +# indistinguishable: reloading a saved config showed whichever came first in the list. +IMPORT_TARIFFS = [ + {"id": "price_cap", "name": "Price cap", "rates_import": [{"rate": PRICE_CAP_IMPORT_P}]}, { "id": "eon_next_drive", - "name": "Eon Next Drive import / Fixed export", - "rates_import": [{"rate": 6.7, "start": "00:00:00", "end": "07:00:00"}, {"rate": 24.86, "start": "07:00:00", "end": "00:00:00"}], - "rates_export": [{"rate": 16.5}], - }, - { - "id": "igo_fixed", - "name": "Intelligent GO import / Fixed export", - "import_octopus_url": "{}/INTELLI-VAR-24-10-29/electricity-tariffs/E-1R-INTELLI-VAR-24-10-29-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": _OUTGOING_FIXED, - }, - { - "id": "igo_prime", - "name": "Intelligent GO import / Prime export", - "import_octopus_url": "{}/INTELLI-VAR-24-10-29/electricity-tariffs/E-1R-INTELLI-VAR-24-10-29-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": _OUTGOING_PRIME, - }, - { - "id": "igo_agile", - "name": "Intelligent GO import / Agile export", - "import_octopus_url": "{}/INTELLI-VAR-24-10-29/electricity-tariffs/E-1R-INTELLI-VAR-24-10-29-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": "{}/AGILE-OUTGOING-19-05-13/electricity-tariffs/E-1R-AGILE-OUTGOING-19-05-13-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - }, - { - "id": "go_fixed", - "name": "GO import / Fixed export", - "import_octopus_url": "{}/GO-VAR-22-10-14/electricity-tariffs/E-1R-GO-VAR-22-10-14-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": _OUTGOING_FIXED, - }, - { - "id": "go_prime", - "name": "GO import / Prime export", - "import_octopus_url": "{}/GO-VAR-22-10-14/electricity-tariffs/E-1R-GO-VAR-22-10-14-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": _OUTGOING_PRIME, - }, - { - "id": "go_agile", - "name": "GO import / Agile export", - "import_octopus_url": "{}/GO-VAR-22-10-14/electricity-tariffs/E-1R-GO-VAR-22-10-14-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": "{}/AGILE-OUTGOING-19-05-13/electricity-tariffs/E-1R-AGILE-OUTGOING-19-05-13-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - }, - { - "id": "agile_fixed", - "name": "Agile import / Fixed export", - "import_octopus_url": "{}/AGILE-24-10-01/electricity-tariffs/E-1R-AGILE-24-10-01-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": _OUTGOING_FIXED, - }, - { - "id": "agile_prime", - "name": "Agile import / Prime export", - "import_octopus_url": "{}/AGILE-24-10-01/electricity-tariffs/E-1R-AGILE-24-10-01-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": _OUTGOING_PRIME, - }, - { - "id": "agile_agile", - "name": "Agile import / Agile export", - "import_octopus_url": "{}/AGILE-24-10-01/electricity-tariffs/E-1R-AGILE-24-10-01-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": "{}/AGILE-OUTGOING-19-05-13/electricity-tariffs/E-1R-AGILE-OUTGOING-19-05-13-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - }, - { - "id": "flux", - "name": "Flux import / Flux export", - "import_octopus_url": "{}/FLUX-IMPORT-23-02-14/electricity-tariffs/E-1R-FLUX-IMPORT-23-02-14-{{dno_region}}/standard-unit-rates".format(_OCTOPUS), - "export_octopus_url": "{}/FLUX-EXPORT-23-02-14/electricity-tariffs/E-1R-FLUX-EXPORT-23-02-14-{{dno_region}}/standard-unit-rates".format(_OCTOPUS), - }, - { - "id": "cosy_fixed", - "name": "Cosy import / Fixed export", - "import_octopus_url": "{}/COSY-22-12-08/electricity-tariffs/E-1R-COSY-22-12-08-{{dno_region}}/standard-unit-rates".format(_OCTOPUS), - "export_octopus_url": _OUTGOING_FIXED, - }, - { - "id": "cosy_prime", - "name": "Cosy import / Prime export", - "import_octopus_url": "{}/COSY-22-12-08/electricity-tariffs/E-1R-COSY-22-12-08-{{dno_region}}/standard-unit-rates".format(_OCTOPUS), - "export_octopus_url": _OUTGOING_PRIME, - }, - { - "id": "cosy_agile", - "name": "Cosy import / Agile export", - "import_octopus_url": "{}/COSY-22-12-08/electricity-tariffs/E-1R-COSY-22-12-08-{{dno_region}}/standard-unit-rates".format(_OCTOPUS), - "export_octopus_url": "{}/AGILE-OUTGOING-19-05-13/electricity-tariffs/E-1R-AGILE-OUTGOING-19-05-13-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - }, - { - "id": "snug_fixed", - "name": "Snug import / Fixed export", - "import_octopus_url": "{}/SNUG-24-11-07/electricity-tariffs/E-1R-SNUG-24-11-07-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": _OUTGOING_FIXED, - }, - { - "id": "snug_prime", - "name": "Snug import / Prime export", - "import_octopus_url": "{}/SNUG-24-11-07/electricity-tariffs/E-1R-SNUG-24-11-07-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": _OUTGOING_PRIME, - }, - { - "id": "iflux", - "name": "Intelligent Flux import / export", - # There is no INTELLI-FLUX-EXPORT product - Octopus publishes both the import and - # export rates for Intelligent Flux under the import product code below. Do not - # "fix" this to a distinct export code; that product does not exist and 404s. - "import_octopus_url": "{}/INTELLI-FLUX-IMPORT-23-07-14/electricity-tariffs/E-1R-INTELLI-FLUX-IMPORT-23-07-14-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - "export_octopus_url": "{}/INTELLI-FLUX-IMPORT-23-07-14/electricity-tariffs/E-1R-INTELLI-FLUX-IMPORT-23-07-14-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS), - }, + "name": "Eon Next Drive", + "rates_import": [{"rate": 6.7, "start": "00:00:00", "end": "07:00:00"}, {"rate": PRICE_CAP_IMPORT_P, "start": "07:00:00", "end": "00:00:00"}], + }, + {"id": "agile", "name": "Octopus Agile", "import_octopus_url": "{}/AGILE-24-10-01/electricity-tariffs/E-1R-AGILE-24-10-01-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS)}, + {"id": "intelligent_go", "name": "Octopus Intelligent GO", "import_octopus_url": "{}/INTELLI-VAR-24-10-29/electricity-tariffs/E-1R-INTELLI-VAR-24-10-29-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS)}, + {"id": "go", "name": "Octopus GO", "import_octopus_url": "{}/GO-VAR-22-10-14/electricity-tariffs/E-1R-GO-VAR-22-10-14-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS)}, + {"id": "cosy", "name": "Octopus Cosy", "import_octopus_url": "{}/COSY-22-12-08/electricity-tariffs/E-1R-COSY-22-12-08-{{dno_region}}/standard-unit-rates".format(_OCTOPUS)}, + {"id": "snug", "name": "Octopus Snug", "import_octopus_url": "{}/SNUG-24-11-07/electricity-tariffs/E-1R-SNUG-24-11-07-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS)}, + {"id": "flux", "name": "Octopus Flux", "import_octopus_url": "{}/FLUX-IMPORT-23-02-14/electricity-tariffs/E-1R-FLUX-IMPORT-23-02-14-{{dno_region}}/standard-unit-rates".format(_OCTOPUS)}, + {"id": "intelligent_flux", "name": "Octopus Intelligent Flux", "import_octopus_url": _INTELLI_FLUX}, +] + +EXPORT_TARIFFS = [ + {"id": NO_EXPORT_ID, "name": "No export payment", "rates_export": [{"rate": 0.0}]}, + {"id": "seg", "name": "SEG fixed rate ({}p)".format(SEG_EXPORT_P), "rates_export": [{"rate": SEG_EXPORT_P}]}, + {"id": "outgoing_fixed", "name": "Octopus Outgoing Fixed", "export_octopus_url": _OUTGOING_FIXED}, + {"id": "outgoing_prime", "name": "Octopus Outgoing Prime", "export_octopus_url": _OUTGOING_PRIME}, + {"id": "agile_outgoing", "name": "Octopus Agile Outgoing", "export_octopus_url": "{}/AGILE-OUTGOING-19-05-13/electricity-tariffs/E-1R-AGILE-OUTGOING-19-05-13-{{dno_region}}/standard-unit-rates/".format(_OCTOPUS)}, + {"id": "flux_export", "name": "Octopus Flux export", "export_octopus_url": "{}/FLUX-EXPORT-23-02-14/electricity-tariffs/E-1R-FLUX-EXPORT-23-02-14-{{dno_region}}/standard-unit-rates".format(_OCTOPUS)}, + {"id": "intelligent_flux_export", "name": "Octopus Intelligent Flux export", "export_octopus_url": _INTELLI_FLUX}, + {"id": "eon_next_export", "name": "Eon Next export (16.5p)", "rates_export": [{"rate": 16.5}]}, ] @@ -177,25 +133,81 @@ def convert_compare_entry(entry): return converted -def merged_catalogue(compare_list=None): - """Return the dropdown's entries: built-ins, then the user's own, then Custom. +def _merged(builtins, keys, compare_list, custom_name): + """Return one side's dropdown entries: built-ins, then the user's own, then Custom. + + ``keys`` are the rate-source keys belonging to this side; a user entry carrying + none of them has nothing to offer this dropdown and is left out of it. That is how + one paired ``compare_list`` entry lands in both lists, or in only the list it can + actually supply - an import-only entry is not offered as an export tariff. - A user entry sharing a built-in id replaces it rather than appearing twice - - the user's own definition is the more specific one. Malformed entries are - skipped so one bad line in apps.yaml cannot empty the dropdown. + A user entry sharing a built-in id replaces it rather than appearing twice: the + user's own definition is the more specific one. Malformed entries are skipped so + one bad line in apps.yaml cannot empty the dropdown. """ - catalogue = [dict(entry) for entry in BUILTIN_TARIFFS] + catalogue = [dict(entry) for entry in builtins] by_id = {entry["id"]: index for index, entry in enumerate(catalogue)} for entry in compare_list or []: converted = convert_compare_entry(entry) if converted is None: continue - if converted["id"] in by_id: - catalogue[by_id[converted["id"]]] = converted + side = {key: converted[key] for key in keys if converted.get(key)} + if not side: + continue + side["id"] = converted["id"] + side["name"] = converted["name"] + if side["id"] in by_id: + catalogue[by_id[side["id"]]] = side else: - by_id[converted["id"]] = len(catalogue) - catalogue.append(converted) + by_id[side["id"]] = len(catalogue) + catalogue.append(side) - catalogue.append({"id": CUSTOM_ID, "name": "Custom - enter URLs below"}) + catalogue.append({"id": CUSTOM_ID, "name": custom_name}) return catalogue + + +def match_entry(catalogue, tariff, url_key, rates_key): + """Return the catalogue entry one side of ``tariff`` was chosen from, or None. + + The single rule for mapping a stored tariff back to the choice that produced it. + Both the configuration form (to re-select its dropdowns) and the compare table (to + name a run's tariffs) go through here, so the two cannot describe the same tariff + differently. + + Basic-rates entries are matched on their RATES, not a URL: several entries ("Price + cap", "Eon Next Drive", "No export payment") carry no URL at all, and matching on + URL alone identified none of them. + + Each side is matched against its OWN list. The single paired dropdown this catalogue + replaced matched on the import URL alone, so "Agile / Prime" and "Agile / Fixed" + were indistinguishable and a saved config reloaded as whichever came first. + + Returns None both for a side that is unset and for one that matches nothing - a + hand-entered URL, or a ``compare_list`` entry the user has since removed. The Custom + placeholder is never returned: it carries no rate source, so naming a tariff after + it would describe nothing. Callers that want an id for the dropdown map None onto + ``CUSTOM_ID`` themselves, which is a question about the form rather than about the + catalogue. + """ + tariff = tariff or {} + current_url = tariff.get(url_key) + current_rates = tariff.get(rates_key) + if not current_url and not current_rates: + return None + for entry in catalogue or []: + if current_url and entry.get(url_key) == current_url: + return entry + if not current_url and entry.get(rates_key) == current_rates: + return entry + return None + + +def merged_import_catalogue(compare_list=None): + """Return the import dropdown's entries: built-ins, then the user's own, then Custom.""" + return _merged(IMPORT_TARIFFS, ["import_octopus_url", "rates_import"], compare_list, "Custom - enter URL below") + + +def merged_export_catalogue(compare_list=None): + """Return the export dropdown's entries: built-ins, then the user's own, then Custom.""" + return _merged(EXPORT_TARIFFS, ["export_octopus_url", "rates_export"], compare_list, "Custom - enter URL below") diff --git a/apps/predbat/tests/test_annual_config.py b/apps/predbat/tests/test_annual_config.py index ea6022803..ed7f157ce 100644 --- a/apps/predbat/tests/test_annual_config.py +++ b/apps/predbat/tests/test_annual_config.py @@ -174,6 +174,30 @@ def test_annual_config(my_predbat): del config["annual"]["tariff"] failed = expect_error("no tariff", config, "annual.tariff is required", failed) + print("Test: a broken baseline_tariff names the baseline block, not the main tariff") + # Both blocks go through the same validator. Reporting every baseline problem against + # "annual.tariff" sent the user to fix a block that was perfectly valid - the one + # thing an error message must not do. + for broken, fragment, label in [ + ({"rates_export": [{"rate": 4.1}]}, "annual.baseline_tariff requires either import_octopus_url or rates_import", "no import source"), + ({"import_octopus_url": "https://example.com/{dno_region}/x/"}, "annual.baseline_tariff.import_octopus_url uses {dno_region}", "templated with no region"), + ({"rates_import": [{"rate": 26.11}], "standing_charge_p_per_day": "abc"}, "annual.baseline_tariff.standing_charge_p_per_day", "non-numeric standing charge"), + ("not-a-dict", "annual.baseline_tariff is required and must be a mapping", "not a mapping"), + ]: + config = base_config() + config["annual"]["baseline_tariff"] = broken + failed = expect_error("baseline {}".format(label), config, fragment, failed) + + print("Test: the main tariff's own messages still name the main tariff") + # The parameterised path must not have shifted the ordinary case onto the baseline. + config = base_config() + config["annual"]["tariff"] = {"rates_export": [{"rate": 4.1}]} + failed = expect_error("main tariff with no import source", config, "annual.tariff requires either import_octopus_url or rates_import", failed) + config = base_config() + config["annual"]["tariff"]["import_octopus_url"] = "https://example.com/{dno_region}/x/" + config["annual"]["tariff"].pop("dno_region", None) + failed = expect_error("main tariff templated with no region", config, "annual.tariff.import_octopus_url uses {dno_region}", failed) + print("Test: car_rate_kw defaults to 7.4 kW when omitted") config = base_config() result = validate_config(config, today=date(2026, 7, 25)) diff --git a/apps/predbat/tests/test_annual_integration.py b/apps/predbat/tests/test_annual_integration.py index 3ba38874e..96176a866 100644 --- a/apps/predbat/tests/test_annual_integration.py +++ b/apps/predbat/tests/test_annual_integration.py @@ -329,6 +329,30 @@ def _run_car_config(): print(" ERROR: {}.{} = {} without capture but {} with capture - save is leaking into the billed numbers".format(key, field, without_capture[key][field], with_capture[key][field])) failed = True + print("Test: the baseline tariff prices no_pvbat only and does not leak into the other scenarios") + # A flat price-cap-style baseline against the banded main tariff. Two properties + # matter and they pull in opposite directions: the counterfactual MUST change (or the + # feature does nothing), and every other scenario must be bit-for-bit identical (or + # the swap has leaked and silently repriced the system being evaluated). _apply_rates + # mutates predbat in place and replicates what it is given, so a leak here is a very + # live possibility rather than a theoretical one. + reset_inverter(my_predbat) + baseline_midnight = pytz.utc.localize(datetime(day.year, day.month, day.day)) + baseline_load_source = SyntheticLoadProfile(annual_kwh=config["load"]["annual_kwh"], shape=config["load"]["shape"], year=config["year"]) + flat_cap = StubTariff(cheap=26.11, normal=26.11, peak=26.11, export=4.1) + without_baseline = _run_scenarios(my_predbat, config, weather, StubTariff(), baseline_load_source, day, baseline_midnight, car_kwh=0.0, car_rate_kw=DEFAULT_CAR_RATE_KW) + reset_inverter(my_predbat) + with_baseline = _run_scenarios(my_predbat, config, weather, StubTariff(), baseline_load_source, day, baseline_midnight, car_kwh=0.0, car_rate_kw=DEFAULT_CAR_RATE_KW, baseline_tariff=flat_cap) + + if abs(with_baseline["no_pvbat"]["cost_p"] - without_baseline["no_pvbat"]["cost_p"]) < 1.0: + print(" ERROR: a flat baseline tariff should reprice no_pvbat away from the banded main tariff, got {} vs {}".format(without_baseline["no_pvbat"]["cost_p"], with_baseline["no_pvbat"]["cost_p"])) + failed = True + for key in ["pv_only", "without_predbat", "with_predbat"]: + for field in SCENARIO_FIELDS: + if without_baseline[key][field] != with_baseline[key][field]: + print(" ERROR: {}.{} changed from {} to {} - the baseline tariff has leaked past no_pvbat".format(key, field, without_baseline[key][field], with_baseline[key][field])) + failed = True + print("Test: capturing plans does not leak predbat.debug_enable on") # The annual "debug" flag means "save the plan info", nothing more - it must never be # wired to Predbat's own debug_enable, which kernel_supported() requires False to use diff --git a/apps/predbat/tests/test_annual_store.py b/apps/predbat/tests/test_annual_store.py index 3d8e73119..ce29097d5 100644 --- a/apps/predbat/tests/test_annual_store.py +++ b/apps/predbat/tests/test_annual_store.py @@ -12,7 +12,7 @@ import asyncio import datetime -from annual_store import INDEX_NAME, MAX_RUNS, STORAGE_MODULE, _discard_run, backfill_summaries, build_label, build_summary, list_runs, load_plan, load_run, save_run +from annual_store import INDEX_NAME, MAX_RUNS, STORAGE_MODULE, _discard_run, backfill_summaries, build_label, build_summary, delete_run, list_runs, load_plan, load_run, save_run from tests.test_infra import run_async @@ -251,6 +251,36 @@ def test_annual_store(my_predbat): print(" ERROR: an unavailable payback should carry its reason, got {}".format(unavailable)) failed = True + print("Test: the summary carries both tariff dicts so the compare table can name all three sides") + # Deliberately the raw dicts rather than a rendered name: naming needs the merged + # catalogue, which includes the user's own compare_list entries, and only the web + # layer can read those. A name baked in here could not see them. + tariffs = { + "tariff": {"import_octopus_url": "https://api.octopus.energy/v1/products/AGILE-24-10-01/x/", "rates_export": [{"rate": 0.0}]}, + "baseline_tariff": {"rates_import": [{"rate": 26.11}]}, + } + with_tariffs = build_summary(results, tariffs) + if with_tariffs.get("tariff") != tariffs["tariff"]: + print(" ERROR: the summary should carry the run's tariff dict, got {}".format(with_tariffs.get("tariff"))) + failed = True + if with_tariffs.get("baseline_tariff") != tariffs["baseline_tariff"]: + print(" ERROR: the summary should carry the baseline tariff dict, got {}".format(with_tariffs.get("baseline_tariff"))) + failed = True + + print("Test: the summary's tariffs are copies, so later edits to the config cannot rewrite a stored run") + mutable = {"tariff": {"rates_import": [{"rate": 5.0}]}, "baseline_tariff": {"rates_import": [{"rate": 26.11}]}} + copied = build_summary(results, mutable) + mutable["tariff"]["rates_import"][0]["rate"] = 99.0 + if copied["tariff"]["rates_import"][0]["rate"] != 5.0: + print(" ERROR: the summary should hold its own copy of the tariff, got {}".format(copied["tariff"])) + failed = True + + print("Test: a config with no tariff at all summarises as empty dicts rather than raising") + bare = build_summary(results, {}) + if bare.get("tariff") != {} or bare.get("baseline_tariff") != {}: + print(" ERROR: a config with no tariff should summarise as empty, got {}".format(bare)) + failed = True + print("Test: save_run records the summary on the index entry") storage = FakeStorage() asyncio.run(save_run(storage, results, config, "20260728-090000")) @@ -300,7 +330,7 @@ def test_annual_store(my_predbat): print(" ERROR: backfill should still write the index exactly once despite the corrupt entry, got {} writes".format(len(storage.save_calls) - writes_before)) failed = True - print("Test: backfill writes nothing when every entry already has a summary") + print("Test: backfill writes nothing when every entry already has a current summary") storage = FakeStorage() asyncio.run(save_run(storage, results, config, "20260728-090400")) asyncio.run(save_run(storage, results, config, "20260728-090401")) @@ -310,6 +340,32 @@ def test_annual_store(my_predbat): print(" ERROR: a compare page that changes nothing must not write storage, got {} writes".format(len(storage.save_calls) - writes_before)) failed = True + print("Test: a summary predating the tariff fields is refreshed, not left as it is") + # A summary that merely EXISTS is no longer enough: runs stored before the compare + # table showed three tariffs have one, but it describes none of them. Left alone, + # those rows would show dashes forever despite their document holding the answer. + storage = FakeStorage() + # The config rides along inside the document, which is where backfill reads it from - + # the index entry's own copy is exactly what is being rebuilt here. + stale_config = {"tariff": {"rates_import": [{"rate": 7.5}]}, "baseline_tariff": {"rates_import": [{"rate": 26.11}]}} + stale_results = dict(results, config=stale_config) + asyncio.run(save_run(storage, stale_results, stale_config, "20260728-090500")) + stale = asyncio.run(list_runs(storage)) + # Exactly the shape the old code wrote: a summary with a rendered name and no dicts. + stale[0]["summary"] = {"total_kwp": 5.6, "battery_kwh": 9.5, "tariff": "Agile", "cost_with_predbat_p": 66000.0} + asyncio.run(storage.save(STORAGE_MODULE, INDEX_NAME, stale, format="json")) + writes_before = len(storage.save_calls) + filled = asyncio.run(backfill_summaries(storage, asyncio.run(list_runs(storage)))) + if filled[0]["summary"].get("tariff") != {"rates_import": [{"rate": 7.5}]}: + print(" ERROR: a stale summary should be rebuilt from the document's config, got {}".format(filled[0]["summary"].get("tariff"))) + failed = True + if filled[0]["summary"].get("baseline_tariff") != {"rates_import": [{"rate": 26.11}]}: + print(" ERROR: the rebuilt summary should carry the baseline the run actually used, got {}".format(filled[0]["summary"])) + failed = True + if len(storage.save_calls) != writes_before + 1: + print(" ERROR: refreshing a stale summary should persist the index once, got {} writes".format(len(storage.save_calls) - writes_before)) + failed = True + print("Test: save_run strips the plans out of the stored document and keys them per leg") storage = FakeStorage() debug_results = { @@ -449,4 +505,44 @@ def test_annual_store(my_predbat): print(" ERROR: an evicted run's plan keys should be discarded too") failed = True + print("Test: deleting a run removes its document, its plans and its index entry") + storage = FakeStorage() + debug_doc = { + "annual": {"scenarios": {"no_pvbat": {"cost_p": 10.0}, "with_predbat": {"cost_p": 5.0}}, "months_included": 12}, + "months": [{"month": 1, "status": "ok", "plans": [{"day": "2025-01-08", "leg": "single", "scenarios": {"with_predbat": {"rows": [1]}}}]}], + } + asyncio.run(save_run(storage, debug_doc, {}, "doomed")) + asyncio.run(save_run(storage, sample_results(1), sample_config(), "keeper")) + plan_keys = [key for key in storage.store if "doomed" in str(key) and "plans" in str(key)] + if not plan_keys: + print(" ERROR: the fixture should have produced plan keys to delete") + failed = True + + if asyncio.run(delete_run(storage, "doomed")) is not True: + print(" ERROR: deleting a stored run should report success") + failed = True + if [entry["id"] for entry in asyncio.run(list_runs(storage))] != ["keeper"]: + print(" ERROR: only the deleted run should leave the index, got {}".format(asyncio.run(list_runs(storage)))) + failed = True + if asyncio.run(load_run(storage, "doomed")) is not None: + print(" ERROR: a deleted run's document should no longer load") + failed = True + # Dropping it from the index alone would leave the plans orphaned - unreachable, but + # still occupying storage that nothing will ever clean up. + if any(storage.store.get(key) is not None for key in plan_keys): + print(" ERROR: a deleted run's plan keys should be discarded too, got {}".format(plan_keys)) + failed = True + if asyncio.run(load_run(storage, "keeper")) is None: + print(" ERROR: deleting one run must not touch another") + failed = True + + print("Test: deleting an unknown run reports failure rather than pretending it worked") + if asyncio.run(delete_run(storage, "never-existed")) is not False: + print(" ERROR: deleting a run that was never there should return False") + failed = True + for bad in [None, ""]: + if asyncio.run(delete_run(storage, bad)) is not False: + print(" ERROR: delete_run({!r}) should return False".format(bad)) + failed = True + return failed diff --git a/apps/predbat/tests/test_annual_tariff.py b/apps/predbat/tests/test_annual_tariff.py index 43be6386c..8e12323be 100644 --- a/apps/predbat/tests/test_annual_tariff.py +++ b/apps/predbat/tests/test_annual_tariff.py @@ -774,4 +774,75 @@ async def tariff_d_bare_fetch(url): print(" ERROR: tariff D's own export rate expected 30.0, got {} - if this is 5.0 instead, tariff D was served tariff C's cached current-rates pattern".format(d_export.get(0))) failed = True + print("Test: a mixed tariff prices each side from its own source") + + # Import and export are selected independently in the web UI, so "basic-rate import + # with a downloaded export tariff" (price cap import + Octopus Outgoing Prime, say) + # is an ordinary choice. rates_for used to gate BOTH sides on one + # `self.import_url or self.export_url` test: this config took the URL branch, found + # no import rows to stamp, and priced the whole year's import at zero - a plausible + # looking run that was silently free. Asserting on the VALUES, not just presence, + # is what makes that failure visible. + async def mixed_export_fetch(url): + """Serve a flat 12p export payload for the export URL; no import URL is ever requested.""" + return {"results": [{"value_inc_vat": 12.0, "valid_from": "2025-03-01T00:00:00Z", "valid_to": None}], "next": None} + + mixed_config = {"rates_import": [{"rate": 26.11}], "export_octopus_url": "https://example.com/mixed/export/", "standing_charge_p_per_day": 57.19} + mixed = AnnualTariff(mixed_config, log=print, predbat=my_predbat, fetch_json=mixed_export_fetch, timezone="Europe/London") + if not asyncio.run(mixed.fetch_month(2025, 3)): + print(" ERROR: a basic-rate import with a downloaded export should be a usable month") + failed = True + mixed_import, mixed_export = mixed.rates_for(pytz.utc.localize(datetime(2025, 3, 10)), 24 * 60) + if len(mixed_import) < 24 * 60: + print(" ERROR: expected a full day of import rates from the basic pattern, got {} minutes".format(len(mixed_import))) + failed = True + if abs(mixed_import.get(0, -1) - 26.11) > 0.01: + print(" ERROR: import should come from rates_import (26.11p), got {} - zero or missing means the basic import was ignored because export had a URL".format(mixed_import.get(0))) + failed = True + if abs(mixed_export.get(0, -1) - 12.0) > 0.01: + print(" ERROR: export should come from the downloaded URL (12.0p), got {}".format(mixed_export.get(0))) + failed = True + + print("Test: the mirror image - a downloaded import with a basic-rate export") + + async def mixed_import_fetch(url): + """Serve a flat 30p import payload for the import URL.""" + return {"results": [{"value_inc_vat": 30.0, "valid_from": "2025-03-01T00:00:00Z", "valid_to": None}], "next": None} + + mirror_config = {"import_octopus_url": "https://example.com/mirror/import/", "rates_export": [{"rate": 4.1}], "standing_charge_p_per_day": 0.0} + mirror = AnnualTariff(mirror_config, log=print, predbat=my_predbat, fetch_json=mixed_import_fetch, timezone="Europe/London") + if not asyncio.run(mirror.fetch_month(2025, 3)): + print(" ERROR: a downloaded import with a basic-rate export should be a usable month") + failed = True + mirror_import, mirror_export = mirror.rates_for(pytz.utc.localize(datetime(2025, 3, 10)), 24 * 60) + if abs(mirror_import.get(0, -1) - 30.0) > 0.01: + print(" ERROR: import should come from the downloaded URL (30.0p), got {}".format(mirror_import.get(0))) + failed = True + if abs(mirror_export.get(0, -1) - 4.1) > 0.01: + print(" ERROR: export should come from rates_export (4.1p), got {} - missing means the basic export was ignored because import had a URL".format(mirror_export.get(0))) + failed = True + + print("Test: a deliberate zero export rate is priced, not left unrated") + # "No export payment" is a flat 0p rate. Every minute must be stamped 0, not left + # absent: an absent rate reads as "unknown" to the caller rather than "unpaid", and + # it must not raise the unpaid-export caveat, which is for a download that failed. + zero_config = {"rates_import": [{"rate": 26.11}], "rates_export": [{"rate": 0.0}], "standing_charge_p_per_day": 0.0} + zero = AnnualTariff(zero_config, log=print, predbat=my_predbat, timezone="Europe/London") + if not asyncio.run(zero.fetch_month(2025, 3)): + print(" ERROR: a zero export rate should still be a usable month") + failed = True + zero_import, zero_export = zero.rates_for(pytz.utc.localize(datetime(2025, 3, 10)), 24 * 60) + if len(zero_export) < 24 * 60: + print(" ERROR: expected a full day of export rates stamped at zero, got {} minutes".format(len(zero_export))) + failed = True + if set(zero_export.values()) != {0.0}: + print(" ERROR: every export minute should be 0p, got values {}".format(sorted(set(zero_export.values()))[:5])) + failed = True + if abs(zero_import.get(0, -1) - 26.11) > 0.01: + print(" ERROR: import should be unaffected by a zero export, got {}".format(zero_import.get(0))) + failed = True + if zero.unpaid_export_months: + print(" ERROR: a deliberate zero export must not raise the unpaid-export caveat, got {}".format(zero.unpaid_export_months)) + failed = True + return failed diff --git a/apps/predbat/tests/test_tariff_catalogue.py b/apps/predbat/tests/test_tariff_catalogue.py index 7d6139d0c..d34338abb 100644 --- a/apps/predbat/tests/test_tariff_catalogue.py +++ b/apps/predbat/tests/test_tariff_catalogue.py @@ -7,62 +7,93 @@ # pylint: disable=consider-using-f-string # pylint: disable=line-too-long -"""Tests for the tariff catalogue used by the Annual tab's dropdown.""" +"""Tests for the tariff catalogue used by the Annual tab's dropdowns.""" -from tariff_catalogue import BUILTIN_TARIFFS, CUSTOM_ID, convert_compare_entry, merged_catalogue +from tariff_catalogue import ( + BASELINE_DEFAULT_EXPORT_ID, + BASELINE_DEFAULT_IMPORT_ID, + CUSTOM_ID, + EXPORT_TARIFFS, + IMPORT_TARIFFS, + NO_EXPORT_ID, + convert_compare_entry, + match_entry, + merged_export_catalogue, + merged_import_catalogue, +) def test_tariff_catalogue(my_predbat): - """Verify the built-in catalogue, the Compare key mapping, and the merge.""" + """Verify the built-in catalogues, the Compare key mapping, and the two merges.""" failed = False print("**** Testing tariff_catalogue ****") - print("Test: every built-in entry has an id, a name and at least an import URL") - if not BUILTIN_TARIFFS: - print(" ERROR: the built-in catalogue is empty") - failed = True - seen_ids = set() - for entry in BUILTIN_TARIFFS: - for key in ["id", "name"]: - if not entry.get(key): - print(" ERROR: entry {} is missing '{}'".format(entry, key)) - failed = True - if not entry.get("import_octopus_url") and not entry.get("rates_import"): - print(" ERROR: entry {} has neither an import URL nor fixed rates".format(entry.get("id"))) - failed = True - if entry.get("id") in seen_ids: - print(" ERROR: duplicate id {}".format(entry.get("id"))) + print("Test: every built-in entry has an id, a name and a rate source for its own side") + for name, builtins, url_key, rates_key in [ + ("import", IMPORT_TARIFFS, "import_octopus_url", "rates_import"), + ("export", EXPORT_TARIFFS, "export_octopus_url", "rates_export"), + ]: + if not builtins: + print(" ERROR: the built-in {} catalogue is empty".format(name)) failed = True - seen_ids.add(entry.get("id")) - - print("Test: the built-in catalogue contains exactly the expected ids") - expected_ids = [ - "cap_seg", - "eon_next_drive", - "igo_fixed", - "igo_prime", - "igo_agile", - "go_fixed", - "go_prime", - "go_agile", - "agile_fixed", - "agile_prime", - "agile_agile", - "flux", - "cosy_fixed", - "cosy_prime", - "cosy_agile", - "snug_fixed", - "snug_prime", - "iflux", - ] - actual_ids = [entry["id"] for entry in BUILTIN_TARIFFS] - if actual_ids != expected_ids: - print(" ERROR: built-in ids changed, expected {} got {}".format(expected_ids, actual_ids)) + seen_ids = set() + for entry in builtins: + for key in ["id", "name"]: + if not entry.get(key): + print(" ERROR: {} entry {} is missing '{}'".format(name, entry, key)) + failed = True + if entry.get(url_key) is None and entry.get(rates_key) is None: + print(" ERROR: {} entry {} has neither a URL nor fixed rates".format(name, entry.get("id"))) + failed = True + if entry.get("id") in seen_ids: + print(" ERROR: duplicate {} id {}".format(name, entry.get("id"))) + failed = True + seen_ids.add(entry.get("id")) + + print("Test: an import entry carries no export keys, and vice versa") + # The whole point of the split is that a side is chosen independently. An entry + # carrying the other side's keys would silently reintroduce a fixed pairing. + for entry in IMPORT_TARIFFS: + for stray in ["export_octopus_url", "rates_export"]: + if stray in entry: + print(" ERROR: import entry {} carries export key '{}'".format(entry.get("id"), stray)) + failed = True + for entry in EXPORT_TARIFFS: + for stray in ["import_octopus_url", "rates_import"]: + if stray in entry: + print(" ERROR: export entry {} carries import key '{}'".format(entry.get("id"), stray)) + failed = True + + print("Test: the built-in catalogues contain exactly the expected ids") + expected_import = ["price_cap", "eon_next_drive", "agile", "intelligent_go", "go", "cosy", "snug", "flux", "intelligent_flux"] + actual_import = [entry["id"] for entry in IMPORT_TARIFFS] + if actual_import != expected_import: + print(" ERROR: import ids changed, expected {} got {}".format(expected_import, actual_import)) + failed = True + expected_export = [NO_EXPORT_ID, "seg", "outgoing_fixed", "outgoing_prime", "agile_outgoing", "flux_export", "intelligent_flux_export", "eon_next_export"] + actual_export = [entry["id"] for entry in EXPORT_TARIFFS] + if actual_export != expected_export: + print(" ERROR: export ids changed, expected {} got {}".format(expected_export, actual_export)) + failed = True + + print("Test: the baseline defaults name entries that actually exist") + if BASELINE_DEFAULT_IMPORT_ID not in actual_import: + print(" ERROR: baseline import default {} is not in the import catalogue".format(BASELINE_DEFAULT_IMPORT_ID)) + failed = True + if BASELINE_DEFAULT_EXPORT_ID not in actual_export: + print(" ERROR: baseline export default {} is not in the export catalogue".format(BASELINE_DEFAULT_EXPORT_ID)) + failed = True + + print("Test: 'no export' is a flat zero rate, not an absent tariff") + # An absent export leaves every minute unstamped, which reads as "unknown" rather + # than "unpaid"; the whole option is only meaningful as an explicit zero. + no_export = [entry for entry in EXPORT_TARIFFS if entry["id"] == NO_EXPORT_ID][0] + if no_export.get("rates_export") != [{"rate": 0.0}]: + print(" ERROR: no-export entry should be a flat 0p rate, got {}".format(no_export)) failed = True print("Test: no built-in entry uses Compare's key names") - for entry in BUILTIN_TARIFFS: + for entry in IMPORT_TARIFFS + EXPORT_TARIFFS: for stale in ["rates_import_octopus_url", "rates_export_octopus_url"]: if stale in entry: print(" ERROR: entry {} still uses Compare's key '{}'".format(entry.get("id"), stale)) @@ -108,36 +139,124 @@ def test_tariff_catalogue(my_predbat): print(" ERROR: a non-dict should be rejected rather than raising") failed = True - print("Test: merged_catalogue with no user list returns the built-ins plus Custom") - merged = merged_catalogue(None) - if len(merged) != len(BUILTIN_TARIFFS) + 1: - print(" ERROR: expected {} entries, got {}".format(len(BUILTIN_TARIFFS) + 1, len(merged))) + print("Test: each merge with no user list returns its built-ins plus Custom") + for name, merged, builtins in [ + ("import", merged_import_catalogue(None), IMPORT_TARIFFS), + ("export", merged_export_catalogue(None), EXPORT_TARIFFS), + ]: + if len(merged) != len(builtins) + 1: + print(" ERROR: {} expected {} entries, got {}".format(name, len(builtins) + 1, len(merged))) + failed = True + if merged[-1]["id"] != CUSTOM_ID: + print(" ERROR: {} Custom should be the last entry, got {}".format(name, merged[-1])) + failed = True + + print("Test: one paired compare_list entry is offered in BOTH dropdowns") + # A user's compare_list entry is a pair, but the dropdowns are not - so the entry + # has to be decomposed rather than dropped from one side or forced into both. + paired = [{"id": "mine", "name": "My deal", "rates_import_octopus_url": "https://example.com/i/", "rates_export_octopus_url": "https://example.com/e/"}] + import_side = [entry for entry in merged_import_catalogue(paired) if entry["id"] == "mine"] + export_side = [entry for entry in merged_export_catalogue(paired) if entry["id"] == "mine"] + if not import_side or import_side[0].get("import_octopus_url") != "https://example.com/i/": + print(" ERROR: the paired entry should appear in the import list, got {}".format(import_side)) failed = True - if merged[-1]["id"] != CUSTOM_ID: - print(" ERROR: Custom should be the last entry, got {}".format(merged[-1])) + if not export_side or export_side[0].get("export_octopus_url") != "https://example.com/e/": + print(" ERROR: the paired entry should appear in the export list, got {}".format(export_side)) + failed = True + if import_side and ("export_octopus_url" in import_side[0] or "rates_export" in import_side[0]): + print(" ERROR: the import half should not carry export keys, got {}".format(import_side[0])) failed = True - print("Test: a user's compare_list is merged in and does not clobber a built-in id") - builtin_id = BUILTIN_TARIFFS[0]["id"] - merged = merged_catalogue( - [ - {"id": builtin_id, "name": "My override", "rates_import_octopus_url": "https://example.com/mine/"}, - {"id": "my_tariff", "name": "My tariff", "rates_import": [{"rate": 20.0}]}, - ] - ) - ids = [entry["id"] for entry in merged] - if ids.count(builtin_id) != 1: - print(" ERROR: a user entry sharing a built-in id should not duplicate it, ids were {}".format(ids)) + print("Test: an import-only compare_list entry is not offered as an export tariff") + # Offering it would produce a run priced against an export tariff that does not + # exist, which reads downstream as "export earns nothing" with no warning. + import_only = [{"id": "import_only", "name": "Import only", "rates_import": [{"rate": 9.0}]}] + if "import_only" not in [entry["id"] for entry in merged_import_catalogue(import_only)]: + print(" ERROR: an import-only entry should appear in the import list") + failed = True + if "import_only" in [entry["id"] for entry in merged_export_catalogue(import_only)]: + print(" ERROR: an import-only entry must not appear in the export list") failed = True - if "my_tariff" not in ids: - print(" ERROR: a new user entry should appear, ids were {}".format(ids)) + + print("Test: a user entry sharing a built-in id replaces it rather than duplicating it") + for name, merge, builtin_id in [ + ("import", merged_import_catalogue, IMPORT_TARIFFS[0]["id"]), + ("export", merged_export_catalogue, EXPORT_TARIFFS[0]["id"]), + ]: + merged = merge( + [ + {"id": builtin_id, "name": "My override", "rates_import_octopus_url": "https://example.com/mine/", "rates_export_octopus_url": "https://example.com/mine-e/"}, + {"id": "my_tariff", "name": "My tariff", "rates_import": [{"rate": 20.0}], "rates_export": [{"rate": 3.0}]}, + ] + ) + ids = [entry["id"] for entry in merged] + if ids.count(builtin_id) != 1: + print(" ERROR: {} a user entry sharing a built-in id should not duplicate it, ids were {}".format(name, ids)) + failed = True + if "my_tariff" not in ids: + print(" ERROR: {} a new user entry should appear, ids were {}".format(name, ids)) + failed = True + + print("Test: a malformed user entry is skipped rather than breaking the dropdowns") + for name, merge in [("import", merged_import_catalogue), ("export", merged_export_catalogue)]: + merged = merge([{"junk": True}, None, "string", {"id": "ok", "name": "Ok", "rates_import": [{"rate": 5.0}], "rates_export": [{"rate": 5.0}]}]) + if "ok" not in [entry["id"] for entry in merged]: + print(" ERROR: {} the valid entry should survive alongside malformed ones".format(name)) + failed = True + + print("Test: match_entry finds the entry a tariff side was chosen from, by URL and by rates") + imports = merged_import_catalogue() + exports = merged_export_catalogue() + agile = [entry for entry in IMPORT_TARIFFS if entry["id"] == "agile"][0] + matched = match_entry(imports, {"import_octopus_url": agile["import_octopus_url"]}, "import_octopus_url", "rates_import") + if not matched or matched.get("id") != "agile": + print(" ERROR: a URL side should match its catalogue entry, got {}".format(matched)) + failed = True + price_cap = [entry for entry in IMPORT_TARIFFS if entry["id"] == BASELINE_DEFAULT_IMPORT_ID][0] + matched = match_entry(imports, {"rates_import": price_cap["rates_import"]}, "import_octopus_url", "rates_import") + if not matched or matched.get("id") != BASELINE_DEFAULT_IMPORT_ID: + print(" ERROR: a basic-rates side should match on its rates, got {}".format(matched)) + failed = True + no_export = [entry for entry in EXPORT_TARIFFS if entry["id"] == NO_EXPORT_ID][0] + matched = match_entry(exports, {"rates_export": no_export["rates_export"]}, "export_octopus_url", "rates_export") + if not matched or matched.get("id") != NO_EXPORT_ID: + print(" ERROR: a deliberate no-export side should match its own entry, got {}".format(matched)) + failed = True + + print("Test: match_entry returns None for a side that matches nothing, and for one that is not set") + # None rather than the Custom entry: Custom carries no rate source, so returning it + # would let a caller read a name off an entry that describes no actual tariff. + for tariff, label in [ + ({"import_octopus_url": "https://example.com/mine/"}, "a hand-entered URL"), + ({"rates_import": [{"rate": 99.5}]}, "a rate that is in no entry"), + ({}, "an unset side"), + (None, "no tariff at all"), + ]: + matched = match_entry(imports, tariff, "import_octopus_url", "rates_import") + if matched is not None: + print(" ERROR: {} should match nothing, got {}".format(label, matched)) + failed = True + + print("Test: match_entry matches each side against its own list, so the two cannot be confused") + # An export URL handed to the import side must not match: the pairing bug this + # catalogue replaced came from matching one side's value against the wrong list. + outgoing = [entry for entry in EXPORT_TARIFFS if entry.get("export_octopus_url")][0] + if match_entry(imports, {"import_octopus_url": outgoing["export_octopus_url"]}, "import_octopus_url", "rates_import") is not None: + print(" ERROR: an export URL should not match an import catalogue entry") + failed = True + + print("Test: match_entry sees the user's own compare_list entries, not just the built-ins") + # This is why naming happens where the merged catalogue is available: a user tariff + # would otherwise fall back to a bare product code despite having a name. + user_list = [{"id": "mine", "name": "My deal", "rates_import": [{"rate": 12.34}]}] + matched = match_entry(merged_import_catalogue(user_list), {"rates_import": [{"rate": 12.34}]}, "import_octopus_url", "rates_import") + if not matched or matched.get("name") != "My deal": + print(" ERROR: a compare_list entry should be matchable by name, got {}".format(matched)) failed = True - print("Test: a malformed user entry is skipped rather than breaking the dropdown") - merged = merged_catalogue([{"junk": True}, None, "string", {"id": "ok", "name": "Ok", "rates_import": [{"rate": 5.0}]}]) - ids = [entry["id"] for entry in merged] - if "ok" not in ids: - print(" ERROR: the valid entry should survive alongside malformed ones, ids were {}".format(ids)) + print("Test: the Custom placeholder is never returned even when a tariff carries nothing") + if any(match_entry(catalogue, {}, url_key, rates_key) for catalogue, url_key, rates_key in [(imports, "import_octopus_url", "rates_import"), (exports, "export_octopus_url", "rates_export")]): + print(" ERROR: an empty tariff should match nothing at all") failed = True return failed diff --git a/apps/predbat/tests/test_web_annual.py b/apps/predbat/tests/test_web_annual.py index 97e901330..9be86aa9a 100644 --- a/apps/predbat/tests/test_web_annual.py +++ b/apps/predbat/tests/test_web_annual.py @@ -20,7 +20,7 @@ from annual import AnnualConfigError, validate_config from annual_store import list_runs, load_run, save_run -from tariff_catalogue import CUSTOM_ID +from tariff_catalogue import BASELINE_DEFAULT_IMPORT_ID, CUSTOM_ID, EXPORT_TARIFFS, IMPORT_TARIFFS, NO_EXPORT_ID, PRICE_CAP_IMPORT_P from web import WebInterface from web_annual import DEFAULT_CONFIG, AnnualPage, _json_for_script @@ -116,7 +116,8 @@ def valid_postdata(): "load_shape": "flat", "load_car_charging_kwh": "2500", "load_car_rate_kw": "7.4", - "tariff_id": CUSTOM_ID, + "tariff_import_id": CUSTOM_ID, + "tariff_export_id": CUSTOM_ID, "tariff_import_url": "https://example.com/import/", "tariff_export_url": "https://example.com/export/", "tariff_standing_charge": "60.0", @@ -275,14 +276,20 @@ def test_web_annual(my_predbat): my_predbat.args.pop("rates_import_octopus_url", None) my_predbat.args.pop("rates_export_octopus_url", None) - print("Test: the catalogue merges the user's compare_list") + print("Test: the catalogues merge the user's compare_list, each side taking what it can use") my_predbat.args["compare_list"] = [{"id": "mine", "name": "My tariff", "rates_import_octopus_url": "https://example.com/x"}] - ids = [entry["id"] for entry in make_page(my_predbat).catalogue()] - if "mine" not in ids: - print(" ERROR: a user compare_list entry should appear in the catalogue, got {}".format(ids)) + import_ids = [entry["id"] for entry in make_page(my_predbat).import_catalogue()] + export_ids = [entry["id"] for entry in make_page(my_predbat).export_catalogue()] + if "mine" not in import_ids: + print(" ERROR: a user compare_list entry should appear in the import catalogue, got {}".format(import_ids)) failed = True - if "agile_agile" not in ids: - print(" ERROR: built-in entries should still be present, got {}".format(ids)) + # This entry supplies an import URL only, so offering it as an export tariff would + # price export against a source it does not have. + if "mine" in export_ids: + print(" ERROR: an import-only compare_list entry should not appear in the export catalogue, got {}".format(export_ids)) + failed = True + if "agile" not in import_ids or "outgoing_prime" not in export_ids: + print(" ERROR: built-in entries should still be present, got {} / {}".format(import_ids, export_ids)) failed = True print("Test: the default config validates on its own") @@ -385,13 +392,18 @@ def test_web_annual_form(my_predbat): failed = True my_predbat.args.pop("soc_max", None) - print("Test: the tariff dropdown lists the catalogue and a Custom entry") + print("Test: the tariff dropdowns list their catalogues and a Custom entry each") if CUSTOM_ID not in html: - print(" ERROR: the dropdown should offer a Custom entry") - failed = True - if "Agile import / Agile export" not in html: - print(" ERROR: the dropdown should list the built-in tariffs") + print(" ERROR: the dropdowns should offer a Custom entry") failed = True + for select_id, expected in [("tariff_import_id", "Octopus Agile"), ("tariff_export_id", "Octopus Agile Outgoing")]: + block = re.search(r'', combo_form, re.S) + if not import_select or not re.search(r'value="{}"[^>]*selected'.format(import_id), import_select.group(0)): + print(" ERROR: import {} should re-select itself on reload".format(import_id)) + failed = True + if not export_select or not re.search(r'value="{}"[^>]*selected'.format(export_id), export_select.group(0)): + print(" ERROR: export {} should re-select itself on reload".format(export_id)) + failed = True + + print("Test: two export tariffs sharing an import are told apart on reload") + # The single paired dropdown matched on the import URL alone, so "Agile / Prime" + # and "Agile / Fixed" were indistinguishable and a saved config came back as + # whichever appeared first in the list. Pin that it no longer can. + prime_post = valid_postdata() + prime_post.update({"tariff_import_id": "agile", "tariff_export_id": "outgoing_prime", "tariff_import_url": "", "tariff_export_url": ""}) + prime_form = make_page(my_predbat).render_form(make_page(my_predbat).config_from_post(prime_post)) + prime_select = re.search(r'', no_export_form, re.S) + if not no_export_select or not re.search(r'value="{}"[^>]*selected'.format(NO_EXPORT_ID), no_export_select.group(0)): + print(" ERROR: no export should re-select itself on reload") + failed = True + + print("Test: a config with no export source at all shows as 'no export' rather than Custom") + # A prefill from an Octopus account with an import tariff but no export agreement + # lands here. Falling through to Custom would open an empty URL box and read as a + # broken config rather than the ordinary situation it is. + bare_form = make_page(my_predbat).render_form({"tariff": {"import_octopus_url": "https://example.com/import/", "standing_charge_p_per_day": 60.0}}) + bare_select = re.search(r'', baseline_form, re.S) + if not baseline_select: + print(" ERROR: the baseline tariff dropdown should render") + failed = True + else: + # Custom is a hand-entered URL, which makes no sense for "what would they + # otherwise be on". + if 'value="{}"'.format(CUSTOM_ID) in baseline_select.group(0): + print(" ERROR: Custom should not be offered as a baseline tariff") + failed = True + if not re.search(r'value="price_cap"[^>]*selected', baseline_select.group(0)): + print(" ERROR: the baseline should default to the price cap, got {}".format(baseline_select.group(0)[:200])) + failed = True + + print("Test: a chosen baseline tariff reaches the config and validates") + baseline_post = valid_postdata() + baseline_post["baseline_tariff_id"] = "price_cap" + baseline_config = make_page(my_predbat).config_from_post(baseline_post) + if not (baseline_config.get("baseline_tariff") or {}).get("rates_import"): + print(" ERROR: the chosen baseline tariff should reach the config, got {}".format(baseline_config.get("baseline_tariff"))) + failed = True + validated = validate_config(baseline_config) + if not (validated.get("baseline_tariff") or {}).get("rates_import"): + print(" ERROR: the baseline tariff should survive validation, got {}".format(validated.get("baseline_tariff"))) + failed = True + + print("Test: a baseline matching no catalogue entry still marks an option selected") + # A baseline written by hand in YAML resolves to CUSTOM_ID, which this dropdown + # deliberately does not offer - so nothing was marked selected and the form + # depended on the browser showing the first option to display anything at all. + # Anything reading the HTML rather than rendering it saw a dropdown with no + # selection, and the page stated no baseline where it means the price cap. + custom_baseline = make_page(my_predbat).prefill_config() + custom_baseline["baseline_tariff"] = {"import_octopus_url": "https://api.octopus.energy/v1/products/MY-OWN-DEAL/x/", "rates_export": [{"rate": 4.1}]} + custom_select = re.search(r'', html_text, re.S) + return block.group(0) if block else "" + matched_config = copy.deepcopy(config) matched_config["tariff"] = {"import_octopus_url": built_in["import_octopus_url"], "standing_charge_p_per_day": 60.0} - matched_html = page.render_form(matched_config) + matched_html = import_select(page.render_form(matched_config)) matched_tag = option_tag(matched_html, built_in["id"]) if matched_tag is None or "selected" not in matched_tag: print(" ERROR: the option matching the config's import URL should be selected, got {}".format(matched_tag)) @@ -896,7 +1066,7 @@ def test_web_annual_form(my_predbat): custom_config = copy.deepcopy(config) custom_config["tariff"] = {"import_octopus_url": "https://example.com/not-in-the-catalogue/", "standing_charge_p_per_day": 60.0} - custom_html = page.render_form(custom_config) + custom_html = import_select(page.render_form(custom_config)) custom_tag = option_tag(custom_html, CUSTOM_ID) if custom_tag is None or "selected" not in custom_tag: print(" ERROR: a hand-entered URL with no catalogue match should select Custom, got {}".format(custom_tag)) @@ -1264,9 +1434,15 @@ def sample_run_results(): "solar": [{"kwp": 5.6, "declination": 35, "azimuth": 180}], "battery": {"size_kwh": 9.5, "inverter_kw": 5.0, "export_limit_kw": 5.0, "hybrid": True}, "load": {"annual_kwh": 3800, "shape": "night", "car_charging_kwh": 3000, "car_rate_kw": 7.4}, + # Templated, with dno_region beside them: results["config"] is the RAW config + # (annual.py stores self.config["raw"]), and AnnualTariff substitutes the + # region at fetch time without writing it back. A pre-substituted URL here + # would not be what a real run stores, and would quietly stop matching the + # catalogue that names it. "tariff": { - "import_octopus_url": "https://api.octopus.energy/v1/products/AGILE-24-10-01/electricity-tariffs/E-1R-AGILE-24-10-01-A/standard-unit-rates/", - "export_octopus_url": "https://api.octopus.energy/v1/products/OUTGOING-PRIME-FIX-12M-26-06-23/electricity-tariffs/E-1R-OUTGOING-PRIME-FIX-12M-26-06-23-A/standard-unit-rates/", + "import_octopus_url": "https://api.octopus.energy/v1/products/AGILE-24-10-01/electricity-tariffs/E-1R-AGILE-24-10-01-{dno_region}/standard-unit-rates/", + "export_octopus_url": "https://api.octopus.energy/v1/products/OUTGOING-PRIME-FIX-12M-26-06-23/electricity-tariffs/E-1R-OUTGOING-PRIME-FIX-12M-26-06-23-{dno_region}/standard-unit-rates/", + "dno_region": "A", "standing_charge_p_per_day": 60.0, }, "samples_per_month": 2, @@ -1330,10 +1506,33 @@ def test_web_annual_results(my_predbat): print("Test: a run states the key settings it actually used") details = page._render_run_details(results) - for expected in ["5.6 kWp", "9.5 kWh", "AGILE-24-10-01", "OUTGOING-PRIME-FIX-12M-26-06-23", "3,800 kWh a year", "more at night", "3,000 kWh a year", "60p a day"]: + for expected in ["5.6 kWp", "9.5 kWh", "Octopus Agile", "Octopus Outgoing Prime", "3,800 kWh a year", "more at night", "3,000 kWh a year", "60p a day"]: if expected not in details: print(" ERROR: the run details should state {}, got {}".format(expected, details)) failed = True + # The full URL stays as the cell's title for anyone checking the exact endpoint - the + # catalogue name replaces the product code on screen, it does not hide the source. + for expected in ["AGILE-24-10-01", "OUTGOING-PRIME-FIX-12M-26-06-23"]: + if expected not in details: + print(" ERROR: the run details should keep {} as the cell's title, got {}".format(expected, details)) + failed = True + + print("Test: the run details name the baseline tariff the saving is measured against") + # Without it the page states a saving but not what it is a saving FROM, which cannot + # be checked - and the compare table beside it does show the baseline. + baseline_run = copy.deepcopy(results) + baseline_run["config"]["baseline_tariff"] = {"rates_import": [{"rate": PRICE_CAP_IMPORT_P}]} + baseline_details = page._render_run_details(baseline_run) + if "Baseline tariff" not in baseline_details or "Price cap" not in baseline_details: + print(" ERROR: the run details should name the baseline tariff, got {}".format(baseline_details)) + failed = True + + print("Test: a run that recorded no baseline omits the row rather than inventing one") + no_baseline = copy.deepcopy(results) + no_baseline["config"].pop("baseline_tariff", None) + if "Baseline tariff" in page._render_run_details(no_baseline): + print(" ERROR: a run with no stored baseline should not show a baseline row") + failed = True print("Test: the details describe the RUN's own config, not the live form") # The selector can show a run from a completely different system; labelling it with @@ -1743,6 +1942,16 @@ def test_web_annual_pages(my_predbat): failed = True print("Test: the compare table lists every run with its own figures") + + def catalogue_entry(entries, entry_id): + """Return one built-in catalogue entry by id, so fixtures cannot drift from it.""" + return [entry for entry in entries if entry["id"] == entry_id][0] + + agile = catalogue_entry(IMPORT_TARIFFS, "agile") + cosy = catalogue_entry(IMPORT_TARIFFS, "cosy") + price_cap = catalogue_entry(IMPORT_TARIFFS, "price_cap") + outgoing_fixed = catalogue_entry(EXPORT_TARIFFS, "outgoing_fixed") + no_export = catalogue_entry(EXPORT_TARIFFS, NO_EXPORT_ID) runs = [ { "id": "20260728-0900", @@ -1750,12 +1959,15 @@ def test_web_annual_pages(my_predbat): # this test used a label like "9.5 kWh battery, 5.6 kWp, Agile", which meant # the Solar/Battery assertions below were satisfied by the label text alone - # deleting the Solar/Battery cells outright would still have passed. - "label": "System Alpha, Agile", + # It carries no tariff name either, for the same reason: the three tariff + # cells must be doing the work, not the label. + "label": "System Alpha", "summary": { "total_kwp": 5.6, "battery_kwh": 9.5, "total_gbp": 12250.0, - "tariff": "Agile", + "tariff": {"import_octopus_url": agile["import_octopus_url"], "export_octopus_url": outgoing_fixed["export_octopus_url"]}, + "baseline_tariff": {"rates_import": price_cap["rates_import"]}, "cost_with_predbat_p": 66000.0, "saving_vs_none_p": 114000.0, "payback_years": {"pv_only": 17.8, "pv_battery": 13.61, "pv_battery_predbat": 11.78}, @@ -1765,13 +1977,14 @@ def test_web_annual_pages(my_predbat): }, { "id": "20260728-0800", - "label": "System Beta, Cosy", + "label": "System Beta", "summary": { "total_kwp": 12.0, "battery_kwh": 20.0, "total_gbp": 25400.0, "quoted": True, - "tariff": "Cosy", + "tariff": {"import_octopus_url": cosy["import_octopus_url"], "rates_export": no_export["rates_export"]}, + "baseline_tariff": {"rates_import": price_cap["rates_import"]}, "cost_with_predbat_p": 40000.0, "saving_vs_none_p": 140000.0, "payback_years": {"pv_only": 9.1, "pv_battery": 8.2, "pv_battery_predbat": 7.0}, @@ -1781,11 +1994,93 @@ def test_web_annual_pages(my_predbat): }, ] table = page.render_compare(runs, "20260728-0900") - for expected in ["5.6 kWp", "9.5 kWh", "Agile", "13.6", "12 kWp", "20 kWh", "Cosy", "8.2"]: + for expected in ["5.6 kWp", "9.5 kWh", "Octopus Agile", "13.6", "12 kWp", "20 kWh", "Octopus Cosy", "8.2"]: if expected not in table: print(" ERROR: the compare table should show {}, got {}".format(expected, table)) failed = True + print("Test: the compare table has a column each for the baseline, import and export tariffs") + # One "Tariff" column could not describe a run once import and export became + # independent choices: two runs differing only in their export deal looked + # identical, and the baseline the saving is measured against was invisible. + header = re.search(r"]*>.*?", table, re.S).group(0) + for heading in ["Baseline", "Import", "Export"]: + if "{}".format(heading) not in header: + print(" ERROR: the compare table should have a {} column, got {}".format(heading, header)) + failed = True + if "Tariff" in header: + print(" ERROR: the single Tariff column should have been replaced by the three, got {}".format(header)) + failed = True + + print("Test: the tariff cells show the catalogue's own names, on the run that used them") + alpha_row, beta_row = [row for row in re.findall(r"]*>.*?", table, re.S) if "" not in row] + for row, label, expected_names, unexpected_names in [ + (alpha_row, "System Alpha", ["Price cap", "Octopus Agile", "Octopus Outgoing Fixed"], ["Octopus Cosy", "No export payment"]), + (beta_row, "System Beta", ["Price cap", "Octopus Cosy", "No export payment"], ["Octopus Agile", "Octopus Outgoing Fixed"]), + ]: + for name in expected_names: + if name not in row: + print(" ERROR: the {} row should name {}, got {}".format(label, name, row)) + failed = True + for name in unexpected_names: + if name in row: + print(" ERROR: the {} row should not carry {}, got {}".format(label, name, row)) + failed = True + + print("Test: a tariff that matches no catalogue entry falls back to something readable, not a blank") + # A hand-entered URL, or a compare_list entry the user has since deleted. The + # product code is the part of a 130-character URL anyone recognises. + odd = [ + { + "id": "odd", + "label": "System Delta", + "summary": { + "tariff": {"import_octopus_url": "https://api.octopus.energy/v1/products/MY-OWN-DEAL-24/electricity-tariffs/x/", "rates_export": [{"rate": 15.0}]}, + "baseline_tariff": {}, + "payback_years": {}, + }, + } + ] + odd_table = page.render_compare(odd, "odd") + if "MY-OWN-DEAL-24" not in odd_table: + print(" ERROR: an unmatched import URL should fall back to its product code, got {}".format(odd_table)) + failed = True + if "flat 15.0p" not in odd_table: + print(" ERROR: an unmatched export rate should be described rather than blanked, got {}".format(odd_table)) + failed = True + + print("Test: a run stored before the tariff fields renders as dashes rather than raising") + # backfill_summaries normally refreshes these, but it skips a run whose document has + # gone; the table must still draw rather than 500 the whole compare page. + legacy = [{"id": "legacy", "label": "System Epsilon", "summary": {"total_kwp": 4.0, "tariff": "Agile", "payback_years": {}}}] + legacy_table = page.render_compare(legacy, "legacy") + if "System Epsilon" not in legacy_table: + print(" ERROR: a legacy summary should still render its row, got {}".format(legacy_table)) + failed = True + + print("Test: the compare table's own compare_list tariffs are named, not reduced to a rate") + # The reason naming happens here rather than in annual_store: only the web layer can + # read compare_list, so a name computed at save time would be blind to the user's + # own tariffs. + my_predbat.args["compare_list"] = [{"id": "mine", "name": "My works deal", "rates_import": [{"rate": 11.11}]}] + mine = [{"id": "mine-run", "label": "System Zeta", "summary": {"tariff": {"rates_import": [{"rate": 11.11}]}, "baseline_tariff": {}, "payback_years": {}}}] + if "My works deal" not in make_page(my_predbat).render_compare(mine, "mine-run"): + print(" ERROR: a compare_list tariff should be named in the compare table") + failed = True + my_predbat.args.pop("compare_list", None) + + print("Test: only the data cells are held on one line, so the headers can wrap and the columns shrink") + # nowrap on the whole table forced every column to at least its header width on one + # line - "PV + battery pays back in" is ~24 characters holding open a column whose + # data is "4.2 years". Thirteen columns of that scrolled far further than they needed. + css = page.render_css() + if "table.annual-compare th" not in css.replace("\n", " "): + print(" ERROR: the compare table's headers should be allowed to wrap") + failed = True + if "table.annual-compare { white-space: nowrap; }" in css: + print(" ERROR: nowrap must not apply to the whole compare table, or the headers cannot wrap") + failed = True + print("Test: each run's figures land in its OWN row, not merely somewhere in the table") # A per-table substring check (above) cannot tell "each run got its own row" apart # from "the rows are swapped/shifted but every value still appears somewhere" - a @@ -1800,8 +2095,8 @@ def test_web_annual_pages(my_predbat): failed = True else: row_expectations = [ - ("System Alpha, Agile", ["5.6 kWp", "9.5 kWh", "Agile", "13.6"], ["Cosy", "8.2", "12 kWp", "20 kWh"]), - ("System Beta, Cosy", ["12 kWp", "20 kWh", "Cosy", "8.2"], ["Agile", "13.6", "5.6 kWp", "9.5 kWh"]), + ("System Alpha", ["5.6 kWp", "9.5 kWh", "Octopus Agile", "13.6"], ["Octopus Cosy", "8.2", "12 kWp", "20 kWh"]), + ("System Beta", ["12 kWp", "20 kWh", "Octopus Cosy", "8.2"], ["Octopus Agile", "13.6", "5.6 kWp", "9.5 kWh"]), ] for row, (label_fragment, must_contain, must_not_contain) in zip(data_rows, row_expectations): if label_fragment not in row: diff --git a/apps/predbat/web.py b/apps/predbat/web.py index 8ab82315e..7f88f9e0f 100644 --- a/apps/predbat/web.py +++ b/apps/predbat/web.py @@ -420,6 +420,7 @@ def _register_annual_routes(self, app): app.router.add_post("/annual", self.annual_page.html_annual_post) app.router.add_post("/annual_reset", self.annual_page.html_annual_reset) app.router.add_post("/annual_array", self.annual_page.html_annual_array) + app.router.add_post("/annual_delete", self.annual_page.html_annual_delete) app.router.add_get("/annual_cost_preview", self.annual_page.html_annual_cost_preview) app.router.add_post("/annual_run", self.annual_page.html_annual_run) app.router.add_get("/annual_status", self.annual_page.html_annual_status) diff --git a/apps/predbat/web_annual.py b/apps/predbat/web_annual.py index ebd393dbc..d4ab90f0a 100644 --- a/apps/predbat/web_annual.py +++ b/apps/predbat/web_annual.py @@ -28,8 +28,19 @@ from annual import AnnualConfigError, validate_config from annual_costs import DEFAULT_COSTS, build_costs, resolve_costs from annual_job import AnnualJob -from annual_store import backfill_summaries, list_runs, load_plan, load_run, save_run -from tariff_catalogue import CUSTOM_ID, merged_catalogue +from annual_store import backfill_summaries, delete_run, list_runs, load_plan, load_run, save_run +from tariff_catalogue import ( + BASELINE_DEFAULT_EXPORT_ID, + BASELINE_DEFAULT_IMPORT_ID, + CUSTOM_ID, + NO_EXPORT_ID, + PRICE_CAP_IMPORT_P, + PRICE_CAP_STANDING_CHARGE_P, + SEG_EXPORT_P, + match_entry, + merged_export_catalogue, + merged_import_catalogue, +) from web_helper import get_plan_css, get_plan_renderer_js # Validated with the dataviz palette checker in BOTH light and dark mode, all pairs. @@ -52,7 +63,7 @@ "solar": [{"kwp": 5.0, "declination": 35, "azimuth": 180, "efficiency": 0.95}], "battery": {"size_kwh": 9.5, "inverter_kw": 5.0, "export_limit_kw": 5.0, "hybrid": True}, "load": {"annual_kwh": 3800, "shape": "flat", "car_charging_kwh": 0, "car_rate_kw": 7.4}, - "tariff": {"rates_import": [{"rate": 24.86}], "rates_export": [{"rate": 4.1}], "standing_charge_p_per_day": 60.0}, + "tariff": {"rates_import": [{"rate": PRICE_CAP_IMPORT_P}], "rates_export": [{"rate": SEG_EXPORT_P}], "standing_charge_p_per_day": PRICE_CAP_STANDING_CHARGE_P}, "samples_per_month": 2, } @@ -252,21 +263,16 @@ def prefill_config(self): if dno_region: config["tariff"]["dno_region"] = dno_region - api_key, account_id = self._octopus_from_args() - if api_key and account_id: - # Both are needed to download consumption - a key without an account (or the - # reverse) cannot fetch anything, so only a complete pair is worth offering. - # Selecting the Octopus source as well as filling the fields is the point: - # this user has real metered consumption available, which models their year - # far better than the synthetic annual-kWh profile the default falls back to. - # - # Replaces the load block rather than adding to it: _validate_load treats - # octopus and annual_kwh/car_charging_kwh as mutually exclusive (the metered - # series already includes car charging, so carrying both would double-count - # it) and rejects a config holding both. render_form falls back to - # DEFAULT_CONFIG for the manual inputs, so they still show sensible values if - # the user switches the radio back to "Enter my usage". - config["load"] = {"octopus": {"api_key": str(api_key), "account_id": str(account_id)}, "shape": config["load"].get("shape", "flat")} + # The load source is deliberately NOT prefilled from the Octopus credentials, even + # when a complete pair is configured. Octopus gives us the IMPORT meter - what was + # bought from the grid - and on a home that already has solar or a battery, that + # system's self-consumption and discharge have already been subtracted from every + # reading; modelling a system on top counts the same saving twice. The form says so + # in its own banner. Anyone whose apps.yaml holds Octopus credentials is by + # definition a configured Predbat with a battery or an array, so auto-selecting it + # picked the one source that is wrong for them. Predicted consumption stays the + # default and the choice is left to the user; render_form still fills the key and + # account boxes from these args, so switching is a click rather than a paste. return config @@ -281,9 +287,13 @@ def _octopus_from_args(self): """ return self._arg("octopus_api_key", None, indirect=False), self._arg("octopus_api_account", None, indirect=False) - def catalogue(self): - """Return the tariff dropdown entries: built-ins merged with the user's own.""" - return merged_catalogue(self._arg("compare_list", None)) + def import_catalogue(self): + """Return the import tariff dropdown entries: built-ins merged with the user's own.""" + return merged_import_catalogue(self._arg("compare_list", None)) + + def export_catalogue(self): + """Return the export tariff dropdown entries: built-ins merged with the user's own.""" + return merged_export_catalogue(self._arg("compare_list", None)) def _config_path(self): """Return the path of the saved annual configuration.""" @@ -338,6 +348,50 @@ def _text_field(self, name, label, value, wide=False): name=name, label=label, wide=" annual-field-wide" if wide else "", value=html.escape(str(value), quote=True) if value is not None else "" ) + @staticmethod + def _selected_side_id(tariff, catalogue, url_key, rates_key): + """Return the catalogue id matching one side of this tariff, Custom, or None if unset. + + The matching itself is ``tariff_catalogue.match_entry``, shared with the compare + table so the dropdown and the table cannot describe the same tariff differently. + What is added here is the dropdown's own two special cases: None for a side that + was never set, so the caller can leave the select alone, and Custom for one that + matches no entry - a hand-entered URL is exactly what Custom means, and falling + back to it beats leaving the dropdown showing the first entry in the list. + """ + tariff = tariff or {} + if not tariff.get(url_key) and not tariff.get(rates_key): + return None + entry = match_entry(catalogue, tariff, url_key, rates_key) + return entry["id"] if entry else CUSTOM_ID + + @staticmethod + def _tariff_select(name, label, catalogue, selected_id, url_key): + """Return one tariff dropdown as HTML. + + The URL each entry carries rides along in ``data-url`` so picking Custom can + start from the tariff that was on screen rather than from an empty box. + """ + text = '
\n" + + @classmethod + def _selected_import_id(cls, tariff, catalogue): + """Return the import catalogue id matching this tariff, or Custom, or None if unset.""" + return cls._selected_side_id(tariff, catalogue, "import_octopus_url", "rates_import") + + @classmethod + def _selected_export_id(cls, tariff, catalogue): + """Return the export catalogue id matching this tariff, or Custom, or None if unset.""" + return cls._selected_side_id(tariff, catalogue, "export_octopus_url", "rates_export") + def render_form(self, config, errors=None): """Return the configuration form as HTML, populated from ``config``. @@ -477,46 +531,54 @@ def render_form(self, config, errors=None): text += "\n" text += "
Tariff\n" - text += '
\n" - # The URL boxes belong to Custom alone. The dropdown is authoritative for every - # other entry (see config_from_post), so leaving them on screen for a built-in + # Import and export are picked separately so any combination can be modelled - + # including an import tariff paired with no export payment at all, which is what + # a household without an export agreement is actually on. + import_catalogue = self.import_catalogue() + export_catalogue = self.export_catalogue() + selected_import = self._selected_import_id(tariff, import_catalogue) + # A tariff carrying no export source at all means export is not paid for, which is + # a real selection rather than an absent one. Stated explicitly so the default does + # not rest on "No export payment" happening to be first in the list. + selected_export = self._selected_export_id(tariff, export_catalogue) or NO_EXPORT_ID + text += self._tariff_select("tariff_import_id", "Import tariff", import_catalogue, selected_import, "import_octopus_url") + # The URL box belongs to Custom alone. The dropdown is authoritative for every + # other entry (see config_from_post), so leaving it on screen for a built-in # tariff invites someone to edit a value that is then ignored - and worse, used # to look like the selection when it no longer matches it. - text += '
\n'.format("block" if selected_id == CUSTOM_ID else "none") + text += '
\n'.format("block" if selected_import == CUSTOM_ID else "none") text += self._text_field("tariff_import_url", "Import rates URL", tariff.get("import_octopus_url", ""), wide=True) + text += "
\n" + text += self._tariff_select("tariff_export_id", "Export tariff", export_catalogue, selected_export, "export_octopus_url") + text += '
\n'.format("block" if selected_export == CUSTOM_ID else "none") text += self._text_field("tariff_export_url", "Export rates URL", tariff.get("export_octopus_url", ""), wide=True) text += "
\n" text += self._text_field("tariff_dno_region", "Octopus region letter", tariff.get("dno_region", "")) - text += self._number_field("tariff_standing_charge", "Standing charge", tariff.get("standing_charge_p_per_day", 60.0), suffix="p/day") + baseline = config.get("baseline_tariff") or {} + # Falls back to the price cap when the config carries no baseline - a config + # saved before this existed, or a fresh prefill. Matches validate_config's own + # default, so what the form shows is what a run would actually use. + # + # CUSTOM_ID counts as "no baseline" here, because this dropdown deliberately has + # no Custom option (the baseline answers "what would they otherwise be on", so a + # hand-entered URL has no place in it - see config_from_post). A hand-edited YAML + # baseline matching no entry therefore left NOTHING marked selected, and the form + # relied on the browser falling back to the first option to show anything at all. + # Naming the default explicitly is what makes the rendered form state its + # selection rather than imply it. + baseline_id = self._selected_import_id(baseline, import_catalogue) + if not baseline_id or baseline_id == CUSTOM_ID: + baseline_id = BASELINE_DEFAULT_IMPORT_ID + text += '
\n" + # Import only, deliberately: with no PV and no battery there is nothing to export, + # so an export tariff for this scenario would be a control that changes no figure. + text += '

Used only for the no-PV/battery comparison, which has nothing to export - so only the import side applies. Someone with no system would not be on a battery tariff, so pricing that scenario on one credits it with a saving it could never have had. The price cap is the sensible default.

\n' + text += self._number_field("tariff_standing_charge", "Standing charge", tariff.get("standing_charge_p_per_day", PRICE_CAP_STANDING_CHARGE_P), suffix="p/day") text += "
\n" # The estimate is filled in by JavaScript from ./annual_cost_preview rather than @@ -655,20 +717,25 @@ def numeric(name, default=None): # below then silently ran the run at the price-cap default instead of the tariff # named in the dropdown - a different set of rates to the one on screen. tariff = {"standing_charge_p_per_day": numeric("tariff_standing_charge", 0)} - selected_tariff = value("tariff_id") or CUSTOM_ID - if selected_tariff != CUSTOM_ID: - for entry in self.catalogue(): - if entry.get("id") != selected_tariff: + for field, catalogue, url_key, rates_key, url_field in [ + ("tariff_import_id", self.import_catalogue(), "import_octopus_url", "rates_import", "tariff_import_url"), + ("tariff_export_id", self.export_catalogue(), "export_octopus_url", "rates_export", "tariff_export_url"), + ]: + selected = value(field) or CUSTOM_ID + if selected == CUSTOM_ID: + if value(url_field): + tariff[url_key] = value(url_field) + continue + for entry in catalogue: + if entry.get("id") != selected: continue - for key in ["import_octopus_url", "export_octopus_url", "rates_import", "rates_export"]: - if entry.get(key): + for key in [url_key, rates_key]: + # `is not None` rather than truthiness: "No export payment" is + # rates_export [{"rate": 0.0}], a perfectly real selection, and a + # rate of zero must not be dropped here as if nothing were chosen. + if entry.get(key) is not None: tariff[key] = copy.deepcopy(entry[key]) break - else: - if value("tariff_import_url"): - tariff["import_octopus_url"] = value("tariff_import_url") - if value("tariff_export_url"): - tariff["export_octopus_url"] = value("tariff_export_url") if value("tariff_dno_region"): tariff["dno_region"] = value("tariff_dno_region") @@ -679,6 +746,25 @@ def numeric(name, default=None): tariff["rates_export"] = DEFAULT_CONFIG["tariff"]["rates_export"] config["tariff"] = tariff + # The baseline is chosen from the catalogue only - it answers "what would they + # otherwise be on", so a hand-entered URL has no place in it. An unrecognised id + # is left out entirely, and validate_config then supplies the price-cap default + # rather than this silently inventing rates. + baseline_id = value("baseline_tariff_id") + if baseline_id and baseline_id != CUSTOM_ID: + for entry in self.import_catalogue(): + if entry.get("id") == baseline_id: + baseline = {key: copy.deepcopy(entry[key]) for key in ["import_octopus_url", "rates_import"] if entry.get(key)} + # The scenario exports nothing, so this export side is inert - it is + # carried only so the baseline tariff has the same shape as any other + # and AnnualTariff has a defined export source rather than none. + for export_entry in self.export_catalogue(): + if export_entry["id"] == BASELINE_DEFAULT_EXPORT_ID: + baseline.update({key: copy.deepcopy(export_entry[key]) for key in ["export_octopus_url", "rates_export"] if export_entry.get(key)}) + break + config["baseline_tariff"] = baseline + break + if value("year"): config["year"] = numeric("year") config["samples_per_month"] = numeric("samples_per_month", 2) @@ -895,6 +981,17 @@ def number(name): costs = build_costs(number("total_kwp"), number("battery_kwh"), settings) return web.json_response(costs) + async def html_annual_delete(self, request): + """Delete one stored run and return to the comparison table. + + Redirects rather than rendering in place so a refresh cannot re-post the + deletion - by then the run is gone, and the second attempt would report a + failure for something that already succeeded. + """ + postdata = await request.post() + await delete_run(self._storage(), str(postdata.get("run", ""))) + raise web.HTTPFound("./annual_compare") + async def html_annual_array(self, request): """Add or remove a solar array and re-render the form. @@ -934,9 +1031,13 @@ async def html_annual_reset(self, request): "Default" here means ``prefill_config()``, not the bare example: it reads whatever this Predbat knows - solar arrays, battery and inverter sizes, tariff - URLs, region, Octopus credentials - and fills only the gaps with the example UK - system. So on a configured instance this resets to *your* setup rather than to a - stranger's, which is the point of offering it. + URLs, region - and fills only the gaps with the example UK system. So on a + configured instance this resets to *your* setup rather than to a stranger's, + which is the point of offering it. + + The load source is the exception: a reset always returns it to predicted + consumption rather than the Octopus import meter, which is only sound for a home + with no solar or battery fitted. See ``prefill_config()``. The result is saved, not merely displayed: a reset the user has to remember to confirm with Save would leave the old configuration on disk and running, which is @@ -1022,12 +1123,44 @@ def _describe_tariff_side(tariff, url_key, rates_key): rates = tariff.get(rates_key) if isinstance(rates, list) and rates: values = [entry.get("rate") for entry in rates if isinstance(entry, dict) and entry.get("rate") is not None] + # "flat 0p" is accurate but reads like a missing figure. Naming the choice + # keeps a deliberate no-export run distinguishable from a broken one. + if values and set(values) == {0} and rates_key == "rates_export": + return "no export payment", "" if values and len(set(values)) == 1: return "flat {}p".format(values[0]), "" if values: return "{} rates, {}p to {}p".format(len(values), min(values), max(values)), "" return "not set", "" + @classmethod + def _name_tariff_side(cls, catalogue, tariff, url_key, rates_key): + """Return (display text, title) naming one side of a tariff from the catalogue. + + The catalogue's own name first - "Octopus Agile", "Price cap", "No export + payment" - because that is the wording the user picked on the form, and it is + the only thing that can describe an entry carrying no URL to parse. The user's + own ``compare_list`` tariffs are named too, which is precisely why this happens + here rather than at save time: only the web layer can read ``compare_list``. + + A side matching nothing - a hand-entered URL, or a ``compare_list`` entry since + deleted - falls through to ``_describe_tariff_side``, which digs out whatever is + readable rather than leaving the cell blank. + + The full URL is still carried as the title in both cases, so naming a tariff + never hides which endpoint it actually priced against. + """ + tariff = tariff or {} + entry = match_entry(catalogue, tariff, url_key, rates_key) + if entry: + return html.escape(entry["name"], quote=True), html.escape(str(tariff.get(url_key) or ""), quote=True) + return cls._describe_tariff_side(tariff, url_key, rates_key) + + @staticmethod + def _tariff_cell(value, title): + """Return one tariff cell's HTML, wrapped in a title only when there is one to show.""" + return '{}'.format(title, value) if title else value + def _render_run_details(self, results): """Return a small table of the key settings this run actually used. @@ -1086,11 +1219,22 @@ def _render_run_details(self, results): car = "{:,.0f} kWh a year at {:g} kW".format(car_kwh, float(load.get("car_rate_kw", 7.4) or 7.4)) if car_kwh > 0 else "none" rows.append(("Car charging", html.escape(car, quote=True))) + # The baseline is what the no-PV/battery scenario was priced on, and therefore + # what every saving on this page is measured FROM. Stating the saving without it + # leaves a figure nobody can check. Only shown when the run recorded one - an + # older run did not, and inventing today's default for it would be a guess about + # what that run actually did. + baseline = config.get("baseline_tariff") + if isinstance(baseline, dict) and baseline: + rows.append(("Baseline tariff", self._tariff_cell(*self._name_tariff_side(self.import_catalogue(), baseline, "import_octopus_url", "rates_import")))) + tariff = config.get("tariff") or {} if isinstance(tariff, dict): - for label, url_key, rates_key in [("Import tariff", "import_octopus_url", "rates_import"), ("Export tariff", "export_octopus_url", "rates_export")]: - value, title = self._describe_tariff_side(tariff, url_key, rates_key) - rows.append((label, '{}'.format(title, value) if title else value)) + for label, catalogue, url_key, rates_key in [ + ("Import tariff", self.import_catalogue(), "import_octopus_url", "rates_import"), + ("Export tariff", self.export_catalogue(), "export_octopus_url", "rates_export"), + ]: + rows.append((label, self._tariff_cell(*self._name_tariff_side(catalogue, tariff, url_key, rates_key)))) if tariff.get("standing_charge_p_per_day") is not None: rows.append(("Standing charge", html.escape("{:g}p a day".format(float(tariff["standing_charge_p_per_day"])), quote=True))) @@ -1439,21 +1583,32 @@ def render_compare(self, runs, selected_id): if not runs: return '

No runs have been stored yet — run some simulations to compare them here.

\n' + # Built once and passed down rather than per row: each merge walks the built-ins + # and the user's compare_list, and twenty rows would repeat that work twenty times + # for an answer that cannot change between them. + catalogues = (self.import_catalogue(), self.export_catalogue()) + text = '
\n\n' text += ( - "" "\n" + "" + "" + "\n" ) for run in runs: - text += self._render_compare_row(run, selected_id) + text += self._render_compare_row(run, selected_id, catalogues) text += "
RunSolarBatterySystem costTariffCost with PredbatSaving vs no systemPV only pays back inPV + battery pays back inWith Predbat pays back in
RunSolarBatterySystem costBaselineImportExportCost with PredbatSaving vs no systemPV only pays back inPV + battery pays back inWith Predbat pays back in
\n
\n" return text - def _render_compare_row(self, run, selected_id): + def _render_compare_row(self, run, selected_id, catalogues): """Return one comparison row, reading entirely from ``run``'s own summary. Reading anything other than ``run["summary"]`` here - another run in the list, or the live configuration form - has already been a defect once on this branch: it misattributes every figure on the row to the wrong system. + + ``catalogues`` is the (import, export) pair used to name the three tariffs; it + describes the tariffs on offer, not any one run, so sharing it across rows cannot + leak one run's figures into another's. """ summary = run.get("summary") or {} run_id = str(run.get("id", "")) @@ -1474,14 +1629,35 @@ def _render_compare_row(self, run, selected_id): else: price = "—" text += "{}\n".format(price) - tariff = summary.get("tariff") - text += "{}\n".format(html.escape(str(tariff), quote=True) if tariff else "—") + import_catalogue, export_catalogue = catalogues + # Baseline first: it is what the other two are being judged against, so reading + # left to right gives "instead of this, on these, it costs...". + for catalogue, tariff, url_key, rates_key in [ + (import_catalogue, summary.get("baseline_tariff"), "import_octopus_url", "rates_import"), + (import_catalogue, summary.get("tariff"), "import_octopus_url", "rates_import"), + (export_catalogue, summary.get("tariff"), "export_octopus_url", "rates_export"), + ]: + # A summary written before these fields existed holds a plain string here + # rather than a dict. backfill_summaries normally refreshes those, but it + # skips a run whose document has gone, so the table must still draw a row for + # one rather than failing the whole page over a value it cannot describe. + if not isinstance(tariff, dict) or not tariff: + text += "—\n" + continue + text += "{}\n".format(self._tariff_cell(*self._name_tariff_side(catalogue, tariff, url_key, rates_key))) text += "{}\n".format(self._compare_money(summary.get("cost_with_predbat_p"))) text += "{}\n".format(self._compare_money(summary.get("saving_vs_none_p"))) payback_years = summary.get("payback_years") or {} payback_reason = summary.get("payback_reason") for key in ["pv_only", "pv_battery", "pv_battery_predbat"]: text += self._render_payback_cell(payback_years, payback_reason, key) + # Its own little form: a button cannot post on its own, and wrapping the whole + # table in one form would make every row's button submit the same run id. + # Confirms first - a deleted run cannot be recovered, only re-run. + text += '
' + text += ''.format(html.escape(run_id, quote=True)) + text += '' + text += "
\n" text += "\n" return text @@ -1621,11 +1797,21 @@ def render_css(self): .annual-caveats li { margin-bottom: 0.35rem; } .annual-unavailable { opacity: 0.6; font-style: italic; } .annual-synthesised-tag { font-size: 0.75rem; font-weight: 600; color: #D55E00; border: 1px solid #D55E00; border-radius: 3px; padding: 0.05rem 0.35rem; margin-left: 0.35rem; } -/* Nine columns is wide - the table scrolls within its own container instead of +/* Thirteen columns is wide - the table scrolls within its own container instead of forcing the whole page wider, which would otherwise push the nav strip and everything else sideways on anything narrower than a desktop monitor. */ .annual-compare-scroll { overflow-x: auto; } -table.annual-compare { white-space: nowrap; } +/* nowrap on the DATA only. Applied to the whole table it caught the headers too, and + since a column can be no narrower than its widest unbreakable content, every column + was held open at its heading set on one line - "PV + battery pays back in" is ~24 + characters propping open a column whose data reads "4.2 years". Letting the headings + wrap onto two or three short lines lets each column shrink to the figures it holds, + which is what keeps three tariff columns affordable. */ +table.annual-compare td { white-space: nowrap; } +table.annual-compare th { white-space: normal; vertical-align: bottom; } +/* The headings are now the tall part of the table, so the padding that suited a + single-line row is more than they need between columns. */ +table.annual-compare th, table.annual-compare td { padding: 4px 8px 4px 4px; } .annual-compare-current { font-weight: 600; } """ @@ -1642,16 +1828,22 @@ def render_progress(self): def render_script(self): """Return the polling and tariff-picker script.""" return """