goodwe_controller/CHANGELOG.md

@@ -2,6 +2,58 @@
 
 ## Unreleased
 
+**`PeakStore.record_block()` now has its POLICY ARGUMENT graded, and the
+SAFETY-20 import check now looks at all three ways of writing an import
+(TEST-13).** Two survivors from TARIFF-01's review, both closed in
+`test_tariff.py` only - `app/` is untouched and no shipped behaviour changed.
+
+* **The plumbing of the per-phase reduction was unobservable.** Mutating
+  `record_block()`'s `billed_w(block, policy)` to `billed_w(block, POLICY_NET)`
+  left all 17 suites green (+4 B; recorded by TARIFF-01's reviewer, re-run
+  through the purge harness for TEST-15, and re-run again here). The cause is
+  the third instance of the same shape as WITNESS-02's span normalisation and
+  TARIFF-01's own DST window: **a stimulus evaluated where the quantity it
+  grades is degenerate.** `billed_w()` short-circuits to `block.offtake_avg_w`
+  whenever `per_phase_offtake_avg_w` is absent, and every block that reached a
+  store anywhere in the suite was single-phase or built with
+  `per_phase_offtake_avg_w=None` - so NET, SUM and MAX collapsed onto one
+  number and the argument could not be seen. Section 7 already owns an
+  UNBALANCED fixture, `(3000, 500, -200) W`, whose three reductions are 3300 /
+  3500 / 3000 W; that block is now round-tripped **through the store** under
+  each policy and read back from both tables. NET is the smallest of the three
+  here, so a store that quietly billed NET while asked for MAX would
+  under-record the month's peak - maximum headroom for SAFETY-07, the same
+  under-guarding direction as the `getattr(..., 0.0)` default SAFETY-07 already
+  removed from `project()`.
+* **The import half of the SAFETY-20 grader made a claim it did not check.** It
+  collected `ImportFrom.module` only, which is blind to two of Python's three
+  import spellings: `from . import arbiter as _ab` is an `ImportFrom` whose
+  `.module` is **None** (the module name is in `.names`), and `import
+  app.arbiter` is an `ast.Import` node the walk never visited. Both were added
+  to `app/tariff.py` and the shipped suite stayed **green**. Never a live
+  exposure - the `_names` half still catches any USE of either alias - but the
+  printed sentence "it imports nothing from arbiter, clamp or control" was
+  false for two forms out of three, which is worse than claiming less. The
+  grader now walks `Import` as well, joins `from . import X` onto its base, and
+  matches per dotted SEGMENT so `app.arbiter` cannot hide behind a package
+  prefix while `clamps` / `arbiters` / `controller` are not falsely flagged.
+  Both graders are also now shown non-vacuous against `main.py`, which really
+  does import all three modules.
+* **Two items on the card were already closed and are struck, not
+  re-implemented.** `PeakReading.is_observed` is `self.state == OBSERVED`
+  (`app/tariff.py:266`), already false for `NO_RECORD`; and `project()`'s
+  `held` read is `max(averager.held_net_w, 0.0)` with **no** `getattr` default,
+  fixed by SAFETY-07 in 616b62d with a comment naming TEST-13 item 4. Restoring
+  a default there would re-open a money guard. Removing `clear_partial()` also
+  stays green and that remains a **PASS**: the `quarter_start` PRIMARY KEY plus
+  `ON CONFLICT ... WHERE excluded > existing` makes a replay a structural
+  no-op, which is TARIFF-01's claim rather than a hole in it.
+
+Verified through `tools/mutation_run.sh` (purge, `python3 -B`, tree re-read from
+disk, mutated line grepped off disk, byte delta recorded): 7 mutations, all red
+on the named checks, plus green baselines before and after and a control run
+showing the shipped suite passing every one of them.
+
 **The two `arbitrage()` rungs SAFETY-26's AC4 sweep found pinned from ONE SIDE
 ONLY are now pinned from both - and, the actual finding, are pinned so they stop
 MASKING EACH OTHER (TEST-16).** `spread == 0.0` weakened to `<= 0.001` survived

goodwe_controller/MUTATION.md

@@ -172,7 +172,7 @@ commit and PR bodies.
 | **SAFETY-07** wiring mutations B1/B2/B3 and `test_peakguard.py` parts 7 and 10 (2d570d8, e7c95bf) | **not at risk** | those mutations are **executed in-process**, monkeypatched onto the imported module. No source file is edited, so no `.pyc` is consulted. |
 | every **"all red, no survivors"** claim in the window — TARIFF-02 (24/0), TARIFF-03 (11), MAINT-04 (10), SAFETY-18 (6+4), SAFETY-03 (6), TEL-04 (10), TEL-05 (6) | **not at risk** | direction. Stale bytecode turns a red into a green survivor, never the reverse. A red is evidence that *something* changed under the check. |
 | **TEST-06**'s deleted assertion, "no new notification while parked at 0 W" (c34b5a2) | **not at risk / superseded** | it was recorded as undecidable and then *falsified* — `test_freeze.py` records the one-line mutation that takes the file red 9 times. The stronger later claim is a red one. |
