SAFETY-86 review: hold the site at the peak floor while a HOLD gives

At N>=2 the holding pack's pin follows the guard's floor while the loop
still asks for +5000 W, so the ALLOCATOR "could not be placed" row was
re-logged on every 1 W change and filled the 40-row action log. While the
pin is raised, cycle() now files a site `hold` ceiling at the floor
(P_MAINTENANCE), as SAFETY-84 does for a charge. Filed only when the
holding pack gives; hi == the floor, so it never clashes with it.

Tests: a noisy N=2 house logs no ALLOCATOR row and at most one clamp
row (20 and 20 without the ceiling); the ceiling is the floor itself;
a CHARGE is not refiled at N=1. The DECISIONS entry records the owner's
2026-10-01 decision: the HOLD ends on its clock, no top-up.

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

goodwe_controller/DECISIONS/SAFETY-86-peak-over-hold.md

@@ -20,6 +20,10 @@ battery gave: 4500 W against a 3950 W aim (SAFETY-84 rig test, check 6).
   (owner order), and the owner rule for an Off battery is the same, "give way
   only for the shortfall". A battery Off by its mode (P_MODE, below
   maintenance) therefore still gives before the holding one.
+- N>=2, while the holding battery gives: a site `hold` ceiling at the floor
+  (P_MAINTENANCE), so the loop does not ask for watts the hold keeps back.
+  Without it the moving pin re-logged the ALLOCATOR "could not be placed"
+  row each cycle and filled the action log (review, PR #161).
 - Below the aim nothing changes: the floor is 0 W, the hold is 0 W, the rows
   are identical to before (test_safety86.py part 2).
 - A site stop (a SET above `P_PEAK`) decides alone: nothing is raised at N>=2,
@@ -29,14 +33,11 @@ battery gave: 4500 W against a 3950 W aim (SAFETY-84 rig test, check 6).
 - The hold resumes by itself: the rule is evaluated each cycle, so when the
   guard lets go the command is 0 W again.
 
-**The level dips below the target: the hold finishes anyway.** The HOLD does
-not read the level; it ends on its clock (`maintenance_hold_min`). Topping up
-would mean going back to CHARGE, which waits for sun or a cheap window (up to
-36 h, `CHARGE_TIMEOUT_MIN`) while the pack keeps the site's one maintenance
-slot, so the other battery's cycle waits, and then a new full hold that the
-guard can interrupt again. The guard only gives while the quarter is at risk,
-and the next monthly cycle balances again. If the owner wants a top-up
-instead, it is a change to `maintenance.py`'s HOLD branch, not to this rule.
+**The level dips below the target: the hold finishes anyway.** Owner
+decision 2026-10-01: the HOLD ends on its clock (`maintenance_hold_min`) even
+when the peak guard pulls the level down; no top-up. A top-up would re-enter
+CHARGE (up to 36 h, `CHARGE_TIMEOUT_MIN`) and keep the site's one maintenance
+slot meanwhile.
 
 **Not changed.**
 - The peak guard still grades the emptiest battery (SAFETY-32): at N=2 with

goodwe_controller/app/arbiter.py

@@ -202,6 +202,10 @@ CLAIM_SCOPE = {
     # sibling would have to give against it. Never filed at N=1.
     # DECISIONS/SAFETY-84-charge-at-two.md.
     "charge": SCOPE_SITE,
+    # main.py: `Claim.limit("hold", P_MAINTENANCE, hi=...)`, SAFETY-86. While a
+    # holding pack gives the peak shortfall at N>=2, the site target stays at
+    # the peak floor, what the pin and the others place. Never filed at N=1.
+    "hold": SCOPE_SITE,
 
     # --- UNIT: about one pack, one inverter, one device -------------------
     # main.py: `Claim.limit("device", P_SAFETY, self.dev_min, self.dev_max,

goodwe_controller/app/main.py

@@ -2533,13 +2533,14 @@ class Controller:
             if c.source == peakguard.SOURCE and c.lo > rate:
                 claims[i] = dataclasses.replace(c, priority=P_MAINTENANCE)
 
-    def _peak_over_hold(self, claims, maint_results) -> None:
+    def _peak_over_hold(self, claims, maint_results):
         """SAFETY-86: a peak floor outranks a maintenance HOLD (0 W).
 
         At N=1 the floor is refiled at P_MAINTENANCE, as for a drain. At N>=2
         the holding pack gives only what the others cannot: its pin is raised
-        to the floor minus what they may place. A site stop still decides
-        alone. DECISIONS/SAFETY-86-peak-over-hold.md.
+        to the floor minus what they may place, and the floor is returned as
+        the site `hold` ceiling (else None). A site stop still decides alone.
+        DECISIONS/SAFETY-86-peak-over-hold.md.
         """
         hold = {self.actuator_of("maintenance", inv)
                 for inv, res in zip(self.inverters, maint_results)
@@ -2569,13 +2570,15 @@ class Controller:
             if p is not None and p < 0.0:
                 return          # a pack pinned to charge: never split both ways
             others += allocator.hi_w(b) if p is None else p
-        give = max(claims[i].lo for i in peak) - others
+        floor = max(claims[i].lo for i in peak)
+        give = floor - others
         if give <= 0.0:
             return
         for i, c in enumerate(claims):
             if c.source == "maintenance" and c.kind == SET \
                     and c.actuator in hold:
                 claims[i] = dataclasses.replace(c, value=give)
+        return floor
 
     def _charge_headroom(self, floor: float, claims) -> float:
         """SAFETY-84: at N>=2 a cheap-window charge pulls no more than the
@@ -4352,7 +4355,13 @@ class Controller:
                                       reason="maintenance drain",
                                       actuator=self.actuator_of("drain")))
         self._peak_over_drain(claims, maint_results)
-        self._peak_over_hold(claims, maint_results)
+        # --- SAFETY-86: while a HOLD gives, the site stays at the peak floor --
+        # So the loop never asks for watts the hold keeps back.
+        hold_hi = self._peak_over_hold(claims, maint_results)
+        if hold_hi is not None:
+            claims.append(Claim.limit("hold", P_MAINTENANCE, hi=hold_hi,
+                                      reason="maintenance hold",
+                                      actuator=self.actuator_of("hold")))
         # --- SAFETY-84: a cheap-window charge is a ceiling over the site ----
         charge = self._charge_ceiling(claims)
         if charge is not None:

goodwe_controller/app/messages.py

@@ -836,6 +836,7 @@ WHY_LIMIT = {
     "reserve": "the battery reserve",
     "maintenance": "battery maintenance",
     "charge": "battery maintenance",
+    "hold": "battery maintenance",
     "refused": "a battery that refuses power",
 }
 

goodwe_controller/test_arbiter.py

@@ -149,7 +149,7 @@ check("...and `source` is still the first positional argument, which is what "
       Claim.limit("device", P_SAFETY, -1, 1).source == "device")
 
 print()
-print("ARB-03 2. the split: fifteen sources, each classified where it is BUILT")
+print("ARB-03 2. the split: sixteen sources, each classified where it is BUILT")
 # The classification is graded against what each claim is ABOUT, restated here
 # so that the table in arbiter.py and the expectation in a test are not the same
 # sentence read twice. test_peakguard.py part 10f pins this set against main.py's
@@ -179,6 +179,9 @@ SITE_SOURCES = {
     "charge":    "the site target may not go above a cheap-window maintenance "
                  "charge, or a sibling would give against it. Filed at N>1 "
                  "only",
+    # SAFETY-86.
+    "hold":      "the site target stays at the peak floor while a holding pack "
+                 "gives the shortfall. Filed at N>1 only",
 }
 UNIT_SOURCES = {
     "device":     "one inverter's own reported min/max rating",
@@ -210,9 +213,9 @@ for _src, _why in sorted(SITE_SOURCES.items()):
     print(f"    site  {_src:<11} {_why}")
 for _src, _why in sorted(UNIT_SOURCES.items()):
     print(f"    unit  {_src:<11} {_why}")
-check("all FIFTEEN claim sources are classified, and only those fifteen",
+check("all SIXTEEN claim sources are classified, and only those sixteen",
       set(CLAIM_SCOPE) == set(SITE_SOURCES) | set(UNIT_SOURCES))
-check("the eight that are about the GRID CONNECTION are site claims",
+check("the nine that are about the GRID CONNECTION are site claims",
       {s for s, v in CLAIM_SCOPE.items() if v == SCOPE_SITE} == set(SITE_SOURCES))
 check("the seven that are about a PACK, a DEVICE or one write are per-inverter",
       {s for s, v in CLAIM_SCOPE.items() if v == SCOPE_UNIT} == set(UNIT_SOURCES))
@@ -234,7 +237,7 @@ check("...and with none configured at all, likewise",
       all(actuator_for(s, "inverter-a", ()) == ACTUATOR_SITE for s in CLAIM_SCOPE))
 check("with TWO the identity breaks and the split becomes real: the seven "
       "per-inverter sources route to the unit this controller drives, the "
-      "eight site sources stay on the grid connection",
+      "nine site sources stay on the grid connection",
       {s: actuator_for(s, "inverter-a", TWO) for s in CLAIM_SCOPE}
       == {**{s: "inverter-a" for s in UNIT_SOURCES},
           **{s: ACTUATOR_SITE for s in SITE_SOURCES}})
@@ -515,7 +518,7 @@ check("...and at N=1, where every source legitimately routes to the site, the "
           Claim.limit("maintenance", P_MAINT_SHAPE, hi=0.0)]) == [])
 check("an unclassified ninth source is a violation wherever it is filed",
       scope_violations([Claim.limit("reserve", P_LOOP, hi=0.0)]) != [])
-check("and the eight, routed by actuator_for() at a two-inverter site, are "
+check("and the sixteen, routed by actuator_for() at a two-inverter site, are "
       "clean - which is what says the router and the checker agree",
       scope_violations(
           [Claim.limit(s, P_SAFETY, -1, 1,

goodwe_controller/test_peakguard.py

@@ -2691,9 +2691,12 @@ check("...and so does the persistent notification, which outlives the page and "
 # site `hi` at P_MAINT_SHAPE, filed only at N>=2 while a pack's cheap-window
 # maintenance charge makes it charge, equal to that pack's own `hi`. Never at
 # N=1. DECISIONS/SAFETY-84-charge-at-two.md.
+# SAFETY-86 added `hold`: a site `hi` at the peak floor while a holding pack
+# gives at N>=2. Not a reserve either. DECISIONS/SAFETY-86-peak-over-hold.md.
 CLAIM_SOURCES = {"device", "supervised", "failsafe", "safety", "peak",
                  "arbitrage", "maintenance", "loop", "circulation",
-                 "sign_site", "pack", "refused", "mode", "drain", "charge"}
+                 "sign_site", "pack", "refused", "mode", "drain", "charge",
+                 "hold"}
 # The two appends that file a claim BUILT ELSEWHERE in the method. Named here so
 # that a THIRD such indirection reads as an unrecognised append rather than as a
 # silent gap in the scan.
@@ -2735,7 +2738,7 @@ check("every `claims.append(...)` in main.py's cycle is one this scan can READ "
       "- a claim filed through a shape it cannot parse has to fail here rather "
       "than be counted as nothing, which is how a scan like this goes quietly "
       "vacuous", len(_appends) >= 10 and None not in _filed)
-check("...and the sources that cycle can file are EXACTLY these FIFTEEN. THIS "
+check("...and the sources that cycle can file are EXACTLY these SIXTEEN. THIS "
       "is what makes part 10 a tripwire: replay_loop() hand-builds the claim "
       "list, so it cannot see a claim main.py adds - and a loop-side SoC "
       "reserve is a new source. Filing one takes this line red on the line it "
@@ -2875,7 +2878,7 @@ print("    site: " + ", ".join(sorted(s for s, v in CLAIM_SCOPE.items()
       + "  |  per-inverter: "
       + ", ".join(sorted(s for s, v in CLAIM_SCOPE.items() if v == SCOPE_UNIT)))
 check("...and the split is the one ARB-03 argued, with SAFETY-31's and "
-      "SAFETY-30's, OPT-09's, SAFETY-78's and SAFETY-84's additions: the eight about the GRID CONNECTION "
+      "SAFETY-30's, OPT-09's, SAFETY-78's, SAFETY-84's and SAFETY-86's additions: the nine about the GRID CONNECTION "
       "are site claims and the five about a pack, a device or one write are "
       "per-inverter. `circulation` is a site claim because its quantity is "
       "defined over the roster and is identically zero for any one unit; "
@@ -2885,7 +2888,7 @@ check("...and the split is the one ARB-03 argued, with SAFETY-31's and "
       "against that unit's OWN measured power",
       {s for s, v in CLAIM_SCOPE.items() if v == SCOPE_SITE}
       == {"loop", "peak", "arbitrage", "safety", "circulation", "sign_site",
-          "drain", "charge"}
+          "drain", "charge", "hold"}
       and {s for s, v in CLAIM_SCOPE.items() if v == SCOPE_UNIT}
       == {"device", "supervised", "failsafe", "maintenance", "pack",
           "refused", "mode"})
@@ -2990,7 +2993,8 @@ check("⚠️ AND THIS IS THE BREAKING HALF. Give the same Controller a second "
                   "loop": ACTUATOR_SITE, "peak": ACTUATOR_SITE,
                   "arbitrage": ACTUATOR_SITE, "safety": ACTUATOR_SITE,
                   "circulation": ACTUATOR_SITE, "sign_site": ACTUATOR_SITE,
-                  "drain": ACTUATOR_SITE, "charge": ACTUATOR_SITE})
+                  "drain": ACTUATOR_SITE, "charge": ACTUATOR_SITE,
+                  "hold": ACTUATOR_SITE})
 check("...and the roster it was restored to is the shipped one, so nothing "
       "below inherits a two-inverter Controller",
       c_src.units == (app_main.UNIT_A,) and len(c_src.inverters) == 1)
@@ -3069,13 +3073,13 @@ _unit_sources_unrouted = [
     if CLAIM_SCOPE.get(r[0]) == SCOPE_UNIT and "," not in r[1]]
 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 SIXTEEN of `Controller.cycle()`'s DIRECT appends route through "
+check("all SEVENTEEN of `Controller.cycle()`'s DIRECT appends route through "
       "`self.actuator_of(<string literal>)`, 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) == 16
+      len(_direct) == 17
       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 ELEVEN THE LITERAL IT ROUTES THROUGH IS THE "
@@ -3109,8 +3113,8 @@ check("...and the only appends this pin does NOT reach are the two that file a "
       "arbitrage.claim() by test_arbitrage.py",
       sorted(_indirect) == sorted(CLAIM_BUILDERS)
       and len(_direct) + len(_indirect) == len(_appends))
-check("...and between them the eighteen appends account for exactly the "
-      "fifteen "
+check("...and between them the nineteen appends account for exactly the "
+      "sixteen "
       "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())
@@ -3171,7 +3175,7 @@ check("...and naming those three is not an accident of this fixture but the "
       {c.source for c in _cl2}
       == CLAIM_SOURCES - {"failsafe", "safety", "maintenance", "peak",
                           "arbitrage", "circulation", "sign_site",
-                          "refused", "mode", "drain", "charge"})
+                          "refused", "mode", "drain", "charge", "hold"})
 # ⚠️ AND NOW THE HALF THAT WAS EVIDENCE-SHAPED AND OBSERVED NOTHING. The three
 # checks above grade WHICH ACTUATOR each claim lands on. They say nothing about
 # WHOSE NUMBERS the bound is built from - and those are two different defects

goodwe_controller/test_safety86.py

@@ -38,7 +38,8 @@ from app import packmode                                       # noqa: E402
 from app.arbiter import (Claim, P_MAINTENANCE, P_PEAK, P_SAFETY,  # noqa
                          resolve)
 from app.main import Controller                                # noqa: E402
-from app.maintenance import DRAIN, HOLD, IDLE, MaintResult     # noqa: E402
+from app.maintenance import (CHARGE, DRAIN, HOLD, IDLE,          # noqa: E402
+                             MaintResult)
 from app.store import unit_key                                 # noqa: E402
 
 fails = []
@@ -206,6 +207,47 @@ check("⭐ battery modes on, B Off: the Off battery still gives before the "
       and 540.0 <= cmd(_off_rows[-1])["a"] <= 560.0
       and refused(_off_rows) == 0 and not alarmed(_off))
 
+
+
+def noisy(t):
+    """A 6500 W house that moves up to 150 W from one second to the next."""
+    return 6500.0 + ((int(t) * 7919) % 301) - 150.0
+
+
+def no_ceiling(fn):
+    """Run with the pin raised but no `hold` site ceiling (review check)."""
+    real = Controller._peak_over_hold
+
+    def pin_only(self, claims, results):
+        real(self, claims, results)
+    Controller._peak_over_hold = pin_only
+    try:
+        return fn()
+    finally:
+        Controller._peak_over_hold = real
+
+
+def rows_logged(c, word):
+    return sum(1 for e in c.events if word in e)
+
+
+_nz_rows, _nz = run(noisy, 2)
+_nz_old = no_ceiling(lambda: run(noisy, 2))[1]
+print(f"    noisy house: ALLOCATOR rows {rows_logged(_nz, 'ALLOCATOR')}, "
+      f"clamp rows {rows_logged(_nz, 'OUTPUT_CLAMP')} (without the site "
+      f"ceiling {rows_logged(_nz_old, 'ALLOCATOR')} and "
+      f"{rows_logged(_nz_old, 'OUTPUT_CLAMP')})")
+check("⭐ while A gives, the site target is the guard's floor (the `hold` "
+      "ceiling), so a noisy house does not fill the action log: no ALLOCATOR "
+      "row and at most one clamp row",
+      on_aim(_nz) and all(abs(r[1] - r[-1][1][1]) < 1e-6 for r in _nz_rows
+                          if cmd(r)["a"] > 0.0)
+      and rows_logged(_nz, "ALLOCATOR") == 0
+      and rows_logged(_nz, "OUTPUT_CLAMP") <= 1)
+check("non-vacuous: without the ceiling the loop asks 5000 W and the "
+      "ALLOCATOR and clamp rows fill the 40-row log",
+      rows_logged(_nz_old, "ALLOCATOR") >= 10 and len(_nz_old.events) == 40)
+
 # ============================================================================ #
 print("\n4. a month peak banked above the limit")
 BANK = {n: (run(6500.0, n, month_peak=4000.0),
@@ -301,7 +343,8 @@ def pin_after(floor, *extra, live=(A, B), results=HOLDING, c=_c):
           Claim.set("maintenance", P_MAINTENANCE, 0.0, HOLD, actuator=A),
           Claim.limit("peak", P_PEAK, lo=floor, reason="quarter peak"),
           *extra]
-    Controller._peak_over_hold(fake, cl, results)
+    ceiling = Controller._peak_over_hold(fake, cl, results)
+    pin_after.ceiling = ceiling
     return next((x.value for x in cl
                  if x.source == "maintenance" and x.kind == "set"), None), cl
 
@@ -331,6 +374,15 @@ check("⭐ a site safety SET: A stays at 0 W and the site resolves to 0 W; "
       _p == 0.0 and resolve(_cl, units=(A, B), live={A, B}).target_w == 0.0
       and pin_after(3000.0, Claim.set("failsafe", P_SAFETY, 0.0,
                                       actuator=B))[0] == 3000.0)
+_p, _cl = pin_after(2600.0)
+_hi = pin_after.ceiling
+_cl += [Claim.limit("hold", P_MAINTENANCE, hi=_hi, actuator="site"),
+        Claim.set("loop", 10, 5000.0, "the loop asks for the whole house")]
+pin_after(2500.0)
+check("⭐ at N=2 the `hold` site ceiling is the floor itself, only when A "
+      "gives, and a 5000 W loop ask resolves to that floor",
+      _hi == 2600.0 and pin_after.ceiling is None
+      and resolve(_cl, units=(A, B), live={A, B}).target_w == 2600.0)
 check("only a HOLD: a DRAIN pin or an idle pack is not raised",
       pin_after(4000.0, results=[MaintResult(DRAIN, True, 0.0, False, []),
                                  HOLDING[1]])[0] == 0.0
@@ -355,8 +407,11 @@ _stop = refile(3000.0) + [Claim.set("safety", P_SAFETY, 0.0,
                                     "inputs missing")]
 check("⭐ at N=1 a refiled floor stays under a safety SET: 0 W",
       resolve(_stop).target_w == 0.0)
-check("at N=1 outside a HOLD the claim is not touched by this rule",
+check("at N=1 outside a HOLD (idle, or a CHARGE) the claim is not touched "
+      "by this rule",
       refile(3000.0, (MaintResult(IDLE, False, None, False, []),))[1]
+      .priority == P_PEAK
+      and refile(3000.0, (MaintResult(CHARGE, False, None, True, []),))[1]
       .priority == P_PEAK)
 
 print()