MAINT-24 follow-up (rig finding F-1). With A in maintenance CHARGE and the house over the month aim, the pause ended each time the guard idled, so A flipped charge/give (468 flips in 45 min on the rig, 11 at base 02eac3b). Now peakguard.shaping_held() keeps the pause until the house is 300 W under the aim. No cut-back is delayed or weakened; MAINT-24's night rate (1475 W live shape), N=1, the drain and modes off are unchanged.
Sim, real cycle(), house 6500 W for 45 min, banked month peak 6000 W (aim 5950 W):
CHANGELOG: During a real peak above this month's peak, the monthly battery charge now stays paused instead of restarting every few seconds, until the house load is clear of the limit.
MAINT-24 follow-up (rig finding F-1). With A in maintenance CHARGE and the house over the month aim, the pause ended each time the guard idled, so A flipped charge/give (468 flips in 45 min on the rig, 11 at base 02eac3b). Now `peakguard.shaping_held()` keeps the pause until the house is 300 W under the aim. No cut-back is delayed or weakened; MAINT-24's night rate (1475 W live shape), N=1, the drain and modes off are unchanged.
Sim, real `cycle()`, house 6500 W for 45 min, banked month peak 6000 W (aim 5950 W):
| | 02eac3b | 1b5f7c9 | MAINT-28 |
|---|---|---|---|
| A charge/give flips in the load | 0 | 52 | 1 |
| grid min / max in the load (W) | 3950 / 9024 | 3952 / 7994 | 3950 / 7994 |
| quarter averages (W) | 5959 5950 5950 5438 5502 | 5356 5923 5924 5041 4479 | 5356 4012 4000 4226 4476 |
| A charging seconds in the load | 0 | 367 | 301 (before the first trip) |
DECISIONS: `goodwe_controller/DECISIONS/MAINT-28-hold-the-pause-in-a-peak.md`. Tests: `test_maint28.py`.
CHANGELOG: During a real peak above this month's peak, the monthly battery charge now stays paused instead of restarting every few seconds, until the house load is clear of the limit.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
MAINT-28 review of 7edf848 (independent, Opus). Verdict: NOT READY. One docs change only; the code is fine.
The code, the tests, CI and the local suite all pass. The only thing blocking is a wrong reason in DECISIONS for the energy trade-off.
Findings
M-1 (Medium, docs) goodwe_controller/DECISIONS/MAINT-28-hold-the-pause-in-a-peak.md:17. The note says: "A cap on that give would conflict with the guard's lo (lo > hi resolves to 0 W), so it is not tried here." That is not true for the state the hold adds:
While the guard is idle, peakguard.claim() files no claim at all (app/peakguard.py:482).
While the guard is engaged, suspended is true and the shaping claim is dropped as before.
So a hi = 0 claim filed only while the pause is held and the guard is idle never meets a lo. I measured that option in a scratch tree (not proposed for merge; same 45-min sim, scratch/MAINT-28/review/repro_alt.txt):
A gives 0.44 kWh instead of 1.66 kWh.
Quarters 5919 and 5915 W, under the aim.
1 charge/give flip.
But A switches between giving and idle 32 times in 40 min, and the guard toggles (engaged in 662 of 2400 cycles).
The choice in this PR is still the better one: one steady give, which is the point of the card. But the stated reason is wrong. Fix: replace that sentence with the real reason and these numbers. A cap is possible. It saves about 1.2 kWh per 45-min peak but brings back a give/idle toggle at the guard's edge (32 in 40 min in the sim). No code change.
L-1 (Low, style) goodwe_controller/app/ctl_cycle.py:571-574. The comment is 4 lines and has no DECISIONS pointer; the reset comment at 540-542 has one. Make it one line plus the pointer.
L-2 (Low, report only). The "grid min in the load" values in the dev report and PR table (452 / 2988 / 3485 W) are the last sample of the load: the house steps down while A is still giving. On all three heads the real minimum in the load is about 3950 W (see the review report). The DECISIONS table does not carry these values, so no change is needed there.
Info. After a real peak, while the house sits 0-300 W under the aim, the charge stays off and A gives for the house. At 1b5f7c9, A would charge 150 W or less in that band. This is rare and only ever moves toward less grid draw.
Checks
Numbers match the dev's table exactly: flips, quarters, A charge seconds, out/in kWh, charge back after 3 s, on 02eac3b, 1b5f7c9 and 7edf848.
Live-shape night: 0 / -1476 / -1476 W. The pause never comes on 1b5f7c9 or 7edf848 (02eac3b: 596 paused cycles).
The B-in-CHARGE mirror (15-min load): 18 flips on 1b5f7c9, 1 on 7edf848.
Safety: no issue.
The hold only stops a shaping claim from being filed. It never adds a claim or delays the guard's claim.
The N=1, n1_peak, drain and modes-off digests are the same as 1b5f7c9.
Forecast unreadable: the hold only follows a real guard step-in, and with neither forecast nor house reading it lets go (SAFETY-63).
Late readings: the two early returns write nothing; a reading late by stale_after or more files the safety SET of 0 W, which wins.
Shadow mode: same path, nothing written.
Month start: the maint_aim > peak_aim_w() gate ends the hold on the first cycle of a new month.
Give cost. While the pause is held and the guard is idle, no shaping claim is filed, so the loop runs A as a normal battery: A gives a mean of 2496 W, its full rating, and the grid sits near 4000 W. The quarter only needs about 550 W. The cost is about 1.2 kWh more through A per 45-min real peak:
Money: a few cents of round-trip loss. The energy replaces grid power bought at the same night price.
Delay: about 50 min more maintenance charge at 1475 W. That is far inside CHARGE_TIMEOUT_MIN (2160 open minutes).
Acceptable as is: it buys the steadiness the card asked for. It only needs the correct reason in DECISIONS (M-1).
_maint_peak_hold lifetime: keep as is.
Any cycle that reaches the maintenance loop without the charge-only branch clears the flag (part 1b, M4).
On a same-cycle pack switch the flag carries over, but each cycle checks it again against the measured house (more than aim - 300) and the month gate, so it can only keep a charge of 150 W or less off.
Restart: the flag starts at False and is not stored.
Moving the reset to the top of cycle() would be worse: each missed poll would restart the charge, one flap per missed poll.
Tests.
test_maint28 passes on 7edf848 and fails on 1b5f7c9 (6 checks).
M1-M6 each fail (4/2/4/2/1/1 failing checks).
The five AT_BASE digests are the same on a clean 1b5f7c9 with a stub, so they are true base values.
test_safety84n2_charge_banked: 64 of 480 rows differ. Rows 310-371 (t = 1550-1855 s): A no longer charges -127 W; A and B give 2500 W each while the load runs. Rows 387-388: a small difference at the restart. The other five pins are the same. It moves by design.
The replay clock: ctl_cycle reads datetime as a global (line 49), so main_files.live patches it with the same _DT. It is a faithful stand-in.
Docs. The DECISIONS note is short, has no duplicate paragraph, and its sha and numbers are right; the one wrong sentence is M-1. The CHANGELOG line is in the PR body and CHANGELOG.md is not touched. No new user text.
CI and suite.
CI runs 1634 (checks / unit) and 1635 (infra-pin / fresh): success on head_sha 7edf848.
Local full suite (goodwe-addon-checks:1, --network none, :ro mount, real hwsim): 107 files, 0 failed, 1593 s wall. test_maint28 78 s.
Not merged (M-1 is above Low). Details: reports/MAINT-28-review-report.md.
**MAINT-28 review of 7edf848 (independent, Opus). Verdict: NOT READY. One docs change only; the code is fine.**
The code, the tests, CI and the local suite all pass. The only thing blocking is a wrong reason in DECISIONS for the energy trade-off.
### Findings
**M-1 (Medium, docs) `goodwe_controller/DECISIONS/MAINT-28-hold-the-pause-in-a-peak.md:17`.** The note says: "A cap on that give would conflict with the guard's `lo` (`lo > hi` resolves to 0 W), so it is not tried here." That is not true for the state the hold adds:
- While the guard is idle, `peakguard.claim()` files no claim at all (`app/peakguard.py:482`).
- While the guard is engaged, `suspended` is true and the shaping claim is dropped as before.
So a `hi = 0` claim filed only while the pause is held and the guard is idle never meets a `lo`. I measured that option in a scratch tree (not proposed for merge; same 45-min sim, `scratch/MAINT-28/review/repro_alt.txt`):
- A gives 0.44 kWh instead of 1.66 kWh.
- Quarters 5919 and 5915 W, under the aim.
- 1 charge/give flip.
- But A switches between giving and idle 32 times in 40 min, and the guard toggles (engaged in 662 of 2400 cycles).
The choice in this PR is still the better one: one steady give, which is the point of the card. But the stated reason is wrong. Fix: replace that sentence with the real reason and these numbers. A cap is possible. It saves about 1.2 kWh per 45-min peak but brings back a give/idle toggle at the guard's edge (32 in 40 min in the sim). No code change.
**L-1 (Low, style) `goodwe_controller/app/ctl_cycle.py:571-574`.** The comment is 4 lines and has no DECISIONS pointer; the reset comment at 540-542 has one. Make it one line plus the pointer.
**L-2 (Low, report only).** The "grid min in the load" values in the dev report and PR table (452 / 2988 / 3485 W) are the last sample of the load: the house steps down while A is still giving. On all three heads the real minimum in the load is about 3950 W (see the review report). The DECISIONS table does not carry these values, so no change is needed there.
**Info.** After a real peak, while the house sits 0-300 W under the aim, the charge stays off and A gives for the house. At 1b5f7c9, A would charge 150 W or less in that band. This is rare and only ever moves toward less grid draw.
### Checks
1. **Numbers match the dev's table exactly**: flips, quarters, A charge seconds, out/in kWh, charge back after 3 s, on 02eac3b, 1b5f7c9 and 7edf848.
- Live-shape night: 0 / -1476 / -1476 W. The pause never comes on 1b5f7c9 or 7edf848 (02eac3b: 596 paused cycles).
- The B-in-CHARGE mirror (15-min load): 18 flips on 1b5f7c9, 1 on 7edf848.
2. **Safety: no issue.**
- The hold only stops a shaping claim from being filed. It never adds a claim or delays the guard's claim.
- The N=1, `n1_peak`, drain and modes-off digests are the same as 1b5f7c9.
- Forecast unreadable: the hold only follows a real guard step-in, and with neither forecast nor house reading it lets go (SAFETY-63).
- Late readings: the two early returns write nothing; a reading late by `stale_after` or more files the safety SET of 0 W, which wins.
- Shadow mode: same path, nothing written.
- Month start: the `maint_aim > peak_aim_w()` gate ends the hold on the first cycle of a new month.
3. **Give cost.** While the pause is held and the guard is idle, no shaping claim is filed, so the loop runs A as a normal battery: A gives a mean of 2496 W, its full rating, and the grid sits near 4000 W. The quarter only needs about 550 W. The cost is about 1.2 kWh more through A per 45-min real peak:
- Money: a few cents of round-trip loss. The energy replaces grid power bought at the same night price.
- Delay: about 50 min more maintenance charge at 1475 W. That is far inside `CHARGE_TIMEOUT_MIN` (2160 open minutes).
**Acceptable as is**: it buys the steadiness the card asked for. It only needs the correct reason in DECISIONS (M-1).
4. **`_maint_peak_hold` lifetime: keep as is.**
- Any cycle that reaches the maintenance loop without the charge-only branch clears the flag (part 1b, M4).
- On a same-cycle pack switch the flag carries over, but each cycle checks it again against the measured house (more than aim - 300) and the month gate, so it can only keep a charge of 150 W or less off.
- Restart: the flag starts at False and is not stored.
- Moving the reset to the top of `cycle()` would be worse: each missed poll would restart the charge, one flap per missed poll.
5. **Tests.**
- `test_maint28` passes on 7edf848 and fails on 1b5f7c9 (6 checks).
- M1-M6 each fail (4/2/4/2/1/1 failing checks).
- The five AT_BASE digests are the same on a clean 1b5f7c9 with a stub, so they are true base values.
- `test_safety84` `n2_charge_banked`: 64 of 480 rows differ. Rows 310-371 (t = 1550-1855 s): A no longer charges -127 W; A and B give 2500 W each while the load runs. Rows 387-388: a small difference at the restart. The other five pins are the same. It moves by design.
- The replay clock: `ctl_cycle` reads `datetime` as a global (line 49), so `main_files.live` patches it with the same `_DT`. It is a faithful stand-in.
6. **Docs.** The DECISIONS note is short, has no duplicate paragraph, and its sha and numbers are right; the one wrong sentence is M-1. The CHANGELOG line is in the PR body and CHANGELOG.md is not touched. No new user text.
7. **CI and suite.**
- CI runs 1634 (`checks / unit`) and 1635 (`infra-pin / fresh`): success on head_sha 7edf848.
- Local full suite (`goodwe-addon-checks:1`, `--network none`, `:ro` mount, real hwsim): 107 files, 0 failed, 1593 s wall. `test_maint28` 78 s.
Not merged (M-1 is above Low). Details: `reports/MAINT-28-review-report.md`.
Re-review of e770615: MERGE-READY as is, and now merged. release/1.0 is f3f9483 (normal merge; parents 1b5f7c9 and e770615).
git diff 7edf848 e770615 changes only DECISIONS and one code comment. No behaviour change.
M-1 fixed. DECISIONS now gives the measured reason for not capping A's give: A gives 0.44 vs 1.66 kWh, quarters 5919/5915 W, 32 give/idle switches in 40 min. These numbers match my repro_alt.txt.
L-1 fixed.ctl_cycle.py comment is now one line plus the DECISIONS pointer.
L-2 fixed. The grid minimum is now 3950 / 3952 / 3950 W in the PR body table and in the dev report.
CI: runs 1636 (checks / unit) and 1637 (infra-pin / fresh) both succeeded on e770615 (checked through the API).
No findings left above Low.
**Re-review of e770615: MERGE-READY as is, and now merged.** release/1.0 is f3f9483 (normal merge; parents 1b5f7c9 and e770615).
- `git diff 7edf848 e770615` changes only DECISIONS and one code comment. No behaviour change.
- **M-1 fixed.** DECISIONS now gives the measured reason for not capping A's give: A gives 0.44 vs 1.66 kWh, quarters 5919/5915 W, 32 give/idle switches in 40 min. These numbers match my `repro_alt.txt`.
- **L-1 fixed.** `ctl_cycle.py` comment is now one line plus the DECISIONS pointer.
- **L-2 fixed.** The grid minimum is now 3950 / 3952 / 3950 W in the PR body table and in the dev report.
- **CI:** runs 1636 (`checks / unit`) and 1637 (`infra-pin / fresh`) both succeeded on e770615 (checked through the API).
- No findings left above Low.
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.
MAINT-24 follow-up (rig finding F-1). With A in maintenance CHARGE and the house over the month aim, the pause ended each time the guard idled, so A flipped charge/give (468 flips in 45 min on the rig, 11 at base
02eac3b). Nowpeakguard.shaping_held()keeps the pause until the house is 300 W under the aim. No cut-back is delayed or weakened; MAINT-24's night rate (1475 W live shape), N=1, the drain and modes off are unchanged.Sim, real
cycle(), house 6500 W for 45 min, banked month peak 6000 W (aim 5950 W):02eac3b1b5f7c9DECISIONS:
goodwe_controller/DECISIONS/MAINT-28-hold-the-pause-in-a-peak.md. Tests:test_maint28.py.CHANGELOG: During a real peak above this month's peak, the monthly battery charge now stays paused instead of restarting every few seconds, until the house load is clear of the limit.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
MAINT-28 review of
7edf848(independent, Opus). Verdict: NOT READY. One docs change only; the code is fine.The code, the tests, CI and the local suite all pass. The only thing blocking is a wrong reason in DECISIONS for the energy trade-off.
Findings
M-1 (Medium, docs)
goodwe_controller/DECISIONS/MAINT-28-hold-the-pause-in-a-peak.md:17. The note says: "A cap on that give would conflict with the guard'slo(lo > hiresolves to 0 W), so it is not tried here." That is not true for the state the hold adds:peakguard.claim()files no claim at all (app/peakguard.py:482).suspendedis true and the shaping claim is dropped as before.So a
hi = 0claim filed only while the pause is held and the guard is idle never meets alo. I measured that option in a scratch tree (not proposed for merge; same 45-min sim,scratch/MAINT-28/review/repro_alt.txt):The choice in this PR is still the better one: one steady give, which is the point of the card. But the stated reason is wrong. Fix: replace that sentence with the real reason and these numbers. A cap is possible. It saves about 1.2 kWh per 45-min peak but brings back a give/idle toggle at the guard's edge (32 in 40 min in the sim). No code change.
L-1 (Low, style)
goodwe_controller/app/ctl_cycle.py:571-574. The comment is 4 lines and has no DECISIONS pointer; the reset comment at 540-542 has one. Make it one line plus the pointer.L-2 (Low, report only). The "grid min in the load" values in the dev report and PR table (452 / 2988 / 3485 W) are the last sample of the load: the house steps down while A is still giving. On all three heads the real minimum in the load is about 3950 W (see the review report). The DECISIONS table does not carry these values, so no change is needed there.
Info. After a real peak, while the house sits 0-300 W under the aim, the charge stays off and A gives for the house. At
1b5f7c9, A would charge 150 W or less in that band. This is rare and only ever moves toward less grid draw.Checks
Numbers match the dev's table exactly: flips, quarters, A charge seconds, out/in kWh, charge back after 3 s, on
02eac3b,1b5f7c9and7edf848.1b5f7c9or7edf848(02eac3b: 596 paused cycles).1b5f7c9, 1 on7edf848.Safety: no issue.
n1_peak, drain and modes-off digests are the same as1b5f7c9.stale_afteror more files the safety SET of 0 W, which wins.maint_aim > peak_aim_w()gate ends the hold on the first cycle of a new month.Give cost. While the pause is held and the guard is idle, no shaping claim is filed, so the loop runs A as a normal battery: A gives a mean of 2496 W, its full rating, and the grid sits near 4000 W. The quarter only needs about 550 W. The cost is about 1.2 kWh more through A per 45-min real peak:
CHARGE_TIMEOUT_MIN(2160 open minutes).Acceptable as is: it buys the steadiness the card asked for. It only needs the correct reason in DECISIONS (M-1).
_maint_peak_holdlifetime: keep as is.cycle()would be worse: each missed poll would restart the charge, one flap per missed poll.Tests.
test_maint28passes on7edf848and fails on1b5f7c9(6 checks).1b5f7c9with a stub, so they are true base values.test_safety84n2_charge_banked: 64 of 480 rows differ. Rows 310-371 (t = 1550-1855 s): A no longer charges -127 W; A and B give 2500 W each while the load runs. Rows 387-388: a small difference at the restart. The other five pins are the same. It moves by design.ctl_cyclereadsdatetimeas a global (line 49), somain_files.livepatches it with the same_DT. It is a faithful stand-in.Docs. The DECISIONS note is short, has no duplicate paragraph, and its sha and numbers are right; the one wrong sentence is M-1. The CHANGELOG line is in the PR body and CHANGELOG.md is not touched. No new user text.
CI and suite.
checks / unit) and 1635 (infra-pin / fresh): success on head_sha7edf848.goodwe-addon-checks:1,--network none,:romount, real hwsim): 107 files, 0 failed, 1593 s wall.test_maint2878 s.Not merged (M-1 is above Low). Details:
reports/MAINT-28-review-report.md.Re-review of
e770615: MERGE-READY as is, and now merged. release/1.0 isf3f9483(normal merge; parents1b5f7c9ande770615).git diff 7edf848 e770615changes only DECISIONS and one code comment. No behaviour change.repro_alt.txt.ctl_cycle.pycomment is now one line plus the DECISIONS pointer.checks / unit) and 1637 (infra-pin / fresh) both succeeded one770615(checked through the API).