Skip to content

Trim controller.py prose, pass 2: 3723 → 3186 (−537)#633

Merged
bdraco merged 1 commit into
mainfrom
trim-controller-prose-pass2
May 12, 2026
Merged

Trim controller.py prose, pass 2: 3723 → 3186 (−537)#633
bdraco merged 1 commit into
mainfrom
trim-controller-prose-pass2

Conversation

@bdraco
Copy link
Copy Markdown
Member

@bdraco bdraco commented May 12, 2026

What does this implement/fix?

Pass 1 (#630) cut method-level docstrings but left the inline
# explain what this line does paragraphs intact — which were
the actual wall-of-text problem. This pass strips those per
CLAUDE.md's "don't explain WHAT the code does, since
well-named identifiers already do that" rule.

controller.py: 3723 → 3186 lines (−537).
Prose-to-code ratio: ~1.4:1 → ~1:1 (was ~2:1 in the
pre-trim original).

Concretely

  • __init__: attribute-explainer comments collapsed to
    one or two lines each — the well-named attributes already
    say what each holds. 208 → 110 lines.
  • start: lost the 6-8 line preludes before every block.
    183 → 78 lines.
  • stop: lost the per-drain rationale paragraphs that
    the _drain_tasks helper already encodes. 99 → 44 lines.
  • request_pair: dropped the "APPROVED here means…",
    "sweep stale entry…", "re-pair against same pin…" preludes
    before each near-self-explanatory block. 140 → 97 lines.
  • unpair: lost the per-clear-call rationale
    paragraphs. 89 → 24 lines.
  • _apply_pair_status_result: lost the inline preludes
    on each terminal branch. 117 → 88 lines.
  • record_pair_request: lost the receiver-side branch
    explainers + the 11-line PENDING-entry refresh
    explanation. 106 → 78 lines.
  • submit_job / register_peer_link_session /
    set_settings / rotate_identity / set_pairing_window
    / _run_cleanup_loop
    : similarly trimmed.
  • Module-level constant explainers (the 8-line
    cleanup-sweep cadence comment, the 6-line debounce-window
    comment, the 10-line resolve-timeout comment, etc.)
    reduced to one or two lines each.

What was kept

The WHY-style notes per CLAUDE.md:

  • Post-fix bug references (e.g. the Route firmware/install through pick_build_path + REMOTE upload chain (7a-3) #568 cold-connect
    regression in register_peer_link_session).
  • Hidden invariants ("fire AFTER the dict insert so
    subscriber lookups see the just-registered session").
  • Surprising orderings ("snapshot to a list before
    iterating — each terminate mutates the dict").
  • Non-obvious type-coercion gotchas (the bool-subclass-of-int
    caveat on cleanup_ttl_seconds).
  • Security-defense rationale (the
    same-dashboard_id-different-pubkey refusal in
    record_pair_request).

Behaviour preservation

No code lines changed; only comment and docstring lines
removed. All 3117 tests pass.

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

  • No frontend change needed
  • Companion frontend PR: esphome/device-builder-dashboard-frontend#

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.json has 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.

The first pass cut method-level docstrings but left the
inline ``# explain what this line does`` paragraphs intact —
which were the actual wall-of-text problem. This pass strips
those (per CLAUDE.md's "don't explain WHAT the code does" rule)
across the largest methods:

* ``__init__`` attribute-explainer comments collapsed to one
  or two lines each — the well-named attributes already say
  what each is for. 208 → 110 lines.
* ``start`` lost the 6-8 line preludes before every block.
  183 → 78 lines.
* ``stop`` lost the per-drain rationale paragraphs the
  ``_drain_tasks`` helper already encodes. 99 → 44 lines.
* ``request_pair`` body: dropped the "APPROVED here means …"
  / "sweep stale entry …" / "re-pair against same pin …" /
  per-line PENDING-vs-APPROVED preludes. 140 → 97 lines.
* ``unpair`` lost the per-clear-call rationale paragraphs.
  89 → 24 lines.
* ``_apply_pair_status_result`` lost the inline preludes on
  each terminal branch. 117 → 88 lines.
* ``record_pair_request`` lost the receiver-side
  pair-request branch explainers + the 11-line PENDING-entry
  refresh explanation. 106 → 78 lines.
* ``submit_job`` / ``register_peer_link_session`` /
  ``set_settings`` / ``rotate_identity`` /
  ``set_pairing_window`` / ``_run_cleanup_loop`` similarly
  trimmed.
* Module-level constant explainers (the 8-line cleanup-sweep
  cadence comment, the 6-line debounce-window comment, the
  10-line resolve-timeout comment, etc.) reduced to one or
  two lines each.

Net: prose-to-code ratio drops to ~1:1 (was ~2:1 originally,
1.4:1 after pass 1). Kept the WHY-style notes (post-fix bug
refs like the #568 cold-connect regression, hidden invariants
like "fire AFTER the dict insert", surprising orderings like
the snapshot-list-before-iterate guard).

Behaviour preserving — no code lines changed; only comment
and docstring lines removed. All 3117 tests pass.
Copilot AI review requested due to automatic review settings May 12, 2026 00:21
@github-actions github-actions Bot added the docs Documentation label May 12, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 12, 2026

Merging this PR will not alter performance

✅ 12 untouched benchmarks


Comparing trim-controller-prose-pass2 (aed5b62) with main (b946b69)

Open in CodSpeed

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 further reduces the “wall-of-text” in RemoteBuildController by trimming inline explanatory prose and condensing docstrings, aligning the file more closely with the comment style guidance in CLAUDE.md (focus on why and invariants vs. narrating what the code does).

Changes:

  • Collapsed/removed large inline comment blocks and constant explainers across controller.py.
  • Condensed several lifecycle and remote-build flow docstrings to shorter summaries while keeping key invariants/security rationale notes.
  • Left runtime logic unchanged (documentation-only refactor).

Comment thread esphome_device_builder/controllers/remote_build/controller.py
Comment thread esphome_device_builder/controllers/remote_build/controller.py
Comment thread esphome_device_builder/controllers/remote_build/controller.py
@bdraco bdraco merged commit f65a49d into main May 12, 2026
17 checks passed
@bdraco bdraco deleted the trim-controller-prose-pass2 branch May 12, 2026 00:24
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #633   +/-   ##
=======================================
  Coverage   99.21%   99.21%           
=======================================
  Files          90       90           
  Lines       10757    10757           
=======================================
  Hits        10673    10673           
  Misses         84       84           
Flag Coverage Δ
py3.12 99.19% <ø> (ø)
py3.14 99.21% <ø> (ø)

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

Files with missing lines Coverage Δ
...ice_builder/controllers/remote_build/controller.py 99.67% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants