From 63dda4b0d34b186287a2f3565e6b9481e4e2dd78 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Sun, 1 Nov 2026 03:02:51 +0100 Subject: [PATCH] MAINT-20 part 1 review: restore three lost lines, rewrap ragged comments Comment, docstring and DECISIONS only (same-code script OK; python -OO bytecode now identical for all six files). - peakguard.charge_room_w: one-line docstring + SAFETY-125 pointer (cut whole). - peakguard.shaping_suspended: keep 'the one place this conjunction is written; do not inline it'. - price_forecast module note: an hour with no price and no yesterday is left out, never filled (test_price.py 14d cites the module note for it). - Refill comment blocks the cut left ragged; DECISIONS pointers on one line. - FCST-04 note: _yesterday_of(), not _previous_day(); FCST-03 note: the 49 stronger cases are why the rule is a real change. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC --- .../FCST-03-the-hours-not-yet-published.md | 3 +- .../FCST-04-the-day-that-is-carried.md | 2 +- goodwe_controller/app/custom_formula.py | 6 +- goodwe_controller/app/peakguard.py | 17 +++-- goodwe_controller/app/price.py | 73 ++++++++----------- goodwe_controller/app/price_forecast.py | 9 ++- goodwe_controller/app/price_history.py | 29 +++----- 7 files changed, 61 insertions(+), 78 deletions(-) diff --git a/goodwe_controller/DECISIONS/FCST-03-the-hours-not-yet-published.md b/goodwe_controller/DECISIONS/FCST-03-the-hours-not-yet-published.md index a238a1e..90b0ad7 100644 --- a/goodwe_controller/DECISIONS/FCST-03-the-hours-not-yet-published.md +++ b/goodwe_controller/DECISIONS/FCST-03-the-hours-not-yet-published.md @@ -52,7 +52,8 @@ files. It cannot be made complete from its arguments: `published` is over the whole unwindowed read while the fill emits `[since, until)`, so a published point outside the window is legitimately absent. FCST-05 built "every published point is emitted" (+162 B) and it took the 1 h, 12 h and 48 h horizon checks red on a -healthy feed (49 cases scored stronger), so the rule as stated is wrong, and +healthy feed (a real change: the differential scored it 49 cases stronger), +so the rule as stated is wrong, and asserting a completeness the instrument lacks is MUTATION.md habit 16's defect. ## The fill keys on the wall clock diff --git a/goodwe_controller/DECISIONS/FCST-04-the-day-that-is-carried.md b/goodwe_controller/DECISIONS/FCST-04-the-day-that-is-carried.md index 63258c9..2576c03 100644 --- a/goodwe_controller/DECISIONS/FCST-04-the-day-that-is-carried.md +++ b/goodwe_controller/DECISIONS/FCST-04-the-day-that-is-carried.md @@ -205,7 +205,7 @@ cache on the ephemeral layer. "Cannot persist" is one question asked after the attempt: see [[FCST-06]]; `self._days` never holds a point the file does not. **Brussels date, not UTC.** 23:30 UTC in July is already tomorrow in Belgium. -`_previous_day()` subtracts on the local `date`, not the instant: the old +`_yesterday_of()` subtracts on the local `date`, not the instant: the old `brussels(stamp) - timedelta(days=1)` took 24 absolute hours and, at 2026-03-30T00:30+02:00, returned 2026-03-28 (a two-day-old profile flagged as yesterday's). Rows are keyed by instant, not local hour, so the two 02:00s of diff --git a/goodwe_controller/app/custom_formula.py b/goodwe_controller/app/custom_formula.py index e6f88c8..638be6c 100644 --- a/goodwe_controller/app/custom_formula.py +++ b/goodwe_controller/app/custom_formula.py @@ -48,8 +48,7 @@ INDEXES: dict = { # 0.01 in the card's own units, the last digit both real cards print (Eneco # 15,55 vs 15.5471; ENGIE 7,522 vs 7.5218): the card's rounding and nothing -# more. -# DECISIONS/TARIFF-16-the-injection-leg.md. +# more. DECISIONS/TARIFF-16-the-injection-leg.md. CARD_TOLERANCE = 0.01 # The all-in plausibility band, EUR/kWh: the low end, where a factor-1000 error @@ -346,8 +345,7 @@ def parse_price_formula(value, *, option: str = "price_formula"): # The card self-check still runs wherever there is a card, with the same # tolerance. Where the card prints no estimate, `implausible_injection()` - # bounds - # the coefficients and a startup line says the leg is ungraded. + # bounds the coefficients and a startup line says the leg is ungraded. if spec.card_graded: delta = card_discrepancy(spec) if abs(delta) > CARD_TOLERANCE: diff --git a/goodwe_controller/app/peakguard.py b/goodwe_controller/app/peakguard.py index 238a9ba..b189970 100644 --- a/goodwe_controller/app/peakguard.py +++ b/goodwe_controller/app/peakguard.py @@ -29,8 +29,7 @@ FLOOR_AT_SOC_FLOOR = "the battery level is at or below the peak reserve" SOURCE = "peak" # Mirror of config.yaml's peak options, pinned to the file by -# test_peakguard.py. -# Not a read: `decide()` takes them as arguments. +# test_peakguard.py. Not a read: `decide()` takes them as arguments. PEAK_OPTION_DEFAULTS = { # Percent a forced discharge may not go below. 20 is derived from the # reference pack, 300 Ah at 51.2 V with DoD 90 - 13.8 kWh usable (a test @@ -42,8 +41,7 @@ PEAK_OPTION_DEFAULTS = { # SAFETY-79. Watts the guard aims under `peak_cap_w`. "peak_margin_w": 50, } -# SAFETY-79. Range of `peak_margin_w` in config.yaml, pinned by -# test_peakguard.py. +# SAFETY-79. Range of `peak_margin_w` in config.yaml; test_peakguard.py pins it. PEAK_MARGIN_RANGE_W = (0, 500) @@ -75,8 +73,8 @@ class PeakDecision: held: tuple = () able_w: float | None = None # SAFETY-126: power is wanted but the quarter still ends within - # ALARM_MISS_W - # of the aim with the batteries at 0 W (`misses_aim()`). Set by main.py. + # ALARM_MISS_W of the aim with the batteries at 0 W (`misses_aim()`). Set + # by main.py. within_aim: bool = False def explain(self) -> str: @@ -422,6 +420,9 @@ def charge_headroom_w(decision) -> float | None: def charge_room_w(decision, projection) -> float | None: + """SAFETY-125: `charge_headroom_w()` less the unread seconds counted at the + last reading, so a late reading is not room for a grid charge. + DECISIONS/SAFETY-125-grid-charge-under-the-month-peak.md.""" room = charge_headroom_w(decision) if not room or projection is None or projection.block_start is None: return room @@ -437,7 +438,9 @@ def shaping_suspended(decision, peak_forecast_w, cap_w) -> bool: Crossing limits resolve to 0 W, so the rule is suppression, not priority. Both terms are needed: the forecast entity is empty on a default install, where the measured guard fires. Dropping either parks the battery in the - billed quarter (test_peakguard.py part 7). DECISIONS/SAFETY-07-peak-guard.md.""" + billed quarter (test_peakguard.py part 7). The one place this conjunction is + written, public so the test grades the shipped expression: do not inline it. + DECISIONS/SAFETY-07-peak-guard.md.""" if peak_at_risk(peak_forecast_w, float(cap_w)): return True return decision is not None and decision.engaged diff --git a/goodwe_controller/app/price.py b/goodwe_controller/app/price.py index 7eda081..b2f6d0a 100644 --- a/goodwe_controller/app/price.py +++ b/goodwe_controller/app/price.py @@ -40,8 +40,8 @@ from .custom_formula import ( _LOG = logging.getLogger("goodwe.price") -# Brussels wall time, without a tz database (DECISIONS/TARIFF-02-price- -# provider-plugins.md). +# Brussels wall time, without a tz database. +# DECISIONS/TARIFF-02-price-provider-plugins.md. CET = timezone(timedelta(hours=1)) CEST = timezone(timedelta(hours=2)) @@ -176,9 +176,8 @@ class PriceProduct(Enum): # specs.md 8.3: real-time / imbalance products are high-volatility and widen -# the -# arbitrage thresholds. The ONLY place that says which products are volatile. -# OPT-04 reads it; nothing checks plugin ids. +# the arbitrage thresholds. The ONLY place that says which products are +# volatile. OPT-04 reads it; nothing checks plugin ids. _WIDE = frozenset({PriceProduct.REALTIME}) @@ -194,8 +193,8 @@ def volatility_regime(product: PriceProduct) -> str: # FCST-03: ONE widening path, driven by a SET of active uncertainty sources # (real-time product 8.3, no weather data 11.4). The combination rule is `max`, # not a product. The factors are policy defaults, not derived, and they differ -# so the combination rules stay distinguishable (DECISIONS/TARIFF-02-price- -# provider-plugins.md). +# so the combination rules stay distinguishable. +# DECISIONS/TARIFF-02-price-provider-plugins.md. class Uncertainty(Enum): """Why an arbitrage threshold is currently wider than the tariff alone says.""" REALTIME_PRODUCT = "realtime_product" # §9.2 row 2, §8.3 @@ -460,9 +459,9 @@ def plan_is_provisional(read: PriceRead) -> bool: # Each row is one line of the Eneco residential card for Flanders, September # 2026, including 6 % VAT, in EUR/kWh. The default of `price_taxes_eur_kwh` is # their sum (test_price.py section 1b re-adds them). Per DSO, and set near the -# low -# end of the band on purpose; `energiebijdrage` is a zero row since 2026-08-01. -# Why: DECISIONS/TARIFF-02-price-provider-plugins.md, "Taxes components". +# low end of the band on purpose; `energiebijdrage` is a zero row since +# 2026-08-01. Why: DECISIONS/TARIFF-02-price-provider-plugins.md, "Taxes +# components". FLEMISH_TAXES_COMPONENTS_EUR_KWH = ( ("Fluvius distribution, Antwerpen", 0.0535), ("Bijzondere accijns, 'Alle verbruik' - no volume bands", 0.04876), @@ -472,17 +471,14 @@ FLEMISH_TAXES_COMPONENTS_EUR_KWH = ( # TARIFF-18: moving this default changes which configs start. validate.py # (TARIFF-17 rung) compares `price_taxes_eur_kwh` to the default, so a -# superseded -# default (0.06) must still read as untouched. Do not change a default without -# that check. DECISIONS/TARIFF-02-price-provider-plugins.md. +# superseded default (0.06) must still read as untouched. Do not change a +# default without that check. DECISIONS/TARIFF-02-price-provider-plugins.md. SUPERSEDED_TAXES_DEFAULTS_EUR_KWH = (0.06,) # TARIFF-02 .. TARIFF-17 # CFG-03. The one in-code mirror of config.yaml for the price options: -# validate.py -# imports it, main.py has no price `.get()` fallbacks, and test_price.py -# section 1 -# pins it to config.yaml key for key. +# validate.py imports it, main.py has no price `.get()` fallbacks, and +# test_price.py section 1 pins it to config.yaml key for key. PRICE_OPTION_DEFAULTS = { "price_provider": "off", "price_markup_eur_kwh": 0.03, # §14 tariffs.energy_price.markup_eur_kwh @@ -498,14 +494,13 @@ PRICE_OPTION_DEFAULTS = { "price_horizon_h": 48, "price_timeout_s": 10, # TARIFF-03. A JSON static day/night or fixed tariff; "" is unset. Parsed - # by - # app/static_tariff.py, refused at startup by validate.py. Here, not in + # by app/static_tariff.py, refused at startup by validate.py. Here, not in # static_tariff.py, so there is one defaults mirror (CFG-03). "static_profile": "", # TARIFF-17. A period table in `static_profile`'s grammar whose - # `eur_per_kwh` - # is the TAXES component per band; "" is unset and the scalar decides. In - # FRAMEWORK_KEYS, so a plugin cannot add taxes a second time per hour. + # `eur_per_kwh` is the TAXES component per band; "" is unset and the scalar + # decides. In FRAMEWORK_KEYS, so a plugin cannot add taxes a second time + # per hour. "price_taxes_profile": "", # TARIFF-15. One contract's two coefficients, units, VAT rate and the card # figures they are graded against; "" is unset. Parsed by @@ -513,15 +508,13 @@ PRICE_OPTION_DEFAULTS = { "price_formula": "", # TARIFF-16. The INJECTION leg in `price_formula`'s own grammar, read by # `custom_formula.parse_export_formula()`; "" is unset and - # `export_price_eur_kwh` - # decides. In FRAMEWORK_KEYS, so a consumption plugin cannot fold it in. + # `export_price_eur_kwh` decides. In FRAMEWORK_KEYS, so a consumption + # plugin cannot fold it in. "price_export_formula": "", # OPT-04, 10.1 priority 3, and `optimizer.cycle_cost_threshold_eur`. The - # two - # SoC keys are not the maintenance keys. 0.10 is the 10.1 name, not a value - # that - # makes anything trade (static spread is about 0.08): an owner decision. - # DECISIONS/TARIFF-02-price-provider-plugins.md. + # two SoC keys are not the maintenance keys. 0.10 is the 10.1 name, not a + # value that makes anything trade (static spread is about 0.08): an owner + # decision. DECISIONS/TARIFF-02-price-provider-plugins.md. "arbitrage_enabled": False, "cycle_cost_threshold_eur": 0.10, "arbitrage_soc_floor": 20, @@ -658,8 +651,8 @@ class PriceProvider(ABC): id = "?" # PRICE-01. Ids to try in order when this source cannot answer, and whether - # the - # last answer is kept for a day. Empty / False for every earlier plugin. + # the last answer is kept for a day. Empty / False for every earlier + # plugin. FALLBACKS: tuple = () KEEPS_LAST_GOOD: bool = False @@ -688,8 +681,7 @@ class PriceProvider(ABC): # Names a plugin's own CODE may not mention: FRAMEWORK_KEYS plus `_o`, # `price_option` and `PRICE_OPTION_DEFAULTS`, the other routes PR #16 review -# used -# to double-count. +# used to double-count. BANNED_IN_PLUGIN_CODE = tuple(sorted(FRAMEWORK_KEYS)) + ( "PRICE_OPTION_DEFAULTS", "price_option", "_o") @@ -717,8 +709,7 @@ def code_leaks(cls) -> list: # TARIFF-07: the ids that USED to resolve here. A retired id is not a typo and # has no silent alias (it published retail prices, `generic_dayahead` reads # wholesale). Each value is the id-specific half of the refusal; validate.py -# owns -# the framing. DECISIONS/TARIFF-07-withdrawing-three-plugins.md. +# owns the framing. DECISIONS/TARIFF-07-withdrawing-three-plugins.md. RETIRED_PROVIDERS: dict = { "engie_dynamic": ( "its endpoint https://api.engie.be/dynamic/be does not exist - the " @@ -751,8 +742,7 @@ def register(cls): raise ValueError(f"duplicate provider id {cls.id!r}") if cls.id in RETIRED_PROVIDERS: # At import: re-registering a retired id silences validate.py's - # refusal. Take - # the id out of RETIRED_PROVIDERS in the same commit. + # refusal. Take the id out of RETIRED_PROVIDERS in the same commit. raise ValueError( f"{cls.id!r} is in RETIRED_PROVIDERS and cannot also be a shipped " f"plugin. A site configured on it is refused at startup with a " @@ -920,10 +910,8 @@ def check_egress(url: str, caps: ProviderCapabilities) -> str: # TARIFF-08: the COLUMNAR document the public EPEX feed publishes (parallel # `unix_seconds` / `price` arrays plus `unit`). The only parser; the row parser -# it -# replaced read a shape no feed published. Two shapes, two functions, no -# sniffing. -# DECISIONS/TARIFF-02-price-provider-plugins.md. +# it replaced read a shape no feed published. Two shapes, two functions, no +# sniffing. DECISIONS/TARIFF-02-price-provider-plugins.md. COLUMNAR_TIME_KEY = "unix_seconds" COLUMNAR_PRICE_KEY = "price" @@ -1247,8 +1235,7 @@ class CustomFormula(PriceProvider): body = await transport(url, float(opts.get("price_timeout_s"))) # `scale=1.0`, so these RawPoints carry EUR/**MWh** in a field spelled # `eur_per_kwh`. Contained: destructured on the next line and never - # leaves this - # method (`_validate_raw()` would refuse an escape). + # leaves this method (`_validate_raw()` would refuse an escape). index = _points_from_columnar_json(body, expect_unit="EUR / MWh", scale=1.0, provisional_default=False) diff --git a/goodwe_controller/app/price_forecast.py b/goodwe_controller/app/price_forecast.py index f71a4a4..85911cc 100644 --- a/goodwe_controller/app/price_forecast.py +++ b/goodwe_controller/app/price_forecast.py @@ -6,8 +6,10 @@ price. This module owns the fill and the gate; the widening arithmetic is in price.py. The flag is never passed through: a carried point is built only by `_carried()` (no flag parameter), a published point is passed by identity, and `_audit()` raises on a mismatch. The fill keys on the Brussels wall-clock hour, -and a sub-hourly feed is never decimated. `begin_phase()` is the price-justified -entry point only; the calendar path is untouched. All of it: +and a sub-hourly feed is never decimated. An hour with neither a published +price nor a yesterday value is left out, never filled (`at()` raises `Absence`, +11.4 applies). `begin_phase()` is the price-justified entry point only; the +calendar path is untouched. All of it: DECISIONS/FCST-03-the-hours-not-yet-published.md.""" import logging @@ -124,8 +126,7 @@ def fill_from_yesterday(read: PriceRead, *, yesterday: PriceRead, published = _published_by_hour(read) # Yesterday's profile, keyed by BRUSSELS WALL-CLOCK HOUR. First point wins - # when - # yesterday itself was a 25-hour day. + # when yesterday itself was a 25-hour day. by_local_hour = {} yesterday.match( series=lambda r: [by_local_hour.setdefault(brussels(p.start).hour, p) diff --git a/goodwe_controller/app/price_history.py b/goodwe_controller/app/price_history.py index ad9acef..c3fb6c3 100644 --- a/goodwe_controller/app/price_history.py +++ b/goodwe_controller/app/price_history.py @@ -156,11 +156,9 @@ class PriceHistory: return except (ValueError, OSError) as err: # A corrupt cache must not stop the controller nor be silently re- - # created: the - # record is discarded and the reason kept, so `yesterday()` reports - # it. `ValueError` - # is the family caught, not `json.JSONDecodeError` (one non-UTF-8 - # byte raised + # created: the record is discarded and the reason kept, so + # `yesterday()` reports it. `ValueError` is the family caught, not + # `json.JSONDecodeError` (one non-UTF-8 byte raised # `UnicodeDecodeError` and the add-on did not start, FCST-05). self.last_error = f"unreadable ({err})" _LOG.warning("Price history at %s is %s, so it starts empty. " @@ -269,10 +267,8 @@ class PriceHistory: no_data=lambda _r: None) # The state this read started from, kept so it can be put back - # (FCST-06): the - # merge runs in place and is undone if `_save()` did not take it. Deep - # copy, since - # the loop rebinds the day records. + # (FCST-06): the merge runs in place and is undone if `_save()` did not + # take it. Deep copy, since the loop rebinds the day records. before = copy.deepcopy(self._days) touched = set() @@ -282,12 +278,10 @@ class PriceHistory: rec = self._days.setdefault( day, {"product": product, "provider": provider, "points": []}) # Rule 4: MERGE by instant; a later read updates the hours it - # carries. Every held - # row goes through `_point_of()` and the key is the validated - # instant, not its - # spelling (FCST-05). A row that is not a point is dropped and - # reported, and - # `_save()` writes the record back clean. + # carries. Every held row goes through `_point_of()` and the key is + # the validated instant, not its spelling (FCST-05). A row that is + # not a point is dropped and reported, and `_save()` writes the + # record back clean. raw = rec.get("points") held = {} for q in (raw if isinstance(raw, list) else ()): @@ -315,9 +309,8 @@ class PriceHistory: self._prune(stamp) if not self._save(): # A store that cannot persist records nothing (FCST-06): - # `_save()`'s answer decides - # what is kept, so `yesterday()` never answers a series the file - # does not hold. + # `_save()`'s answer decides what is kept, so `yesterday()` never + # answers a series the file does not hold. self._days = before return _LOG.debug("price history: %d point(s) filed under %s; %d day(s) held",