refactor: preserve ordered isinstance dispatch

[tykeal]

Summary

Refactors the two ordered type-dispatch sites reported by ai-slop/python-isinstance-ladder without changing observable behavior.

Closes #656.

Proposed change

Coordinator _kmlocks_to_dict before:

• Dataclass fields were serialized inline.
• datetime was converted first, then the resulting value continued into the separate time / list / dict / scalar chain.

Coordinator _kmlocks_to_dict after:

• Field conversion is delegated to _kmlock_field_to_dict().
• Temporal conversion is isolated in _kmlock_temporal_value_to_dict() and still uses the same two sequential checks, so a datetime is converted to an ISO string before collection/scalar handling sees it.
• List, dict, and scalar serialization are isolated in _kmlock_collection_value_to_dict() with the same recursive behavior as before.

Zigbee2MQTT async_get_usercodes before:

• Gathered slot query results were handled inline with CodeSlot, HomeAssistantError, Exception, then BaseException checks.

Zigbee2MQTT async_get_usercodes after:

• Per-result handling is delegated to _handle_usercode_query_result().
• match class patterns keep the same most-specific-first order: HomeAssistantError is matched before generic Exception, and BaseException remains last and is re-raised.

Ordering subtleties preserved:

• Coordinator: the original sequential-if reassignment is preserved by running temporal conversion first, then collection/scalar conversion. A datetime still becomes an ISO string and is not treated as time, list, or dict.
• Zigbee2MQTT: exception subclass dispatch remains ordered by specificity, so HomeAssistantError takes the warning path, generic Exception takes the error path, and BaseException propagates.

Validation:

• tox -e py314: 1186 passed, 3 skipped, 1 deselected; coverage 95.02%.
• tox -e lint: passed.
• Pristine aislop scan: total findings 34 → 32; ai-slop/python-isinstance-ladder 2 → 0; ai-slop/python-chained-dict-get remains 2.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New feature (which adds functionality)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes aislop: ai-slop/python-isinstance-ladder in coordinator and zigbee2mqtt #656
• This PR is related to issue: aislop: ai-slop/python-isinstance-ladder in coordinator and zigbee2mqtt #656

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.02%. Comparing base (cdb4922) to head (5969c4d).
⚠️ Report is 311 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #773       +/-   ##
===========================================
+ Coverage   84.14%   95.02%   +10.87%     
===========================================
  Files          10       35       +25     
  Lines         801     5606     +4805     
===========================================
+ Hits          674     5327     +4653     
- Misses        127      279      +152     

Flag	Coverage Δ
python	95.02% <100.00%> (?)

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

[Copilot]

🟢 Approval recommended

The refactor preserves the original dispatch ordering and behavior, and the added tests directly assert the intended semantics for both modified sites.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[secondof9]

📋 Review Summary

Tip

Review Status: 🟢 APPROVED
Change Type: 🛠️ Refactor (ordered-dispatch extraction)
Review Effort: 🟡 Medium
Core Impact: Splits two isinstance ladders in coordinator.py and zigbee2mqtt.py into named helper methods, preserving the original most-specific-first dispatch order, and adds regression tests for both paths. No behavior change; a match/class-pattern block replaces the elif chain in the Z2M provider.

---

🚦 CI & Pipeline Health Summary

Check / Workflow Name	Status	Impact on Review
Pytest (3.14)	✅ PASSED	1186 passed, 3 skipped, 1 deselected; coverage 95.02%.
coverage	✅ PASSED	Codecov confirms all modified lines covered; 95.02% project coverage.
Prek	✅ PASSED	Pre-commit / lint clean.
HACS Validation	✅ PASSED	No integration manifest regression.
Hassfest Validation	✅ PASSED	No manifest regression.
Autolabel PR	✅ PASSED	Labeled correctly.
Autolabel PR (re-run)	⏩ SKIPPED	Duplicate run, benign.

Note

CI Pipeline Clear: All 7 GitHub Actions workflows completed successfully. No failures, no in-progress runs, and the only skipped run is a benign duplicate Autolabel PR rerun.

---

🔍 Architectural Walkthrough

Coordinator: dataclass field serialization helper extraction

• custom_components/keymaster/coordinator.py
  • The inline isinstance ladder inside _kmlocks_to_dict (datetime → time → list → dict → scalar) is extracted into three named helpers: _kmlock_field_to_dict (composes the two stages), _kmlock_temporal_value_to_dict (preserves the sequential datetime-then-time reassignment), and _kmlock_collection_value_to_dict (preserves the list/dict recursive behavior with hasattr(item, "__dataclass_fields__")).
  • Semantic preservation is correct: temporal conversion runs to completion (rewriting field_value to an ISO str in stage 1) before collection handling runs in stage 2, so a datetime is never re-classified as a list/dict. The original sequential if … if … pattern was also non-short-circuiting across types (each test was independent on a value that may have been reassigned), and the helper split preserves that exact semantics by reassigning the same field_value reference between the two stages.
  • The class is a KeymasterCoordinator and these helpers are bound self methods; the existing coordinator_for_dict fixture in the tests is a perfectly valid call site, and the new test exercises it directly without mocking.

