chore: full code review — bugs, error handling, tech debt, docs, tests, features

[eman]

Summary

Full code review pass covering correctness, error handling, performance, technical debt, documentation sync, and new features. All 31 identified issues addressed.

Before → After: 182 tests → 226 tests (+44), 82.7% → 84.5% coverage, ruff/mypy/docs all green.

---

Phase 1 — Critical Bug Fixes

• Truthiness drops valid zeros — Systemic or None / if value patterns were silently dropping legitimate 0/0.0 values (temperatures, signal strength, humidity, setpoints) across 8+ model files. Replaced with explicit is not None checks and proto-sentinel-aware guards.
• AUTO mode setpoint ordering — Deadband clamp (cool - heat >= 2.5°C) was applied after setpoint routing, so the clamped value was never used. Fixed ordering.
• Settings bool coercion — bool("false") is True. Replaced with isinstance(uf, bool) guard so only actual booleans are accepted.
• Stale client cache on re-login — login() didn't clear _system_id, _system_name, or _snapshot_cache, causing stale data to be returned for a new session.
• Auth bare except Exception — Narrowed to QuiltAuthError | ClientError so programming errors inside refresh hooks are no longer silently swallowed.
• Stream reconnect — Added subscription lock and logging for safe reconnect; explained why queue reset is safe (_topics is the source of truth).

Phase 2 — Error Handling

• grpc_call() context manager (services/__init__.py) — Shared translator that maps AioRpcError to QuiltConnectionError (transient) or QuiltError (other), and wraps unexpected non-gRPC exceptions. Applied to system.py as demonstration; other services can adopt incrementally.
• CLI friendly errors — _run() now catches QuiltAuthError / QuiltError and prints formatted messages instead of stack traces.
• CLI login — Catches QuiltAuthError specifically (not all exceptions) on silent-login attempt.
• CLI set-space — Invalid HVAC mode now shows valid options instead of KeyError.
• QuiltStreamError re-exported from quilt_hp.__init__.

Phase 3 — Documentation Sync

• Fixed stale dict-based SystemSnapshot examples in 4 doc files → list iteration + apply_*() methods.
• Documented 15+ previously undocumented public exports (enums, energy, sensors, comfort types).
• Documented all NotifierStream callback and lifecycle methods (on_outdoor_unit_update, subscribe, run_forever, etc.).
• Documented QuiltClient.close() and get_current_token().

Phase 4 — Technical Debt

• Assert → guards — All assert self._hds is not None in client.py replaced with explicit QuiltError raises (asserts are disabled with -O).
• Snapshot resolution dedup — Extracted _resolve_system_id() and _resolve_snapshot_item() helpers in client.py.
• CLI boilerplate — Centralized repeated login/snapshot setup.
• models/_helpers.py — Extracted shared lookup_hardware() and WiFi-info parsing from controller.py, outdoor_unit.py, qsm.py.
• Atomic file writes — cli/store.py token writes now use temp file + os.replace() to prevent corruption on crash.
• HVACMode enum — ScheduleEvent.hvac_mode changed from raw int to HVACMode.
• MetricBucketStatus enum — Added to enums.py; used in energy.py and services/system.py.
• close() — Now clears _hds, _sysinfo, _user_svc to prevent accidental reuse.

Phase 5 — Performance & New Features

• Structured logging — logging.getLogger(__name__) added to auth, transport, client, all services, and store. DEBUG/INFO/WARNING at appropriate callsites. No secrets logged.
• Connection health — NotifierStream.is_connected, last_event_at, stream_state properties for building health dashboards.
• Stream debouncing — debounce_s parameter on NotifierStream (and exposed via QuiltClient.stream()) coalesces rapid events to reduce redundant processing.
• Retry policies — grpc_call(max_retries=N, retry_delay=1.0, retry_backoff=2.0) adds opt-in exponential backoff for transient errors. Default max_retries=0 preserves current behavior.
• Signature cache — inspect.signature() result in transport interceptor is cached after first call.

Phase 6 — Test Coverage (+44 tests)

New test file	What it covers
test_models_extra.py	SoftwareUpdateInfo, EnergyBucket, ComfortSetting, Controller, OutdoorUnit, RemoteSensor from_proto()
test_hds_payloads.py	gRPC payload contract for HEAT/COOL/AUTO/STANDBY + deadband clamp
test_streaming_health.py	is_connected, last_event_at, stream_state
test_grpc_retry.py	Transient error retry with backoff
test_streaming_debounce.py	Event coalescing under rapid updates
test_streaming_concurrency.py	Concurrent start/stop/subscribe races
conftest.py	Shared _ns, fake_space, fake_snapshot fixtures

---

Checklist

• ruff check src/ tests/ — clean
• ruff format --check src/ tests/ — clean
• mypy src/quilt_hp/ — clean (43 files)
• pytest — 226 passed, 84.46% coverage
• mkdocs build --strict — clean

[Copilot]

Pull request overview

This PR is a broad correctness + resiliency pass across the async Quilt client library, focused on fixing proto “zero value” handling, improving gRPC error translation/retry behavior, hardening streaming reconnect/concurrency, expanding structured logging, and syncing docs/tests to the updated public API.

