Dashboard diagram fixes - #23
Merged
Merged
Conversation
Same bug class as the bus-edge fix in #22, on a sibling edge that wasn't caught at the time: the battery-charge stripe used the whole household gridImportW as its grid-sourced component, but grid power can only reach the battery by flowing backward over the Inverter<-> Junction bus edge (netBus<0) - crediting the household's raw import figure double-counts grid power that's actually being used for unrelated Load/Backup consumption served directly at Junction. Verified against real history on the pi (2026-09-13 08:08:23): PV 2584W, battery charging 2209W, grid importing 73W, load 435W, backup 9W. PV alone covers the full charge (2584 > 2209), so the charge arrow should be 100% green - it showed a ~3%-wide orange sliver instead, sized from the unrelated 73W import. Also fixes a second, smaller Kirchhoff gap while auditing every edge for the same class of bug: the bus edge's netBus only balanced against load and grid, but Junction has four edges, not three, whenever Backup is grid-bypass-fed - omitting backupBypassW under-reported the bus's real flow by Backup's own wattage. Extracted the Kirchhoff balance (netBus) and the charge-mix cap (batteryChargeGridWatts) into diagram-calc.js as pure functions - they're arithmetic, not DOM, and diagram-render.js previously had no way to regression-test them at all. Added four node --test cases covering: the real Pi sample that exposed the bug, the four-edges correction, the inverter-fed (islanding) case staying unaffected, and a genuine grid-charging scenario (netBus negative) still reporting correctly. Every other edge was re-checked against the same "which watts could this physically carry" question and found sound: Grid-export and Backup-inverter-fed only ever see gridImportW as 0 by construction (mutually exclusive with the states that gate those branches), and Load/Backup-bypass are correctly modeled as drawing from the shared Junction mix in full, since they're the two sinks actually tied to that combined line. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
batteryChargeGridWatts(data) recomputed busFlow(data) from scratch (re-deriving gridState/loadState/backupState/backupSource) even though diagram-render.js already had the identical netBus sitting in a local one line above, from its own calc.busFlow(data) call. Not a correctness bug (both calls were pure functions of the same immutable data), but duplicate plumbing for no reason. batteryChargeGridWatts now takes netBus directly - Math.max(0, -netBus) - and the one call site passes the already-computed local. Verified this is behavior-preserving, not just test-passing: the caller passes exactly calc.busFlow(data).netBus, the same value the old code derived internally from the same data, so the output is identical by construction, not by coincidence. Re-ran the exact 2026-09-13 08:08:23 Pi sample from the bug report before and after (netBus 371, chargeGridW 0 both times) plus the full suite (141 Python + 38 JS tests, including the new direct batteryChargeGridWatts(number) unit tests). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1. Backup's arrow no longer falls back to solid red on a phase-overload alert - that flat color was only ever reached when `active` is false (no stripes to draw), an implausible combo since a real overload's wattage is always far above the noise threshold, but it was still the wrong signal to encode there regardless: the arrow communicates the physical source mix, not alarm status. Split backupNodeColor (still red - the node is exactly the right place for a status flag) from a new backupArrowFallbackColor (never red). 2. gridState() now returns explicit importing/exporting booleans as the semantic source of truth, already correctly accounting for the Fault/ Not-connected overrides - callers (gridImportW, busFlow's meterSigned, the Grid edge's stripe/direction conditions) now read grid.importing/grid.exporting instead of comparing grid.color against a hardcoded string. color is now derived from the same booleans rather than computed separately, so the two can't drift, and future palette changes can't silently break status logic that happened to be comparing against a color string. Full suite: 38 JS + 141 Python tests pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…netBus The Kirchhoff arithmetic (busFlow, batteryChargeGridWatts) was already tested, but everything downstream of it - which color/thickness/stripe mix each of the 6 arrows actually gets - lived entirely inline inside redrawLines(), intermixed with the SVG drawing calls, so none of it could be covered by node --test (diagram-render.js has no DOM harness). That's exactly the class of bug this session kept finding (spurious stripes, wrong fallback colors) - all in code that only ever got manually verified against the running app. Moved the full per-edge decision logic into a new pure function, calc.edgeStates(data), alongside isBackupActive/backupNodeColor/ backupArrowFallbackColor (also pure, previously stranded in diagram-render.js for no real reason). redrawLines() is now a thin loop: computeEdges() for DOM geometry, edgeStates() for what to draw, one drawEdgeState() call per arrow. Verified this refactor is behavior-preserving, not just test-passing: reimplemented the exact pre-refactor inline logic as a standalone function and fuzzed both against 20,000 random synthetic datasets (random grid/battery/backup/work modes and wattages) - zero mismatches. Also did a live smoke test (--dry-run) to confirm the page and both JS files still serve cleanly. Added 10 new tests covering isBackupActive/backupNodeColor/ backupArrowFallbackColor directly, plus edgeStates scenarios: the real Pi sample, genuine grid-charging, a real phase overload (arrow stays orange, only the node goes red), Backup inverter-fed/islanding, a grid Fault, and no-battery-installed. Full suite: 48 JS + 141 Python tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
piomar123
added a commit
that referenced
this pull request
Sep 14, 2026
…erlap (#25) The bus edge (Inverter->Junction) and the inverter-fed Backup edge both leave Inverter's bottom border at the same point, and the islanding edge then runs straight down through Junction's own position before bending into Backup - so with no offset between them, the two draw on exactly the same pixels. During islanding this made Backup's flow visually indistinguishable from (and drawn right on top of) a grid-bypassed feed, even overlapping the "grid disconnected" crossed-X mark on the bus edge, making it look like power was flowing straight through it. Confirmed against a real Pi sample (2026-09-13 19:57:52, grid_mode Fault/work_mode Off-Grid: load 725W, backup 726W, battery discharging 700W, meter/pgrid ~0W) and visually verified with a standalone harness loading the real static assets in headless Chrome, before/after. The v5 mockup (docs/superpowers/mockups/live-power-flow-dashboard- mockup-v5.html) already solved this with BACKUP_ISLANDING_SHIFT - offset the bus's exit point right and the islanding edge's own exit point left by the same amount, and shift Junction/Load right by it too, so the two lines run alongside each other instead of on top of each other - but its own comment flagged it as "pending a production port" that never happened, across the original port (#15) and four follow-up fixup PRs (#19, #21, #22, #23). Those fixups all hardened diagram-calc.js's *arithmetic* (Kirchhoff's law, source-mix stripes), which is unit-tested - diagram-render.js's SVG *geometry* has no DOM harness, so nothing ever regression-tested it, and manual verification only ever happened to see the far-more-common grid-connected state. Ported the shift amount as a pure, testable function (backupIslandingShiftPx in diagram-calc.js, covered by node --test) alongside the constant; diagram-render.js applies the actual DOM transform/geometry offset, which still has no automated coverage - the same gap that let this slip through five previous PRs. Also audited the rest of the mockup's own "pending production port" comments: the other two (sqrt-scale arrow thickness, dischargeLimit===0 battery check) were already ported in later commits despite the mockup's stale comments; the adaptive backup-threshold formula remains deliberately deferred, already tracked by its own comment in diagram-calc.js. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix power-flow diagram source-attribution bugs; make arrow logic testable
Three real bugs found via manual review and production data, plus a refactor to close the resulting test gap:
busFlow), which also corrects a second gap: Junction has 4 edges, not 3, whenever Backup is grid-bypass-fed.grid.color === 'orange') replaced with explicitgrid.importing/grid.exportingbooleans, so future palette changes can't silently break state logic.edgeStates()— all six arrows' color/thickness/direction/stripe decisions, previously inline in the untestable DOM-drawing function, are now one pure function indiagram-calc.js. Verified behavior-preserving via a 20,000-iteration fuzz-equivalence check against the old inline logic.