Skip to content

Treat dot-prefixed top-level keys as block boundaries in the line scanners - #2652

Open
bdraco wants to merge 3 commits into
mainfrom
dot-key-block-bounds
Open

Treat dot-prefixed top-level keys as block boundaries in the line scanners#2652
bdraco wants to merge 3 commits into
mainfrom
dot-key-block-bounds

Conversation

@bdraco

@bdraco bdraco commented Sep 2, 2026

Copy link
Copy Markdown
Member

What does this implement/fix?

Backend side of the dot-prefixed key blind spot from #2651. ESPHome ignores top-level keys starting with a dot, the documented way to park YAML anchors. Two line scanners only ended a top-level block at an alphabetic column-0 line, so a dot-prefixed key did not terminate the block above it; a component add or migration nudge on such a file could splice into the wrong place.

The rule now has one home: is_ignored_top_level_key and opens_top_level_block in helpers/yaml/scan.py. block_end_index and _locate_singleton_block treat a column-0 dot key as a block boundary, matching the scanners that already handled it; column-0 comments deliberately keep their current behaviour. The two resolved-config dict walks skip dot keys too: extract_directly_referenced_integrations (defense in depth, the loaded-integrations intersection already kept dot keys out of the drawer) and the automations _iter_instance_targets walk, so a dot-parked instance can no longer resolve as a ComponentTarget.

The editor rendering bug reported in the issue is fixed in the companion frontend PR.

Related issue or feature (if applicable):

Types of changes

  • Bugfix (non-breaking change which fixes an issue) — bugfix
  • New feature (non-breaking change which adds functionality) — new-feature
  • Enhancement to an existing feature — enhancement
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) — breaking-change
  • Refactor (no behaviour change) — refactor
  • Documentation only — docs
  • Maintenance / chore — maintenance
  • CI / workflow change — ci
  • Dependencies bump — dependencies

Frontend coordination

