Skip to content

fix(control): size per-phase relief by the site's phase count - #812

Merged
frahlg merged 1 commit into
masterfrom
agent/fix-per-phase-relief-phase-count
Aug 5, 2026
Merged

fix(control): size per-phase relief by the site's phase count#812
frahlg merged 1 commit into
masterfrom
agent/fix-per-phase-relief-phase-count

Conversation

@frahlg

@frahlg frahlg commented Aug 5, 2026

Copy link
Copy Markdown
Member

Replaces #808, which GitHub closed by itself when #803 merged and its base branch was deleted. A force-push after that close blocks a reopen, so the same commit lands here instead: rebased onto master with #803's now-merged commit dropped. The content did not change — git diff master...HEAD is byte-identical to the diff #808 carried.

One commit on master now. #809 stacks on this one.

The bug

perPhaseOverageW(...) * 3.0 was written twice in dispatch.go — once in applyFuseGuard, once in forceFuseDischarge — while SiteFusePhases sat in the same State struct and fuseSafetyMarginW, two functions away, read it correctly and says so in its own comment: "No hardcoded 230 V / 3 phases here — both come from config."

A worst-phase overage is watts on one phase. Dispatch commands aggregate battery watts. The conversion between the two is the site's phase count: a battery spreads its action across the phases, so each aggregate watt relieves 1/N on the worst phase and an overage of N watts needs N × phases of action. Writing that as 3.0 is only right on a three-phase site.

On a 1-phase site the aggregate meter and the single phase are the same wire, so the two numbers must agree — and they didn't:

1Φ site, 16 A fuse, 0.5 A margin, meter at 5520 W (24 A) relief demanded resulting meter
correct (× 1 phase) 1955 W 3565 W import — exactly the ceiling
before (× 3) 5865 W 345 W export — through zero, wrong side of the same breaker

perPhaseOverageW had two more of the same flavour. It walked L1/L2/L3 unconditionally, so current reported on a phase a 1Φ site does not own fired the clamp on a site nowhere near its fuse. And it fell back to a hardcoded 230 for voltage — the exact thing fuseSafetyMarginW refuses to do.

The fix

One helper, perPhaseReliefW, owns the conversion, and both call sites use it.

  • Phase count comes from SiteFusePhases.
  • Voltage comes from SiteFuseVoltage, the way every other watt in this file gets it (fuseSafetyMarginW, Fuse.MaxPowerW, main.go's fuseMaxW). No invented 230 V.
  • Only the configured phases are read, in L1/L2/L3 order — the same rule sitePhaseCurrentsAt already follows in cmd/ftw/site_dispatch_safety.go.
  • A fuse described by amps alone now gets no per-phase clamp instead of one computed from numbers nobody configured. config.go defaults fuse.voltage to 230 and fuse.phases to 3 and rejects <= 0 in validation, and control_state.go copies all three fields, so no configured site loses protection — only harnesses that wire SiteFuseAmps alone.

Was the import-only gate intentional or drift?

Intentional, and it stays. The two call sites genuinely need different behaviour:

  • forceFuseDischarge has exactly one lever: command more discharge. That relieves an import-side phase and worsens an export-side one. Honouring a direction-agnostic overage there would push the over-current phase further over the breaker.
  • applyFuseGuard has both levers — shrink charge, shrink discharge — so it uses relief in both directions and attributes it by the live aggregate sign.

The evidence: the gate arrived with the PR #219 review, its comment says so, and TestForceFuseDischargeIgnoresExportSidePerPhase has pinned it since. So the answer to "one of these two is wrong" is that the duplicated * 3.0 was the wrong half, not the gate. What was wrong about the gate is that it was the only thing distinguishing the sites while the conversion was silently duplicated. The direction limit now sits at the call that needs it; the conversion is stated once.

Tests

fuse_saver_test.go pinned 3-phase behaviour only. New siblings, all four of which fail against the old code:

  • TestPerPhaseReliefMatchesSinglePhaseSiteOnImport — 1Φ site, idle battery: forced discharge equals the overage once, and the post-dispatch meter stays on the import side of zero.
  • TestPerPhaseReliefMatchesSinglePhaseSiteOnExport — the export direction forceFuseDischarge deliberately skips and applyFuseGuard owns: same law, one phase count. Under × 3 the discharge is zeroed and the site swings to 520 W of import.
  • TestPerPhaseClampReadsConfiguredPhasesOnly — a 1Φ site with L1 at 10 A ignores 24 A reported on phases it does not have.
  • TestPerPhaseClampOffWhenFuseDescriptionIncomplete — amps without a voltage or a phase count clamp nothing.
  • TestPerPhaseClampUsesConfiguredVoltage — a 240 V site converts at 240 V (2560 W, not the 2620 W a hardcoded 230 V gives).

setupPerPhase now states the site it builds (3Φ, 230 V) and delegates to setupPerPhaseSite, which takes both.

Golden corpus

Predicted before looking: zero records move. The corpus was recorded on 3-phase sites, so × SiteFusePhases is arithmetically × 3.0 there, the walk covers the same three phases, and the configured voltage is the same 230 V the fallback invented.

Verified after: TestGoldenCorpusReplay passes unchanged at the 0.01 W tolerance, all 590 records, and TestGoldenCorpusCoverage still reports per_phase=33 — the same records exercise the path, none of them silently switched off. Every record in the corpus that carries phase amps is site_fuse_voltage=230, site_fuse_phases=3; the 33 with a non-zero per-phase overage are all in that set.

make verify green (pre-commit gate).

A worst-phase overage is watts on one phase; dispatch commands
aggregate battery watts. The conversion between them is the site's
phase count, and it was written twice as a bare `* 3.0` — once in
applyFuseGuard, once in forceFuseDischarge — while SiteFusePhases sat
in the same struct and fuseSafetyMarginW read it correctly two
functions away.

On a 1-phase site the aggregate meter and the single phase are the
same wire, so relief is the overage once. Demanding three times it
overshoots the ceiling and can push the meter through zero into a
violation on the other side.

One helper now owns the conversion. perPhaseOverageW reads only the
configured phases, in L1/L2/L3 order, the way sitePhaseCurrentsAt
already does in cmd/ftw, and converts amps to watts through the
configured voltage only — no invented 230 V, the same law
fuseSafetyMarginW states in its own comment. A fuse described by amps
alone gets no per-phase clamp instead of one computed from numbers
nobody configured; config fills both fields (230 V / 3 phases by
default, validation rejects <= 0), so only harnesses that wire
SiteFuseAmps alone see that.

forceFuseDischarge keeps its import-only gate. That is deliberate,
not drift: its single lever is more discharge, which relieves import
and worsens an export-side phase trip, and applyFuseGuard's export
branch already shrinks discharge for that case. The limit now sits at
the call that needs it rather than inside a second copy of the
conversion.

Three-phase 230 V behaviour is arithmetically identical: the golden
corpus replays unmoved, with the same 33 records exercising the
per-phase path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@frahlg
frahlg merged commit 1bee6e4 into master Aug 5, 2026
13 checks passed
@frahlg
frahlg deleted the agent/fix-per-phase-relief-phase-count branch August 5, 2026 04:21
frahlg added a commit that referenced this pull request Aug 5, 2026
)

