diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index e4068ed..a9b8084 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -39,8 +39,22 @@ numbers.** an allocator** - `ALLOC-01` builds it. With at most one unit it passes the target through (§6.2's N=1 identity); with two or more **it declines, no shares, `allocated=False`**, because the obvious placeholder - hand the target - to each - IS the fault `ARCH-01` was filed about, and `main.py` refuses to - write rather than broadcast. + to each - IS the fault `ARCH-01` was filed about. **What `main.py` then does + is SUBSTITUTE, not abstain**: it replaces the resolution with a 0 W `failsafe`, + logs "refusing to broadcast the site target", and writes that 0 W through + `write_setpoint()` like any other cycle. The refusal is to BROADCAST; the + write still happens, in the fail-toward-inaction direction. Graded end to end + on a real two-inverter cycle by new **part 10h** - the branch is unreachable + on the shipped roster, and replacing it with `if False:` had left all 17 + suites green. +* **`resolve()` HONOURS the roster it is handed, and that is now graded too.** + Left `None` the roster is derived from the claims, which is the documented + default; passed explicitly it is used. Deriving it *always* - ignoring the + argument - makes a unit that filed nothing this cycle cease to exist, so a + site of two reads as a site of one and the seam passes the whole grid target + through to the inverter that spoke: `ARCH-01`'s fault with its own evidence + removed. `test_arbiter.py` part 7 grades it on the one case that separates the + two, a **silent unit**. * **The contradiction is no longer guarded against, it CANNOT BE WRITTEN DOWN.** The partition is by `actuator` and it is a partition: a unit's `hi` and the site's `lo` are never both terms of one `lo > hi` test. Constructed and swept - @@ -68,12 +82,34 @@ numbers.** reading it, which is why `actuator` is the LAST field of `Claim` and not the first. Its spy gained `**kw` so the roster passes through unread rather than being silently dropped. New **part 10g** pins `CLAIM_SCOPE` to the same - eight-source set, so a ninth source now goes red twice; and it grades a **real - two-inverter cycle**, which is the only place a mis-typed `actuator_of(...)` - at a call site is observable - at N=1 every source legitimately routes to the - site, so a swap there is invisible by construction. Verified by breaking it: - building SAFETY-25's Option B as a ninth source still takes 10f red on the + eight-source set, so a ninth source now goes red twice. Verified by breaking + it: building SAFETY-25's Option B as a ninth source still takes 10f red on the line it is written. +* **And the CALL SITES are pinned statically, at all eight of them.** 10g's + first cut graded routing only through a **real two-inverter cycle** and called + that "the line a mis-typed `actuator_of(...)` at a call site goes red on". + **It was not.** That cycle reaches `device`, `supervised` and `loop` and + nothing else - three of the eight direct appends - so a mis-type at the other + five survived every suite in this repository. Proved on the shipped tree by PR + #40's reviewer: `actuator_of("maintenance")` -> `actuator_of("peak")` at the + maintenance-shaping append left `test_arbiter.py`, `test_peakguard.py` and + `test_arbitrage.py` all green while, at a roster of two, it built a site loop + of -1500 W against the guard's site `lo` of 1500 W and the now-misrouted + charge-only `hi = 0` - `lo > hi`, arbiter clause 3, **0 W in the billed + quarter**, SAFETY-25 §0a's fatal case from a one-word typo. That is this + repository's named failure mode - **a check observing something other than + what it claimed** - reproduced inside the check written to prevent it, so it + is closed the way 10f closes it: over the **same AST scan**. All eight direct + appends expose their actuator as a literal `actuator=self.actuator_of('...')`, + so the literal is required to EQUAL the source the claim is filed under - all + eight, on the line the typo is written, in every configuration and at every + roster, whether or not any fixture reaches the state that files them. + **Measured: each of the eight mutated to a wrong actuator in turn, each one + red.** The two INDIRECT appends (`peak_claim`, `arb_claim`) carry no + `actuator=` at the call site to mis-type, and are pinned where their routing + is actually chosen - `peakguard.claim()` by the `SWEEP_SITE` sweep, + `arbitrage.claim()` by `test_arbitrage.py`. 10g's dynamic cycle is kept and + now says which three of the eight it sees instead of claiming the class. **SAFETY-25's open question is ANSWERED: the BMS cut-out is the DELIBERATE and only backstop for demand-driven discharge, and a loop-side reserve floor is diff --git a/goodwe_controller/app/arbiter.py b/goodwe_controller/app/arbiter.py index 31fe144..6cd826a 100644 --- a/goodwe_controller/app/arbiter.py +++ b/goodwe_controller/app/arbiter.py @@ -433,6 +433,13 @@ def resolve(claims: list, *, units=None) -> Resolution: one that does not, so ALLOC-01 will pass the real roster rather than let a silent unit look like a smaller site. + ⚠️ AND THE ARGUMENT HAS TO BE HONOURED, WHICH IS NOT THE SAME AS EXISTING. + Ignore it - derive always - and a unit that filed nothing this cycle ceases + to exist: a site of two reads as a site of one and stage 3 passes the whole + grid target through to the inverter that spoke, which is ARCH-01's fault + with its own evidence removed. That mutation survived the whole suite until + test_arbiter.py part 7 graded it on a SILENT unit. + ⚠️ THE PARTITION IS BY `actuator`, AND IT IS A PARTITION, NOT A FILTER. Every claim lands in exactly one stage. That is the whole structural argument: a unit's `hi` and the site's `lo` are never both terms of the same `lo > hi` diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 8739953..06ab844 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -845,14 +845,27 @@ class Controller: self.loop_clamped = (False, "") resolution = resolve(claims, units=self.units) - # ⚠️ N>=2 WITHOUT AN ALLOCATOR IS A REFUSAL, NOT A BROADCAST, and this - # branch is unreachable while the roster holds one unit. It is here + # ⚠️ N>=2 WITHOUT AN ALLOCATOR IS A REFUSAL TO BROADCAST, NOT A REFUSAL + # TO WRITE, and the difference is the next four lines: the resolution is + # REPLACED by a 0 W failsafe and then written like any other cycle at + # `write_setpoint(self.target)` below. Nothing is skipped. 0 W is the + # fail-toward-inaction direction - the setpoint is commanded to zero + # rather than left wherever the last cycle put it. + # + # This branch is unreachable while the roster holds one unit. It is here # because the alternative default is the fault ARCH-01 was filed about: # hand the site target to each inverter and each supplies it IN FULL, so # the loop gain at the meter becomes N times the per-unit gain - a # permanent +-2 kW limit cycle at the gain config.yaml documents as its # maximum, with every watchdog in §11.2 reporting healthy. `allocate()` # declines rather than invent a split; this is what declining costs. + # + # ⚠️ AND BEING UNREACHABLE IS NOT A REASON TO LEAVE IT UNGRADED. It was: + # PR #40's reviewer replaced this `if` with `if False:` and the whole + # 17-file suite stayed green, so the branch was one refactor away from + # being deleted as dead code. test_peakguard.py part 10h now drives a + # real two-inverter cycle and grades all four halves - the 0 W, the + # `failsafe` winner, the log line, and the write that follows it. if not resolution.allocated: self.log_event(f"ARBITER: no allocator for {len(self.units)} " f"inverters - refusing to broadcast the site target") diff --git a/goodwe_controller/test_arbiter.py b/goodwe_controller/test_arbiter.py index d9d1810..6cd426b 100644 --- a/goodwe_controller/test_arbiter.py +++ b/goodwe_controller/test_arbiter.py @@ -350,6 +350,44 @@ check("with two it DECLINES rather than inventing a split. ⚠️ THE OBVIOUS " _shares == {} and _ok is False) check("...and specifically it does not broadcast", 1500.0 not in _shares.values()) +# ⚠️ AND `resolve()` HAS TO HONOUR THE ROSTER IT IS GIVEN, WHICH IS NOT THE SAME +# THING AS HAVING A `units` PARAMETER. Left None the roster is DERIVED from the +# claims, and that derivation is right only for a caller that files a claim for +# every unit every cycle. Ignore the argument and derive it always - `if units +# is None:` -> `if True:` - and a unit that filed nothing this cycle simply +# CEASES TO EXIST: the site of two looks like a site of one, the seam passes the +# whole grid target through to the single inverter that spoke, and ARCH-01's +# fault is built with the evidence for it removed. That mutation left this whole +# suite green until this check was written (PR #40 review), so it is graded on +# the one observation that distinguishes the two - a SILENT unit. +_silent = resolve([Claim.set("loop", P_LOOP, 900.0), + Claim.limit("device", P_SAFETY, -2000, 2000, "rating", + actuator="inverter-a")], + units=TWO) +print(f" roster of two, only inverter-a files: bounds=" + f"{sorted(_silent.unit_bounds)} allocated={_silent.allocated} " + f"shares={_silent.shares}") +check("resolve() HONOURS the roster it is handed rather than inferring one from " + "who happened to speak. inverter-b filed nothing this cycle and is still " + "a unit: it gets its own interval, and the site is still a site of TWO, " + "so the seam still DECLINES", + sorted(_silent.unit_bounds) == ["inverter-a", "inverter-b"] + and _silent.allocated is False and _silent.shares == {}) +check("...and that is the whole cost of ignoring the argument, stated as the " + "number it would produce: derive the roster from the claims instead and " + "the site is a site of ONE, so the seam passes 900 W through to " + "inverter-a - the silent inverter's absence read as a smaller site", + resolve([Claim.set("loop", P_LOOP, 900.0), + Claim.limit("device", P_SAFETY, -2000, 2000, "rating", + actuator="inverter-a")]).shares + == {"inverter-a": 900.0}) +check("...and left None the roster IS derived from the claims, which is the " + "documented default every N=1 replay above depends on - so the parameter " + "changes the answer only where the two disagree", + resolve([Claim.set("loop", P_LOOP, 900.0), + Claim.limit("device", P_SAFETY, -2000, 2000, "rating", + actuator="inverter-a")], units=None).allocated is True) + print() print("ARB-03 8. verified by breaking it: a claim routed to the wrong stage (AC 6)") # The `peak` floor, misfiled on a unit. It is the site's floor - if it lands on diff --git a/goodwe_controller/test_peakguard.py b/goodwe_controller/test_peakguard.py index cf250f8..5c13c4a 100644 --- a/goodwe_controller/test_peakguard.py +++ b/goodwe_controller/test_peakguard.py @@ -2563,27 +2563,169 @@ check("...and the roster it was restored to is the shipped one, so nothing " # IT RIGHT. `actuator_of("maintenance")` written at the `device` append routes a # rating limit to whatever `maintenance` routes to, and at the shipped # one-inverter roster BOTH answers are the site, so nothing above can see it. -# The only place it becomes observable is a cycle at a roster of two - so take -# one, on real claims, spied at main.py's own resolve. +# ⚠️ AND THE DYNAMIC CYCLE BELOW SEES ONLY THREE OF THE EIGHT DIRECT APPENDS. +# That was THIS PART committing this repository's own named failure - a check +# observing something other than what it claimed - inside the check written to +# prevent it. The first cut of 10g called the two-inverter cycle "the line a +# mis-typed `actuator_of(...)` at a call site goes red on", full stop. IT IS +# NOT. That cycle is auto, P1-healthy, no maintenance procedure, no tripped sign +# watchdog, the guard idle and arbitrage off, so it reaches `device`, +# `supervised` and `loop` AND NOTHING ELSE. PR #40's reviewer mutated the other +# five appends one at a time and every suite in this repository stayed GREEN, +# this part included. At the maintenance-shaping append, +# `actuator_of("maintenance")` -> `actuator_of("peak")` then produced, at a +# roster of two, a site loop of -1500 W against the guard's site `lo` of 1500 W +# and the now-misrouted charge-only `hi = 0`: `lo > hi`, arbiter clause 3, 0 W +# IN THE BILLED QUARTER - SAFETY-25 §0a's fatal case, built by a one-word typo +# under a green suite. The same mutation at the `failsafe` append survived too. +# +# So THE CALL SITES ARE PINNED STATICALLY, over 10f's own AST scan of +# `Controller.cycle()` and for 10f's reason: a static pin sees every append on +# the line it is written, in every configuration, whether or not any fixture +# here reaches the state that files it. All eight direct appends expose their +# actuator as a LITERAL - `actuator=self.actuator_of('')` - so the +# literal is simply required to EQUAL the source the claim is filed under. That +# is what makes "a mis-typed `actuator_of(...)` goes red" a true sentence about +# all eight rather than about the three a fixture happens to reach. + + +def append_actuator(node): + """The actuator expression a DIRECT append routes its claim through. + + ⚠️ RETURNED AS SOURCE TEXT, NOT EVALUATED, and that is the whole point. + `self.actuator_of('device')` and `self.actuator_of('maintenance')` are the + SAME VALUE at the shipped roster and two different routings; only the text + written at the call site tells them apart, which is exactly why every + dynamic check in this file is blind to the swap at N=1. + """ + kw = [k for k in node.keywords if k.arg == "actuator"] + if len(kw) != 1: + return None + return ast.unparse(kw[0].value) + + +# The appends that BUILD their claim in place - the eight whose routing is +# chosen by hand, and the only ones where a typo is possible - against the two +# that append a claim built elsewhere in the method and carry no `actuator=` +# here at all. +_direct = [n.args[0] for n in _appends if isinstance(n.args[0], ast.Call)] +_indirect = [n.args[0].id for n in _appends if isinstance(n.args[0], ast.Name)] +_routing = sorted(((c.args[0].value, append_actuator(c), c.lineno) + for c in _direct), key=lambda r: r[2]) +_misrouted = [r for r in _routing if r[1] != f"self.actuator_of({r[0]!r})"] +print(f" {len(_direct)} direct appends, each read at its own line: " + + ", ".join(f"main.py:{ln} {s} -> {a}" for s, a, ln in _routing)) +check("all eight of `Controller.cycle()`'s DIRECT appends route through " + "`self.actuator_of()`, so this scan can read the routing " + "of EVERY one of them. An append that computed its actuator, or omitted " + "the argument, or hand-wrote a string, fails HERE rather than being " + "silently skipped - which is how a scan like this goes quietly vacuous, " + "and 10f is in this file because that already happened once", + len(_direct) == 8 + and all(a is not None and a.startswith("self.actuator_of('") + for _s, a, _ln in _routing)) +check("⚠️ AND AT EVERY ONE OF THE EIGHT THE LITERAL IT ROUTES THROUGH IS THE " + "SOURCE THE CLAIM IS FILED UNDER. THIS is the line a mis-typed " + "`actuator_of(...)` at a call site goes red on - all eight of them, on " + "the line the typo is written, at every roster, INCLUDING the five " + "appends no fixture in this repository reaches. " + "`actuator_of(\"peak\")` at the maintenance append is a different string " + "here even though it is the same VALUE at the shipped roster of one", + _misrouted == []) +check("...and the only appends this pin does NOT reach are the two that file a " + "claim BUILT ELSEWHERE - `peak_claim` and `arb_claim` - which have no " + "`actuator=` at the call site to mis-type. Their routing is pinned where " + "it is CHOSEN instead: peakguard.claim() by the SWEEP_SITE sweep above, " + "over every load x elapsed x battery x SoC x floor combination, and " + "arbitrage.claim() by test_arbitrage.py", + sorted(_indirect) == sorted(CLAIM_BUILDERS) + and len(_direct) + len(_indirect) == len(_appends)) +check("...and between them the ten appends account for exactly the eight " + "sources 10f pins, so the routing pin and the source set are one scan " + "and neither can drift from the other", + {s for s, _a, _ln in _routing} | set(CLAIM_BUILDERS.values()) + == CLAIM_SOURCES) + +# THE DYNAMIC HALF, kept, and now stated for what it actually observes: a real +# cycle proving the routing reaches the arbiter THROUGH main.py rather than only +# through this file's reading of main.py. It grades three of the eight appends. +# The static pin above is what covers the other five. c_src.units = ("inverter-a", "inverter-b") +_n2_writes_before = len(c_src.hass.writes) try: _, _cl2 = cycle_claims(lambda: run_cycle(c_src)) + _n2_target, _n2_reason = c_src.target, c_src.reason + _n2_events = list(c_src.events) + _n2_writes = c_src.hass.writes[_n2_writes_before:] + _n2_last = c_src.last_write finally: c_src.units = (app_main.UNIT_A,) print(" a REAL two-inverter cycle files: " + ", ".join(f"{c.source}@{c.actuator}" for c in _cl2)) check("a real cycle at a roster of TWO partitions its own claims correctly - " - "every claim main.py filed is on an actuator its source's scope permits. " - "This is the line a mis-typed `actuator_of(...)` at a call site goes red " - "on, and it is the only place one can be seen: at N=1 every source " - "legitimately routes to the site, so a swap there is invisible by " - "construction", len(_cl2) >= 3 and scope_violations(_cl2) == []) + "every claim main.py filed is on an actuator its source's scope permits, " + "observed on the list main.py's own `resolve` was handed. ⚠️ AND THIS " + "SEES THREE OF THE EIGHT APPENDS, NOT THE CLASS: this state files " + "`device`, `supervised` and `loop`, so it is the static pin above and not " + "this line that covers `failsafe`, `safety` and the two `maintenance` " + "appends", len(_cl2) >= 3 and scope_violations(_cl2) == []) check("...and the partition is the expected one on the sources this state " "reaches: the two rating limits bind the unit, the loop claim stays on " "the grid connection", {c.source: c.actuator for c in _cl2} == {"device": "inverter-a", "supervised": "inverter-a", "loop": ACTUATOR_SITE}) +check("...and naming those three is not an accident of this fixture but the " + "reason the static pin exists - five of the eight sources need a state " + "no check in this file reaches", + {c.source for c in _cl2} + == CLAIM_SOURCES - {"failsafe", "safety", "maintenance", "peak", + "arbitrage"}) + + +# --- 10h. ARB-03: AND WHAT MAIN.PY DOES WITH `allocated=False` -------------- +# ⚠️ AN UNREACHABLE BRANCH THAT NOTHING GRADED. The roster ships as one unit, so +# `allocate()` never declines in production and no fixture went looking: PR +# #40's reviewer replaced main.py's `if not resolution.allocated:` with +# `if False:` and the whole 17-file suite stayed GREEN. The branch is the right +# instinct - the alternative default is the fault ARCH-01 was filed about, N +# inverters each supplying the site target IN FULL, a permanent limit cycle at +# the meter with every §11.2 watchdog reporting healthy - and an ungraded right +# instinct is one refactor away from being deleted as dead code. The +# two-inverter cycle above IS that state, so it is graded on that cycle rather +# than with a fixture of its own. +# +# ⚠️ AND WHAT IT DOES IS SUBSTITUTE, NOT ABSTAIN. main.py does not skip the +# write. It replaces the resolution with a 0 W `failsafe` and then calls +# `write_setpoint(0.0)` like any other cycle. 0 W is the correct +# fail-toward-inaction direction and the log line is accurate about what it +# refuses ("refusing to broadcast the site target"), but "main.py refuses to +# write" would be the same overclaim this part just took out of 10g, so it is +# graded as what it is: a substitution followed by a write. +print(f" at N=2 main.py resolved to {_n2_target:+.0f} W, reason " + f"{_n2_reason!r}, wrote {_n2_writes}") +print(" and logged: " + + next((e for e in _n2_events if "ARBITER" in e), "(nothing)")) +check("with two inverters and no allocator main.py does NOT hand the site " + "target to the roster. The resolution it acts on is the 0 W failsafe it " + "substitutes, and `explain()` says so - this is the branch a `if False:` " + "deleted with the suite still green", + _n2_target == 0.0 and "failsafe" in _n2_reason) +check("...and it says WHY, in the action log, in the operator's own words - " + "the refusal is to BROADCAST, and it names the roster size that caused it", + any("refusing to broadcast the site target" in e and "2 inverters" in e + for e in _n2_events)) +check("...and then it WRITES that 0 W, on the same line as every other cycle. " + "It is a substitution, not an abstention: the setpoint is commanded to " + "zero rather than left wherever the last cycle put it, which is the " + "fail-toward-inaction direction and the opposite of leaving a stale " + "command standing", _n2_writes == [0.0] and _n2_last == 0.0) +check("...and the 0 W is NOT what the site target happened to be - resolve the " + "same claims at a roster of ONE, where the seam passes through, and a " + "real number comes back. That is what says the branch does work rather " + "than agreeing with the resolution by luck", + resolve(_cl2, units=("inverter-a",)).allocated is True + and resolve(_cl2, units=("inverter-a",)).target_w != 0.0) # ==============================================================================