bdraco · 2026-08-06T00:37:54Z

What does this implement/fix?

Since #16718 every config load deep copies the whole raw config into result.user_config so esphome config --no-defaults can dump exactly what the user wrote, but it was never noticed because our CI failed to run the benchmarks on that PR; the benchmarks job is not triggered by top-level esphome/*.py changes, so the author never got a signal, which is fixed in #18114. Profiling shows the copy takes about a third of read_config time; the config tree is full of dynamically generated node classes carrying source location metadata, so copy.deepcopy goes through the slow generic reconstruct path for every node.

Only the config command with --no-defaults ever reads that snapshot, so take it only when asked: a snapshot_user_config flag is threaded from the read_config call site down to validate_config, defaulting to off. Compile, upload, logs, dashboard and the vscode language server all skip the copy entirely; --no-defaults behaves exactly as before, and the existing fallback in command_config still covers any path without a snapshot.

CodSpeed measured the win on #18115, a test PR combining this change with the #18114 gating fix: test_read_config_uncached went from 111.9 ms to 73 ms, a 53% improvement, with all other benchmarks untouched; full report at #18115 (comment).

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
New developer-facing feature (adds functionality for component developers; no end-user configuration change)
Breaking change (fix or feature that would cause existing functionality to not work as expected) — policy
Developer breaking change (an API change that could break external components) — policy
Undocumented C++ API change (removal or change of undocumented public methods that lambda users may depend on) — policy
Code quality improvements to existing code or addition of tests
Other

Related issue or feature (if applicable):

related to [config] Add --no-defaults flag to config command #16718

Pull request in esphome.io with documentation (if applicable):

esphome/esphome.io#<esphome.io PR number goes here>

Pull request in developers.esphome.io with developer documentation (if applicable):

esphome/developers.esphome.io#<developers.esphome.io PR number goes here>

Test Environment

Example entry for `config.yaml`:

# Example config.yaml

Checklist:

The code change is tested and works locally.
Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in esphome.io.

…eds it

codecov · 2026-08-06T00:42:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.28%. Comparing base (a774009) to head (858d5b7).

Additional details and impacted files

@@           Coverage Diff           @@
##              dev   #18113   +/-   ##
=======================================
  Coverage   87.28%   87.28%           
=======================================
  Files          64       64           
  Lines       14696    14697    +1     
  Branches     2216     2217    +1     
=======================================
+ Hits        12827    12828    +1     
  Misses       1560     1560           
  Partials      309      309

