adminandClaude Opus 5
c45893ffad
checks / unit (pull_request) Successful in 7m14s
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
126 lines
6.6 KiB
Markdown
126 lines
6.6 KiB
Markdown
# 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.
|