diff --git a/script/sync_components.py b/script/sync_components.py index 3800d0f8..4888eb63 100644 --- a/script/sync_components.py +++ b/script/sync_components.py @@ -554,6 +554,15 @@ # ``"5min"``, ``"1h30s"``. This regex matches that shape. _TIME_PERIOD_DEFAULT = re.compile(r"^\d+(\.\d+)?\s*(ms|us|ns|s|min|h|d)(\d+\s*\w+)*$") +# On-the-wire spelling of the schema's UI-hint enum (mirrors +# upstream esphome's ``cv.Visibility`` ``StrEnum`` values from +# esphome/esphome#16267). The dumper emits these strings; the +# catalog consumer compares against them. Two-tier strictness: +# ``YAML_ONLY`` is strictly stronger than ``ADVANCED``, which is +# strictly stronger than no setting at all. +_VISIBILITY_ADVANCED = "advanced" +_VISIBILITY_YAML_ONLY = "yaml_only" + # Base entity / framework fields that always render under "Advanced" by # default — valid but rarely tweaked. Same set as the previous sync. _ADVANCED_BASE_KEYS: frozenset[str] = frozenset( @@ -1619,7 +1628,60 @@ def _extract_config_entries( schema = config_schema.get("schema") or {} if not schema: return [] - return _convert_config_vars(schema, schema_dir, component_id=component_id) + entries = _convert_config_vars(schema, schema_dir, component_id=component_id) + # Apply the visibility-cascade rule once at the top of the + # tree: a stricter parent forces all descendants at-least + # as strict. ``YAML_ONLY`` > ``ADVANCED`` > no setting. The + # cascade is at-the-top so nested-NESTED structures get the + # full chain of ancestors considered without recursive + # bookkeeping inside ``_convert_field``. + _apply_visibility_cascade(entries, parent_advanced=False, parent_yaml_only=False) + return entries + + +def _apply_visibility_cascade( + entries: list[dict], + *, + parent_advanced: bool, + parent_yaml_only: bool, +) -> None: + """In-place push parent strictness onto descendants. + + ``YAML_ONLY`` (mapped to ``hidden=True``) is strictly stronger + than ``ADVANCED`` (``advanced=True``), which is strictly + stronger than the un-marked default. A child can declare its + own setting independently — but the child's *effective* + setting after this pass is ``max(parent_chain, self)``. + + The rationale is UX: if a parent block is "advanced", every + field inside it is at-least advanced (otherwise the disclosure + is leaky — you'd hide the parent header but render a child on + the main form). Same one level deeper for ``YAML_ONLY`` — a + block hidden from the editor must hide every descendant or + the user gets a half-rendered control with no way to set the + surrounding context. + """ + for entry in entries: + own_advanced = entry.get("advanced", False) + own_hidden = entry.get("hidden", False) + # Strictness ordering: ``YAML_ONLY`` (``hidden=True``) is + # strictly stronger than ``ADVANCED`` (``advanced=True``). + # Apply that locally first — a self-hidden entry is also + # implicitly advanced — then OR with the parent's + # strictness so the cascade pushes both flags down. + entry["advanced"] = own_advanced or own_hidden or parent_advanced or parent_yaml_only + entry["hidden"] = own_hidden or parent_yaml_only + # Recurse into NESTED groups, MAP value templates, and any + # other shape that carries inner ``config_entries``. The + # child's effective state becomes the parent state for the + # next level. + inner = entry.get("config_entries") + if isinstance(inner, list): + _apply_visibility_cascade( + inner, + parent_advanced=entry["advanced"], + parent_yaml_only=entry["hidden"], + ) def _convert_config_vars( @@ -1819,7 +1881,26 @@ def _convert_field(key: str, raw: dict, schema_dir: Path) -> dict | None: # noq # form even when optional — users almost always want to see what's # wired to what. is_structural = entry_type == "pin" or bool(references) - advanced = _classify_advanced(key, required=required, is_structural=is_structural) + # Schema-author UI hint from upstream esphome + # (esphome/esphome#16267): the dumper emits ``"visibility": + # "advanced" | "yaml_only"`` for fields whose ``cv.Optional`` / + # ``cv.Required`` set ``visibility=Visibility.ADVANCED`` or + # ``=Visibility.YAML_ONLY``. Absent → fall back to the name-based + # heuristic (the long tail of fields the schema doesn't yet + # annotate; as upstream adoption grows the heuristic rules out + # of ``_classify_advanced`` can shrink toward zero). + # + # The cascade rule (a stricter parent forces its descendants + # at-least as strict) is applied after leaf conversion by + # :func:`_apply_visibility_cascade`. This function records the + # per-field setting as the schema author wrote it; the cascade + # pass walks the resulting tree and pushes parent strictness + # down where descendants would otherwise be more visible. + schema_visibility = raw.get("visibility") + advanced = schema_visibility == _VISIBILITY_ADVANCED or _classify_advanced( + key, required=required, is_structural=is_structural + ) + yaml_only = schema_visibility == _VISIBILITY_YAML_ONLY entry: dict[str, Any] = { "key": key, @@ -1841,7 +1922,11 @@ def _convert_field(key: str, raw: dict, schema_dir: Path) -> dict | None: # noq "pin_features": _resolve_pin_features(raw) if entry_type == "pin" else [], "pin_mode": None, "advanced": advanced, - "hidden": False, + # ``yaml_only`` from the schema → ``hidden`` on the catalog + # entry. The frontend already knows how to skip ``hidden`` + # entries; the rename keeps the consumer-facing surface + # unchanged. + "hidden": yaml_only, "help_link": docs.url, "translation_key": None, "translation_params": None, diff --git a/tests/test_sync_components_ui_hints.py b/tests/test_sync_components_ui_hints.py new file mode 100644 index 00000000..94d6c411 --- /dev/null +++ b/tests/test_sync_components_ui_hints.py @@ -0,0 +1,272 @@ +"""Unit tests for the schema-author UI-hint passthrough + cascade. + +Pairs with esphome/esphome#16267, which adds a ``visibility`` +kwarg (``cv.Visibility`` ``StrEnum``) to ``cv.Optional`` / +``cv.Required`` so the field author can mark a config_var's UI +treatment in the schema itself. The dumper emits the str form +(``"advanced"`` / ``"yaml_only"``) onto the per-field dict in +the schema bundle. + +This module pins three behavioural invariants: + +1. The schema's ``visibility`` value drives the catalog entry's + ``advanced`` / ``hidden`` flags directly when present; the + name-based heuristic is the fallback. +2. The cascade rule: a stricter parent forces its descendants + at-least as strict (``YAML_ONLY`` > ``ADVANCED`` > unset). The + schema marker is per-field as the author wrote it; the + catalog generator computes the effective value when it walks + the tree. +3. ``yaml_only`` maps cleanly onto the catalog's existing + ``hidden`` field — no rename, no consumer-facing surface change. +""" + +from __future__ import annotations + +from pathlib import Path + +from script.sync_components import ( # type: ignore[import-not-found] + _apply_visibility_cascade, + _convert_field, +) + +# ``_convert_field`` only touches ``schema_dir`` for nested +# ``extends`` resolution, which the leaf-field cases below don't +# trigger. ``Path("/")`` is fine for these tests. +_SCHEMA_DIR = Path("/") + + +def _leaf(**raw: object) -> dict: + """Build a minimal raw schema dict for a string-typed Optional leaf.""" + return { + "key": "Optional", + "type": "string", + **raw, + } + + +def test_schema_visibility_advanced_wins_over_heuristic_false() -> None: + """``visibility: "advanced"`` flips a heuristic-False field to advanced. + + ``name`` is in ``_IMPORTANT_KEYS`` so the heuristic returns + False; the schema flag flips it to True. + """ + entry = _convert_field("name", _leaf(visibility="advanced"), _SCHEMA_DIR) + assert entry is not None + assert entry["advanced"] is True + assert entry["hidden"] is False + + +def test_schema_visibility_yaml_only_sets_hidden() -> None: + """``visibility: "yaml_only"`` sets the catalog's ``hidden`` flag.""" + entry = _convert_field("foo", _leaf(visibility="yaml_only"), _SCHEMA_DIR) + assert entry is not None + assert entry["hidden"] is True + # ``advanced`` from the heuristic isn't relevant when ``hidden`` + # is set — the frontend skips the entry entirely. Don't pin it + # here; let the heuristic decide. + + +def test_no_visibility_falls_back_to_heuristic() -> None: + """Without the schema flag, the heuristic decides ``advanced``. + + ``setup_priority`` has the heuristic-derived ``True``; + ``name`` has ``False``. + """ + advanced_entry = _convert_field("setup_priority", _leaf(), _SCHEMA_DIR) + assert advanced_entry is not None + assert advanced_entry["advanced"] is True + assert advanced_entry["hidden"] is False + + name_entry = _convert_field("name", _leaf(), _SCHEMA_DIR) + assert name_entry is not None + assert name_entry["advanced"] is False + assert name_entry["hidden"] is False + + +def test_unrecognised_visibility_string_falls_back_to_heuristic() -> None: + """An unknown ``visibility`` value falls back to the heuristic. + + Future-compatibility: if upstream adds a third visibility level + (e.g. ``"deprecated"``) the field doesn't silently disappear + from the form. Neither ``advanced`` nor ``hidden`` flips on + speculatively; the heuristic still applies. + """ + entry = _convert_field("name", _leaf(visibility="someday-new-value"), _SCHEMA_DIR) + assert entry is not None + assert entry["advanced"] is False + assert entry["hidden"] is False + + +# --------------------------------------------------------------------------- +# Cascade rule +# --------------------------------------------------------------------------- + + +def _entry( + key: str, *, advanced: bool = False, hidden: bool = False, inner: list | None = None +) -> dict: + """Minimal catalog-entry shape for the cascade pass.""" + e: dict = {"key": key, "advanced": advanced, "hidden": hidden} + if inner is not None: + e["config_entries"] = inner + return e + + +def test_cascade_parent_advanced_pushes_to_unset_children() -> None: + """An ``ADVANCED`` parent makes every un-marked descendant ``ADVANCED``. + + Without the cascade an ``advanced`` parent would render under + a disclosure but its inner fields would surface on the main + form — leaky disclosure UX. + """ + entries = [ + _entry( + "parent", + advanced=True, + inner=[ + _entry("child_a"), + _entry("child_b"), + ], + ), + ] + _apply_visibility_cascade(entries, parent_advanced=False, parent_yaml_only=False) + inner = entries[0]["config_entries"] + assert entries[0]["advanced"] is True + assert inner[0]["advanced"] is True + assert inner[1]["advanced"] is True + # No ``YAML_ONLY`` anywhere → ``hidden`` stays False. + assert all(e["hidden"] is False for e in [entries[0], *inner]) + + +def test_cascade_yaml_only_parent_hides_all_descendants() -> None: + """A ``YAML_ONLY`` parent hides every descendant. + + A block the user shouldn't edit through a UI must take its + children with it — otherwise the form renders an unrooted + control with no surrounding context to interpret it. + """ + entries = [ + _entry( + "parent", + hidden=True, + inner=[ + _entry("child_a"), + _entry("child_b", advanced=True), + ], + ), + ] + _apply_visibility_cascade(entries, parent_advanced=False, parent_yaml_only=False) + inner = entries[0]["config_entries"] + # ``hidden`` cascades into both children regardless of what + # they originally said. + assert entries[0]["hidden"] is True + assert inner[0]["hidden"] is True + assert inner[1]["hidden"] is True + # And ``advanced`` is also forced True under a hidden parent — + # ``YAML_ONLY`` is strictly stronger than ``ADVANCED``. + assert all(e["advanced"] is True for e in [entries[0], *inner]) + + +def test_cascade_inner_yaml_only_under_advanced_parent() -> None: + """A child marked ``YAML_ONLY`` keeps its hidden status under an ``ADVANCED`` parent. + + The cascade is monotonically-non-decreasing in strictness: + children can be stricter than their parent (the child's own + ``hidden=True`` survives), but never less strict (the parent's + ``advanced=True`` is pushed onto sibling children). + """ + entries = [ + _entry( + "parent", + advanced=True, + inner=[ + _entry("child_unmarked"), + _entry("child_yaml_only", hidden=True), + ], + ), + ] + _apply_visibility_cascade(entries, parent_advanced=False, parent_yaml_only=False) + inner = entries[0]["config_entries"] + # Parent stays ``advanced``. + assert entries[0]["advanced"] is True + assert entries[0]["hidden"] is False + # Sibling without its own setting picks up the parent's + # ``advanced``. + assert inner[0]["advanced"] is True + assert inner[0]["hidden"] is False + # ``YAML_ONLY`` child stays hidden — and its ``advanced`` is + # also True (every yaml-only field is also advanced by the + # strictness ordering). + assert inner[1]["advanced"] is True + assert inner[1]["hidden"] is True + + +def test_cascade_recurses_through_multiple_levels() -> None: + """The cascade walks all the way down nested groups. + + Three-level structure: parent (advanced) → middle (unset) → + leaf (unset). The leaf must end up advanced because its + grandparent is. + """ + entries = [ + _entry( + "grandparent", + advanced=True, + inner=[ + _entry( + "parent", + inner=[ + _entry("leaf"), + ], + ), + ], + ), + ] + _apply_visibility_cascade(entries, parent_advanced=False, parent_yaml_only=False) + parent = entries[0]["config_entries"][0] + leaf = parent["config_entries"][0] + assert entries[0]["advanced"] is True + assert parent["advanced"] is True + assert leaf["advanced"] is True + + +def test_cascade_no_op_when_no_strict_parent() -> None: + """A tree with no strict markers stays as the heuristic decided. + + The cascade only ever flips flags from False to True; it + never flips True to False. A baseline tree should round-trip + through the cascade unchanged. + """ + entries = [ + _entry("a", advanced=False), + _entry("b", advanced=True), + _entry( + "c", + advanced=False, + inner=[_entry("c1", advanced=True)], + ), + ] + _apply_visibility_cascade(entries, parent_advanced=False, parent_yaml_only=False) + assert entries[0]["advanced"] is False + assert entries[1]["advanced"] is True + assert entries[2]["advanced"] is False + assert entries[2]["config_entries"][0]["advanced"] is True + assert all(e["hidden"] is False for e in entries) + + +def test_cascade_non_list_inner_is_skipped() -> None: + """``config_entries: None`` is tolerated without traversal. + + Some catalog entry shapes carry an explicit ``None`` for the + nested list (NESTED-but-empty groups); the cascade must skip + those rather than crashing on attribute access. + """ + entries = [ + _entry("a", advanced=True), + ] + # Add the ``config_entries`` key as None — the catalog uses + # this for nested-but-empty groups. + entries[0]["config_entries"] = None + _apply_visibility_cascade(entries, parent_advanced=False, parent_yaml_only=False) + assert entries[0]["advanced"] is True