Files with missing lines	Coverage Δ
esphome/__main__.py	`80.13% <ø> (ø)`
esphome/config.py	`76.34% <100.00%> (+0.02%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle A 8000 nalysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bluetoothbot · 2026-08-06T00:55:21Z

PR Review — [core] Only snapshot the user config when esphome config --no-defaults asks for it

Correct, minimal opt-in for an expensive deep copy. Merge-ready; one test-coverage nit.

Specific things done well:

The flag is threaded as a keyword-only-in-practice parameter appended after skip_external_update, so every existing positional caller (script/build_helpers.py:377, esphome/vscode.py:136) keeps working unchanged — no developer-facing break.
The pass-through calls in _load_config/load_config/read_config were converted from positional to keyword arguments, which removes the easy way to transpose the two booleans.
I verified Config.user_config has exactly one consumer in the tree (command_config, esphome/main.py:1461), and that --no-defaults is registered only on the config subparser — so defaulting the snapshot off cannot silently starve another code path. The multi-config path re-invokes the CLI with the original argv, so --no-defaults still reaches the child process.
Tests cover both branches of the new conditional, including the substitutions is None case that previously had no coverage.

Key issue:

The snapshot_user_config=getattr(args, "no_defaults", False) wiring in run_esphome has no test; a future rename would degrade --no-defaults to a warning plus defaulted output rather than a failure. test_run_esphome_skip_external_update_per_command is a drop-in template for the missing test.

🟢 Suggestions

1. The one wiring line that keeps `--no-defaults` working is untested
esphome/__main__.py:2601-2605

Every other part of this change is covered (config.py threading, both snapshot branches in test_substitutions.py), but the single line that connects the CLI flag to the new parameter is not exercised by any test.

Why it matters: if getattr(args, "no_defaults", False) ever stops matching the argparse dest (rename, moved flag, a new command that dumps the config), esphome config --no-defaults silently degrades — command_config finds user_config is None, logs a warning, and prints the validated config with schema defaults instead of failing. That is a quiet output regression, exactly the class of bug the PR description says CI missed on #16718.

There is already a ready-made pattern for this: test_run_esphome_skip_external_update_per_command (tests/unit_tests/test_main.py:6350) patches esphome.config.read_config and asserts call_args.kwargs. A parametrized twin would cost a few lines:

@pytest.mark.parametrize(
    ("argv_extra", "expected"),
    [(["--no-defaults"], True), ([], False)],
)
def test_run_esphome_snapshot_user_config_only_for_no_defaults(
    tmp_path: Path, argv_extra: list[str], expected: bool
) -> None:
    yaml_file = tmp_path / "device.yaml"
    yaml_file.write_text("esphome:\n  name: test\n")
    with patch("esphome.config.read_config", return_value=None) as mock_read:
        run_esphome(["esphome", "config", str(yaml_file), *argv_extra])
    assert mock_read.call_args.kwargs["snapshot_user_config"] is expected

Non-blocking — the wiring is correct as written (verified --no-defaults is registered only on the config subparser at esphome/main.py:2236, and command_config at :1461 reads the same attribute name).

        config = read_config(
            command_line_substitutions,
            skip_external_update=skip_external,
            # Snapshot only needed by `esphome config --no-defaults`.
            snapshot_user_config=getattr(args, "no_defaults", False),
        )

Checklist

Backward compatibility for existing callers (positional args preserved)
All consumers of the removed-by-default user_config snapshot identified
New branches covered by tests — suggestion ＃1
Diff matches the PR description (no scope creep)
No error handling or resource-lifecycle changes

Automated review by Kōan (Claude) HEAD=684136a 2 min 9s

bluetoothbot

Tip

No blocking issues found — ready to merge.

esphome · 2026-08-06T01:01:42Z

👋 Hi there! This PR modifies 2 file(s) with codeowners.

@esphome/core - As codeowner(s) of the affected files, your review would be appreciated! 🙏

Note: Automatic review request may have failed, but you're still welcome to review.

Copilot

Pull request overview

Reduces configuration load overhead by avoiding an unconditional deep-copy of the raw user config during validation, while preserving esphome config --no-defaults behavior by taking the snapshot only when that command requests it.

Changes:

Add a snapshot_user_config flag to validate_config() and thread it through load_config()/read_config() so the user-config snapshot is optional.
Update CLI invocation to enable snapshotting only for esphome config --no-defaults.
Extend unit tests to cover the opt-in snapshot behavior and the default “no snapshot” path.

Reviewed changes

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

File	Description
`esphome/config.py`	Makes user-config snapshot deep-copy conditional via `snapshot_user_config` and propagates the flag through config-loading helpers.
`esphome/__main__.py`	Passes `snapshot_user_config` to `read_config()` only when `--no-defaults` is set for the `config` command.
`tests/unit_tests/test_substitutions.py`	Updates existing snapshot tests to opt in, and adds coverage for “no substitutions” and default snapshot skipping.
`tests/unit_tests/test_main.py`	Verifies `run_esphome` only requests snapshotting for `config --no-defaults`.

codspeed-hq · 2026-08-06T01:54:45Z

Merging this PR will improve performance by 53.42%

⚡ 1 improved benchmark
✅ 152 untouched benchmarks

Performance Changes

	Benchmark	`BASE`	`HEAD`	Efficiency
⚡	`test_read_config_uncached`	112 ms	73 ms	+53.42%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing skip-user-config-deepcopy (858d5b7) with dev (a774009)}

bdraco · 2026-08-06T02:20:49Z

thanks

Skip the user_config deep copy unless esphome config --no-defaults ne…

6500632

…eds it

esphome Bot added code-quality core has-tests medium-pr PR < 100 lines labels Aug 6, 2026

bdraco mentioned this pull request Aug 6, 2026

[ci] Run CodSpeed benchmarks when top-level esphome Python modules change #18114

Merged

19 tasks

Cover the snapshot path for configs without substitutions

684136a

bluetoothbot approved these changes Aug 6, 2026

View reviewed changes

Test the CLI wiring for snapshot_user_config

53e229c

bdraco mentioned this pull request Aug 6, 2026

DNM: [core] Prove CodSpeed gating reports the user_config deepcopy win #18115

Closed

19 tasks

bdraco marked this pull request as ready for review August 6, 2026 01:01

bdraco requested a review from a team as a code owner August 6, 2026 01:01

Copilot AI lite review requested due to automatic review settings August 6, 2026 01:01

Copilot started reviewing on behalf of bdraco August 6, 2026 01:01 View session

Copilot AI reviewed Aug 6, 2026

View reviewed changes

Merge branch 'dev' into skip-user-config-deepcopy

858d5b7

swoboda1337 approved these changes Aug 6, 2026

View reviewed changes

bdraco merged commit eb0fac9 into dev Aug 6, 2026
37 checks passed

bdraco deleted the skip-user-config-deepcopy branch August 6, 2026 02:20

github-actions Bot locked and limited conversation to collaborators Aug 8, 2026

Navigation Menu

Uh oh!

Uh oh!

[core] Only snapshot the user config when esphome config --no-defaults asks for it - #18113

[core] Only snapshot the user config when esphome config --no-defaults asks for it#18113
bdraco merged 4 commits into
devfrom
skip-user-config-deepcopy

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

Uh oh!

What does this implement/fix?

Types of changes

Test Environment

Example entry for config.yaml:

Checklist:

Uh oh!

Uh oh!

Codecov Report

Uh oh!

PR Review — [core] Only snapshot the user config when esphome config --no-defaults asks for it

🟢 Suggestions

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Merging this PR will improve performance by 53.42%

Performance Changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Example entry for `config.yaml`: