Cache editor/validate_yaml results per session content hash#892
Conversation
The editor's CodeMirror linter and ``_saveYaml`` both fire ``editor/validate_yaml`` against the same buffer back-to-back (linter at typing-stop+600ms, save on click). On large configs with ``external_components`` / ``packages:`` / ``!include`` the ``esphome vscode`` subprocess takes several seconds per pass — visible as 7-9s click-to-popup latency on a Celeron NUC. Add a per-session content-hash cache (60s TTL, ``fnv1a_32`` hash since this is a cache key, not a security boundary). The save-time re-validate hits the linter's just-stored result instead of spinning up another subprocess pass. Different content or expiry invalidates and re-validates as before. The 60s TTL bounds staleness for ``!include`` / external-component files mutated outside the editor; long enough to cover any realistic linter→save hand-off, short enough that an editor- external change recovers on the next debounce tick.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #892 +/- ##
=======================================
Coverage 99.31% 99.32%
=======================================
Files 190 190
Lines 13823 13843 +20
=======================================
+ Hits 13729 13749 +20
Misses 94 94
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves YAML editor responsiveness by caching editor/validate_yaml results per _EditorSession, keyed by a content hash with a 60s TTL, so save-time validation can reuse the linter’s just-computed result instead of re-running the expensive esphome vscode subprocess.
Changes:
- Add a per-session validate-result cache (content-hash + timestamp) with a 60s TTL in
EditorController.validate_yaml. - Add
fnv-hash-fastdependency to compute a cheap non-cryptographic content hash for cache keys. - Add tests covering cache hit/miss behavior, TTL expiry, and per-configuration isolation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
esphome_device_builder/controllers/editor.py |
Implements validate-result caching keyed by content hash with TTL. |
pyproject.toml |
Adds fnv-hash-fast dependency for hashing editor content. |
tests/test_editor_controller.py |
Adds unit tests validating cache behavior (repeat content, content change, TTL, config isolation). |
…check * The three ``cached_hash`` / ``cached_result`` / ``cached_at`` fields on ``_EditorSession`` move together — group them into a single ``_CachedValidation`` dataclass with an ``is_fresh_for(content_hash)`` predicate. Fewer load-bearing invariants spread across the session class. * Codecov flagged line 226 (the under-lock cache-hit branch) as uncovered — every test missed because the fast-path check catches them first. New test ``test_validate_yaml_inner_lock_recheck_coalesces_concurrent_calls`` fires two concurrent validate calls against the same content: first wins the lock and runs the subprocess, second hits the inner re-check and returns the just-populated cache entry.
|
@bluetoothbot review |
PR Review — Cache editor/validate_yaml results per session content hashClean, well-scoped change. The atomic-ref-swap pattern in 🟢 Suggestions1. 32-bit hash collision is astronomically unlikely but theoretically possible (`esphome_device_builder/controllers/editor.py`, L34-50)
2. Consider invalidating the cache when the subprocess is terminated (`esphome_device_builder/controllers/editor.py`, L225-240)On Checklist
SummaryClean, well-scoped change. The atomic-ref-swap pattern in |
bluetoothbot's PR #892 review flagged two non-blocking polish items: * ``fnv1a_32`` is non-cryptographic with a ~4-billion-value range. Realistic collision exposure inside one 60s TTL window is the few dozen buffer versions an editor session produces (birthday-paradox risk ≈ 1.7e-8). ``fnv-hash-fast`` only ships the 32-bit variant, so a switch would mean another dependency for a non-issue. Acknowledge the trade in the TTL comment so the next reader doesn't re-litigate the choice. * ``_CachedValidation.result: dict`` → ``dict[str, Any]`` for consistency with the rest of the module. The reviewer's second suggestion (invalidate cache on subprocess termination) doesn't materialize with a content-hash key — byte-identical content gives a byte-identical validate result; the only escape hatch is hash collision (covered above) or an external ``!include`` mutation (already bounded by the 60s TTL).
|
Both items addressed in 9c4a7a4: Suggestion #1 — hash class: Suggestion #2 — invalidate on subprocess teardown: The concern only materializes if cache content X matches a previously-validated content X but now triggers a wedge. Content X is keyed by content hash, so byte-identical content gives byte-identical validate behavior; the only escape hatch is a hash collision (covered by #1) or external mutation of |
Self-audit caught two drifts: * ``validate_yaml`` docstring used an em-dash inside the cache paragraph; CLAUDE.md / memory bans em-dashes. * ``test_validate_yaml_inner_lock_recheck_coalesces_concurrent_calls`` had a 6-line docstring re-telling the production-side story. Test docstrings should name what the test pins in one sentence.
|
definitely better to work with on heavy configs. still a lot of cpu churn for revalidate on heavy configs, but any improvement there is win. Will need to dig into lvgl more as esphome/esphome#16478 is just the tip of it and we have 0 benchmarks for it. |
What does this implement/fix?
The editor's CodeMirror linter and
_saveYamlboth calleditor/validate_yamlagainst the same buffer back-to-back: thelinter at typing-stop+600ms, the save on click. On large configs
with
external_components/packages:/!includetheesphome vscodesubprocess takes several seconds per pass.Visible on user reports as 7-9s click-to-popup latency on a
Celeron NUC (MasterOfNone, Slack 2026-05-16).
This PR caches
editor/validate_yamlresults on_EditorSessionkeyed by content hash with a 60s TTL. The save-time re-validate
hits the linter's just-stored result instead of spinning up
another subprocess pass. Different content or expiry invalidates
and re-validates as before. Uses
fnv1a_32fromfnv-hash-fastsince this is a cache key, not a securityboundary.
The 60s TTL bounds staleness for
!include/ external-componentfiles mutated outside the editor; long enough to cover any
realistic linter-to-save hand-off, short enough that an external
change recovers on the next debounce tick.
Related issue or feature (if applicable):
Types of changes
bugfixnew-featureenhancementbreaking-changerefactordocsmaintenancecidependenciesFrontend coordination
Backend cache makes the save-time
validate_yamlcheap; thefrontend companion skips the WS round-trip entirely when the
linter's last result still matches the buffer. The two compose;
either alone is an improvement.
Checklist
ruff,codespell, yaml/json/python checks).tests/where applicable.components.jsonhas not been hand-edited (regenerate viascript/sync_components.pyif a sync is needed).docs/ARCHITECTURE.mdand/ordocs/API.md.