fix: allow negative Export Compensation price - #672
Conversation
Some grid operators charge for export instead of paying nätnytta, making the effective compensation negative. The field's `min: 0` was a soft UI-only restriction (no backend enforcement, no downstream code assumes a non-negative sign — the sell-price and DP economics already handle negative sell prices, as negative Nordpool spot prices already exercise the same path) that blocked entering a value users legitimately have. Refs #666
Step 10 — E2E tests failing, likely environmental, needs your inputCI is green everywhere except E2E tests, which fails at the same step both times I've run it: "E2E: Run smoke & navigation tests" ( Why I don't think this is my diff:
Why I can't confirm from here: this dispatched container's network egress only reaches Given I can't read the log and can't run the compose stack locally (this dispatch has no podman-socket access, so no |
|
@claude-bot review |
There was a problem hiding this comment.
Verdict: APPROVE
Root cause match: Confirmed independently, not just taken from the PR description. PricingFormSection.tsx:135-137's min: 0 was the only restriction — backend/api_dataclasses.py:1144 (taxReduction: float | None) has no Pydantic bound, and core/bess/price_manager.py:467 (base_price * self.export_spot_multiplier + self.tax_reduction) is sign-agnostic plain arithmetic. The ENTSO-e help copy already said "Use negative values for deductions," directly contradicting the widget it sat next to. Matches the linked issue #666 (Nätnytta going negative) exactly.
Test: PricingFormSection.test.tsx asserts the min attribute is gone AND that onChange/the sell-price preview correctly reflect a negative value end-to-end. The PR body documents a real RED→GREEN cycle (temporarily restoring min: 0 made the new assertion fail) — this is genuine regression coverage.
Minimal / no scope creep: One-line diff. The identical min: 0 on the Octopus "Tax Reduction" field is explicitly and correctly left alone (issue is Swedish nätnytta only). CHANGELOG entry added in-house style.
Rule compliance: No new classes, no hasattr/getattr, no exception-string matching, no backend change (none needed — chain already tolerates negative values). Clearly a local fix (single attribute on an existing field); no scope-assessment statement required for a change this size.
Nit (non-blocking): PR correctly flags a pre-existing, out-of-scope doc bug — docs/SOFTWARE_DESIGN.md:201 has the wrong sign for tax_reduction vs. actual code/UI. Worth a follow-up issue, doesn't block this PR.
Note: gh api was permission-gated in this run (as flagged as possible in the bot instructions), so no inline comments were posted — everything is folded into this summary.
|
Bot review: APPROVED. Checks green, mergeable CLEAN, no commits since approval — marked ready for review. Nothing left but the merge. |
Summary
min: 0constraint on the Export Compensation field (Nordpool/ENTSO-e view) — it was UI-only, with no backend enforcement and no downstream code assuming a non-negative sign.Root cause
The "Export Compensation" (Nätnytta) field is UI-restricted to non-negative values only, with no backend enforcement and no downstream code that assumes a non-negative sign.
frontend/src/components/settings/PricingFormSection.tsx:135-137—numField('Export Compensation', form.taxReduction, ..., { min: 0, step: 0.001 }). The HTMLminattribute only affects the spinner; there's nocheckValidity()/reportValidity()call before save (confirmed: neitherPricingFormSection.tsxnor its callers wrap the form in a real<form>/onSubmit), so it's a soft, misleading restriction.core/bess/settings.py'sPriceSettingshas no__post_init__,backend/api_dataclasses.py'staxReduction: float | Nonehas no Pydantic bound, andbackend/api.py'spatch_settingspasses the section through with no numeric check.core/bess/price_manager.py:467computessell_price = base_price * export_spot_multiplier + tax_reductionas plain arithmetic; the DP reward math and discharge gate both handle a negativesell_pricecorrectly, with nomax(0, ...)ever applied to price terms. Negative Nordpool spot prices already exercise this same code path today.min: 0on the widget it describes. Pre-existing inconsistency, not introduced by this fix.Fix
Removed
min: 0from the "Export Compensation"numFieldcall in the Nordpool/ENTSO-e view (PricingFormSection.tsx:135-137). No backend change needed — the value already flows correctly end-to-end once the UI stops blocking it. The Octopus "Tax Reduction" field has the identicalmin: 0but is out of scope (issue is specifically about Swedish nätnytta) — left untouched to keep the diff minimal.Documentation check
Grepped
docs/agents/bess-knowledge.mdanddocs/SOFTWARE_DESIGN.mdfortax_reduction/export compensation/nätnytta. Found a pre-existing, unrelated sign inconsistency:docs/SOFTWARE_DESIGN.md:201statessell_price = spot_price * export_rate - tax_reduction, but the actual code (price_manager.py:467) and this same frontend preview both addtax_reduction. This predates my change (the sign mismatch existed for any positivetax_reductiontoo) and isn't something this fix's mechanism touches — flagging here rather than silently fixing it out of scope.Test plan
./scripts/quality-check.sh— fast suite: 2177 passed. One pre-existing failure unrelated to this diff:test_dismiss_persists_across_requests(backend/tests/test_dashboard_api.py) fails only under the full suite (test-order pollution), passes standalone (59/59 in that file). This diff touches only frontend files, so it cannot be the cause..venv/bin/pytest -m slow): 554 passed, 0 failed.--with-composewas not used), so the mock-HA + backend stack could not be brought up. Verification here is limited to the automated test suite below.Evidence the test discriminates
min: 0constraint (temporarily restored{ unit: ..., min: 0, step: 0.001 }on the Export Compensation field).PricingFormSection.test.tsx > PricingFormSection > allows a negative Export Compensation value (nätnytta can be a cost, not just a credit)FAILED —expect(input).not.toHaveAttribute('min', '0')foundmin="0"present. 1 test failed, 1 passed (2 total).Outcome-level coverage
minattribute) and the outcome (onChangeround-trips a negative value; the sell-price preview arithmetic reflects it:0.95 SEK/kWhfortaxReduction = -0.05).Refs #666