From 112d8b26ee817edf243967412cf869e61763da6d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 4 Aug 2026 21:24:38 -0500 Subject: [PATCH] [core] Skip source range tracking when loading the validated config cache (#18046) --- esphome/compiled_config.py | 9 +- esphome/yaml_util.py | 96 ++++++++++++++---- tests/unit_tests/test_compiled_config.py | 5 + tests/unit_tests/test_yaml_util.py | 119 +++++++++++++++++++++++ 4 files changed, 210 insertions(+), 19 deletions(-) diff --git a/esphome/compiled_config.py b/esphome/compiled_config.py index f4fd205285..1bcd567b84 100644 --- a/esphome/compiled_config.py +++ b/esphome/compiled_config.py @@ -61,7 +61,14 @@ def load_compiled_config(conf_path: Path) -> ConfigType | None: from esphome import yaml_util try: - config = yaml_util.load_yaml(cache_path, clear_secrets=False) + # Fast path never validates or generates code - no source ranges + # needed (see load_yaml). Callers must not feed this config into + # read_config/write_cpp: the esp_range consumers in config.py and + # cpp_generator.py are isinstance-guarded and would degrade + # silently (wrong error/lambda locations) instead of raising. + config = yaml_util.load_yaml( + cache_path, clear_secrets=False, track_document_range=False + ) except Exception: # noqa: BLE001 # pylint: disable=broad-except return None diff --git a/esphome/yaml_util.py b/esphome/yaml_util.py index 03d1e81073..ca993b2ca5 100644 --- a/esphome/yaml_util.py +++ b/esphome/yaml_util.py @@ -546,6 +546,13 @@ def _add_data_ref(fn): # Let generator finish for _ in generator: pass + # Fast mode keeps this per-node attribute check instead of a second + # constructor table: measured, fast mode already parses within ~8% + # of a raw CSafeLoader, so a parallel table isn't worth the + # duplication (and undecorated constructors return generators with + # different resolution ordering). + if not loader.track_document_range: + return res res = make_data_base(res) if isinstance(res, ESPHomeDataBase): res.from_node(node) @@ -585,14 +592,19 @@ def _resolve_merge_include(value: Any, node: yaml.Node, value_node: yaml.Node) - class ESPHomeLoaderMixin: - """Loader class that keeps track of line numbers.""" + """Loader that tracks line numbers unless track_document_range is off.""" def __init__( - self, name: Path, yaml_loader: Callable[[Path], dict[str, Any]] + self, + name: Path, + yaml_loader: Callable[[Path], dict[str, Any]], + *, + track_document_range: bool, ) -> None: - """Initialize the loader.""" + """Initialize the loader. See load_yaml for track_document_range.""" self.name = name self.yaml_loader = yaml_loader + self.track_document_range = track_document_range @_add_data_ref def construct_yaml_int(self, node): @@ -655,8 +667,10 @@ class ESPHomeLoaderMixin: f'Invalid key "{key}" (not hashable)', key_node.start_mark ) from None - key = make_data_base(str(key)) - key.from_node(key_node) + key = str(key) + if self.track_document_range: + key = make_data_base(key) + key.from_node(key_node) # Check if it is a duplicate key if key in seen_keys: @@ -852,29 +866,37 @@ class ESPHomeLoaderMixin: class ESPHomeLoader(ESPHomeLoaderMixin, FastestAvailableSafeLoader): - """Loader class that keeps track of line numbers.""" + """C-accelerated loader; see ESPHomeLoaderMixin.""" def __init__( self, stream: TextIOBase | BytesIO, name: Path, yaml_loader: Callable[[Path], dict[str, Any]], + *, + track_document_range: bool, ) -> None: FastestAvailableSafeLoader.__init__(self, stream) - ESPHomeLoaderMixin.__init__(self, name, yaml_loader) + ESPHomeLoaderMixin.__init__( + self, name, yaml_loader, track_document_range=track_document_range + ) class ESPHomePurePythonLoader(ESPHomeLoaderMixin, PurePythonLoader): - """Loader class that keeps track of line numbers.""" + """Pure-Python loader with readable errors; see ESPHomeLoaderMixin.""" def __init__( self, stream: TextIOBase | BytesIO, name: Path, yaml_loader: Callable[[Path], dict[str, Any]], + *, + track_document_range: bool, ) -> None: PurePythonLoader.__init__(self, stream) - ESPHomeLoaderMixin.__init__(self, name, yaml_loader) + ESPHomeLoaderMixin.__init__( + self, name, yaml_loader, track_document_range=track_document_range + ) for _loader in (ESPHomeLoader, ESPHomePurePythonLoader): @@ -902,20 +924,31 @@ for _loader in (ESPHomeLoader, ESPHomePurePythonLoader): _loader.add_constructor("!remove", _loader.construct_remove) -def load_yaml(fname: Path, clear_secrets: bool = True) -> Any: +def load_yaml( + fname: Path, clear_secrets: bool = True, *, track_document_range: bool = True +) -> Any: + """Load a YAML file. + + track_document_range=False skips wrapping every node in an + ESPHomeDataBase subclass carrying its source range. That metadata + serves validation error messages and lambda source locations in + generated code; callers that neither validate nor generate code (the + upload/logs fast path re-reading the validated config cache) can skip + it, roughly halving parse time. + """ if clear_secrets: _SECRET_VALUES.clear() _SECRET_CACHE.clear() - return _load_yaml_internal(fname) + return _load_yaml_internal(fname, track_document_range=track_document_range) -def _load_yaml_internal(fname: Path) -> Any: +def _load_yaml_internal(fname: Path, *, track_document_range: bool = True) -> Any: """Load a YAML file.""" for listener in _load_listeners: listener(fname) try: with fname.open(encoding="utf-8") as f_handle: - res = parse_yaml(fname, f_handle) + res = parse_yaml(fname, f_handle, track_document_range=track_document_range) except (UnicodeDecodeError, OSError) as err: raise EsphomeError(f"Error reading file {fname}: {err}") from err # Top-level !include returns a deferred IncludeFile; resolve it so @@ -925,13 +958,32 @@ def _load_yaml_internal(fname: Path) -> Any: return res -def parse_yaml(file_name: Path, file_handle: TextIOWrapper, yaml_loader=None) -> Any: +_FAST_YAML_LOADER = functools.partial(_load_yaml_internal, track_document_range=False) + + +def parse_yaml( + file_name: Path, + file_handle: TextIOWrapper, + yaml_loader=None, + *, + track_document_range: bool = True, +) -> Any: """Parse a YAML file.""" if yaml_loader is None: - yaml_loader = _load_yaml_internal + # Nested loads (!include, !secret, !include_dir_*) inherit the + # same tracking mode. + yaml_loader = _load_yaml_internal if track_document_range else _FAST_YAML_LOADER + elif not track_document_range: + # A caller-supplied loader would silently revert nested loads to + # tracked mode; reject the combination instead of half-applying it. + raise ValueError("track_document_range=False requires the default yaml_loader") try: return _load_yaml_internal_with_type( - ESPHomeLoader, file_name, file_handle, yaml_loader + ESPHomeLoader, + file_name, + file_handle, + yaml_loader, + track_document_range=track_document_range, ) except EsphomeError: # Loading failed, so we now load with the Python loader which has more @@ -939,7 +991,11 @@ def parse_yaml(file_name: Path, file_handle: TextIOWrapper, yaml_loader=None) -> # Rewind the stream so we can try again file_handle.seek(0, 0) return _load_yaml_internal_with_type( - ESPHomePurePythonLoader, file_name, file_handle, yaml_loader + ESPHomePurePythonLoader, + file_name, + file_handle, + yaml_loader, + track_document_range=track_document_range, ) @@ -948,6 +1004,8 @@ def _load_yaml_internal_with_type( fname: Path, content: TextIOWrapper, yaml_loader: Callable[[Path], dict[str, Any]], + *, + track_document_range: bool, ) -> Any: """Load a YAML file. @@ -958,7 +1016,9 @@ def _load_yaml_internal_with_type( configuration. Frontmatter is ignored by config validation and code generation. """ - loader = loader_type(content, fname, yaml_loader) + loader = loader_type( + content, fname, yaml_loader, track_document_range=track_document_range + ) try: documents: list[Any] = [] while loader.check_data(): diff --git a/tests/unit_tests/test_compiled_config.py b/tests/unit_tests/test_compiled_config.py index 4219424aa1..b852d2d596 100644 --- a/tests/unit_tests/test_compiled_config.py +++ b/tests/unit_tests/test_compiled_config.py @@ -26,6 +26,7 @@ from esphome.const import ( KEY_VARIANT, ) from esphome.core import CORE +from esphome.yaml_util import ESPHomeDataBase _VALIDATED_CONFIG_YAML = """\ esphome: @@ -125,6 +126,10 @@ def test_load_compiled_config_happy_path(fresh_cache_files: Path) -> None: assert config[CONF_API]["encryption"]["key"] == "6dGhpcyBpcyBhIHRlc3Q=" assert config["ota"][0]["password"] == "secret" + # The fast path loads without per-node source ranges (the full + # contract lives in test_yaml_util; this checks the flag is wired up). + assert not isinstance(config[CONF_ESPHOME][CONF_NAME], ESPHomeDataBase) + # apply_to_core populated exactly what upload/logs read off CORE. assert CORE.name == "lite_test" assert CORE.build_path == Path("/build/lite_test") diff --git a/tests/unit_tests/test_yaml_util.py b/tests/unit_tests/test_yaml_util.py index 1bb70864a3..e0a81652e3 100644 --- a/tests/unit_tests/test_yaml_util.py +++ b/tests/unit_tests/test_yaml_util.py @@ -1780,3 +1780,122 @@ def test_merge_include_no_overlap_records_nothing(tmp_path: Path) -> None: assert result["api"] == {"reboot_timeout": "5min"} assert result["logger"] == {"level": "DEBUG"} assert yaml_util.take_dropped_merge_keys() == [] + + +# --------------------------------------------------------------------------- +# track_document_range=False (validated-config-cache fast path) +# --------------------------------------------------------------------------- + +FAST_MODE_MAIN_YAML = """\ +defaults: &defaults + port: 6053 + reboot_timeout: 15min + +esphome: + name: !secret devname + +api: + <<: *defaults + port: 6054 + +number_value: 42 +float_value: 3.5 +lambda_value: !lambda 'return x * 2;' +extend_value: !extend some_id +remove_value: !remove some_id +literal_value: !literal keep_me_verbatim +included: !include included.yaml +""" + + +@pytest.fixture +def fast_mode_config_dir(tmp_path: Path) -> Path: + _write(tmp_path, "main.yaml", FAST_MODE_MAIN_YAML) + _write(tmp_path, "included.yaml", "inner_key: inner_value\ninner_num: 7\n") + _write(tmp_path, "secrets.yaml", "devname: livingroom\n") + return tmp_path + + +def _resolve_includes(config: dict) -> dict: + return { + key: value.load() if isinstance(value, yaml_util.IncludeFile) else value + for key, value in config.items() + } + + +def test_load_yaml_fast_mode_matches_default(fast_mode_config_dir: Path) -> None: + """Both modes produce equal values; only the metadata wrapping differs.""" + yaml_file = fast_mode_config_dir / "main.yaml" + + normal = _resolve_includes(yaml_util.load_yaml(yaml_file)) + fast = _resolve_includes(yaml_util.load_yaml(yaml_file, track_document_range=False)) + + # Lambda has no __eq__; compare it by value and the rest structurally. + fast_lambda = fast.pop("lambda_value") + normal_lambda = normal.pop("lambda_value") + assert fast == normal + assert isinstance(fast_lambda, core.Lambda) + assert fast_lambda.value == normal_lambda.value == "return x * 2;" + assert fast["esphome"]["name"] == "livingroom" + assert fast["api"]["port"] == 6054 + assert fast["api"]["reboot_timeout"] == "15min" + assert fast["extend_value"] == Extend("some_id") + assert fast["remove_value"] == Remove("some_id") + # !literal wraps via make_literal, independent of range tracking. + assert isinstance(fast["literal_value"], ESPLiteralValue) + assert fast["literal_value"] == "keep_me_verbatim" + + # Fast mode returns plain values; default mode keeps the range metadata. + assert not isinstance(fast["number_value"], ESPHomeDataBase) + assert not isinstance(fast["float_value"], ESPHomeDataBase) + assert all(type(key) is str for key in fast) + assert isinstance(normal["number_value"], ESPHomeDataBase) + assert normal["number_value"].esp_range is not None + assert all(isinstance(key, ESPHomeDataBase) for key in normal) + + # Nested includes inherit fast mode through the recursive loader. + included = fast["included"] + assert not isinstance(included["inner_num"], ESPHomeDataBase) + assert all(type(key) is str for key in included) + + +def test_load_yaml_fast_mode_survives_pure_python_fallback( + fast_mode_config_dir: Path, +) -> None: + """The ESPHomePurePythonLoader retry must honour fast mode too.""" + yaml_file = fast_mode_config_dir / "main.yaml" + + class _AlwaysFailingLoader(yaml_util.ESPHomeLoader): + def __init__(self, *args, **kwargs) -> None: + raise EsphomeError("forced fallback to the pure-Python loader") + + with patch.object(yaml_util, "ESPHomeLoader", _AlwaysFailingLoader): + fast = yaml_util.load_yaml(yaml_file, track_document_range=False) + + assert not isinstance(fast["number_value"], ESPHomeDataBase) + assert all(type(key) is str for key in fast) + + +def test_load_yaml_fast_mode_rejects_custom_loader() -> None: + """A caller-supplied yaml_loader cannot combine with fast mode.""" + with pytest.raises(ValueError, match="default yaml_loader"): + yaml_util.parse_yaml( + Path("x.yaml"), + io.StringIO("a: 1"), + lambda f: {}, + track_document_range=False, + ) + + +def test_load_yaml_fast_mode_records_dropped_merge_keys( + fast_mode_config_dir: Path, +) -> None: + """The duplicate-merge-key bookkeeping must not crash on plain str keys. + + With plain keys there is no esp_range, so the recorded location falls + back to the parent file name. + """ + yaml_file = fast_mode_config_dir / "main.yaml" + + yaml_util.load_yaml(yaml_file, track_document_range=False) + assert yaml_util.take_dropped_merge_keys() == [("port", str(yaml_file))]