Repository navigation
test(site): run Core against device-simulator, short set on PRs, full set nightly - #1797
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d09cf0c17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if r.Site == nil { | ||
| return out | ||
| } |
There was a problem hiding this comment.
Reject missing simulator measurements before judging safety
If the simulator's site endpoint fails during a judged window, the recorder still writes timestamped samples containing site_err, but sample discards that error and this fallback turns the missing measurement into zero current. blown likewise returns false for a missing site. Consequently, safety-only judges such as S3, S8, S9 and S10 can report success without observing the fuse or phase currents; the longest-gap check does not catch this because samples continue arriving. Validate measurement completeness before evaluating these criteria, and fail the run when required simulator readings are unavailable.
Useful? React with 👍 / 👎.
| return m, []criterion{ | ||
| crit("fuse holds", !before["fuse_blown"].(bool) && !after["fuse_blown"].(bool)), | ||
| crit("over fuse - 1 A at most 3 s after the step", after["over_limit_longest_s"].(float64) <= 3), | ||
| } |
There was a problem hiding this comment.
Assert that both chargers reach the load-sharing setup
If multi-charger dispatch stops charging one or both cars, S10 still passes: its only criteria check fuse safety, while pre_step_w is merely recorded. plug confirms connection and charger recognition, not power delivery, and successful manual-hold requests do not establish that both cars reached 10 A before the step. This allows the test to pass without exercising the delayed release of capacity it is intended to cover, even when the single-charger scenarios remain healthy. Require both chargers to reach the intended pre-step draw before applying and judging the cut/raise sequence.
Useful? React with 👍 / 👎.
|
Back to draft: direction changed (Fredrik, 22 Oct). The rig must not run in GitHub Actions; a runner on its own machine will publish |
58aaea1 to
d66b449
Compare
7e30d42 to
4d7b0b1
Compare
… machine of its own The end-to-end check from the 21 October simulator test becomes a maintained part of the stack, split as Fredrik asked: the house lives in srcfl/device-simulator (its headless run builds it from a spec, applies a timeline and faults, and records what flows through each fuse), and FTW keeps what is FTW's. go/test/site (test files only) holds the scenario pack: the house spec, FTW's configuration and requests, and criteria read from the house's record and FTW's API, for every case of the report plus s11 (pause, then restart), s9r, s5t, s12 (the one-phase night), s13 (socket reconnect) and s14 (a one-phase car). A criterion that fails on master is marked known; when a fix makes it pass the run fails until the mark goes. Each case names the fault-catalog entries it covers and its sources. FTW_SITE picks short, full, names or random:<seed>:<count>; lanes run several cases at once on ports 100 apart. A random seed draws FTW's side and the house draws its own timeline from the same seed; eight invariants are checked every second, and a failing seed is cut after its first violation and kept for the full set. make siterig, siterig-full, siterig-random and siterig-coverage run it. go/test/site/runner is siterig-runner: on its own machine it runs the short set on pull requests that touch control, loadpoint, OCPP or telemetry code and the full set on each master commit, and posts siterig/short, siterig/full and siterig/random commit statuses. Nothing runs in GitHub Actions. Core is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2c4fc4f1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if err != nil { | ||
| st.mark("ftw:error") | ||
| } |
There was a problem hiding this comment.
Fail random runs when an FTW action cannot be applied
If a generated pause, plan, ceiling change or restart fails, the run only records ftw:error and continues successfully. judgeInvariants never checks that mark, so the seed can pass without exercising its requested actions—for example, a rejected plan leaves an idle, fuse-safe house, and a failed restart can leave FTW unavailable without violating the safety-only checks. Return the action error or add a mandatory action-success criterion so broken API requests and restart failures cannot produce a successful random status.
Useful? React with 👍 / 👎.
| o = judgeRun(sc, rundir, meta, o) | ||
| t.Logf("%s: %s %v", sc.name, o.Verdict, append(o.Failures, o.Known...)) | ||
| outs = append(outs, o) |
There was a problem hiding this comment.
Propagate recorded-run failures to the judge test result
When rejudging a recording produces failed criteria or a loading error, TestJudge only logs and saves the outcome; it never marks the test failed. Consequently, make siterig-judge exits successfully even when the revised criteria reject the recording, including when a known criterion now passes and requires its exemption to be removed. Check o.Error and o.Failures as TestSite does so the command's exit status reflects the rejudged results.
Useful? React with 👍 / 👎.
Every change to Core should meet a simulated house with a real fuse (#1772). On 21 October a rig outside the repo ran the real
ftwbinary against srcfl/device-simulator and found four bugs that review andgo/internal/sitesimmissed. The release check was manual. This PR makes it a maintained part of the stack. Core is unchanged.The split (Fredrik, 22 October)
The house lives in one place, srcfl/device-simulator; FTW keeps only what is FTW's.
fuse/package), Sungrow battery following EMS writes, OCPP chargers and the car, models keyed by device-drivers driver id with per-model timingsimulator run -spec: port base, timeline,/api/run/event, record (plant.jsonl, OCPP frames), seeded random timelinesmake siterig,siterig-full,siterig-random,siterig-coveragesiterig-runneron its own machine, postingsiterig/short,siterig/full,siterig/randomcommit statuses. No GitHub Actions.siterig/full(#1798, proposal)Paired PRs, all merged into device-simulator's
main: srcfl/device-simulator#8 (the six gaps the first rig patched around, driver ids, model parameters), #9 (the thermalfusepackage), #10 (headless runs) and #11 (faults and random timelines).go/test/site/devsim.versionpinsmainata3030d3.What is here
go/test/site(test files only):TestSitestartssimulator runwith a house spec per scenario, startsftwagainst it, drives FTW's API, records/api/statusand/api/loadpointsonce a second, and judges each run from what flowed through the fuse (the house's record) and what FTW said. A criterion in a scenario'sknownmap must still fail; when a fix makes it pass, the run fails until the fix deletes the line. A record with a hole over 15 s (a sleeping laptop) fails as "host paused".make siterig-judge DIR=…judges a recorded run with the criteria as they are now, so a changed criterion or known line is checked against a 56-minute run in a second.FTW_SITE_LANES=nruns n scenarios at once, 100 ports apart fromFTW_SITE_PORT_BASE.FTW_SITE_SHARD=k/nsplits a set over machines.FTW_SITE=random:<first seed>:<count>. Each seed draws FTW's side (fuse, house, phase mode and steps, Charge now, pause, plan, Core restart, import ceiling at random times); the house draws its own timeline from the same seed. Eight invariants every second: the fuse holds; over the limit no longer than the fastest lever + 2 s + 3 s after an outside step, and at most 3 s after FTW's own raise; Home use measured while every device reads; Charge now charges when there is room; no offer above the charger's rating; no discharge asked at the battery's floor; no raise on a stale meter. A failing seed is cut a minute after its first violation and written tofound/<seed>.json; copied totestdata/found/, it runs in the full set, known until triaged.make siterig-random SEED=nreplays a seed.reports/felkatalog/catalog.yamlin the FTW stack workspace) it covers and its sources;make siterig-coverage CATALOG=…prints which faults have a scenario, per area. Today 29 of 840 entries have one; infuse_charging14 of 112 faults, 7 of its 34 open ones.siterig-runner(go/test/site/runner, stdlib only): pollsmasterand open pull requests withgh api, runs the short set on PR heads that touch control, loadpoint, OCPP or telemetry code and the full set on each newmastercommit, a night's random seeds once a day, and posts the result on the exact SHA with counts in the description. It skips forks unless-allow-forks: the rig runs the pull request's code. launchd and systemd files and install steps in its README. This Mac can run it today, a dedicated box later; never the home energy box.Results
The full set on this Mac, three lanes, Core =
masterat bbeac31 (7202ace only adds tests), device-simulator at 76e3fdf, judged again with this branch's criteria (make siterig-judge; s5t rerun with the charger that switches phases): 16 pass, 7 fail as known, 0 unexpected (23 scenarios; s7s runs in the short set only). It took 72 minutes; s12 alone takes 56, so four lanes would not be faster.✓ no OCPP disconnects
✓ no OCPP disconnects
✓ holds 95 % of the time
✓ fuse holds
✓ holds 95 % of the time
✓ fuse holds
✓ no fuse_limit with room to spare
✓ fuse holds
✓ over fuse - 1 A no longer than the response time + 3 s after the step
✓ over fuse - 1 A no longer than the response time + 3 s after the step
surplus_unlock_bat_socsaved but not armed, then Charge now✓ no offer drops to 0 W
✓ car starts
✓ full power over the last minute
✓ no offer drops to 0 W
✓ car starts
✓ full power over the last minute
✓ fuse holds
✓ only valid steps (6-16 A)
✗ charges at least the minimum 90 % of the time
✓ fuse holds
✓ only valid steps (6-16 A)
✗ charges at least the minimum 90 % of the time
✓ Home use marked estimated throughout the outage
✓ measured before the outage
✓ measured after the outage
✓ never estimated while every driver reads
✓ Home use back within 30 s
✓ Charge now never stops
✓ full power within 420 s
✓ over fuse - 1 A no longer than the charger's delay + 7 s
✓ over fuse - 1 A no longer than the charger's delay + 7 s
✓ over fuse - 1 A at most 3 s on 3p
✓ over fuse - 1 A at most 3 s after the switch to 1p
✓ over fuse - 1 A at most 3 s on 3p
✓ over fuse - 1 A at most 3 s after the switch to 1p
✓ over fuse - 1 A at most 3 s after the step
✓ over fuse - 1 A at most 3 s
✓ fuse holds
✓ fuse holds
✗ charges at least 6 A on one phase 90 % of the time
✓ planned charging never orders one phase
✗ no order cut while every phase has room
✓ at least 90 % of the plan by the ready time
✓ no unplug seen
✗ never called not following
What the new cases found on
master:not_following(power_below_target). Known until fix D.fuse_limit(see fix(loadpoint): planned charging runs on three phases, and room never cuts a running order #1799 below). s13 ends Charge now at a socket reconnect (Should a charger socket reconnect end Charge now? #1788).On the pin as pushed (
26430d2, with Codex's eight fixes): the short set plus s9, s9r, s11 and s13 gave the same verdicts as on 76e3fdf: 10 pass, s13 known (33 minutes, two lanes).Against #1799's head (74603d0): s5t still fails. Core now orders 16 A on one phase, then cuts it to 0 W within 4 s for
fuse_limit, while each phase exports 5.5 A and/api/statusshowsphase_amps[5.5, 5.5, 5.5] withphase_powers−1 270 W each: export read as import, the same fault s2 keeps known. So s5t's known line names both. s12, on the other hand, is fixed there: no cuts, 6 862 Wh of the 6 600 planned, the first order 57 s after the plan (8 A, rising to 15 A on three phases), so its known line points at the right fix and goes once this branch is on amasterwith #1799.Random mode: seed 1 (52 events over 20 min) tripped "no discharge asked at the battery's floor" for 902 s (catalog ev-099); random cases tolerate that finding until it is fixed.
Update: master 2e84710 and a one-phase charger (22 October, afternoon)
The branch is now on
master8ac2750 (#1799, #1795, #1802, #1801, #1804). The full set on 2e84710 (72 min, three lanes; device-simulator 26430d2, drivers 5cc7bb1a), judged with this branch's criteria:fuse_limit_with_room_s, seconds offuse_limitwith 6 A of room: each ramp step against the 40 s charger waits up to 144 s.not_following, but the one-phase car now stops. After a replan (reactive-loadin one run,loadpoint_soc_changedin a second) the plan has no EV energy,commanded_w0, reasonplan: 0 W and 128 W over the last 10 min. Before fix(loadpoint): a one-phase car on a three-phase charger is seen, planned and ordered as one #1801 (62ed7ea) the car kept 3.1 kW. Its known lines are gone, so the set fails until the fix (in progress) lands.New cases, with device-simulator#11's
wired_phases: 1(a charger wired on L1 gives an order's current there, up to its rating):s15-25one-phase charger, Charge now as the app sends it (short set)fuse_limits15b-25the same, phase mode1ps15c-25the same, phase modeautos15p-25one-phase charger, plan for 600 Wh(At the time,
devsim.versionpinned #11's head, cba7142.) The full set is now about 270 scenario-minutes: about 90 min in three lanes, 70 in four.Release v0.155.1-beta.1: 16c4ad0 (22 October)
The full set on the release commit: 28 scenarios, four lanes, 63 minutes, drivers 5cc7bb1a, device-simulator cba7142. Judged with this branch at 4d7b0b1: 24 pass, 4 known, 0 fail;
siterig/fullsuccess is posted on 16c4ad0.s5t's known line was the only failure, because it passes now; it is gone.
s15bat-16(one-phase charger, 16 A, Charge now while Core's battery raise is on its way; added for #1805): 8ac2750 fails (car charging after 40 s), 5b697a5 and 16c4ad0 pass (3.1 s and 2.1 s). It needs the battery to follow 8 s late; with the model's 2 s, Charge now catches a raise in flight only by chance.The thermal fuse (23 October)
device-simulator#9 merged: each phase's fuse is now a thermal model, within IEC 60269-1's gates and following ABB's gG curves for 16-63 A. The stack #8 → #10 → #11 is rebased on it.
devsim.versionpinned Docs: per-package CLAUDE.md for IO + plumbing layers #11's head, 3bf737d; it now pins device-simulator'smain, a3030d3, the same tree.site.fuse_ambient_candsite.fuse_preheat_a, so a scenario can start with warm fuses; no scenario uses them yet.Ready (23 October)
On
master7cccf99 (with #1812) and device-simulatormaina3030d3, drivers 219c462e: the short set passes 8 of 8 (s0, s1-16, s1-25, s3-16, s3-25, s7s, s8-60, s15; 23 minutes, two lanes, device-simulator fetched by the pin). The beta gate (#1798) stays a draft until the own-machine runner has run steadily for a few days.🤖 Generated with Claude Code