Zigbee2MQTT provider: ordered exception-dispatch extraction

• custom_components/keymaster/providers/zigbee2mqtt.py
  • The per-result isinstance chain in async_get_usercodes is extracted into _handle_usercode_query_result(slot_num, res). The new implementation uses match/case class patterns, which match in source order — first match wins — and class patterns use isinstance semantics, so a HomeAssistantError will still match the HomeAssistantError case before it could match a generic Exception case.
  • Ordering is preserved exactly: CodeSlot → HomeAssistantError (warning) → Exception (error) → BaseException (re-raise). The BaseException arm must remain last in the source so that Exception subclasses are caught by the more specific Exception case; this matches the prior elif ordering and the prior control-flow guarantee that BaseException propagates only after Exception has had its chance to claim it.
  • match here is genuinely the right tool: the case BaseException(): raise res form is more self-documenting than the original elif isinstance(res, BaseException): raise res, and avoids a common reader-pitfall where someone "fixes" the raise res to raise (which would lose the original exception).
  • asyncio.gather(..., return_exceptions=True) is unchanged; results are still drained in lockstep with zip(missing_slots, results, strict=False), so cancellation/reordering semantics are untouched.

Regression tests

• tests/test_coordinator.py
  • test_kmlocks_to_dict_preserves_ordered_field_serialization exercises every branch the helper split could have perturbed in one go: datetime → ISO str, time → ISO str, nested-dataclass list items → recursed dicts, plain list items preserved, nested-dataclass dict values → recursed dicts, plain dict values preserved, and scalar passthrough. The test correctly asserts isinstance(result["timestamp"], str) so a future regression that leaves the datetime un-converted would fail.
• tests/providers/test_zigbee2mqtt.py
  • test_get_usercodes_preserves_ordered_query_result_handling feeds a six-element side_effect covering success, expected error, unexpected error, and a re-raise path, then asserts both cache state and log-level routing. The test uses record.getMessage() (not record.message), which is robust against Formatter mutating the rendered msg attribute — a nit from the prior review round has been addressed. Log messages are filtered by levelno rather than substring matching alone, so a regression that flips warning ↔ error for HomeAssistantError would still trip the level-based assertion.

---

Architectural / Defensive Notes (non-blocking)

• aislop/python-chained-dict-get remains at 2 findings per the PR body. That is a separate, pre-existing class of aislop finding and is explicitly out of scope for this refactor — the PR is scoped to isinstance ladders only. No action required.
• The _handle_usercode_query_result signature uses CodeSlot | BaseException as a union; this is accurate (a successful result is a CodeSlot, and a failure result is a BaseException because asyncio.gather(..., return_exceptions=True) returns the full BaseException tree), but a future maintainer might appreciate a TypeAlias like type _SlotResult = CodeSlot | BaseException if this is reused. Not a blocker.
• The match block does not include a default case _: arm. That is correct here because the type union is exhaustive (CodeSlot | BaseException covers every value gather(return_exceptions=True) can yield), and an unhandled arm would silently swallow KeyboardInterrupt / SystemExit — a BaseException arm must re-raise, not pass. No change recommended.

---

✅ Approval Rationale

• All 7 CI checks pass — Pytest (3.14), coverage, Prek, HACS, Hassfest, and the two Autolabel PR runs (one benign skip).
• Ordering semantics are preserved — both refactors map the original sequential if/elif chain onto helpers that run the same checks in the same order against the same field_value reference (coordinator) and onto a match block whose class-pattern source order is documented to be "first match wins" with isinstance semantics (Z2M provider).
• Regression tests cover the ordering — the coordinator test exercises temporal, list, dict, and scalar paths in one fixture; the Z2M test exercises the warning vs error vs re-raise branching, and uses record.getMessage() (the prior nit is addressed).
• No new async / event-loop hazards — neither refactor touches the event loop, executor boundaries, or signal/cleanup paths. asyncio.gather(return_exceptions=True) is preserved verbatim.
• No new dict-key traps — the refactor works on field_value references that the loop already extracted via getattr; the new helpers do not introduce new key lookups against upstream payloads.
• No defensive-data concerns — this is an in-process serializer; it does not consume external API data.

No inline comments are necessary for an approved refactor of this nature.