SAFETY-30: pin WHICH thresholds each sign detector is armed with, not just what the numbers are
checks / unit (pull_request) Successful in 4m48s
checks / unit (pull_request) Successful in 4m48s
REVIEW FOUND A MUTATION SURVIVOR THAT UNDOES THE WHOLE PRODUCT OF AC 5, and it
is a coverage defect rather than a behaviour one - the shipped code arms the
detectors correctly today. In `_arm_sign_watchdogs()`, collapsing
inv.sign_wd.arm(self.sign_cfg if one else self.unit_sign_cfg, ...)
to `arm(self.sign_cfg, ...)` throws the re-derived 250 W / 85 s per-unit pair
away and arms every unit with SAFETY-02's meter-facing 300 W / 180 s - and left
ALL 19 FILES GREEN. Reproduced here before fixing anything: fresh per-mutation
tree, no __pycache__, mounted :ro, graded through tools/mutation_run.sh; -31 B;
19 pass, 0 fail.
WHY NOTHING CAUGHT IT. `grep -rn "sign_wd.cfg" test_*.py` found exactly ONE
threshold assertion, part 7's N=1 arm at (300.0, 1.0) - which the mutation
PRESERVES. Nothing at a roster of two read `inv.sign_wd.cfg` at all. The ledger
mutates the VALUES in SIGN_OPTION_DEFAULTS (M10 250->300, M11 325->180, both
correctly red through the config pins) and never the WIRING that delivers them.
Part 3's behavioural checks cannot close the gap either, and the comment now
says why: B is reversed at ~2000 W with short holds, so it trips under EITHER
pair, and a fault big enough and long enough to trip under both thresholds is
evidence about neither.
THE FIX - test_watchdog.py part 3, on the real two-inverter Controller. Every
`inv.sign_wd.cfg` equals the per-unit pair, `site_sign_wd.cfg` equals the site
pair, and each detector's `against` is its own instrument. Graded against
LITERALS, because `== self.unit_sign_cfg` cannot tell "the per-unit pair" from
"whatever this Controller was handed". Both pairs are printed.
⚠️ AND THE FIXTURE HAD TO CHANGE FOR THE PIN TO MEAN ANYTHING - the recursion
worth naming, because it is the same defect class as the one being closed.
`_sign_site` set `sign_disagree_s`, `unit_sign_disagree_s` AND
`site_sign_disagree_s` all to 1 s, with two of the three magnitude gates at
300 W. A bare equality there discriminates on `min_w` alone: a mis-wiring that
delivered the right magnitude with the WRONG HOLD compares EQUAL to the pair it
was supposed to have. A check comparing two things that happen to be equal
cannot see the difference it names (MUTATION.md's third failure mode). The
three pairs are now pairwise distinct IN BOTH COORDINATES - 250 W / 1 s per
unit, 320 W / 2 s site, 300 W / 3 s shipped - a PRECONDITION asserts that
distinctness so a later edit collapsing them goes red rather than quietly
vacuous, and a second PRECONDITION asserts the literals are the pairs this
Controller actually holds.
VERIFIED BY BREAKING IT. Three mutations, each on its own freshly copied tree
with no __pycache__, mounted :ro, through tools/mutation_run.sh:
M14 arm() handed self.sign_cfg at every roster -31 B RED (was the
survivor; the AC 5 WIRING check names the wiring)
M15 right min_w, SHIPPED hold - hold-only mis-wire +78 B RED, same pin
M16 right min_w, SITE hold - hold-only, and it still
trips inside the drive +83 B RED, and the
ONLY red in test_watchdog.py
M16 is the one that says the hold half of the equality bites on its own.
Unmutated tree: 19 pass, 0 fail, purged and fingerprinted.
Ledger rows M14-M16 and the write-up in DECISIONS/SAFETY-30-per-unit-sign.md
section 5; a CHANGELOG bullet.
NOT IN THIS PR, so it gets a card rather than a quiet fix: a stuck per-unit
power sensor would false-trip the new per-unit check at N>=2. Already-merged
SAFETY-31 has the identical exposure on the same fixture. Pre-existing N>=2
class, unreachable at the deployed roster of one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
co-authored by
Claude Opus 5
parent
f000547518
commit
d5c1577212
@@ -67,6 +67,17 @@ evidence.
|
||||
row - and the site detector's window is never so much as opened.
|
||||
`unit_sign_disagree_*` and `site_sign_disagree_*` do nothing on a
|
||||
one-inverter install.
|
||||
* **The threshold WIRING is pinned, not just the threshold values.** Review
|
||||
found that `arm(self.sign_cfg if one else self.unit_sign_cfg, ...)` could be
|
||||
collapsed to `arm(self.sign_cfg, ...)` — arming every unit with the
|
||||
meter-facing pair instead of the re-derived per-unit one — with all 19 test
|
||||
files still green, because nothing at a roster of two read `sign_wd.cfg` and
|
||||
the trips in the fixture happen under either pair. `test_watchdog.py` part 3
|
||||
now asserts the pair and the instrument on **every** detector against
|
||||
literals, and the fixture's three threshold pairs were made pairwise distinct
|
||||
in both coordinates so that a hold delivered from the wrong pair cannot
|
||||
compare equal to the right one. M14–M16 in
|
||||
`DECISIONS/SAFETY-30-per-unit-sign.md` §5.
|
||||
* **Known and recorded rather than fixed:** the site comparison is defeated by a
|
||||
square-wave load whose half period is about one control cycle, which holds it
|
||||
opposed for ~1000 s at *both* rosters. The N=1 control arm is what says this is
|
||||
|
||||
@@ -377,6 +377,9 @@ size-preserving edit is what TEST-15 turned into a false survivor once already.
|
||||
| M11 | `watchdog.py` | `site_sign_disagree_s` 325 → 180, i.e. **the number carried** | **+0 B** | **RED** — the measured-215 s check and the CFG-03 pin |
|
||||
| M12 | `arbiter.py` | `sign_site` scoped `SCOPE_UNIT` | **+0 B** | **RED** — `test_arbiter.py` **and** `test_peakguard.py` |
|
||||
| M13 | `validate.py` | the `unit_sign_disagree_w` rule removed | −126 B | **RED** — `test_validate.py` |
|
||||
| **M14** | `main.py` | **`arm()` handed `self.sign_cfg` at EVERY roster** — the WIRING, not the values | −31 B | **SURVIVOR at review — GAP CLOSED**, then RED. See below |
|
||||
| M15 | `main.py` | per-unit `arm()` given the right `min_w` and the SHIPPED hold — a HOLD-ONLY mis-wire | +78 B | **RED** — the same wiring pin |
|
||||
| M16 | `main.py` | per-unit `arm()` given the right `min_w` and the SITE hold — hold-only again, and behaviour-preserving | +83 B | **RED** — the wiring pin, and the ONLY red in `test_watchdog.py` |
|
||||
|
||||
### M4 is recorded as a survivor because it was one
|
||||
|
||||
@@ -400,6 +403,57 @@ cycles, zero calls, printed. A counter can only be zero for one reason. The
|
||||
mutation was re-run against it and is red; the run that found it was discarded
|
||||
along with its tree, so this is a result and not an artefact.
|
||||
|
||||
### M14: the VALUES were pinned and the WIRING was not
|
||||
|
||||
M10 and M11 mutate the numbers in `watchdog.SIGN_OPTION_DEFAULTS` and both are
|
||||
red through the config pins. Neither of them touches the line that DELIVERS
|
||||
those numbers to a detector:
|
||||
|
||||
```python
|
||||
inv.sign_wd.arm(self.sign_cfg if one else self.unit_sign_cfg,
|
||||
AGAINST_METER if one else AGAINST_OWN_POWER)
|
||||
```
|
||||
|
||||
Drop the conditional — `arm(self.sign_cfg, ...)` — and every unit is armed with
|
||||
SAFETY-02's meter-facing pair instead of the re-derived per-unit one. At review
|
||||
that left **all 19 files green**. `grep -rn "sign_wd.cfg" test_*.py` found
|
||||
exactly one threshold assertion, and it is §7's N=1 arm — `(300.0, 1.0)` —
|
||||
which the mutation PRESERVES. Nothing anywhere read `inv.sign_wd.cfg` at a
|
||||
roster of two.
|
||||
|
||||
The behavioural checks in part 3 cannot close it and it is worth saying why: B
|
||||
is reversed at ~2000 W and every hold in that fixture is short, so B trips under
|
||||
either pair. **A fault big enough and long enough to trip under both thresholds
|
||||
is evidence about neither.**
|
||||
|
||||
What the survivor costs in the field is the whole product of §3: at
|
||||
`config.yaml`'s own defaults the per-unit check would run at `sign_disagree_s`
|
||||
**180 s instead of 85 s** — 2.1× slower to park a reversed inverter — and its
|
||||
gate would move from 250 W to 300 W, so a unit reversing a share between the two
|
||||
becomes invisible, while `SIGN_OPTION_DEFAULTS`, `config.yaml` and §3 all go on
|
||||
describing a pair the running code does not use. With CI green.
|
||||
|
||||
**Closed by a wiring pin in `test_watchdog.py` part 3**, on the real
|
||||
two-inverter Controller: every `inv.sign_wd.cfg` equals the per-unit pair,
|
||||
`site_sign_wd.cfg` equals the site pair, and each detector's `against` is its
|
||||
own instrument — all graded **against literals**, since `== self.unit_sign_cfg`
|
||||
cannot tell *the per-unit pair* from *whatever this Controller was handed*.
|
||||
|
||||
⚠️ **And the fixture had to be changed for that pin to mean anything.**
|
||||
`_sign_site` set `unit_sign_disagree_s: 1` **and** `site_sign_disagree_s: 1`
|
||||
**and** `sign_disagree_s: 1`, with two of the three magnitude gates at 300 W. A
|
||||
bare equality there discriminates on `min_w` alone: a mis-wiring that delivered
|
||||
the right magnitude with the wrong hold would compare EQUAL to the pair it was
|
||||
supposed to have. That is this document's own §5 lesson and MUTATION.md's third
|
||||
failure mode — a check comparing two things that happen to be equal cannot see
|
||||
the difference it names — reappearing inside the fix for it. The three pairs are
|
||||
now pairwise distinct **in both coordinates** (250 W / 1 s, 320 W / 2 s,
|
||||
300 W / 3 s), a PRECONDITION check asserts that distinctness so a later edit
|
||||
collapsing them goes red rather than quietly vacuous, and both pairs are
|
||||
**printed**. M15 and M16 exist to show the hold half actually bites: right
|
||||
magnitude, wrong hold, red both times — and M16, whose wrong hold still trips
|
||||
inside the drive, is red on the wiring pin and on **nothing else** in that file.
|
||||
|
||||
## 6. What this card does NOT close
|
||||
|
||||
* **The flapping-load blind spot in the SITE comparison.** §3, at length. Present
|
||||
|
||||
@@ -1342,13 +1342,31 @@ def _sign_site(hass, store=None, **over):
|
||||
stopped by circulation while this section is trying to grade a PER-UNIT
|
||||
attribution, because then every "only B is parked" check below would be
|
||||
measuring the wrong guard.
|
||||
|
||||
⚠️ THE THREE THRESHOLD PAIRS ARE PAIRWISE DISTINCT IN BOTH COORDINATES,
|
||||
AND THAT IS LOAD-BEARING RATHER THAN COSMETIC. This fixture is the only
|
||||
place in the suite where `_arm_sign_watchdogs()` runs at a roster of two, so
|
||||
it is the only place that can see WHICH pair reaches WHICH detector. The
|
||||
first version of it set every hold to 1 s and two of the three magnitude
|
||||
gates to 300 W; a detector armed with the shipped pair, or with the site
|
||||
pair, or with the right magnitude and the wrong hold, then compared EQUAL
|
||||
to the pair it was supposed to have, and the wiring check below would have
|
||||
passed on three of those four arrangements. That is MUTATION.md's third
|
||||
failure mode written into a fixture - a check comparing two things that
|
||||
happen to be equal cannot see the difference it names - and the
|
||||
precondition beside the check is what stops a later edit collapsing them
|
||||
again without a red line.
|
||||
|
||||
Anyone changing a hold here: the PER-UNIT hold is what part 3's trip timing
|
||||
is read against, and the SITE hold must still expire inside `_drive()`'s
|
||||
window (14 cycles x 0.2 s), which part 5 depends on.
|
||||
"""
|
||||
opts = {"setpoint_entity": "number.sp", "meter_entity": "sensor.grid",
|
||||
"soc_entity": "sensor.soc", "batt_entity": "sensor.batt",
|
||||
"meter_source": "off", "max_w": 2000, "gain": 0.6,
|
||||
"sign_disagree_w": 300, "sign_disagree_s": 1,
|
||||
"sign_disagree_w": 300, "sign_disagree_s": 3,
|
||||
"unit_sign_disagree_w": 250, "unit_sign_disagree_s": 1,
|
||||
"site_sign_disagree_w": 300, "site_sign_disagree_s": 1,
|
||||
"site_sign_disagree_w": 320, "site_sign_disagree_s": 2,
|
||||
"circulation_hold_s": 3600, "auto_start": True}
|
||||
opts.update(over)
|
||||
c = Controller(opts, hass, store if store is not None else _MemStore(),
|
||||
@@ -1392,6 +1410,76 @@ async def _one_reversed():
|
||||
|
||||
_c30, _h30, _s30 = asyncio.run(_one_reversed())
|
||||
_a30, _b30 = _c30.inverters
|
||||
|
||||
# ⚠️ THE WIRING PIN FOR AC 5, AND IT IS A DIFFERENT CLAIM FROM EVERY
|
||||
# BEHAVIOURAL CHECK IN THIS SECTION. The rest of part 3 grades what the
|
||||
# detectors CONCLUDED; this grades WHICH THRESHOLDS THEY WERE HANDED, and the
|
||||
# gap between the two was live. Replacing
|
||||
# `arm(self.sign_cfg if one else self.unit_sign_cfg, ...)` with
|
||||
# `arm(self.sign_cfg, ...)` in `Controller._arm_sign_watchdogs()` throws the
|
||||
# re-derived per-unit pair away and arms every unit with SAFETY-02's
|
||||
# meter-facing one - and left ALL 19 FILES GREEN. Nothing at a roster of two
|
||||
# read `inv.sign_wd.cfg` at all; the one place that does is part 7's N=1 arm,
|
||||
# which that edit PRESERVES.
|
||||
#
|
||||
# WHY THE TRIPS BELOW CANNOT CATCH IT, said here because "the suite is green"
|
||||
# and "nothing was wrong" are the two readings this comment exists to separate.
|
||||
# B is reversed at ~2000 W - far over 250 W and far over 300 W - and every hold
|
||||
# in this fixture is short enough to expire inside `_drive()`. A fault that is
|
||||
# big enough and long enough to trip under BOTH pairs is evidence about
|
||||
# neither. Only an equality on the pair itself can tell them apart.
|
||||
#
|
||||
# WHAT THE SURVIVOR COSTS IN THE FIELD, at config.yaml's own defaults: the
|
||||
# per-unit check runs at `sign_disagree_s` 180 s instead of the re-derived 85 s
|
||||
# - 2.1x slower to park a reversed inverter - and its gate moves from 250 W to
|
||||
# 300 W, so a unit reversing a share between the two becomes invisible, while
|
||||
# watchdog.SIGN_OPTION_DEFAULTS, config.yaml and DECISIONS section 3 all go on
|
||||
# describing a pair the running code does not use.
|
||||
_UNIT_PAIR = (250.0, 1.0) # `unit_sign_disagree_w` / `_s` - one per unit
|
||||
_SITE_PAIR = (320.0, 2.0) # `site_sign_disagree_w` / `_s` - one per site
|
||||
_SHIPPED_PAIR = (300.0, 3.0) # `sign_disagree_w` / `_s` - N=1 only
|
||||
_armed = {inv.name: (inv.sign_wd.cfg.min_w, inv.sign_wd.cfg.hold_s)
|
||||
for inv in _c30.inverters}
|
||||
_armed_site = (_c30.site_sign_wd.cfg.min_w, _c30.site_sign_wd.cfg.hold_s)
|
||||
print(f" armed per unit: {_armed}")
|
||||
print(f" armed site: {_armed_site} "
|
||||
f"shipped pair, wanted by NOBODY here: {_SHIPPED_PAIR}")
|
||||
check("PRECONDITION: the three pairs are pairwise distinct IN BOTH "
|
||||
"COORDINATES, so an equality below can fail on the magnitude alone, on "
|
||||
"the hold alone, or on both. This fixture originally gave all three the "
|
||||
"same 1 s hold and two of them the same 300 W gate, and a mis-wiring "
|
||||
"that delivered the right magnitude with the wrong hold would then have "
|
||||
"compared EQUAL to the pair it was supposed to have",
|
||||
len({_UNIT_PAIR, _SITE_PAIR, _SHIPPED_PAIR}) == 3
|
||||
and len({p[0] for p in (_UNIT_PAIR, _SITE_PAIR, _SHIPPED_PAIR)}) == 3
|
||||
and len({p[1] for p in (_UNIT_PAIR, _SITE_PAIR, _SHIPPED_PAIR)}) == 3)
|
||||
check("PRECONDITION: and all three are pairs this Controller actually holds, "
|
||||
"so the literals below are the fixture's own numbers and not three "
|
||||
"constants that agree with nothing. In particular the shipped pair is "
|
||||
"still constructed and still a reachable mis-wiring target",
|
||||
(_c30.unit_sign_cfg.min_w, _c30.unit_sign_cfg.hold_s) == _UNIT_PAIR
|
||||
and (_c30.site_sign_cfg.min_w, _c30.site_sign_cfg.hold_s) == _SITE_PAIR
|
||||
and (_c30.sign_cfg.min_w, _c30.sign_cfg.hold_s) == _SHIPPED_PAIR)
|
||||
check("⚠️ AC 5 WIRING. EVERY per-unit detector on this roster of two is armed "
|
||||
"with the PER-UNIT pair - 250 W / 1 s here, `unit_sign_disagree_w` and "
|
||||
"`unit_sign_disagree_s` in the field. Graded against LITERALS rather "
|
||||
"than against `_c30.unit_sign_cfg`, which cannot tell 'the per-unit "
|
||||
"pair' from 'whatever this Controller was handed'. This is the line that "
|
||||
"goes red when `_arm_sign_watchdogs()` stops choosing between the two "
|
||||
"pairs and hands every unit the site-facing one",
|
||||
len(_armed) == 2 and all(p == _UNIT_PAIR for p in _armed.values()))
|
||||
check("...and the SITE detector is armed with the SITE pair - 320 W / 2 s "
|
||||
"here - which differs from the per-unit pair AND from the shipped pair "
|
||||
"in both coordinates, so this equality is not carried by a number the "
|
||||
"fixture happens to hold twice",
|
||||
_armed_site == _SITE_PAIR)
|
||||
check("...and each detector is pointed at its own INSTRUMENT as well as at its "
|
||||
"own thresholds: the units at their own 0x0518 power, the site at the "
|
||||
"meter. They are two arguments of one call and a pin on one is not a pin "
|
||||
"on the other",
|
||||
all(inv.sign_wd.against == AGAINST_OWN_POWER for inv in _c30.inverters)
|
||||
and _c30.site_sign_wd.against == AGAINST_METER)
|
||||
|
||||
_mid = _s30[3]
|
||||
print(f" cycle 3: stood {[round(x) for x in _mid['stood']]} "
|
||||
f"measured {[round(x) for x in _mid['meas']]} "
|
||||
|
||||
Reference in New Issue
Block a user