-| **TARIFF-01** reviewer survivor: `record_block()` ignoring its `policy` argument (0dbd485) | **re-verified — still a survivor** | re-run here through `tools/mutation_run.sh` against the 15 suites that were on the record, all green with `billed_w(block, policy)` mutated to `billed_w(block, POLICY_NET)`. The recorded diagnosis holds: every block reaching the store is single-phase, where NET, SUM and MAX collapse. |
+| **TARIFF-01** reviewer survivor: `record_block()` ignoring its `policy` argument (0dbd485) | **re-verified — still a survivor** | re-run here through `tools/mutation_run.sh` against the 15 suites that were on the record, all green with `billed_w(block, policy)` mutated to `billed_w(block, POLICY_NET)`. The recorded diagnosis holds: every block reaching the store is single-phase, where NET, SUM and MAX collapse. **Now CLOSED by TEST-13** (`test_tariff.py` section 7), which round-trips section 7's own unbalanced `(3000, 500, -200) W` block THROUGH the store under each policy - three distinct banked figures, so a hardcoded policy takes at least two of them red. Third instance of "a stimulus evaluated where the quantity it grades is degenerate", after WITNESS-02's span normalisation and TARIFF-01's own DST window. |
 | **SAFETY-25** 8 mutations against `test_peakguard.py` part 10, then 9 more after review, all red | **not at risk** | each run against a tree copied fresh with `__pycache__` excluded and mounted `:ro`, so no `.pyc` existed to go stale - the same structural immunity recorded for CI above. Byte deltas recorded on the PR; none is `+0 B`. All are **reds**, which is the direction this defect cannot fabricate. ⚠️ The second round exists because the first round's mutations were all aimed at DISARMING the guard, and the claim part 10 actually made was about the guard being FIXED - the reviewer built the fix and the suite stayed green. Bytecode was never the issue there; an unaimed battery was. |
 | **SAFETY-03** reviewer survivor: `abs()` dropped from `clamp_to_rating`'s limit (6aabb83) | **re-verified — still a survivor** | re-run the same way, all 15 green. Also as recorded: unreachable under the `int(100, 5000)` schema. |
 | **TARIFF-01**'s other three survivors, **SAFETY-03**'s `describe()` survivor, **TARIFF-02**'s M3/M5/M7, **SAFETY-18**'s three | **still unverified** | recorded before the purge discipline, with no byte delta on the record, so a `+0 B` same-second revert cannot be excluded. All are survivors, which is the direction this defect fabricates. TARIFF-02's and SAFETY-18's were **closed by later commits whose claims are reds**, so what is unverified is the *diagnosis*, not the current state of the code. |

goodwe_controller/test_tariff.py

@@ -681,6 +681,65 @@ check(f"...and on a BALANCED load NET and SUM coincide ({b_vals[POLICY_NET]:.0f}
       f"so a balanced fixture could not have told them apart",
       near(b_vals[POLICY_NET], b_vals[POLICY_SUM], 1e-6))
 
