CI-02 probe F: the ORIGINAL gate, unchanged, on the same unmergeable PR

goodwe_controller/app/freeze.py

@@ -151,12 +151,77 @@ THE RAIL TERM, PRECISELY
 reason SAFETY-02 uses the emitted value: the wire carries the emitted number.
 
 RAIL_FRAC is 0.95 and is a constant rather than an option. The command is
-quantised to `step_w` (max 100) and clamped to `max_w` (min 100), so at the rail
-the emitted value IS `max_w` on every schema-legal combination; the 5 % is slack
-for a device rating that clips `max_w` slightly in `write_setpoint()`, not a
-tuning knob. Making it configurable would offer a way to widen the one term that
+quantised to `step_w` (max 100), so at the rail the emitted value IS the ceiling
+on every schema-legal combination; the 5 % is quantisation slack, not a tuning
+knob. Making it configurable would offer a way to widen the one term that
 carries the whole no-false-positive argument.
 
+⚠️ SAFETY-20 - AND THE CEILING IS NOT `max_w`. THE 5 % USED TO BE DESCRIBED AS
+"slack for a device rating that clips `max_w` slightly in `write_setpoint()`",
+AND THAT SENTENCE WAS THE BUG. 5 % of slack does not absorb a device rating; it
+inverts the guard. `write_setpoint()` clamps the emitted command to `max_w` AND
+THEN to the setpoint device's own `[dev_min, dev_max]` range (clamp.py,
+`clamp_to_rating()`), and the arbiter files the same range as a P_SAFETY limit
+claim, so the largest magnitude that can ever stand on the wire is
+
+    min(max_w, |dev_min|, |dev_max|)
+
+not `max_w`. Measured on the shipped predicate at `max_w: 5000` against a device
+of +-4700 - the exact case the ticket is named for:
+
+    looks_frozen(unchanged_s=3600, commanded_w=4700, max_w=5000)  ->  False
+
+The command is pinned at the hardware's rail, the meter has been frozen for an
+hour, and term 2 is unsatisfiable BY CONSTRUCTION because 4700 < 4750. Any
+device rating at or below `RAIL_FRAC * max_w` disarms the conjunction outright.
+The status page then printed `armed (...command is over 4750 W)`, which is the
+"disarmed guard that advertises itself as armed" failure validate.py rule 5 and
+SAFETY-15/19 all exist to prevent - so `rail_ceiling_w()` below takes the device
+range and the rail is measured against the smaller number.
+
+WHY THE SMALLER OF THE TWO DEVICE BOUNDS, AND NOT A SIDE-AWARE PAIR. The rail is
+a MAGNITUDE test against what clamp.py calls a magnitude clamp - it binds import
+and export identically - so the ceiling here is a magnitude that is available in
+BOTH directions. The alternative considered was a per-sign rail:
+`min(max_w, dev_max)` for a positive command, `min(max_w, |dev_min|)` for a
+negative one. That is arithmetically tighter and it is NOT wrong - on a device
+range of -500..+5000 it would grade a -480 W charge command as railed, which the
+symmetric ceiling reaches only by dropping the discharge rail to 475 W too. It
+was rejected on cost, not on correctness: a per-sign rail is two rails, and two
+rails need two coverage verdicts (`dev_min == 0` is a degenerate charge rail
+whatever the discharge side can do) on a status row that carries one number and
+in a predicate that takes one magnitude. That is a bigger change than this
+defect earns, and the ONLY configuration that distinguishes the two - a device
+range narrower than `max_w` on one side - is refused at startup by CFG-01 (see
+the reachability note below). Between the two, the symmetric ceiling can only
+ever LOWER the rail, which widens term 2 rather than disarming it; term 1
+(`unchanged_s >= hold_s`) is the term carrying the no-false-positive argument,
+and it is untouched. Whoever needs the tighter version should take both status
+rows with it.
+
+⚠️ AND WHEN THE CEILING IS ZERO THE RAIL IS NOT "LOW", IT IS GONE. A device that
+accepts no command in one direction at all (`dev_max == 0`, or `dev_min == 0`)
+gives a ceiling of 0 W, and `abs(commanded_w) >= 0` is TRUE FOR EVERY COMMAND
+INCLUDING 0 W. Term 2 would not be disarmed there, it would be degenerately
+true, and the detector would collapse into the bare `unchanged_s > N` threshold
+TEL-05 refused - false-tripping at the converged operating point we aim for.
+That case is reported as NOT COVERED, through the row the unsupported-transport
+case already uses, and `looks_frozen()` returns False rather than firing.
+
+⚠️ HOW REACHABLE IS ANY OF THIS - READ THIS BEFORE "SIMPLIFYING" IT AWAY.
+`validate_startup_config()` rule 2 (CFG-01) already REFUSES TO START on
+`max_w > dev_max` or `-max_w < dev_min`, and `run_control()` re-runs that gate
+once ESPHome is up and the real rating is known. So on any install that reaches
+the control loop with a rating it could read, `min(max_w, |dev_min|, |dev_max|)`
+IS `max_w` and everything above is arithmetically dead. That is not an argument
+against it - it is clamp.py's argument for its own second stage, word for word:
+the case where this bites is the case where the startup gate did not run or no
+longer holds (`hass.limits()` returns (None, None) whenever ESPHome is not up,
+and the range checks are then skipped rather than guessed at, so a rating that
+arrives late is never re-read), and a guard that quietly assumes the gate ran is
+a guard that reports armed in exactly the state it cannot fire in. See AC 4's
+decision, recorded in validate.py under WHAT IS DELIBERATELY *NOT* REFUSED.
+
 ⚠️ NOT GATED ON `following`, and that is deliberate. A maintenance charge
 commands against the meter on purpose and can sit at the rail for hours - but it
 does so while pushing kilowatts through the meter, so the FIRST term is what