Checklist

  • The code change is tested and works locally.
  • Pre-commit hooks pass (ruff, codespell, yaml/json/python checks).
  • Tests have been added or updated under tests/ where applicable.
  • components.index.json / definitions/components/*.json have not been hand-edited (regenerate via script/sync_components.py if a sync is needed).
  • Architecture-level changes are reflected in docs/ARCHITECTURE.md and/or docs/API.md.

@bdraco bdraco added the bugfix Bug fix label Sep 2, 2026
@bdraco

bdraco commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

@esphbot review

@codspeed-hq

codspeed-hq Bot commented Sep 2, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 28 untouched benchmarks
⏩ 1 skipped benchmark1


Comparing dot-key-block-bounds (3afa872) with main (46ad66b)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@bdraco
bdraco marked this pull request as ready for review September 2, 2026 09:51
Copilot AI lite review requested due to automatic review settings September 2, 2026 09:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are small, targeted, and backed by new regression tests covering the dot-prefixed key boundary and integration-extraction behavior.

Pull request overview

Fixes backend YAML “line scanner” handling so dot-prefixed top-level keys (ESPHome’s convention for ignored anchor containers) correctly terminate the preceding block during text splices, and are excluded from the “direct integrations” list used by the device drawer.

Changes:

  • Treat column-0 dot-prefixed keys as top-level block boundaries in block_end_index and _locate_singleton_block, preventing splices from absorbing anchor blocks.
  • Skip dot-prefixed top-level keys in extract_directly_referenced_integrations, matching ESPHome core behavior.
  • Add focused regression tests covering both the splice boundary behavior and the integrations extractor behavior.
File summaries
File Description
tests/test_yaml_helpers.py Adds regression tests ensuring splices stop before dot-prefixed anchor containers and validates updated line-scan helpers.
tests/test_directly_referenced_integrations.py Adds a test asserting dot-prefixed top-level keys are excluded from the “direct integrations” list.
esphome_device_builder/helpers/yaml/writing_layout.py Updates singleton-block scanning to treat dot-prefixed keys as terminating the current block.
esphome_device_builder/helpers/yaml/scan.py Updates block_end_index to recognize dot-prefixed top-level keys as block starts/boundaries.
esphome_device_builder/helpers/device_yaml/_parsing.py Skips dot-prefixed top-level keys when extracting directly referenced integrations.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.72%. Comparing base (46ad66b) to head (3afa872).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2652   +/-   ##
=======================================
  Coverage   99.72%   99.72%           
=======================================
  Files         279      279           
  Lines       23591    23598    +7     
=======================================
+ Hits        23527    23534    +7     
  Misses         64       64           
Flag Coverage Δ
py3.12 99.69% <100.00%> (+<0.01%) ⬆️
py3.14 99.56% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
..._device_builder/controllers/automations/parsing.py 100.00% <100.00%> (ø)
...ome_device_builder/helpers/device_yaml/_parsing.py 100.00% <100.00%> (ø)
esphome_device_builder/helpers/yaml/scan.py 100.00% <100.00%> (ø)
...home_device_builder/helpers/yaml/writing_layout.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bdraco

bdraco commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

@esphbot review

@esphbot

esphbot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Previous review — superseded by a newer review below.

@esphbot esphbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

No blocking issues found — ready to merge.

@bdraco

bdraco commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

1: fixed in 3afa872; the test now pins resolve_component_target, verified failing with the guard removed.
Both description notes addressed: the drawer claim is corrected (the loaded-integrations intersection already masked it) and the automations walk guard is mentioned.

@esphbot review

@esphbot

esphbot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@bdraco: 1: fixed in 3afa872; the test now pins resolve_component_target, verified failing with the guard r...

Confirmed resolved. resolve_component_target(yaml, "parked_button") is None pins the _iter_instance_targets guard directly; the real_button control rules out a vacuous pass. Description notes both addressed. One new non-blocking nit: quoted dot keys (".anchors":) still aren't boundaries — pre-existing, same class.

@esphbot

esphbot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

PR Review — Treat dot-prefixed top-level keys as block boundaries in the line scanners

Prior suggestion is resolved; the fix is correct and well-scoped. Merge-ready with one non-blocking nit.

The follow-up commit (3afa872) does exactly what was asked: test_parse_skips_dot_prefixed_ignored_blocks now asserts resolve_component_target(yaml, "parked_button") is None alongside the positive real_button control, which pins the _iter_instance_targets guard rather than the catalog-domain filter that already masked it. The PR body's drawer claim and the fourth (automations walk) change were both corrected too.

The rest holds up on re-read. opens_top_level_block is exactly equivalent to the old stripped[0].isalpha() and not stripped.startswith(" ") pair for every pre-existing input, since an indented or tab-led line starts with neither an alpha nor a dot — the not startswith(" ") clause was redundant. Every consumer of block_end_index goes through the one helper (migrations.py:359/395/445, component.py:291, inline.py:456, mutations_yaml.py:279, _locate_top_list_item), so there is no second scanner left carrying the old rule — git grep isalpha() returns only the new predicate. Column-0 comments keeping their two-branch treatment in _locate_singleton_block is right and is pinned by test_block_end_index_skips_column_zero_comments. No import cycle: helpers/yaml/scan.py imports only re.

  • Residual gap, pre-existing and low-likelihood: a quoted dot key (".anchors":) is still not a boundary, and top_level_key_index already recognises quoted keys — the two column-0 scanners now disagree on that shape.
  • parsing.py (840) and _parsing.py (932) are over the 800-line soft cap, but AGENTS.md explicitly carves out small bugfixes from the split rule; +3 lines each is fine.


Checklist

  • New boundary predicate is behaviour-equivalent for pre-existing inputs
  • All downstream block_end_index consumers benefit rather than regress
  • Rule has a single home — no scanner left on the old predicate
  • Top-level dict walks that could surface dot keys are all covered
  • New tests pin the behaviour they claim to pin (resolved in 3afa872)
  • PR description matches the diff (drawer claim and automations guard corrected)
  • No security-relevant surface touched (no path/input/deserialization change)
ℹ️ Triage summary

1 pre-existing finding(s) on unchanged code suppressed (freeze).


Automated review by Kōan (Claude) HEAD=3afa872 2 min 45s

@esphbot esphbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

No blocking issues found — ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants