CI-17: test_peakguard's real-cycle claim set no longer depends on host load
Cause. In test_peakguard.py the fixture c_src lays its P1 telegrams up to one datetime.now(), then runs ~24 real cycle()s on it (the 23-state idle sweep, then the real two-inverter cycle). Each cycle() reads the real clock again (ctl_cycle.py:49, now = datetime.now(timezone.utc)). Under --cpus 0.25 a cycle takes about 1.3 s, so by the 23rd cycle (soc 95 %) the last telegram is older than max_hold_s (30 s). The guard counts the gap and files peak@site. Measured (debug copy): gap 3.0 s at cycle 1, 29.5 s at soc 90, 30.8 s at soc 95, and peak appears from there. Unloaded the whole run is a few seconds, so it never gets there. Three checks fail: the idle sweep set (10a/10b), the exact _cl2 list and the exact _cl2 source set.
Fix (test only)._SRC_NOW is read once; the telegrams end there and every run_cycle(c_src, at=_SRC_NOW) runs cycle() at that instant (run_cycle pins ctl_cycle.datetime.now for the call and restores it). The exact checks are unchanged. No product change, no retry, no loosened ==.
Mutant: an unconditional extra safety claim in cycle() turns the exact-set checks red (the _cl2 list now shows safety@site; 17 checks fail, among them the two exact _cl2 checks).
Full suite (.gitea/ci/run-tests.sh) and CI: see the report.
F1: the stale/dead fixtures (_bs, _gone) now set rejected_since from _SRC_NOW, the instant their cycles are pinned to, so the cycle's is_live(now) and live_units() (real clock, later) both see the link as stale. A new check asserts both clocks agree for inverter B.
F2: run_cycle's docstring no longer says ctl_cycle.py:49 is the only clock read.
Head 63043ea: --cpus 0.25 10/10 pass, unloaded 10/10 pass, mutant red (17 checks), full suite 108/108 with the sim mounted, CI run 1666 checks / unit green.
## CI-17: test_peakguard's real-cycle claim set no longer depends on host load
**Cause.** In `test_peakguard.py` the fixture `c_src` lays its P1 telegrams up to one `datetime.now()`, then runs ~24 real `cycle()`s on it (the 23-state idle sweep, then the real two-inverter cycle). Each `cycle()` reads the real clock again (`ctl_cycle.py:49`, `now = datetime.now(timezone.utc)`). Under `--cpus 0.25` a cycle takes about 1.3 s, so by the 23rd cycle (soc 95 %) the last telegram is older than `max_hold_s` (30 s). The guard counts the gap and files `peak@site`. Measured (debug copy): gap 3.0 s at cycle 1, 29.5 s at soc 90, 30.8 s at soc 95, and `peak` appears from there. Unloaded the whole run is a few seconds, so it never gets there. Three checks fail: the idle sweep set (10a/10b), the exact `_cl2` list and the exact `_cl2` source set.
**Fix (test only).** `_SRC_NOW` is read once; the telegrams end there and every `run_cycle(c_src, at=_SRC_NOW)` runs `cycle()` at that instant (`run_cycle` pins `ctl_cycle.datetime.now` for the call and restores it). The exact checks are unchanged. No product change, no retry, no loosened `==`.
**Proof** (`goodwe-addon-checks:1`, `--network none`, `--memory`, `timeout`):
- `release/1.0` (6c0546b), `--cpus 0.25`, 10 runs: 10 failed (3 checks each, `peak@site` in the two-inverter list).
- this head, `--cpus 0.25`: 10/10 pass. Unloaded (`--cpus 2`): 10/10 pass.
- Mutant: an unconditional extra `safety` claim in `cycle()` turns the exact-set checks red (the `_cl2` list now shows `safety@site`; 17 checks fail, among them the two exact `_cl2` checks).
- Full suite (`.gitea/ci/run-tests.sh`) and CI: see the report.
No CHANGELOG line (test only).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
## Round 2 (review comment 2630)
- F1: the stale/dead fixtures (`_bs`, `_gone`) now set `rejected_since` from `_SRC_NOW`, the instant their cycles are pinned to, so the cycle's `is_live(now)` and `live_units()` (real clock, later) both see the link as stale. A new check asserts both clocks agree for inverter B.
- F2: `run_cycle`'s docstring no longer says ctl_cycle.py:49 is the only clock read.
- Head 63043ea: `--cpus 0.25` 10/10 pass, unloaded 10/10 pass, mutant red (17 checks), full suite 108/108 with the sim mounted, CI run 1666 `checks / unit` green.
c_src's telegrams were laid up to one datetime.now(); each cycle() then read
the real clock again (ctl_cycle.py:49). Under --cpus 0.25 a cycle takes ~1.3 s,
so by the 23rd sweep cycle the last telegram was older than max_hold_s (30 s),
the guard counted the gap and filed peak@site. The cycles now run AT the
fixture's own instant. Test only; the exact-set checks are unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
CI-17 review (head 39ca1df vs release/1.0 6c0546b): changes requested, not merged.
The cause is right, and the fix removes the flake. Under --cpus 0.25: 6c0546b failed 5/5 (3 checks each, peak@site in _cl2); 39ca1df passed 10/10. The exact checks are byte-identical. A mutant (one extra no-op hold@site claim, two-inverter cycles only) turns both exact _cl2 checks red. CI run 1664 is green.
Finding 1 (medium): the pin puts the stale/dead fixtures on two clocks (test_peakguard.py:3403 / :3412, :3454 / :3461) rejected_since is set from the real clock. These two cycles then run at _SRC_NOW, which is 8-40 s earlier. Inside the cycle, ctl_cycle.py:207 calls is_live(now) with the pinned time and finds inverter-b live. live_units() (ctl_inputs.py:402) reads the real clock and finds it stale. Measured: unloaded the gap is 8.7 s, under load 38 s; is_live(_SRC_NOW) = True both times, is_live(real) = False. Before this PR both clocks agreed. The checks still pass, but they no longer test the stale state they describe.
Fix: set both rejected_since values from _SRC_NOW - timedelta(seconds=LINK_STALE_S + 5). Tried in a scratch copy: both clocks say stale, and it passed 3/3 at --cpus 0.25 and 1/1 unloaded.
Finding 2 (low): wrong comment (test_peakguard.py:2793-2794)
"ctl_cycle.py:49, its only clock read" is false. During cycle(), live_units() (ctl_inputs.py:402) and ctl_trade.py:296 / :635 read the real clock too, and the pin does not cover them (that is Finding 1). Say instead that the pin covers cycle()'s own now only.
No other problems. The pin is restored in finally, and later code reads the real clock (the gap above grows by 37 s). test_price.py:2858 is a different shape: a new fixture per row and one cycle each, so the gap is seconds, not about 30 s. No follow-up card needed.
**CI-17 review (head 39ca1df vs release/1.0 6c0546b): changes requested, not merged.**
The cause is right, and the fix removes the flake. Under `--cpus 0.25`: 6c0546b failed 5/5 (3 checks each, `peak@site` in `_cl2`); 39ca1df passed 10/10. The exact checks are byte-identical. A mutant (one extra no-op `hold@site` claim, two-inverter cycles only) turns both exact `_cl2` checks red. CI run 1664 is green.
**Finding 1 (medium): the pin puts the stale/dead fixtures on two clocks** (`test_peakguard.py:3403` / `:3412`, `:3454` / `:3461`)
`rejected_since` is set from the real clock. These two cycles then run at `_SRC_NOW`, which is 8-40 s earlier. Inside the cycle, `ctl_cycle.py:207` calls `is_live(now)` with the pinned time and finds inverter-b **live**. `live_units()` (`ctl_inputs.py:402`) reads the real clock and finds it **stale**. Measured: unloaded the gap is 8.7 s, under load 38 s; `is_live(_SRC_NOW)` = True both times, `is_live(real)` = False. Before this PR both clocks agreed. The checks still pass, but they no longer test the stale state they describe.
Fix: set both `rejected_since` values from `_SRC_NOW - timedelta(seconds=LINK_STALE_S + 5)`. Tried in a scratch copy: both clocks say stale, and it passed 3/3 at `--cpus 0.25` and 1/1 unloaded.
**Finding 2 (low): wrong comment** (`test_peakguard.py:2793-2794`)
"ctl_cycle.py:49, its only clock read" is false. During `cycle()`, `live_units()` (`ctl_inputs.py:402`) and `ctl_trade.py:296` / `:635` read the real clock too, and the pin does not cover them (that is Finding 1). Say instead that the pin covers `cycle()`'s own `now` only.
No other problems. The pin is restored in `finally`, and later code reads the real clock (the gap above grows by 37 s). `test_price.py:2858` is a different shape: a new fixture per row and one cycle each, so the gap is seconds, not about 30 s. No follow-up card needed.
The two fixtures ran their cycles at _SRC_NOW but set rejected_since from the
real clock, so the cycle saw inverter-b live while live_units() saw it stale.
Both now use _SRC_NOW, and a check asserts both clocks agree. run_cycle's
comment no longer claims ctl_cycle.py:49 is the only clock read.
Co-Authored-By: Claude <noreply@anthropic.com>
admin
merged commit 6bf37597f8 into release/1.02026-11-15 00:50:50 +01:00
CI-17 re-review (head 63043ea): approved and merged.
F1 is fixed: rejected_since and _gone now come from _SRC_NOW. With an instrumented copy, the pinned and real clocks both say stale, for B and for both units in the dead case, unloaded and at --cpus 0.25.
F2 is fixed: the docstring is now accurate. The diff from 39ca1df changes nothing else.
At --cpus 0.25, 63043ea passed 6/6. CI run 1666 checks / unit is green.
**CI-17 re-review (head 63043ea): approved and merged.**
- F1 is fixed: `rejected_since` and `_gone` now come from `_SRC_NOW`. With an instrumented copy, the pinned and real clocks both say stale, for B and for both units in the dead case, unloaded and at `--cpus 0.25`.
- F2 is fixed: the docstring is now accurate. The diff from 39ca1df changes nothing else.
- At `--cpus 0.25`, 63043ea passed 6/6. CI run 1666 `checks / unit` is green.
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.
CI-17: test_peakguard's real-cycle claim set no longer depends on host load
Cause. In
test_peakguard.pythe fixturec_srclays its P1 telegrams up to onedatetime.now(), then runs ~24 realcycle()s on it (the 23-state idle sweep, then the real two-inverter cycle). Eachcycle()reads the real clock again (ctl_cycle.py:49,now = datetime.now(timezone.utc)). Under--cpus 0.25a cycle takes about 1.3 s, so by the 23rd cycle (soc 95 %) the last telegram is older thanmax_hold_s(30 s). The guard counts the gap and filespeak@site. Measured (debug copy): gap 3.0 s at cycle 1, 29.5 s at soc 90, 30.8 s at soc 95, andpeakappears from there. Unloaded the whole run is a few seconds, so it never gets there. Three checks fail: the idle sweep set (10a/10b), the exact_cl2list and the exact_cl2source set.Fix (test only).
_SRC_NOWis read once; the telegrams end there and everyrun_cycle(c_src, at=_SRC_NOW)runscycle()at that instant (run_cyclepinsctl_cycle.datetime.nowfor the call and restores it). The exact checks are unchanged. No product change, no retry, no loosened==.Proof (
goodwe-addon-checks:1,--network none,--memory,timeout):release/1.0(6c0546b),--cpus 0.25, 10 runs: 10 failed (3 checks each,peak@sitein the two-inverter list).--cpus 0.25: 10/10 pass. Unloaded (--cpus 2): 10/10 pass.safetyclaim incycle()turns the exact-set checks red (the_cl2list now showssafety@site; 17 checks fail, among them the two exact_cl2checks)..gitea/ci/run-tests.sh) and CI: see the report.No CHANGELOG line (test only).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
Round 2 (review comment 2630)
_bs,_gone) now setrejected_sincefrom_SRC_NOW, the instant their cycles are pinned to, so the cycle'sis_live(now)andlive_units()(real clock, later) both see the link as stale. A new check asserts both clocks agree for inverter B.run_cycle's docstring no longer says ctl_cycle.py:49 is the only clock read.63043ea:--cpus 0.2510/10 pass, unloaded 10/10 pass, mutant red (17 checks), full suite 108/108 with the sim mounted, CI run 1666checks / unitgreen.CI-17 review (head
39ca1dfvs release/1.06c0546b): changes requested, not merged.The cause is right, and the fix removes the flake. Under
--cpus 0.25:6c0546bfailed 5/5 (3 checks each,peak@sitein_cl2);39ca1dfpassed 10/10. The exact checks are byte-identical. A mutant (one extra no-ophold@siteclaim, two-inverter cycles only) turns both exact_cl2checks red. CI run 1664 is green.Finding 1 (medium): the pin puts the stale/dead fixtures on two clocks (
test_peakguard.py:3403/:3412,:3454/:3461)rejected_sinceis set from the real clock. These two cycles then run at_SRC_NOW, which is 8-40 s earlier. Inside the cycle,ctl_cycle.py:207callsis_live(now)with the pinned time and finds inverter-b live.live_units()(ctl_inputs.py:402) reads the real clock and finds it stale. Measured: unloaded the gap is 8.7 s, under load 38 s;is_live(_SRC_NOW)= True both times,is_live(real)= False. Before this PR both clocks agreed. The checks still pass, but they no longer test the stale state they describe.Fix: set both
rejected_sincevalues from_SRC_NOW - timedelta(seconds=LINK_STALE_S + 5). Tried in a scratch copy: both clocks say stale, and it passed 3/3 at--cpus 0.25and 1/1 unloaded.Finding 2 (low): wrong comment (
test_peakguard.py:2793-2794)"ctl_cycle.py:49, its only clock read" is false. During
cycle(),live_units()(ctl_inputs.py:402) andctl_trade.py:296/:635read the real clock too, and the pin does not cover them (that is Finding 1). Say instead that the pin coverscycle()'s ownnowonly.No other problems. The pin is restored in
finally, and later code reads the real clock (the gap above grows by 37 s).test_price.py:2858is a different shape: a new fixture per row and one cycle each, so the gap is seconds, not about 30 s. No follow-up card needed.CI-17 re-review (head
63043ea): approved and merged.rejected_sinceand_gonenow come from_SRC_NOW. With an instrumented copy, the pinned and real clocks both say stale, for B and for both units in the dead case, unloaded and at--cpus 0.25.39ca1dfchanges nothing else.--cpus 0.25,63043eapassed 6/6. CI run 1666checks / unitis green.