@@ -192,6 +257,8 @@ otherwise would be worse than the gap.
 from dataclasses import dataclass
 from datetime import datetime
 
+from .clamp import is_commandable
+
 # See "THE RAIL TERM, PRECISELY" above. A constant, on purpose.
 RAIL_FRAC = 0.95
 
@@ -203,13 +270,72 @@ class FreezeWatchdogConfig:
     hold_s: float = 300.0
 
 
-def rail_w(max_w) -> float:
-    """The magnitude at or above which a command counts as railed."""
-    return abs(float(max_w)) * RAIL_FRAC
+def rail_ceiling_w(max_w, *, dev_min=None, dev_max=None) -> float:
+    """The largest command magnitude that can actually stand on the wire.
+
+    SAFETY-20, and the whole point of the ticket: this is NOT `max_w`. The
+    emitted command is bounded by `max_w` AND by the setpoint device's own
+    `[dev_min, dev_max]` range - twice, in `clamp_to_rating()` and again as the
+    arbiter's `device` P_SAFETY limit claim - so the reachable ceiling is the
+    smallest of the three magnitudes. See "THE RAIL TERM, PRECISELY" for why it
+    is the smaller of the two device bounds rather than a per-sign pair, and for
+    why this is arithmetically dead on any install CFG-01 let start.
+
+    `dev_min` / `dev_max` are `hass.limits()`'s answer and are None whenever
+    ESPHome could not be reached. UNKNOWN IS NOT ZERO: a missing bound means
+    this add-on applies no device clamp at all on that side (clamp.py's second
+    stage is skipped unless BOTH are known), so an unknown bound must not lower
+    the ceiling. `is_commandable()` is write_setpoint()'s own predicate, reused
+    rather than retyped: a NaN rating must not become a 0 W ceiling.
+
+    Returns 0.0 when the device can command nothing - see rail_unreachable().
+    """
+    ceiling = abs(float(max_w))
+    for bound in (dev_min, dev_max):
+        if is_commandable(bound):
+            ceiling = min(ceiling, abs(float(bound)))
+    return max(0.0, ceiling)
+
+
+def rail_unreachable(max_w, *, dev_min=None, dev_max=None) -> str:
+    """"" when the rail is a real threshold, else WHY it is not. SAFETY-20 AC2.
+
+    The status page renders this as a NOT COVERED row - the same row, for the
+    same reason, that an unsupported transport already gets. It is a second
+    reason to use that surface, not a new mechanism: in both states the honest
+    report is "this guard is not watching", and the one thing this project has
+    decided repeatedly it will not ship is a page that says `armed` instead.
+
+    ⚠️ THE UNREACHABLE CASE IS NOT "THE RAIL IS HIGH", IT IS "THE RAIL IS 0 W".
+    A ceiling of zero makes `abs(commanded_w) >= 0` true for every command
+    including a parked 0 W one, so term 2 is not disarmed but DEGENERATELY TRUE,
+    and the conjunction decays into the bare `unchanged_s > N` threshold TEL-05
+    refused - which false-trips at the converged operating point this controller
+    aims for. Silence plus a NOT COVERED row is strictly better than that.
+    """
+    if not max_w or not is_commandable(max_w):
+        return (f"max_w is {max_w!r}, so there is no ceiling to measure a rail "
+                f"against")
+    if rail_ceiling_w(max_w, dev_min=dev_min, dev_max=dev_max) > 0.0:
+        return ""
+    return (f"the setpoint device reports a range of {dev_min}...{dev_max} W, "
+            f"which leaves no command magnitude available in both directions, "
+            f"so the rail term would be true of every command including 0 W")
+
+
+def rail_w(max_w, *, dev_min=None, dev_max=None) -> float:
+    """The magnitude at or above which a command counts as railed.
+
+    Measured against the REACHABLE ceiling, not against `max_w` - SAFETY-20.
+    Callers that do not know the device range pass none and get the old
+    behaviour, which is correct precisely when there is no device clamp to
+    account for.
+    """
+    return rail_ceiling_w(max_w, dev_min=dev_min, dev_max=dev_max) * RAIL_FRAC
 
 
 def looks_frozen(unchanged_s, commanded_w, max_w,
-                 cfg: FreezeWatchdogConfig) -> bool:
+                 cfg: FreezeWatchdogConfig, *, dev_min=None, dev_max=None) -> bool:
     """The predicate, and the ONLY place the conjunction is written down.
 
     Public for the same reason `watchdog.disagrees()` is: the checks that grade
@@ -220,12 +346,28 @@ def looks_frozen(unchanged_s, commanded_w, max_w,
     `unchanged_s`  seconds since the meter served a different measurement, or
                    None on a transport that cannot know (see the module note).
     `commanded_w`  the EMITTED setpoint standing on the wire.
-    `max_w`        the configured output bound the rail is measured against.
+    `max_w`        the configured output bound.
+    `dev_min` /    the setpoint device's own reported range, or None when it
+    `dev_max`      could not be read. SAFETY-20: the rail is measured against
+                   `min(max_w, |dev_min|, |dev_max|)`, because that - not
+                   `max_w` - is what the wire can carry.
+
+    ⚠️ THE DEVICE BOUNDS ARE KEYWORD-ONLY AND DEFAULT TO None, WHICH IS THE
+    CONSERVATIVE DIRECTION AND ALSO THE HONEST ONE. A caller that does not know
+    the rating gets a rail measured against `max_w` - correct, because when this
+    add-on has no rating it applies no device clamp either (clamp.py's second
+    stage needs both bounds), so `max_w` really is the ceiling on that path.
     """
     if unchanged_s is None or commanded_w is None or not max_w:
         return False
