fix(execute): resolve multi-inverter status from all inverters, not just the last one processed

[chalfontchubby]

Summary

• execute_plan()'s headline status was a single variable overwritten once per inverter in the per-inverter loop - the dashboard silently showed whichever inverter happened to be processed last, hiding real disagreement between inverters.
• Most importantly this hid genuine cross-charging (one inverter charging while another discharges at the same time) - a real, known phenomenon on multi-inverter systems, confirmed by @gcoan in Predbat sets full rate charge when plan says Hold Charge #4440.
• Each inverter's own final core state (Charging/Freeze charging/Hold charging/Exporting/Freeze exporting/Hold exporting) is now tracked in status_per_inverter, keyed by inverter id. After the loop, resolve_multi_inverter_status() resolves one headline:
  • Inverters disagree across the charge/export divide → "Cross-charging", surfaced explicitly rather than picking one side arbitrarily.
  • Inverters only disagree on sub-state within the same side (e.g. one still Charging, another already Hold charging) → show the most active one, since that's what the fleet is actually still doing overall.
  • No inverter reached a core charge/export state (pure Demand, Read-Only, Calibration, or a Hold-for-car/iBoost annotation) → unchanged.
• Extracted the resolution as a small pure function (resolve_multi_inverter_status) for direct, fast unit testing rather than needing to drive execute_plan()'s full branching to construct genuine multi-inverter disagreement scenarios.
• Three existing tests (charge_imbalance2, charge_freeze_imb1, charge_freeze_imb4) asserted the old "last inverter wins" artifact as correct (e.g. expecting "Hold charging" when one inverter was actually still charging toward target, per the test's own comment) - updated to assert the corrected, more informative aggregate.
• Follow-up commit: the status tooltip (status_extra, the detail attribute on predbat.status) already concatenated each inverter's SoC→target numerically for multi-inverter setups ("target 80%-40% / 60%-40%") but didn't say which state each entry belonged to - now prefixes each entry with that inverter's own state ("target Charging 80%-40% / Hold charging 60%-40%"), reusing the same values already computed for the headline fix. Single-inverter setups unaffected.

Fixes #4446

Test plan

• New test_execute_multi_inverter_status.py covering: no-op fallthrough, single-inverter passthrough, full agreement, same-side precedence (both directions), cross-charging detection (including from the mildest sub-states on each side)
• assert_status_extra added to the shared execute test harness, real values locked in for charge_imbalance2 and charge_freeze_imb1 (both already exercise genuine multi-inverter disagreement)
• Full ./run_all --quick suite passes
• ./run_pre_commit passes

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a multi-inverter reporting bug in execute_plan() where the headline status could be overwritten once per inverter, causing the dashboard to reflect only the last-processed inverter and potentially hiding real fleet disagreement (including cross-charging). It introduces per-inverter status tracking and a resolver function to compute a single, informative headline status, plus adds targeted unit tests around the new resolution logic.

Changes:

• Track each inverter’s final core charge/export state during execute_plan() and resolve a single fleet headline via resolve_multi_inverter_status() (including explicit “Cross-charging” detection).
• Enhance status_extra in multi-inverter setups to prefix each inverter’s entry with that inverter’s own core state.
• Add a focused unit test module for multi-inverter headline resolution, and update existing execute tests that previously encoded the “last inverter wins” artifact.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
apps/predbat/execute.py	Adds per-inverter status tracking and fleet headline resolution; updates status_extra formatting to include per-inverter state.
apps/predbat/tests/test_execute.py	Extends the execute test harness to assert status_extra; updates expectations for multi-inverter disagreement cases.
apps/predbat/tests/test_execute_multi_inverter_status.py	New unit tests covering the fleet headline status resolution rules (including cross-charging).
apps/predbat/unit_test.py	Registers the new multi-inverter status test in the unit test runner.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[gcoan]

Inverters disagree across the charge/export divide → "Cross-charging", surfaced explicitly rather than picking one side arbitrarily.

@chalfontchubby

if there is a new cross charging status then this should be added to the documentation of predbat statii https://springfall2008.github.io/batpred/what-does-predbat-do/#predbat-modes

(and maybe I'll be writing an automation to detect it, be interesting to see if it ever happens)

[chalfontchubby]

Good call - added it: 3dbf4747

[gcoan]

Good call - added it: 3dbf4747

perfect thanks

[chalfontchubby]

Dug into this. Copilot's suggested patch (guard at the resolve_multi_inverter_status() call site checking status == "Hold for car"/"Hold for iBoost") turns out to be a no-op in practice: status is never reset between inverters in the loop, so once any inverter sets a core charge/export state, status can never revert to "Demand" again - which means the direct-overwrite branches (if status == "Demand": status = "Hold for car") can never re-fire after that point. So status can't actually be the literal "Hold for car"/"Hold for iBoost" string at the point resolve() runs while status_per_inverter is non-empty - the two conditions are mutually exclusive given the existing control flow.

There was a real (if narrower) version of this bug, just one guard earlier and specific to iBoost: the iBoost-hold block's entry condition included status not in ["Exporting", "Charging"], gating not just the pause/reserve actions but the status_hold_iboost text update too - so once status reached "Charging"/"Exporting", the whole block (including the text-only part) was skipped, permanently losing the annotation. The car-holding block just above it doesn't have this problem, because its if not isExporting: only gates the actions, not the status text.

Pushed 993853 - split the iBoost guard so it still gates the discharge-pause/reserve actions, but the status text always updates, matching the car-holding pattern. Existing execute, iboost_smart, and multi_inverter_status tests all still pass.

Caveat worth flagging: I couldn't construct a test that fails on the old code and passes on the new one. charge_window_best/export_window_best are fleet-wide (one plan applied uniformly to every inverter, differing only by each inverter's own SoC), so whenever any inverter is in an active window, every inverter in that window gets its own state assignment rather than staying inherited/idle - SoC imbalance always leaves at least one inverter in a non-excluded Hold/Freeze substate, which self-heals the annotation regardless of processing order. So this may be a real latent bug that isn't reachable with today's harness or fleet-uniform-plan architecture, rather than one with a live repro. Keeping it anyway since it removes a fragile dependency on shared mutable state for a display-only concern, at zero behavioural cost on all passing scenarios - but flagging in case either of you sees a path to actually trigger it that I'm missing.

[springfall2008]

Review

Dug into why a test couldn't be written that fails without the final commit (9938536, the iBoost-hold change) — the answer is more specific than "hard to reproduce": pre-PR, the "Hold for iBoost" status text update was nested inside the same combined if that gated the actual pause/reserve action, so the annotation and the real action were structurally coupled to the identical boolean. There was no reachable state where an inverter is genuinely held for iBoost but the label gets dropped — the bug as described couldn't happen, independent of test coverage.

The fix splits that into an outer guard plus an inner status not in ["Exporting", "Charging"] check that still only gates the pause action, but the status-text update is now a sibling of that inner guard rather than nested inside it — so it now fires whenever the outer condition holds, regardless of whether the inner guard let a pause actually happen. That reintroduces the same class of bug in the opposite direction, and unlike the original, this one is testable:

execute.py ~536-554 — In a multi-inverter fleet, inverter B can show "Hold for iBoost" even when nothing was paused for it that cycle, because the shared status variable can still read e.g. "Charging" (left by inverter A earlier in the loop) when B's own inner guard is false. boostHolding (which drives real inverter commands) is unaffected — this is display-only, but genuine.

A few more from tracing resolve_multi_inverter_status() and its callers:

1. execute.py:689 — The calibration branch (lines 114-122) sets status directly and breaks the loop without populating or clearing status_per_inverter. If an earlier inverter already wrote e.g. 'Charging' into status_per_inverter, resolve_multi_inverter_status() can silently prefer that stale core state over "Calibration" (its charge_states_present check short-circuits before consulting current_status), hiding that an inverter just had its controls force-reset. Not covered by the existing calibration test, which applies in_calibration uniformly to every inverter so status_per_inverter is always empty when it fires.
2. docs/what-does-predbat-do.md:389 — Docs describe "Cross-charging" as one inverter "genuinely charging" while another is "genuinely exporting", but the code (execute.py:33-36, 58-67) also fires it for two idle/paused states (Hold charging + Hold exporting) — locked in by this PR's own cross-charging detected even from the mildest sub-states test. Worth tightening the doc wording since no current is actually flowing in that case.
3. output.py:3153 and inverter.py:1152 — Both do substring/exact-match checks against predbat_status ('charging' in slot_status, == "Charging") that don't account for the new "Cross-charging" value, so those minutes get silently misclassified in yesterday's-plan reconstruction and dropped from charge-curve learning. Possibly fine as a tradeoff for curve learning, but worth a deliberate decision rather than an accidental string-match miss.
4. tests/test_execute.py:1799 — charge_freeze_imb4 only updates assert_status and, unlike its sibling cases (charge_imbalance2, charge_freeze_imb1), doesn't add assert_status_extra, so it wouldn't catch a regression in the per-inverter status embedded in status_extra along this specific path.

Net: the core architectural direction (resolve status across all inverters instead of last-write-wins) is right and worth keeping. The hold-annotation variables (status_hold_iboost, status_hold_car, status_freeze_export) are still on the old shared-mutable-across-the-loop pattern though, which is what let the iBoost issue back in through the very fix meant to close a similar gap. I'll push a follow-up with fixes for the confirmed issues.

[springfall2008]

Followed up on the coverage question, since "can you write a test that fails without it" is exactly what was in doubt on the original commit. Both fixes I pushed earlier were only covered at the extracted-function level, which proves the helper's logic but not that the surrounding code actually reaches it. Both now have integration coverage, and I verified each genuinely fails when its own fix is reverted:

calibration_after_charging_inverter (test_execute.py) — drives the real execute_plan() with a two-inverter fleet where inverter 0 reaches Charging before inverter 1 enters calibration and breaks the loop. Reverting the guard gives ERROR: Inverter 0 status should be Calibration got Charging.

This one needed new per-inverter escape hatches (in_calibration_array, plus array/override forms of the immediate-target and isCharging asserts) because the break leaves the fleet genuinely half-processed: inverter 0 keeps the charge window and immediate targets it was already given, inverter 1 never reaches those calls. The test asserts that real asymmetric state rather than smoothing it over.

Worth noting one pre-existing quirk it documents: isCharging stays True while the headline reads Calibration, because the calibration branch breaks without resetting it. That predates this PR (it behaves the same way before the aggregation change), so I left it alone rather than widening scope — but plan.py:1954 does read self.isCharging to bias keeping the current charge target, so it may be worth a separate look.

Cross-charging window reconstruction (test_calculate_yesterday.py) — feeds a full Cross-charging status history through the real calculate_yesterday() and asserts both charge and export windows come back. Reverting gives ERROR: a Cross-charging history should rebuild export windows too, got none. The reconstructed windows only exist between the fake-window block and the restore at the end of the function, so they're captured from inside a publish_html_plan mock.

Test-only commit, no production code touched: 18613749. Full quick suite and pre-commit both clean.