Skip to content

Validate that a board's featured bus dependencies are satisfied - #1755

Merged
bdraco merged 1 commit into
mainfrom
bus-dep-validate-guard
Jun 30, 2026
Merged

Validate that a board's featured bus dependencies are satisfied#1755
bdraco merged 1 commit into
mainfrom
bus-dep-validate-guard

Conversation

@bdraco

@bdraco bdraco commented Jun 30, 2026

Copy link
Copy Markdown
Member

What does this implement/fix?

Follow-up to #1752 and #1753 (both merged; this is rebased onto main). Those PRs
fixed boards whose imported full-setup config
wouldn't compile because a featured leaf's bus dependency was never lifted into
the catalog. We only found those by hand, because nothing validated that a
board's featured components form a compilable set: validate_definitions.py
checks each piece structurally (schema, cross-references, pins) but never that a
featured leaf's bus dependency is actually present.

This adds that check. _validate_featured_dependencies flags any featured leaf
(on an imported board) whose catalog dependencies include a bus
(i2c/spi/uart/modbus/one_wire/canbus) that no featured component on the board
provides, with a message pointing at the sync-script lift. It is a cheap static
graph check, no esphome config compile and no esphome.components import, so it
fits the existing pre-commit / CI hook.

Five kincony boards remain knowingly unsatisfied for a different, source-level
reason (the source has no top-level bus block, or a one_wire: block with no
platform:); they are tracked in an explicit _UNSATISFIED_BUS_ALLOW_LIST with
the per-board reason rather than silently skipped, so removing one must be paired
with a fix. Run against the current catalog this passes; drop a board from the
allow list (or regress the sync script) and it fails with the offending leaf.

Related issue or feature (if applicable):

Types of changes

  • Maintenance / chore — maintenance

Frontend coordination

  • No frontend change needed

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.
  • Architecture-level changes are reflected in docs/ARCHITECTURE.md and/or docs/API.md.

@bdraco bdraco added the maintenance Maintenance / chores label Jun 30, 2026
Base automatically changed from bus-dep-relax-id to main June 30, 2026 00:21
@bdraco
bdraco force-pushed the bus-dep-validate-guard branch from 8d96faa to 17e7c53 Compare June 30, 2026 00:21
@bdraco
bdraco marked this pull request as ready for review June 30, 2026 00:24
Copilot AI review requested due to automatic review settings June 30, 2026 00:24
@esphbot

esphbot commented Jun 30, 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.

No blocking issues found.

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.

Pull request overview

Adds a static validation step to script/validate_definitions.py to ensure that imported boards’ featured component sets are compilable with respect to bus dependencies (i2c/spi/uart/modbus/one_wire/canbus), plus targeted unit tests for the new validation helper.

Changes:

  • Introduce _validate_featured_dependencies() to flag featured components whose bus dependencies are missing from the same featured set (imported boards only, with an explicit allow-list).
  • Wire the new dependency validation into _validate_featured() so it runs alongside existing structural and cross-reference checks.
  • Add unit tests exercising the new dependency validation behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
script/validate_definitions.py Adds bus-category constants, an allow-list for known-unsatisfied imports, and the new _validate_featured_dependencies() check integrated into featured validation.
tests/test_validate_featured.py Adds a small helper to construct minimal featured entries and new tests covering dependency validation outcomes.

Comment thread script/validate_definitions.py Outdated
Comment on lines +337 to +338
present = {e.get("component_id") for e in featured if isinstance(e, dict)}
present_domains = {cid.split(".")[0] for cid in present if isinstance(cid, str)}
Comment on lines +343 to +345
cid = entry.get("component_id")
if not isinstance(cid, str) or "." not in cid:
continue
Comment on lines +435 to +439
def test_featured_dep_satisfied_by_featured_bus(_index: dict | None) -> None:
"""The board passes once the i2c bus is also featured."""
featured = [_leaf("i2c", "bus"), _leaf("sensor.sht3xd", "t")]
assert _validate_featured_dependencies("demo", featured, _index, True) == []

@codspeed-hq

codspeed-hq Bot commented Jun 30, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 27 untouched benchmarks


Comparing bus-dep-validate-guard (ae5b398) with main (4e3dba0)

Open in CodSpeed

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.54%. Comparing base (4e3dba0) to head (ae5b398).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1755   +/-   ##
=======================================
  Coverage   99.54%   99.54%           
=======================================
  Files         228      228           
  Lines       18202    18203    +1     
=======================================
+ Hits        18120    18121    +1     
  Misses         82       82           
Flag Coverage Δ
py3.12 99.52% <100.00%> (+<0.01%) ⬆️
py3.14 99.54% <100.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
esphome_device_builder/constants.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 force-pushed the bus-dep-validate-guard branch from 17e7c53 to 1a38fcd Compare June 30, 2026 00:40
@bdraco

bdraco commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

