admin/goodwe-addon
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

SAFETY-42 round 2: pin the two thresholds at a config that is not the shipped one
checks / unit (pull_request) Successful in 6m7s
Details

Browse Source 
THE BLOCKER. At the `delinquent()` call site, `self.tuning.saturation_w` ->
`250.0` (-19 B) was a survivor of all 23 suites. It is not an equivalent
mutant: two copies of inverter A, B wedged, `saturation_w: 1800` (inside its
own schema int(100,2000)) and a load of 1600 W put B's shortfall strictly
between the literal and the configured value - head names nobody, the mutant
names B. At the shipped 500 both name it, which is exactly why every fixture
in part 4 was blind: their gaps are all above both thresholds.

It matters beyond a mutation score. `delivery.py`'s MIN_W_IS_SATURATION_W note
and the `delivery_hold_s` block in config.yaml both promise an operator, in
shipped prose, that there is no fourth copy of a threshold to keep in step and
no way for the two to drift - raise `saturation_w` and this check moves with
you. A literal at the call site removes exactly that promise, silently, and
the promise is why the design was chosen over a `delivery_w` key.

ASKING THE SAME QUESTION OF EVERY OTHER NUMBER THIS CHECK READS FOUND A SECOND
SURVIVOR OF ALL 23 SUITES: `bms_floor=inv.bms_soc_floor` -> `10.0` (-13 B),
invisible because config.yaml's own example states 10 % and every fixture in
this file used it. Two arms bracket it: a pack at 15 % against its own 25 %
cut-out is cold and must stay unnamed (the literal names an empty pack), and a
pack at 8 % against its own 5 % cut-out is wedged and must be named (the
literal explains the fault away). One arm on each side leaves no constant at
all.

`delivery_hold_s` and `is_live()`'s window were ALREADY pinned - the fixtures
had to move them to run - and `LINK_STALE_S` has no option behind it, so
"pinned across a changed config" is not available for it and is not claimed.
DECISIONS section 9 enumerates all of it with the deltas and which check kills
which.

Verified in the CI image, tree mounted :ro, `python3 -B`, `__pycache__` purged
and counted before every run (TEST-15):
  R5 saturation_w -> 250.0                    -19 B  killed by part 4b(a)
  R6 bms_soc_floor -> 10.0                    -13 B  killed by part 4b(b), both arms
  R7 arm(self.delivery_cfg) -> DeliveryConfig()  -1 B  killed (already)
  R8 is_live(now) -> is_live(now, 3600.0)      +8 B  killed (already)
  R9 pack cut-out -> conjoined site floor    +105 B  killed twice
  head: 23/23 test files PASS, no bytecode written during the run.