+    # SAFETY-20. A ceiling of 0 W would make term 2 true of EVERY command; that
+    # is the degenerate case, not the armed one. Silence, and the status page
+    # says NOT COVERED - see rail_unreachable().
+    if rail_unreachable(max_w, dev_min=dev_min, dev_max=dev_max):
+        return False
     return (float(unchanged_s) >= cfg.hold_s
-            and abs(float(commanded_w)) >= rail_w(max_w))
+            and abs(float(commanded_w))
+            >= rail_w(max_w, dev_min=dev_min, dev_max=dev_max))
 
 
 class FreezeWatchdog:
@@ -249,24 +391,37 @@ class FreezeWatchdog:
         self.reason = ""
         self.episodes = 0
 
-    def evaluate(self, unchanged_s, commanded_w, max_w, now: datetime) -> bool:
+    def evaluate(self, unchanged_s, commanded_w, max_w, now: datetime, *,
+                 dev_min=None, dev_max=None) -> bool:
         """Advance one cycle. Returns True ONLY on the rising edge.
 
         `now` is the caller's clock so this stays testable and replayable - the
-        same discipline as `SignWatchdog.evaluate()`.
+        same discipline as `SignWatchdog.evaluate()`. `dev_min`/`dev_max` are
+        the setpoint device's reported range; see looks_frozen().
         """
-        if not looks_frozen(unchanged_s, commanded_w, max_w, self.cfg):
+        if not looks_frozen(unchanged_s, commanded_w, max_w, self.cfg,
+                            dev_min=dev_min, dev_max=dev_max):
             self.active = False
             self.since = None
             self.reason = ""
             return False
 
+        # ⚠️ SAFETY-20. THE REASON QUOTES THE CEILING IT ACTUALLY USED, not
+        # `max_w`. The old line said "at the 5000 W rail (... command over
+        # 4750 W)" on an install whose device stopped at 4700 W, i.e. it named
+        # a threshold the command could not reach - the same untruth the status
+        # page was telling, printed into the log and the HA notification where
+        # it would outlive the page.
+        ceiling = rail_ceiling_w(max_w, dev_min=dev_min, dev_max=dev_max)
+        capped = ("" if ceiling >= abs(float(max_w))
+                  else f", capped from max_w {float(max_w):.0f} W by the "
+                       f"setpoint device's own rating")
         self.reason = (
             f"METER_FROZEN: the meter has answered {float(unchanged_s):.0f} s "
             f"without its measurement changing while the command stands at "
-            f"{float(commanded_w):+.0f} W, at the {float(max_w):.0f} W rail "
+            f"{float(commanded_w):+.0f} W, at the {ceiling:.0f} W rail{capped} "
             f"(limits: unchanged over {self.cfg.hold_s:.0f} s, command over "
-            f"{rail_w(max_w):.0f} W)")
+            f"{rail_w(max_w, dev_min=dev_min, dev_max=dev_max):.0f} W)")
         if self.active:
             return False
         self.active = True