Skip to content

Release v5.12.0 - #77

Merged
tis24dev merged 9 commits into
mainfrom
dev
Aug 11, 2026
Merged

Release v5.12.0#77
tis24dev merged 9 commits into
mainfrom
dev

Conversation

@tis24dev

@tis24dev tis24dev commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Automated release PR for v5.12.0.

Summary by Sourcery

Introduce per-phase budgeting, hierarchical phase tracking, and improved error attribution for hOn client setup, authentication, appliance hydration, and diagnostics, addressing timeout misclassification and enhancing debuggability (issue #76), and bump the integration to v5.12.0.

Enhancements:

  • Add a dedicated per-phase budgeting system and hierarchical phase tracking to more accurately attribute setup, authentication, appliance loading, and MQTT timeouts.
  • Refine error classification and representation, including new auth timeout codes, structural transport classification, and single-present error codes/details in logs and Home Assistant messages.
  • Improve HonClient loop watchdog handling, including better phase attribution, preservation of real causes, special handling for MFA challenges, and lifecycle-safe teardown semantics.
  • Contain and track partial appliance hydration during setup, expose degraded-appliance and phase-ledger information, and rehydrate commands on the first poll to avoid silently crippled entities.
  • Extend diagnostics with model catalogue metadata, richer command parameter schemas (including small range grids), and leak-proof phase-ledger reporting for failed setups.
  • Enhance air purifier switch creation by logging capability-based rejections with redacted identifiers and full capability context.
  • Expose model catalogue attributes on appliances and propagate them through engine, diagnostics, and tests for better zone and model introspection.

Tests:

  • Add extensive tests for per-phase budgets, hierarchical phase tracking, auth retry policy, loop watchdog behaviour, appliance rehydration, setup failure recording, diagnostics phase-ledger, model attributes, transport classification, and configuration-flow error codes.
  • Update existing tests to cover new timeout codes, error detail handling, lifecycle lock behaviour, watchdog logging content, and the revised appliance setup/hydration sequence without statistics.
  • Extend diagnostics, switch, native session, coordinator resilience, transport MQTT, air purifier, program select, and auth error classification tests to validate the new behaviours and logging/diagnostic outputs.

Chores:

  • Bump the Home Assistant addhon integration manifest version from 5.11.0 to 5.12.0.

Summary by CodeRabbit

  • New Features

    • Added clearer setup progress and timing diagnostics, including phase history and summaries.
    • Added appliance model metadata to diagnostics.
    • Appliances with temporary connection issues can be rehydrated during updates.
    • Added improved retry handling for transient authentication and connection failures.
  • Bug Fixes

    • Improved timeout classification and guidance for authentication and session refresh failures.
    • Removed duplicated error-code prefixes from displayed messages.
    • Improved handling of partially unavailable appliances and purifier controls.
  • Documentation

    • Updated diagnostics documentation for model metadata.
  • Release

    • Updated the component version to 5.12.0.

Greptile Summary

The release introduces phase-specific timeout budgets and attribution, bounded authentication retries, degraded-appliance recovery, and richer diagnostics and model metadata.

  • Tracks hierarchical setup phases and reports phase-specific timeout codes.
  • Uses workload-specific budgets and watchdog caps for authentication, setup, polling, commands, and teardown.
  • Retains partially hydrated appliances and retries command hydration before initial entity discovery.
  • Improves transport classification, error presentation, MQTT lifecycle handling, and diagnostics.
  • Updates the integration version to 5.12.0.

Confidence Score: 5/5

The pull request appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
custom_components/addhon/client/budget.py Adds phase budgets, caller-budget suspension during nested authentication, and workload-specific watchdog caps.
custom_components/addhon/client/phase.py Adds hierarchical phase context, cross-thread phase mirroring, and a bounded diagnostic ledger.
custom_components/addhon/client/session.py Applies setup budgets, contains per-appliance hydration failures, and records appliances requiring first-poll rehydration.
custom_components/addhon/client/transport/connection.py Wraps authentication and MFA paths in attributed budgets while preserving refresh and reauthentication serialization.
custom_components/addhon/client/transport/retry.py Adds deadline-aware bounded retries for explicitly idempotent authentication transport steps.
custom_components/addhon/hon_client.py Introduces call-site watchdog caps, lifecycle-safe cancellation attribution, first-poll command rehydration, and phase-ledger preservation.
custom_components/addhon/error_codes.py Adds phase-specific timeout and shutdown codes, structural transport classification, and normalized error details.
custom_components/addhon/diagnostics.py Extends diagnostics with phase-ledger and model-catalogue metadata.
custom_components/addhon/manifest.json Bumps the integration release version to 5.12.0.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Home Assistant setup or poll] --> B[HonClient dedicated loop]
    B --> C[Phase scope and workload budget]
    C --> D{Authentication needed?}
    D -- Yes --> E[Suspend caller budget]
    E --> F[Refresh or full authentication]
    F --> G[Resume caller budget]
    D -- No --> H[Cloud request]
    G --> H
    H --> I{Appliance hydration succeeds?}
    I -- Yes --> J[Poll and expose entities]
    I -- Temporary failure --> K[Record degraded appliance]
    K --> L[Rehydrate commands on first poll]
    L --> J
    I -- All retryable failures --> M[Raise representative coded failure]
    C --> N{Budget or watchdog expires?}
    N -- Yes --> O[Attribute timeout to active phase]