Changes:

• Introduces a shared grpc_call() context manager with optional transient-error retries; applies it to SystemInformationService.
• Refactors NotifierStream for safer reconnect/subscription concurrency and adds stream health + debouncing features.
• Fixes multiple model conversion edge cases (especially “0” preservation), expands docs, and adds extensive new test coverage.

Reviewed changes

Copilot reviewed 46 out of 46 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tests/test_transport_interceptor_extra.py	Adds coverage for refresh-callback signature caching behavior.
tests/test_streaming.py	Adds regression test for resubscription behavior after queue reset.
tests/test_streaming_reconnect_dispatch_extra.py	Validates reconnect queue reset logging + behavior.
tests/test_streaming_health.py	New tests for stream health properties and lifecycle state exposure.
tests/test_streaming_debounce.py	New tests for stream update debouncing/coalescing.
tests/test_streaming_concurrency.py	New tests for start/stop/subscribe race handling.
tests/test_models_from_proto.py	Expands model conversion tests, especially for “zero value preserved” cases.
tests/test_models_extra.py	New model conversion coverage for additional entities and edge cases.
tests/test_hds_payloads.py	Validates HDS UpdateSpace payload shaping (including AUTO deadband clamp).
tests/test_grpc_retry.py	Tests grpc_call() translation + retry/backoff behavior.
tests/test_client_service_error_paths.py	Tests client “requires login” guards and close() cleanup.
tests/test_cli_surfaces_extra.py	Verifies CLI surfaces friendly auth/general error handling.
tests/test_cli_login.py	Aligns CLI login test to raise QuiltAuthError for OTP-required path.
tests/test_auth_store_settings_edges.py	Adjusts settings-store expectation for boolean coercion.
tests/conftest.py	Adds shared fixtures/helpers (_ns, _make_header, fake snapshot/space).
src/quilt_hp/transport.py	Adds logging + refresh callback signature caching.
src/quilt_hp/services/user.py	Adds debug logging around user RPCs.
src/quilt_hp/services/system.py	Uses grpc_call() for error translation and wraps energy status in enum.
src/quilt_hp/services/streaming.py	Major streaming refactor: locks, reconnect behavior, debouncing, health properties.
src/quilt_hp/services/hds.py	Adds structured RPC logging and fixes AUTO deadband clamp ordering.
src/quilt_hp/services/init.py	Adds grpc_call() helper with exception translation and optional retries.
src/quilt_hp/models/space.py	Fixes setpoint display logic to avoid dropping valid 0 values.
src/quilt_hp/models/sensor.py	Preserves explicit zero values when parsing sensor state.
src/quilt_hp/models/schedule.py	Changes ScheduleEvent.hvac_mode to HVACMode enum.
src/quilt_hp/models/qsm.py	Uses shared WiFi parsing helper to preserve zeros.
src/quilt_hp/models/outdoor_unit.py	Uses shared hardware lookup; improves performance_data presence detection.
src/quilt_hp/models/indoor_unit.py	Fixes louver-mode parsing to avoid truthiness dropping valid zeros.
src/quilt_hp/models/enums.py	Introduces MetricBucketStatus enum.
src/quilt_hp/models/energy.py	Switches bucket status to enum; adds validity helpers.
src/quilt_hp/models/controller.py	Uses shared hardware lookup + WiFi parsing that preserves zero signal.
src/quilt_hp/models/_helpers.py	New shared helpers for hardware lookup and WiFi parsing.
src/quilt_hp/models/init.py	Re-exports MetricBucketStatus.
src/quilt_hp/client.py	Adds logging; replaces asserts with guards; refactors snapshot/system resolution; adds debounce option.
src/quilt_hp/cli/store.py	Adds logging and atomic token writes.
src/quilt_hp/cli/settings.py	Fixes boolean coercion for use_fahrenheit.
src/quilt_hp/cli/main.py	Centralizes login/snapshot boilerplate and adds friendly error handling.
src/quilt_hp/auth.py	Narrows exception handling; adds structured logging and safer ExpiresIn handling.
src/quilt_hp/init.py	Re-exports QuiltStreamError.
docs/reference/models.md	Updates model and streaming docs to match list-based snapshot + new callbacks.
docs/reference/client.md	Updates lifecycle + documents close() and get_current_token().
docs/how-to/tui-app.md	Updates TUI example to merge stream diffs and handle fatal stream error.
docs/how-to/stream-updates.md	Updates streaming how-to with additional callbacks and lifecycle patterns.
docs/how-to/control-spaces.md	Fixes snapshot list indexing in examples.
docs/how-to/configure-schedules.md	Updates schedule examples for new model structure.
docs/how-to/configure-comfort-settings.md	Updates comfort-setting examples for list-based snapshot.
docs/how-to/automation-daemon.md	Updates daemon example to apply stream diffs and align fields with new models.

[Copilot]

Pull request overview

Copilot reviewed 46 out of 46 changed files in this pull request and generated 3 comments.