From c45893ffadf2166d26ac8b3882fc04ac2e41664a Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Thu, 10 Sep 2026 08:43:17 +0200 Subject: [PATCH] FCST-06: "cannot persist" becomes one question, asked once, about the outcome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit price_history.py's docstring states its rule unconditionally - "A store that cannot persist now records nothing, and both of its answers say the same thing" - and FCST-04 enforced it against `not self.ok` only, which is one SYMPTOM: the durable volume is missing. `ok` is about the DIRECTORY, and a read-only remount and a full disk both leave it True. Reproduced on 5059e28 against two real volumes, a /data bind-mounted read-only and a full tmpfs: last_error = 'could not write ([Errno 30] Read-only file system)' words = '2026-01-14 (24 points over 24 h); 2026-01-15 (24 points over 24 h) [could not write (...)]' yesterday()= series, 24 points ON DISK = {} `words` and `yesterday()` were describing two different stores, and the 24 points were gone on the next restart with nothing saying so - the silently-forgetting cache that rule exists to refuse, reached by the condition it did not name. No shipped path reaches it: /data is the Supervisor's durable volume and this module is its only writer. The defect is that a guarantee the module states about itself did not hold in one condition. Route taken: `_remember()` refuses to retain what it could not write, and the docstring's claim stands as written. It is now ONE derived predicate rather than two places testing two symptoms. "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. No volume is one False, an OSError is another. `_remember()` keeps only what that one answer says was kept, and the `not self.ok` test in `_remember()` is GONE rather than joined by a second one. FCST-04's M4 and FCST-05's HIGH were the same shape, so a third cause now needs no third branch. What is undone is the MERGE, not the store. The state restored is the last one that IS on disk, so 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. It stops adding to it. The snapshot is copy.deepcopy: a one-level copy shares the day record the merge rebinds `points` on, and measured with one, a store holding 12 persisted points answered 24 after a failed merge. Not fatal, deliberately: §6 and this module's opening - a broken cache must not be a broken controller, and an add-on that will not start because /data filled up is a worse defect than this one. `_save()` reports and returns; the shipped resolve_tariff() path still gives a provider plan with no §11.4 degradation. What changed is that the failure is now acted on rather than only logged. test_price_history.py section 5d drives it on two REAL failures with different syscalls and different errnos - a directory where the file goes (os.replace, EISDIR) and a volume moved out from under an open store (mkstemp, ENOENT) - because the rule is enforced on the outcome and one cause would grade a coincidence. DOCS.md's "where there is no /data" sentence is widened to the condition the code now holds to. MUTATION.md gains what M4 exposed: a mutation whose red is a TRACEBACK reports however many checks ran before it and grades none after it. M4 first printed 1 FAIL; the true figure is 3. An f-string operand read `len(read.points)`, and `.points` on a no_data raises `Absence` by design - why()'s rule arriving in what a check PRINTS rather than in what it asserts. `npts()` is now beside why(), and a red count is a lower bound until the run is seen to reach the end of its section. 6 mutations, purged tree, every one RED on named checks, no survivors: M1 self._days = before -> dict(self._days) +10 B 9 red M2 _save() OSError handler returns True -1 B 9 red M3 _save() returns True on `not self.ok` -1 B 3 red M4 self._days = before -> self._days = {} -4 B 3 red M5 copy.deepcopy -> dict -9 B 3 red M6 `if not self.ok: return` back in _remember() +43 B 1 red All 29 test files green. Nothing deployed; no option added; config.yaml and app/ behaviour on a healthy /data untouched. No version number picked - that is the open decision the Unreleased block names. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU --- goodwe_controller/CHANGELOG.md | 57 ++++++ .../FCST-06-cannot-persist-is-one-question.md | 125 ++++++++++++ goodwe_controller/DOCS.md | 18 +- goodwe_controller/MUTATION.md | 22 ++ goodwe_controller/app/price_history.py | 123 ++++++++++-- goodwe_controller/test_price_history.py | 189 ++++++++++++++++++ 6 files changed, 512 insertions(+), 22 deletions(-) create mode 100644 goodwe_controller/DECISIONS/FCST-06-cannot-persist-is-one-question.md diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index 6ee3d27..2c3ffc2 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -14,6 +14,63 @@ if it has, it needs 0.10.0. Whoever deploys decides. |---|---| | _this branch_ | FCST-04 - the carry-forward fill reaches the plan, and the day it carries survives a restart | | _this branch_ | FCST-05 - one byte stops stopping the add-on, and the write path gets the read path's standard | +| _this branch_ | FCST-06 - "cannot persist" stops meaning "has no /data" and starts meaning what it says | + +### FCST-06 - the second way to be a store that cannot persist + +**A price cache that could not write went on answering a yesterday the file did +not contain.** `price_history.py`'s docstring states its rule unconditionally - +*"A store that cannot persist now records nothing, and both of its answers say +the same thing"* - and FCST-04 enforced it against `not self.ok`, which is one +**symptom**: the durable volume is missing. `ok` is about the DIRECTORY, and a +read-only remount and a full disk both leave it `True`. Reproduced on `5059e28` +against two real volumes - a `/data` bind-mounted read-only and a full tmpfs: + +| | with a `/data` that cannot be written | +|---|---| +| `last_error` | `could not write ([Errno 30] Read-only file system)` | +| `words` | `2026-01-14 (24 points over 24 h); 2026-01-15 (24 points over 24 h) [could not write (...)]` | +| `yesterday()` | **series, 24 points** | +| on disk | `{}` | + +`words` and `yesterday()` were describing two different stores, and the 24 +points were gone on the next restart with nothing saying so - the +silently-forgetting cache that rule exists to refuse, reached by the condition +it did not name. **No shipped path reaches it**: `/data` is the Supervisor's +durable volume and this module is its only writer. The defect is that a +guarantee the module states about itself did not hold in one condition. + +**"Cannot persist" is now ONE derived predicate, asked once, about the +outcome.** It cannot be answered *before* the attempt - a directory that existed +and was writable a moment ago is not a write that completed - so `_save()` +returns whether the file now holds what memory holds: `False` with no durable +volume, `False` on an `OSError` from the write, `True` otherwise. `_remember()` +keeps only what that one answer says was kept, and **the `not self.ok` test in +`_remember()` is gone** rather than joined by a second one. FCST-04's M4 and +FCST-05's HIGH were the same shape - a rule enforced on the conditions somebody +thought of - so a third cause now needs no third branch. + +**What is undone is the MERGE, not the store.** The state restored is the last +one that *is on disk*, so 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. It simply stops adding to it. The +snapshot is a `copy.deepcopy`: a one-level copy shares the day record the merge +rebinds `points` on, and measured with one, a store holding 12 persisted points +answered **24** after a failed merge. + +⚠️ **A failed write is still not fatal, and that was a constraint rather than an +oversight.** §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 being fixed, so `_save()` reports and returns - the failure +stays in the log, in `last_error`, in `words` and in `yesterday()`'s reason - +and the shipped `resolve_tariff()` path still returns a provider plan with no +§11.4 degradation. What changed is that it is now *acted on*. + +`test_price_history.py` section 5d drives it on two real failures with different +syscalls and different errnos - a directory where the file goes (`os.replace`, +EISDIR) and a volume that moves out from under an open store (`mkstemp`, +ENOENT) - because the rule is enforced on the outcome and one cause would grade +a coincidence. See `DECISIONS/FCST-06-cannot-persist-is-one-question.md`. ### FCST-05 - what FCST-04's own contract still did not hold diff --git a/goodwe_controller/DECISIONS/FCST-06-cannot-persist-is-one-question.md b/goodwe_controller/DECISIONS/FCST-06-cannot-persist-is-one-question.md new file mode 100644 index 0000000..94b01b8 --- /dev/null +++ b/goodwe_controller/DECISIONS/FCST-06-cannot-persist-is-one-question.md @@ -0,0 +1,125 @@ +# FCST-06 — "cannot persist" is one question, and it is asked about the outcome + +Two decisions. The first is the one the ticket put up for judgement; the second +is the one that decides what a store answers *after* it has failed to write. + +--- + +## 1. One derived predicate, not two places testing two symptoms — yes + +`price_history.py`'s module docstring states, unconditionally: + +> ⚠️ AND THAT RULE IS CARRIED OUT IN `_remember()`, NOT ONLY IN `_save()`. … +> A store that cannot persist now records nothing, and both of its answers say +> the same thing. + +It was enforced against `not self.ok`, and `ok` is **about the directory**: it +says `/data` was there when the store opened. A read-only remount and a full +volume both leave it `True`. So the second way to be a store that cannot +persist — `ok=True`, the directory present, `_save()` failing — was outside the +one condition the rule named, and in that state `words` printed two full days +beside the write error while `yesterday()` answered 24 real points, from memory, +until a restart dropped them without a word. + +**The ticket's question was whether "cannot persist" should be one derived +predicate rather than two places each testing a different symptom. It should, +and the reason is not tidiness — it is that the enumeration cannot be +completed.** FCST-04's M4 enforced the rule on the symptom that was in front of +it. FCST-05's HIGH was the same shape — an `except` clause naming the one +content-failure member somebody had met (`json.JSONDecodeError`) instead of the +`ValueError` family — and its fix was to name the family. This is the third instance in +one module, and it is the last place in this file where the rule can be spelt +twice. A fourth cause — an `OSError` subclass, a filesystem that accepts the +`mkstemp` and refuses the `os.replace`, a container whose volume is unmounted +mid-refresh — needs no fourth branch now, because none of them is asked about. + +### Where the one question can be asked, which is the whole design + +**"Can this store persist?" cannot be answered before the attempt.** A +directory that exists and was writable a moment ago is not a write that +completed; there is no pre-check that is not itself a guess, and adding one +(`os.access`, a probe write) would be a *third* symptom test — precisely what +the ticket rules out. So the predicate is an **outcome**: + + _save() -> bool # True only if the file now holds what memory holds + +Both ways of not persisting are the same answer from the same call: no durable +volume returns `False` without touching the disk, an `OSError` out of the write +returns `False` after recording why. `_remember()` reads that one answer and +keeps only what it says was kept. **The `not self.ok` test in `_remember()` is +gone** — not moved, gone — because there is nothing left for a second test to +add, and a check in `test_price_history.py` 5d asserts its absence from the +source. + +⚠️ **That last check is deliberately a source check, and habit 4 is why it is +allowed to be.** The claim "there are not two places testing two symptoms" is a +claim about the *shape of the code*, not about a behaviour — the behaviour is +identical whether the rule is enforced once or twice. Every other check in 5d +is driven. + +### What was NOT done, and why + +- **Not a `can_persist()` helper.** It would have to answer before the attempt, + which is the thing that cannot be done honestly, and it would be a second + place to keep in step with the first. +- **Not a probe write at `_open()`.** It would make `ok` mean something new, it + would write to `/data` on every start, and a volume that is writable at 08:00 + and full at 14:00 would still defeat it. A predicate that can go stale is a + symptom test wearing a predicate's name. +- **Not making a failed write fatal.** §6, and this module's own 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 being fixed. The + store degrades; it does not raise. `_save()` reports and returns. + +--- + +## 2. The MERGE is undone, not the store + +When `_save()` answers no, `_remember()` restores the state the read started +from — **not** an empty one. + +The alternative considered was to clear `self._days` outright, on the argument +that a store which cannot write is a store that should stop answering. It is +wrong, and measurably so: the state being restored is the last one that **is on +disk**, so answering it is not a promise the store cannot keep. A site whose +volume fills at 14:00 goes on answering the yesterday it wrote at 13:30 — +through the restart, because that record is in the file. What it stops doing is +*adding* to it. + +Clearing would throw away a good yesterday that survives a restart in order to +punish a write that failed, and it would make the morning fill worse on exactly +the day the operator is already dealing with a full disk. + +That gives the file the invariant its rule was always describing, stated once +in the class docstring: + +> `self._days` **never holds a point the file does not.** + +⚠️ Not *"mirrors the file"* — `_usable_days()` drops records the file still +holds, and that asymmetry is deliberate. The only direction that can **invent a +yesterday** is the one named. + +⚠️ **And the snapshot is `copy.deepcopy`, not `dict(self._days)`.** A one-level +copy shares the day *records*, and the merge rebinds `points` on the ones it +touches — so the "restored" store would answer the merge that never reached the +file. Measured: with a shallow copy, a store holding 12 persisted points +answered **24** after a failed merge. It would also be correct only for as long +as nobody makes the merge mutate a `points` list or a row in place, which is the +same "correct for the conditions someone thought of" this ticket is about. The +store holds at most `KEEP_DAYS + 2` records and writes 48 times a day; a copy +that is right by construction is worth more than the allocations. + +--- + +## What this does not change + +- **No shipped path reaches the defect.** `/data` is the Supervisor's durable + volume and this module is its only writer, so on a real install `_save()` + succeeds and nothing above is on the hot path. The fix is that a guarantee the + module states about itself now holds in the condition it did not. +- **The failure was already visible and still is** — `_LOG.error("could not + persist price history: %s")` and `last_error`, reported by both `words` and + `yesterday()`'s reason. What changed is that it is now *acted on*. +- **Nothing about the prices.** The series served is the feed's own, every + carried point is still flagged provisional, and a broken cache is still a + short horizon rather than a §11.4 degradation. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index fe54bce..c55d594 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -1488,10 +1488,20 @@ first fill is possible the day after the first refresh. The same applies to a file that is *partly* broken: a day record this build cannot read is dropped, the row says how many were dropped and why, and the rest -of the file still loads. And where there is no `/data` at all the store records -**nothing** — not even in memory. An in-memory yesterday would answer for one -run and vanish on the next restart, which is a cache that forgets without saying -so; `unavailable` and *no prices carried* are the honest pair. +of the file still loads. And a store that **cannot persist records nothing** — +not even in memory. That covers a missing `/data`, and it also covers a `/data` +that is there and cannot be **written**: a full volume, a read-only remount. An +in-memory yesterday would answer for one run and vanish on the next restart, +which is a cache that forgets without saying so; the row naming the reason and +*no prices carried* are the honest pair. + +⚠️ **What a failed write does not do is throw away what is already in the +file.** A volume that fills at 14:00 leaves the yesterday written at 13:30 still +answering, because that record survives a restart — the store stops *adding* to +it, nothing more. And it never stops the add-on: a write that cannot complete is +logged and reported on the status row, never raised. A cache that cannot answer +is not allowed to be a controller that cannot regulate, and that applies to the +volume being full exactly as it applies to the volume being absent. **Injection is priced separately** (`export_price_eur_kwh`). In Belgium it is often near zero, which by itself strongly favours self-consumption over export. diff --git a/goodwe_controller/MUTATION.md b/goodwe_controller/MUTATION.md index 03dbf2d..d434dbe 100644 --- a/goodwe_controller/MUTATION.md +++ b/goodwe_controller/MUTATION.md @@ -142,6 +142,28 @@ red on 15 named checks. Same shape as TEST-16 and the same fix: **grade the reason, not the verdict**, and when two rungs answer one stimulus, mutate them together. +⚠️ **AND FCST-06 FOUND THE COUNT ITSELF LYING — LOW, NOT SILENT.** A mutation +whose red is a **traceback** does not report one red; it reports however many +checks ran before the traceback, and grades **none** of the ones after it. The +new section drove M4 (`self._days = before` → `self._days = {}`) and the run +printed **1 FAIL**. The true figure is **3**: an f-string in the next check read +`len(read.points)`, and `.points` on a `no_data` **raises `Absence` by design**, +so the harness died mid-section with the remaining checks ungraded. That is +`why()`'s and `derived()`'s rule — *"it answered nothing" and "it broke on the +way to answering" must not grade the same* — arriving in the **printed operand** +of a check rather than in its condition, which is the half that is easy to miss +because it looks like formatting. Two things follow: + +* **a mutation report's red COUNT is only a lower bound until the run is seen to + have reached the end of its section.** Read the output, not the count; +* **the total accessor has to cover what a check PRINTS, not only what it + asserts.** `test_price_history.py` 5d added `npts()` beside `why()` for + exactly this, and re-driving M4 against it moved 1 FAIL to 3. + +Worse, had the traceback landed one check earlier the mutation would have +produced **no named red at all** — a result that reads like a broken harness and +gets retried rather than read. + ## The third way a check lies: its DATA cannot reach the failure it names Stale bytecode manufactures survivors from a mutation that never ran. Masking diff --git a/goodwe_controller/app/price_history.py b/goodwe_controller/app/price_history.py index 40efc91..f7745c4 100644 --- a/goodwe_controller/app/price_history.py +++ b/goodwe_controller/app/price_history.py @@ -61,6 +61,38 @@ 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 ===================================== @@ -148,6 +180,7 @@ and turning it on is a product decision, not a wiring one. See DECISIONS/FCST-04-the-day-that-is-carried.md. """ +import copy import json import logging import os @@ -333,6 +366,14 @@ class PriceHistory: 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. """ def __init__(self, path: str = "/data/price_history.json"): @@ -478,14 +519,31 @@ class PriceHistory: "it", self.path, self.last_error) return kept - def _save(self) -> None: + 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. + """ if not self.ok: - return - # ⚠️ 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. + return False tmp = None try: fd, tmp = tempfile.mkstemp(dir=os.path.dirname(self.path)) @@ -498,12 +556,14 @@ class PriceHistory: except OSError as err: self.last_error = f"could not write ({err})" _LOG.error("could not persist price history: %s", err) + return False finally: if tmp is not None: try: os.unlink(tmp) except OSError: # nothing further to try, and a pass # failed cleanup is not a fault + return True # -- the one public entry point ----------------------------------------- def carry(self, read: PriceRead, *, since: datetime, until: datetime, @@ -540,15 +600,6 @@ class PriceHistory: nothing outside this file names it, and `test_price_history.py` checks the tree for that rather than trusting the underscore. """ - if not self.ok: - # ⚠️ A STORE THAT CANNOT PERSIST RECORDS NOTHING. See the module - # docstring's third warning: filling `self._days` here would give a - # site with no /data an in-memory yesterday that answers a series - # all day and is gone on the next restart - a cache that forgets - # without saying so, which is the one thing that rule refuses. The - # honest degraded state is `words` = "unavailable" and `yesterday()` - # = `no_data`, and those two must agree. - return points = read.match(series=lambda r: r.points, no_data=lambda _r: ()) if not points: return @@ -557,6 +608,25 @@ 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. + before = copy.deepcopy(self._days) + touched = set() junked = {} for p in points: @@ -605,7 +675,9 @@ class PriceHistory: # 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. + # 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. self.last_error = ( f"{sum(junked.values())} malformed point row(s) dropped from " + ", ".join(f"{d} ({n})" for d, n in sorted(junked.items()))) @@ -613,7 +685,22 @@ class PriceHistory: "without them and the refresh continues", self.path, self.last_error) self._prune(stamp) - self._save() + 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. + 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)) diff --git a/goodwe_controller/test_price_history.py b/goodwe_controller/test_price_history.py index c90fb87..dc2cf8d 100644 --- a/goodwe_controller/test_price_history.py +++ b/goodwe_controller/test_price_history.py @@ -1379,6 +1379,195 @@ check(f"...and a revision of that hour REPLACES it rather than sitting beside " and abs(_y15.points[0].eur_per_kwh - allin(YESTERDAY_MWH[6])) < 1e-9) +# --------------------------------------------------------------------------- # +print() +print("5d. FCST-06 - the OTHER way to be a store that cannot persist") + +# THE DEFECT. Section 5 above grades "a store that cannot persist records +# nothing" against `not ok` - the missing-durable-volume SYMPTOM - and that is +# the only condition the rule was ever enforced on. `ok` stays TRUE on a volume +# that is present and cannot be written: a read-only remount, a full disk. In +# that state `_remember()` had already populated `self._days`, `_save()` logged +# and returned, and the store went on answering a series for the rest of the +# process's life. Measured on 5059e28, twice, with a `/data` bind-mounted +# read-only and again on a full tmpfs: +# +# last_error = 'could not write ([Errno 30] Read-only file system)' +# words = '2026-01-14 (24 points over 24 h); 2026-01-15 (24 points +# over 24 h) [could not write (...)]' +# yesterday()= series, 24 points +# ON DISK = {} <- and gone on the next restart +# +# `words` and `yesterday()` were describing two different stores, which is +# exactly what section 5's own comment refuses - reached by the condition the +# rule did not name. +# +# ⚠️ THE LEVERS ARE REAL FAILURES, NOT PATCHED ONES, AND THERE ARE TWO OF THEM +# ON PURPOSE. The fix is written against the OUTCOME - `_save()` answering "no" +# - and not against an errno, so one cause would grade a coincidence. A +# DIRECTORY where the file goes fails `os.replace`; a volume that moves out +# from under an already-open store fails `mkstemp`. Different syscall, +# different errno, one answer (habit 10: grade the family). + +def npts(read): + """A read's point count, or "no_data". Never raises. + + ⚠️ NOT `len(read.points)`. On a `no_data` that attribute RAISES `Absence` + by design, so a mutation that empties a store where this section expects + one would take the harness down with a traceback instead of reporting a + FAIL - and MUTATION.md's rule is that "it answered nothing" and "it broke + on the way to answering" must not grade the same. Found by driving M4. + """ + return read.match(series=lambda r: len(r.points), + no_data=lambda _r: "no_data") + + +_d16 = tmpdir() +_p16 = os.path.join(_d16, "price_history.json") +_h16 = PriceHistory(_p16) +os.mkdir(_p16) # `os.replace` cannot complete onto a directory +check(f"the fixture is an `ok` store on a directory that IS there " + f"(ok={_h16.ok}, last_error={_h16.last_error!r}) - not the " + f"no-durable-volume case section 5 already grades, or this section is " + f"that one again (habit 1)", + _h16.ok and _h16.last_error is None and os.path.isdir(_d16)) +_carried16 = _h16.carry(series_of(YDAY_H + TODAY_H, TODAY_MWH), + since=NOW, until=_until, stamp=NOW) +_y16 = _h16.yesterday(NOW) +check(f"a write that could not complete is REPORTED and leaves the store " + f"`ok` (ok={_h16.ok}, {(_h16.last_error or '')[:44]!r}). `ok` is about " + f"the DIRECTORY, and a full volume and a read-only remount both leave " + f"it True - which is why testing `ok` alone could not see this", + _h16.ok and "could not write" in (_h16.last_error or "")) +check(f"...and the file really does not hold the day " + f"(isfile={os.path.isfile(_p16)}), or 'it answers what is not there' is " + f"not a measurement (habit 1)", + not os.path.isfile(_p16)) +check(f"A STORE THAT COULD NOT WRITE RECORDS NOTHING, exactly as one with no " + f"durable volume does: it holds {sorted(_h16._days)} and `yesterday()` " + f"is {_y16.kind!r}. Retaining the merge is an in-memory yesterday that " + f"answers all day and is gone on the next restart - the " + f"silently-forgetting cache this module's docstring exists to refuse", + _h16._days == {} and _y16.kind == "no_data") +check(f"...and `words` and `yesterday()` AGREE about that store " + f"{_h16.words[:44]!r}: neither names a day the file does not hold, and " + f"both name the write failure. They disagreed - `words` printed two " + f"full days BESIDE the error while `yesterday()` answered 24 points", + YDAY_D not in _h16.words and TODAY_D not in _h16.words + and "could not write" in _h16.words + and _y16.kind == "no_data" and "could not write" in why(_y16)) +check(f"...and it DEGRADES rather than raising: the carry still returned a " + f"real series ({_carried16.kind!r}, {npts(_carried16)} points) with " + f"nothing carried. §6 and this module's opening - an add-on that will " + f"not start because /data filled up is a WORSE defect than the one this " + f"section is about", + _carried16.kind == "series" + and not _carried16.match( + series=lambda r: any(p.provisional for p in r.points), + no_data=lambda _r: True)) +_plan16 = derived(lambda: run(resolve_tariff( + CFG, NOW, _until, transport=transport_for(_body_today), now=NOW, + history=_h16)).origin) +check(f"...and the SHIPPED `resolve_tariff()` path is unmoved by it " + f"({_plan16!r}): a provider plan, no §11.4 degradation, on a store that " + f"cannot write a byte", + _plan16 == "provider") +# Habit 9: the same wrong input a SECOND time. A refresh runs 48 times a day, +# and a volume that is full at 14:00 is still full at 14:30. +_h16.carry(series_of(YDAY_H, YESTERDAY_MWH), since=NOW, until=_until, + stamp=NOW) +check(f"...and the SECOND failing refresh behaves the same rather than " + f"accumulating: still {sorted(_h16._days)} held and " + f"{_h16.yesterday(NOW).kind!r}. The write fails 48 times a day, so the " + f"call that matters is the one after the first (habit 9)", + _h16._days == {} and _h16.yesterday(NOW).kind == "no_data") + +# ⚠️ 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. This is also the fixture that separates a deep snapshot from +# `dict(self._days)`: a one-level copy shares the day RECORD the merge rebinds +# `points` on, so the "restored" store would answer the 24 points the merge +# made and the file never took. +_d17 = os.path.join(tmpdir(), "data") +os.mkdir(_d17) +_p17 = os.path.join(_d17, "price_history.json") +_h17 = PriceHistory(_p17) +_ph.PriceHistory._remember(_h17, series_of(YDAY_H[:12], YESTERDAY_MWH), NOW) +_first17 = _h17.yesterday(NOW) +check(f"a HALF day is persisted first ({npts(_first17)} points, " + f"last_error={_h17.last_error!r}), or 'the merge is undone and the " + f"store is not' grades nothing (habit 1)", + _h17.last_error is None and os.path.isfile(_p17) + and npts(_first17) == 12) +_persisted17 = open(_p17, encoding="utf-8").read() +_moved17 = _d17 + ".moved" +os.rename(_d17, _moved17) # the volume goes out from under an OPEN store: + # `mkstemp` fails, a different call and a + # different errno from the fixture above, and + # the bytes already written stay intact +_ph.PriceHistory._remember(_h17, series_of(YDAY_H[12:], YESTERDAY_MWH), NOW) +_y17 = _h17.yesterday(NOW) +_n17 = npts(_y17) +check(f"the second lever failed the write by a DIFFERENT route " + f"{(_h17.last_error or '')[:44]!r} - `mkstemp`, not `os.replace`, and " + f"ENOENT, not EISDIR. The rule is enforced on the outcome, so it must " + f"be driven by more than one cause (habit 10)", + "could not write" in (_h17.last_error or "") + and "No such file" in (_h17.last_error or "")) +check(f"...and the store keeps the last state that IS ON DISK, not an empty " + f"one: {_n17} points held - the 12 it wrote before the volume went, not " + f"the 24 the unwritten merge made, and not nothing. The MERGE is undone; " + f"the store is not", + npts(_y17) == 12) +_ph.PriceHistory._remember(_h17, series_of(YDAY_H[12:], YESTERDAY_MWH), NOW) +_y17b = _h17.yesterday(NOW) +check(f"...and again on the second failing refresh ({npts(_y17b)} points), " + f"because a volume that has gone is still gone 30 minutes later " + f"(habit 9)", + npts(_y17b) == 12) +os.rename(_moved17, _d17) # the volume comes back: what a RESTART finds +_r17 = PriceHistory(_p17).yesterday(NOW) +check(f"...and that is EXACTLY what a restart finds ({npts(_r17)} points, the " + f"file byte-for-byte as it was left), which is the whole claim: the " + f"store answers what survives a restart and nothing else", + open(_p17, encoding="utf-8").read() == _persisted17 + and npts(_r17) == 12 + and _r17.match(series=lambda a: _y17.match( + series=lambda b: [p.start for p in a.points] + == [p.start for p in b.points], + no_data=lambda _r: False), no_data=lambda _r: False)) + +# ⚠️ AND "CANNOT PERSIST" IS ONE PREDICATE, ASKED ONCE. FCST-04's M4 and +# FCST-05's HIGH were both a rule enforced on the conditions somebody thought +# of rather than on the family; this is the third of the shape and the last +# place it can be spelt twice. `_save()` returns whether the file now holds +# what memory holds - no volume is one False, an `OSError` is another - and +# `_remember()` keeps only what that answer says was kept. +check(f"`_save()` answers for BOTH symptoms and is the only thing that does: " + f"no durable volume -> {_ph.PriceHistory._save(_missing)!r}, a write " + f"that fails -> {_ph.PriceHistory._save(_h16)!r}, a store that " + f"persisted -> {_ph.PriceHistory._save(_hok)!r}", + _ph.PriceHistory._save(_missing) is False + and _ph.PriceHistory._save(_h16) is False + and _ph.PriceHistory._save(_hok) is True) +check(f"...and asking it once leaves the two symptoms with ONE behaviour: " + f"no volume holds {sorted(_missing._days)}/" + f"{_missing.yesterday(NOW).kind!r} and a failed write holds " + f"{sorted(_h16._days)}/{_h16.yesterday(NOW).kind!r}. A third cause " + f"nobody has thought of needs no third branch - which is how this defect " + f"arrived, twice", + (_missing._days, _missing.yesterday(NOW).kind) + == (_h16._days, _h16.yesterday(NOW).kind)) +check("...and `_remember()` no longer tests a symptom of its own: the source " + "holds no `self.ok` test between the docstring and the merge. This one " + "IS about the code's shape - two places testing two symptoms is the " + "defect, not a behaviour (habit 4's stated exception)", + "if not self.ok:" not in inspect.getsource(_ph.PriceHistory._remember) + and "if not self.ok:" in inspect.getsource(_ph.PriceHistory._save)) + + # --------------------------------------------------------------------------- # print() print("6. rule 4 - hours accumulate within a day; they do not replace it") -- 2.54.0