fix: stop planning a sub-floor solar export the inverter will absorb (#630)

[johanzander]

Summary

• Withhold the SOLAR_EXPORT-below-max bypass candidate wherever the intent classifier would not actually label the period SOLAR_EXPORT, so the DP stops planning an export the inverter absorbs.
• Charge-side twin of the _residual_cover_p gate that already closes the same failure on the discharge side (Replace DP per-period grid search with closed-form breakpoint enumeration (Approach 1 of #276) #282/LOAD_SUPPORT overshoot in the 0.01-0.1 kWh band produces inconsistent flows and phantom export revenue #497), on the same constant.
• Corrects the reported plan only. Realized cost is unchanged to the bit — the plan came to meet reality, reality did not move.

Root cause

The issue attributes the gap to SOE_STEP_KWH grid-snapping making a sub-step absorb unrepresentable. That is not the mechanism: the forward replay carries continuous SoE, so absorbing 0.0034 kWh is perfectly representable, and period 32's SoE is held bit-exactly, which snapping would not produce.

What actually happens is that the DP deliberately offers a second IDLE-shaped action — the SOLAR_EXPORT-below-max bypass (#313), consider(0.0, forced_next_soe=soe): hold SoE, export this period's own surplus. Nothing commands that hold directly. It is delivered by classify_strategic_intent labelling the period SOLAR_EXPORT, which is the intent that writes charge rate 0 and so stops the inverter absorbing the surplus.

That label requires grid_exported > FLOW_NOISE_FLOOR_KWH (0.01 kWh). At or below it the period falls through to IDLE, whose command is load_first at charge rate 100 — which absorbs. So the plan books export revenue on energy the hardware puts in the battery, and the battery runs fuller than planned until it reaches a bound and spills the difference.

At period 32 of regression_2026_08_17_624 (surplus 0.0034 kWh, export below the floor), the battery then runs 0.0033 kWh fuller than planned for 32 periods until period 64, where it hits max_soe and exports that much solar instead. The two periods' cost deltas sum to the observed +0.001555 SEK.

GRID_FLOW_RESOLUTION_KWH (0.1) is not a second band here, despite being what the discharge-side twin uses: EnergyData's noise fold applies only to battery_to_grid when the battery is discharging (models.py:193). Solar export is never folded, so the classifier's floor is the only binding threshold.

Fix

_solar_export_bypass_is_unexecutable() in action_selector.py — a per-period scalar (with no battery flow, _ac_flows reads only solar, load and the AC cap, so the export does not depend on SoE). The candidate is withheld at all three sites that define the action set (P1): the shared selector, and both backward passes.

The grid backward pass's existing "deliberately NOT masked by _discharge_is_unexecutable" exception does not transfer, and the code says why: that exception rests on the coarse lattice having no exact-cover point near the deficit breakpoint, so an in-band action is the better proxy. The bypass is an exact action, not a lattice approximation, and its executable neighbour (plain IDLE) is in the action set at every state.

Withholding is always safe: plain IDLE is offered unconditionally, and wherever the bypass diverts nothing (no surplus, or a battery already at max_soe) the two candidates coincide exactly.

Honest note on the two backward-pass gates: they are inert on today's corpus. Reverting them alone leaves every test green, including the new one. They are there because P1 requires one action-space definition — leaving those columns valuing an uncommandable action is the #236 class — not because they were measured to change a decision. Stated here rather than left for the reviewer to discover.

Declined one review suggestion: _solar_export_bypass_is_unexecutable re-derives the AC cap even though all three call sites hold it. Accepting it as a parameter is what _period_flows' docstring warns against — a cap a caller can get wrong is the reward-vs-flows divergence P4 exists to remove. One derivation per period is not worth reopening that seam.

Test plan

• ./scripts/quality-check.sh passes locally (2052 passed / 50 skipped; Black, Ruff, TypeScript, ESLint, permission surface all clean)

• pytest -m slow passes (549 passed, 7 skipped)

• Every plan pin unchanged across the 32-fixture corpus: test_current_plan_is_pinned (bit-exact actions, intents, SoE trajectory), test_all_scenarios, the VPP baselines. Zero golden churn — consistent with a corpus scan finding exactly one affected period.

• The real fixture, observed. regression_2026_08_17_624 (the user-derived quarterly day) run through the real optimizer and the real inverter simulator, with PR fix: bisect a tie window too long for one exact PWL solve (#624) #629 layered on locally so it can run at all:

  R=-47.814616  P=-47.814616  gap=+0.000000 SEK      (issue reports +0.001555)
  
  period 32  intent=IDLE
      soe        19.31579 -> 19.31910     (was: held at 19.31579)
      charged     0.00341                 (was: 0.00000)
      exported    0.00000                 (was: 0.00341)
  
  periods still planning a sub-floor held-SoE export: []
  

  R is unchanged from its pre-fix value — the entire gap was the plan overstating, and it closes exactly, so no second mechanism is hiding underneath. The two PRs also apply cleanly together.

• App-level E2E via docker-compose.ci.yml + mock-HA (ci-growatt-vpp, libfaketime-pinned clock): backend healthy, optimizer ran end-to-end, served a full 96-period schedule at 11.36 SEK predicted savings. This confirms no regression in the service path; it does not exercise the bug — that scenario is a winter day with zero solar in all 96 periods. The bug-specific observation is the fixture run above.

Evidence the test discriminates

Two mutations, both run:

• Reverted: the select_action gate only (if not _solar_export_bypass_is_unexecutable(...) → if True:), leaving both backward-pass gates in place.
• Result: test_subfloor_solar_export_is_never_planned FAILED — R=2.404206 P=2.401236 gap=+0.002970, 3× PLAN_EXECUTION_TOLERANCE_SEK. 1 failed, 8 passed in that file.
• Reverted: the two backward-pass gates only, leaving the selector gate.
• Result: all 134 tests passed — which is the basis for the "inert on today's corpus" note above, not something discovered afterwards.
• Restored: tree clean, full suites re-run green.

The test was also written RED before the fix existed and observed failing at that same gap.

One bug the mutation work caught: the first version of the dp_battery_algorithm.py gate used continue, which would have skipped the residual-load-cover candidate later in the same loop iteration — harmless on a surplus period, a real defect on any period with a deficit. Restructured as an if block.

Outcome-level coverage

• R == P via run_scenario_realized in test_subfloor_solar_export_is_never_planned — asserts realized cost reproduces planned cost, plus a secondary mechanism assertion that period 0 plans no sub-floor export so a future regression is diagnosable rather than merely red.
• The scenario is built so the bypass is the DP's honest economic choice, not an accident: the 0.0099 kWh surplus sits just under the floor; periods 1–2 fill the battery to max_soe regardless, so storing period 0's surplus is worth only the export it displaces (sell 0.2) rather than its evening value; selling at 0.5 therefore beats storing; and a larger discharge-and-refill is unprofitable at 0.5 sell − 0.4 cycle − 0.2 displaced. terminal_value_per_kwh is pinned to 0.0 so the leftover-SoE term cannot quietly change which candidate wins.
• Existing corpus pins (goldens, test_all_scenarios, VPP baselines) cover the no-churn half.

Documentation

docs/agents/bess-knowledge.md describes this exact bypass under "Deferring absorption uses the SOLAR_EXPORT-below-max bypass (#313)", and claimed unconditionally that such periods classify as SOLAR_EXPORT. That is now conditional, so the section records the executability bound. docs/SOFTWARE_DESIGN.md's classifier table (grid_exported > 0.01 kWh) and intent→mode table are both still accurate and unchanged.

Interaction with PR #629

The fixture that exhibits this bug lives in #629, which is still open, so this branch's regression test is a hand-built scenario on main instead — it does not depend on #629 merging.

Whichever of the two merges second must delete #629's KNOWN_PLAN_EXECUTION_GAP_SEK entry for regression_2026_08_17_624, since this fix takes that gap to exactly 0.000000 and the entry would then pin a gap that no longer exists. Verified above by running the two together.

Closes #630

[bess-agent]

@claude-bot review

[bess-manager-claude-bot]

Review summary — APPROVE

Root cause vs. linked issue (#630): Matches, and the PR body's root-cause is more precise than the issue's own filed text. The issue attributes the gap to SOE_STEP_KWH grid-snapping; the PR body correctly identifies that as wrong (continuous SoE replay can represent 0.0034 kWh; period 32's SoE is held bit-exactly, which snapping would not produce) and instead traces it to the SOLAR_EXPORT-below-max bypass (#313) being offered even where classify_strategic_intent (strategic_intent.py:68, grid_exported > FLOW_NOISE_FLOOR_KWH) would never actually label the period SOLAR_EXPORT, leaving nothing to command the hold and letting load_first/charge_rate=100 absorb the surplus instead. I verified this chain directly in strategic_intent.py and action_selector.py — it's correct, and it explains the specific +0.001555 SEK number the issue reports.

Fix shape: _solar_export_bypass_is_unexecutable() (action_selector.py:131) is the charge-side twin of the existing _residual_cover_p/_discharge_is_unexecutable pattern (#497/#282), gated on the same FLOW_NOISE_FLOOR_KWH constant used by the classifier — exactly the "one threshold" discipline the codebase already established for the discharge side. Applied at all three P1 action-space-definition sites (action_selector.py:703 selector, dp_battery_algorithm.py:1398 grid backward pass, pwl_window_dp.py:264 PWL backward pass), which is what P1 (docs/agents/optimizer-architecture.md) requires — a candidate withheld in only one of the three would be the #236 mirror-drift bug class. I confirmed _effective_ac_cap_kwh(battery_settings, dt) is exactly what each of the three call sites already derives independently, so there's no divergence risk in re-deriving it inside the new predicate (matches the precedent set by _period_flows's own docstring, which I also checked).

Test — would it catch a regression? Yes, verified directly, not just read: I reverted only the select_action gate (action_selector.py:703, if not _solar_export_bypass_is_unexecutable(...) → if True) in a scratch worktree and re-ran test_subfloor_solar_export_is_never_planned. It failed exactly as claimed: R=2.404206 P=2.401236 gap=+0.002970 (3× PLAN_EXECUTION_TOLERANCE_SEK). Reverted the edit immediately after. The test asserts realized cost against planned cost (run_scenario_realized) plus a mechanism-level assertion (period_data[0].energy.grid_exported == 0), i.e. outcome-level per docs/agents/testing.md, not a command/register assertion.

Full suite, run locally (not just re-quoted from the PR body):

• pytest -m "not slow": 2052 passed, 50 skipped — matches PR claim exactly.
• pytest -m slow: 549 passed, 7 skipped — matches PR claim exactly, confirming zero golden churn across the 32-fixture corpus (test_current_plan_is_pinned, test_all_scenarios, VPP baselines all still bit-exact).
• black --check / ruff check on all four touched Python files: clean.

Minimality / scope creep: None found. Six files touched: the new predicate + its three call sites, one new regression test, one CHANGELOG line, one bess-knowledge.md addition documenting the now-conditional bypass. No new classes, no Optional[x], no hasattr/getattr-with-default, no exception-string matching, no hardcoded entity IDs — checked the diff directly for all of these. Single commit, no plan/scratch files.

Rule violations: None found.

Scope-assessment statement (rules.md Debugging Protocol step 9): The PR body doesn't use the literal words "local fix" / "structural fix", but it does the substantive equivalent — it explains why the change touches three sites ("withheld at all three sites that define the action set (P1)") rather than leaving that for the reviewer to infer, and the three-site shape is the architecturally-mandated one (P1 explicitly requires one candidate-space definition shared by the selector and both backward passes), not an ad-hoc spread across unrelated owners. I don't think this needed an explicit escalation — there's one clear new owner (the predicate function, mirroring the existing _discharge_is_unexecutable sibling) and P1 dictates where it's consumed. Flagging as a nit only: an explicit one-line "this is a local fix, mirroring #497's pattern" would have made this checklist item trivially checkable rather than requiring the reviewer to reconstruct it from the "Fix" section.

Notably good PR hygiene, worth calling out: The "Honest note" that the two backward-pass gates are inert on today's corpus (verified by the author's own mutation testing — reverting them alone leaves all 134 tests green) is exactly the kind of self-disclosure that makes a review actually verifiable rather than a trust exercise. Same for stating up front that R is unchanged (the entire gap was the plan overstating) and flagging the #629 interaction (KNOWN_PLAN_EXECUTION_GAP_SEK cleanup needed by whichever PR merges second).

No blockers. Nits only, noted above.

[bess-agent]

@claude-bot review

[bess-manager-claude-bot]

Review summary — PR #633 (fix/issue-630-subfloor-solar-export)

Root-cause match. The PR does not merely fix issue #630 as filed — it corrects the issue's own root-cause attribution. #630 blamed SOE_STEP_KWH grid-snapping; the PR shows (and I independently confirmed against dp_battery_algorithm.py/models.py) that the forward replay carries continuous SoE and period 32's SoE is held bit-exactly, which snapping wouldn't produce. The real mechanism is classify_strategic_intent's FLOW_NOISE_FLOOR_KWH (0.01 kWh, strategic_intent.py:29) gate on the SOLAR_EXPORT label: below it the period falls through to IDLE, whose derived command (load_first, charge_rate_pct=100) absorbs the surplus the plan had booked as export. This is disclosed explicitly in the PR body rather than left for the reviewer to find.

I traced the new predicate's logic by hand: _solar_export_bypass_is_unexecutable (action_selector.py:121-174) calls _ac_flows(solar, home, 0.0, 0.0, ac_cap) and checks only grid_exported <= FLOW_NOISE_FLOOR_KWH. Given battery_discharged=0 for this candidate, EnergyData._calculate_detailed_flows (models.py:152-195) reduces battery_to_grid to 0, so solar_to_grid == grid_exported exactly — meaning the single-condition check is logically equivalent to the classifier's two-condition SOLAR_EXPORT gate (grid_exported > FLOOR and solar_to_grid > FLOOR, strategic_intent.py:67-70) for this specific candidate shape. The gate is sound.

Test would catch a regression. I ran the mutation the PR describes myself, in a throwaway worktree off pr-633: reverting only the select_action gate (action_selector.py:703, if not _solar_export_bypass_is_unexecutable(...) → if True:) reproduces the claimed failure exactly:

AssertionError: plan books a cost its own execution does not reproduce:
R=2.404206 P=2.401236 gap=+0.002970

This matches the PR body's reported numbers to the last digit. With the fix restored, core/bess/tests/integration/test_plan_faithfulness.py and core/bess/tests/unit/test_pwl_window_bisection.py both pass in full (13/13). test_subfloor_solar_export_is_never_planned asserts an outcome-level property (R == P via run_scenario_realized), consistent with docs/agents/testing.md's "assert the outcome, not the command."

Minimal, no scope creep. The candidate-withholding gate is applied at exactly the three sites P1 (docs/agents/optimizer-architecture.md) requires for one action-space definition: the shared selector and both backward passes (dp_battery_algorithm.py, pwl_window_dp.py). The PR is upfront that the two backward-pass gates are currently inert on the corpus and states why they're still required (P1, not measured behavior change) — exactly the kind of disclosure the checklist's "fitness of approach" section asks for. The scope assessment (charge-side twin of _residual_cover_p, same constant, same _discharge_is_unexecutable-style local addition) is stated directly in the PR body, satisfying rules.md's Debugging Protocol step 9. A review suggestion to parameterize the AC-cap derivation was declined with sound P4 reasoning (avoids a caller-supplied-cap divergence seam) rather than silently accepted or silently dropped.

Rule compliance. No Optional[x], no hasattr/getattr fallbacks, no new classes, no exception-message string matching, no hardcoded entity IDs (not applicable to this module). Golden-file and fixture-pin changes are all explained inline with the mechanism, and the now-obsolete KNOWN_PLAN_EXECUTION_GAP_SEK entry for regression_2026_08_17_624 is correctly removed with the interaction against the still-open PR #629 called out explicitly.

No blockers found. Approving.