Skip to content

Show the receiver esphome version only when it builds with it - #2628

Merged
bdraco merged 4 commits into
mainfrom
receiver-build-version
Aug 24, 2026
Merged

Show the receiver esphome version only when it builds with it#2628
bdraco merged 4 commits into
mainfrom
receiver-build-version

Conversation

@bdraco

@bdraco bdraco commented Aug 24, 2026

Copy link
Copy Markdown
Member

What does this implement/fix?

The firmware tasks drawer showed "Building on windoze (2026.8.0)" while the receiver was actually compiling with 2026.8.1 in a provisioned venv. The job snapshotted the pairing's installed esphome verbatim, so a receiver that auto provisions our version was labelled with the wrong one.

The version now only rides on the job when the receiver really builds with its own esphome: a version mismatch with no venv provisioning. Same version, or a provisionable mismatch, shows just the receiver name. The rule lives next to the scheduler's eligibility check in helpers/build_scheduler.py (receiver_build_version / build_source_for_pairing), and JobBuildSource.for_pairing is gone since the model can't reach the version helpers.

Related issue or feature (if applicable):

  • fixes N/A

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 Aug 24, 2026
@bdraco

bdraco commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

@esphbot review

@codspeed-hq

codspeed-hq Bot commented Aug 24, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 28 untouched benchmarks
⏩ 1 skipped benchmark1