* test(control): record the early exits meeting a binding protection

Three branches of ComputeDispatch walk away from a cycle before the
safety pipeline: idle mode, the holdoff window, and the reactive
deadband. The golden corpus had no record of any of them meeting a
protection that binds.

Of the 590 records, 33 returned no targets — 25 idle, 1 holdoff, 8
deadband — and every one of the 8 deadband records ran with
site_fuse_amps=0 and peak_import_ceiling_w=0. Nothing was over any
limit to defend, so 590 recorded ticks could not see the deadband exit
skipping every protection. That is the bug #803 fixed, and #803 left
this gap open on purpose: recording new records is a corpus change, not
a fix.

early_exit_protections adds 21 records, 611 in total. Each shape appears
twice — once where the protection binds, once where the same site is
configured identically and nothing is over its limit. The quiet half is
not filler: a corpus of only-firing records cannot tell a fix from an
over-fire, and a fuse-saver that discharges on every tick drains the
pack as surely as one that never fires trips the breaker.

Covered: deadband against a tariff peak ceiling, against a fuse ceiling
left above the breaker by a misconfigured peak limit, against a phase
over the breaker on three-phase and on single-phase services, with a
safety-amp margin, split across two batteries by discharge headroom,
denied by a pack under the 5 % floor, and reached through a stale plan's
reactive fallback; idle and holdoff against the peak ceiling and against
both phase counts. One record is an export-side phase 10 A over the
breaker that must stay quiet, because forceFuseDischarge's only lever is
more discharge and honouring per-phase relief there would push that
phase further over.

Single-phase sites get their own records because the phase count is the
conversion #812 gave one owner: on one phase the aggregate meter and the
phase are the same wire, so relief is the overage once. Under the bare
`* 3.0` those records would command three times the overage.

This family records the law as fixed, not a bug. slew_limiter
deliberately holds a known defect still so its fix reads as a diff in
watts; these records were taken after #803 landed and state what
dispatch is supposed to do, so a record that moves is a protection that
stopped protecting. The family doc comment says so.

Verified against the un-fixed code. Reverting #803's dispatch.go hunk
locally moves 7 of the 611 records, all of them deadband records where a
protection binds, each reading as the commanded discharge disappearing —
deadband_per_phase_3p_binds goes from -1380 W clamped to no targets. The
6 quiet deadband records and all 8 idle and holdoff records hold still,
because idle and holdoff already ran the fuse-saver before #803.
Re-recording to silence the failure does not work either: the coverage
assertions fail on what was just written, naming the deadband exit as
unguarded again.

The eight existing families are byte-identical; only the new file is
added. A full re-recording on this machine rewrites 27 of the 590 old
records with float noise up to 8.5 microwatts — a thousandth of the
0.01 W tolerance, and nothing a reviewer should have to read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(control): point slew_limiter.json at the commit it was recorded on

The family carried ftw_commit fe2c904, a SHA that is not on master and
never was. The other eight families name real commits — seven at
c7fe6c9, where #790 recorded them, and the new one at c85bfe1.

fe2c904 is the pre-squash tip of #809's branch: `git cat-file` finds it
locally with #809's subject line, and `git merge-base --is-ancestor
fe2c904 origin/master` says it is not an ancestor of master. GitHub
squash-merged that branch as c85bfe1, and `git log --
testdata/golden/slew_limiter.json` shows c85bfe1 as the last commit to
rewrite the file: #809's fix moved 94 lines of records, and they were
re-recorded in the branch worktree before the squash. So the behaviour
these 155 records hold is the behaviour at c85bfe1, which is what the
field now says.

No record changes; only the provenance line. It matters because the
field is the one thing telling a reader which dispatch produced these
watts, and a SHA that resolves nowhere on master sends anyone tracing
the file's history to a dead end.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant