self.i_w — the grid-following loop's integrator — has been a separate quantity from the command since SAFETY-04, carried between cycles by Controller.cycle(), and written to no entity, no MQTT topic, no attribute and no log line. This branch publishes it, as three site-level rows:
entity
what it is
Loop integrator
the accumulator after this cycle — before the output clamp, the slew limit and the 10 W quantisation
Loop integrator lead
that value minus the command it was computed against (the command the previous cycle left standing)
Loop reason
compute()'s own word for this cycle: tracking, deadband, i-clamped, clamped, slew-limited, saturated-freeze, freeze-released, not-running
⚠️app/control.py is not touched by this branch.compute(), its returned Decision and every constant it reads are byte-identical to release/1.0. The diff is a controller recording three values it already had, and a publisher announcing three rows. Nothing here reached the inverter, and nothing here can.
Why it is worth three entities on a live install
Loop reason is a genuinely new sentence, not a second copy of one. The status page's Why row and sensor.goodwe_controller_status both carry the arbiter's line (loop -> -5000 W), which prints the winning claim's NAME. compute()'s own reason has never left the process — and slew-limited, the value the stacked SAFETY-48 PR is entirely about, appears in neither of the other two.
And the cost of not having it is already paid twice. The investigation behind SAFETY-48 needed to know what the integrator was doing during the export episodes and could only reconstruct it from the grid trace plus a replay of the real compute(). The reconstruction reproduced about a third of the engagements it was checked against and was discarded entirely.
⚠️This is the ticket that makes SAFETY-52's and SAFETY-50's open questions answerable, and only deploying it does that. Both are about what the loop's accumulator does in situations that are not reproducible on demand. The integrator is still published nowhere, so both investigations have had to infer it from the grid trace — which is the method that has already produced one discarded reconstruction. Merging this branch does not answer them; a deploy that starts recording the three rows does. Deploying is not part of this PR and needs its own explicit go.
Three, not one per inverter. This is an addition to a live install's entity list, which TEL-07 refused to make for four per-unit rows at a roster of one. §5 of the decision record carries both halves of that argument — why the addition is made here, and why it stays at three site rows rather than scaling with the roster.
What a reviewer should look hardest at
⚠️The row that matters is the one that is usually indistinguishable from Setpoint. With nothing binding, the command is the integrator rounded to 10 W, so on a quiet day publishing the wrong one looks correct. test_telemetry.py therefore refuses to assert that a row exists: every check drives real cycles through the real Controller.cycle() until the two quantities are measurably different, and grades the published number at a tick where reading the wrong one gives the wrong answer.
That discipline caught two of this file's own first-draft checks, both MUTATION.md habit 4:
"the published integrator reaches its bound" was GREEN under the card's own named mutation (self.loop_i_w = loop_prev, publish the COMMAND under the integrator's name) — because the command reaches the same bound too, twelve cycles later. The check now asserts the lead time and a plateau, not the value.
"a stopped controller publishes not-running" was GREEN under deleting that branch's assignments outright, because the fixture started stopped and __init__ writes the same values. It now stops the controller part-way through a run that has already wound the integrator to −5000 W, so a row left standing is a WRONG row and not merely an unset one.
The full mutation battery (M1–M9, with byte deltas and red counts) is in test_telemetry.py's header. M2 is reported as a real survivor rather than quietly dropped.
MQTT compatibility
test_mqtt.py part 1 pins release/1.0's announcement byte-for-byte against a recorded BASELINE that nothing in this branch may add to. The eight entity configs are unchanged and still first; availability is byte-identical and deliberately moves to last (a broker replaying the retained set must see availability arrive after the entities it is the availability of); the three new rows land between them. Both facts are separate checks, so a fourth row cannot hide inside either.
An independent review ran off-PR, and its findings are in b6c6da3
This branch was reviewed before a PR existed, so that history is not in this thread. Two findings landed here; they are in the second commit so they can be re-checked rather than rediscovered.
R6 — a headline that named an assertion the file does not make. Part 1 said "every one of release/1.0's NINE messages is still byte-identical and still first". The assertion under it is announce(...)[:len(BASELINE) - 1] == BASELINE[:-1], which covers eight: the ninth, availability, is byte-identical but deliberately moves to last, so a prefix comparison cannot include it. Nothing was unguarded — the == SITE_NOW on the next check pins it in its new position — but the sentence was not the check. Found while fixing: part 3 carried the same mis-statement two lines from a check that is about all nine; it is corrected the same way, and nobody had asked for that one.
R5 — a mutation count that was decoration. M7's red count was inherited rather than re-measured. Re-driven in the CI image on a cp -a of the tree, purged, :ro, one file at a time, it is 10 reds in test_mqtt.py, not 8 — on this tree and on SAFETY-48's alike. The direction was right; the number was not, which is MUTATION.md's reporting rule 3. A note beside the table now says that a red count is a property of the suite as much as of the mutation and therefore has to name a tree.
The other five review findings are all in the stacked SAFETY-48 PR.
Suite state
Full suite in the CI image, purged tree, PYTHONDONTWRITEBYTECODE=1, :ro mount, one file at a time — the exact loop from .gitea/workflows/checks.yml — run on each branch separately:
branch
result
release/1.0 @ 68c5ede
29/29 PASS, exit 0
TEL-10 @ b6c6da3 (this PR)
30/30 PASS, exit 0
SAFETY-48 @ a6af332 (stacked)
30/30 PASS, exit 0
No deploy, no ha addons update, no writes to /addons/, and no contact of any kind with the live box or the inverter.
## What this does
`self.i_w` — the grid-following loop's integrator — has been a separate quantity from the command since SAFETY-04, carried between cycles by `Controller.cycle()`, and written to **no entity, no MQTT topic, no attribute and no log line**. This branch publishes it, as three site-level rows:
| entity | what it is |
|---|---|
| `Loop integrator` | the accumulator **after** this cycle — before the output clamp, the slew limit and the 10 W quantisation |
| `Loop integrator lead` | that value minus **the command it was computed against** (the command the *previous* cycle left standing) |
| `Loop reason` | `compute()`'s own word for this cycle: `tracking`, `deadband`, `i-clamped`, `clamped`, `slew-limited`, `saturated-freeze`, `freeze-released`, `not-running` |
⚠️ **`app/control.py` is not touched by this branch.** `compute()`, its returned `Decision` and every constant it reads are byte-identical to `release/1.0`. The diff is a controller recording three values it already had, and a publisher announcing three rows. Nothing here reached the inverter, and nothing here can.
## Why it is worth three entities on a live install
**`Loop reason` is a genuinely new sentence, not a second copy of one.** The status page's `Why` row and `sensor.goodwe_controller_status` both carry the *arbiter's* line (`loop -> -5000 W`), which prints the winning claim's NAME. `compute()`'s own reason has never left the process — and `slew-limited`, the value the stacked SAFETY-48 PR is entirely about, appears in neither of the other two.
**And the cost of not having it is already paid twice.** The investigation behind SAFETY-48 needed to know what the integrator was doing during the export episodes and could only **reconstruct** it from the grid trace plus a replay of the real `compute()`. The reconstruction reproduced about a third of the engagements it was checked against and was discarded entirely.
⚠️ **This is the ticket that makes SAFETY-52's and SAFETY-50's open questions answerable, and only deploying it does that.** Both are about what the loop's accumulator does in situations that are not reproducible on demand. The integrator is still published nowhere, so both investigations have had to infer it from the grid trace — which is the method that has already produced one discarded reconstruction. Merging this branch does not answer them; a deploy that starts recording the three rows does. Deploying is **not** part of this PR and needs its own explicit go.
**Three, not one per inverter.** This is an addition to a live install's entity list, which TEL-07 refused to make for four per-unit rows at a roster of one. §5 of the decision record carries both halves of that argument — why the addition is made here, and why it stays at three site rows rather than scaling with the roster.
## What a reviewer should look hardest at
⚠️ **The row that matters is the one that is usually indistinguishable from `Setpoint`.** With nothing binding, the command *is* the integrator rounded to 10 W, so on a quiet day publishing the wrong one looks correct. `test_telemetry.py` therefore refuses to assert that a row *exists*: every check drives real cycles through the real `Controller.cycle()` until the two quantities are measurably different, and grades the published number at a tick where reading the wrong one gives the wrong answer.
That discipline caught two of this file's own first-draft checks, both MUTATION.md habit 4:
* *"the published integrator reaches its bound"* was **GREEN** under the card's own named mutation (`self.loop_i_w = loop_prev`, publish the COMMAND under the integrator's name) — because the command reaches the same bound too, twelve cycles later. The check now asserts the **lead time and a plateau**, not the value.
* *"a stopped controller publishes `not-running`"* was **GREEN** under deleting that branch's assignments outright, because the fixture started stopped and `__init__` writes the same values. It now stops the controller part-way through a run that has already wound the integrator to −5000 W, so a row left standing is a WRONG row and not merely an unset one.
The full mutation battery (M1–M9, with byte deltas and red counts) is in `test_telemetry.py`'s header. **M2 is reported as a real survivor** rather than quietly dropped.
## MQTT compatibility
`test_mqtt.py` part 1 pins release/1.0's announcement byte-for-byte against a recorded `BASELINE` that nothing in this branch may add to. The eight entity configs are unchanged and still first; `availability` is byte-identical and deliberately moves to last (a broker replaying the retained set must see availability arrive after the entities it is the availability *of*); the three new rows land between them. Both facts are separate checks, so a fourth row cannot hide inside either.
---
## An independent review ran off-PR, and its findings are in `b6c6da3`
This branch was reviewed before a PR existed, so that history is not in this thread. Two findings landed here; they are in the second commit so they can be re-checked rather than rediscovered.
* **R6 — a headline that named an assertion the file does not make.** Part 1 said *"every one of release/1.0's NINE messages is still byte-identical and still first"*. The assertion under it is `announce(...)[:len(BASELINE) - 1] == BASELINE[:-1]`, which covers **eight**: the ninth, `availability`, is byte-identical but deliberately moves to last, so a prefix comparison cannot include it. Nothing was unguarded — the `== SITE_NOW` on the next check pins it in its new position — but the sentence was not the check. **Found while fixing:** part 3 carried the same mis-statement two lines from a check that *is* about all nine; it is corrected the same way, and nobody had asked for that one.
* **R5 — a mutation count that was decoration.** M7's red count was inherited rather than re-measured. Re-driven in the CI image on a `cp -a` of the tree, purged, `:ro`, one file at a time, it is **10 reds in `test_mqtt.py`, not 8** — on this tree and on SAFETY-48's alike. The direction was right; the number was not, which is MUTATION.md's reporting rule 3. A note beside the table now says that a red count is a property of the **suite** as much as of the mutation and therefore has to name a tree.
The other five review findings are all in the stacked SAFETY-48 PR.
## Suite state
Full suite in the CI image, purged tree, `PYTHONDONTWRITEBYTECODE=1`, `:ro` mount, one file at a time — the exact loop from `.gitea/workflows/checks.yml` — run on each branch separately:
| branch | result |
|---|---|
| `release/1.0` @ `68c5ede` | **29/29 PASS, exit 0** |
| **`TEL-10` @ `b6c6da3` (this PR)** | **30/30 PASS, exit 0** |
| `SAFETY-48` @ `a6af332` (stacked) | **30/30 PASS, exit 0** |
No deploy, no `ha addons update`, no writes to `/addons/`, and no contact of any kind with the live box or the inverter.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
`self.i_w` has been a separate quantity from the command since SAFETY-04 and
has left the process nowhere at all - no entity, no MQTT attribute, no log
line. The investigation behind SAFETY-48 could only RECONSTRUCT it from the
grid trace plus a replay; the reconstruction reproduced about a third of the
engagements it was checked against and was discarded, so the card it was
gathered for had to be argued from a measurement nobody could stand behind.
It could not have been made to work either: `i_w` depends on every cycle
before it, including the ones where the arbiter overrode the loop and
`main.py` re-seeded `self.i_w = self.target`, and that re-seed is invisible in
the grid trace.
THREE SITE ROWS, APPENDED AFTER release/1.0's EIGHT. `loop_i` is
`Decision.i_w` - the accumulator after the cycle, before the output clamp, the
slew limit and quantisation. `loop_lead` is `decision.i_w - prev_w` against the
command `compute()` was HANDED, which is not a subtraction of the other two
published rows: `Setpoint` is the arbiter's output and is this cycle's, so on
any cycle the arbiter overrode the loop the two differ - and that is exactly
the cycle on which the integrator was re-seeded. `loop_reason` is
`compute()`'s own word; the arbiter's `explain()` prints the winning claim's
NAME and has never carried the claim's reason, so `slew-limited` appears in
neither the `Why` row nor `Controller status`. Measured on the same cycle:
arbiter `'loop -> -5000 W'`, loop `'i-clamped'`.
NOTHING IN THE CONTROL PATH MOVED. `app/control.py` is not in this diff.
`compute()`, its returned `Decision` and every constant it reads are identical
to release/1.0, and all nine of release/1.0's retained discovery messages are
byte-identical and still FIRST in the announcement.
Evidence: 30/30 suites green in the CI image. Nine mutations, ONE survivor,
reported as one: `decision.i_w - self.target` for the lead is +2 B and green
because `cycle()` has not reassigned `self.target` at that line - the name is
kept for the coupling, not for a behaviour it buys, and M2b (+8 B) is the
mutation that does change the value and is red.
AND TWO OF THIS CARD'S OWN CHECKS COULD NOT REACH THE FAILURE THEY NAMED.
"the published integrator reaches its bound" was GREEN under the card's named
mutation (publish `prev_w`, -3 B), because the command reaches the bound too -
twelve cycles later. It asserts the LEAD TIME now and is red on 5. "a stopped
controller publishes not-running" was GREEN under deleting that branch's
assignments (-45 B, -67 B), because the fixture STARTED stopped and `__init__`
writes the same values; it stops the controller part-way through a run that
has already wound the integrator to -5000 W. Both are MUTATION.md habit 4,
found by the battery rather than by review.
No CHANGELOG section: `test_version_gate.py` 11b refuses a newest `## `
heading that is not config.yaml's `version:`, and opening one would mean
taking a packaging decision this card does not get to take. The operator
account is in DOCS.md and the release note belongs to the next PKG ticket.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Both are review findings on the merged-in-spirit branch, and neither is in
executable code.
R6 - test_mqtt.py part 1's headline said "every one of release/1.0's NINE
messages is still byte-identical and still first". The assertion under it is
`announce(...)[:len(BASELINE) - 1] == BASELINE[:-1]`, which covers EIGHT: the
ninth, `goodwe_ctl/availability`, is byte-identical but deliberately moves to
last, so a prefix comparison cannot include it. Nothing was unguarded - the
`== SITE_NOW` on the next check pins it in its new position - but the sentence
was not the check. Part 3 carried the same mis-statement two lines from a check
that IS about all nine, and is corrected the same way.
R5 - M7's red count was decoration. Re-driven in the CI image on a `cp -a` of
the tree, purged, `:ro`, one file at a time, it is 10 reds in test_mqtt.py and
not 8, on this tree and on SAFETY-48's alike. The direction was right; the
number was inherited rather than re-measured, which is MUTATION.md's reporting
rule 3. A note beside the table now says that a red count is a property of the
SUITE as much as of the mutation and therefore has to name a tree.
Verified: the full suite in the CI image, purged tree, `:ro` mount, one file at
a time, exactly the command in .gitea/workflows/checks.yml - 30/30, exit 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Re-run independently on a fresh clone. b6c6da3 confirmed against git ls-remote before anything else. Every suite run below is checks.yml's exact loop — CI image built from .gitea/ci/Dockerfile, purged tree, PYTHONDONTWRITEBYTECODE=1, :ro mount, one file at a time — and, like CI-02, on the merge of the head into the base rather than on the branch tip.
1. "app/control.py is byte-identical" — checked mechanically, not read
ref
blob
sha256 of contents
release/1.0
180f58a
58519c6346ac7871…
TEL-10
180f58a
58519c6346ac7871…
Same blob id, same content hash, and git diff release/1.0 TEL-10 -- goodwe_controller/app/control.py is empty. release/1.0 is a direct ancestor of TEL-10 (merge-base = 68c5ede = the base tip), so this is a fast-forward and there is no merge-base divergence for a change to hide behind.
The claim is broader than one file, so I checked the rest of the diff for anything that reaches the decision path:
app/main.py's only executable edit inside cycle() is prev_w=self.target → loop_prev = self.target / prev_w=loop_prev. Same value, same call, same arguments.
There are three write sites for self.i_w on this branch and only two have a telemetry counterpart. I chased the third before concluding: self.i_w = self.target at app/main.py:3049 is the arbiter re-seed, it sits inside the same if (self.auto or self.shadow) and self.inputs_ok: branch, and compute() runs immediately after it — so it cannot leave loop_i_w holding a previous cycle's value. Not a defect; recording it because it is the one place a reader would have to check to believe the claim.
app/mqtt.pyappends to SENSORS, and discovery_payloads() walks the list in order, so every existing entity keeps its index. publish() drops None; loop_reason is always a string and the other two are always floats, so none of the three can be silently dropped.
Nothing in this diff writes to an inverter.
2. Suite, run by me
tree
result
release/1.0 @ 68c5ede (bare)
29/29 PASS, exit 0
TEL-10 merged into release/1.0
30/30 PASS, exit 0
SAFETY-48 merged into release/1.0
30/30 PASS, exit 0
SAFETY-48 merged into TEL-10
30/30 PASS, exit 0
Matches the PR's table.
3. Mutation spot-check — re-applied on purged cp -a trees, one file at a time
The previous round's reported counts were irreproducible, so I re-drove four of this card's battery myself rather than reading the table. Every claimed count is exact, including the byte deltas and the tree-dependence.
#
byte delta measured
reds measured
claimed
M1 on TEL-10
−3 B
5 in test_telemetry.py
5 ✓
M1 on SAFETY-48
−3 B
4 in test_telemetry.py
4 ✓
M2
+2 B
GREEN — survivor
survivor ✓
M7
−96 B
10 in test_mqtt.py + 1 in test_telemetry.py
10+1 ✓
M9
+0 B
7 in test_mqtt.py
7 ✓
Two notes a re-runner will want:
M1's 5→4 difference between the two trees is real and is exactly the reason the header gives — SAFETY-48 replaced the "the gap is the slew limit and not an accident" check with one written against the bound. I saw both lists; the missing red is that check and nothing else.
M7 does not print a count. Deleting the loop_i row from SENSORS also removes test_mqtt.py part 8's own mutation anchor, so the file aborts with AssertionError: mutation anchor not uniqueafter its named checks have already reported. Counting the FAIL lines gives exactly 10. The claim is right; whoever re-runs it should expect a traceback rather than the summary line.
4. R6's correction is true, not merely made
The risk with a corrected claim is that it is still wrong. It is not:
Part 1's headline now says EIGHT, and the assertion under it is announce(...)[:len(BASELINE) - 1] == BASELINE[:-1] — eight entity configs. The ninth is named in the same sentence as byte-identical-but-moved-to-last, and the == SITE_NOW check below does pin it.
Part 3's AC 1 … NINE MESSAGES headline is backed by (m, c) == ([], []) — a full missing/changed diff, which genuinely does cover all nine. That one was already correct.
The unasked-for fix two lines down now reads "A's eight entity configs are still the FIRST eight" against the same eight-prefix slice.
Each sentence now names the assertion it actually makes. M9 (+0 B, pure reordering, no payload change) going red on 7 checks is what demonstrates the ordering claim is graded rather than assumed.
Verdict: clean. Merging.#93 will be retargeted to release/1.0 afterwards so that checks / unit (pull_request) actually runs on it.
## Review — verified, no blocking findings
Re-run independently on a fresh clone. `b6c6da3` confirmed against `git ls-remote` before anything else. Every suite run below is `checks.yml`'s exact loop — CI image built from `.gitea/ci/Dockerfile`, purged tree, `PYTHONDONTWRITEBYTECODE=1`, `:ro` mount, one file at a time — and, like CI-02, on the **merge** of the head into the base rather than on the branch tip.
### 1. "`app/control.py` is byte-identical" — checked mechanically, not read
| ref | blob | sha256 of contents |
|---|---|---|
| `release/1.0` | `180f58a` | `58519c6346ac7871…` |
| `TEL-10` | `180f58a` | `58519c6346ac7871…` |
Same blob id, same content hash, and `git diff release/1.0 TEL-10 -- goodwe_controller/app/control.py` is empty. `release/1.0` is a **direct ancestor** of `TEL-10` (`merge-base` = `68c5ede` = the base tip), so this is a fast-forward and there is no merge-base divergence for a change to hide behind.
The claim is broader than one file, so I checked the rest of the diff for anything that reaches the decision path:
* `app/main.py`'s only executable edit inside `cycle()` is `prev_w=self.target` → `loop_prev = self.target` / `prev_w=loop_prev`. Same value, same call, same arguments.
* There are **three** write sites for `self.i_w` on this branch and only two have a telemetry counterpart. I chased the third before concluding: `self.i_w = self.target` at `app/main.py:3049` is the arbiter re-seed, it sits *inside* the same `if (self.auto or self.shadow) and self.inputs_ok:` branch, and `compute()` runs immediately after it — so it cannot leave `loop_i_w` holding a previous cycle's value. Not a defect; recording it because it is the one place a reader would have to check to believe the claim.
* `app/mqtt.py` **appends** to `SENSORS`, and `discovery_payloads()` walks the list in order, so every existing entity keeps its index. `publish()` drops `None`; `loop_reason` is always a string and the other two are always floats, so none of the three can be silently dropped.
Nothing in this diff writes to an inverter.
### 2. Suite, run by me
| tree | result |
|---|---|
| `release/1.0` @ `68c5ede` (bare) | **29/29 PASS, exit 0** |
| `TEL-10` merged into `release/1.0` | **30/30 PASS, exit 0** |
| `SAFETY-48` merged into `release/1.0` | **30/30 PASS, exit 0** |
| `SAFETY-48` merged into `TEL-10` | **30/30 PASS, exit 0** |
Matches the PR's table.
### 3. Mutation spot-check — re-applied on purged `cp -a` trees, one file at a time
The previous round's reported counts were irreproducible, so I re-drove four of this card's battery myself rather than reading the table. **Every claimed count is exact, including the byte deltas and the tree-dependence.**
| # | byte delta measured | reds measured | claimed |
|---|---|---|---|
| M1 on `TEL-10` | −3 B | 5 in `test_telemetry.py` | 5 ✓ |
| M1 on `SAFETY-48` | −3 B | 4 in `test_telemetry.py` | 4 ✓ |
| M2 | +2 B | **GREEN — survivor** | survivor ✓ |
| M7 | −96 B | 10 in `test_mqtt.py` + 1 in `test_telemetry.py` | 10+1 ✓ |
| M9 | **+0 B** | 7 in `test_mqtt.py` | 7 ✓ |
Two notes a re-runner will want:
* M1's 5→4 difference between the two trees is real and is exactly the reason the header gives — SAFETY-48 replaced the "the gap is the slew limit and not an accident" check with one written against the bound. I saw both lists; the missing red is that check and nothing else.
* M7 does not print a count. Deleting the `loop_i` row from `SENSORS` also removes `test_mqtt.py` part 8's own mutation anchor, so the file aborts with `AssertionError: mutation anchor not unique` **after** its named checks have already reported. Counting the `FAIL` lines gives exactly 10. The claim is right; whoever re-runs it should expect a traceback rather than the summary line.
### 4. R6's correction is true, not merely made
The risk with a corrected claim is that it is still wrong. It is not:
* Part 1's headline now says **EIGHT**, and the assertion under it is `announce(...)[:len(BASELINE) - 1] == BASELINE[:-1]` — eight entity configs. The ninth is named in the same sentence as byte-identical-but-moved-to-last, and the `== SITE_NOW` check below does pin it.
* Part 3's `AC 1 … NINE MESSAGES` headline is backed by `(m, c) == ([], [])` — a full missing/changed diff, which genuinely does cover all nine. That one was already correct.
* The unasked-for fix two lines down now reads "A's **eight** entity configs are still the FIRST eight" against the same eight-prefix slice.
Each sentence now names the assertion it actually makes. M9 (+0 B, pure reordering, no payload change) going red on 7 checks is what demonstrates the ordering claim is graded rather than assumed.
---
**Verdict: clean. Merging.** `#93` will be retargeted to `release/1.0` afterwards so that `checks / unit (pull_request)` actually runs on it.
admin
merged commit 660d079bf3 into release/1.02026-09-19 10:54:03 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
What this does
self.i_w— the grid-following loop's integrator — has been a separate quantity from the command since SAFETY-04, carried between cycles byController.cycle(), and written to no entity, no MQTT topic, no attribute and no log line. This branch publishes it, as three site-level rows:Loop integratorLoop integrator leadLoop reasoncompute()'s own word for this cycle:tracking,deadband,i-clamped,clamped,slew-limited,saturated-freeze,freeze-released,not-running⚠️
app/control.pyis not touched by this branch.compute(), its returnedDecisionand every constant it reads are byte-identical torelease/1.0. The diff is a controller recording three values it already had, and a publisher announcing three rows. Nothing here reached the inverter, and nothing here can.Why it is worth three entities on a live install
Loop reasonis a genuinely new sentence, not a second copy of one. The status page'sWhyrow andsensor.goodwe_controller_statusboth carry the arbiter's line (loop -> -5000 W), which prints the winning claim's NAME.compute()'s own reason has never left the process — andslew-limited, the value the stacked SAFETY-48 PR is entirely about, appears in neither of the other two.And the cost of not having it is already paid twice. The investigation behind SAFETY-48 needed to know what the integrator was doing during the export episodes and could only reconstruct it from the grid trace plus a replay of the real
compute(). The reconstruction reproduced about a third of the engagements it was checked against and was discarded entirely.⚠️ This is the ticket that makes SAFETY-52's and SAFETY-50's open questions answerable, and only deploying it does that. Both are about what the loop's accumulator does in situations that are not reproducible on demand. The integrator is still published nowhere, so both investigations have had to infer it from the grid trace — which is the method that has already produced one discarded reconstruction. Merging this branch does not answer them; a deploy that starts recording the three rows does. Deploying is not part of this PR and needs its own explicit go.
Three, not one per inverter. This is an addition to a live install's entity list, which TEL-07 refused to make for four per-unit rows at a roster of one. §5 of the decision record carries both halves of that argument — why the addition is made here, and why it stays at three site rows rather than scaling with the roster.
What a reviewer should look hardest at
⚠️ The row that matters is the one that is usually indistinguishable from
Setpoint. With nothing binding, the command is the integrator rounded to 10 W, so on a quiet day publishing the wrong one looks correct.test_telemetry.pytherefore refuses to assert that a row exists: every check drives real cycles through the realController.cycle()until the two quantities are measurably different, and grades the published number at a tick where reading the wrong one gives the wrong answer.That discipline caught two of this file's own first-draft checks, both MUTATION.md habit 4:
self.loop_i_w = loop_prev, publish the COMMAND under the integrator's name) — because the command reaches the same bound too, twelve cycles later. The check now asserts the lead time and a plateau, not the value.not-running" was GREEN under deleting that branch's assignments outright, because the fixture started stopped and__init__writes the same values. It now stops the controller part-way through a run that has already wound the integrator to −5000 W, so a row left standing is a WRONG row and not merely an unset one.The full mutation battery (M1–M9, with byte deltas and red counts) is in
test_telemetry.py's header. M2 is reported as a real survivor rather than quietly dropped.MQTT compatibility
test_mqtt.pypart 1 pins release/1.0's announcement byte-for-byte against a recordedBASELINEthat nothing in this branch may add to. The eight entity configs are unchanged and still first;availabilityis byte-identical and deliberately moves to last (a broker replaying the retained set must see availability arrive after the entities it is the availability of); the three new rows land between them. Both facts are separate checks, so a fourth row cannot hide inside either.An independent review ran off-PR, and its findings are in
b6c6da3This branch was reviewed before a PR existed, so that history is not in this thread. Two findings landed here; they are in the second commit so they can be re-checked rather than rediscovered.
announce(...)[:len(BASELINE) - 1] == BASELINE[:-1], which covers eight: the ninth,availability, is byte-identical but deliberately moves to last, so a prefix comparison cannot include it. Nothing was unguarded — the== SITE_NOWon the next check pins it in its new position — but the sentence was not the check. Found while fixing: part 3 carried the same mis-statement two lines from a check that is about all nine; it is corrected the same way, and nobody had asked for that one.cp -aof the tree, purged,:ro, one file at a time, it is 10 reds intest_mqtt.py, not 8 — on this tree and on SAFETY-48's alike. The direction was right; the number was not, which is MUTATION.md's reporting rule 3. A note beside the table now says that a red count is a property of the suite as much as of the mutation and therefore has to name a tree.The other five review findings are all in the stacked SAFETY-48 PR.
Suite state
Full suite in the CI image, purged tree,
PYTHONDONTWRITEBYTECODE=1,:romount, one file at a time — the exact loop from.gitea/workflows/checks.yml— run on each branch separately:release/1.0@68c5edeTEL-10@b6c6da3(this PR)SAFETY-48@a6af332(stacked)No deploy, no
ha addons update, no writes to/addons/, and no contact of any kind with the live box or the inverter.🤖 Generated with Claude Code
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Review — verified, no blocking findings
Re-run independently on a fresh clone.
b6c6da3confirmed againstgit ls-remotebefore anything else. Every suite run below ischecks.yml's exact loop — CI image built from.gitea/ci/Dockerfile, purged tree,PYTHONDONTWRITEBYTECODE=1,:romount, one file at a time — and, like CI-02, on the merge of the head into the base rather than on the branch tip.1. "
app/control.pyis byte-identical" — checked mechanically, not readrelease/1.0180f58a58519c6346ac7871…TEL-10180f58a58519c6346ac7871…Same blob id, same content hash, and
git diff release/1.0 TEL-10 -- goodwe_controller/app/control.pyis empty.release/1.0is a direct ancestor ofTEL-10(merge-base=68c5ede= the base tip), so this is a fast-forward and there is no merge-base divergence for a change to hide behind.The claim is broader than one file, so I checked the rest of the diff for anything that reaches the decision path:
app/main.py's only executable edit insidecycle()isprev_w=self.target→loop_prev = self.target/prev_w=loop_prev. Same value, same call, same arguments.self.i_won this branch and only two have a telemetry counterpart. I chased the third before concluding:self.i_w = self.targetatapp/main.py:3049is the arbiter re-seed, it sits inside the sameif (self.auto or self.shadow) and self.inputs_ok:branch, andcompute()runs immediately after it — so it cannot leaveloop_i_wholding a previous cycle's value. Not a defect; recording it because it is the one place a reader would have to check to believe the claim.app/mqtt.pyappends toSENSORS, anddiscovery_payloads()walks the list in order, so every existing entity keeps its index.publish()dropsNone;loop_reasonis always a string and the other two are always floats, so none of the three can be silently dropped.Nothing in this diff writes to an inverter.
2. Suite, run by me
release/1.0@68c5ede(bare)TEL-10merged intorelease/1.0SAFETY-48merged intorelease/1.0SAFETY-48merged intoTEL-10Matches the PR's table.
3. Mutation spot-check — re-applied on purged
cp -atrees, one file at a timeThe previous round's reported counts were irreproducible, so I re-drove four of this card's battery myself rather than reading the table. Every claimed count is exact, including the byte deltas and the tree-dependence.
TEL-10test_telemetry.pySAFETY-48test_telemetry.pytest_mqtt.py+ 1 intest_telemetry.pytest_mqtt.pyTwo notes a re-runner will want:
loop_irow fromSENSORSalso removestest_mqtt.pypart 8's own mutation anchor, so the file aborts withAssertionError: mutation anchor not uniqueafter its named checks have already reported. Counting theFAILlines gives exactly 10. The claim is right; whoever re-runs it should expect a traceback rather than the summary line.4. R6's correction is true, not merely made
The risk with a corrected claim is that it is still wrong. It is not:
announce(...)[:len(BASELINE) - 1] == BASELINE[:-1]— eight entity configs. The ninth is named in the same sentence as byte-identical-but-moved-to-last, and the== SITE_NOWcheck below does pin it.AC 1 … NINE MESSAGESheadline is backed by(m, c) == ([], [])— a full missing/changed diff, which genuinely does cover all nine. That one was already correct.Each sentence now names the assertion it actually makes. M9 (+0 B, pure reordering, no payload change) going red on 7 checks is what demonstrates the ordering claim is graded rather than assumed.
Verdict: clean. Merging.
#93will be retargeted torelease/1.0afterwards so thatchecks / unit (pull_request)actually runs on it.