Closes TEL-14. Some PV entities (kWh meters, ESPHome clamps in the PV circuit) read positive for consumption, negative for production; 0.12.0 passed that through, so the page said "✓ PV production: 13 W" at night and daytime production would publish negative.
Option: pv_invert_entities (list, default []). It lists the PV entities whose sign is flipped. The flip is per member inside pv.total(), before the sum.
Shape measured, not assumed.tools/supervisor_shape_probe.py §H, Supervisor 2026.08.0, against a 0.12.0 blob:
parallel list: the 0.12.0 blob validates unchanged; the tab renders it as its own translated repeatable field. Shipped.
list of dicts on pv_entities: Invalid dict for option 'pv_entities', so the upgrade would be bricked.
dicts as a third key: validates, but the per-entry labels render raw.
-sensor.x prefix: validates, but it is invisible syntax.
global bool: validates, but it cannot describe a mixed roster.
Status page: flipped entities are marked [sign inverted]. There is a caution per entity below −50 W that says ADD or REMOVE, and one for a pv_invert_entities entry naming no roster entity. All are cautions, never bad.
Telemetry only.test_pv.py part 12 re-runs the 40-cycle command identity with a flip set. tel11_pv_trace.py control documents are identical to release/1.0 for unset/one/multi.
Evidence:
Full suite (checks.yml command, clean clone, :ro): 33/33 PASS.
Mutations T1–T12: all red, every red named (test_pv part 11 table).
predeploy_check.py on a 0.12.0-shaped blob with pv_entities: validator passes on both trees.
Not in this PR:
CHANGELOG: test_version_gate.py 11b/11d refuse ## Unreleased. The entry text for PKG is in DECISIONS/TEL-14-pv-sign.md §5.
Closes TEL-14. Some PV entities (kWh meters, ESPHome clamps in the PV circuit) read **positive for consumption, negative for production**; 0.12.0 passed that through, so the page said "✓ PV production: 13 W" at night and daytime production would publish negative.
**Option: `pv_invert_entities`** (list, default `[]`). It lists the PV entities whose sign is flipped. The flip is per member inside `pv.total()`, before the sum.
**Shape measured, not assumed.** `tools/supervisor_shape_probe.py` §H, Supervisor 2026.08.0, against a 0.12.0 blob:
- parallel list: the 0.12.0 blob validates unchanged; the tab renders it as its own translated repeatable field. **Shipped.**
- list of dicts on `pv_entities`: `Invalid dict for option 'pv_entities'`, so the upgrade would be bricked.
- dicts as a third key: validates, but the per-entry labels render raw.
- `-sensor.x` prefix: validates, but it is invisible syntax.
- global bool: validates, but it cannot describe a mixed roster.
**Status page:** flipped entities are marked `[sign inverted]`. There is a caution **per entity** below −50 W that says ADD or REMOVE, and one for a `pv_invert_entities` entry naming no roster entity. All are cautions, never `bad`.
**Telemetry only.** `test_pv.py` part 12 re-runs the 40-cycle command identity with a flip set. `tel11_pv_trace.py` control documents are identical to release/1.0 for unset/one/multi.
**Evidence:**
- Full suite (checks.yml command, clean clone, `:ro`): 33/33 PASS.
- Mutations T1–T12: all red, every red named (test_pv part 11 table).
- `predeploy_check.py` on a 0.12.0-shaped blob with `pv_entities`: validator passes on both trees.
**Not in this PR:**
- CHANGELOG: `test_version_gate.py` 11b/11d refuse `## Unreleased`. The entry text for PKG is in `DECISIONS/TEL-14-pv-sign.md` §5.
- No version bump.
Record: `DECISIONS/TEL-14-pv-sign.md` (reverses TEL-11 §2).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Some PV entities (kWh meters, ESPHome clamps in the PV circuit) read positive
for consumption and negative for production. `pv_invert_entities` lists the
ones to flip; the flip is applied per member inside pv.total(), before the sum,
so the MQTT `pv` row and the status page carry the corrected figure. Default []
is byte-identical to 0.12.0.
Shape measured in supervisor_shape_probe.py section H (Supervisor 2026.08.0):
a parallel list validates the 0.12.0 blob unchanged and renders as its own
translated repeatable field; dicts on pv_entities refuse 0.12.0's stored
strings; a prefix is invisible syntax; a global bool cannot describe a mixed
roster. Status page marks flipped entities and cautions per entity below
-50 W, saying ADD or REMOVE. Telemetry only: control documents unchanged.
Record: DECISIONS/TEL-14-pv-sign.md (reverses TEL-11 section 2).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Review (TEL-14) at head 7559d21, graded as merged into release/1.0 @ 3e38baf
Result: one BLOCKING comment (B1) and three non-blocking ones. Everything else checks out.
What I measured
Upgrade safety (passes). I ran the owner's live bare options (jq .data.options, read only, never in the repo) in goodwe-addon-checks:1 with --network none and :ro, using tools/predeploy_check.py --deployed-version 0.12.0. I ran four blobs: the live blob; the live blob with pv_entities: [espressif_vermogen, kwh_meter_actueel_vermogen] and no pv_invert_entities key (the 0.12.0 shape); the live blob with neither key; and the live blob with both sensors flipped, one of them whitespace-padded. Each printed live options pass this version's validator. Its only ❌ is the expected version no-op, because the bump is left for the PKG cut. The Supervisor's own AppOptions (2026.08.0 image, defaults deep-merged over the stored blob) accepts all four, and pv_invert_entities defaults to []. tools/supervisor_shape_probe.py passes all cases, §H included.
CI-10 (passes). Run 838 (job 1963) printed PRNUM: 124, base release/1.0 3e38baf, head refs/pull/124/head 7559d21, then GRADED: refs/pull/124/head merged into release/1.0. It ran with run_attempt 1 and its conclusion was success. I also ran the full suite locally on the same merge: green.
Control untouched (passes).pv_w and pv_invert_entities are read only by the MQTT pv row (values["pv"]), checks(), and the page's PV row. No control path reads either one. Part 12's 40-cycle identity with a flip set holds.
Correctness (passes).inverted() uses the same normalisation as entities() (str(x or "").strip(), then drop empties). A flipped absent member stays None, so the total is absent. 0.0 - w gives no -0.0. The MQTT pv row carries the corrected sum. The −50 W per-entity threshold is reasoned soundly: it sits above standby of 13 W per meter, and a reversed sensor reads −hundreds of watts or more by day. Checking per entity rather than on the total is the right call, and the text treats it as a caution, never bad.
Mutations, merged tree, fresh copies, test_pv.py:
#
mutation
result
M1
a flipped absent member becomes 0.0 instead of None
RED: "a flipped member that is ABSENT is still an absence…"
M3
threshold < → <=
RED: "the threshold is -50 W per ENTITY…"
M4
negative caution computed on the total, not per entity
RED, 4 named (ADD / REMOVE / CAUTION / PER ENTITY)
M2
stray_inverts() roster = set(opts.get("pv_entities") or []) (ignores pv_entity)
survives
M5
_read_pv() passes frozenset(self.o.get("pv_invert_entities") or []) (no strip)
survives
B1 (BLOCKING): the normalisation and stray-caution paths are not pinned end to end
M2 and M5 both survive, and both break the owner's case:
M2: with a single pv_entity listed in pv_invert_entities (exactly the reported install), the page would show a false "pv_invert_entities names X, which is not a PV entity in use, so it flips nothing" while X is being flipped. The "reported case" check at part 12 asserts the row and "no negative caution", but not "no stray caution".
M5: a whitespace-padded entry (for example " sensor.x ") would not be flipped on the read path, yet the page, which uses inverted(), would still mark it [sign inverted] and raise no caution. The only whitespace test is the inverted() unit check.
Fix: in the reported-case check, add _row(_c_own, "flips nothing") == []. Add one run() case where the roster and the invert entry differ only in surrounding whitespace, and assert pv_w is flipped, the marker is present, and there is no stray caution. Code change: none needed. Please re-measure that M2 and M5 go red.
Non-blocking
N1 (length):DECISIONS/TEL-14-pv-sign.md runs 85 lines. The §2 probe transcript duplicates the table under it, since the probe is re-runnable, and §1 is one dense paragraph. Owner standard: keep the table, drop the transcript, and cut §1 to the reason. The 12-row mutation table printed in test_pv.py part 11 is also more than a reader of the test output needs.
N2 (follow-up): §1 records that tools/tel11_pv_trace.py --compare now REFUSES on any honest run and is "not fixed here". A broken tool belongs on a card, not in a paragraph. I'll name it as a follow-up in my report.
N3 (nit):pv_invert_entities set with no PV roster at all says nothing, because the block sits under if roster:. It is harmless because nothing is read. Leave it or add a caution; your call.
Other notes
#122 (TEST-26): I merged #122 on top of this PR locally (clean) and ran test_named_absence.py: "no unguarded raising read in any of the 35 test files (0 found)". test_pv.py stays green, so part 12's _at/_row reads do not trip the guard.
No rebase needed: the branch is based on 6e65272, not the tip, but it merges cleanly, and CI grades the merge.
## Review (TEL-14) at head 7559d21, graded as merged into release/1.0 @ 3e38baf
**Result: one BLOCKING comment (B1) and three non-blocking ones. Everything else checks out.**
### What I measured
**Upgrade safety (passes).** I ran the owner's live bare options (`jq .data.options`, read only, never in the repo) in `goodwe-addon-checks:1` with `--network none` and `:ro`, using `tools/predeploy_check.py --deployed-version 0.12.0`. I ran four blobs: the live blob; the live blob with `pv_entities: [espressif_vermogen, kwh_meter_actueel_vermogen]` and no `pv_invert_entities` key (the 0.12.0 shape); the live blob with neither key; and the live blob with both sensors flipped, one of them whitespace-padded. Each printed `live options pass this version's validator`. Its only ❌ is the expected version no-op, because the bump is left for the PKG cut. The Supervisor's own `AppOptions` (2026.08.0 image, defaults deep-merged over the stored blob) accepts all four, and `pv_invert_entities` defaults to `[]`. `tools/supervisor_shape_probe.py` passes all cases, §H included.
**CI-10 (passes).** Run 838 (job 1963) printed `PRNUM: 124`, base `release/1.0 3e38baf`, head `refs/pull/124/head 7559d21`, then `GRADED: refs/pull/124/head merged into release/1.0`. It ran with `run_attempt` 1 and its conclusion was success. I also ran the full suite locally on the same merge: green.
**Control untouched (passes).** `pv_w` and `pv_invert_entities` are read only by the MQTT `pv` row (`values["pv"]`), `checks()`, and the page's PV row. No control path reads either one. Part 12's 40-cycle identity with a flip set holds.
**Correctness (passes).** `inverted()` uses the same normalisation as `entities()` (`str(x or "").strip()`, then drop empties). A flipped absent member stays `None`, so the total is absent. `0.0 - w` gives no `-0.0`. The MQTT `pv` row carries the corrected sum. The −50 W per-entity threshold is reasoned soundly: it sits above standby of 13 W per meter, and a reversed sensor reads −hundreds of watts or more by day. Checking per entity rather than on the total is the right call, and the text treats it as a caution, never `bad`.
**Mutations, merged tree, fresh copies, `test_pv.py`:**
| # | mutation | result |
|---|---|---|
| M1 | a flipped absent member becomes `0.0` instead of `None` | **RED**: "a flipped member that is ABSENT is still an absence…" |
| M3 | threshold `<` → `<=` | **RED**: "the threshold is -50 W per ENTITY…" |
| M4 | negative caution computed on the total, not per entity | **RED, 4 named** (ADD / REMOVE / CAUTION / PER ENTITY) |
| M2 | `stray_inverts()` roster = `set(opts.get("pv_entities") or [])` (ignores `pv_entity`) | **survives** |
| M5 | `_read_pv()` passes `frozenset(self.o.get("pv_invert_entities") or [])` (no strip) | **survives** |
### B1 (BLOCKING): the normalisation and stray-caution paths are not pinned end to end
M2 and M5 both survive, and both break the owner's case:
- **M2:** with a single `pv_entity` listed in `pv_invert_entities` (exactly the reported install), the page would show a false "`pv_invert_entities` names X, which is not a PV entity in use, so it flips nothing" while X *is* being flipped. The "reported case" check at part 12 asserts the row and "no negative caution", but not "no stray caution".
- **M5:** a whitespace-padded entry (for example `" sensor.x "`) would not be flipped on the read path, yet the page, which uses `inverted()`, would still mark it `[sign inverted]` and raise no caution. The only whitespace test is the `inverted()` unit check.
Fix: in the reported-case check, add `_row(_c_own, "flips nothing") == []`. Add one `run()` case where the roster and the invert entry differ only in surrounding whitespace, and assert `pv_w` is flipped, the marker is present, and there is no stray caution. Code change: none needed. Please re-measure that M2 and M5 go red.
### Non-blocking
- **N1 (length):** `DECISIONS/TEL-14-pv-sign.md` runs 85 lines. The §2 probe transcript duplicates the table under it, since the probe is re-runnable, and §1 is one dense paragraph. Owner standard: keep the table, drop the transcript, and cut §1 to the reason. The 12-row mutation table printed in `test_pv.py` part 11 is also more than a reader of the test output needs.
- **N2 (follow-up):** §1 records that `tools/tel11_pv_trace.py --compare` now REFUSES on any honest run and is "not fixed here". A broken tool belongs on a card, not in a paragraph. I'll name it as a follow-up in my report.
- **N3 (nit):** `pv_invert_entities` set with no PV roster at all says nothing, because the block sits under `if roster:`. It is harmless because nothing is read. Leave it or add a caution; your call.
### Other notes
- **#122 (TEST-26):** I merged #122 on top of this PR locally (clean) and ran `test_named_absence.py`: *"no unguarded raising read in any of the 35 test files (0 found)"*. `test_pv.py` stays green, so part 12's `_at`/`_row` reads do not trip the guard.
- **No rebase needed:** the branch is based on 6e65272, not the tip, but it merges cleanly, and CI grades the merge.
git ls-remote: 7fa045af85aedb3616ff7f5e9f28074522209968 refs/heads/TEL-14-pv-sign. Full suite on the branch merged into release/1.0 @ 43b2740, in goodwe-addon-checks:1 with :ro and --network none: 35/35 PASS.
B1: done, tests only (no code change)
Reported case (part 12): added _row(_c_own, "flips nothing") == [], and the check name now says why ("a lone pv_entity is in the roster").
New run() case, PADDING: pv_entities=[E1, " sensor.pv_west", E3] and pv_invert_entities=["sensor.pv_west "]. The two entries differ only by surrounding whitespace, padded on opposite sides. It asserts pv_w == 3270 (flipped on the read path), sensor.pv_west 2100 W in kW [sign inverted] in the row, and no flips nothing row. It also covers M2 from the roster side, because unstripped pv_entities misses the stripped invert entry.
Re-measured. Merged tree (43b2740 + 93f349a). Each mutant is a fresh cp -a with __pycache__ purged (0 cache dirs before each run), CI image, :ro, --network none, test_pv.py. The edit was confirmed on disk by diff. An identity control on the same harness was green.
#
mutation
bytes
result
M2
stray_inverts(): roster = set(opts.get("pv_entities") or [])
+15 B
RED, 2 named: THE REPORTED CASE, PADDING
M5
_read_pv(): frozenset(self.o.get("pv_invert_entities") or [])
+25 B
RED, 1 named: PADDING
No tracebacks. Neither delta is +0 B.
N1: done
DECISIONS/TEL-14-pv-sign.md: I dropped the probe transcript. §2 keeps the table and a pointer to re-run supervisor_shape_probe.py §H. §1 is cut to the reason (control path untouched and pinned by part 12; the "announces itself" premise failed on a real site; the alternative is a template sensor). The --compare paragraph is gone, see N2.
test_pv.py part 11: the 12-row TEL-14 table is replaced by a two-line pointer. The ledger moved to DECISIONS §4 and now includes M2/M5, so the byte deltas still have a durable home. That is why the file is 83 lines and not much shorter. The transcript and §1 cuts were offset by the ledger moving in.
N2: not fixed here, as asked
The tel11_pv_trace.py --compare note is removed from §1. The manager is filing a card.
N3: left as is
With no PV roster, pv_roster is empty, so there is no PV row on the page (main.py status rows are gated on pv_roster) and no pv MQTT row. The missing channel is already visible, and nothing reads a sign. A stray caution there would also fire on the legitimate case of clearing pv_entities while keeping the invert list for later.
Also checked (manager's request): DOCS.md "The sign, and why there is no pv_invert"
This branch already rewrites it, since 7559d21, as "The sign, and pv_invert_entities (TEL-14)". The new section describes the option, the [sign inverted] marker, the −50 W caution and the history caveat. The options table links to it. The only remaining "there is no pv_invert" text is the released 0.12.0 entry in CHANGELOG.md (line 411). I left it alone because it is history.
## Reply to #1628: new head `7fa045a` (93f349a B1, 7fa045a N1)
`git ls-remote`: `7fa045af85aedb3616ff7f5e9f28074522209968 refs/heads/TEL-14-pv-sign`. Full suite on the branch merged into release/1.0 @ 43b2740, in `goodwe-addon-checks:1` with `:ro` and `--network none`: **35/35 PASS**.
### B1: done, tests only (no code change)
- Reported case (part 12): added `_row(_c_own, "flips nothing") == []`, and the check name now says why ("a lone `pv_entity` is in the roster").
- New `run()` case, **PADDING**: `pv_entities=[E1, " sensor.pv_west", E3]` and `pv_invert_entities=["sensor.pv_west "]`. The two entries differ only by surrounding whitespace, padded on opposite sides. It asserts `pv_w == 3270` (flipped on the read path), `sensor.pv_west 2100 W in kW [sign inverted]` in the row, and no `flips nothing` row. It also covers M2 from the roster side, because unstripped `pv_entities` misses the stripped invert entry.
**Re-measured.** Merged tree (43b2740 + 93f349a). Each mutant is a fresh `cp -a` with `__pycache__` purged (0 cache dirs before each run), CI image, `:ro`, `--network none`, `test_pv.py`. The edit was confirmed on disk by `diff`. An identity control on the same harness was green.
| # | mutation | bytes | result |
|---|---|---|---|
| M2 | `stray_inverts()`: `roster = set(opts.get("pv_entities") or [])` | +15 B | **RED, 2 named**: THE REPORTED CASE, PADDING |
| M5 | `_read_pv()`: `frozenset(self.o.get("pv_invert_entities") or [])` | +25 B | **RED, 1 named**: PADDING |
No tracebacks. Neither delta is `+0 B`.
### N1: done
- `DECISIONS/TEL-14-pv-sign.md`: I dropped the probe transcript. §2 keeps the table and a pointer to re-run `supervisor_shape_probe.py` §H. §1 is cut to the reason (control path untouched and pinned by part 12; the "announces itself" premise failed on a real site; the alternative is a template sensor). The `--compare` paragraph is gone, see N2.
- `test_pv.py` part 11: the 12-row TEL-14 table is replaced by a two-line pointer. The ledger moved to DECISIONS §4 and now includes M2/M5, so the byte deltas still have a durable home. That is why the file is 83 lines and not much shorter. The transcript and §1 cuts were offset by the ledger moving in.
### N2: not fixed here, as asked
The `tel11_pv_trace.py --compare` note is removed from §1. The manager is filing a card.
### N3: left as is
With no PV roster, `pv_roster` is empty, so there is no PV row on the page (`main.py` status rows are gated on `pv_roster`) and no `pv` MQTT row. The missing channel is already visible, and nothing reads a sign. A stray caution there would also fire on the legitimate case of clearing `pv_entities` while keeping the invert list for later.
### Also checked (manager's request): DOCS.md "The sign, and why there is no `pv_invert`"
This branch already rewrites it, since 7559d21, as "The sign, and `pv_invert_entities` (TEL-14)". The new section describes the option, the `[sign inverted]` marker, the −50 W caution and the history caveat. The options table links to it. The only remaining "there is no `pv_invert`" text is the released 0.12.0 entry in CHANGELOG.md (line 411). I left it alone because it is history.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Review round 2 on #1641, head 7fa045a: all resolved, merging
B1: resolved. I re-measured both myself on 7fa045a merged into release/1.0 @ 20260e8, using fresh copies, the CI image, :ro and --network none. M2 is RED with 2 named checks (THE REPORTED CASE, PADDING). M5 is RED with 1 named check (PADDING). There were no tracebacks, and the unmutated control is green.
N1: resolved. DECISIONS is trimmed to the reason plus the table. The test-output table is now a pointer, and the ledger lives in DECISIONS §4. The file length is fine.
N2: out of scope here. It will be a follow-up card for tel11_pv_trace.py --compare.
N3: the refutation holds. With no roster there is no PV row and no MQTT pv row, so nothing reads a sign. A caution there would also fire on the legitimate case of clearing the roster while keeping the invert list.
#122 (now merged):test_named_absence.py passes on this merge, both locally and in CI.
Upgrade safety: re-checked on this merge. The owner's live blob, and the same blob as a 0.12.0 blob with pv_entities set and no new key, both give live options pass this version's validator. No app code changed since 7559d21.
CI-10: run 842 (job 1967) printed PRNUM: 124, base release/1.0 20260e8 (the current tip), head 7fa045a, then GRADED: refs/pull/124/head merged into release/1.0. It ran with run_attempt 1 and passed 35 of 35 tests.
I'm merging with head_commit_id pinned to 7fa045a.
## Review round 2 on #1641, head 7fa045a: all resolved, merging
- **B1: resolved.** I re-measured both myself on 7fa045a merged into release/1.0 @ 20260e8, using fresh copies, the CI image, `:ro` and `--network none`. **M2 is RED** with 2 named checks (THE REPORTED CASE, PADDING). **M5 is RED** with 1 named check (PADDING). There were no tracebacks, and the unmutated control is green.
- **N1: resolved.** DECISIONS is trimmed to the reason plus the table. The test-output table is now a pointer, and the ledger lives in DECISIONS §4. The file length is fine.
- **N2:** out of scope here. It will be a follow-up card for `tel11_pv_trace.py --compare`.
- **N3: the refutation holds.** With no roster there is no PV row and no MQTT `pv` row, so nothing reads a sign. A caution there would also fire on the legitimate case of clearing the roster while keeping the invert list.
- **#122 (now merged):** `test_named_absence.py` passes on this merge, both locally and in CI.
- **Upgrade safety:** re-checked on this merge. The owner's live blob, and the same blob as a 0.12.0 blob with `pv_entities` set and no new key, both give `live options pass this version's validator`. No app code changed since 7559d21.
- **CI-10:** run 842 (job 1967) printed `PRNUM: 124`, base `release/1.0 20260e8` (the current tip), head `7fa045a`, then `GRADED: refs/pull/124/head merged into release/1.0`. It ran with `run_attempt` 1 and passed 35 of 35 tests.
I'm merging with `head_commit_id` pinned to 7fa045a.
admin
merged commit 16c796fa30 into release/1.02026-09-25 01:30:34 +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.
Closes TEL-14. Some PV entities (kWh meters, ESPHome clamps in the PV circuit) read positive for consumption, negative for production; 0.12.0 passed that through, so the page said "✓ PV production: 13 W" at night and daytime production would publish negative.
Option:
pv_invert_entities(list, default[]). It lists the PV entities whose sign is flipped. The flip is per member insidepv.total(), before the sum.Shape measured, not assumed.
tools/supervisor_shape_probe.py§H, Supervisor 2026.08.0, against a 0.12.0 blob:pv_entities:Invalid dict for option 'pv_entities', so the upgrade would be bricked.-sensor.xprefix: validates, but it is invisible syntax.Status page: flipped entities are marked
[sign inverted]. There is a caution per entity below −50 W that says ADD or REMOVE, and one for apv_invert_entitiesentry naming no roster entity. All are cautions, neverbad.Telemetry only.
test_pv.pypart 12 re-runs the 40-cycle command identity with a flip set.tel11_pv_trace.pycontrol documents are identical to release/1.0 for unset/one/multi.Evidence:
:ro): 33/33 PASS.predeploy_check.pyon a 0.12.0-shaped blob withpv_entities: validator passes on both trees.Not in this PR:
test_version_gate.py11b/11d refuse## Unreleased. The entry text for PKG is inDECISIONS/TEL-14-pv-sign.md§5.Record:
DECISIONS/TEL-14-pv-sign.md(reverses TEL-11 §2).🤖 Generated with Claude Code
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Review (TEL-14) at head
7559d21, graded as merged into release/1.0 @3e38bafResult: one BLOCKING comment (B1) and three non-blocking ones. Everything else checks out.
What I measured
Upgrade safety (passes). I ran the owner's live bare options (
jq .data.options, read only, never in the repo) ingoodwe-addon-checks:1with--network noneand:ro, usingtools/predeploy_check.py --deployed-version 0.12.0. I ran four blobs: the live blob; the live blob withpv_entities: [espressif_vermogen, kwh_meter_actueel_vermogen]and nopv_invert_entitieskey (the 0.12.0 shape); the live blob with neither key; and the live blob with both sensors flipped, one of them whitespace-padded. Each printedlive options pass this version's validator. Its only ❌ is the expected version no-op, because the bump is left for the PKG cut. The Supervisor's ownAppOptions(2026.08.0 image, defaults deep-merged over the stored blob) accepts all four, andpv_invert_entitiesdefaults to[].tools/supervisor_shape_probe.pypasses all cases, §H included.CI-10 (passes). Run 838 (job 1963) printed
PRNUM: 124, baserelease/1.0 3e38baf, headrefs/pull/124/head 7559d21, thenGRADED: refs/pull/124/head merged into release/1.0. It ran withrun_attempt1 and its conclusion was success. I also ran the full suite locally on the same merge: green.Control untouched (passes).
pv_wandpv_invert_entitiesare read only by the MQTTpvrow (values["pv"]),checks(), and the page's PV row. No control path reads either one. Part 12's 40-cycle identity with a flip set holds.Correctness (passes).
inverted()uses the same normalisation asentities()(str(x or "").strip(), then drop empties). A flipped absent member staysNone, so the total is absent.0.0 - wgives no-0.0. The MQTTpvrow carries the corrected sum. The −50 W per-entity threshold is reasoned soundly: it sits above standby of 13 W per meter, and a reversed sensor reads −hundreds of watts or more by day. Checking per entity rather than on the total is the right call, and the text treats it as a caution, neverbad.Mutations, merged tree, fresh copies,
test_pv.py:0.0instead ofNone<→<=stray_inverts()roster =set(opts.get("pv_entities") or [])(ignorespv_entity)_read_pv()passesfrozenset(self.o.get("pv_invert_entities") or [])(no strip)B1 (BLOCKING): the normalisation and stray-caution paths are not pinned end to end
M2 and M5 both survive, and both break the owner's case:
pv_entitylisted inpv_invert_entities(exactly the reported install), the page would show a false "pv_invert_entitiesnames X, which is not a PV entity in use, so it flips nothing" while X is being flipped. The "reported case" check at part 12 asserts the row and "no negative caution", but not "no stray caution"." sensor.x ") would not be flipped on the read path, yet the page, which usesinverted(), would still mark it[sign inverted]and raise no caution. The only whitespace test is theinverted()unit check.Fix: in the reported-case check, add
_row(_c_own, "flips nothing") == []. Add onerun()case where the roster and the invert entry differ only in surrounding whitespace, and assertpv_wis flipped, the marker is present, and there is no stray caution. Code change: none needed. Please re-measure that M2 and M5 go red.Non-blocking
DECISIONS/TEL-14-pv-sign.mdruns 85 lines. The §2 probe transcript duplicates the table under it, since the probe is re-runnable, and §1 is one dense paragraph. Owner standard: keep the table, drop the transcript, and cut §1 to the reason. The 12-row mutation table printed intest_pv.pypart 11 is also more than a reader of the test output needs.tools/tel11_pv_trace.py --comparenow REFUSES on any honest run and is "not fixed here". A broken tool belongs on a card, not in a paragraph. I'll name it as a follow-up in my report.pv_invert_entitiesset with no PV roster at all says nothing, because the block sits underif roster:. It is harmless because nothing is read. Leave it or add a caution; your call.Other notes
test_named_absence.py: "no unguarded raising read in any of the 35 test files (0 found)".test_pv.pystays green, so part 12's_at/_rowreads do not trip the guard.6e65272, not the tip, but it merges cleanly, and CI grades the merge.Reply to #1628: new head
7fa045a(93f349aB1,7fa045aN1)git ls-remote:7fa045af85aedb3616ff7f5e9f28074522209968 refs/heads/TEL-14-pv-sign. Full suite on the branch merged into release/1.0 @43b2740, ingoodwe-addon-checks:1with:roand--network none: 35/35 PASS.B1: done, tests only (no code change)
_row(_c_own, "flips nothing") == [], and the check name now says why ("a lonepv_entityis in the roster").run()case, PADDING:pv_entities=[E1, " sensor.pv_west", E3]andpv_invert_entities=["sensor.pv_west "]. The two entries differ only by surrounding whitespace, padded on opposite sides. It assertspv_w == 3270(flipped on the read path),sensor.pv_west 2100 W in kW [sign inverted]in the row, and noflips nothingrow. It also covers M2 from the roster side, because unstrippedpv_entitiesmisses the stripped invert entry.Re-measured. Merged tree (
43b2740+93f349a). Each mutant is a freshcp -awith__pycache__purged (0 cache dirs before each run), CI image,:ro,--network none,test_pv.py. The edit was confirmed on disk bydiff. An identity control on the same harness was green.stray_inverts():roster = set(opts.get("pv_entities") or [])_read_pv():frozenset(self.o.get("pv_invert_entities") or [])No tracebacks. Neither delta is
+0 B.N1: done
DECISIONS/TEL-14-pv-sign.md: I dropped the probe transcript. §2 keeps the table and a pointer to re-runsupervisor_shape_probe.py§H. §1 is cut to the reason (control path untouched and pinned by part 12; the "announces itself" premise failed on a real site; the alternative is a template sensor). The--compareparagraph is gone, see N2.test_pv.pypart 11: the 12-row TEL-14 table is replaced by a two-line pointer. The ledger moved to DECISIONS §4 and now includes M2/M5, so the byte deltas still have a durable home. That is why the file is 83 lines and not much shorter. The transcript and §1 cuts were offset by the ledger moving in.N2: not fixed here, as asked
The
tel11_pv_trace.py --comparenote is removed from §1. The manager is filing a card.N3: left as is
With no PV roster,
pv_rosteris empty, so there is no PV row on the page (main.pystatus rows are gated onpv_roster) and nopvMQTT row. The missing channel is already visible, and nothing reads a sign. A stray caution there would also fire on the legitimate case of clearingpv_entitieswhile keeping the invert list for later.Also checked (manager's request): DOCS.md "The sign, and why there is no
pv_invert"This branch already rewrites it, since
7559d21, as "The sign, andpv_invert_entities(TEL-14)". The new section describes the option, the[sign inverted]marker, the −50 W caution and the history caveat. The options table links to it. The only remaining "there is nopv_invert" text is the released 0.12.0 entry in CHANGELOG.md (line 411). I left it alone because it is history.🤖 Generated with Claude Code
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Review round 2 on #1641, head
7fa045a: all resolved, merging7fa045amerged into release/1.0 @20260e8, using fresh copies, the CI image,:roand--network none. M2 is RED with 2 named checks (THE REPORTED CASE, PADDING). M5 is RED with 1 named check (PADDING). There were no tracebacks, and the unmutated control is green.tel11_pv_trace.py --compare.pvrow, so nothing reads a sign. A caution there would also fire on the legitimate case of clearing the roster while keeping the invert list.test_named_absence.pypasses on this merge, both locally and in CI.pv_entitiesset and no new key, both givelive options pass this version's validator. No app code changed since7559d21.PRNUM: 124, baserelease/1.0 20260e8(the current tip), head7fa045a, thenGRADED: refs/pull/124/head merged into release/1.0. It ran withrun_attempt1 and passed 35 of 35 tests.I'm merging with
head_commit_idpinned to7fa045a.