Loading

Reviews (2): Last reviewed commit: "fix(diagnostics): forward the phase thro..." | Re-trigger Greptile

tis24dev and others added 8 commits August 10, 2026 07:07
The diagnostics dump described an appliance purely through its shadow: the
telemetry it happens to publish. That cannot answer what the appliance IS.
A fridge that reports `tempZ3` may simply not have that zone, and the dump
gave no way to tell the two apart, so a zone-indexing report needed a round
trip to the reporter before it could even be diagnosed (issue #75).

The cloud already sends the answer in `applianceModel.attributes`: `zones`,
`seriesVersion`, `vtRoom1`/`vtRoom2`, `doorNumber`. We were fetching it and
keeping only `options`. The hOn app itself treats those rows as authoritative
where the shadow is not - it decides which fridge zones exist from
`zones`.split("|"), never from which `tempZ*` keys the shadow carries.

Expose them as `HonAppliance.model_attributes`, flattened parName -> parValue
the same way __init__ already flattens the appliance-level attributes, and
emit them as a `model_attributes` block ahead of `attributes`: what the model
is, then what it is doing. Read off the appliance rather than the coordinator
entry, since it is per-model and immutable for the session. Redaction is
unchanged - the block goes through _redact like every other section.
A field report has been open for two weeks: an HHP50CA011 shows every purifier
control except Child Lock and Sounds. Neither artifact we ask for could answer
it, and that is a hole in this integration, not bad luck.

The AP branch of the switch platform gated on two conditions and did a bare
`continue` on each, while the summary line named only what it DID build. So a
purifier missing a toggle looked exactly like a purifier that never reached the
branch. The sibling platforms have always logged their skips: light.py names the
capability, the live values and the reported state, and select.py does the same
for the aroma and panel-light selects. The AP switches now match them, and the
line separates the two gates, so "the schema does not declare it" and "the device
does not report it" stop looking alike.

The dump had the matching hole. `_param_schema` emits min/max/step for a range,
but param_range() casts through float(), so a schema spelling its bounds "0"/"1"
and one spelling them "0.0"/"1.0" print identically -- while the materialised
grid differs, and the capability gates compare exactly those strings
(`lock_values == {"0", "1"}`). A single decimal-spelled bound removes a control
and left no trace anywhere. Small grids now carry their values.

The bound is what keeps the standing rule intact: a setpoint range is still never
enumerated, because the point count is computed arithmetically from min/max/step
and `.values` is never read for a grid over the cap. A test proves that by making
`.values` raise.

Deliberately NOT changed: the gates themselves. They compare strings, which is
the trap described above, but for this reporter they are not the cause -- the
same funnel feeds the fan, both selects and both timing numbers, and all of those
exist on his unit, so his schema is spelled with integers and the gate passes.
Fixing the comparison without also canonicalising the write path would be worse
than the trap: `_checked_value` validates an outgoing value against the same set,
so the switch would exist and refuse every write.

Mutation evidence: restoring the silent `continue` fails 4 tests, emitting the
grid unconditionally fails 3, never emitting it fails 3.
…dline

The whole setup used to run under one 60s cap. A cold sign-in alone is nine
sequential round-trips, and each appliance adds more, so a working but slow
account blew the cap and surfaced as a network timeout it never was.

Budgets are now per phase and share one absolute deadline through a ContextVar
stack of the live asyncio.Timeout scopes. A sign-in scope SUSPENDS the scopes it
interrupts -- it pushes their deadline out on entry and gives the unused
remainder back on exit -- so a lazy login nested inside a caller's scope can no
longer be killed by the shorter budget wrapping it.

Two kinds of number, kept apart on purpose: scope budgets (own work,
suspendable) and caps awaited across the thread boundary on a
concurrent.futures.Future (not suspendable, so each must contain a sign-in).
…ally

ADDHON-400 could not tell a slow sign-in from a slow appliance list: the lazy
login runs inside the load_appliances phase, so every login timeout was filed
against the list.

phase_timeout_code now resolves composed phases leaf first, so
load_appliances/auth/refresh reports 406 and load_appliances/auth reports 405
while every flat phase resolves exactly as before. 480 covers the call that was
still in flight when the dedicated loop was torn down.

classify() gains a structural layer -- exception type and response status are
read before the text of the message, which stays as the fallback it always
should have been.
…'s deadline

A single network blip during validation was a final error: the login path had no
retry at all, while the runtime poll had three attempts.

Retry is by explicit inclusion and covers only the five round-trips that can be
replayed safely; the steps that mint an OTP or advance the Salesforce session
are deliberately left out, so a retry can never multiply a verification code.
The delay is fixed, not exponential.

The gate measures the deadline of the scope actually in force rather than
rebuilding one from AUTH_FULL, so a retry cannot be the reason its own budget
expires. Each auth scope suspends its caller.
The watchdog read the phase after cancelling and draining the task, by which
point the scope had already unwound and the mirror was empty, so it fell back to
the flat phase and reported ADDHON-400 again. It now samples the phase before
scheduling the cancellation, and keeps the real cause when the coroutine loses
the race with the watchdog.

Since 3.11 concurrent.futures.TimeoutError IS TimeoutError, so the cap expiry can
no longer be told from a timeout raised BY the task by type. It is now told apart
by state, through future.done().

A transport fault on one appliance no longer fails the whole entry: the appliance
is queued for rehydration and reloaded before the first poll decides which
entities exist, and a second failure is not contained a second time. Statistics
move to the first poll, which reloads them anyway. MQTT startup gets the scope
its budget was already summed into, and the wait no longer holds the lifecycle
lock across the thread boundary.
Download Diagnostics now carries a per-phase ledger of duration and outcome, so
the next report like #76 says which phase burned the time without needing a
live probe.

The log line used to read "Validation failed [ADDHON-400]: ADDHON-400: ..." --
the label was formatted next to an error whose message already began with it.
The code appears once now, in the config flow and in the ConfigEntryNotReady and
UpdateFailed messages alike.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @tis24dev, your pull request is larger than the review limit of 150000 diff characters

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Setup resilience and diagnostics

Layer / File(s) Summary
Budgets, phases, retries, and classification
custom_components/addhon/client/budget.py, custom_components/addhon/client/phase.py, custom_components/addhon/client/transport/retry.py, custom_components/addhon/error_codes.py
Adds shared timeout budgets, nested phase tracking, bounded retry policies, transport classification, timeout codes, and representative failure selection.
Authentication and transport integration
custom_components/addhon/client/transport/auth.py, custom_components/addhon/client/transport/connection.py, custom_components/addhon/client/transport/mqtt.py
Propagates phase tracking through authentication and connection flows. Applies scoped budgets to authentication, refresh, MFA, and MQTT phase updates.
Setup, watchdogs, and rehydration
custom_components/addhon/client/session.py, custom_components/addhon/hon_client.py, custom_components/addhon/client/engine/appliance.py
Adds setup ledgers, appliance degradation and rehydration, per-operation watchdogs, first-poll command recovery, and normalized setup failures.
Diagnostics and user-facing errors
custom_components/addhon/diagnostics.py, custom_components/addhon/__init__.py, custom_components/addhon/config_flow.py, custom_components/addhon/switch.py, custom_components/addhon/translations/*, custom_components/addhon/manifest.json
Adds model metadata and phase diagnostics, bounds range values, sanitizes error details, logs purifier gate decisions, adds timeout translations, and updates the version to 5.12.0.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Suggested reviewers: telard-pixel

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the v5.12.0 release, which matches the primary objective and manifest version change.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Reviewer's Guide

Refactors the hOn client’s timeout, phase attribution, and transport error handling to introduce per-phase budgets and hierarchical phases for better diagnostics and resilience (issue #76), adds model/catalogue metadata and richer diagnostics, tightens retry semantics, and updates tests and manifest for v5.12.0.

Sequence diagram for lazy sign-in with hierarchical phases and shared budgets

sequenceDiagram
    participant HC as HonClient
    participant NH as NativeHon
    participant CN as HonConnection
    participant HA as HonAuth
    participant PM as BudgetModule
    participant PT as PhaseTracker
    participant RB as RetryBudget

    HC->>NH: setup_sync()
    NH->>NH: setup()
    NH->>PT: PhaseTracker()
    NH->>CN: create(phase_tracker)
    CN->>HA: HonAuth(..., phase_tracker)

    NH->>CN: request needing auth
    CN->>PT: phase("auth", phase_tracker)
    CN->>PM: budgeted(AUTH_FULL, suspends_caller=True)
    activate PM
    PM-->>RB: current_deadline()
    RB-->>HA: RetryBudget(deadline)

    HA->>HA: authenticate()
    HA->>PT: _phase("introduce") -> step("introduce")
    HA->>PM: retry_transport(budget, "introduce", _introduce)
    HA->>PT: _phase("redirects") -> step("redirects")
    HA->>PM: retry_transport(budget, "manual_redirect", _manual_redirect)
    HA->>PT: _phase("login_page") -> step("login_page")
    HA->>PM: retry_transport(budget, "login_page", _open_login_page)
    HA->>PT: _phase("api_auth") -> step("api_auth")
    HA->>PM: retry_transport(budget, "api_auth", _api_auth)

    PM-->>CN: budgeted scope completes
    deactivate PM
    PT-->>CN: current_phase == "auth/..."
    CN-->>NH: tokens ready
    NH-->>HC: setup complete or coded timeout via HonCodedError
Loading

File-Level Changes

Change Details Files
Introduce per-phase budgets, hierarchical phase tracking, and improved timeout/teardown behavior in the Hon client and transport stack to fix misattribution and hanging setup/auth flows.
  • Add client.budget module defining per-phase budgets, retry coupling, and scoped timeouts with suspension semantics, and wire caps into HonClient call sites (setup, commands, teardown, polling).
  • Add client.phase module and PhaseTracker to provide hierarchical phase context with a leak-free per-phase ledger; use this across NativeHon, connection, auth, MQTT, and HonClient watchdog logging.
  • Rewrite HonClient._run_on_hon_loop to accept per-call timeouts, distinguish coroutine-raised timeouts from watchdog expiries, preserve real causes, handle teardown cancellation as CLIENT_SHUTDOWN, and log watchdog events with phase and ledger.
  • Refine NativeHon.setup/appliance creation to use hierarchical phases, budgets for appliance list/appliance loads/MQTT start, and add partial-hydration tracking with needs_rehydration, representative failure re-raising, and degraded_appliances exposure.
  • Update transport.connection/auth to thread PhaseTracker through, bound auth/refresh/MFA flows with budgets that suspend callers, add a shared RetryBudget for idempotent auth steps, and ensure 2FA flows share the same watchdog behavior as setup.
  • Adjust HonClient.setup_sync/MFA methods/close_sync/dispatch/update paths to pass the appropriate budget caps, record last_phase_ledger/last_error_phase from errors instead of flat auth mirror, and propagate phases through first-poll wrappers.
custom_components/addhon/client/budget.py
custom_components/addhon/client/phase.py
custom_components/addhon/hon_client.py
custom_components/addhon/client/session.py
custom_components/addhon/client/transport/connection.py
custom_components/addhon/client/transport/auth.py
custom_components/addhon/client/transport/mqtt.py
custom_components/addhon/client/transport/retry.py
Improve error classification, diagnostics, and logging so timeouts, transport faults, MFA flows, and setup failures surface precise, non-duplicated codes, phases, and phase ledgers.
  • Extend error_codes with auth-specific timeout codes, CLIENT_SHUTDOWN, structural transport classification (TLS/DNS/refused), phase-timeout segment mapping, error_detail helper, and move representative_failure into error_codes for reuse.
  • Update init and config_flow to use error_detail so error codes appear exactly once in HA-facing messages, and ensure CannotConnect/InvalidAuth surface the refined codes (including auth/refresh timeouts).
  • Enhance diagnostics to include model_attributes from applianceModel.attributes, bounded materialised range values for small grids, a leak-proof last_error.phase_ledger, and ensure redaction/missing surfaces are handled safely.
  • Update switch air purifier entity logic to log capability/state rejections with redacted IDs and full capability sets, rather than silently skipping controls.
custom_components/addhon/error_codes.py
custom_components/addhon/__init__.py
custom_components/addhon/config_flow.py
custom_components/addhon/diagnostics.py
custom_components/addhon/switch.py
Broaden and adjust test coverage to lock in the new timeout/budget/phase semantics, diagnostics content, and error behavior, and align existing tests with the new _run_on_hon_loop signature.
  • Add extensive new test modules for setup budgets, auth retry policy, hierarchical phase contexts, and adjust aiohttp contract tests to pin structural attributes used by the classifier.
  • Extend native session, hon_client realtime, diagnostics, engine appliance root, coordinator resilience, auth error classification, config flow error code, MQTT transport, air purifier entities, and error_codes tests to cover partial hydration, rehydration, hierarchical phases, ledger behavior, logging content, and new error codes.
  • Update existing tests and helpers to pass the new timeout parameter to _run_on_hon_loop and to assert new behaviors such as non-blocking lifecycle locks, teardown semantics, and no-duplication of error codes in user-facing messages.
  • Bump manifest version from 5.11.0 to 5.12.0 and ensure translations files are present (no content changes shown).
tests/test_setup_budgets.py
tests/test_auth_retry_policy.py
tests/test_phase_context.py
tests/_aiohttp_contract.py
tests/test_hon_client_realtime.py
tests/test_native_session.py
tests/test_diagnostics.py
tests/test_engine_appliance_root.py
tests/test_coordinator_resilience.py
tests/test_error_codes.py
tests/test_transport_mqtt.py
tests/test_air_purifier_entities.py
tests/test_config_flow_error_codes.py
tests/test_auth_error_classification.py
tests/test_program_select.py
custom_components/addhon/manifest.json
custom_components/addhon/translations/en.json
custom_components/addhon/translations/it.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (7)
custom_components/addhon/hon_client.py (2)

801-808: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Read the session from one attribute, and fix the stale method name in the comment.

Two issues in this pair:

  • _needs_rehydration reads self._api, while the phase scope on Line 847 reads self._hon_instance. In production both hold the same NativeHon object, because NativeHon.__aenter__ returns self and setup_sync assigns that to self._api. The split is not a defect today, but it makes the rehydration path depend on two names for one object. tests/test_hon_client_realtime.py Line 508 already sets only _api, so the test silently passes tracker=None.
  • The comment on Line 836 names NativeHon._build_appliance. The method in custom_components/addhon/client/session.py is _create_appliance. A reader following the reference finds nothing.
♻️ Proposed change
-        Under the SAME scope pair `NativeHon._build_appliance` opens for
+        Under the SAME scope pair `NativeHon._create_appliance` opens for

Also applies to: 836-848

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@custom_components/addhon/hon_client.py` around lines 801 - 808, Update
_needs_rehydration and the surrounding rehydration phase to use a single session
attribute consistently, matching the attribute already initialized by setup and
used by the tests, instead of mixing _api with _hon_instance. Correct the nearby
comment to reference NativeHon._create_appliance rather than the stale
_build_appliance name.

516-522: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

After a drain timeout, drained is read while the loop thread can still write it.

drain_future.result(timeout=self._CANCEL_TIMEOUT) establishes the happens-before for the normal path. If that wait expires, the except Exception on Line 519 swallows it and Line 522 reads drained while _drain_task may still be running on the loop thread. The dict operations are individually atomic, so nothing corrupts, but the recovered cause can be missed or read mid-handoff, and the attribution silently degrades to the synthetic phase timeout.

The path is already degraded, so this is optional hardening. Capture the drain outcome in a local before the read, and record whether the drain completed.

♻️ Optional hardening
+            drain_completed = True
             try:
                 loop.call_soon_threadsafe(_cancel_and_drain)
                 drain_future.result(timeout=self._CANCEL_TIMEOUT)
             except Exception as err:
+                drain_completed = False
                 _LOGGER.debug("Timeout while cancelling hOn task: %s", err)
             elapsed = round(time.monotonic() - started, 1)
-            original = drained.get("error")
+            original = drained.get("error") if drain_completed else None
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@custom_components/addhon/hon_client.py` around lines 516 - 522, Update the
cancellation flow around _cancel_and_drain and drain_future.result to capture
the drain wait outcome in a local variable and track whether draining completed,
including when the timeout/exception path runs. Use that completion state when
reading drained so the recovered error attribution does not assume the
loop-thread handoff finished after a timeout.

Source: Linters/SAST tools

tests/test_native_session.py (1)

608-608: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use an async stub for _make_mqtt.

lambda hon: None replaces an async def method. This test passes only because setup() raises before it reaches await self._make_mqtt(). The same pattern appears on Lines 633 and 676. If the abort condition ever changes, the await fails with "object NoneType can't be used in 'await' expression" and the test reports the wrong cause. Line 710 in this file already uses the correct form.

♻️ Proposed change (apply to Lines 608, 633, and 676)
+        async def no_mqtt(hon):
+            return None
+
         self._patch(factory, "create_appliance", fake_create_appliance)
-        self._patch(NativeHon, "_make_mqtt", lambda hon: None)
+        self._patch(NativeHon, "_make_mqtt", no_mqtt)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_native_session.py` at line 608, Replace the synchronous lambda
stubs for NativeHon._make_mqtt in the tests at the referenced setup points with
async stubs matching the existing pattern used near line 710. Update all three
occurrences, including those around lines 608, 633, and 676, so awaiting
_make_mqtt remains valid if setup proceeds.
tests/test_hon_client_realtime.py (1)

177-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Close the event loop in the cleanup.

_stop_hon_loop closes the loop in production, but this helper only stops it and joins the thread. Six tests in this class use the helper, so the run leaks one unclosed loop each and can emit ResourceWarning: unclosed event loop.

♻️ Proposed cleanup
         def _stop() -> None:
             loop = client._hon_loop
             if loop is not None:
                 loop.call_soon_threadsafe(loop.stop)
             thread = client._hon_thread
             if thread is not None:
                 thread.join(timeout=5)
+            if loop is not None and not loop.is_closed():
+                loop.close()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_hon_client_realtime.py` around lines 177 - 190, Update the _client
cleanup helper’s _stop function to close the client’s event loop after stopping
it and joining the thread, matching _stop_hon_loop’s production cleanup
behavior. Ensure the loop is closed only when it exists and preserve the
existing thread shutdown flow.
tests/test_auth_retry_policy.py (1)

159-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Narrow the expected exception type.

assertRaises(BaseException) passes on any failure, including an AttributeError or ImportError from a broken double. This file states that the non-regression half is the valuable half, so the assertion should pin the injected error. All three callers inject asyncio.TimeoutError, and every non-retried step lets it propagate. The call-count assertion below still guards the delivery-once rule.

♻️ Proposed change
     def _step_is_delivered_once(self, step: str) -> None:
         index = _STEP_INDEX[step]
         session = _FlakySession(_happy_responses(), index, asyncio.TimeoutError())
         auth = _auth(session)
-        with self.assertRaises(BaseException):
+        with self.assertRaises(asyncio.TimeoutError):
             asyncio.run(auth.authenticate())
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_auth_retry_policy.py` around lines 159 - 167, Update
_step_is_delivered_once to assert asyncio.TimeoutError specifically instead of
BaseException, while preserving the existing authenticate invocation, call-count
verification, and sleeper-delay assertion.

Source: Linters/SAST tools

tests/test_setup_budgets.py (1)

663-683: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Widen the timing margin in this test.

The suspended deadlines land at about 0.85s and 0.80s against about 0.70s of elapsed time. That leaves a margin of roughly 0.10s. A loaded CI runner can exceed it and fail the test for a scheduling delay rather than a stack regression. The sibling tests in this class keep a margin of about 2x. Scale the five numbers up so the margin matches, or express the assertion against a factor of the sleeps.

♻️ Proposed change
-        # 0.60 outer / 0.55 middle; a sign-in budgeted 0.30 that really takes 0.25, then
-        # 0.45s of the caller's own work. Suspended properly both enclosing deadlines end
-        # at ~0.85/0.80 against 0.70 elapsed; with only the parent suspended the outer one
-        # still sits at 0.60 and fires.
-        async with budgeted(0.60):
+        # 1.20 outer / 1.10 middle; a sign-in budgeted 0.60 that really takes 0.50, then
+        # 0.90s of the caller's own work. Suspended properly both enclosing deadlines end
+        # at ~1.70/1.60 against 1.40 elapsed; with only the parent suspended the outer one
+        # still sits at 1.20 and fires.
+        async with budgeted(1.20):
             self.assertEqual(1, len(budget._ACTIVE.get()))
-            async with budgeted(0.55):
+            async with budgeted(1.10):
                 # The shape, stated directly: the enclosing scope is still on the stack.
                 self.assertEqual(2, len(budget._ACTIVE.get()))
-                async with budgeted(0.30, suspends_caller=True):
-                    await asyncio.sleep(0.25)
-                await asyncio.sleep(0.45)
+                async with budgeted(0.60, suspends_caller=True):
+                    await asyncio.sleep(0.50)
+                await asyncio.sleep(0.90)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_setup_budgets.py` around lines 663 - 683, Widen the timing margin
in test_a_sign_in_suspends_every_enclosing_scope_not_just_its_parent by scaling
the five budget and sleep durations proportionally, preserving the same ordering
and suspension behavior while increasing the gap between the expected suspended
deadlines and elapsed runtime to roughly 2x.
custom_components/addhon/client/transport/mqtt.py (1)

210-224: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Log the suppressed failures instead of passing silently.

Both blocks swallow every exception. If hon._phase_tracker is missing or step() fails, the MQTT connect/subscribe refinement is lost and no line records it. That is the unfalsifiable-report class this PR removes elsewhere. Add a debug log to each handler. This also clears the Ruff S110/BLE001 findings.

♻️ Proposed change
             try:
                 hon._setup_phase = phase
-            except Exception:  # pragma: no cover - defensive
-                pass
+            except Exception:  # noqa: BLE001 - defensive, never fatal
+                _LOGGER.debug("MQTT: could not record the flat setup phase", exc_info=True)
             try:
                 # The HIERARCHICAL mirror too (client/phase.py). `NativeHon.setup()`
                 # wraps this call in a `phase("mqtt_start")` scope for the MQTT_START
                 # budget, and that scope is what the cross-thread watchdog reads --
                 # without the refinement the outer name would SHIELD the flat mirror
                 # above and cost the connect/subscribe distinction. `step()` only
                 # rewrites the mirror (no ContextVar push), and the enclosing scope
                 # restores it on exit, so a refinement cannot outlive the start.
                 hon._phase_tracker.step(phase)
-            except Exception:  # pragma: no cover - defensive
-                pass
+            except Exception:  # noqa: BLE001 - defensive, never fatal
+                _LOGGER.debug("MQTT: could not refine the hierarchical phase mirror", exc_info=True)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@custom_components/addhon/client/transport/mqtt.py` around lines 210 - 224,
Update both defensive exception handlers around hon._setup_phase and
hon._phase_tracker.step(phase) to log the caught exception at debug level
instead of silently passing; include enough context to identify whether updating
the setup phase or phase tracker failed, while preserving the existing
best-effort flow.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@custom_components/addhon/hon_client.py`:
- Around line 1145-1153: Update the all-failed HonCodedError raise in
async_get_appliances_data to forward phase=getattr(cause, "phase", None) from
the _representative_failure result, matching the existing first-poll wrapper.
Preserve the existing error code, message, and cause chaining.

In `@tests/test_hon_client_realtime.py`:
- Around line 323-378: Correct
test_a_teardown_gives_the_in_flight_caller_a_coded_error by removing or revising
the assertion that the concurrent.futures.CancelledError cause is not an
asyncio.CancelledError. Since both names resolve to the same class on supported
Python versions, assert the expected cancellation type without claiming they are
distinct, while preserving the HonCodedError and CLIENT_SHUTDOWN checks.

---

Nitpick comments:
In `@custom_components/addhon/client/transport/mqtt.py`:
- Around line 210-224: Update both defensive exception handlers around
hon._setup_phase and hon._phase_tracker.step(phase) to log the caught exception
at debug level instead of silently passing; include enough context to identify
whether updating the setup phase or phase tracker failed, while preserving the
existing best-effort flow.

In `@custom_components/addhon/hon_client.py`:
- Around line 801-808: Update _needs_rehydration and the surrounding rehydration
phase to use a single session attribute consistently, matching the attribute
already initialized by setup and used by the tests, instead of mixing _api with
_hon_instance. Correct the nearby comment to reference
NativeHon._create_appliance rather than the stale _build_appliance name.
- Around line 516-522: Update the cancellation flow around _cancel_and_drain and
drain_future.result to capture the drain wait outcome in a local variable and
track whether draining completed, including when the timeout/exception path
runs. Use that completion state when reading drained so the recovered error
attribution does not assume the loop-thread handoff finished after a timeout.

In `@tests/test_auth_retry_policy.py`:
- Around line 159-167: Update _step_is_delivered_once to assert
asyncio.TimeoutError specifically instead of BaseException, while preserving the
existing authenticate invocation, call-count verification, and sleeper-delay
assertion.

In `@tests/test_hon_client_realtime.py`:
- Around line 177-190: Update the _client cleanup helper’s _stop function to
close the client’s event loop after stopping it and joining the thread, matching
_stop_hon_loop’s production cleanup behavior. Ensure the loop is closed only
when it exists and preserve the existing thread shutdown flow.

In `@tests/test_native_session.py`:
- Line 608: Replace the synchronous lambda stubs for NativeHon._make_mqtt in the
tests at the referenced setup points with async stubs matching the existing
pattern used near line 710. Update all three occurrences, including those around
lines 608, 633, and 676, so awaiting _make_mqtt remains valid if setup proceeds.

In `@tests/test_setup_budgets.py`:
- Around line 663-683: Widen the timing margin in
test_a_sign_in_suspends_every_enclosing_scope_not_just_its_parent by scaling the
five budget and sleep durations proportionally, preserving the same ordering and
suspension behavior while increasing the gap between the expected suspended
deadlines and elapsed runtime to roughly 2x.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0ec20790-6853-4ef3-b5b7-dda46ae37457

📥 Commits

Reviewing files that changed from the base of the PR and between c5e1113 and 62d64e5.

📒 Files selected for processing (32)
  • custom_components/addhon/__init__.py
  • custom_components/addhon/client/budget.py
  • custom_components/addhon/client/engine/appliance.py
  • custom_components/addhon/client/phase.py
  • custom_components/addhon/client/session.py
  • custom_components/addhon/client/transport/auth.py
  • custom_components/addhon/client/transport/connection.py
  • custom_components/addhon/client/transport/mqtt.py
  • custom_components/addhon/client/transport/retry.py
  • custom_components/addhon/config_flow.py
  • custom_components/addhon/diagnostics.py
  • custom_components/addhon/error_codes.py
  • custom_components/addhon/hon_client.py
  • custom_components/addhon/manifest.json
  • custom_components/addhon/switch.py
  • custom_components/addhon/translations/en.json
  • custom_components/addhon/translations/it.json
  • tests/_aiohttp_contract.py
  • tests/test_air_purifier_entities.py
  • tests/test_auth_error_classification.py
  • tests/test_auth_retry_policy.py
  • tests/test_config_flow_error_codes.py
  • tests/test_coordinator_resilience.py
  • tests/test_diagnostics.py
  • tests/test_engine_appliance_root.py
  • tests/test_error_codes.py
  • tests/test_hon_client_realtime.py
  • tests/test_native_session.py
  • tests/test_phase_context.py
  • tests/test_program_select.py
  • tests/test_setup_budgets.py
  • tests/test_transport_mqtt.py

Comment thread custom_components/addhon/hon_client.py
Comment thread tests/test_hon_client_realtime.py
…pper

A steady-state cycle where every appliance fails wraps the representative cause
in a HonCodedError, and __init__.py reads `phase` off THAT object without ever
walking the __cause__ chain. The wrapper passed no phase, so Download
Diagnostics filed phase=null for causes that knew exactly where they died --
the first-poll twin already forwarded it.

Also read the phase tracker from the same session attribute the rehydration
guard asks (both names hold the one NativeHon), and correct a comment that
pointed at NativeHon._build_appliance, which does not exist; the method is
_create_appliance.
@tis24dev
tis24dev merged commit 5a8ad75 into main Aug 11, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant