MAINT-20 part 1 of 3: comments and docstrings in peakguard.py, price.py, price_history.py, price_forecast.py, price_chain.py and custom_formula.py cut to what the code does, the why where it is not obvious, and a DECISIONS pointer. No code change, no behaviour change, no CHANGELOG entry, no version change.
Proof of no code change:python3 goodwe_controller/tools/maint20_same_code.py release/1.0 (from the repository root). For each touched file, ast.dump() without docstrings is identical at release/1.0 and at the head. It also prints the ratio (comment-only + docstring lines over code lines, via tokenize).
file
code
comment+doc before
after
ratio before
after
peakguard.py
273
893
119
3.27
0.44
price.py
830
1308
343
1.58
0.41
price_history.py
303
576
112
1.90
0.37
custom_formula.py
303
533
107
1.76
0.35
price_forecast.py
215
362
60
1.68
0.28
price_chain.py
120
79
44
0.66
0.37
priceruns.py was already at 0.45 and is not touched.
DECISIONS: new SAFETY-07-peak-guard, TARIFF-02-price-provider-plugins, TARIFF-16-the-injection-leg, FCST-03-the-hours-not-yet-published; FCST-04-the-day-that-is-carried gets section 5 (store details). The long reasoning that was in the module comments is there. One commit per file, plus one per DECISIONS note.
Do not merge before the reviewer has read the DECISIONS notes against the removed comments.
MAINT-20 part 1 of 3: comments and docstrings in `peakguard.py`, `price.py`, `price_history.py`, `price_forecast.py`, `price_chain.py` and `custom_formula.py` cut to what the code does, the why where it is not obvious, and a DECISIONS pointer. No code change, no behaviour change, no CHANGELOG entry, no version change.
**Proof of no code change:** `python3 goodwe_controller/tools/maint20_same_code.py release/1.0` (from the repository root). For each touched file, `ast.dump()` without docstrings is identical at `release/1.0` and at the head. It also prints the ratio (comment-only + docstring lines over code lines, via `tokenize`).
| file | code | comment+doc before | after | ratio before | after |
|---|---|---|---|---|---|
| peakguard.py | 273 | 893 | 119 | 3.27 | 0.44 |
| price.py | 830 | 1308 | 343 | 1.58 | 0.41 |
| price_history.py | 303 | 576 | 112 | 1.90 | 0.37 |
| custom_formula.py | 303 | 533 | 107 | 1.76 | 0.35 |
| price_forecast.py | 215 | 362 | 60 | 1.68 | 0.28 |
| price_chain.py | 120 | 79 | 44 | 0.66 | 0.37 |
`priceruns.py` was already at 0.45 and is not touched.
DECISIONS: new `SAFETY-07-peak-guard`, `TARIFF-02-price-provider-plugins`, `TARIFF-16-the-injection-leg`, `FCST-03-the-hours-not-yet-published`; `FCST-04-the-day-that-is-carried` gets section 5 (store details). The long reasoning that was in the module comments is there. One commit per file, plus one per DECISIONS note.
Do not merge before the reviewer has read the DECISIONS notes against the removed comments.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
Comment, docstring and DECISIONS only (same-code script OK; python -OO
bytecode now identical for all six files).
- peakguard.charge_room_w: one-line docstring + SAFETY-125 pointer (cut whole).
- peakguard.shaping_suspended: keep 'the one place this conjunction is
written; do not inline it'.
- price_forecast module note: an hour with no price and no yesterday is left
out, never filled (test_price.py 14d cites the module note for it).
- Refill comment blocks the cut left ragged; DECISIONS pointers on one line.
- FCST-04 note: _yesterday_of(), not _previous_day(); FCST-03 note: the 49
stronger cases are why the rule is a real change.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
Reviewed head 6e2fde6, base release/1.08afadbd (both as expected). I pushed one comment/DECISIONS-only commit, 63dda4b.
Checks
No code change: tools/maint20_same_code.py release/1.0 OK, 6/6 SAME. My own check: compile(optimize=2) of base vs head, comparing the code objects recursively (bytecode, consts, names, flags, exception table; line tables ignored). On 6e2fde6 the only difference was one NOP in charge_room_w: its docstring was cut entirely, which drops the line-event NOP. That is not a semantic change. After 63dda4b all six files are byte-identical. The plugin SOURCES: / The framework ADDS: docstring lines are unchanged. Only charge_room_w lost its docstring completely. No __doc__ is read at runtime in app/. code_leaks() reads source through the AST, so comments don't affect it.
Ratios match the dev table (manager's ratio.py). After 63dda4b: peakguard 0.45, price 0.40, price_history 0.35, custom_formula 0.35, price_chain 0.37, price_forecast 0.28.
DECISIONS notes: I read SAFETY-07, TARIFF-02, TARIFF-16, FCST-03 and FCST-04 section 5 against the removed text. They are faithful and concise. I spot-checked figures (878 Wh, 1.6 days, 0x0560, 162 B, 08:17, 70 of 71, 149 040, 40 h soak, WITNESS-03, 2026-03-30T00:30): every one is in the removed text, so no reasoning was invented. Every DECISIONS/... and [[ID]] pointer resolves.
Full suite locally on 6e2fde6 (goodwe-addon-checks:1, --network none, GOODWE_REQUIRE_HWSIM=1, sim at the infra pin): 93 files, 0 failed, 1260 s. On 63dda4b: test_peakguard, test_price, test_price_history, test_price_forecast, test_named_absence, test_docs, test_bytecode and test_safety125 all pass.
CI on 6e2fde6: run 1482 (unit) and run 1483 (infra-pin) both success.
Findings
MEDIUM, lost and restored. app/peakguard.pycharge_room_w: the docstring was cut entirely, which took the SAFETY-125 pointer with it. Restored as one line plus the pointer.
MEDIUM, lost and restored. app/peakguard.pyshaping_suspended: the "one place this conjunction is written, public so the test grades the shipped expression" warning was lost, and it is a "do not inline" rule. Restored as one line.
LOW, lost and restored. app/price_forecast.py module note: "an hour with no published price and no yesterday value is left out, never filled" now lives only in FCST-03, but test_price.py:3129 cites "price_forecast.py's module note" for it. Restored as one sentence.
LOW, fixed. The re-wrap left many ragged comment blocks (one- to three-word lines mid-paragraph) in price.py, price_history.py, peakguard.py, custom_formula.py and price_forecast.py. Two pointers were also split across lines (DECISIONS/TARIFF-02-price- / provider-plugins.md, price.py:43 and :197), which breaks a grep for the note name. Refilled to 79 columns, comment lines only.
LOW, fixed. DECISIONS/FCST-04 section 5 names _previous_day(). No such function exists; it is _yesterday_of().
LOW, fixed. DECISIONS/FCST-03: "(49 cases scored stronger), so the rule as stated is wrong" mixes up two arguments. The 49 cases show the rule is a real change; the red horizon checks are what show it is wrong. Reworded.
LOW, not fixed, out of scope. config.yaml:347 says "See peakguard.py's module note". The content now lives in DECISIONS/SAFETY-07 (the module note points there), so the pointer still works but goes the long way. This is a candidate for DOC-3/MAINT-6.
INFO, script. maint20_same_code.py crashes with CalledProcessError on a file that is added or deleted under app/, and with ZeroDivisionError on a file with no code (e.g. an empty __init__.py). It ignores changes outside app/ and does not compare docstrings that are read at runtime. That is fine for this PR. For parts 2-3 I suggest adding: added/removed files counted as CODE DIFFERS, a guard for zero code lines, and a check that the diff touches only app/*.py, DECISIONS/*.md and the script. I would not put it in the general CI, because any real code PR would fail it. Wiring it only for MAINT-20-* branches is possible but not needed.
Verdict: approve. I merge once checks / unit (pull_request) is green on 63dda4b.
## MAINT-20 part 1 review (independent, Opus)
Reviewed head `6e2fde6`, base `release/1.0` `8afadbd` (both as expected). I pushed one comment/DECISIONS-only commit, `63dda4b`.
**Checks**
- No code change: `tools/maint20_same_code.py release/1.0` OK, 6/6 SAME. My own check: `compile(optimize=2)` of base vs head, comparing the code objects recursively (bytecode, consts, names, flags, exception table; line tables ignored). On `6e2fde6` the only difference was one `NOP` in `charge_room_w`: its docstring was cut entirely, which drops the line-event NOP. That is not a semantic change. After `63dda4b` all six files are byte-identical. The plugin `SOURCES:` / `The framework ADDS:` docstring lines are unchanged. Only `charge_room_w` lost its docstring completely. No `__doc__` is read at runtime in `app/`. `code_leaks()` reads source through the AST, so comments don't affect it.
- Ratios match the dev table (manager's `ratio.py`). After `63dda4b`: peakguard 0.45, price 0.40, price_history 0.35, custom_formula 0.35, price_chain 0.37, price_forecast 0.28.
- DECISIONS notes: I read SAFETY-07, TARIFF-02, TARIFF-16, FCST-03 and FCST-04 section 5 against the removed text. They are faithful and concise. I spot-checked figures (878 Wh, 1.6 days, 0x0560, 162 B, 08:17, 70 of 71, 149 040, 40 h soak, WITNESS-03, 2026-03-30T00:30): every one is in the removed text, so no reasoning was invented. Every `DECISIONS/...` and `[[ID]]` pointer resolves.
- Full suite locally on `6e2fde6` (`goodwe-addon-checks:1`, `--network none`, `GOODWE_REQUIRE_HWSIM=1`, sim at the infra pin): 93 files, 0 failed, 1260 s. On `63dda4b`: test_peakguard, test_price, test_price_history, test_price_forecast, test_named_absence, test_docs, test_bytecode and test_safety125 all pass.
- CI on `6e2fde6`: run 1482 (unit) and run 1483 (infra-pin) both success.
**Findings**
1. MEDIUM, lost and restored. `app/peakguard.py` `charge_room_w`: the docstring was cut entirely, which took the SAFETY-125 pointer with it. Restored as one line plus the pointer.
2. MEDIUM, lost and restored. `app/peakguard.py` `shaping_suspended`: the "one place this conjunction is written, public so the test grades the shipped expression" warning was lost, and it is a "do not inline" rule. Restored as one line.
3. LOW, lost and restored. `app/price_forecast.py` module note: "an hour with no published price and no yesterday value is left out, never filled" now lives only in FCST-03, but `test_price.py:3129` cites "price_forecast.py's module note" for it. Restored as one sentence.
4. LOW, fixed. The re-wrap left many ragged comment blocks (one- to three-word lines mid-paragraph) in price.py, price_history.py, peakguard.py, custom_formula.py and price_forecast.py. Two pointers were also split across lines (`DECISIONS/TARIFF-02-price-` / `provider-plugins.md`, price.py:43 and :197), which breaks a grep for the note name. Refilled to 79 columns, comment lines only.
5. LOW, fixed. `DECISIONS/FCST-04` section 5 names `_previous_day()`. No such function exists; it is `_yesterday_of()`.
6. LOW, fixed. `DECISIONS/FCST-03`: "(49 cases scored stronger), so the rule as stated is wrong" mixes up two arguments. The 49 cases show the rule is a real change; the red horizon checks are what show it is wrong. Reworded.
7. LOW, not fixed, out of scope. `config.yaml:347` says "See peakguard.py's module note". The content now lives in DECISIONS/SAFETY-07 (the module note points there), so the pointer still works but goes the long way. This is a candidate for DOC-3/MAINT-6.
8. INFO, script. `maint20_same_code.py` crashes with CalledProcessError on a file that is added or deleted under app/, and with ZeroDivisionError on a file with no code (e.g. an empty `__init__.py`). It ignores changes outside `app/` and does not compare docstrings that are read at runtime. That is fine for this PR. For parts 2-3 I suggest adding: added/removed files counted as CODE DIFFERS, a guard for zero code lines, and a check that the diff touches only `app/*.py`, `DECISIONS/*.md` and the script. I would not put it in the general CI, because any real code PR would fail it. Wiring it only for `MAINT-20-*` branches is possible but not needed.
Verdict: approve. I merge once `checks / unit (pull_request)` is green on `63dda4b`.
admin
merged commit d4c01daca5 into release/1.02026-11-01 03:24:55 +01: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.
MAINT-20 part 1 of 3: comments and docstrings in
peakguard.py,price.py,price_history.py,price_forecast.py,price_chain.pyandcustom_formula.pycut to what the code does, the why where it is not obvious, and a DECISIONS pointer. No code change, no behaviour change, no CHANGELOG entry, no version change.Proof of no code change:
python3 goodwe_controller/tools/maint20_same_code.py release/1.0(from the repository root). For each touched file,ast.dump()without docstrings is identical atrelease/1.0and at the head. It also prints the ratio (comment-only + docstring lines over code lines, viatokenize).priceruns.pywas already at 0.45 and is not touched.DECISIONS: new
SAFETY-07-peak-guard,TARIFF-02-price-provider-plugins,TARIFF-16-the-injection-leg,FCST-03-the-hours-not-yet-published;FCST-04-the-day-that-is-carriedgets section 5 (store details). The long reasoning that was in the module comments is there. One commit per file, plus one per DECISIONS note.Do not merge before the reviewer has read the DECISIONS notes against the removed comments.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
MAINT-20 part 1 review (independent, Opus)
Reviewed head
6e2fde6, baserelease/1.08afadbd(both as expected). I pushed one comment/DECISIONS-only commit,63dda4b.Checks
tools/maint20_same_code.py release/1.0OK, 6/6 SAME. My own check:compile(optimize=2)of base vs head, comparing the code objects recursively (bytecode, consts, names, flags, exception table; line tables ignored). On6e2fde6the only difference was oneNOPincharge_room_w: its docstring was cut entirely, which drops the line-event NOP. That is not a semantic change. After63dda4ball six files are byte-identical. The pluginSOURCES:/The framework ADDS:docstring lines are unchanged. Onlycharge_room_wlost its docstring completely. No__doc__is read at runtime inapp/.code_leaks()reads source through the AST, so comments don't affect it.ratio.py). After63dda4b: peakguard 0.45, price 0.40, price_history 0.35, custom_formula 0.35, price_chain 0.37, price_forecast 0.28.DECISIONS/...and[[ID]]pointer resolves.6e2fde6(goodwe-addon-checks:1,--network none,GOODWE_REQUIRE_HWSIM=1, sim at the infra pin): 93 files, 0 failed, 1260 s. On63dda4b: test_peakguard, test_price, test_price_history, test_price_forecast, test_named_absence, test_docs, test_bytecode and test_safety125 all pass.6e2fde6: run 1482 (unit) and run 1483 (infra-pin) both success.Findings
app/peakguard.pycharge_room_w: the docstring was cut entirely, which took the SAFETY-125 pointer with it. Restored as one line plus the pointer.app/peakguard.pyshaping_suspended: the "one place this conjunction is written, public so the test grades the shipped expression" warning was lost, and it is a "do not inline" rule. Restored as one line.app/price_forecast.pymodule note: "an hour with no published price and no yesterday value is left out, never filled" now lives only in FCST-03, buttest_price.py:3129cites "price_forecast.py's module note" for it. Restored as one sentence.DECISIONS/TARIFF-02-price-/provider-plugins.md, price.py:43 and :197), which breaks a grep for the note name. Refilled to 79 columns, comment lines only.DECISIONS/FCST-04section 5 names_previous_day(). No such function exists; it is_yesterday_of().DECISIONS/FCST-03: "(49 cases scored stronger), so the rule as stated is wrong" mixes up two arguments. The 49 cases show the rule is a real change; the red horizon checks are what show it is wrong. Reworded.config.yaml:347says "See peakguard.py's module note". The content now lives in DECISIONS/SAFETY-07 (the module note points there), so the pointer still works but goes the long way. This is a candidate for DOC-3/MAINT-6.maint20_same_code.pycrashes with CalledProcessError on a file that is added or deleted under app/, and with ZeroDivisionError on a file with no code (e.g. an empty__init__.py). It ignores changes outsideapp/and does not compare docstrings that are read at runtime. That is fine for this PR. For parts 2-3 I suggest adding: added/removed files counted as CODE DIFFERS, a guard for zero code lines, and a check that the diff touches onlyapp/*.py,DECISIONS/*.mdand the script. I would not put it in the general CI, because any real code PR would fail it. Wiring it only forMAINT-20-*branches is possible but not needed.Verdict: approve. I merge once
checks / unit (pull_request)is green on63dda4b.