Cache editor/validate_yaml results per session content hash (esphome#892)

Author: bdraco

esphome_device_builder/controllers/editor.py

@@ -9,10 +9,13 @@
 
 import asyncio
 import logging
+import time
 from dataclasses import dataclass, field
 from pathlib import Path
 from typing import TYPE_CHECKING, Any
 
+from fnv_hash_fast import fnv1a_32
+
 from ..helpers.api import api_command
 from ..helpers.json import JSONDecodeError, dumps, loads
 from ..helpers.process import kill_quietly
@@ -25,15 +28,38 @@
 _LOGGER = logging.getLogger(__name__)
 _STARTUP_TIMEOUT = 15.0
 _VALIDATE_TIMEOUT = 30.0
+# Linter and save both call ``validate_yaml`` on identical
+# content (typing-stops → linter at 600 ms → user clicks save).
+# Cache covers that hand-off; longer would risk staleness for
+# ``!include`` / ``external_components`` files mutated outside
+# the editor. ``fnv1a_32`` keys the cache (non-cryptographic;
+# collision risk negligible for the ≤dozens of buffers an editor
+# session sees inside one TTL window).
+_VALIDATE_CACHE_TTL = 60.0
+
+
+@dataclass
+class _CachedValidation:
+    """Snapshot of a validate_yaml result, with the inputs needed to reuse it."""
+
+    content_hash: int
+    result: dict[str, Any]
+    at: float
+
+    def is_fresh_for(self, content_hash: int) -> bool:
+        return (
+            self.content_hash == content_hash and time.monotonic() - self.at < _VALIDATE_CACHE_TTL
+        )
 
 
 @dataclass
 class _EditorSession:
-    """Per-configuration validator state: one warm subprocess plus a serialization lock."""
+    """Per-configuration validator state: warm subprocess, lock, and result cache."""
 
     configuration: str
     proc: asyncio.subprocess.Process | None = None
     lock: asyncio.Lock = field(default_factory=asyncio.Lock)
+    cached: _CachedValidation | None = None
 
 
 class EditorController:
@@ -180,20 +206,39 @@ async def validate_yaml(
         the same shape upstream ``vscode.py`` produces. Each error has a
         ``message`` and (for validation errors) a ``range`` with
         ``{start_line, start_col, end_line, end_col}`` (0-indexed).
+
+        Results are cached per session by content hash for
+        ``_VALIDATE_CACHE_TTL`` seconds; the linter and the
+        save-time re-validate hit the same content back-to-back.
         """
         session = self._sessions.setdefault(
             configuration, _EditorSession(configuration=configuration)
         )
+        content_hash = fnv1a_32(content.encode("utf-8"))
+        # Fast path: avoid the lock when the previous result is
+        # still fresh for the same content.
+        cached = session.cached
+        if cached is not None and cached.is_fresh_for(content_hash):
+            return cached.result
         async with session.lock:
+            # Re-check under the lock so a concurrent linter+save
+            # for the same content only spawns one subprocess pass.
+            cached = session.cached
+            if cached is not None and cached.is_fresh_for(content_hash):
+                return cached.result
             try:
-                return await asyncio.wait_for(
+                result = await asyncio.wait_for(
                     self._validate_locked(session, configuration, content),
                     timeout=_VALIDATE_TIMEOUT,
                 )
             except (TimeoutError, RuntimeError, BrokenPipeError):
                 # Subprocess wedged or died — kill it so the next call respawns.
                 await self._terminate_subprocess(session)
                 raise
+            session.cached = _CachedValidation(
+                content_hash=content_hash, result=result, at=time.monotonic()
+            )
+            return result
 
     async def _validate_locked(
         self, session: _EditorSession, configuration: str, content: str

pyproject.toml

@@ -22,6 +22,9 @@ dependencies = [
   "aiohttp-asyncmdnsresolver>=0.1.1",
   "colorlog>=6.8.0",
   "cryptography>=42.0.2",
+  # Cheap non-cryptographic hash for the editor's
+  # validate-result cache (content → cached subprocess result).
+  "fnv-hash-fast>=1.0.0",
   "ifaddr>=0.2.0",
   "mashumaro>=3.13",
   "orjson>=3.9.0",

tests/test_editor_controller.py

@@ -36,6 +36,7 @@
 
 import pytest
 
+from esphome_device_builder.controllers import editor as editor_module
 from esphome_device_builder.controllers.editor import (
     EditorController,
     _EditorSession,
@@ -799,3 +800,141 @@ async def _raise_runtime(*_args: Any, **_kwargs: Any) -> dict:
         await controller.validate_yaml(configuration="kitchen.yaml", content="")
 
     terminated.assert_awaited_once()
+
+
+# ---------------------------------------------------------------------------
+# validate_yaml — content-hash cache
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_validate_yaml_caches_result_for_repeated_content(tmp_path: Path) -> None:
+    """A second call with identical content returns the cached result.
+
+    The linter and the save-time re-validate hit the same buffer
+    back-to-back; the cache turns the second one into a no-op
+    instead of paying for another ``esphome vscode`` round-trip.
+    """
+    controller = _make_controller(tmp_path)
+    calls: list[str] = []
+
+    async def _record(session: Any, configuration: str, content: str) -> dict:
+        calls.append(content)
+        return {"yaml_errors": [], "validation_errors": []}
+
+    controller._validate_locked = _record  # type: ignore[method-assign]
+
+    first = await controller.validate_yaml(
+        configuration="kitchen.yaml", content="esphome:\n  name: kitchen\n"
+    )
+    second = await controller.validate_yaml(
+        configuration="kitchen.yaml", content="esphome:\n  name: kitchen\n"
+    )
+
+    assert first == second == {"yaml_errors": [], "validation_errors": []}
+    assert len(calls) == 1
+
+
+@pytest.mark.asyncio
+async def test_validate_yaml_cache_misses_on_different_content(tmp_path: Path) -> None:
+    """Distinct content bypasses the cache and re-validates."""
+    controller = _make_controller(tmp_path)
+    calls: list[str] = []
+
+    async def _record(session: Any, configuration: str, content: str) -> dict:
+        calls.append(content)
+        return {"yaml_errors": [], "validation_errors": []}
+
+    controller._validate_locked = _record  # type: ignore[method-assign]
+
+    await controller.validate_yaml(
+        configuration="kitchen.yaml", content="esphome:\n  name: kitchen\n"
+    )
+    await controller.validate_yaml(
+        configuration="kitchen.yaml", content="esphome:\n  name: kitchen-2\n"
+    )
+
+    assert calls == ["esphome:\n  name: kitchen\n", "esphome:\n  name: kitchen-2\n"]
+
+
+@pytest.mark.asyncio
+async def test_validate_yaml_cache_expires_after_ttl(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """The cache TTL bounds staleness of ``!include`` / external-component reads."""
+    controller = _make_controller(tmp_path)
+    calls: list[str] = []
+
+    async def _record(session: Any, configuration: str, content: str) -> dict:
+        calls.append(content)
+        return {"yaml_errors": [], "validation_errors": []}
+
+    controller._validate_locked = _record  # type: ignore[method-assign]
+    fake_now = {"t": 1000.0}
+    monkeypatch.setattr(editor_module.time, "monotonic", lambda: fake_now["t"])
+
+    await controller.validate_yaml(
+        configuration="kitchen.yaml", content="esphome:\n  name: kitchen\n"
+    )
+    fake_now["t"] += editor_module._VALIDATE_CACHE_TTL + 1
+    await controller.validate_yaml(
+        configuration="kitchen.yaml", content="esphome:\n  name: kitchen\n"
+    )
+
+    assert len(calls) == 2
+
+
+@pytest.mark.asyncio
+async def test_validate_yaml_cache_is_per_configuration(tmp_path: Path) -> None:
+    """Same content under a different configuration doesn't share the cache."""
+    controller = _make_controller(tmp_path)
+    calls: list[tuple[str, str]] = []
+
+    async def _record(session: Any, configuration: str, content: str) -> dict:
+        calls.append((configuration, content))
+        return {"yaml_errors": [], "validation_errors": []}
+
+    controller._validate_locked = _record  # type: ignore[method-assign]
+
+    await controller.validate_yaml(configuration="kitchen.yaml", content="esphome:\n  name: same\n")
+    await controller.validate_yaml(configuration="bedroom.yaml", content="esphome:\n  name: same\n")
+
+    assert len(calls) == 2
+    assert {c[0] for c in calls} == {"kitchen.yaml", "bedroom.yaml"}
+
+
+@pytest.mark.asyncio
+async def test_validate_yaml_inner_lock_recheck_coalesces_concurrent_calls(
+    tmp_path: Path,
+) -> None:
+    """Two concurrent calls for identical content share a single subprocess pass."""
+    controller = _make_controller(tmp_path)
+    started = asyncio.Event()
+    release = asyncio.Event()
+    calls = 0
+
+    async def _slow_validate(_session: Any, _configuration: str, _content: str) -> dict:
+        nonlocal calls
+        calls += 1
+        started.set()
+        await release.wait()
+        return {"yaml_errors": [], "validation_errors": []}
+
+    controller._validate_locked = _slow_validate  # type: ignore[method-assign]
+
+    first = asyncio.create_task(
+        controller.validate_yaml(configuration="kitchen.yaml", content="esphome:\n")
+    )
+    # Wait for the first call to enter ``_validate_locked`` so the
+    # second call lands in the fast-path miss + lock-wait window.
+    await started.wait()
+    second = asyncio.create_task(
+        controller.validate_yaml(configuration="kitchen.yaml", content="esphome:\n")
+    )
+    # Hand the second task a tick to reach the lock acquire.
+    await asyncio.sleep(0)
+    release.set()
+    results = await asyncio.gather(first, second)
+
+    assert calls == 1
+    assert results[0] == results[1] == {"yaml_errors": [], "validation_errors": []}