diff --git a/apps/predbat/components.py b/apps/predbat/components.py index 9e7835290..7c4a6db9d 100644 --- a/apps/predbat/components.py +++ b/apps/predbat/components.py @@ -123,9 +123,9 @@ def load_component_class(component_info): "solcast_host": {"required": False, "config": "solcast_host", "default": "https://api.solcast.com.au/"}, "solcast_api_key": {"required": False, "secret": True, "config": "solcast_api_key"}, "solcast_sites": {"required": False, "config": "solcast_sites"}, - "solcast_poll_hours": {"required": False, "config": "solcast_poll_hours", "default": 8}, + "solcast_poll_hours": {"required": False, "config": "solcast_poll_hours", "default": 8.0}, "forecast_solar": {"required": False, "config": "forecast_solar", "default": False}, - "forecast_solar_max_age": {"required": False, "config": "forecast_solar_max_age", "default": 8}, + "forecast_solar_max_age": {"required": False, "config": "forecast_solar_max_age", "default": 8.0}, "forecast_solar_open_meteo_backup": {"required": False, "config": "forecast_solar_open_meteo_backup", "default": False}, "forecast_solar_open_meteo_first": {"required": False, "config": "forecast_solar_open_meteo_first", "default": False}, "pv_forecast_today": {"required": False, "config": "pv_forecast_today"}, @@ -134,7 +134,7 @@ def load_component_class(component_info): "pv_forecast_d4": {"required": False, "config": "pv_forecast_d4"}, "pv_scaling": {"required": False, "config": "pv_scaling", "default": 1.0}, "open_meteo_forecast": {"required": False, "config": "open_meteo_forecast", "default": False}, - "open_meteo_forecast_max_age": {"required": False, "config": "open_meteo_forecast_max_age", "default": 4}, + "open_meteo_forecast_max_age": {"required": False, "config": "open_meteo_forecast_max_age", "default": 4.0}, }, "required_or": ["solcast_api_key", "forecast_solar", "pv_forecast_today", "open_meteo_forecast"], "phase": 2, # Solar component moved to phase 2 so that any Predbat cloud components (such as GEcloud) have been started and initialised pv_today, etc @@ -458,7 +458,7 @@ def load_component_class(component_info): # one from, so it is estimated from poinv. This is the escape hatch for a user # who knows their pack's real limit. "battery_rate_max": {"required": False, "config": "alphaess_battery_rate_max"}, - "api_delay": {"required": False, "default": 2, "config": "alphaess_api_delay"}, + "api_delay": {"required": False, "default": 2.0, "config": "alphaess_api_delay"}, "min_write_interval": {"required": False, "default": 300, "config": "alphaess_min_write_interval"}, }, # Gate activation on having an AppID. Without this the component would start for @@ -605,7 +605,7 @@ def load_component_class(component_info): "event_filter": "predbat_axle_", "args": { "api_key": {"required": False, "secret": True, "config": "axle_api_key"}, - "pence_per_kwh": {"required": False, "config": "axle_pence_per_kwh", "default": 100}, + "pence_per_kwh": {"required": False, "config": "axle_pence_per_kwh", "default": 100.0}, "automatic": {"required": False, "config": "axle_automatic", "default": True}, "managed_mode": {"required": False, "config": "axle_managed_mode", "default": False}, "site_id": {"required": False, "secret": True, "config": "axle_site_id"}, diff --git a/apps/predbat/octopus.py b/apps/predbat/octopus.py index bbb79a5a6..64cc3b6c7 100644 --- a/apps/predbat/octopus.py +++ b/apps/predbat/octopus.py @@ -1323,7 +1323,7 @@ def get_saving_session_data(self): # Default saving session rate in octopoints/kWh # octopus_saving_session_rate is in p/kWh, convert to octopoints octopoints_per_penny = self.get_arg("octopus_saving_session_octopoints_per_penny", 8) - default_rate_pence = self.get_arg("octopus_saving_session_rate", 100) # 100p/kWh default + default_rate_pence = self.get_arg("octopus_saving_session_rate", 100.0) # 100p/kWh default default_octopoints = default_rate_pence * octopoints_per_penny if not has_joined: @@ -4212,7 +4212,7 @@ def fetch_octopus_sessions(self, axle_sessions=None): if "octopus_saving_session" in self.args: saving_rate = 200 # Default rate if not reported octopoints_per_penny = self.get_arg("octopus_saving_session_octopoints_per_penny", 8) # Default 8 octopoints per penny - octopoints_min_threshold = self.get_arg("octopus_saving_session_min_octopoints_per_kwh", 0) + octopoints_min_threshold = self.get_arg("octopus_saving_session_min_octopoints_per_kwh", 0.0) join_lead_hours = self.get_arg("octopus_saving_auto_join_lead_hours", 0) joined_events = [] @@ -4327,7 +4327,7 @@ def fetch_octopus_sessions(self, axle_sessions=None): # Default saving session rate for when octopoints_per_kwh is not available # (e.g. new flexibility API events that don't report reward rates) - default_rate_pence = self.get_arg("octopus_saving_session_rate", 0) + default_rate_pence = self.get_arg("octopus_saving_session_rate", 0.0) if joined_events: for event in joined_events: diff --git a/apps/predbat/tests/test_integer_config.py b/apps/predbat/tests/test_integer_config.py index 0aa8b5832..48174eb5c 100644 --- a/apps/predbat/tests/test_integer_config.py +++ b/apps/predbat/tests/test_integer_config.py @@ -478,3 +478,102 @@ def test_metric_battery_value_scaling_step_resolves_export_margin(my_predbat): print("✓ Test passed: metric_battery_value_scaling step {} keeps the first nudge ({:.4f}) clear of the {:.4f} flip point and keeps old 0.1-step values valid".format(step, first_nudge, flip_point)) return False + + +def _float_declared_keys(): + """The APPS_SCHEMA keys whose declared type includes "float".""" + from config import APPS_SCHEMA + + return {key for key, spec in APPS_SCHEMA.items() if "float" in str(spec.get("type", "")).split("|")} + + +def test_float_declared_keys_never_read_with_int_default(my_predbat): + """ + Guard for #4925: no call site may read a key APPS_SCHEMA declares "float" with a bare int literal default. + + get_arg() coerces its return value on the *type* of the default it is handed and applies that to + whatever value was resolved, real configured value or not. So `get_arg("solcast_poll_hours", 8)` + ran a configured 4.8 through int(float(value)) and returned 4 - shortening the Solcast poll TTL + to 4h and pushing a two-site hobbyist account past its 10 poll/day quota. Nothing warned: + validate_config() checks the raw apps.yaml value against the float schema and passes it. + + The fix is a float literal at each call site. This scans the source rather than asserting on + today's eight sites, so a new `get_arg("", 8)` or a COMPONENT_LIST spec with + `"default": 8` fails here instead of truncating in production. It sees literals only - a + default passed through a variable, or a key read through a wrapper accessor, is not checked. + """ + print("**** test_float_declared_keys_never_read_with_int_default ****") + import ast + import glob + import os + import components + + float_keys = _float_declared_keys() + source_dir = os.path.dirname(os.path.abspath(components.__file__)) + offenders = [] + scanned = 0 + for path in sorted(glob.glob(os.path.join(source_dir, "*.py"))): + scanned += 1 + with open(path, encoding="utf-8") as source: + tree = ast.parse(source.read()) + for node in ast.walk(tree): + if isinstance(node, ast.Call) and getattr(node.func, "attr", getattr(node.func, "id", None)) == "get_arg" and node.args and isinstance(node.args[0], ast.Constant) and node.args[0].value in float_keys: + default = node.args[1] if len(node.args) > 1 else None + for keyword in node.keywords: + if keyword.arg == "default": + default = keyword.value + if isinstance(default, ast.Constant) and type(default.value) is int: + offenders.append("{}:{} get_arg({!r}, {})".format(os.path.basename(path), node.lineno, node.args[0].value, default.value)) + elif isinstance(node, ast.Dict): + keys = [key.value if isinstance(key, ast.Constant) else None for key in node.keys] + if "config" in keys and "default" in keys: + config = node.values[keys.index("config")] + default = node.values[keys.index("default")] + if isinstance(config, ast.Constant) and config.value in float_keys and isinstance(default, ast.Constant) and type(default.value) is int: + offenders.append("{}:{} arg spec {!r} default {}".format(os.path.basename(path), node.lineno, config.value, default.value)) + + assert scanned > 10 and float_keys, "Guard found nothing to scan ({} files, {} float keys) - has the layout changed?".format(scanned, len(float_keys)) + assert not offenders, "Float-declared keys read with an int default, which truncates a configured fraction (#4925) - write e.g. 8.0: {}".format("; ".join(offenders)) + + print("✓ Test passed: {} float-declared keys, no int defaults across {} source files".format(len(float_keys), scanned)) + return False + + +def test_component_arg_specs_resolve_float_declared_keys_as_float(my_predbat): + """ + Behavioural companion to the source guard above (#4925): every COMPONENT_LIST arg spec with a + default whose "config" key APPS_SCHEMA declares "float" must resolve a configured fraction + intact, resolved exactly the way Components.initialize() does + (`arg_dict[arg] = self.base.get_arg(arg_info["config"], default, indirect=indirect)`). + """ + print("**** test_component_arg_specs_resolve_float_declared_keys_as_float ****") + + from components import COMPONENT_LIST + + float_keys = _float_declared_keys() + original_args = my_predbat.args.copy() + checked = [] + try: + for component_name, component_info in COMPONENT_LIST.items(): + for arg, arg_info in component_info.get("args", {}).items(): + config_name = arg_info.get("config", None) + if not config_name or arg_info.get("config_late_resolve", False): + continue + if config_name not in float_keys: + continue + if arg_info.get("default", None) is None: + # No default means no coercion, so there is nothing that could truncate + continue + + my_predbat.args[config_name] = 4.8 + value = my_predbat.get_arg(config_name, arg_info.get("default", None), indirect=arg_info.get("indirect", False)) + my_predbat.args.pop(config_name, None) + assert value == 4.8, "{}.{} ({}) resolved a configured 4.8 as {} ({}) - a float-declared key must not be truncated by its arg spec default".format(component_name, arg, config_name, value, type(value)) + checked.append(config_name) + finally: + my_predbat.args = original_args + + assert checked, "No float-declared component arg specs found to check - has APPS_SCHEMA or COMPONENT_LIST changed shape?" + + print("✓ Test passed: {} float-declared component arg spec(s) resolve fractional values intact: {}".format(len(checked), ", ".join(sorted(set(checked))))) + return False diff --git a/apps/predbat/unit_test.py b/apps/predbat/unit_test.py index 0f3d5a499..89cfdaef7 100644 --- a/apps/predbat/unit_test.py +++ b/apps/predbat/unit_test.py @@ -254,6 +254,8 @@ test_get_ha_config_normalises_int_default_for_fractional_step, test_metric_battery_cycle_fractional_value_not_truncated, test_metric_battery_value_scaling_step_resolves_export_margin, + test_float_declared_keys_never_read_with_int_default, + test_component_arg_specs_resolve_float_declared_keys_as_float, ) from tests.test_predbat_metrics_data_age import test_data_age_metrics_round_trip from tests.test_metrics_dashboard_control_conflicts import test_control_conflicts_metrics_round_trip, test_control_conflicts_dashboard_renders_section @@ -669,6 +671,8 @@ def main(): ("get_ha_config_fractional_default", test_get_ha_config_normalises_int_default_for_fractional_step, "get_ha_config normalises int default to float for fractional-step items (#4296)", False), ("metric_battery_cycle_fractional", test_metric_battery_cycle_fractional_value_not_truncated, "metric_battery_cycle fractional value not truncated by get_arg (#4296)", False), ("metric_battery_value_scaling_step", test_metric_battery_value_scaling_step_resolves_export_margin, "metric_battery_value_scaling step resolves the export margin (#4840)", False), + ("float_keys_float_defaults", test_float_declared_keys_never_read_with_int_default, "No float-declared APPS_SCHEMA key is read with an int default (#4925)", False), + ("component_arg_float_specs", test_component_arg_specs_resolve_float_declared_keys_as_float, "COMPONENT_LIST arg specs resolve float-declared keys as float (#4925)", False), ("data_age_metrics", test_data_age_metrics_round_trip, "Metrics dashboard data_age_days/data_age_required_days tests", False), ("control_conflicts_metrics", test_control_conflicts_metrics_round_trip, "Metrics dashboard control_conflicts round-trip tests", False), ("control_conflicts_dashboard", test_control_conflicts_dashboard_renders_section, "Metrics dashboard control_conflicts section render tests", False),