+# --- TEST-13 item 1: the POLICY ARGUMENT'S PLUMBING, not just billed_w() ------
+# `PeakStore.record_block(block, policy)` really does call `billed_w(block,
+# policy)` - the product code is correct and is not what this adds. What was
+# missing is that NO CHECK COULD TELL that call apart from a hardcoded
+# `billed_w(block, POLICY_NET)`. Mutating it to exactly that left all 17 suites
+# green, twice measured (TARIFF-01's reviewer, re-run through the purge harness
+# for TEST-15 and again here).
+#
+# The reason is this section's own subject, one level up. `billed_w()`
+# short-circuits to `block.offtake_avg_w` whenever `per_phase_offtake_avg_w` is
+# absent, and EVERY block that reached a store anywhere in this file was
+# single-phase or built with `per_phase_offtake_avg_w=None` - so all three
+# policies collapsed onto one number and the argument was unobservable at the
+# call site. The checks above grade the FUNCTION on an unbalanced fixture; they
+# say nothing about whether its CALLER forwards what it was handed. That is the
+# same shape as WITNESS-02's span normalisation and TARIFF-01's own DST window:
+# a stimulus evaluated where the quantity it grades is degenerate.
+#
+# So the stimulus here is `blk` - the unbalanced three-phase block from the top
+# of this section, whose three reductions were asserted three lines up to be
+# three DIFFERENT numbers. A `record_block()` that ignores its argument stores
+# one policy's figure under all three names and at least two of these
+# round-trips go red.
+#
+# Which direction the defect runs in, because that is what makes it worth
+# plumbing-testing at all: NET is the SMALLEST of the three on this fixture (the
+# exporting phase nets off against the other two). A store that silently billed
+# NET while its caller asked for MAX would under-record the month's peak, and an
+# under-recorded peak is maximum headroom for SAFETY-07 - the same
+# under-guarding direction as item 4's `getattr(..., 0.0)` default.
+_banked = {}
+for _pol in (POLICY_NET, POLICY_SUM, POLICY_MAX):
+    _st = fresh_store(f"policy-{_pol}")
+    _st.record_block(blk, _pol)
+    _read = _st.peak(month=month_key(blk.start))
+    _derived = _st.recompute_month(month_key(blk.start))
+    _banked[_pol] = _read.peak_w
+    check(f"record_block(block, {_pol!r}) banks {vals[_pol]:.0f} W - the store "
+          f"applies the reduction it was HANDED, not one of its own",
+          _read.is_observed and near(_read.peak_w, vals[_pol]))
+    check(f"...and re-deriving from the 15-min tier gives the same {_pol!r} "
+          f"figure, so BOTH writes in that one transaction used that reduction",
+          _derived.is_observed and near(_derived.peak_w, vals[_pol]))
+
+_st_def = fresh_store("policy-default")
+_st_def.record_block(blk)
+check(f"...and record_block() with NO policy argument banks the NET figure "
+      f"({vals[POLICY_NET]:.0f} W), matching billed_w()'s own shipped default",
+      near(_st_def.peak(month=month_key(blk.start)).peak_w, vals[POLICY_NET]))
+# The anti-degeneracy pin, stated over the BANKED numbers rather than over
+# `vals`, because it is the banked ones this triple is made of. If a future
+# fixture change ever made two of these coincide, the triple above would start
+# passing under a hardcoded policy and would be silently back where it started.
+print(f"    banked through the store: "
+      + ", ".join(f"{k}={v:.0f} W" for k, v in _banked.items()))
+check("the three BANKED figures are three distinct numbers, so this triple is "
+      "not itself degenerate - it can only pass if the argument is read",
+      len({round(v, 6) for v in _banked.values() if v is not None}) == 3)
+
 
 # ==============================================================================
 print()
@@ -976,24 +1035,106 @@ _names |= {n.name for n in _ast.walk(_tree)
 _FORBIDDEN = {"setpoint", "write_setpoint", "target", "Decision", "i_clamped",
               "clamped", "Claim", "resolve", "P_PEAK", "arbiter"}
 _found = sorted(_names & _FORBIDDEN)
-_imports = sorted(m.module or "" for m in _ast.walk(_tree)
-                  if isinstance(m, _ast.ImportFrom))
+
+# ⚠️ THE IMPORT HALF WALKS `Import` TOO, AND READS THE IMPORTED NAMES - TEST-13
+# item 2. It used to be `m.module for m in walk(_tree) if isinstance(m,
+# ImportFrom)`, matched against a literal list of six spellings, and that is
+# blind to TWO OF PYTHON'S THREE import forms:
+#
+#   * `from . import arbiter as _ab` is an `ImportFrom` whose `.module` is
+#     **None** - the dots live in `.level` and the module name lives in
+#     `.names` - so the old grader collected `""` and matched nothing;
+#   * `import app.arbiter` is an `ast.Import` node, a different class the walk
+#     filtered out entirely and therefore never looked at.
+#
+# Neither was a live exposure: the `_names` half above catches any USE of either
+# alias, because reaching anything through it produces an `Attribute` whose
+# `.attr` is a forbidden name. The defect was in the SENTENCE. A check that
+# prints "it imports nothing from arbiter, clamp or control" while verifying
+# that for one spelling out of three is worse than one that claims less, because
+# the next reader stops looking. So: collect every module any import statement
+# names, in all three forms, and match per dotted SEGMENT - `app.arbiter` and
+# `.arbiter` must not be able to hide behind a package prefix either.
+_FORBIDDEN_MODULES = {"arbiter", "clamp", "control"}
+
+
+def _imported_modules(tree):
+    """Every module path an import statement in `tree` names, all three forms."""
+    out = set()
+    for node in _ast.walk(tree):
+        if isinstance(node, _ast.Import):
+            # `import app.arbiter`, `import app.arbiter as x` - the whole
+            # dotted path is the module.
+            out |= {a.name for a in node.names}
+        elif isinstance(node, _ast.ImportFrom):
+            base = node.module or ""      # None for `from . import arbiter`
+            if base:
+                out.add(base)             # `from .arbiter import resolve`
+            # ...and in the `module is None` form the MODULE is in .names, so
+            # the names have to be joined on whether or not there is a base.
+            out |= {f"{base}.{a.name}" if base else a.name for a in node.names}
+    return out
+
+
+def _forbidden_imports(mods):
+    # Segment-wise, not substring: `arbitrage` contains no forbidden segment and
+    # must not be flagged, while `app.arbiter` must be.
+    return sorted(m for m in mods
+                  if _FORBIDDEN_MODULES & {p for p in m.split(".") if p})
+
+
+_imports = sorted(_imported_modules(_tree))
 print(f"    tariff.py evaluates {len(_names)} distinct names and imports "
-      f"from {_imports or ['(stdlib only)']}")
+      f"{sorted({m.split('.')[0] for m in _imports}) or ['(nothing)']}")
 check(f"none of them is a command-side name {sorted(_FORBIDDEN)} - the utility "
       f"bills what FLOWED, so this reads the meter and nothing else"
       + (f"; found {_found}" if _found else ""), not _found)