Comparing receiver-build-version (ec11e0b) with main (a807997)

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.

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.72%. Comparing base (fec63f0) to head (ec11e0b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2628   +/-   ##
=======================================
  Coverage   99.72%   99.72%           
=======================================
  Files         276      276           
  Lines       23490    23497    +7     
=======================================
+ Hits        23426    23433    +7     
  Misses         64       64           
Flag Coverage Δ
py3.12 99.68% <100.00%> (+<0.01%) ⬆️
py3.14 99.72% <100.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
...phome_device_builder/controllers/firmware/clean.py 100.00% <100.00%> (ø)
...ce_builder/controllers/firmware/remote_dispatch.py 100.00% <100.00%> (ø)
...er/controllers/remote_build/submit_job_commands.py 95.45% <100.00%> (+0.21%) ⬆️
esphome_device_builder/helpers/build_scheduler.py 100.00% <100.00%> (ø)
esphome_device_builder/models/firmware.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 Aug 24, 2026

Copy link
Copy Markdown
Member Author

@esphbot review

@bdraco
bdraco marked this pull request as ready for review August 24, 2026 02:41
Copilot AI lite review requested due to automatic review settings August 24, 2026 02:41

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

This PR fixes incorrect receiver ESPHome version labeling in the firmware tasks drawer by only attaching a receiver-side ESPHome version to a job when the receiver will actually build using its own installed ESPHome (i.e., when there’s a version mismatch and the receiver cannot provision the offloader’s version into a venv). This aligns the displayed build source with the real compiler environment used on the receiver.

Changes:

  • Introduce receiver_build_version() and build_source_for_pairing() in helpers/build_scheduler.py to centralize the “when should we surface receiver ESPHome version” rule alongside provisioning eligibility logic.
  • Remove JobBuildSource.for_pairing() from the firmware model to avoid model-layer coupling to provisioning/version logic, updating call sites to use build_source_for_pairing().
  • Add/adjust tests to cover the new stamping behavior, including dispatch behavior when auto_provision_supported is set.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_build_scheduler.py Adds focused unit tests for receiver_build_version() and build_source_for_pairing().
tests/models/test_firmware_job.py Removes tests tied to the deleted JobBuildSource.for_pairing() constructor.
tests/controllers/firmware/test_remote_dispatch.py Verifies remote dispatch stamps source_esphome_version only when the receiver truly builds with its own ESPHome.
tests/controllers/firmware/conftest.py Extends pairing stubs to include auto_provision_supported for scheduler/dispatch tests.
esphome_device_builder/models/firmware.py Removes JobBuildSource.for_pairing() and updates source_esphome_version semantics documentation.
esphome_device_builder/helpers/build_scheduler.py Adds receiver_build_version() + build_source_for_pairing() and refactors provisioning check into _can_provision().
esphome_device_builder/controllers/remote_build/submit_job_commands.py Switches reset-build-env job creation to use build_source_for_pairing().
esphome_device_builder/controllers/firmware/remote_dispatch.py Switches remote dispatch stamping to use build_source_for_pairing() with the offloader version from the scheduler snapshot.
esphome_device_builder/controllers/firmware/clean.py Switches remote clean fan-out jobs to use build_source_for_pairing().

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

@esphbot

esphbot commented Aug 24, 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 Aug 24, 2026

Copy link
Copy Markdown
Member Author

Suggestion 2 taken in b2d2cf1: the reset build-env path imports the installed version directly instead of building a scheduler snapshot for one field.

Suggestion 1 left as is. A cross-reference comment pointing at _resolve_esphome_cmd is the kind of framing this repo keeps out of code, and a receiver-reported build version is a wire change beyond this fix. The dev offloader window closes as soon as the receiver refuses to provision and the job re-routes local; the uncached CLEAN case is already surfaced in the job log by the receiver.

@bdraco

bdraco commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

@esphbot review

1 similar comment
@bdraco

bdraco commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

@esphbot review

@bdraco

bdraco commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

@esphbot review

@esphbot

esphbot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

@bdraco: Suggestion 2 taken in b2d2cf1: the reset build-env path imports the installed version directly ins...

Confirmed both. b2d2cf1 imports esphome.const.__version__ directly, matching remote_runner.py:37; no snapshot copy remains in reset_peer_build_env.

Suggestion 1 accepted as won't-fix. Your reasoning holds: cross-reference prose contradicts AGENTS.md, and a receiver-reported version widens the wire. Dev-offloader window self-closes on provision refusal; uncached CLEAN already logs the fallback at controller.py:671. Not re-raised.

@esphbot

esphbot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

PR Review — Show the receiver esphome version only when it builds with it

Merge-ready. Prior suggestion 2 is fixed, suggestion 1 is dismissed with a reason I accept, and re-reading the head commit turned up nothing new.

What's solid here:

  • The stamping rule now shares one predicate (_can_provision) with the eligibility filter in helpers/build_scheduler.py:291, so the label and the routing decision cannot drift apart — that was the actual bug, not just the wrong string in the drawer.

  • Dropping JobBuildSource.for_pairing removes the model layer's last reference to remote_build (models/firmware.py no longer needs the TYPE_CHECKING import); the classmethod could only ever copy the pairing verbatim since the model can't see is_pinnable_version.

  • All three stamping sites are converted — remote_dispatch.py:172, clean.py:54, submit_job_commands.py:101; git grep for_pairing shows no stale callers.

  • b2d2cf1 follows the remote_runner.py:37 precedent and imports esphome.const.__version__ directly instead of copying two dicts through build_scheduler_snapshot() for one field.

  • Coverage matches the rule: the five-way table in tests/test_build_scheduler.py:1141 plus a dispatch-level parametrisation asserting the stamped field rather than an internal call. The pre-existing test_clean.py:364 assertion still holds because those pairings are non-provisionable mismatches.

  • No new findings. Suggestion 1 (the rule approximates the receiver's _resolve_esphome_cmd decision, diverging for a dev offloader and an uncached CLEAN) stands dismissed per @bdraco — a cross-reference comment is against this repo's docstring conventions, and a receiver-reported build version is a wire change beyond this fix.

  • Note on verification: the review shell is read-only, so I could not execute the test suite; findings above rest on reading the code at HEAD, not on a test run.


✅ Resolved since last review (1)

Previously-flagged issues verified fixed
  • esphome_device_builder/controllers/remote_build/submit_job_commands.py:99 Whole scheduler snapshot built to read one constant


Checklist

  • All call sites of the removed JobBuildSource.for_pairing converted
  • Stamping rule shares one predicate with the eligibility filter (no drift)
  • No import cycle introduced by helpers -> models.firmware
  • Behaviour change covered by tests (unit + dispatch level)
  • Pre-existing assertions on source_esphome_version still hold
  • Wire/persistence compatibility (field kept, semantics narrowed; frontend PR paired)
  • Docstring/comment conventions (one-line contracts, no rationale prose)
  • No scope creep beyond the stated fix

Automated review by Kōan (Claude) HEAD=ec11e0b 1 min 36s

@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 merged commit eb2baf8 into main Aug 24, 2026
22 checks passed
@bdraco
bdraco deleted the receiver-build-version branch August 24, 2026 04:11
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 26, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants