goodwe_controller/app/clamp.py

@@ -154,8 +154,9 @@ REASON_CODE = "OUTPUT_CLAMP"
 # and 2 loop-side ones". On the live install the counter reached 48 loop-side
 # ENGAGEMENTS in ~25 minutes of ordinary operation - roughly one cycle in six,
 # not a pinned rail, but far more often than a guard sized for 2-in-60 assumed -
-# and at a 30 s re-assert the owner received a persistent notification about it
-# all day.
+# and the owner received a persistent notification about it all day. ⚠️ NOT
+# because of the 30 s re-assert - see the block below; those notifications never
+# consulted this constant at all.
 #
 # ⚠️ BOTH FIGURES ARE ENGAGEMENT COUNTS; only the regime differs - 2-in-60 was
 # measured over a §10.3 RUNAWAY, 48-in-305 was ordinary running on the live
@@ -210,6 +211,28 @@ REASSERT_S = 150.0
 # Consecutive clean cycles before the alarm de-asserts (10.2 hysteresis).
 CLEAR_CYCLES = 3
 
+# ⚠️ SAFETY-41. Consecutive ENGAGED cycles after which an episode speaks even if
+# the notification budget is spent. THE ESCALATION EXISTS BECAUSE THE RATE LIMIT
+# ALONE DELAYS A RUNAWAY, and review measured it: a benign one-cycle clamp
+# followed by a continuous runaway held the runaway's first notification for up
+# to 140 s - one REASSERT_S minus a cycle. On the live install, where ordinary
+# railing spends the budget essentially all the time, a runaway arriving during
+# normal operation would be delayed with near-certainty. That is the "benign
+# noise suppresses the bug signal" shape this module's docstring is written
+# against.
+#
+# THE DISCRIMINATOR IS RUN LENGTH, and the two populations genuinely differ: the
+# live flood was runs of length ONE (engage, de-assert, engage), while a §10.3
+# runaway engages on EVERY cycle. So a sustained run speaks regardless of budget,
+# and scattered chatter does not.
+#
+# Measured, shipped-without vs shipped-with - it costs nothing it was protecting:
+#   live scatter flood (305 cycles, 1-in-6)   11 -> 11 notifications
+#   chatter 1-engaged/3-clean over 8000 s     50 -> 50 notifications
+#   pinned 900 s quarter (floor is 5)          6 ->  6 alarms
+#   runaway delay after a benign episode     140 s -> 10 s
+SUSTAINED_CYCLES = 3
+
 
 @dataclass(frozen=True)
 class ClampResult:
@@ -315,6 +338,10 @@ class ClampWatchdog:
         self.last_alarm: datetime | None = None
         self.reason = ""
         self.clean_cycles = 0
