From a3f725e8a483793bd899b28854ce3796ebef56f3 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Sun, 1 Nov 2026 01:16:53 +0100 Subject: [PATCH] MAINT-20 part 1: price_history.py comments cut; FCST-04 note gets the store details Co-Authored-By: Claude --- .../FCST-04-the-day-that-is-carried.md | 61 ++ goodwe_controller/app/price_history.py | 654 +++--------------- 2 files changed, 156 insertions(+), 559 deletions(-) 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 d0e85bd..63258c9 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 @@ -188,3 +188,64 @@ still raises; so does the same published point emitted twice; and a carried point sharing an hour with a real print now raises too, which the count never looked at. Each of those three is driven by its own mutation in `test_price_forecast.py`. + +## 5. Store details (moved here from `app/price_history.py` by MAINT-20) + +**Why a file, and which one.** FCST-03 shipped the fill with no caller, so no +series ever contained a carried point. Without a yesterday that survives a +restart, the mornings after an operator touched something spend the whole +window on the truncated series. The cache is `/data/price_history.json`, not +`/data/state.json`: `Store.set()` rewrites the whole of state.json on each +write, and the refresh runs every `TARIFF_REFRESH_S` (1800 s), so 48 extra +rewrites a day would go through the file whose corruption reads as "no +maintenance ever ran". Separate file, separate blast radius. The directory is +not created (`tariff.PeakStore`'s rule): `/data` is always mounted, so a +`makedirs` could only succeed where the durable volume is missing and build a +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 +`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 +the 25-hour October day both survive and `fill_from_yesterday()`'s own rule is +the only collapsing rule. The merge key is the validated instant; the old +string key merged by spelling and sorted the `+01:00` 02:00 before the `+02:00`. +`KEEP_DAYS` is a sort-order cap, not a horizon: nothing is deleted by arithmetic +against the clock (a jumped clock, WITNESS-03, would otherwise delete everything +on the one morning the fill is for), and the two dates `yesterday()` can be +asked for are pinned on top of the cap, which can only retain. + +**One validator (FCST-05, MEDIUM).** The read path was defensive about every +field and the write path trusted `q["start"]` to be hashable and sortable. An +int, null, dict or list `start` raised `TypeError` out of `carry()` on the day +being written; `run_tariff_refresh` caught it, so the add-on stayed up and every +refresh failed for good (the plan retires after `TARIFF_MAX_AGE_S`, arbitrage and +the cheap window go off, and `_remember()` raised before `_save()`, so no +self-heal). `_point_of()` is now the one standard for both paths and returns +`None` rather than raising. + +**A broken cache is a short horizon, never a controller that will not start** +(6: absent machinery must not disable the loop). `PriceHistory(...)` and `.words` +are unguarded in `Controller.__init__`. Shipped defects, all fixed: + +- `_load()` caught `json.JSONDecodeError` and `OSError` only; one non-UTF-8 byte + raised `UnicodeDecodeError` (a `ValueError`, not an `OSError`) and the add-on + did not start (FCST-05, HIGH). The handler names the `ValueError` family. The + schema version is compared with its type (`1.0 == 1` in Python). +- `{"days": {"2026-01-14": "oops"}}` loaded as healthy and raised + `AttributeError` out of `words`, `yesterday()` and `resolve_tariff()`. + `_usable_days()` drops such records, and counts (not drops) malformed rows + inside a kept record so the store stops reporting health it lacks. +- `words` printed `f"{d} ({_n_points(rec)} h)"`: points under the hour's name + (`'2026-01-13 (96 h)'` for a 24-hour quarter-hourly day, FCST-05 LOW). Both + numbers are printed now; only rows that are points are counted. `words` must + never raise, `_held()` is total. +- A failed merge is undone from a deep copy (a one-level copy shares the day + records the merge rebinds). + +`NO_HISTORY` is a name, not `None`, so `history=` can be required: `None` +meaning "skip the fill" is how a caller silently gets the truncated horizon. +`carry()` records first, then fills, so the filled series exists only as a +return value that the store never sees again (rule 1). diff --git a/goodwe_controller/app/price_history.py b/goodwe_controller/app/price_history.py index 804f48d..ad9acef 100644 --- a/goodwe_controller/app/price_history.py +++ b/goodwe_controller/app/price_history.py @@ -1,185 +1,24 @@ -"""FCST-04 - the day the carry-forward carries, kept somewhere a restart cannot reach. +"""FCST-04 - the day the carry-forward carries, kept where a restart cannot reach. -WHAT WAS MISSING, AND WHY IT WAS INVISIBLE. FCST-03 shipped -`price_forecast.fill_from_yesterday()` complete and checked, and shipped the -consumers that were written to expect it - `TariffPlan.firm_points()`, -`cheap_now()`'s refusal of a provisional hour, and `arbitrage()`'s refusal 3, -which drops provisional hours from the trade set instead of refusing the whole -series. Every one of those was live. The FILL ITSELF HAD NO CALLER: nothing in -`app/` referenced `fill_from_yesterday`, so no series ever contained a carried -point and every one of those consumers was exercising its all-firm branch. +`price_forecast.fill_from_yesterday()` needs a yesterday that survives a +restart; this module is that store and the one caller of the fill. Four rules: -The defect that produced is not an error anywhere. `load_prices()` returns -whatever the feed published and nothing widens it to the requested horizon, so -on a dynamic site between midnight and the 14-16 h CET day-ahead publication -the series covers the rest of TODAY and nothing more - typically 10-24 of the -48 hours `price_horizon_h` asked for. `TariffPlan.arbitrage()` then computes -`max - min` over that truncated window and reports a spread. The number is -real, the window is half the one the operator configured, and NOTHING SAYS SO: -tomorrow evening's peak - the hour a morning charge would be sold into - is -simply not in the series, so the spread it would have created cannot clear a -threshold it was never compared against. A missing hour is not a wrong price; -it is an hour the optimiser never learns exists. +1. It records what the PROVIDER published, never what the fill produced + (feeding a fill back in rots the numbers behind a true `provisional` flag). + `carry()` is the only public entry point and `_remember()` has exactly one + caller; test_price_history.py parses every shipped module for the name and + grades the order by driving it. +2. "Yesterday" is the Brussels day before the instant asked about, not the + newest record; any other date is `no_data` naming both dates. +3. A store that cannot answer says so in the type: `yesterday()` returns a + `PriceRead`, never an empty series. +4. Hours accumulate within a day (merge by instant, the newer price wins), so a + feed that serves only the remaining hours cannot overwrite a full day. -So this module is the half FCST-03 left out, and it is deliberately the SMALL -half: the fill's arithmetic, its DST rule and its provenance audit all stay in -`price_forecast.py`, untouched. What lives here is the one thing that -arithmetic needs and could not have: A YESTERDAY THAT SURVIVES A RESTART. - - -WHY PERSISTENCE, AND NOT A VARIABLE ON THE REFRESH TASK -====================================================== -An in-memory "the last full day I saw" costs nothing and is wrong exactly when -it matters. The window the fill exists for is midnight -> publication; an -add-on restarted at 08:00 - a config change, an HA update, a power cut - would -come up with no yesterday at all and spend that whole window on the truncated -series, which is the pre-FCST-03 behaviour arriving silently on the mornings -after an operator touched something. The fill's own value is a function of -surviving a restart, so the day it carries has to. - -⚠️ AND NOT IN `/data/state.json`. That file holds the maintenance history and -SAFETY-02's sign latch, and `Store.set()` rewrites the whole of it on every -write. The tariff refresh runs every `TARIFF_REFRESH_S` (1800 s), so folding a -price cache into it would put 48 extra whole-file rewrites a day through the -file whose corruption reads as "no maintenance ever ran" and "never tripped". -The cache is worth having; it is not worth putting anywhere near that. Separate -file, separate blast radius. - -⚠️ AND THE DIRECTORY IS NOT CREATED, which is `tariff.PeakStore`'s rule and its -reason applies unchanged: `/data` is mounted by the Supervisor and always -exists on a real install, so `makedirs` here could only ever succeed where the -durable volume is MISSING - manufacturing a cache on the container's ephemeral -layer that writes fine, reads fine, and is thrown away on the next restart. -That is a yesterday that silently forgets, which is worse than one that says it -cannot answer. - -⚠️ AND THAT RULE IS CARRIED OUT IN `_remember()`, NOT ONLY IN `_save()`. FCST-04 -first shipped it half-done: `_save()` short-circuited on `not self.ok` while -`_remember()` filled `self._days` anyway, so a store with no durable volume -answered `words` = "unavailable" and `yesterday()` = A SERIES - an in-memory -yesterday that works all day and is gone on the next restart, which is exactly -the silently-forgetting cache this rule exists to refuse. A store that cannot -persist now records nothing, and both of its answers say the same thing. - -⚠️ AND "CANNOT PERSIST" IS ONE QUESTION, ASKED ONCE, ABOUT THE OUTCOME - -FCST-06. The paragraph above enforced that rule against `not self.ok`, which is -one SYMPTOM of not persisting: the durable volume is missing. There is a second, -and FCST-06 measured it. With `ok=True` and the directory present, `_save()` can -still fail - a read-only remount (`[Errno 30] Read-only file system`), a full -volume (`[Errno 28] No space left on device`) - and it logs, records -`last_error` and returns. `_remember()` had already populated `self._days`, so -`words` read `'2026-01-14 (24 points over 24 h); 2026-01-15 (24 points over -24 h) [could not write (...)]'` while the file on disk held `{}`, and -`yesterday()` answered 24 REAL POINTS for the rest of the process's life. Gone -on the next restart, and nothing said it would be - the same silently-forgetting -cache, reached by the condition the rule did not name. - -⚠️ SO IT IS NOT TWO RULES, AND IT IS DELIBERATELY NOT TWO TESTS. "Can this -store persist?" CANNOT BE ANSWERED BEFORE THE ATTEMPT - a directory that existed -and was writable a moment ago is not a write that completed - so it is answered -AFTER it, once, by the only thing holding the evidence: `_save()` returns -whether the file now holds what memory holds. A missing volume is one of its -False cases and an `OSError` from the write is another, and `_remember()` KEEPS -ONLY WHAT THAT ONE ANSWER SAYS WAS KEPT. There is no `not self.ok` test in -`_remember()` any more, because there is nothing left for a second test to add - -and a third symptom nobody has thought of is how this defect arrived both times. - -⚠️ AND WHAT IS KEPT IS THE LAST STATE THAT IS ON DISK, NOT AN EMPTY ONE. The -MERGE is undone, not the store. A site whose volume fills at 14:00 goes on -answering the yesterday it wrote at 13:30 - that record survives a restart, so -answering it is not a promise the store cannot keep. What it stops doing is -adding to it. The invariant the rule was always describing, stated once: -`self._days` NEVER HOLDS A POINT THE FILE DOES NOT. (Not "mirrors the file" - -`_usable_days()` drops records the file still holds. The only direction that can -invent a yesterday is the one named.) - - -THE FOUR RULES THIS FILE IS SHAPED BY -===================================== - -1. IT RECORDS WHAT THE PROVIDER PUBLISHED, NEVER WHAT THE FILL PRODUCED. - Feeding a filled series back in is the defect that makes the whole mechanism - rot: day 2 carries day 1's guesses, day 3 carries day 2's carry of them, and - after a week of a dead feed the "yesterday" being carried forward is a - fossil of the last real publication with nothing anywhere reporting its age. - Every point still carries `provisional=True`, so the flag never lies - what - decays is the NUMBER, silently, behind a flag that is telling the truth. - - ⚠️ NO VALUE-LEVEL CHECK CAN CATCH THAT, and saying so is more useful than a - guarantee that would not hold. A carried point and a published-provisional - point (a real-time product's current interval, §9.2) are the same three - fields with - the same flag; the difference is provenance, and provenance is not in the - value. So the mechanism is ORDERING, not validation: `carry()` is the only - public entry point, it remembers the argument and returns the fill, and the - filled series exists only as a return value that `carry()` itself never - sees again. `_remember()` is private, is called from exactly one place, and - `test_price_history.py` PARSES every shipped module under `app/`, `tools/`, - `scenarios/` and `estop/` and fails if any of them uses the NAME - an - attribute, a bare name, a def, or the string constant a `getattr` needs. By - name rather than by substring, so a comment about the rule does not fail it - and a call from `tools/` cannot pass unseen. - - ⚠️ AND THE ORDER IS GRADED BY DRIVING IT, NOT BY READING THE SOURCE. The - first version of that check compared positions in `ast.walk`, which is - breadth-first: `self._remember(...)` is a statement and `self.yesterday(...)` - is an ARGUMENT of the fill, one level deeper, so `_remember` came out first - whichever statement was written first - shipped and reversed scored the - same. What grades it now is the behaviour: a feed carrying yesterday - alongside today, into an EMPTY store, must fill the whole horizon, and - reversed it fills nothing. - -2. "YESTERDAY" MEANS YESTERDAY, NOT "THE NEWEST DAY I HAVE". - `newest()` is the natural accessor and it is the one that decays: an add-on - down from Friday to Monday would answer Friday's prices to a fill that flags - them as one day old. The lookup is BY DATE - the Brussels day before the - Brussels day of the instant asked about - and a record for any other date is - NO_RECORD with a reason naming both dates. A three-day-old profile is not a - worse yesterday; it is not a yesterday. - -3. A STORE THAT CANNOT ANSWER SAYS SO, IN THE TYPE. - `yesterday()` returns a `PriceRead`: a `series` when a full record exists, - `no_data(reason)` for every other outcome - no file, no record for that - date, an unreadable file, a record that parses to nothing. There is no - empty-series answer and no `.get(..., [])`-shaped accessor, for - `price.py`'s reason: an empty series is how "the cache is broken" gets to - wear the shape of "yesterday was free". `fill_from_yesterday()` takes a - `PriceRead` and handles `no_data` on its own, so the degraded case needs no - branch at the call site - it fills nothing and the horizon stays short, - which is exactly the behaviour that shipped before this ticket. - -4. HOURS ACCUMULATE WITHIN A DAY; THEY DO NOT REPLACE IT. - A feed that serves only the REMAINING hours of the day (a legitimate shape - - nothing in the plugin contract promises a whole day) would, under - replace-the-record, overwrite a complete 24-hour profile with the two points - left at 22:00 and hand the next morning a yesterday with two hours in it. So - a record is MERGED by instant: a later read updates the hours it carries and - leaves the rest, and a revised price for an hour already held wins because - it is the newer statement about the same hour. - -⚠️ KEYED BY INSTANT, NOT BY LOCAL HOUR, AND THAT IS THE DST DECISION. Storing -`hour -> price` would collapse the two 02:00s of a 25-hour October day into -one, inventing a second and different collapsing rule beside the one -`fill_from_yesterday()` already documents ("the first of its two 02:00 points -is the one used, deterministically"). Storing the instants keeps both, in -order, and lets the fill's own rule be the only one in the system. - -WHAT THIS DOES NOT DO, ON PURPOSE -================================= -It does not extend the fill to a feed that published NOTHING. `resolve_tariff()` -applies `carry()` only on a `series` read; a `no_data` read still falls to -`_degrade()` and §11.4's row - "No price data -> fall back to the static -profile, mark the plan provisional, no arbitrage" - which is the fallback the -operator configured `static_profile` FOR. Preferring yesterday's feed over it -would change a documented degradation without being asked, and it would buy -nothing measurable: a fully-carried series is 100 % provisional, so -`arbitrage()` finds fewer than two firm hours and refuses, and `cheap_now()` is -False, exactly as on the §11.4 fallback. The two differ only in which numbers -the status page shows. `fill_from_yesterday()` handles a `no_data` read anyway, -and that branch is therefore not reached from production - it stays checked, -and turning it on is a product decision, not a wiring one. See -DECISIONS/FCST-04-the-day-that-is-carried.md. -""" +Rows are keyed by instant, not local hour (the DST rule stays in the fill). A +store that cannot persist records nothing (FCST-06). It does not extend the fill +to a feed that published nothing: that stays 11.4's static fallback. +DECISIONS/FCST-04-the-day-that-is-carried.md (section 5 has the store details).""" import copy import json @@ -194,89 +33,31 @@ from .price_forecast import fill_from_yesterday _LOG = logging.getLogger("goodwe.price.history") -# How many Brussels days of published prices the SORT-ORDER cap keeps. The fill -# needs exactly one - yesterday - and rule 2 refuses anything older, so this is -# not a horizon: it is the number of days a record may sit unused before -# pruning removes it, and it is small because a bigger cache would only hold -# days no lookup can ask for. Three covers today, yesterday, and the day a -# fetch spanning midnight writes ahead of itself. -# -# ⚠️ IT IS A CAP, NOT THE RETENTION RULE, AND THREE HAS NO SLACK. The steady -# state is already {yesterday, today, tomorrow}; a feed that publishes to D+2, -# or ONE stray point in the wrong year - which sorts LAST and is therefore -# immortal under a newest-N cap - pushes yesterday out of a three-slot store -# and keeps it out, permanently and silently. So `_prune()` PINS the two days a -# lookup can actually ask for on top of the cap; see its docstring. The store -# is then bounded at KEEP_DAYS + 2 records rather than at KEEP_DAYS. +# Brussels days kept: a sort-order cap, not a horizon (the fill needs only +# yesterday). KEEP_DAYS = 3 SCHEMA = 1 def _bdate(instant: datetime) -> str: - """The Brussels calendar date of an instant, as `YYYY-MM-DD`. - - ⚠️ BRUSSELS, NOT UTC. 23:30 UTC in July is already tomorrow in Belgium, and - a cache keyed on the UTC date would file the last 90-120 minutes of every - day under the wrong one - so a lookup on the following morning would find a - "yesterday" missing its own evening hours, which are the expensive ones. - """ + """The Brussels calendar date of an instant, as `YYYY-MM-DD` (not UTC: 23:30 UTC in July is tomorrow).""" return brussels(instant).strftime("%Y-%m-%d") def _yesterday_of(instant: datetime) -> str: """The Brussels date of the day BEFORE `instant`'s Brussels day. - - ⚠️ THE ARITHMETIC IS ON THE LOCAL DATE, NOT ON THE INSTANT, AND THAT IS A - DST DEFECT THIS FILE SHIPPED. `brussels()` renders at the FIXED offset in - force at that instant, so `brussels(stamp) - timedelta(days=1)` subtracts 24 - ABSOLUTE hours from a wall clock - and the Brussels day before a - spring-forward is 23 hours long. Measured at 2026-03-30T00:30+02:00, the - first hour of the morning after the March transition: it returned - 2026-03-28, so the fill carried a TWO-DAY-OLD profile flagged as yesterday's - (or, with only the last three days held, answered `no_data` naming a date - that is not yesterday). - - A calendar day minus one day is `date` arithmetic, and a `date` has no - offset to lose. The 25-hour October day is right for the same reason. - """ + Arithmetic on the local `date`, not the instant: subtracting 24 absolute hours + was wrong after the spring-forward. DECISIONS/FCST-04-the-day-that-is-carried.md.""" return (brussels(instant).date() - timedelta(days=1)).strftime("%Y-%m-%d") def _point_of(row): """One stored row -> a `PricePoint`, or `None` if the row is not one. - THE ONE VALIDATOR, AND THE ASYMMETRY IT CLOSES (FCST-05's MEDIUM). This - file used to hold TWO standards for what a stored point is. The READ path - (`_parse_points()`) was properly defensive - every field checked, a - malformed row dropped and reported. The WRITE path was - `{q["start"]: q for q in raw if isinstance(q, dict) and "start" in q}` - followed by `sorted(..., key=lambda q: q["start"])`, which trusts - `q["start"]` to be a hashable, sortable string it never checked. Measured, - with the malformed row under the day the feed is currently writing to: - - start is an int -> TypeError: '<' not supported between instances - of 'str' and 'int' - start is a dict -> TypeError: cannot use 'dict' as a dict key - start is null -> TypeError: '<' not supported between 'str' and - 'NoneType' - start is a list -> TypeError: cannot use 'list' as a dict key - - ⚠️ AND THE BLAST RADIUS IS PERMANENT, WHICH IS WHY IT IS NOT MERELY UNTIDY. - `_remember()` is reached through `carry()` -> `resolve_tariff()` -> - `run_tariff_refresh`, which catches `Exception` - so the add-on stays UP - and EVERY refresh raises forever. The plan retires after - `TARIFF_MAX_AGE_S`, and arbitrage and the cheap window are off from then - on. There is no self-heal, because `_remember()` raises before it reaches - `_save()`. `_usable_days()` only required `rec["points"]` to be a LIST; the - elements were unchecked on the write path, and the existing fixture put its - malformed row under a day the feed was NOT writing to, so `_remember()` - never touched it. - - So both paths call this, and neither of them has a standard of its own. - Returning `None` rather than raising is the contract: a caller decides - whether a bad row is dropped (both of them do) and how it is reported. - """ + The one validator for both the read and the write path (FCST-05): a malformed + row used to raise out of `carry()` and fail every refresh for good. Returns + `None` rather than raising; the caller decides how a bad row is reported.""" if not isinstance(row, dict): return None try: @@ -293,33 +74,8 @@ def _point_of(row): def _held(rec) -> tuple: """`(points, whole hours they cover)` for a day record. `(0, 0)` otherwise. - - ⚠️ POINTS AND HOURS ARE TWO NUMBERS, AND FCST-04's STATUS ROW PRINTED THE - FIRST UNDER THE SECOND'S NAME. The row was `f"{d} ({_n_points(rec)} h)"`, - which counts POINTS: on the quarter-hourly feed this add-on already - supports it read `'2026-01-13 (96 h)'` - 96 "h" for a 24-hour day. That is - the points/hours conflation H1 was raised for, on the row H1's own ticket - shipped. Both numbers are reported now, so an operator can see a day that - is 24 points over 24 h and a day that is 96 points over 24 h and tell them - apart, and neither number is wearing the other's unit. - - ⚠️ TOLERANT, AND IT IS THE SECOND LINE OF DEFENCE, NOT THE FIRST. `_load()` - drops a malformed day record and says so, so this should never see one. It - is written this way because `words` is read from `Controller.__init__` and - from every status render: a status LINE that raises takes the add-on down, - and "the cache is broken" must never be able to mean "the controller does - not start". That is exactly what FCST-04 shipped - `{"days": {"2026-01-14": - "oops"}}` gave `ok=True`, `last_error=None`, and an `AttributeError` out of - `words`, `yesterday()` and `resolve_tariff()` alike, with no self-heal - because `_remember()` raised before it could rewrite the file. - - ⚠️ AND IT COUNTS ONLY ROWS THAT ARE POINTS. A row `_point_of()` refuses is - not held prices, and a row count that included it would report a store as - healthier than it is - which is the second half of FCST-05's MEDIUM: the - status row said `'2026-01-15 (1 h)'` about a record whose single row was - killing every tariff refresh. What is dropped is named in `last_error`, and - `words` prints that beside the counts. - """ + Two numbers, each in its own unit (96 points over 24 h on a quarter-hourly + feed). Total, and counts only rows that are points.""" if not isinstance(rec, dict): return (0, 0) raw = rec.get("points") @@ -337,15 +93,7 @@ def _held(rec) -> tuple: def _row_of(day: str, points: int, hours: int) -> str: - """One day's cell of the status row: what is held, in BOTH of its units. - - ⚠️ THE UNIT IS SPELT OUT ON EACH NUMBER. FCST-04's cell was - `f"{day} ({points} h)"` - the point count wearing the hour's name, which on - a quarter-hourly feed reads `'2026-01-13 (96 h)'` for a 24-hour day. What - an operator needs from this row is whether tomorrow morning's fill has a - whole day to carry, and that is the HOUR figure; the point count says how - finely it is priced. Neither answers for the other. - """ + """One day's cell of the status row, with the unit spelt out on each number.""" # UX-10: the day as the page writes dates ("26 Sep"), not ISO. return (f"{points} price{'' if points == 1 else 's'} over {hours} " f"hour{'' if hours == 1 else 's'} for {day_words(day)}") @@ -373,21 +121,8 @@ def _bad_rows(rec) -> int: class PriceHistory: """The published price series of the last few Brussels days, in /data. - - ⚠️ `ok` IS ABOUT THE FILE, NOT ABOUT THE DATA. A healthy store with no - record for yesterday is `ok` and answers `no_data` - a normal state on a - site's first day, and a different thing from a store that could not be - opened. Both are visible: `ok`/`last_error` describe the file, and - `yesterday()`'s reason describes the answer. - - ⚠️ AND `ok` IS NOT "CAN PERSIST" (FCST-06). It says the directory was - there when the store opened, which a full volume and a read-only remount - both leave true. The one thing that answers "can this store persist" is - `_save()`'s return value, and `self._days` NEVER HOLDS A POINT THE FILE DOES - NOT, because `_remember()` undoes a merge that answer refused. That is what - makes `words` and `yesterday()` describe the same store in EVERY degraded - state rather than only in the ones somebody enumerated. - """ + `ok` is about the file, not the data: a healthy store with no record for + yesterday is `ok` and answers `no_data`. `ok`/`last_error` describe the file.""" def __init__(self, path: str = "/data/price_history.json"): self.path = path @@ -396,15 +131,14 @@ class PriceHistory: self._days: dict = {} self._open() - # -- the file ------------------------------------------------------------ + # the file def _open(self) -> None: parent = os.path.dirname(self.path) - # See the module note: NOT created. A cache on the ephemeral layer is - # one that forgets without saying so. + # NOT created (see FCST-04 section 5): a cache on the ephemeral layer + # forgets silently. if parent and not os.path.isdir(parent): self.last_error = f"no durable volume at {parent}" - # INFO, not ERROR: on a dev host or in a test container there is no - # /data and nothing is wrong. + # INFO, not ERROR: a dev host or test container has no /data. _LOG.info("No price history is kept (%s), so missing hours are " "not filled from yesterday's prices.", self.last_error) return @@ -421,32 +155,13 @@ class PriceHistory: "after that", self.path) return except (ValueError, OSError) as err: - # A corrupt cache must not stop the controller, and must not be - # silently re-created either: the record is discarded, the reason - # is kept, and `yesterday()` reports it rather than answering - # "no record", which would read as a normal first day. - # - # ⚠️ `ValueError`, NOT `json.JSONDecodeError`, AND THAT IS - # FCST-05's HIGH. Reading this file has two failure families: the - # BYTES (`OSError`) and the CONTENT - and the content family has at - # least two members, because `open(..., encoding="utf-8")` decodes - # before `json` ever sees a character. `UnicodeDecodeError` is - # `ValueError -> UnicodeError -> UnicodeDecodeError`; it is NOT a - # `json.JSONDecodeError` and it is NOT an `OSError` - # (`issubclass(UnicodeDecodeError, OSError)` is False). So ONE - # non-UTF-8 byte in /data/price_history.json escaped `_load()` -> - # `_open()` -> `PriceHistory.__init__`, which is unguarded at - # main.py's `Controller.__init__`, AND THE ADD-ON DID NOT START. - # Measured: `UnicodeDecodeError: 'utf-8' codec can't decode byte - # 0xff in position 39`. Valid-UTF-8-invalid-JSON, a UTF-8 BOM and a - # non-dict day record all degraded correctly - it was the one byte - # class left uncovered, which is the same class as the defect the - # `_usable_days()` rework was raised for. - # - # The tuple names the FAMILY rather than the two members known - # today, because "the file's content is not readable as JSON" is - # exactly what this handler says, and a third `ValueError` subclass - # nobody has thought of is how this defect arrived the first time. + # 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 + # `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. " "Missing hours cannot be filled in until a full day " @@ -454,10 +169,7 @@ class PriceHistory: self.path, self.last_error) return found = blob.get("version") if isinstance(blob, dict) else None - # ⚠️ THE TYPE IS PART OF THE VERSION. `1.0 == 1` in Python, so a float - # written by some other tool would pass a bare `!=` and be read as - # schema 1 - a version check that cannot tell 1 from 1.0 is one that - # would not notice a writer this build has never met. + # The type is part of the version: `1.0 == 1` in Python. if not isinstance(blob, dict) or isinstance(found, bool) \ or not isinstance(found, int) or found != SCHEMA: self.last_error = f"version {found!r}, not {SCHEMA}" @@ -474,35 +186,8 @@ class PriceHistory: def _usable_days(self, days: dict) -> dict: """The day records that are shaped like day records. The rest are dropped. - - ⚠️ THE ASYMMETRY THAT PRODUCED THE DEFECT: `_parse_points()` below is - carefully defensive about every field of a POINT, and `_load()` was not - defensive about the record holding them at all. `{"version": 1, "days": - {"2026-01-14": "oops"}}` therefore loaded as `ok=True`, - `last_error=None` - and then `rec.get(...)` raised `AttributeError` out - of `words`, `yesterday()` and `resolve_tariff()`. `PriceHistory(...)` - and `.words` are both unguarded in `Controller.__init__`, so THE ADD-ON - DID NOT START; and it never self-healed, because `_remember()` raised on - the same record before it could reach `_save()`. - - A broken cache is a short horizon. It is not a controller that will not - regulate a battery, and §6's rule - absent machinery must not disable - the loop - does not stop applying because the machinery is a JSON file. - - Dropped, not repaired: an unusable record has nothing in it to keep, and - the next `_remember()` writes a fresh one under that date. - - ⚠️ AND THE ROWS INSIDE A KEPT RECORD ARE COUNTED, NOT ONLY THE RECORD - (FCST-05). This checked that `rec["points"]` is a LIST and stopped - there, so a record holding one row with a `start` of `7` loaded as - healthy: `last_error=None`, and `words` said `'2026-01-15 (1 h)'` about - a store whose next `_remember()` would raise on that row and go on - raising forever. The rows are NOT dropped here - `_parse_points()` is - the read path's one filter and stays so - but they are COUNTED, so the - store stops reporting health it does not have from the moment it is - opened. `_remember()` drops them on the next refresh and `_save()` - rewrites the file, after which a restart clears the note. - """ + A broken cache is a short horizon, never a controller that will not start. + Rows inside a kept record are counted, not dropped (`_parse_points()` filters).""" kept, dropped, malformed = {}, [], {} for key, rec in days.items(): if not isinstance(key, str) or not isinstance(rec, dict) \ @@ -524,9 +209,8 @@ class PriceHistory: + ", ".join(f"{day_words(d)} ({n})" for d, n in sorted(malformed.items()))) if notes: - # Recorded, not silent: `yesterday()` and `words` both report - # `last_error`, and a cache that quietly discards half a file reads - # to an operator exactly like a normal first day. + # Recorded, not silent: a cache that quietly discards half a file + # reads like a first day. self.last_error = " and ".join(notes) _LOG.warning("Price history at %s: %s. The add-on starts anyway, " "and fills in only what the rest holds.", @@ -535,27 +219,7 @@ class PriceHistory: def _save(self) -> bool: """Write `self._days` out. True only if the file now holds it. - - ⚠️ THE RETURN VALUE IS THE "CAN THIS STORE PERSIST?" PREDICATE, AND - IT IS THE ONLY ONE (FCST-06). Both ways of not persisting end here and - are the same answer: no durable volume returns False without touching - the disk, an `OSError` out of the write returns False after recording - why. `_remember()` acts on that one answer, so the second place that - used to test the first symptom on its own - and missed the second - has - nothing left to test. See the module docstring's fifth warning. - - ⚠️ AND IT REPORTS, IT DOES NOT RAISE. §6 and this module's opening: a - broken cache must not be a broken controller. An add-on that will not - start because /data filled up is a worse defect than the one FCST-06 - fixed, so a failed write is logged, kept in `last_error` - where `words` - and `yesterday()` both read it - and RETURNED, never thrown. - - ⚠️ THE TEMP FILE IS REMOVED ON EVERY FAILING PATH. A write that dies - between `mkstemp` and `os.replace` used to leave its temp behind, and - the refresh task runs 48 times a day: a persistent write failure - a - full volume being the likeliest cause - leaked 48 inodes a day onto the - volume that was already the problem. - """ + The one "can this store persist?" answer (FCST-06), asked after the attempt.""" if not self.ok: return False tmp = None @@ -563,8 +227,7 @@ class PriceHistory: fd, tmp = tempfile.mkstemp(dir=os.path.dirname(self.path)) with os.fdopen(fd, "w", encoding="utf-8") as fh: json.dump({"version": SCHEMA, "days": self._days}, fh, indent=1) - # Atomic: a power cut mid-write must not leave a truncated file - # that parses as an empty day. + # Atomic: a power cut mid-write must not leave a truncated file. os.replace(tmp, self.path) tmp = None # it IS the file now; do not unlink except OSError as err: @@ -579,45 +242,24 @@ class PriceHistory: pass # failed cleanup is not a fault return True - # -- the one public entry point ----------------------------------------- + # the one public entry point def carry(self, read: PriceRead, *, since: datetime, until: datetime, stamp: datetime, remember: bool = True) -> PriceRead: - """Remember what this read published, then §8.3-fill the missing hours. - - THE ORDER IS DELIBERATE AND IS THE POINT OF THE METHOD. The read is - recorded FIRST, so a feed that hands back yesterday alongside today - refreshes yesterday's record from live data before it is consumed as - the profile; and the filled series - the one thing that must never be - recorded - exists only as this method's return value, which this object - never sees again. See rule 1, including how that order is graded. - - Returns whatever `fill_from_yesterday()` returns: a series over the - whole hours of [since, until), carrying EVERY point the provider - published inside it - by identity, and four of them in one hour on a - quarter-hourly feed - plus one carried point for each remaining whole - hour FOR WHICH YESTERDAY HELD THAT WALL-CLOCK HOUR. An hour neither - source can price is left out (§11.4 for that hour), and `no_data` comes - back when that is every hour. - """ + """Remember what this read published, then 8.3-fill the missing hours. + Recorded FIRST, so the filled series exists only as the return value (rule 1). + Returns `fill_from_yesterday()`'s series, or `no_data` when no hour can be priced.""" if remember: - # PRICE-01: False only for a read that CAME FROM this store - # (`stored()`), which is already filed; filing it again would - # rewrite the same file for nothing, and nothing else may pass it. + # PRICE-01: False only for a read that came from this store, + # already filed. self._remember(read, stamp) return fill_from_yesterday(read, yesterday=self.yesterday(stamp), since=since, until=until, stamp=stamp) - # -- the halves, private because rule 1 is an ordering ------------------ + # the halves, private because rule 1 is an ordering def _remember(self, read: PriceRead, stamp: datetime) -> None: """File a provider read's points under their own Brussels days. - - ⚠️ PRIVATE, AND CALLED FROM EXACTLY ONE PLACE. A caller that can reach - this can record a filled series, which is the decay in rule 1. There is - no value-level defence against that - a carried point and a published - provisional point are the same three fields - so the defence is that - nothing outside this file names it, and `test_price_history.py` checks - the tree for that rather than trusting the underscore. - """ + Private, called from exactly one place: no value-level defence can tell a + carried point from a published-provisional one, so the defence is the ordering.""" points = read.match(series=lambda r: r.points, no_data=lambda _r: ()) if not points: return @@ -626,23 +268,11 @@ class PriceHistory: provider = read.match(series=lambda r: r.provider, no_data=lambda _r: None) - # ⚠️ THE STATE THIS READ STARTED FROM, KEPT SO IT CAN BE PUT BACK - # (FCST-06). Everything below merges into `self._days` IN PLACE, and - # whether any of it may be KEPT is not knowable until `_save()` has - # tried - so the merge runs, and it is undone if the file did not take - # it. There is no `not self.ok` branch above any more: a store with no - # durable volume is one of the answers `_save()` gives, not a separate - # symptom with a test of its own. Module docstring, fifth warning. - # - # ⚠️ DEEP, NOT `dict(self._days)`. A one-level copy shares the day - # RECORDS, and the loop below rebinds keys on the ones it touches - so - # the "previous" state would hand back records the merge had already - # edited, and a store with a good yesterday on disk would go on - # answering the merge of it that never reached the file. It would also - # be right only for as long as nobody makes this loop mutate a `points` - # list or a row in place. A copy that is right by construction outlives - # that, and the store holds at most KEEP_DAYS + 2 records, 48 times a - # day. + # 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. before = copy.deepcopy(self._days) touched = set() @@ -651,29 +281,13 @@ class PriceHistory: day = _bdate(p.start) rec = self._days.setdefault( day, {"product": product, "provider": provider, "points": []}) - # Rule 4: MERGE by instant. A later read updates the hours it - # carries and leaves the rest; a revised price for an hour already - # held wins, because it is the newer statement about that hour. - # - # ⚠️ EVERY HELD ROW GOES THROUGH `_point_of()` (FCST-05's MEDIUM). - # This was `{q["start"]: q for q in raw if isinstance(q, dict) and - # "start" in q}` sorted by `q["start"]` - a key and a sort key that - # trusted a value read off disk to be a hashable, sortable string. - # An `int`, `null`, a `dict` or a `list` there raised `TypeError` - # out of `carry()` on the day the feed was writing to, and - # `run_tariff_refresh` caught it, so the add-on stayed up while - # EVERY refresh failed for good. A row that is not a point is now - # dropped and reported here, exactly as the read path drops it, and - # `_save()` below writes the record back clean - which is the - # self-heal the raising version could never reach. - # - # ⚠️ AND THE MERGE KEY IS THE VALIDATED INSTANT, NOT ITS SPELLING. - # Aware datetimes hash and compare by instant, so this is rule 4 as - # rule 4 is written. The old string key merged by SPELLING, and the - # old lexicographic sort put the second 02:00 of the 25-hour - # October day (`+01:00`) BEFORE the first (`+02:00`). Only the - # stored order was affected - `_parse_points()` re-sorts by instant - # on the way out - but the store now holds what it says it holds. + # 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. raw = rec.get("points") held = {} for q in (raw if isinstance(raw, list) else ()): @@ -689,13 +303,8 @@ class PriceHistory: touched.add(day) if junked: - # Said, not swallowed. `words` prints `last_error`, so a store that - # has just discarded rows off disk stops reporting plain health - - # and after `_save()` below the file is clean, so a restart clears - # this. `_save()` overwrites it if the write itself fails, which is - # the more urgent fact of the two - and on that path the drop is - # undone with the rest of the merge, so those rows are still in the - # file and the next write that completes drops them again. + # Said, not swallowed: `words` prints `last_error`. A failed + # `_save()` overwrites it and undoes the drop. self.last_error = ( f"{messages.count(sum(junked.values()), 'bad price')} dropped " f"from " + ", ".join(f"{day_words(d)} ({n})" @@ -705,48 +314,20 @@ class PriceHistory: self.path, self.last_error) self._prune(stamp) if not self._save(): - # ⚠️ A STORE THAT CANNOT PERSIST RECORDS NOTHING - the rule, asked - # ONCE as a question about the outcome instead of tested twice - # against two symptoms of it. Keeping the merge here is exactly the - # cache the module docstring refuses: `yesterday()` would answer a - # series the file does not contain, all day, until a restart - # dropped it without a word - and `words` and `yesterday()` would - # be describing two different stores. The reason is already in - # `last_error` (from `_save()`, or from `_open()` when there is no - # volume at all), so both answers now describe what is on disk. - # - # It does NOT raise, and §6 is why: a broken cache must not be a - # broken controller. `carry()` still returns the fill, the horizon - # is as short as the feed makes it, and the add-on regulates. + # 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. self._days = before return _LOG.debug("price history: %d point(s) filed under %s; %d day(s) held", len(points), ", ".join(sorted(touched)), len(self._days)) def _prune(self, stamp: datetime) -> None: - """Drop records no lookup can reach. A sort-order cap, plus two pins. - - ⚠️ NOTHING IS DELETED BY ARITHMETIC AGAINST THE CLOCK. A clock that - jumped - the failure mode WITNESS-03 spent a session on - would - otherwise delete every record in one pass and leave the fill with - nothing on the morning after, which is the one morning it is for. - Deleting on a bad clock is unrecoverable; holding a few days too many - is not, so the cap is `sorted(...)[-KEEP_DAYS:]` and stays that way. - - ⚠️ AND A CAP ALONE IS DEFEATED BY ONE POINT IN THE WRONG YEAR. Sorting - is lexicographic on the date string, so a stray far-future record sorts - LAST and is immortal - it consumes a slot for good, and with the steady - state already {yesterday, today, tomorrow} filling all three, yesterday - is what falls out. A feed that publishes to D+2 does the same thing - without any stray at all. So the two days `yesterday()` can ACTUALLY be - asked for are pinned on top of the cap. - - `stamp` is READ here, which it was not before: the pin is the only use - of it, and it is additive. Pinning can only RETAIN a record, never - delete one, so a jumped `stamp` pins two dates that are not held and - changes nothing - the WITNESS-03 property is preserved by construction - rather than by remembering to check it. - """ + """Drop records no lookup can reach: a sort-order cap plus two pins. Nothing is + deleted by arithmetic against the clock (a jumped clock would delete everything, + WITNESS-03). The pin of the two dates `yesterday()` can be asked for only + retains, so a far-future record cannot push yesterday out.""" if len(self._days) <= KEEP_DAYS: return keep = set(sorted(self._days)[-KEEP_DAYS:]) @@ -756,11 +337,7 @@ class PriceHistory: def yesterday(self, stamp: datetime) -> PriceRead: """The published series of the Brussels day BEFORE `stamp`'s, or no_data. - - ⚠️ THE DATE IS COMPUTED, NOT LOOKED UP. See rule 2: `newest()` would - answer Friday's prices to a Monday fill, and the fill would flag them as - one day old because that is the only thing its flag can say. - """ + Computed, not looked up (rule 2).""" if stamp.tzinfo is None or stamp.utcoffset() is None: raise ValueError( f"yesterday({stamp!r}): a naive datetime is not an instant, " @@ -803,19 +380,9 @@ class PriceHistory: stamp) def stored(self, stamp: datetime) -> PriceRead: - """PRICE-01: every price this store holds, as one series, or no_data. - - This is the LAST step of the price server's fallback (price_chain.py): - the answer of the last fetch that worked, kept on disk, so a site can - plan for as long as the days it holds last. It is a read of what was - PUBLISHED and filed, in the order they happened, and it is never the - carried fill (rule 1 holds: `carry(..., remember=False)` is how this - read is filled, and it is not filed again). - - The prices are the all-in ones stored when they were fetched. They are - not worked out again, so a change of markup or taxes shows from the - next fetch that works, not in these. - """ + """PRICE-01: every price this store holds, as one series, or no_data. The last + step of the price server's fallback (price_chain.py): what was PUBLISHED and + filed, never what a fill produced.""" if stamp.tzinfo is None or stamp.utcoffset() is None: raise ValueError(f"stored({stamp!r}): a naive datetime is not an " f"instant") @@ -833,25 +400,12 @@ class PriceHistory: return PriceRead.series(points, PriceProduct.DAY_AHEAD, "price_history", stamp) - # -- observability ------------------------------------------------------- + # observability @property def words(self) -> str: """One line for the status page. Says what is held, or why nothing is. - - ⚠️ THIS MUST NOT RAISE, EVER. It is read once from `Controller.__init__` - and again on every status render, and neither call site guards it - so - an exception here is an add-on that does not start. It went out that - way: a day record that was a string instead of a dict raised - `AttributeError` from the `len(...)` below. `_load()` now drops such a - record and `_held()` is total, so this has two independent reasons not - to fail on a broken file. - - ⚠️ AND IT COUNTED POINTS AND CALLED THEM HOURS (FCST-05's LOW). The row - was `f"{d} ({_n_points(rec)} h)"`; on the quarter-hourly feed this - add-on already supports that reads `'2026-01-13 (96 h)'` for a 24-hour - day. Both numbers are printed now - see `_held()` - because they are - two different facts and only one of them was ever being shown. - """ + Must not raise, ever: it runs from `Controller.__init__` and every status + render, unguarded. Prints points and hours separately (FCST-05).""" if not self.ok: return f"unavailable - {self.last_error}" if not self._days: @@ -859,26 +413,20 @@ class PriceHistory: return f"empty{why} - nothing to carry forward yet" held = ", ".join(_row_of(d, *_held(self._days[d])) for d in sorted(self._days, key=str)) - # A file that PARTLY loaded says so, or the dropped days read as days - # that were simply never written. + # A file that partly loaded says so, or dropped days read as never + # written. return held + (f" [{self.last_error}]" if self.last_error else "") def _parse_points(raw) -> tuple: - """Canonical dicts -> PricePoints, and a phrase naming what was dropped. - - ⚠️ A MALFORMED POINT IS DROPPED, NOT DEFAULTED, and the drop is reported. - `float(row.get("eur_per_kwh", 0))` is the shape of the whole family of - defects this repo has been removing: a cache that lost a field would answer - "that hour was free" to a fill that flags it as an ordinary guess. - """ + """Canonical dicts -> PricePoints, and a phrase naming what was dropped. A + malformed point is dropped, not defaulted: a lost field must not read as a free hour.""" if not isinstance(raw, list): return (), "the `points` field is not a list" out, dropped = [], 0 for row in raw: - # ⚠️ `_point_of()` IS THE ONE VALIDATOR, and this loop used to BE it. - # The write path had its own, weaker standard; see `_point_of()` for - # what that cost. One standard, two callers, no asymmetry to re-open. + # `_point_of()` is the one validator; this loop used to be a weaker + # one. pt = _point_of(row) if pt is None: dropped += 1 @@ -889,20 +437,8 @@ def _parse_points(raw) -> tuple: class NoHistory: - """The explicit "this caller keeps no history" - a NAME, not a `None`. - - ⚠️ IT EXISTS SO THE PARAMETER CAN BE REQUIRED. `history=None` meaning "skip - the fill" is the shape this repo keeps removing: a caller who forgets the - argument gets the truncated-horizon behaviour of the bug this ticket is - about, silently, and the code reads as though the fill is happening. With - no default there is nothing to forget, and a caller with no durable volume - - a check, a replay harness - has to say `NoHistory()` and be visible in - the diff. - - It carries out the ordering faithfully: it records nothing and carries - nothing, so `carry()` returns the provider's read unchanged and the plan is - exactly what it was before FCST-04. - """ + """The explicit "this caller keeps no history": a NAME, not `None`, so the + parameter can be required. It records nothing and carries nothing.""" ok = False last_error = "no history is kept by this caller"