-check("...and it imports nothing from arbiter, clamp or control: it files no "
-      "claim, applies no clamp, and commands nothing - SAFETY-07's job",
-      not [m for m in _imports if m in ("arbiter", "clamp", "control",
-                                        ".arbiter", ".clamp", ".control")])
-# ...and the guard is not vacuous: the same grader run over main.py, which DOES
-# command, must light up.
-_main = {n.attr for n in _ast.walk(
-    _ast.parse((HERE / "app" / "main.py").read_text(encoding="utf-8")))
-    if isinstance(n, _ast.Attribute)}
-check(f"the same grader over main.py finds {sorted(_main & _FORBIDDEN)}, so it "
-      f"is capable of failing", bool(_main & _FORBIDDEN))
+check("...and it imports nothing from arbiter, clamp or control in ANY of the "
+      "three import spellings: it files no claim, applies no clamp, and "
+      "commands nothing - SAFETY-07's job",
+      not _forbidden_imports(_imports))
+
+# The import grader graded. Every one of these is a real way to reach the
+# command side from this module, and the first two are the ones the previous
+# version let through; the last two it already caught and must keep catching.
+_EVASIONS = ["from .arbiter import resolve",
+             "from . import arbiter as _ab",
+             "import app.arbiter",
+             "from app.control import compute",
+             "import app.clamp as _c"]
+_missed = [e for e in _EVASIONS
+           if not _forbidden_imports(_imported_modules(_ast.parse(e)))]
+check(f"...and that import grader is not spelling-blind: all {len(_EVASIONS)} "
+      f"ways of importing the command side are caught"
+      + (f"; MISSED {_missed}" if _missed else ""), not _missed)
+# ...and it is not simply flagging everything, which would make the line above
+# free. The last three are the reason the match is per SEGMENT and not `f in m`:
+# `clamps`, `arbiters` and `controller` all CONTAIN a forbidden word and none of
+# them is the command side. `controller` is not hypothetical - this package is
+# `goodwe_controller`, and a substring grader pointed at a path rather than a
+# module name would refuse the whole repository.
+_INNOCENT = ["import json", "from datetime import datetime, timezone",
+             "from . import arbitrage", "from .price import Uncertainty",
+             "from zoneinfo import ZoneInfo",
+             "import clamps", "from .arbiters import x",
+             "from . import controller"]
+_false = [i for i in _INNOCENT
+          if _forbidden_imports(_imported_modules(_ast.parse(i)))]
+check("...and it does not fire on innocent imports whose names merely CONTAIN a "
+      "forbidden word (`clamps`, `arbiters`, `controller`) - a substring match "
+      "would have"
+      + (f"; FALSELY FLAGGED {_false}" if _false else ""), not _false)
+
+# ...and neither half is vacuous: the same two graders run over main.py, which
+# DOES command, must both light up. main.py carries all three import forms.
+_main_tree = _ast.parse((HERE / "app" / "main.py").read_text(encoding="utf-8"))
+_main = {n.attr for n in _ast.walk(_main_tree) if isinstance(n, _ast.Attribute)}
+_main_imports = _forbidden_imports(_imported_modules(_main_tree))
+check(f"the same NAME grader over main.py finds {sorted(_main & _FORBIDDEN)}, "
+      f"so it is capable of failing", bool(_main & _FORBIDDEN))
+check(f"...and the same IMPORT grader over main.py finds {_main_imports}, so "
+      f"it is capable of failing on a real module and not only on a fixture",
+      bool(_main_imports))
 
 
 # ==============================================================================