+        # SAFETY-41. Consecutive engaged cycles, and whether THIS episode has
+        # spoken yet - the two the escalation clause needs.
+        self.run_len = 0
+        self.spoken = False
 
     def note(self, engaged: bool, reason: str, now: datetime) -> bool:
         """Advance one cycle. True when the caller should raise the alarm.
@@ -326,10 +353,12 @@ class ClampWatchdog:
             self.engagements += 1
             self.clean_cycles = 0
             self.reason = reason
+            self.run_len += 1
             if not self.active:
                 self.active = True
                 self.since = now
                 self.episodes += 1
+                self.spoken = False
                 # ⚠️ SAFETY-41. THE RISING EDGE IS RATE-LIMITED TOO, and it did
                 # not used to be. It fired unconditionally, so a condition that
                 # engages and clears repeatedly raised one notification PER
@@ -358,15 +387,27 @@ class ClampWatchdog:
                 if (self.last_alarm is None
                         or (now - self.last_alarm).total_seconds() >= REASSERT_S):
                     self.last_alarm = now
+                    self.spoken = True
                     return True
                 return False
+            # ⚠️ THE ESCALATION. An episode that has not yet spoken because the
+            # budget was spent by an EARLIER, unrelated episode speaks anyway
+            # once it has engaged SUSTAINED_CYCLES times in a row. A runaway
+            # engages every cycle and reaches it; the scattered one-cycle
+            # railing that produced the live flood never does.
+            if not self.spoken and self.run_len >= SUSTAINED_CYCLES:
+                self.last_alarm = now
+                self.spoken = True
+                return True
             # Still engaged: re-assert on the slow cadence, not every cycle.
             if (self.last_alarm is None
                     or (now - self.last_alarm).total_seconds() >= REASSERT_S):
                 self.last_alarm = now
+                self.spoken = True
                 return True
             return False
 
+        self.run_len = 0
         if not self.active:
             return False
         self.clean_cycles += 1

goodwe_controller/test_clamp.py

@@ -78,7 +78,8 @@ import sys
 from datetime import datetime, timedelta, timezone
 
 from app.clamp import (
-    CLEAR_CYCLES, REASON_CODE, REASSERT_S, ClampWatchdog, clamp_to_rating,
+    CLEAR_CYCLES, REASON_CODE, REASSERT_S, SUSTAINED_CYCLES, ClampWatchdog,
+    clamp_to_rating,
     is_commandable,
 )
 from app.allocator import Allocation, UnitPlan, plan
@@ -296,13 +297,54 @@ check(f"...and the rate honours the same interval the pinned case does: at most
 check("...while EVERY episode and EVERY engagement is still counted - the "
       "budget is on notifications, never on evidence",
       scat.episodes == scat.engagements == SCATTER_CYCLES // SCATTER_ONE_IN + 1)
-# ⚠️ THE COUNTERFACTUAL. A runaway that begins from quiet must NOT be delayed by
-# a budget spent on an earlier, unrelated episode - so a cold watchdog speaks at
-# once, and this is the half that would go red if the rate limit were applied
-# without regard to how long it has been silent.
-check("...and a rising edge from a COLD watchdog still fires immediately, so a "
-      "runaway beginning from quiet is never held back",
-      ClampWatchdog().note(True, "cold runaway", T0) is True)
+# ⚠️ THE COUNTERFACTUAL, AND THE FIRST VERSION OF IT COULD NOT FAIL FOR THE
+# DEFECT IT NAMED. It read `ClampWatchdog().note(True, ...) is True` - a FRESH
+# object, so it only ever exercised the `last_alarm is None` branch, which is the
+# trivially-true one. A fresh watchdog has no earlier episode to be held back BY.
+# That is the third instance in this file of the family recorded at line 216 and
+# at the 224 s sweep, and it was guarding the most safety-critical property in
+# the change. Review found it.
+#
+# The stimulus must therefore SPEND THE BUDGET FIRST: a benign one-cycle clamp,
+# a gap long enough to de-assert, then a continuous runaway. Without the
+# escalation clause the runaway's first notification is held for up to
+# REASSERT_S - measured at 140 s before it was added.
+_esc = ClampWatchdog()
+_esc.note(True, "benign", T0)                                   # spends the budget
+for _i in range(1, 1 + CLEAR_CYCLES + 1):                       # de-assert
+    _esc.note(False, "", T0 + timedelta(seconds=5.0 * _i))
+_RUNAWAY_AT = 5.0 * (CLEAR_CYCLES + 2)
+_esc_fires = [5.0 * (CLEAR_CYCLES + 2 + _j)
+              for _j in range(200)
+              if _esc.note(True, "runaway",
+                           T0 + timedelta(seconds=5.0 * (CLEAR_CYCLES + 2 + _j)))]
+_esc_delay = _esc_fires[0] - _RUNAWAY_AT if _esc_fires else None
+print(f"    budget spent by a benign episode, then a continuous runaway from "
+      f"t={_RUNAWAY_AT:.0f}s: first alarm at t={_esc_fires[0]:.0f}s "
+      f"(delay {_esc_delay:.0f}s)")
+check(f"⚠️ a SUSTAINED runaway speaks even when an earlier, unrelated episode "
+      f"spent the budget - it is not held for REASSERT_S ({_esc_delay:.0f}s "
+      f"delay, escalation is {SUSTAINED_CYCLES} cycles)",
+      _esc_delay is not None and _esc_delay < REASSERT_S / 2)
+check("...and it speaks at the escalation boundary rather than immediately, "
+      "which is what keeps the scattered flood suppressed",
+      _esc_delay >= 5.0 * (SUSTAINED_CYCLES - 1))
+
+# ⚠️ `last_alarm` SURVIVING THE DE-ASSERT IS NOW LOAD-BEARING, AND NOTHING
+# PINNED IT. Review built the mutation: if the de-assert path ALSO refreshed
+# `last_alarm = now`, the chatter case collapses from 50 notifications to 1 - the
+# alarm never fires again. That is the "alarm that never fires" class, which is a
+# different bug from one that fires later, and the suite was green on it.
+_starve = ClampWatchdog()
+_starve_fires = sum(1 for _i in range(1600)
+                    if _starve.note(_i % 4 == 0, "chatter",
+                                    T0 + timedelta(seconds=5.0 * _i)))
+print(f"    chatter 1-engaged/{CLEAR_CYCLES}-clean over 8000 s: "
+      f"{_starve.episodes} episodes, {_starve_fires} notification(s)")
+check(f"a de-assert does not RESET the notification budget, so repeated chatter "
+      f"cannot starve the alarm to silence ({_starve_fires} notifications over "
+      f"8000 s, not 1)",
+      _starve_fires > 10)
 
 # --- THE HYSTERESIS, PINNED AGAINST THE CHATTER DIRECTION --------------------
 # ⚠️ REVIEW-FOUND. This used to issue `range(CLEAR_CYCLES - 1)` clean cycles,