Conversation
…Program) AC models whose `settings` command has no onOffStatus (e.g. AD71S2SM3FA(H)) drive power/mode via startProgram/stopProgram, not via onOffStatus/machMode in `settings`. The climate entity only ever wrote into `settings`, so on/off and mode commands raised "Parameter(s) not found" before the request reached the cloud and every command looked ignored; temp/fan were sent but had no visible effect while the unit was off. Route the write path by capability: when `settings` lacks onOffStatus and a startProgram command exists, send OFF via stopProgram, a concrete mode via startProgram with the mapped iot_<mode> program (AC_PROGRAM_MAP; FAN_ONLY maps to iot_fan, not iot_fan_only), and turn_on via iot_simple_start. The program is capability-gated against the device's live startProgram enum and raises program_not_supported when absent (no silent fallback to the settings path). Temperature and fan stay on the settings path (tempSel/windSpeed exist on `settings` in both models). Settings-based models (AS35PBPHRA-PRE) are unchanged; the read path is untouched. Reuses program_options.async_send_program (category-swap aware) for startProgram and a generic stopProgram send. Adds program-based write-path tests plus an AS35-style regression guard that keeps settings-based on/off/mode even when startProgram/stopProgram also exist. Full suite green (1023 passed, 1 skipped).
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Reviewer's GuideRelease v5.8.0 extends addhon’s program/command engine, transport auth, diagnostics, and HA entities with robustness fixes, program-based AC write-path support, safer favourite/program handling, better rollback on failed sends, and more resilient options/debug handling, plus tests for all new behaviours. Sequence diagram for program-based vs settings-based AC write pathsequenceDiagram
actor User
participant HomeAssistant as HomeAssistant
participant ClimateEntity as AddhonClimate
participant HonClient as HonClient
participant HonAPI as HonAPI
User->>HomeAssistant: change HVACMode / turn_on
HomeAssistant->>ClimateEntity: async_set_hvac_mode(hvac_mode)
ClimateEntity->>ClimateEntity: _is_program_based()
alt program_based & hvac_mode == OFF
ClimateEntity->>HonClient: async_send_command(HonAPI, stopProgram)
else program_based & hvac_mode != OFF
ClimateEntity->>ClimateEntity: _program_for_mode(hvac_mode)
ClimateEntity->>HonClient: async_send_program(HonAPI, program_code)
else settings_based & hvac_mode == OFF
ClimateEntity->>HonClient: _send_command_in_executor(onOffStatus="0")
else settings_based & hvac_mode != OFF
ClimateEntity->>HonClient: _send_command_in_executor(onOffStatus="1", machMode)
end
User->>HomeAssistant: turn_on
HomeAssistant->>ClimateEntity: async_turn_on()
ClimateEntity->>ClimateEntity: _is_program_based()
alt program_based
ClimateEntity->>ClimateEntity: _assert_program_available(AC_PROGRAM_SIMPLE_START)
ClimateEntity->>HonClient: async_send_program(HonAPI, AC_PROGRAM_SIMPLE_START)
else settings_based
ClimateEntity->>HonClient: _send_command_in_executor(onOffStatus="1")
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR hardens the hOn integration across several areas: debug-toggle baseline caching, availability/coordinator-data normalization, command/parameter copy isolation with trigger-safe rule application, native auth/connection re-auth single-flight and cookie-clear fixes, AC program-based write routing, config-flow reauth guard, diagnostics redaction/schema fixes, program-select label disambiguation, and rollback-on-failure for button/switch commands, plus corresponding tests and translations. ChangesCore reliability fixes and AC program-based control
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant RequestA
participant RequestB
participant HonConnection
participant Auth
RequestA->>HonConnection: 401/403 at loop 1
HonConnection->>HonConnection: _reauth_after_rejection(gen)
HonConnection->>Auth: create()/authenticate()
RequestB->>HonConnection: 401/403 same gen
HonConnection-->>RequestB: reuse cached result/error
sequenceDiagram
participant HonProgramCommandButton
participant Appliance
participant Command
HonProgramCommandButton->>HonProgramCommandButton: snapshot params pre-swap
HonProgramCommandButton->>Appliance: swap active command
HonProgramCommandButton->>HonProgramCommandButton: snapshot params post-swap
HonProgramCommandButton->>Command: send()
Command-->>HonProgramCommandButton: raises exception
HonProgramCommandButton->>Appliance: restore original command
HonProgramCommandButton->>HonProgramCommandButton: restore parameter snapshots
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The per-module rollback logic that snapshots and restores parameter
__dict__(button, switch) is quite ad hoc and duplicated; consider centralizing this into a reusable helper on the parameter/command to avoid subtle state mismatches and make future rollback paths easier to maintain. - In
HonCommand.__copy__, you rely oncopy(param)plus manual rebinding forHonParameterProgram; if other parameter subclasses hold back-references or mutable internal structures, they may still leak changes from favourites into the base command, so it may be safer to expose an explicitclone()on parameters instead of a genericcopy(). - The
_is_program_basedgate inclimate.HaierClimateEntityassumes that the absence ofonOffStatusand presence ofstartProgramuniquely identifies program-based models; if hOn adds hybrid or variant models, this heuristic may misroute writes, so it could be worth making the capability detection pluggable or keyed off a more explicit model flag.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The per-module rollback logic that snapshots and restores parameter `__dict__` (button, switch) is quite ad hoc and duplicated; consider centralizing this into a reusable helper on the parameter/command to avoid subtle state mismatches and make future rollback paths easier to maintain.
- In `HonCommand.__copy__`, you rely on `copy(param)` plus manual rebinding for `HonParameterProgram`; if other parameter subclasses hold back-references or mutable internal structures, they may still leak changes from favourites into the base command, so it may be safer to expose an explicit `clone()` on parameters instead of a generic `copy()`.
- The `_is_program_based` gate in `climate.HaierClimateEntity` assumes that the absence of `onOffStatus` and presence of `startProgram` uniquely identifies program-based models; if hOn adds hybrid or variant models, this heuristic may misroute writes, so it could be worth making the capability detection pluggable or keyed off a more explicit model flag.
## Individual Comments
### Comment 1
<location path="tests/test_ac_write_path.py" line_range="653-568" />
<code_context>
+ async def test_program_send_failure_raises_command_error(self) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the failure-path test by asserting that settings/stop commands are not touched when startProgram send fails
In `test_program_send_failure_raises_command_error`, we only assert the translation key, `start.send_calls == 1`, and `coordinator.refreshes == 0`. This failure path should also ensure that no `settings` or `stopProgram` commands are sent. Please add assertions like `self.assertEqual(0, settings.send_calls)` and `self.assertEqual(0, stop.send_calls)` so the test more clearly verifies that the rollback path prevents partial writes to these commands.
Suggested implementation:
```python
self.assertEqual(1, start.send_calls)
self.assertEqual(0, settings.send_calls)
self.assertEqual(0, stop.send_calls)
self.assertEqual(0, coordinator.refreshes)
```
This change assumes that `settings`, `start`, `stop`, and `coordinator` are all defined within `test_program_send_failure_raises_command_error` as shown in your snippet, and that the existing assertions for `start.send_calls` and `coordinator.refreshes` are exactly as in the SEARCH block. If the assertions differ slightly (e.g., using `assertEqual` with reversed arguments or different spacing), adjust the SEARCH text to match the exact existing lines so the replacement applies correctly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| entity, settings, start, stop, coord = _program_climate() | ||
| await entity.async_turn_on() | ||
| # turn_on = resume last mode -> iot_simple_start on startProgram, NOT settings. | ||
| self.assertEqual(1, start.send_calls) |
There was a problem hiding this comment.
suggestion (testing): Strengthen the failure-path test by asserting that settings/stop commands are not touched when startProgram send fails
In test_program_send_failure_raises_command_error, we only assert the translation key, start.send_calls == 1, and coordinator.refreshes == 0. This failure path should also ensure that no settings or stopProgram commands are sent. Please add assertions like self.assertEqual(0, settings.send_calls) and self.assertEqual(0, stop.send_calls) so the test more clearly verifies that the rollback path prevents partial writes to these commands.
Suggested implementation:
self.assertEqual(1, start.send_calls)
self.assertEqual(0, settings.send_calls)
self.assertEqual(0, stop.send_calls)
self.assertEqual(0, coordinator.refreshes)This change assumes that settings, start, stop, and coordinator are all defined within test_program_send_failure_raises_command_error as shown in your snippet, and that the existing assertions for start.send_calls and coordinator.refreshes are exactly as in the SEARCH block. If the assertions differ slightly (e.g., using assertEqual with reversed arguments or different spacing), adjust the SEARCH text to match the exact existing lines so the replacement applies correctly.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
custom_components/addhon/sensor.py (1)
837-838: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated
coordinator.datadict-guard pattern across platforms.The same
data_map = coordinator.data if isinstance(coordinator.data, dict) else {}guard is duplicated identically inbinary_sensor.py,select.py, andsensor.py. Consider extracting a small shared helper (e.g. inbase_entity.pyor a utils module) to avoid drift if the guard logic ever needs to change.🤖 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/sensor.py` around lines 837 - 838, The repeated coordinator.data dict-guard is duplicated across the platform entity setup loops, so extract that logic into a shared helper in base_entity.py or a small utils module and have sensor.py, binary_sensor.py, and select.py call it instead of inlining the isinstance check. Keep the helper focused on returning a safe dict from coordinator.data, and update the setup code that iterates appliance_id/data to use the shared function so the behavior stays consistent across platforms.custom_components/addhon/select.py (1)
325-341: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueLabel disambiguation and current_option routing look correct.
Collision-suffixing on unique codes keeps
_program_reverseinjective, and bothcurrent_optionbranches (pending at Line 468, token-in-map at Lines 514-516) now consistently resolve through_program_display, matching the new_attr_options.One narrow edge case: if a device attribute reports a bare colliding label via
programName(Line 517'stoken in self._program_reversecheck), it won't match since reverse-map keys are now suffixed for collisions — it falls through to the next candidate key. This is self-recovering (other keys likeprCode/programresolve via the token-in-map branch), so not blocking.Also applies to: 462-531
🤖 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/select.py` around lines 325 - 341, Handle the edge case where current_option receives a bare colliding program label from programName, since _program_reverse now only stores suffixed display labels for collisions. Update HonProgramOptionSelect’s lookup logic in current_option to normalize through _program_display/_program_reverse so a raw label can still resolve to the correct code before falling back to other identifiers like prCode or program.custom_components/addhon/diagnostics.py (1)
218-239: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRange-first ordering correctly avoids the enumerated-grid dump.
Good fix — checking
param_range()beforeparam_values()prevents the expensive/dangerous grid enumeration for range params, and is covered by the new test. Consider updating the docstring at Line 219 ("value + enum + range + metadata") to reflect that enum is now only emitted as a fallback when no range exists, since the current wording still implies both are always considered independently.🤖 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/diagnostics.py` around lines 218 - 239, The range-first logic in _param_schema is correct, but the docstring still implies value, enum, and range are always emitted together. Update the _param_schema docstring and nearby comment so it reflects the actual behavior: range is checked first, and enum is only added as a fallback when param_range() returns None. Keep the explanation aligned with _param_schema, param_range(), and param_values().custom_components/addhon/client/transport/connection.py (1)
195-206: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDon't cache
CancelledErrorfor sibling reauth flows.
except BaseExceptionalso catchesasyncio.CancelledError, so if one request is cancelled duringcreate()/authenticate(), that instance gets cached and later re-raised into siblings that were never cancelled. CatchCancelledErrorseparately: advance the generation, but re-raise it without storing it in_reauth_error.🤖 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/connection.py` around lines 195 - 206, The reauth retry block in connection handling is caching cancellations as shared auth failures because it catches BaseException in the create()/authenticate() path. Update the logic in the reauth flow around self.create(), self.auth.authenticate(), and the _reauth_error/_reauth_error_gen assignment so asyncio.CancelledError is handled separately: still bump _refresh_gen and advance the generation, but re-raise it immediately without storing it in _reauth_error. Keep the existing caching behavior only for non-cancellation exceptions so sibling requests do not inherit a cancelled task.custom_components/addhon/button.py (2)
154-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the snapshot/restore rollback helper into a shared module.
The same
dict(p.__dict__)snapshot /__dict__.clear()+update()restore pattern is reimplemented independently here, inswitch.py's_send_pause_command, and (per the comment on line 160) inhon_commands.async_send_command. Consolidating into one shared helper (e.g.engine/rollback.py) would reduce the risk of the three copies drifting apart on future edge-case fixes.♻️ Sketch of a shared helper
# e.g. custom_components/addhon/client/engine/param_rollback.py def snapshot_params(params: dict) -> dict: if not isinstance(params, dict): return {} return {k: dict(p.__dict__) for k, p in params.items() if hasattr(p, "__dict__")} def restore_params(params: dict, snapshot: dict) -> None: if not isinstance(params, dict): return for key, saved in snapshot.items(): param = params.get(key) if param is not None and hasattr(param, "__dict__"): param.__dict__.clear() param.__dict__.update(saved)Also applies to: 191-195, 244-246, 285-285, 322-331
🤖 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/button.py` around lines 154 - 178, The rollback snapshot/restore logic in the button send flow is duplicated and should be centralized. Extract the shared `dict(p.__dict__)` snapshot and `__dict__.clear()+update()` restore behavior from this code path, `switch.py`’s `_send_pause_command`, and `hon_commands.async_send_command` into a single helper module such as `param_rollback` (or similar), then update the existing callers to use that helper so future fixes stay consistent.
154-178: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winExtract the rollback snapshot/restore helper
This logic is duplicated incustom_components/addhon/hon_commands.py,custom_components/addhon/switch.py, andcustom_components/addhon/program_options.py; centralizing it would keep rollback behavior aligned across all send paths.🤖 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/button.py` around lines 154 - 178, Extract the rollback snapshot/restore logic used in the button send flow into a shared helper so it can be reused consistently by the send paths in button, hon_commands.async_send_command, switch, and program_options. Move the _snapshot_params and _restore_params behavior into a common utility, then have the existing rollback handling in button.py call that shared helper instead of keeping a local copy.
🤖 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.
Nitpick comments:
In `@custom_components/addhon/button.py`:
- Around line 154-178: The rollback snapshot/restore logic in the button send
flow is duplicated and should be centralized. Extract the shared
`dict(p.__dict__)` snapshot and `__dict__.clear()+update()` restore behavior
from this code path, `switch.py`’s `_send_pause_command`, and
`hon_commands.async_send_command` into a single helper module such as
`param_rollback` (or similar), then update the existing callers to use that
helper so future fixes stay consistent.
- Around line 154-178: Extract the rollback snapshot/restore logic used in the
button send flow into a shared helper so it can be reused consistently by the
send paths in button, hon_commands.async_send_command, switch, and
program_options. Move the _snapshot_params and _restore_params behavior into a
common utility, then have the existing rollback handling in button.py call that
shared helper instead of keeping a local copy.
In `@custom_components/addhon/client/transport/connection.py`:
- Around line 195-206: The reauth retry block in connection handling is caching
cancellations as shared auth failures because it catches BaseException in the
create()/authenticate() path. Update the logic in the reauth flow around
self.create(), self.auth.authenticate(), and the _reauth_error/_reauth_error_gen
assignment so asyncio.CancelledError is handled separately: still bump
_refresh_gen and advance the generation, but re-raise it immediately without
storing it in _reauth_error. Keep the existing caching behavior only for
non-cancellation exceptions so sibling requests do not inherit a cancelled task.
In `@custom_components/addhon/diagnostics.py`:
- Around line 218-239: The range-first logic in _param_schema is correct, but
the docstring still implies value, enum, and range are always emitted together.
Update the _param_schema docstring and nearby comment so it reflects the actual
behavior: range is checked first, and enum is only added as a fallback when
param_range() returns None. Keep the explanation aligned with _param_schema,
param_range(), and param_values().
In `@custom_components/addhon/select.py`:
- Around line 325-341: Handle the edge case where current_option receives a bare
colliding program label from programName, since _program_reverse now only stores
suffixed display labels for collisions. Update HonProgramOptionSelect’s lookup
logic in current_option to normalize through _program_display/_program_reverse
so a raw label can still resolve to the correct code before falling back to
other identifiers like prCode or program.
In `@custom_components/addhon/sensor.py`:
- Around line 837-838: The repeated coordinator.data dict-guard is duplicated
across the platform entity setup loops, so extract that logic into a shared
helper in base_entity.py or a small utils module and have sensor.py,
binary_sensor.py, and select.py call it instead of inlining the isinstance
check. Keep the helper focused on returning a safe dict from coordinator.data,
and update the setup code that iterates appliance_id/data to use the shared
function so the behavior stays consistent across platforms.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 59726632-d533-4557-b07b-500239483e78
📒 Files selected for processing (26)
custom_components/addhon/__init__.pycustom_components/addhon/base_entity.pycustom_components/addhon/binary_sensor.pycustom_components/addhon/button.pycustom_components/addhon/client/engine/appliance.pycustom_components/addhon/client/engine/commands.pycustom_components/addhon/client/engine/parameter/base.pycustom_components/addhon/client/engine/rules.pycustom_components/addhon/client/transport/auth.pycustom_components/addhon/client/transport/connection.pycustom_components/addhon/climate.pycustom_components/addhon/config_flow.pycustom_components/addhon/const.pycustom_components/addhon/diagnostics.pycustom_components/addhon/manifest.jsoncustom_components/addhon/select.pycustom_components/addhon/sensor.pycustom_components/addhon/switch.pycustom_components/addhon/translations/en.jsoncustom_components/addhon/translations/it.jsontests/test_ac_write_path.pytests/test_diagnostics.pytests/test_engine_cluster.pytests/test_options_flow.pytests/test_program_select.pytests/test_transport_connection.py
…ering select case) - connection.py: don't cache CancelledError in the reauth single-flight. The `except BaseException` also caught asyncio.CancelledError and stored it in _reauth_error, so a cancellation on one request was re-raised into sibling requests that were never cancelled. Handle CancelledError separately: advance the generation (create() already reset auth to token-less) but re-raise without storing it. - param_rollback.py (new): shared snapshot_params/restore_params helper for the send-path rollback (copy __dict__ directly so rules are not re-fired and values/min/max are restored). Route hon_commands, button, switch and program_options through it instead of four drifting copies. - base_entity.py: shared coordinator_data_map() guard; sensor/binary_sensor/ select now call it instead of inlining the isinstance check. - diagnostics.py: _param_schema docstring now reflects range-first, enum-as- fallback ordering. Behavior-preserving refactors + one docstring + the CancelledError fix. Full suite green (1039 passed, 1 skipped). Skipped the select.py current_option nitpick: CodeRabbit itself flagged it as self-recovering and non-blocking.
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated release PR for
v5.8.0.Summary by Sourcery
Improve the addhon integration’s reliability, AC program handling, command/program execution, auth/transport behavior, diagnostics privacy, and debug options, and bump the integration to v5.8.0.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests:
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Greptile Summary
This release bundles a large set of targeted bug fixes and one new feature: explicit support for program-based AC models (e.g. AD71S2SM3FA(H)) that drive power and mode via
startProgram/stopProgramrather than thesettingscommand. All other fixes address concrete defects in auth, engine state isolation, rollback behaviour, diagnostics privacy, and platform setup guards.climate.py,const.py,program_options.py): adds_is_program_based()capability gate; routes ON/mode tostartProgramand OFF tostopProgram, while temperature and fan remain onsettings. Regression guard ensures settings-based models keep their existing path even when startProgram/stopProgram are present.connection.py): introduces_reauth_after_rejectionwith generation-keyed error caching so a burst of failing requests collapses to onecreate()+authenticate()instead of triggering N logins/OTP prompts; HTML 403s are redirected to theDECODE_ERRORtransient path.commands.py,rules.py,param_rollback.py,hon_commands.py,button.py,switch.py):HonCommand.__copy__gives favourites isolated parameter dicts and rebound trigger tables;HonRuleSet.reboundre-attaches triggers against the copy; sharedsnapshot_params/restore_paramshelpers centralise rollback for all send paths.Confidence Score: 5/5
Safe to merge — all fixes address well-defined defects, each has a targeted test, and no regressions were found across the settings-based and program-based AC paths.
The changes are surgical and backed by comprehensive tests covering the new program-based AC write path, concurrent re-auth single-flighting (including the failure case), engine favourite isolation, rollback behaviour, and diagnostics privacy. The auth cookie-clear fix and the select disambiguation are straightforward corrections with no observable side-effects on the existing flows. No unguarded mutation paths, missing rollback cases, or logic inversions were identified during review.
No files require special attention. The most complex change is connection.py's _reauth_after_rejection, which is well-commented and covered by two dedicated concurrency tests.
Important Files Changed
Comments Outside Diff (1)
custom_components/addhon/select.py, line 477-482 (link)_program_map(code → original label) still appears in the "pending code not in map" debug message. Since users now see disambiguated labels from_program_display, logging_program_displaywould make it easier to cross-reference the debug output with what the UI actually shows.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (2): Last reviewed commit: "refactor: address CodeRabbit nitpicks on..." | Re-trigger Greptile