Skip to content

fix: enforce forecast validity at optimizer boundaries - #1151

Open
MMicieli wants to merge 6 commits into
davidusb-geek:masterfrom
MMicieli:fix/1135-forecast-validity-contract
Open

MMicieli wants to merge 6 commits into
davidusb-geek:masterfrom
MMicieli:fix/1135-forecast-validity-contract

Conversation

@MMicieli

@MMicieli MMicieli commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1135

Problem

EMHASS had no single enforced validity contract for forecast values before they reach the optimizer:

  • runtime forecast validation used isinstance(x, int | float), which accepts NaN, ±Inf and booleans (bool subclasses int), and only warned before continuing;
  • a finite negative household load (e.g. an unconstrained regression excursion) from any load method (runtime list/mapping, CSV, native mlforecaster, …) reached the optimizer unchanged;
  • there was no common final check on the optimizer-facing P_Load after the optional mix/feedback step;
  • an invalid runtime value could silently fall back to the configured forecast method (or, for outdoor temperature, to the weather-forecast temperature).

Maintainer-approved contract

As agreed on #1135:

  • Numerical validity. Every externally supplied forecast value must be a finite real number. NaN, ±Inf, booleans and non-numeric values are rejected, and the optimization cycle fails before the solver. Signed prices and temperatures stay valid.
  • Physical-domain validity. External load_power_forecast is the canonical non-negative household consumption (zero load is valid). The optimizer-facing household load must be finite and >= 0 W. A final max(P_Load, 0) correction for finite negative loads is applied once at the Forecast.get_load_forecast() boundary, with one summarized warning. A non-finite P_Load fails the cycle.
  • The agreed documentation updates and regression tests are included.

