UX-02: the panel was weaker than the startup gate for one field
checks / unit (pull_request) Successful in 6m9s
checks / unit (pull_request) Successful in 6m9s
Applying review finding N1, which was non-blocking because it fails closed, and
N2, which is a fabricated number.
N1. Clearing the `max_w` box answered HTTP 500 on a non-JSON body, which the
page rendered as "cannot reach the add-on" - untrue, and for the most prominent
limit field's most likely transient state. The cause was a disagreement between
two files that `inverter.py`'s own docstring says to keep in step:
validate.py `_number()` reads "" as "use the default", so rule 9 saw 2000
and was content
inverter.py line 515 did a bare `float(opts.get("max_w", 2000))`, which
raises ValueError on ""
So a shape rule 9 TOLERATED could not be composed at all - the docstring's rule
inverted. The roster path never had this: `_entry_max_w` already guards. Only
the flat single-inverter path did. `_flat_max_w()` now reads empty as the
default exactly as `_number()` does, and unparseable as 0.0 exactly as
`_entry_max_w` does, where rule 9 refuses it by name at line 790.
⚠️ THE FIRST ATTEMPT AT THIS MADE IT WORSE, AND THE PROBE CAUGHT IT. Guarding
`build_roster` in `selfconfig` with `except: return validate(opts), []` turned
the 500 into ZERO PROBLEMS for `max_w: ""` - the validator has nothing to say
about it, so an empty list came back and the page would have ENABLED SAVE. A
crash became a silent acceptance. The guard is kept as a backstop for shapes
neither file anticipates, but it can no longer return an empty list: if the
roster cannot be composed and the validator names no reason, that disagreement
is itself reported.
MEASURED after the fix, `problems_and_ranges` with no HA:
max_w 5000, slew_w 400 0 problems (the live shape)
max_w "", slew_w 400 1 problem rule 10, "against a site rail of
2000 W" - clearing the box FAILS
CLOSED on the live install rather
than silently halving the rail
max_w "", slew_w 0 0 problems accepted at the 2000 default
max_w "abc" 1 problem "max_w 'abc' is not a number"
Before the fix, both "" and "abc" raised ValueError instead.
N2. `translations/en.yaml:7` said "all 71 keys ... so ALL SEVENTY are rendered".
The 70->71 correction had been made in one half of the sentence and not the
other. 59-of-71 required is correct and was re-measured.
Suite: 24 files, all pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
co-authored by
Claude Opus 5
parent
fdaf3fb54f
commit
2bb07ccdde
@@ -512,7 +512,7 @@ def build_roster(opts: dict, store, sign_cfg=None,
|
|||||||
soc_entity=opts.get("soc_entity", ""),
|
soc_entity=opts.get("soc_entity", ""),
|
||||||
batt_entity=opts.get("batt_entity", ""),
|
batt_entity=opts.get("batt_entity", ""),
|
||||||
batt_invert=bool(opts.get("batt_invert")),
|
batt_invert=bool(opts.get("batt_invert")),
|
||||||
max_w=float(opts.get("max_w", 2000)),
|
max_w=_flat_max_w(opts),
|
||||||
# SAFETY-32. THE FLAT KEY, because this IS the single-inverter shape
|
# SAFETY-32. THE FLAT KEY, because this IS the single-inverter shape
|
||||||
# - the one pack the flat options describe is the one pack whose
|
# - the one pack the flat options describe is the one pack whose
|
||||||
# cut-out `bms_soc_floor` states. Defaulted from CFG-03's single
|
# cut-out `bms_soc_floor` states. Defaulted from CFG-03's single
|
||||||
@@ -588,6 +588,36 @@ def _entry_bms_floor(entry: dict):
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def _flat_max_w(opts: dict) -> float:
|
||||||
|
"""The single-inverter shape's cap, tolerating what rule 9 tolerates.
|
||||||
|
|
||||||
|
⚠️ THIS USED TO BE A BARE `float(opts.get("max_w", 2000))` AND IT
|
||||||
|
DISAGREED WITH THE VALIDATOR. `validate.py`'s `_number()` reads `""` as
|
||||||
|
"use the default", so rule 9 looks at an empty `max_w` box, sees 2000 and
|
||||||
|
is content - while the bare `float("")` here raised ValueError from inside
|
||||||
|
`Controller.__init__`. Through UX-02's panel that surfaced as HTTP 500 on a
|
||||||
|
non-JSON body, which the page reported as "cannot reach the add-on": untrue,
|
||||||
|
and for the most prominent limit field's most likely transient state.
|
||||||
|
|
||||||
|
That inverted this module's own docstring rule - "keep the two in step: a
|
||||||
|
shape this tolerates that rule 9 does not name is a shape that runs" - in
|
||||||
|
the other direction: a shape rule 9 TOLERATED could not be composed at all.
|
||||||
|
The roster path never had the bug, because `_entry_max_w` below already
|
||||||
|
guards; only the flat path did.
|
||||||
|
|
||||||
|
Empty or absent is the default, exactly as `_number()` reads it.
|
||||||
|
Unparseable is 0.0, exactly as `_entry_max_w` reads it - and rule 9 refuses
|
||||||
|
it by name at line 790, with a sentence, before a control loop exists.
|
||||||
|
"""
|
||||||
|
raw = opts.get("max_w", 2000)
|
||||||
|
if raw is None or raw == "":
|
||||||
|
return 2000.0
|
||||||
|
try:
|
||||||
|
return float(raw)
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
return 0.0
|
||||||
|
|
||||||
|
|
||||||
def _entry_max_w(entry: dict) -> float:
|
def _entry_max_w(entry: dict) -> float:
|
||||||
"""One entry's supervised cap, or 0.0 when it did not state a usable one.
|
"""One entry's supervised cap, or 0.0 when it did not state a usable one.
|
||||||
|
|
||||||
|
|||||||
@@ -304,7 +304,36 @@ async def problems_and_ranges(opts: dict, hass=None):
|
|||||||
one. That was the first shape of this and it disagreed with itself.
|
one. That was the first shape of this and it disagreed with itself.
|
||||||
"""
|
"""
|
||||||
flat = str(opts.get("setpoint_entity", "") or "").strip()
|
flat = str(opts.get("setpoint_entity", "") or "").strip()
|
||||||
units = build_roster(opts, None)
|
|
||||||
|
# ⚠️ THE ROSTER IS BUILT BEFORE THE VALIDATOR RUNS, AND IT PARSES NUMBERS.
|
||||||
|
# `build_roster` reaches `inverter.py`'s `float(opts.get("max_w", 2000))`,
|
||||||
|
# which raises on a value the validator has a SENTENCE for. Cleared the
|
||||||
|
# `max_w` box and the panel answered HTTP 500 - not JSON, so the page said
|
||||||
|
# "cannot reach the add-on", which was untrue - while `validate.py`'s
|
||||||
|
# "max_w ... is not a number" was unreachable through the panel and reachable
|
||||||
|
# at startup, where `main.py` validates before it composes. That made this
|
||||||
|
# surface WEAKER than the startup gate for one field, which is the opposite
|
||||||
|
# of what this module claims below.
|
||||||
|
#
|
||||||
|
# A malformed scalar therefore falls back to validating WITHOUT device
|
||||||
|
# bounds. Those bounds come from the roster, and there is no roster; the
|
||||||
|
# validator's own complaint about the unparseable value is the answer the
|
||||||
|
# operator needs, and it is strictly more informative than a 500.
|
||||||
|
try:
|
||||||
|
units = build_roster(opts, None)
|
||||||
|
except (TypeError, ValueError) as exc:
|
||||||
|
# ⚠️ AND IT MUST NOT RETURN AN EMPTY LIST. An empty list means "no
|
||||||
|
# problems", and the page enables Save on it. If the roster could not be
|
||||||
|
# composed at all and the validator has nothing to say about why, that
|
||||||
|
# disagreement is itself the finding - so it is reported rather than
|
||||||
|
# swallowed. Returning `[]` here would turn a crash into a silent
|
||||||
|
# acceptance, which is worse than the 500 this replaced.
|
||||||
|
found = list(validate_startup_config(opts))
|
||||||
|
if not found:
|
||||||
|
found = [f"the configuration could not be composed ({type(exc).__name__}: "
|
||||||
|
f"{exc}), and the validator did not name a reason. This is a "
|
||||||
|
f"disagreement between the two and should be reported."]
|
||||||
|
return found, []
|
||||||
if hass is not None:
|
if hass is not None:
|
||||||
async def _limits(unit):
|
async def _limits(unit):
|
||||||
if unit.setpoint_entity:
|
if unit.setpoint_entity:
|
||||||
|
|||||||
@@ -4,7 +4,7 @@
|
|||||||
# Without a `translations/` directory the Configuration tab renders the raw
|
# Without a `translations/` directory the Configuration tab renders the raw
|
||||||
# option KEYS - `unit_sign_disagree_w`, `saturation_cycles`, `integrator_max_w`.
|
# option KEYS - `unit_sign_disagree_w`, `saturation_cycles`, `integrator_max_w`.
|
||||||
# The frontend keeps a field when `e.name in options || e.required`, and all 71
|
# The frontend keeps a field when `e.name in options || e.required`, and all 71
|
||||||
# keys are in `options:` (59 of them are `required` as well), so ALL SEVENTY are
|
# keys are in `options:` (59 of them are `required` as well), so ALL SEVENTY-ONE are
|
||||||
# rendered, always, in `schema:` order, with no description. This file is what
|
# rendered, always, in `schema:` order, with no description. This file is what
|
||||||
# turns that into sentences.
|
# turns that into sentences.
|
||||||
#
|
#
|
||||||
|
|||||||
Reference in New Issue
Block a user