No shipped behaviour changes: the only edits to app/ are comments.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
Claude
2026-09-06 19:25:45 +02:00
committed by glenn schrooyen
parent c7eb8c6b97
commit 090a8ecc35
6 changed files with 366 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/test_delivery.py
+210 -5
View File
@@ -10,6 +10,14 @@ target, per-unit share, per-unit measured power), to the last emitted watt. If
this file's two fixtures ever stop differing, the check has stopped being
graded on the only thing that separates the two cases.
⚠️ AND PART 4b IS THE OTHER HALF OF THE SAME LESSON, ONE LEVEL UP. Part 4 shows
that no threshold on the power domain can separate the two cases; part 4b shows
that a check which only ever runs at the SHIPPED value of a threshold cannot
separate "this call site reads the operator's config" from "this call site has a
number typed into it". Both are the same defect - a fixture whose values cannot
reach the failure it names - and the second one hid two literals from all 23
suites in this repository.
⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run
`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it
stops bytecode being WRITTEN, not a stale one being READ. That is TEST-15, and
@@ -349,14 +357,24 @@ class _DeliveryHass:
return True
def _roster():
def _roster(floors=(_FLOOR_PCT, _FLOOR_PCT)):
"""The two-unit roster. `floors` is (A's own cut-out, B's own cut-out).
⚠️ IT IS AN ARGUMENT RATHER THAN `_FLOOR_PCT` TWICE, AND THAT IS HALF OF
PART 4b. Every fixture in this file used to state 10 %, which is also the
number config.yaml's own `inverters:` example states, so `bms_floor=10.0`
written at main.py's call site in place of `inv.bms_soc_floor` answered
identically in every check here and in the other 22 files. A default that
happens to agree with the fixture is not a pin.
"""
return [{"name": n, "setpoint_entity": f"number.sp_{t}",
"soc_entity": f"sensor.soc_{t}", "batt_entity": f"sensor.batt_{t}",
"max_w": 2000, "bms_soc_floor": _FLOOR_PCT}
for n, t in (("inverter-a", "a"), ("inverter-b", "b"))]
"max_w": 2000, "bms_soc_floor": f}
for (n, t), f in zip((("inverter-a", "a"), ("inverter-b", "b")),
floors)]
def _ctrl(hass, store=None, **over):
def _ctrl(hass, store=None, *, floors=(_FLOOR_PCT, _FLOOR_PCT), **over):
"""A two-inverter Controller, with the NEIGHBOURING guards given long windows.
⚠️ THE LONG `circulation_hold_s` AND SIGN HOLDS ARE ISOLATION, NOT A CLAIM
@@ -367,7 +385,7 @@ def _ctrl(hass, store=None, **over):
below would be measuring the wrong thing.
"""
opts = {"meter_entity": "sensor.grid", "meter_source": "off",
"inverters": _roster(), "gain": 0.6, "slew_w": 1000,
"inverters": _roster(floors), "gain": 0.6, "slew_w": 1000,
# ⚠️ 1 s, NOT THE SHIPPED 95. This window is WALL CLOCK - the
# detector takes the caller's `now`, and `_drive()` runs fourteen
# real cycles 0.2 s apart, so a shipped-length hold would make every
@@ -521,6 +539,193 @@ check("a wedged unit BETWEEN its own cut-out and the site's peak_soc_floor is "
and float(_cbf.o.get("peak_soc_floor")) == 20.0)
# ===========================================================================
print()
print("=" * 72)
print("4b. THE NUMBERS THIS CHECK READS ARE THE CONFIGURED ONES, AND THAT IS")
print(" GRADED AT A CONFIG THAT IS NOT THE SHIPPED ONE")
print("=" * 72)
# ⚠️ WHY THIS PART EXISTS, AND IT IS NOT A MUTATION SCORE. Two of the values
# main.py hands this check are OPTIONS, and the paragraph above
# `delivery.MIN_W_IS_SATURATION_W` and the `delivery_hold_s` block in
# config.yaml both make a PROMISE about them in shipped, operator-readable
# prose: there is no fourth copy of a default to keep in step, and no way for
# the two to drift, because an operator who raises `saturation_w` has told the
# loop that a bigger gap is ordinary tracking and this check moves with them BY
# CONSTRUCTION. A literal written at the call site breaks exactly that promise,
# silently, and the promise is why the design was chosen over a `delivery_w`
# key.
#
# ⚠️ AND EVERY FIXTURE ABOVE IS BLIND TO IT, WHICH IS THIS REPOSITORY'S
# RECURRING DEFECT (MUTATION.md's third failure mode): a fixture whose values
# cannot reach the failure it names. Part 4's gaps are all far above BOTH the
# shipped `saturation_w` and any smaller literal, and every `bms_soc_floor` in
# this file was 10 - the number config.yaml's own example states. So
#
# self.tuning.saturation_w -> 250.0 (-19 B)
# bms_floor=inv.bms_soc_floor -> 10.0 (-13 B)
#
# were BOTH survivors of the whole 23-file suite, not of this file alone. The
# two blocks below are what kills them, and they kill not just those two
# literals but EVERY literal: each pair of arms is answered one way at one
# configured value and the other way at another, on the same fixture, so no
# single constant can satisfy both rows.
print()
print(" (a) THE WATTS - `saturation_w`, raised to a value inside its own")
print(" schema. Same load, same wedged unit, two configured thresholds.")
# 1800 is inside the shipped schema `int(100,2000)` and is not the shipped
# default - both asserted below rather than asserted here in a comment. The
# load is what puts B's shortfall BETWEEN the two: above the shipped threshold,
# below the raised one. The gap itself is printed by the run that produces it
# and is never written down here; what is asserted is the ORDERING.
_SAT_RAISED = 1800
_SAT_LOAD_W = 1600.0
_SAT_SHIPPED = int(read_option("saturation_w"))
_sat_lo, _sat_hi = (int(x) for x in re.match(
r"int\((\d+),(\d+)\)", read_option("saturation_w", "schema")).groups())
async def _at_saturation_w(sat):
hass = _DeliveryHass(load_w=_SAT_LOAD_W)
c = _ctrl(hass, saturation_w=sat)
return c, await _drive(c, watch=_sample)
_c_ship, _r_ship = asyncio.run(_at_saturation_w(_SAT_SHIPPED))
_c_up, _r_up = asyncio.run(_at_saturation_w(_SAT_RAISED))
_b_ship, _b_up = _c_ship.inverters[1], _c_up.inverters[1]
_gap_ship = short_by(_b_ship.standing_w(), _b_ship.batt)
_gap_up = short_by(_b_up.standing_w(), _b_up.batt)
check(f"the raised threshold is a config an operator can actually enter: "
f"{_SAT_RAISED} W is inside the shipped schema "
f"{read_option('saturation_w', 'schema')}, and it is NOT the shipped "
f"default ({_SAT_SHIPPED} W) - both read out of config.yaml, not typed "
f"here",
_sat_lo <= _SAT_RAISED <= _sat_hi and _SAT_RAISED != _SAT_SHIPPED)
check("...and the controller really is running on it - `Tuning.saturation_w` "
"carries the option through, so this is a changed CONFIG and not a "
"changed test",
_c_up.tuning.saturation_w == float(_SAT_RAISED)
and _c_ship.tuning.saturation_w == float(_SAT_SHIPPED))
# ⚠️ THE VACUOUS-PASS GUARD AGAIN, and here it is the whole load. "the raised
# arm named nobody" reads identically against a fixture where B was never
# asked, was not short, or was explained away. So the raised arm is asserted to
# have reached the threshold comparison and to have been turned away by NOTHING
# BUT the number.
check("the raised arm REACHED the comparison: B was asked for a discharge "
"share, is short of it, is live, is not parked, and NOTHING accounts for "
"it - the only thing standing between this unit and an alarm is the "
"configured watts",
float(_b_up.standing_w()) > 0 and _gap_up is not None
and not _b_up.sign_wd.tripped
and account(live=True, parked=False, soc=_b_up.soc,
bms_floor=_b_up.bms_soc_floor) is None)
check("...and its shortfall lands STRICTLY BETWEEN the two configured "
"thresholds, which is the one place the two answers can differ. Part 4's "
"fixtures are all above both, which is exactly why they cannot see this",
_gap_up is not None and float(_SAT_SHIPPED) < _gap_up < float(_SAT_RAISED))
check("AT THE SHIPPED THRESHOLD THE UNIT IS NAMED",
_r_ship[-1]["named"] == ("inverter-b",))
check("AT THE RAISED THRESHOLD, ON THE SAME FIXTURE, IT IS NOT - the operator "
"has told the loop that a gap this size is ordinary tracking, and this "
"check moved with them. THIS PAIR IS THE PIN: a literal at the call site "
"is one number, and no one number answers both of these rows",
_r_up[-1]["named"] == ())
print(f" shipped saturation_w {_SAT_SHIPPED} W: B short by "
f"{_gap_ship:.0f} W -> named {_r_ship[-1]['named']}")
print(f" raised saturation_w {_SAT_RAISED} W: B short by "
f"{_gap_up:.0f} W -> named {_r_up[-1]['named']}")
# And the same two answers out of the pure function, on the raised arm's OWN
# two watt figures - so the flip is a property of `delinquent()`'s threshold
# and not of anything the controller did differently between the two runs.
_pair = [U("inverter-a", float(_c_up.inverters[0].standing_w()),
_c_up.inverters[0].batt),
U("inverter-b", float(_b_up.standing_w()), _b_up.batt)]
check("...and `delinquent()` says the same thing about that one pair of watt "
"figures: named at the shipped number, silent at the raised one. The "
"threshold is the only input that moved",
list(delinquent(_pair, _SAT_SHIPPED)) == ["inverter-b"]
and delinquent(_pair, _SAT_RAISED) == {})
print()
print(" (b) THE PERCENT - each unit's OWN `bms_soc_floor`, at two cut-outs")
print(" that BRACKET the shipped example from both sides.")
# ⚠️ WHY TWO ARMS AND NOT ONE. A single arm at a deeper cut-out kills `10.0`
# but not `50.0`; a single arm at a shallower one kills `10.0` but not `0.0`.
# One arm on each side of the shipped example leaves no constant at all: the
# deep arm must stay SILENT (its pack is genuinely below its own cut-out) and
# the shallow arm must SPEAK (its pack is genuinely above its own cut-out), and
# a literal that satisfies the first cannot satisfy the second.
_FLOOR_DEEP, _DEEP_SOC = 25, 15.0 # pack BELOW its own cut-out: cold
_FLOOR_SHALLOW, _SHAL_SOC = 5, 8.0 # pack ABOVE its own cut-out: wedged
_floor_lo, _floor_hi = (int(x) for x in re.match(
r"int\((\d+),(\d+)\)", read_option("bms_soc_floor", "schema")).groups())
async def _at_floor(b_floor, b_soc, peak):
hass = _DeliveryHass(b_soc=b_soc)
# ⚠️ `peak_soc_floor` IS RAISED WITH THE DEEP ARM SO THE FIXTURE STAYS A
# CONFIG THE ADD-ON WOULD START ON: validate.py rule 8 refuses a
# `peak_soc_floor` at or below ANY unit's `bms_soc_floor`. A fixture that
# only a refused config can produce would grade a state no install reaches.
c = _ctrl(hass, floors=(_FLOOR_PCT, b_floor), peak_soc_floor=peak)
return c, await _drive(c, watch=_sample)
_c_deep, _r_deep = asyncio.run(_at_floor(_FLOOR_DEEP, _DEEP_SOC, 30))
_c_shal, _r_shal = asyncio.run(_at_floor(_FLOOR_SHALLOW, _SHAL_SOC, 20))
_bd, _bs = _c_deep.inverters[1], _c_shal.inverters[1]
check(f"both cut-outs are configs an operator can enter - inside the shipped "
f"schema {read_option('bms_soc_floor', 'schema')} - and NEITHER is the "
f"{read_option('bms_soc_floor')} % this file and config.yaml's own "
f"example both state, which is the number a literal would have agreed "
f"with everywhere",
_floor_lo <= _FLOOR_SHALLOW < int(read_option("bms_soc_floor"))
< _FLOOR_DEEP <= _floor_hi)
check("...and each arm is a config validate.py rule 8 accepts: its "
"peak_soc_floor is above EVERY unit's own cut-out",
all(float(c.o.get("peak_soc_floor")) > float(i.bms_soc_floor)
for c in (_c_deep, _c_shal) for i in c.inverters))
check("...and the two arms are IDENTICAL IN THE POWER DOMAIN on every cycle - "
"grid, site target, every share, every measured watt. The configured "
"cut-out is the only thing that differs, which is part 4's claim made "
"again at two cut-outs that are not the shipped one",
_power_domain(_r_deep) == _power_domain(_r_shal))
check(f"a pack at {_DEEP_SOC:.0f} % against its OWN {_FLOOR_DEEP} % cut-out is "
f"cold, and is NOT named - even though it is far above the "
f"{read_option('bms_soc_floor')} % a literal would have graded it "
f"against, which would have called an empty pack a fault",
_r_deep[-1]["named"] == () and float(_bd.bms_soc_floor) == _FLOOR_DEEP
and _bd.soc == _DEEP_SOC)
check("...and it REACHED that decision: it was asked for a discharge share, "
"was short of it by more than saturation_w, and was turned away by "
"`account()` alone",
float(_bd.standing_w()) > 0
and short_by(_bd.standing_w(), _bd.batt) > _c_deep.tuning.saturation_w
and account(live=True, parked=False, soc=_bd.soc,
bms_floor=_bd.bms_soc_floor) is not None)
check(f"a pack at {_SHAL_SOC:.0f} % against its OWN {_FLOOR_SHALLOW} % cut-out "
f"is WEDGED, and IS named - even though it is below the "
f"{read_option('bms_soc_floor')} % a literal would have explained it "
f"away with, which would have lost the fault this card exists to find. "
f"THIS ROW AND THE ONE ABOVE ARE THE PIN: no constant answers both",
_r_shal[-1]["named"] == ("inverter-b",)
and float(_bs.bms_soc_floor) == _FLOOR_SHALLOW and _bs.soc == _SHAL_SOC)
print(f" deep cut-out {_FLOOR_DEEP} %, SoC {_DEEP_SOC:.0f} % -> named "
f"{_r_deep[-1]['named']}")
print(f" shallow cut-out {_FLOOR_SHALLOW} %, SoC {_SHAL_SOC:.0f} % -> "
f"named {_r_shal[-1]['named']}")
# ===========================================================================
print()
print("=" * 72)