Thanks @esphbot — all three applied in 1a38fcd8:

  1. Per-board → per-(board, bus) allow list. _UNSATISFIED_BUS_ALLOW_LIST is now a set of (board_id, bus) tuples, and the check skips only the matching dep instead of early-returning for the whole board. A different unsatisfied bus on one of the five kincony boards now still fails. Added test_featured_dep_allowlist_is_per_bus_not_per_board (an spi leaf on i2c-waived kincony_mb is flagged).

  2. default_components folded into the present set. _ref_ids() now collects ids from both featured_components and default_components, so a bus provided via defaults counts as present. Added test_featured_dep_satisfied_by_default_component.

  3. BUS_CATEGORIES hoisted to esphome_device_builder/constants.py (the stdlib-only module both scripts already import) and imported in both sync_esphome_devices.py and validate_definitions.py, so the importer and validator can't drift. No esphome import pulled in; catalog regen is byte-identical.

Full suite 5623 passed; validate_definitions.py stays clean on all 525 boards.

Nothing checked that an imported board's featured components form a compilable
set, so a featured leaf binding a bus that was never lifted into the catalog
(the bug #1752 and #1753 fixed by hand) passed validation. Add
_validate_featured_dependencies: flag any featured leaf whose catalog bus
dependency no component on the board provides (featured or default). The allow
list is keyed on (board, bus) so a different unsatisfied bus on an allow-listed
board still fails, and BUS_CATEGORIES is shared through the stdlib-only constants
module so the importer (which lifts buses) and the validator (which checks they
were lifted) can't drift. A cheap static graph check, no esphome config compile.
@bdraco
bdraco force-pushed the bus-dep-validate-guard branch from 1a38fcd to ae5b398 Compare June 30, 2026 00:47
@bdraco

bdraco commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

Thanks @copilot — addressed in ae5b398e:

1. TypeError on a non-string component_id — already fixed in the prior commit: the present-set is now built by a _ref_ids() helper that only adds values passing isinstance(..., str), so a list/dict component_id is skipped, not hashed into a set. Added test_featured_dep_non_string_component_id_does_not_crash to pin it.

2. "." in cid skips top-level components — good catch on the gap, but I tested removing the guard and it flags 6 boards as false positives, all sn74hc595 (es32a08, esp32_relay_x16, kincony_kc868_a256, tuya_wifi_sprinkler_controller, usr_eg118). sn74hc595 is dual-mode: it declares dependencies: ["spi"] but bit-bangs over GPIO (data_pin/clock_pin/latch_pin) on all six — no spi bus needed. The catalog dependencies over-declares the conditional bus. The dotted-leaf scoping is deliberate: platform leaves (sensors/displays/touchscreens) bind a bus unconditionally (it's their only connection), whereas bare top-level components are either buses themselves (no bus dep) or dual-mode hubs with conditional deps, and their bus is lifted by the importer's hub passes. Expanded the code comment to spell this out.

3. Regression test for the top-level case — added test_featured_dep_dual_mode_top_level_hub_not_flagged, which pins the intended behavior (an sn74hc595 over GPIO is not flagged for spi) so the scoping isn't later 'fixed' into the false positives above.

validate_definitions.py stays clean on all 525 boards; 32 tests in test_validate_featured.py.

@esphbot

esphbot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

PR Review — Validate that a board's featured bus dependencies are satisfied

Merge-ready. A cheap, correct static guard that closes a real gap, with all prior-review and Copilot findings already addressed in-branch.

What's solid:

  • Dependency-graph logic is correct against the live catalog: top-level buses match via present, platform-style buses (one_wire/canbus) via present_domains domain match; distinct bus types don't collapse despite sharing category "bus". Verified present_domains can't mask a truly-missing bus.
  • All three prior findings resolved: allow list re-keyed to per-(board, bus) (a different unsatisfied bus on a waived board still fails), default_components folded into the satisfaction set via _ref_ids, and BUS_CATEGORIES hoisted to the shared stdlib-only constants module.
  • Copilot's TypeError-on-non-string-component_id and dual-mode-hub (sn74hc595) scoping concerns both handled, each pinned by a focused test; 32 tests in test_validate_featured.py.

What's left:

  • Only a minor optional note: the _is_bus_dep helper is still duplicated across the two scripts (the constant is now shared, so drift risk is small).


Checklist

  • New validation branches are tested
  • Edge cases covered (None index, non-imported, platform bus, non-string id, dual-mode hub)
  • Allow list does not blanket-mask future regressions
  • No duplicated constants that can drift
  • PR delivers what the description promises

Automated review by Kōan (Claude) HEAD=ae5b398 1 min 44s

@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.

No blocking issues found.

@bdraco
bdraco merged commit 78855c1 into main Jun 30, 2026
22 checks passed
@bdraco
bdraco deleted the bus-dep-validate-guard branch June 30, 2026 00:53
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

maintenance Maintenance / chores

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants