SAFETY-85 review: grade discharge only, over the plant's settle time

Rule 2 graded each battery against last cycle's command and graded charges
too. On plantsim's battery (10 s lag) at the 2 s meter it warned at every
drain or charge start and 15 times an hour under the peak guard at N=2; a
charge tapering near full warned 11 times in 50 min (SAFETY-42 section 7).
Now a discharge is graded against the least it stood at over SHORT_CYCLES /
SHORT_SPAN_S. DOCS lists `mode-bound` under Loop reason. test_safety85 part 7:
peak guard with a dead battery, lagging plant, taper; part 3: the bound ends
with maintenance.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

goodwe_controller/DECISIONS/SAFETY-85-no-false-not-keeping-up.md

@@ -25,14 +25,16 @@ counter counted cycles (861), not episodes.
    the loop. Its number is on the wire, or only a rating (`device`,
    `supervised`) cut it. No bound held it, and nothing was unmet.
    (`Controller._loop_alarm_input`.)
-2. **A battery another owner set alarms when it gives less than it was told.**
-   This covers every battery on a cycle the loop did not decide, and a battery
-   with its own pin otherwise. The test is more than `saturation_w` short in
-   the told direction, for `saturation_cycles` cycles in a row, against what
-   stands on the wire (`clamp.delivery_short`). It covers a real shortfall
-   during maintenance. Before, that shortfall was silent unless the kick
-   happened to reach the rail (house 1000 W, drain 2000 W, battery gives
-   nothing: 0 warnings before, 4 in 10 min now).
+2. **A battery another owner set to discharge alarms when it gives less than
+   it was told.** This covers every battery on a cycle the loop did not
+   decide, and a battery with its own pin otherwise. The test is more than
+   `saturation_w` short of the least it stood at over `SHORT_CYCLES` cycles
+   and `SHORT_SPAN_S` (the plant's measured settle, SAFETY-13/73), for
+   `saturation_cycles` cycles in a row (`clamp.delivery_short`). It covers a
+   real shortfall during maintenance or under the peak guard. Before, that
+   shortfall was silent unless the kick happened to reach the rail (house
+   1000 W, drain 2000 W, battery gives nothing: 0 warnings before, 4 in 10 min
+   now).
 3. **At N>=2, while maintenance holds a battery, the loop is bounded** by
    OPT-11's `loop_bound()` (what the batteries may place). It does not wind,
    and control comes back one slew step a cycle. Why maintenance only: that is
@@ -69,12 +71,21 @@ batteries can give. The `alarmed()` helpers in test_safety84/86 also read
 SAFETY-81's quarter stamp, because ALLOCATOR rows can push its row out of
 the 40-row log.
 
+**Review (PR #163).** The first draft graded each battery against last
+cycle's command and graded charges too. On plantsim's battery (10 s lag) at
+the live 2 s meter, every drain or charge start warned once, and an N=2 drain
+or HOLD under the peak guard with a stepping house warned 15 times an hour. A
+maintenance charge tapering near full warned 11 times in 50 min, which
+SAFETY-42 section 7 had ruled out. With the settle window and discharge only:
+0 in all of those cases. A battery that gives nothing still warns, lagging
+or not. At a hypothetical 20 s lag, it warns at most twice an hour. The cost:
+a battery that does not take a charge another owner set is not named by this
+alarm (the same gap SAFETY-42 states; the charge has its 36 h timeout).
+`test_safety85.py` part 7.
+
 **Not changed, open for the owner.**
 - With the loop in charge and the house above the batteries' total rating,
   it still warns (SAFETY-40). On the rig's 2 x 2000 W N=2, most of SAFETY-84's
   count came from houses of 4500 and 6500 W.
-- A battery that tapers near full during a charge, more than `saturation_w`
-  under its command for 15 s, now warns (rule 2). The text already says
-  "check it is not empty or full".
-- TEL-10's `loop_reason` reads `mode-bound` while the maintenance bound holds
-  the loop, with modes off.
+- TEL-10's `Loop reason` reads `mode-bound` while the maintenance bound holds
+  the loop, with modes off. DOCS lists the value with both meanings.

goodwe_controller/DOCS.md

@@ -577,7 +577,7 @@ accumulator is doing:
 | `Loop integrator lead` | W | that value minus **the command it was computed against**, i.e. the command the previous cycle left standing |
 | `Loop reason` | — | the law's own word for this cycle |
 
-`Loop reason` takes one of ten values:
+`Loop reason` takes one of eleven values:
 
 | value | what happened |
 |---|---|
@@ -588,6 +588,7 @@ accumulator is doing:
 | `slew-limited` | the command could not move as far as the integrator asked for; `slew_w` is what stopped it |
 | `saturated-freeze` | the inverter is not following, so the command's magnitude may fall but not rise |
 | `freeze-released` | the freeze had run out of plant-shaped explanations and the command was let move again |
+| `mode-bound` | the batteries' limits set the command, not the meter: their battery modes, or, with two or more inverters, a battery in its monthly maintenance |
 | `not-running` | control is stopped **and** shadow mode is off, so the loop did not run |
 | `battery refuses` | the battery gave less than 100 W for 12 cycles (a minute at a 5-second meter) while asked for more than 500 W, so the add-on asks 0 W in that direction (see below) |
 | `battery probe` | the add-on asks 200 W to test whether a refusing battery gives power again |
@@ -1954,10 +1955,12 @@ notification budget.
 Why: [`DECISIONS/SAFETY-40-the-integrator-bound-is-not-a-bug.md`](DECISIONS/SAFETY-40-the-integrator-bound-is-not-a-bug.md).
 
 The integrator bound counts only on a cycle the loop decided. While
-maintenance, the peak guard or a safety limit sets a battery, the same alarm
-fires only when that battery gives more than `saturation_w` less than it was
-told, for `saturation_cycles` cycles. At two or more inverters the loop does not
-wind while maintenance holds a battery.
+maintenance, the peak guard or a safety limit sets a battery to discharge, the
+same alarm fires only when that battery gives more than `saturation_w` less
+than the least it was told over the plant's settle time (at least 23 s), for
+`saturation_cycles` cycles. A charge is not graded: a full battery takes less
+as it tops up. At two or more inverters the loop does not wind while
+maintenance holds a battery.
 Why: [`DECISIONS/SAFETY-85-no-false-not-keeping-up.md`](DECISIONS/SAFETY-85-no-false-not-keeping-up.md).
 
 ### The frozen-meter alarm
@@ -3130,8 +3133,9 @@ short time, for example just after a large load switches on or the sun comes
 out. It is not a fault, and control keeps running. If it keeps coming back,
 check that the battery is switched on, and that it is not empty (when it should
 give power) or full (when it should charge). During the monthly maintenance or
-while the peak limit is guarded, it only shows when the battery gives less than
-it was told. The status page counts how many times it happened.
+while the peak limit is guarded, it only shows when the battery gives
+clearly less than it was told. The status page counts how many times it
+happened.
 
 ### Power request too high
 
@@ -3237,8 +3241,8 @@ link cannot jump to the section.
 
 **Battery is not keeping up.** The grid-following loop's integrator reached its
 anti-windup bound (`integrator_max_w`, SAFETY-40), or a battery that another
-owner set gave less than it was told (SAFETY-85). No command went over the
-rating. The log line is a WARNING with reason code `OUTPUT_CLAMP`. See *The
+owner set to discharge gave less than it was told (SAFETY-85). No command went
+over the rating. The log line is a WARNING with reason code `OUTPUT_CLAMP`. See *The
 winding-loop alarm*.
 
 **Power request too high.** A computed command above a limit was cut before it

goodwe_controller/app/clamp.py

@@ -270,12 +270,10 @@ def shortfall_note(named) -> str:
 
 
 def delivery_short(asked_w, given_w, margin_w) -> bool:
-    """SAFETY-85: did a battery give more than `margin_w` less than asked, in
-    the asked direction? Giving more than asked, or 0 W asked, is not short."""
+    """SAFETY-85: did a battery asked to discharge give more than `margin_w`
+    less? A charge is never short: a full pack tapers (SAFETY-42 section 7)."""
     asked, given = float(asked_w), float(given_w)
-    if asked == 0.0:
-        return False
-    return (asked - given) * (1.0 if asked > 0 else -1.0) > abs(float(margin_w))
+    return asked > 0.0 and asked - given > abs(float(margin_w))
 
 
 def write_clamp_note(named) -> str:

goodwe_controller/app/main.py

@@ -281,6 +281,11 @@ def pack_energy_of(inv):
 # ordinary load steps. At 2 s and slower this changes nothing.
 REFUSED_SPAN_S = (_ctl.SETTLE_CYCLES - 1) * _ctl.LOOP_TICK_S
 
+# SAFETY-85 review: a battery another owner set is graded against the least it
+# was told over the same span as above (the plant's measured settle time), so
+# a healthy battery still settling on a new command is not short.
+SHORT_CYCLES, SHORT_SPAN_S = _ctl.SETTLE_CYCLES, REFUSED_SPAN_S
+
 
 def _refused_interval(sign: int) -> tuple:
     """SAFETY-73: the (lo, hi) a refusing unit may be asked: 0 W that way."""
@@ -1180,9 +1185,11 @@ class Controller:
         self.loop_clamped: tuple[bool, str, bool] = (False, "", False)
         # UX-08: the numbers of a loop-side over-rating, for the notification.
         self.loop_clamp_facts: tuple = ()
-        # SAFETY-85: this cycle's loop Decision, and how many cycles in a row
-        # a battery set by another owner gave less than asked.
+        # SAFETY-85: this cycle's loop Decision, each battery's recent
+        # standing commands, and how many cycles in a row a battery set by
+        # another owner gave less than asked.
         self._loop_decision = None
+        self._asked_hist: dict = {}
         self._short_run = 0
         self.stopping = False
         # SAFETY-61. time.monotonic() at the end of the last run_control() pass.
@@ -5084,25 +5091,36 @@ class Controller:
 
     def _loop_alarm_input(self, resolution) -> None:
         """SAFETY-85. The loop's clamp counts only on a cycle the loop decided
-        (a rating may cut it). A battery another owner set counts when it gave
-        less than it was told for `saturation_cycles` cycles in a row.
-        DECISIONS/SAFETY-85-no-false-not-keeping-up.md."""
+        (a rating may cut it). A battery another owner set to discharge counts
+        when it gave less than it was told for `saturation_cycles` cycles in a
+        row. DECISIONS/SAFETY-85-no-false-not-keeping-up.md."""
         d = self._loop_decision
         if d is None:
             self._short_run = 0
+            self._asked_hist = {}
             return
         unmet = self.allocation.unmet_w if self.allocation is not None else 0
         cut = (resolution.bound_by or "").split("(")[0]
         owns = (resolution.winner == "loop" and not d.held and not unmet
                 and (resolution.target_w == d.target_w
                      or cut in ("device", "supervised")))
-        # Graded against what stands on the wire (0 W in shadow mode).
+        # Graded against the least that stood on the wire (0 W in shadow
+        # mode) over SHORT_CYCLES cycles and SHORT_SPAN_S seconds.
+        now, asked = self._now_mono, {}
+        for inv in self.inverters:
+            h = self._asked_hist.setdefault(inv.name, [])
+            h.append((now, inv.standing_w()))
+            while len(h) > SHORT_CYCLES and (
+                    now is None or h[0][0] is None
+                    or now - h[0][0] > SHORT_SPAN_S):
+                h.pop(0)
+            asked[inv.name] = min(w for _t, w in h)
         others = [inv for inv in self.inverters if not owns
                   or allocator.pin_w(resolution.unit_bounds.get(inv.name))
                   is not None]
-        short = [(inv, inv.standing_w(), inv.batt) for inv in others
+        short = [(inv, asked[inv.name], inv.batt) for inv in others
                  if inv.batt is not None
-                 and delivery_short(inv.standing_w(), inv.batt,
+                 and delivery_short(asked[inv.name], inv.batt,
                                     self.tuning.saturation_w)]
         self._short_run = self._short_run + 1 if short else 0
         if owns and self.loop_clamped[0]:

goodwe_controller/test_safety85.py

@@ -19,6 +19,8 @@ while maintenance holds a battery. Drives the REAL Controller
    (a battery at its cut-out) does not warn.
 5. The status row counts episodes, not cycles.
 6. The rules at their boundaries.
+7. Review: a battery the peak guard sets that gives nothing warns; a battery
+   that lags (10 s) and a charge that tapers do not.
 
 Mutations (MUTATION.md, purged copies) are listed in the pull request.
 """
@@ -82,13 +84,15 @@ def store(n, phase):
 
 def run(n, phase, house, cap, *, dur=HOUR, soc=None, cut=None, extra=None,
         maint=True):
-    """Rows and controller; `cap` is the peak limit (20000: never binds)."""
+    """Rows and controller; `cap` is the peak limit (20000: never binds);
+    `house` is watts or a function of time."""
     s84.OPT = dict(OPT0, peak_cap_w=cap, maintenance_enabled=maint)
     if soc is None:
         def soc(u, t):
             return 99.0 if phase == "hold" and u == "a" else 60.0
     try:
-        return s84.run(n, load=lambda t: house, cheap=(phase == "charge"),
+        load = house if callable(house) else (lambda t: house)
+        return s84.run(n, load=load, cheap=(phase == "charge"),
                        store=store(n, phase), soc=soc, dur=dur, cut=cut,
                        extra=dict({"maintenance_hold_min": 600,
                                    "maintenance_discharge_w": 2000},
@@ -209,7 +213,20 @@ check("a dip of two cycles is not an alarm", warned(_dip[1]) == (0, 0, 0))
 # ============================================================================ #
 print("\n3. control coming back from maintenance does not jump")
 END = {"maintenance_hold_min": 10}
-_h2 = run(2, "hold", 4500.0, 20000, dur=900.0, extra=END)
+_bounds = []
+_real_compute = app_main.compute
+
+
+def _spy_compute(*a, **kw):
+    _bounds.append(kw.get("bound"))
+    return _real_compute(*a, **kw)
+
+
+app_main.compute = _spy_compute
+try:
+    _h2 = run(2, "hold", 4500.0, 20000, dur=900.0, extra=END)
+finally:
+    app_main.compute = _real_compute
 _h2_old = no_bound(lambda: run(2, "hold", 4500.0, 20000, dur=900.0,
                                extra=END))
 _h1 = run(1, "hold", 4500.0, 20000, dur=900.0, extra=END)
@@ -224,6 +241,14 @@ print(f"    N=2 hold end: largest step {steps(_h2[0], 560, 700):.0f} W "
 check("⭐ N=2, the HOLD ends under a 4500 W house: the batteries move at most "
       "one slew step a cycle",
       _h2[1].maints[0].phase != "hold" and steps(_h2[0], 560, 700) <= SLEW)
+_held = [any(p != "idle" for p in r[-1][3]) for r in _h2[0]]
+print(f"    bound per cycle: {len(_bounds)} calls, {len(_held)} rows, "
+      f"{sum(_held)} in maintenance, "
+      f"{sum(b is not None for b in _bounds)} bounded")
+check("...and the bound is on while A holds and off from the cycle the HOLD "
+      "ends (review)",
+      len(_bounds) == len(_held) and any(not h for h in _held)
+      and all((b is not None) == h for b, h in zip(_bounds, _held)))
 check("⭐ N=2, A's charge pin lifts at a quarter's start: at most one slew "
       "step a cycle", steps(_cd[0], 1780, 1840) <= SLEW)
 check("...and N=1 already moved one slew step at a time (re-seeded each "
@@ -283,12 +308,11 @@ check("⭐ the page row reads the episodes, not the cycles "
 # ============================================================================ #
 print("\n6. the rules at their boundaries")
 DS = [((2000, 1499, 500), True), ((2000, 1500, 500), False),
-      ((2000, 2600, 500), False), ((-2000, -1499, 500), True),
-      ((-2000, -2600, 500), False), ((0, -900, 500), False),
-      ((0, 900, 500), False),
-      ((2000, -4, 500), True), ((-2000, 600, 500), True)]
-check("delivery_short: more than the margin short in the asked direction; "
-      "more than asked, or 0 W asked, is never short",
+      ((2000, 2600, 500), False), ((2000, -4, 500), True),
+      ((0, -900, 500), False), ((0, 900, 500), False),
+      ((-2000, -1499, 500), False), ((-2000, 600, 500), False)]
+check("delivery_short: a discharge more than the margin short; more than "
+      "asked, 0 W asked, or a charge (a full pack tapers) is never short",
       all(clamp.delivery_short(*a) is want for a, want in DS))
 check("the note names each battery, asked before given",
       clamp.shortfall_note([("inverter-a", 2000.0, -4.0)])
@@ -296,6 +320,129 @@ check("the note names each battery, asked before given",
       and clamp.shortfall_note([(None, -2000.0, 0.0)])
       == "OUTPUT_CLAMP: the battery gives +0 W of the -2000 W asked")
 
+# ============================================================================ #
+print("\n7. review: the peak guard, a battery that lags, a charge that tapers")
+import math                                                     # noqa: E402
+
+
+def under_guard(n, cut):
+    """A HOLD the peak guard raises (house 6500, limit 3500), 20 min."""
+    return run(n, "hold", 6500.0, 3500, dur=1200.0, cut=cut)
+
+
+_g1 = under_guard(1, lambda u, t: t >= 600)
+_g2 = under_guard(2, lambda u, t: u == "a" and t >= 600)
+print(f"    under the guard, the battery gives nothing: N=1 {warned(_g1[1])}, "
+      f"N=2 {warned(_g2[1])}")
+check("⭐ the peak guard sets the battery and it gives nothing: it warns at "
+      "N=1, and at N=2 (A, raised for the peak, gives nothing; B gives all "
+      "it is told)",
+      warned(_g1[1])[0] >= 1 and warned(_g2[1])[0] >= 1
+      and all("peak" in r[6] for rows, _c in (_g1, _g2) for r in rows
+              if 600 <= r[0] <= 620))
+check("...and it gives what it is told until then (no warning before 600 s)",
+      all(e.split(" ", 1)[0] >= "11:10:00" for c in (_g1[1], _g2[1])
+          for e in c.events if "OUTPUT_CLAMP" in e))
+
+
+def lagging(fn, lag=10.0):
+    """Run `fn` on plantsim's battery (800 W/s ramp, `lag` s first-order lag)
+    at the live 2 s meter tick."""
+    real_p, real_t = rig._Hass.power, rig.TICK
+
+    def power(self, u):
+        st = self.__dict__.setdefault("_plant", {})
+        a, tgt, tl = st.get(u, (0.0, 0.0, self.t))
+        for _ in range(int(round((self.t - tl) / 0.1))):
+            d = self.cmd[u] - tgt
+            tgt += math.copysign(min(abs(d), 80.0), d)
+            a += (tgt - a) * (1 - math.exp(-0.1 / lag))
+        st[u] = (a, tgt, self.t)
+        if self.cut(u, self.t) and self.cmd[u] >= -4.0:
+            return -4.0
+        return float(round(a))
+    rig._Hass.power, rig.TICK = power, 2.0
+    try:
+        return fn()
+    finally:
+        rig._Hass.power, rig.TICK = real_p, real_t
+
+
+def kettle(t):
+    """800 W, plus 2 kW 3 min in 10, plus 3 kW 1 min in 4."""
+    return (800.0 + (2000.0 if t % 600 < 180 else 0.0)
+            + (3000.0 if t % 240 < 60 else 0.0))
+
+
+def lag_runs():
+    return [run(n, ph, kettle, 3500, dur=1800.0)
+            for n, ph in ((1, "drain"), (2, "drain"), (2, "hold"))]
+
+
+_lag = lagging(lag_runs)
+_lag_old = patched(app_main, "SHORT_CYCLES", 1)(
+    lambda: patched(app_main, "SHORT_SPAN_S", 0.0)(
+        lambda: lagging(lag_runs)))
+_lag_dead = lagging(lambda: run(1, "drain", 1000.0, 20000, dur=900.0,
+                                cut=gone(600, 900)))
+print(f"    lagging battery, house steps under the guard: "
+      f"{[warned(c) for _r, c in _lag]}, graded on last cycle's command "
+      f"{[warned(c) for _r, c in _lag_old]}; gives nothing "
+      f"{warned(_lag_dead[1])}")
+check("⭐ a battery that lags (10 s) behind maintenance and peak-guard steps "
+      "at a 2 s meter does not warn (N=1 drain, N=2 drain and HOLD, 30 min)",
+      all(warned(c) == (0, 0, 0) for _r, c in _lag))
+check("non-vacuous: graded on last cycle's command alone, it warns",
+      sum(warned(c)[0] for _r, c in _lag_old) >= 3)
+check("...and a lagging battery that gives nothing still warns",
+      warned(_lag_dead[1])[0] >= 1)
+
+
+def taper(fn):
+    """Battery A takes the full charge until 600 s, then its acceptance falls
+    to 150 W over 30 min (constant-voltage taper)."""
+    real_p = rig._Hass.power
+
+    def power(self, u):
+        c = self.cmd[u]
+        if c < 0 and u == "a":
+            c = max(c, -max(150.0, 2500.0 - max(0.0, self.t - 600) / 1800
+                            * 2350.0))
+        return float(round(c))
+    rig._Hass.power = power
+    try:
+        return fn()
+    finally:
+        rig._Hass.power = real_p
+
+
+def charge_runs():
+    s84.OPT = dict(OPT0, peak_cap_w=20000)
+    try:
+        return [s84.run(n, load=lambda t: 800.0, cheap=True,
+                        store=s79.store_charging(n), soc=lambda u, t: 60.0,
+                        dur=2400.0) for n in (1, 2)]
+    finally:
+        s84.OPT = OPT0
+
+
+def both_ways(asked, given, margin):
+    asked, given = float(asked), float(given)
+    return asked != 0.0 and (asked - given) * math.copysign(1.0, asked) > margin
+
+
+_tp = taper(charge_runs)
+_tp_old = patched(app_main, "delivery_short", both_ways)(
+    lambda: taper(charge_runs))
+print(f"    a charge that tapers: {[warned(c) for _r, c in _tp]}, grading "
+      f"charges too {[warned(c) for _r, c in _tp_old]}")
+check("⭐ a maintenance charge that tapers near full does not warn (N=1, N=2)",
+      all(warned(c) == (0, 0, 0) for _r, c in _tp)
+      and all("charge" in {p for r in rows for p in r[-1][3]}
+              for rows, _c in _tp))
+check("non-vacuous: grading charges too, the taper warns",
+      all(warned(c)[0] >= 3 for _r, c in _tp_old))
+
 print()
 if fails:
     print(f"{len(fails)} FAILED: " + ", ".join(fails))