Current-master delta: external PV P10 (#1128)

pv_power_forecast_p10 landed after the original #1135 audit. It is an external forecast input, so it is part of the same finite-real-number contract. _validate_and_align_external_pv_pair() already rejected some non-numeric and non-finite values, but np.asarray(..., dtype=float) ran first and silently coerced other invalid inputs: True/False became 1.0/0.0, and numeric-looking strings such as "2.5" were accepted as floats.

This PR closes that gap by reusing the canonical describe_invalid_forecast_value() helper — the same one used for every other external forecast key — and calling it on the original P50 and P10 source values, for both lists and mappings, before any NumPy/pandas coercion runs. It rejects bool, np.bool_, numeric-looking strings (e.g. "2.5"), ordinary strings, None/null, NaN, +Inf and -Inf for both P50 and P10. The diagnostic names the forecast key, the list position or source timestamp, and the offending value.

A second, independent gap was found and fixed on this same head: individually finite source values can still produce a non-finite result after alignment/aggregation (e.g. an overflowed mean when averaging large finite floats into one time-step bucket). Timestamped P50/P10 pairs are therefore validated a second time, after _align_runtime_forecast_mapping(), with the same helper, so a non-finite aggregate is rejected before it reaches the optimizer.

Nothing else about P10 changes: the list/mapping representation contract, timeline matching, pair-length checks, existing error messages, the P10 bias formula, and the final PV-domain (non-negative) treatment all stay as they were.

Implementation

Where Change
utils.describe_invalid_forecast_value() One small helper. It returns one diagnostic for the first value that is not a finite real number (key, reason, list position or source timestamp, repr of the value), or None. It does not check sign.
utils.treat_runtimeparams() Legacy stringified lists: a forecast supplied as a whole-list string (e.g. "[1,2,3]") is normalized with ast.literal_eval before length and value validation, so existing callers keep working and the parsed members are still subject to the finite-real contract (a parsed [1, "2.5", 3] is rejected). Mappings: source values are validated before _align_runtime_forecast_mapping(), so pandas cannot coerce bool → 1.0 or crash on strings, and the error names the source timestamp. The aligned list is validated again. Lists: values are validated in the existing len >= horizon branch. Invalid: one logger.error, passed_data[key] = None, and the forecast method is forced to "list". This is the same invalid-sentinel pattern #1128 uses for P10, so the cycle fails instead of falling back to the configured method. A rejected outdoor_temperature_forecast additionally sets an internal _outdoor_temperature_forecast_rejected provenance marker (initialized False on every call), so rejection can be distinguished from omission. The old per-value non digits warning loop is removed. The alignment helper itself is untouched.
utils._validate_and_align_external_pv_pair() The canonical describe_invalid_forecast_value() finite-real validator is applied to the original P50 and P10 source values (list positions or source timestamps) before np.asarray(..., dtype=float) coercion, so bool/np.bool_, numeric-looking strings, ordinary strings, None, NaN and ±Inf are all rejected on the same terms as every other external forecast key, with source provenance preserved in the diagnostic. For timestamped mappings, the aligned P50/P10 output is validated a second time after _align_runtime_forecast_mapping(), to catch a non-finite value produced during bucket aggregation.
Forecast.get_load_forecast() Final optimizer-facing load contract, after optional mixing: non-finite/non-numeric → one error and False. Finite negative → one warning (count, minimum, first timestamp, "clipped to 0 W before optimization") and Series.clip(lower=0), which keeps index, name and shape. Valid → unchanged. A minimal pre-mix numeric check runs only when mixing is enabled, because ±Inf in the first step would otherwise crash int(round(...)) inside get_mix_forecast() before the final check. get_mix_forecast() itself is unchanged.
Forecast.get_load_cost_forecast() / get_prod_price_forecast() Treat data_list is None (the rejected sentinel) as a clean False instead of len(None) → TypeError.
command_line._get_dayahead_pv_forecast() / _get_naive_mpc_pv_forecast() Treat a None weather frame as failure. Before, the P10/P50 invalid sentinel reached get_power_from_weather(None) and raised.
command_line.prepare_forecast_and_weather_data() Absence and rejection are separate contracts, keyed on the explicit rejection marker rather than on the configured method: an omitted runtime outdoor_temperature_forecast keeps the existing weather temp_air fallback; a supplied valid one is used; a supplied but rejected one fails the cycle instead of silently using the weather-forecast temperature.
web_server._handle_action_dispatch() An optimization action that returns False now answers 400 with the action log, like a failed set_input_data_dict. Before, it reached get_injection_dict(False) and crashed with a 500. The solver was never reached in either case.

The raw ML model is not clamped: MLForecaster.predict() and forecast-model-predict are unchanged. load_negative and set_zero_min are not repurposed, and RetrieveHass.prepare_data() is untouched. get_power_from_weather() and its p_pv_forecast[p_pv_forecast < 0] = 0 are untouched.

Backwards compatibility

  • Valid finite inputs behave exactly as before, including zero load, negative prices, negative outdoor temperatures, long lists (sliced to the horizon), timestamp mappings (identical aligned output, asserted against _align_runtime_forecast_mapping) and valid P50/P10 pairs (identical bias result).
  • Short plain lists keep their existing behaviour: an error is logged, the value is not used, and nothing is padded.
  • Existing whole-list string payloads (e.g. "[1,2,3]") keep working: they are normalized to a list before validation. Strings inside the resulting list remain invalid.
  • Omitting outdoor_temperature_forecast keeps the existing weather-temperature fallback, including when outdoor_temperature_forecast_method is configured as list. Only a supplied-and-rejected value fails closed.
  • Intentional changes, all of which are the contract:
    • a runtime forecast containing NaN/±Inf/null/bool/strings now fails the action. Before, it was accepted, or it crashed later;
    • a finite negative optimizer-facing load (from any method, including naive history with negative samples when set_zero_min=false) is now clipped to 0 W with one warning;
    • a non-finite final load (e.g. NaN left in history or CSV) now fails cleanly instead of reaching the solver;
    • a rejected forecast on /action/*-optim answers 400 instead of a 500 crash.
  • No new dependency, config option, threshold, forecast backend, optimizer-formulation, tariff, battery or SOC change.

Regression matrix

New: tests/test_forecast_validity_contract.py (28 tests with subtests). Extended: tests/test_external_pv_p10.py (+2 tests), tests/test_command_line_utils.py (test_prepare_forecast_and_weather_data: omitted-vs-rejected outdoor temperature), tests/test_utils.py (test_treat_runtimeparams_failed: stringified lists now asserted as passed through).

Area Case Expected Test
Plain list positive / zero / finite negative load; negative prices; negative temperature passed unchanged, method list test_valid_plain_lists_are_passed_unchanged
Plain list NaN, +Inf, −Inf, True, np.bool_, string, None for all 5 keys rejected; passed_data=None; method forced list; exactly one error with key, position N, repr(value) test_invalid_plain_list_values_fail_closed
Plain list short / long short → existing error; long → accepted test_long_list_is_accepted_and_short_list_keeps_existing_invalid_behavior
Legacy stringified list valid whole-list string repr([0.0, 1.0, …]) normalized before validation; parsed numeric list passed through; method list test_legacy_stringified_list_is_normalized_before_validation
Legacy stringified list parsed list containing string member "2.5" rejected; passed_data=None; one error non-numeric value '2.5' at position 5 test_stringified_list_with_string_member_is_rejected
Legacy stringified list existing stringified PV/load/cost/price lists parsed and passed through to passed_data test_treat_runtimeparams_failed (extended)
Outdoor temperature method configured list, runtime key omitted weather-temperature fallback still succeeds test_prepare_forecast_and_weather_data (Test 3)
Outdoor temperature runtime key supplied but rejected fails closed (False, "was supplied but rejected"); no weather fallback test_prepare_forecast_and_weather_data (Test 3b)
Mapping invalid source values (same 7) for all 5 keys rejected before aggregation; error names source timestamp test_invalid_mapping_source_values_fail_closed_with_timestamp
Mapping local-time mean aggregation, UTC-keyed instants, hold-last, leading backfill, negative price/temp/load exact expected values; identical to _align_runtime_forecast_mapping test_mapping_alignment_semantics
Final load valid non-negative unchanged, name/index preserved, no warning test_valid_non_negative_load_is_unchanged
Final load finite negatives clipped, one warning with count/min/first timestamp test_finite_negative_load_is_clipped_with_one_summary_warning
Final load NaN/Inf/string/None False, one error with timestamp test_non_finite_or_non_numeric_load_fails
Final load typical / naive / csv / list all routed through the boundary clipped test_every_load_method_is_routed_through_the_boundary
CSV valid / negative / nan,inf,-inf,abc,True unchanged / clipped / False TestCsvLoad
Native ML (real MLForecaster.predict, stub estimator) positive / negative / NaN,±Inf unchanged / clipped / False TestNativeMlLoad
Raw ML forecast-model-predict with negative prediction stays raw (−12 W) while the optimizer-facing load is 0 W test_negative_ml_load_is_clipped_but_raw_prediction_stays_raw
Post-mix valid forecast, negative live value → −450 after blend clipped after mix, one warning test_negative_mix_result_is_clipped_after_mixing
Post-mix mix result becomes NaN False test_non_finite_mix_result_cannot_reach_optimization
Pre-mix ±Inf first step with mixing clean False, no OverflowError test_non_finite_forecast_does_not_crash_the_mix
Sign convention positive external list with load_negative=True not inverted test_external_positive_load_is_not_inverted_by_load_negative
Sign convention retrieved history with load_negative=True normalized exactly as before test_load_negative_still_normalizes_retrieved_history
PV negative list PV existing PV correction still clips test_negative_pv_is_still_clipped_by_existing_pv_correction
Solver boundary NaN/+Inf/bool/string for all 5 keys × dayahead + naive-MPC perform_*/perform_optimization never called test_invalid_external_forecast_never_reaches_the_solver
Solver boundary bool in P10 solver never called test_invalid_external_pv_pair_never_reaches_the_solver
Solver boundary negative prices/temperature reach the solver unchanged test_valid_signed_inputs_reach_the_solver
Solver boundary negative external load solver receives clipped P_Load test_negative_external_load_reaches_the_solver_clipped
Web route rejected load / price / temperature 400, solver never called test_rejected_forecast_returns_400_without_solving
P10 bool rejected in list P50, list P10, mapping P50, mapping P10 rejected before coercion; key + position/timestamp named test_boolean_values_are_rejected_in_p50_and_p10
P10 numeric-looking strings (e.g. "2.5"), ordinary strings, None/null rejected in list P50, list P10, mapping P50, mapping P10 rejected before coercion; key + position/timestamp named test_strings_and_null_are_rejected_in_p50_and_p10_before_coercion
P10 np.bool_; ordinary strings; None; NaN/±Inf via direct validator call rejected; error names "boolean"/"non-numeric"/"non-finite" test_direct_pair_validation_rejects_numpy_bool_and_non_finite
P10 individually finite P50 bucket values overflow to a non-finite mean during aggregation rejected after alignment, error names the key and "after timestamp alignment" test_timestamp_alignment_rejects_non_finite_aggregated_pair
P10 existing valid pair/bias/alignment/mismatch tests unchanged, all pass existing test_external_pv_p10.py

Final compatibility findings (Sourcery review of 7344170)

A later Sourcery review found two compatibility issues. Both were fixed in c12fbd5 (code + tests) and c7b57fb (docs), and Sourcery subsequently marked both review threads resolved.

1. Legacy stringified forecast-list compatibility

Existing EMHASS accepts a whole forecast list supplied as a string, such as "[1,2,3]". The earlier head validated before that conversion ran, so such payloads were rejected. The final implementation normalizes a valid stringified list with ast.literal_eval before forecast length/value validation. The parsed members are still subject to the #1135 finite-real contract:

  • "[1,2,3]" is normalized to a numeric list;
  • a parsed list containing a string member, such as [1, "2.5", 3], remains invalid;
  • bool, null/None, NaN and ±Inf remain invalid forecast members;
  • the numerical validity contract is not weakened.

2. Outdoor-temperature omission versus rejection

The previous guard conflated outdoor_temperature_forecast being omitted from runtime input with it being supplied but rejected: with outdoor_temperature_forecast_method: list configured, omission also failed the cycle. The final implementation records explicit rejection provenance (_outdoor_temperature_forecast_rejected), giving this contract:

  • omitted runtime outdoor_temperature_forecast → existing weather temp_air fallback remains available;
  • supplied valid runtime outdoor_temperature_forecast → runtime forecast is used;
  • supplied invalid runtime outdoor_temperature_forecast → fail closed; no silent fallback to weather.

Both cases are covered in the regression matrix above.

RED evidence (unmodified base 237d0fe)

The RED and mutation evidence below was produced during the implementation pass, before the two compatibility fixes above; the compatibility regressions are covered by the final-head test runs under Validation.

The implementation-pass test files were copied into a temporary detached worktree at the exact base SHA, with no source changes, and run there. The worktree was then deleted.

147 failed, 38 passed, 18 subtests passed   (pytest counts failing subtests individually)

The failures are behavioural, not missing names:

Base behaviour Signal
NaN/Inf/bool/None/string runtime values accepted silently no logs of level ERROR (79)
Invalid values reach the mocked solver (dayahead + MPC, all keys; P10 bool; web route) AssertionError: 1 != 0 solver calls (45)
Negative final load not clipped np.float64(-10.0) != 0.0, no logs of level WARNING
String in a timestamp mapping crashes inside pandas TypeError: agg function failed [how->mean,dtype->object]
±Inf reaches the mix blend OverflowError: cannot convert float infinity to integer
Bool in P50/P10 coerced to 1.0/0.0 [1.0, np.True_] is not None

Tests that pass on the base by design, because they guard unchanged behaviour: valid lists, long/short list, mapping alignment semantics, valid CSV, positive ML, load_negative (both directions), negative PV correction, valid signed inputs reaching the solver, and all pre-existing P10 tests.

Mutation / adversarial evidence

Each mutant was applied to the implementation and the focused suites were run. The exact original bytes were then restored, verified by SHA-256 after every mutant. No mutant was committed.

Mutant Result Killed by (examples)
A. coercible non-real values (bool, numeric-looking strings) accepted as numeric (helper + P10 pair) KILLED (32 failed) plain-list/mapping invalid tests, paired-PV regressions covering bool and numeric-looking strings before coercion, solver + web boundary
B. finite check removed KILLED (63) runtime invalid tests, CSV/ML/list non-finite, post-mix NaN, solver boundary
C. negative P_Load clip removed KILLED (9) final-load, CSV, ML, post-mix, every-method, solver-clipped
D. final guard moved before mix only KILLED (2) test_negative_mix_result_is_clipped_after_mixing, test_non_finite_mix_result_cannot_reach_optimization
E. generic >= 0 applied (signed prices/temps rejected) KILLED (19) valid signed lists, mapping semantics, signed solver test
F. raw ML clamped inside MLForecaster.predict() KILLED (2) raw-vs-optimizer ML test
G. web False handling removed KILLED (4) web route test
H. outdoor-temperature silent fallback restored KILLED (10) solver + web boundary
I. PV None weather frame not treated as failure KILLED (9) solver boundary (PV, P10)
J. mapping values not pre-validated before aggregation KILLED (35) mapping invalid test

Documentation audit

One canonical contract lives in docs/passing_data.md § Forecast input contract. Other pages link to it and only correct the context-specific claims.

Surface Disposition
docs/passing_data.md UPDATED. New canonical Forecast input contract: a table (key, unit, external numerical contract, optimizer-facing domain) covering all six keys including P10, plus numerical validity, plain-list and timestamp-mapping semantics, load sign convention (load_negative/set_zero_min = history prep), the final load check, and a fallback philosophy with no blanket zero. outdoor_temperature_forecast added to the key list. Final compatibility semantics: absence is different from rejection for outdoor_temperature_forecast (omitted → weather fallback available; supplied-but-invalid → cycle fails); existing whole-list string compatibility is normalized before validation; strings inside the resulting forecast list remain invalid; the canonical finite-real forecast-value contract is otherwise unchanged. The P10 paragraph covers the complete non-numeric/boolean/non-finite rejection contract: bool, np.bool_, numeric-looking strings, ordinary strings, None/null, NaN and ±Inf are all rejected before coercion, for both P50 and P10, in lists and mappings, with a second post-alignment check for timestamped pairs. Corrected the claim that runtime_params.json is "consumed by the generated OpenAPI spec": scripts/generate_openapi.py does not read it and /action/{action_name} has a generic-object request body.
docs/forecasts.md UPDATED. Replaced stale "nearest-neighbor" wording with the actual mean-aggregation / UTC-instant / hold-last / leading-backfill steps. List length is now "at least one per step, extra ignored, short rejected". Corrected "naive timestamps are assumed local": they are parsed as UTC (pd.to_datetime(..., utc=True)), which is a silent-shift risk. Cross-links the contract. Price-template | float(0) on current Nordpool/Amber prices changed to | float, so an unavailable sensor fails the call instead of sending price 0.
docs/study_cases/good_practices.md UPDATED. Removed "does not auto-resample / padding with zeros". Now covers list vs mapping behaviour, and replaces default(0) troubleshooting advice with a domain-appropriate fallback or fail-closed approach (0 W PV at night given as a legitimate example).
docs/mlforecaster.md UPDATED. New subsection: raw estimator prediction vs optimizer-facing P_Load. Clipping does not improve raw accuracy.
docs/config.md UPDATED. load_negative/set_zero_min are retrieved-history preparation only, with a link to the contract.
docs/cookbook/_template.md UPDATED. Rule B: list vs mapping, "short list rejected, never padded", a finite-value preflight, no blind 0 for load/price, and link rather than restate.
docs/cookbook/transport_nodered_mpc_orchestration.md UPDATED. Removed "pads / truncates silently". The snippet now fails closed (skips the tick) on missing, short or non-finite price arrays instead of fabricating 0.30/0.08 arrays, and allows negative prices. The caveat is rewritten with a link.
docs/cookbook/battery_aware_runtime_params.md NO CHANGE REQUIRED: it validates soc_init (SOC), which is not a forecast input covered by #1135.
docs/cookbook/ev_evcc_executor.md NO CHANGE REQUIRED: its | float(0) usages feed soc_init, def_total_hours and def_current_power, not forecast inputs, and the recipe documents that choice explicitly.
docs/cookbook/forecast_victoriametrics_long_history.md NO CHANGE REQUIRED: ML training-history storage, not runtime forecast ingestion.
docs/cookbook/tariff_demand_charge.md NO CHANGE REQUIRED: capacity-charge params only, no forecast arrays.
docs/cookbook/index.md NO CHANGE REQUIRED: listing only.
docs/study_cases/* (others: mpc, ev, basic_*, dhw, heat_pump_walkthrough, chance_constrained_mpc, reference_configs, legacy_cli) NO CHANGE REQUIRED: no padding, alignment, zero-fallback or sign claims. heat_pump_walkthrough passes a numeric outdoor-temperature list, and reference_configs only names *_method: list.
docs/thermal_model.md NO CHANGE REQUIRED: its literal 0.0 PV/load lists are a deliberate, known-zero example, which is valid under the contract.
docs/thermal_battery.md, docs/heat_topology.md NO CHANGE REQUIRED: their statements about outdoor-temperature precedence and the list weather path are still accurate.
docs/advanced_math_model.md, README.md, docs/usage_guide.md, docs/quick_start.md, docs/differences.md, docs/battery_identification.md NO CHANGE REQUIRED: numeric example payloads or parameter tables only; battery_identification mentions set_zero_min for unrelated battery sensors.
scripts/ NO CHANGE REQUIRED: scripts pass load_negative/set_zero_min from config into history preparation (unchanged semantics) or build numeric literal lists.
Web UI (src/emhass/static/script.js, templates) NOT APPLICABLE: only a pv_power_forecast placeholder key name.
param_definitions.json descriptions of load_negative/set_zero_min NO CHANGE REQUIRED: the descriptions already say "retrieved load variable"/"power consumption data". Editing this structured surface would also regenerate openapi.json, so docs/config.md carries the clarification instead.
CHANGELOG.md NOT APPLICABLE: release notes are maintainer-owned. No historical entry rewritten and no version invented.

Human contributor guidance

Surface Disposition
CONTRIBUTING.md NO CHANGE REQUIRED: it already routes to develop.md, AGENTS.md and develop_ai_coders.md, and this PR adds no new contributor workflow step.
docs/develop.md NO CHANGE REQUIRED: generic dev workflow, no forecast-contract claims.

AI guidance

Surface Disposition
AGENTS.md UPDATED. Replaced the stale "padded gracefully" paragraph with a concise rule set: list ≠ mapping, short list rejected, mapping aggregation/alignment retained, every forecast value must be a finite real number, bool/string/null/non-finite invalid (reuse utils.describe_invalid_forecast_value), legacy whole-list string normalization preserved but strings inside the parsed list invalid, an omitted outdoor_temperature_forecast may use the weather fallback while invalid supplied outdoor-temperature data fails closed, load/PV physical, prices signed, temperature signed, load_negative/set_zero_min = history-only preparation controls, external load is the canonical non-negative household consumption, raw ML stays raw, and timestamp shifts are never silent (naive timestamps are UTC), with a link to the canonical contract. Includes pv_power_forecast_p10. The Last verified against upstream/master marker is updated to 237d0fe, 2026-09-30.
docs/develop_ai_coders.md UPDATED. One pre-PR checklist item: when touching forecast ingestion, runtime forecast processing, mapping alignment, forecast post-processing or load/PV physical-domain handling, verify against the contract and keep tests/test_forecast_validity_contract.py (signed-domain and final-load tests) passing.

Machine-readable / API scope (non-goals)

  • src/emhass/static/data/runtime_params.json: unchanged. No forecast payload keys were added.
  • src/emhass/static/openapi.json and scripts/generate_openapi.py: unchanged. There is no /action schema redesign or new schema composition.
  • The only API-adjacent change is documentation: the inaccurate claim about runtime_params.json feeding the OpenAPI spec is corrected. A complete machine-readable /action forecast payload schema remains a separate follow-up, as noted in Forecast validity contract: non-finite forecast values and negative P_Load can reach optimisation #1135.

Validation

All fork-owned workflows below ran on MMicieli/emhass Actions against the exact final head 079f6a9cdd708581a0ff7b94492c46c5519ff702. Upstream davidusb-geek/emhass Actions are not used as the validation authority for this table.

Validation topology:

  • MMicieli/emhass fork master was safely fast-forwarded to the exact upstream base 237d0fe4089f76ff5f8bbcae24876e64008e60ad (no force push);
  • fork master had no unique fork commits and was simply 72 commits behind;
  • the temporary validation PR therefore represented exactly the real fix: enforce forecast validity at optimizer boundaries #1151 delta, rather than the earlier erroneous synthetic stale-master diff.
Workflow / job Result
Code Quality Scan PASS
ruff check PASS
ruff format --check --diff PASS — 125 files already formatted
Python test — Ubuntu PASS — 1286 passed, 1 skipped, 32 xfailed
Python test — macOS PASS — 1286 passed, 1 skipped, 32 xfailed
Python test — Windows PASS — 1287 passed, 32 xfailed
Docker build — amd64 PASS
Docker build — arm64 PASS
OSV tail jobs PASS
CodeQL PASS
CodeCov PASS

Documentation validation (from the implementation pass; not separately rebuilt by the fork workflow above, since the later remediation commits changed validator code, tests, one AGENTS.md bullet, and added two plain prose paragraphs to an existing docs/passing_data.md section — no new Sphinx headings, directives or links):

Check Result
git diff --check clean
sphinx-build -b html docs docs/_build build succeeded, 19 warnings

Sphinx baseline: the unmodified base build reports 19 warnings, so upstream is not -W clean and no warnings-as-errors success is claimed. With a clean feature build, the normalized warning set (paths and .md line numbers stripped) is identical to the base set: 19 warnings on both, with none added and none removed. This PR introduces no new Sphinx warning or error, including in the autodoc'd utils.describe_invalid_forecast_value docstring.

docs/_build and test-created data/last_run.json / data/plan_latest.json were removed and are not committed.

Base / head

  • Base: davidusb-geek/emhass@237d0fe4089f76ff5f8bbcae24876e64008e60ad (current master at the time of writing)
  • Head: 079f6a9cdd708581a0ff7b94492c46c5519ff702
  • 6 focused commits (17 changed files):
    1. ee0bb7b — initial Forecast validity contract: non-finite forecast values and negative P_Load can reach optimisation #1135 implementation (numerical validity contract, optimizer-facing load contract, docs);
    2. ab688c5 — paired-PV contract completion (pre-coercion validation reusing the canonical helper, post-alignment validation for timestamped pairs);
    3. 7344170 — test-only null-diagnostic expectation alignment;
    4. c12fbd5 — forecast compatibility semantics (legacy stringified-list normalization before validation; outdoor-temperature omission vs rejection provenance) with regressions;
    5. c7b57fb — docs: docs/passing_data.md and AGENTS.md clarify absence vs rejection and whole-list string compatibility;
    6. 079f6a9 — style: ruff formatting only.

Out of scope

  • /action OpenAPI request-body schema or forecast keys in runtime_params.json
  • Fail-closed handling for short lists (existing behaviour kept: error logged, value ignored)
  • Timestamp-less mapping keys (still parsed as UTC; now documented, not changed)
  • soc_init / deferrable-load template fallbacks in cookbook recipes (not forecast inputs)
  • Any change to MLForecaster, get_mix_forecast(), get_power_from_weather(), RetrieveHass.prepare_data(), P10 bias/alignment semantics, the optimizer formulation, tariffs, batteries or SOC
  • New dependency, config option, threshold or forecast backend
  • CHANGELOG / release notes (maintainer-owned)
  • PR fix: exclude pre-observation history from forecast calibration #1149 (fully independent: this branch is based on master only)

Summary by Sourcery

Enforce a consistent fail-closed validity contract for external forecasts and optimizer-facing household load values.

Bug Fixes:

  • Enforce finite-real validation for all externally supplied forecasts, rejecting non-finite, boolean, null, and non-numeric values before optimization.
  • Prevent invalid forecasts from falling back silently or reaching the solver, and return clean HTTP 400 responses for rejected optimization actions.
  • Enforce the optimizer-facing household-load contract by clipping finite negative loads to zero with a summarized warning and failing cleanly on non-finite loads.
  • Extend paired PV P50/P10 validation to reject invalid original values before coercion and non-finite aggregates after timestamp alignment.

Enhancements:

  • Centralize forecast-value diagnostics and document the numerical, alignment, sign, fallback, and optimizer-facing forecast contracts.
  • Preserve raw ML predictions while applying physical-domain correction only to optimizer-facing load forecasts.

Documentation:

  • Update forecast, configuration, cookbook, ML, and AI-coding documentation to describe the enforced forecast validity and alignment behavior.

Tests:

  • Add comprehensive regression coverage for forecast validation, load-domain enforcement, solver boundaries, web responses, mapping alignment, ML/CSV inputs, and paired PV forecasts.

Establish the forecast validity contract agreed in davidusb-geek#1135.

Numerical validity: every externally supplied forecast value
(pv_power_forecast, pv_power_forecast_p10, load_power_forecast,
load_cost_forecast, prod_price_forecast, outdoor_temperature_forecast)
must be a finite real number. NaN, +/-Inf, booleans and non-numeric
values are rejected with one diagnostic (key, reason, list position or
source timestamp, value). Timestamp-mapping values are checked before
pandas aggregation. A rejected key is passed as None with the list
method selected, so the cycle fails instead of falling back to the
configured method or the weather-forecast temperature. Signed prices
and temperatures remain valid. The external PV P50/P10 pair now rejects
booleans too; its other semantics are unchanged.

Physical-domain validity: Forecast.get_load_forecast() enforces the
optimizer-facing load contract once for every method, after the
optional mix: a non-finite value fails the cycle; a finite negative
value is clipped to 0 W with one summarized warning. A minimal pre-mix
check prevents +/-Inf from crashing the blend. Raw MLForecaster
predictions (forecast-model-predict) stay raw.

Rejected inputs now stop cleanly before the solver: None weather frames
and None price lists fail instead of raising, and /action answers 400
when an optimization action returns False.

Docs: canonical Forecast input contract in docs/passing_data.md, with
corrections and cross-links in forecasts.md, good_practices.md,
mlforecaster.md, config.md, the cookbook template and Node-RED recipe,
AGENTS.md and develop_ai_coders.md.

Fixes davidusb-geek#1135

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Enforces a single fail-closed finite-real forecast contract at runtime ingestion and optimizer boundaries, clips only finite negative household loads after mixing, preserves signed prices/temperatures and raw ML outputs, handles failures cleanly through CLI/web paths, and documents and tests the resulting behavior.

Sequence diagram for fail-closed forecast handling

sequenceDiagram
    participant Client
    participant Runtime as treat_runtimeparams
    participant Forecast
    participant Action
    participant Solver
    Client->>Runtime: Submit forecast values
    Runtime->>Runtime: describe_invalid_forecast_value
    alt invalid value
        Runtime-->>Action: passed_data=None, method=list
        Action->>Forecast: Build forecast
        Forecast-->>Action: False
        Action-->>Client: HTTP 400
    else valid values
        Runtime->>Runtime: Align timestamp mapping
        Runtime-->>Action: Valid forecast
        Action->>Forecast: get_load_forecast
        Forecast->>Forecast: Validate final P_Load after mixing
        alt finite negative load
            Forecast->>Forecast: clip(lower=0)
        else non-finite load
            Forecast-->>Action: False
            Action-->>Client: HTTP 400
        end
        Forecast-->>Action: Valid optimizer-facing load
        Action->>Solver: Perform optimization
        Solver-->>Client: Optimization result
    end
Loading

Flow diagram for forecast validation to optimization

flowchart LR
    A[External forecast input] --> B[describe_invalid_forecast_value]
    B -->|invalid finite-real value| C[Reject and set passed_data to None]
    C --> D[Fail action before solver]
    B -->|valid| E[Align or aggregate timestamp mapping]
    E --> F[Forecast generation and optional mixing]
    F --> G[get_load_forecast]
    G -->|non-finite P_Load| D
    G -->|finite negative P_Load| H[Clip load to 0 W]
    G -->|finite non-negative P_Load| I[Optimizer]
    H --> I
Loading

File-Level Changes

Change Details Files
Centralize and enforce finite-real validation for all externally supplied forecast values before coercion or optimizer execution.
  • Added a shared diagnostic validator rejecting NaN, infinities, booleans, nulls, and strings while preserving list-position or timestamp provenance.
  • Validated runtime lists and mappings before and after alignment, failing closed with a rejected sentinel instead of falling back to configured methods.
  • Extended paired PV P50/P10 validation to pre-coercion source values and post-alignment aggregates, including overflow detection.
src/emhass/utils.py
Enforce the optimizer-facing household-load physical domain at the final forecast boundary.
  • Added a pre-mix guard to prevent invalid values from crashing feedback blending.
  • Added a post-mix finite check and one summarized warning for clipping finite negative loads to zero while preserving the Series metadata.
  • Handled rejected runtime sentinels cleanly for price forecasts and prevented rejected weather data from reaching PV conversion.
src/emhass/forecast.py
src/emhass/command_line.py
Make failed forecast validation terminate optimization and produce correct API failure responses.
  • Prevented rejected outdoor-temperature forecasts from falling back to weather temperature.
  • Returned HTTP 400 with the action log when optimization actions return False instead of attempting to render a plan.
  • Ensured invalid forecasts do not invoke dayahead or MPC solvers.
src/emhass/command_line.py
src/emhass/web_server.py
Document the canonical forecast validity, alignment, sign, and fallback contract.
  • Added a reference contract covering all forecast keys, finite-real requirements, signed prices/temperatures, non-negative load/PV domains, and fail-closed behavior.
  • Documented list and timestamp-mapping length, aggregation, timezone, hold-last, and backfill semantics.
  • Updated ML, configuration, cookbook, study-case, and AI coding guidance to reflect the contract and remove zero-padding or silent-fallback claims.
docs/passing_data.md
docs/forecasts.md
docs/mlforecaster.md
docs/config.md
docs/cookbook/_template.md
docs/cookbook/transport_nodered_mpc_orchestration.md
docs/study_cases/good_practices.md
AGENTS.md
docs/develop_ai_coders.md
Add comprehensive regression coverage for validation and optimizer-boundary behavior.
  • Added 26-contract-test coverage across runtime representations, load methods, mixing, CSV/ML paths, signed values, solver boundaries, web routes, and physical clipping.
  • Added paired-PV tests for booleans, strings, nulls, NumPy booleans, non-finite values, and post-alignment overflow.
tests/test_forecast_validity_contract.py
tests/test_external_pv_p10.py

Assessment against linked issues

Issue Objective Addressed Explanation
#1135 Enforce a finite real-number contract for all externally supplied forecast values, rejecting NaN, infinities, booleans, nulls, and non-numeric values before optimisation while preserving signed prices and temperatures. ✅
#1135 Enforce a common optimiser-facing household-load contract after all load processing, including optional mixing: reject non-finite values, clip finite negative loads to zero with a summarized warning, and cover runtime, CSV, native-ML, and other load methods without changing raw ML diagnostics or load_negative semantics. ✅
#1135 Add regression coverage and reconcile the canonical and contributor-facing documentation with the forecast validity, alignment, sign-convention, fallback, and optimiser-boundary contracts without introducing unrelated API or configuration changes. ✅

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. Incorrect validation, clipping, or timestamp alignment could reject an optimization cycle or produce a wrong schedule that is persisted or acted on before the issue is noticed. Reverting restores the prior behavior, but schedules already generated or applied would need to be rerun or corrected.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

sourcery-ai[bot]
sourcery-ai Bot previously approved these changes Sep 30, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery assessment

Approved.

@sourcery-ai
sourcery-ai Bot dismissed their stale review September 30, 2026 01:15

Sourcery withdrew this approval because the latest commits introduced blocking findings.

@MMicieli
MMicieli marked this pull request as draft September 30, 2026 07:14
@MMicieli
MMicieli marked this pull request as ready for review September 30, 2026 07:14

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/emhass/utils.py" line_range="2141" />
<code_context>
+        passed_outdoor_temp is None
+        and input_data_dict["fcst"].optim_conf.get("outdoor_temperature_forecast_method") == "list"
+    ):
+        logger.error(
+            "outdoor_temperature_forecast was supplied but rejected; "
+            "not falling back to the weather forecast temperature."
</code_context>
<issue_to_address>
**issue (broader_impact):** A forecast supplied as a stringified list is rejected before the existing `ast.literal_eval` conversion runs: the new list/type branch logs an error and leaves `passed_data[forecast_key]` unset, so the later conversion cannot make the forecast usable.

**Triggers:** When callers use the pre-existing string representation of a list, such as `"[1, 2, 3]"`, for a forecast runtime parameter.

**Suggested fix:** Parse a stringified list before applying the list-length and value validation, then validate the parsed values while still rejecting strings as individual forecast entries.
</issue_to_address>

### Comment 2
<location path="src/emhass/command_line.py" line_range="2644-2647" />
<code_context>
+    # treat_runtimeparams selects the "list" method but passes no data when it
+    # rejects a supplied outdoor_temperature_forecast (#1135). Fail the cycle
+    # instead of silently substituting the weather-forecast temperature.
+    if (
+        passed_outdoor_temp is None
+        and input_data_dict["fcst"].optim_conf.get("outdoor_temperature_forecast_method") == "list"
+    ):
+        logger.error(
+            "outdoor_temperature_forecast was supplied but rejected; "
</code_context>
<issue_to_address>
**issue (bug_risk):** The new guard treats every missing `outdoor_temperature_forecast` as a rejected runtime forecast whenever the configured method is `list`, so a legitimate configuration that selects the list method without supplying runtime outdoor-temperature data fails instead of using the existing weather-temperature fallback.

**Triggers:** When `outdoor_temperature_forecast_method` is configured as `list` and the runtime payload omits `outdoor_temperature_forecast` rather than supplying an invalid value.

**Suggested fix:** Distinguish an absent runtime key from a rejected supplied key, and only fail for the latter.
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 2 findings to address first, and invalid or negatively clipped forecasts can change the optimizer's dispatch plan, while rejected inputs can stop an optimization cycle and leave automated controls without a new plan. Reverting restores the prior behavior for future cycles, but any control decision already made or missed during the bad cycle cannot be undone.

Blocking findings: src/emhass/utils.py:2141, src/emhass/command_line.py:2647


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread src/emhass/utils.py
Comment thread src/emhass/command_line.py Outdated

This branch has not been deployed

No deployments
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.

Forecast validity contract: non-finite forecast values and negative P_Load can reach optimisation

1 participant