chore: retro-review PR #86 — feat(providers): add OpenElectricity wholesale-price client (Phase 7 PR-2)

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
 
+### Added
+
+- OpenElectricity v4 wholesale-price client module at `custom_components/pricehawk/providers/openelectricity.py`. Standalone — not yet wired into the coordinator or config flow; that's PR-2 part 2 (Plan 07-02b). Pinned `openelectricity>=0.10.1,<0.11` in manifest. Includes CC BY-NC 4.0 attribution on every result, 30s `asyncio.timeout` bound, `ConfigEntryAuthFailed` mapping for 401, distinct 429 rate-limit handling that preserves the last-good cache, and `ConfigEntryNotReady` fallback for missing-SDK installs. API key never appears in `__repr__` or log messages (scrubber). (Phase 7 / PR-2)
+
 ### Changed
 
 - Internal: typed runtime data via `PriceHawkData` dataclass + `PriceHawkConfigEntry` alias; `entry.runtime_data` replaces `hass.data[DOMAIN][entry_id]` for coordinator storage. Service handlers (`analyze_csv`, `backfill_history`, `rank_alternatives`) re-resolve the coordinator on every invocation, eliminating a stale-closure bug latent across `OptionsFlowWithReload` cycles. Unload re-ordered: `async_unload_platforms` runs first, coordinator teardown only on success. Multi-entry service deregistration now sourced from `hass.config_entries.async_entries(DOMAIN)`. No user-facing change. (Phase 7 / PR-1)

DECISIONS.md

@@ -5,6 +5,32 @@
 
 <!-- Add new decisions at the top -->
 
+## 2026-05-20 — Phase 7 Plan 02 (OpenElectricity wholesale-price client)
+
+### D-P7-5 — Adopt `openelectricity` SDK (>=0.10.1,<0.11) as the primary wholesale-price source
+**Decision:** Pin `openelectricity>=0.10.1,<0.11` in `manifest.json:requirements`. Introduce `custom_components/pricehawk/providers/openelectricity.py` exposing `OpenElectricityPriceSource` async client + `WholesalePrice` frozen dataclass. Standalone module — no coordinator/config-flow wiring in this PR (deferred to 07-02b).
+**Rationale:** PR-2 from `PriceHawk v2 — Deep Research Round 2 (Scope-Corrected).md` (Wave 1). OpenElectricity v4 is the right primary wholesale-price source per research §1.1–1.5: official Python SDK (`AsyncOEClient`), 5-minute interval, JSON envelope, CC BY-NC 4.0 license keeps PriceHawk non-commercial. Minor-bounded pin per research §1.4 ("currently under active development").
+**Alternatives:** Stay on `aemo_api.py` (NEMWeb-only, no WEM, brittle CSV-in-ZIP, HTTP deprecation 2026-04-07). OpenNEM v3 (deprecated). jxeeno community endpoints (no SLA).
+**Consequences:** New external dependency; HA wheel resolver picks it up. 07-02b wires into coordinator. 07-03 keeps NEMWeb as no-API-key fallback. CC BY-NC attribution surfacing becomes a cross-PR contract (see D-P7-7).
+
+### D-P7-6 — Lazy SDK import + ConfigEntryNotReady on ImportError
+**Decision:** External SDK imports (`from openelectricity import AsyncOEClient`) live INSIDE the async fetch method, wrapped in `try/except ImportError` that re-raises as `homeassistant.exceptions.ConfigEntryNotReady`. NOT at module top.
+**Rationale:** Module-top imports of HACS-resolved dependencies have two failure modes: (a) test environments mocking the SDK via `sys.modules` would crash at conftest collection, and (b) a partial HACS install crashes the integration module with a hard `ImportError` that HA does NOT recognise as a retry signal — permanent error state until HA restart. `ConfigEntryNotReady` is HA's "try again later" exception with exponential backoff.
+**Alternatives:** Module-top import (rejected). Module-top with `_HAS_SDK` flag (pollutes every call site).
+**Consequences:** Reusable pattern for any future external-SDK provider. Phase 7/8 PRs introducing new SDKs MUST follow this pattern. Slight per-call overhead vs single import but production-correct.
+
+### D-P7-7 — API key handling: `__repr__` redaction + `_scrub()` log filter (CWE-532 stance)
+**Decision:** Any class that owns an API key MUST: (a) override `__repr__` so the key never appears in repr output (use `<redacted>` marker); (b) provide a `_scrub(text)` helper that replaces the full key AND any 8+ char prefix with redaction markers, and call `_scrub()` on EVERY string passed to `_LOGGER.warning/info/debug` that originated from an external dependency. Tests MUST include a `caplog`-based assertion that a leaky SDK exception (one whose `str()` contains the API key) does NOT leak through the integration's log output.
+**Rationale:** CWE-532 (Information Exposure Through Log Files) is a baseline SOC-2 concern. HTTP-client libraries commonly include request URLs, headers, or token fragments in exception messages. Passing raw SDK exceptions to `_LOGGER` leaks the key to `home-assistant.log` and downstream log streams. Audit Gap M1 on 07-02-AUDIT.md.
+**Alternatives:** Trust the SDK (SDKs change). HA's `async_redact_data` (that's for diagnostics platform output, not arbitrary log lines). Global log-filter regex (would need to know the key format ahead of time).
+**Consequences:** Pattern is template-able for existing Amber/LocalVolts/Flow Power providers (separate follow-up — their log paths not yet audited). Future external-API integrations MUST follow as part of their PR. ~10 lines per provider; saves a CWE-532 finding per provider.
+
+### D-P7-8 — Every external-SDK call MUST be bounded by `asyncio.timeout(N)`
+**Decision:** Wrap every `await sdk_client.method(...)` in `async with asyncio.timeout(_TIMEOUT_SECONDS):`. Define `_TIMEOUT_SECONDS` as a module-level `Final[float]` constant per provider (default 30.0 for HTTP-backed SDKs). On `asyncio.TimeoutError` return None + WARNING log naming the timeout.
+**Rationale:** HA's DataUpdateCoordinator eventually times out the whole tick on a hung provider, but the diagnostic signal is misleading (looks like a coordinator bug, not a provider hang). Bounding at the provider level produces a specific WARNING log line. SDK internal defaults are undocumented and can drift between minor versions. Audit Gap M2 on 07-02-AUDIT.md.
+**Alternatives:** Trust SDK default (undocumented + drift risk). HA coordinator timeout (wrong diagnostic signal). `aiohttp.ClientTimeout` (SDK doesn't expose the session).
+**Consequences:** Every external-SDK call site includes an `asyncio.timeout` wrapper. Hung connection → None result + WARNING, not HA-wide stall. Existing `aemo_api.py` uses custom retry logic — bringing under same pattern is a future cleanup.
+
 ## 2026-05-20 — Phase 7 Plan 01 (typed runtime data)
 
 ### D-P7-1 — Adopt `PriceHawkConfigEntry = ConfigEntry[PriceHawkData]` typed-entry alias

custom_components/pricehawk/manifest.json

@@ -9,6 +9,6 @@
   "integration_type": "service",
   "iot_class": "cloud_polling",
   "issue_tracker": "https://github.com/Artic0din/ha-pricehawk/issues",
-  "requirements": [],
+  "requirements": ["openelectricity>=0.10.1,<0.11"],
   "version": "1.5.0-beta.2"
 }

custom_components/pricehawk/providers/openelectricity.py

@@ -0,0 +1,275 @@
+"""OpenElectricity v4 wholesale-price client for PriceHawk (Phase 7 / PR-2).
+
+Standalone module — not yet wired into the coordinator or config flow; that's
+Plan 07-02b. The openelectricity SDK is imported lazily inside
+``fetch_current_price`` so a missing-SDK install raises ``ConfigEntryNotReady``
+(HA retries setup) rather than crashing module import.
+
+Audit invariants (07-02-AUDIT.md):
+- API key NEVER appears in __repr__ output or log messages.
+- SDK call bounded by _FETCH_TIMEOUT_SECONDS via asyncio.timeout.
+- Missing SDK at runtime → ConfigEntryNotReady, not ImportError.
+- 401-equivalent → ConfigEntryAuthFailed (caller must re-prompt for key).
+- 429-equivalent → return None + WARNING (does NOT raise AuthFailed).
+- Other errors → return None + WARNING; last-good cache preserved.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import logging
+from dataclasses import dataclass
+from datetime import datetime, timezone
+from typing import Final, TypeAlias
+
+from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady
+
+_LOGGER = logging.getLogger(__name__)
+
+_ATTRIBUTION: Final[str] = (
+    "Wholesale price data: Open Electricity (Superpower Institute), CC BY-NC 4.0"
+)
+
+# Region → network_code mapping (research §1.2). The SDK accepts "NEM", "WEM", "AU".
+_NEM_REGIONS: Final[frozenset[str]] = frozenset({"NSW1", "QLD1", "SA1", "TAS1", "VIC1"})
+_WEM_REGIONS: Final[frozenset[str]] = frozenset({"WEM"})
+
+# Audit M2: bounded SDK call. 30s is generous for a 5-min cadence; HA polling
+# tolerates slow ticks but not hung ones.
+_FETCH_TIMEOUT_SECONDS: Final[float] = 30.0
+
+_REDACTED: Final[str] = "<redacted-api-key>"
+_REDACTED_PREFIX: Final[str] = "<redacted-prefix>"
+
+
+@dataclass(slots=True, frozen=True)
+class WholesalePrice:
+    """Result type for fetch_current_price().
+
+    IMPORTANT — do NOT add fields without coordinated update of consumers
+    in 07-02b and any subsequent PRs. frozen=True is part of the cross-PR
+    contract (07-02-PLAN.md > CROSS-PR CONTRACTS).
+    """
+
+    price_aud_per_mwh: float
+    interval_end_utc: datetime
+    region: str
+    attribution: str = _ATTRIBUTION
+
+
+_LatestPoint: TypeAlias = tuple[float, datetime]
+
+
+class OpenElectricityPriceSource:
+    """Async client for OpenElectricity v4 wholesale-price queries.
+
+    Owns auth (API key passed at construction, never logged), region→network_code
+    mapping, last-good cache, and error→exception mapping. Does NOT own polling
+    cadence — the caller decides when to invoke fetch_current_price.
+    """
+
+    def __init__(self, api_key: str) -> None:
+        if not api_key:
+            raise ValueError("api_key must be a non-empty string")
+        self._api_key = api_key
+        self._last_good_by_region: dict[str, WholesalePrice] = {}
+
+    def __repr__(self) -> str:
+        # Audit M1: API key MUST NOT appear in repr.
+        cached_regions = sorted(self._last_good_by_region.keys())
+        return (
+            f"OpenElectricityPriceSource(api_key={_REDACTED}, "
+            f"cached_regions={cached_regions!r})"
+        )
+
+    @staticmethod
+    def _network_code_for(region: str) -> str:
+        if region in _NEM_REGIONS:
+            return "NEM"
+        if region in _WEM_REGIONS:
+            return "WEM"
+        raise ValueError(
+            f"Unknown OpenElectricity region {region!r}; "
+            f"expected one of {sorted(_NEM_REGIONS | _WEM_REGIONS)}"
+        )
+
+    def _scrub(self, text: str) -> str:
+        """Redact the API key from any string before logging it (audit M1)."""
+        if not text:
+            return text
+        scrubbed = text.replace(self._api_key, _REDACTED)
+        if len(self._api_key) >= 8:
+            scrubbed = scrubbed.replace(self._api_key[:8], _REDACTED_PREFIX)
+        return scrubbed
+
+    async def fetch_current_price(self, region: str) -> WholesalePrice | None:
+        """Fetch the latest 5-minute dispatch price for the region.
+
+        Returns None on non-auth, non-rate-limit errors and empty-data responses;
+        caller can fall back to last-good via ``last_good(region)``.
+
+        Raises:
+            ValueError: unknown region (caller bug — raised before any network call).
+            ConfigEntryAuthFailed: HTTP 401 / auth-equivalent — API key needs renewal.
+            ConfigEntryNotReady: openelectricity SDK is not importable at runtime.
+        """
+        # Validate region BEFORE any network call.
+        network_code = self._network_code_for(region)
+
+        # Audit M3: lazy import + ImportError → ConfigEntryNotReady.
+        try:
+            from openelectricity import AsyncOEClient  # noqa: PLC0415
+            from openelectricity.types import MarketMetric  # noqa: PLC0415
+        except ImportError as exc:
+            raise ConfigEntryNotReady(
+                "openelectricity SDK is not installed. The HA wheel resolver "
+                "should pick it up from manifest.json:requirements; if this "
+                "persists, install manually: "
+                "pip install 'openelectricity>=0.10.1,<0.11'. "
+                f"({exc})"
+            ) from exc
+
+        # Audit M2: bounded SDK call.
+        try:
+            async with asyncio.timeout(_FETCH_TIMEOUT_SECONDS):
+                async with AsyncOEClient(api_key=self._api_key) as client:
+                    response = await client.get_market(
+                        network_code=network_code,  # type: ignore[arg-type]
+                        metrics=[MarketMetric.PRICE],
+                        interval="5m",
+                        primary_grouping="network_region",
+                    )
+        except asyncio.TimeoutError:
+            _LOGGER.warning(
+                "OpenElectricity fetch timed out after %.0fs for region %s",
+                _FETCH_TIMEOUT_SECONDS,
+                region,
+            )
+            return None
+        except Exception as exc:  # noqa: BLE001 — classified below
+            if _is_auth_error(exc):
+                raise ConfigEntryAuthFailed(
+                    f"OpenElectricity API rejected the key (HTTP 401): "
+                    f"{self._scrub(str(exc))}"
+                ) from exc
+            if _is_rate_limit_error(exc):
+                _LOGGER.warning(
+                    "OpenElectricity rate-limited fetch for region %s; "
+                    "preserving cached last-good. Detail: %s",
+                    region,
+                    self._scrub(str(exc)),
+                )
+                return None
+            _LOGGER.warning(
+                "OpenElectricity fetch failed for %s: %s",
+                region,
+                self._scrub(str(exc)),
+            )
+            return None
+
+        extracted = _extract_latest_for_region(response, region)
+        if extracted is None:
+            _LOGGER.warning(
+                "OpenElectricity returned no data for region %s "
+                "(response had %d point(s))",
+                region,
+                _record_count(response),
+            )
+            return None
+
+        price_value, ts_utc = extracted
+        price = WholesalePrice(
+            price_aud_per_mwh=price_value,
+            interval_end_utc=ts_utc,
+            region=region,
+        )
+        self._last_good_by_region[region] = price
+        return price
+
+    def last_good(self, region: str) -> WholesalePrice | None:
+        """Return the cached last successful fetch for the region, if any."""
+        return self._last_good_by_region.get(region)
+
+
+def _is_auth_error(exc: Exception) -> bool:
+    """Detect 401-equivalent errors. Belt-and-braces: message + class name (audit S1)."""
+    msg = str(exc).lower()
+    if (
+        "401" in msg
+        or "unauthor" in msg
+        or "invalid api key" in msg
+        or "forbidden" in msg
+    ):
+        return True
+    class_name = type(exc).__name__.lower()
+    return any(
+        token in class_name
+        for token in ("auth", "unauthor", "forbidden", "credential")
+    )
+
+
+def _is_rate_limit_error(exc: Exception) -> bool:
+    """Detect 429-equivalent errors (audit S2)."""
+    msg = str(exc).lower()
+    if (
+        "429" in msg
+        or "rate limit" in msg
+        or "rate-limit" in msg
+        or "too many requests" in msg
+    ):
+        return True
+    class_name = type(exc).__name__.lower()
+    return "ratelimit" in class_name or "throttle" in class_name
+
+
+def _extract_latest_for_region(
+    response: object, region: str
+) -> _LatestPoint | None:
+    """Walk the nested TimeSeriesResponse and pull the latest (price, ts) for `region`.
+
+    Verified against openelectricity==0.10.1 (2026-05-20). Real shape:
+        response.data: Sequence[NetworkTimeSeries]
+        NetworkTimeSeries.results: list[TimeSeriesResult]
+        TimeSeriesResult.columns.network_region: str
+        TimeSeriesResult.data: list[<point with .timestamp + .value>]
+
+    Does NOT use response.to_records() — that flattens but emits naive-local
+    timestamps; we need tz-aware UTC.
+    """
+    data = getattr(response, "data", None)
+    if not data:
+        return None
+
+    candidates: list[_LatestPoint] = []
+    for series in data:
+        for result in getattr(series, "results", []):
+            columns = getattr(result, "columns", None)
+            row_region = (
+                getattr(columns, "network_region", None) if columns else None
+            )
+            if row_region != region:
+                continue
+            for point in getattr(result, "data", []):
+                ts = getattr(point, "timestamp", None)
+                val = getattr(point, "value", None)
+                if ts is None or val is None:
+                    continue
+                if ts.tzinfo is None:
+                    ts = ts.replace(tzinfo=timezone.utc)
+                candidates.append((float(val), ts))
+
+    if not candidates:
+        return None
+    return max(candidates, key=lambda pair: pair[1])
+
+
+def _record_count(response: object) -> int:
+    """Helper for log messages — total data points across all series/results."""
+    data = getattr(response, "data", None)
+    if not data:
+        return 0
+    return sum(
+        len(getattr(result, "data", []))
+        for series in data
+        for result in getattr(series, "results", [])
+    )

tests/conftest.py

@@ -58,6 +58,10 @@ def __init__(self, *args, **kwargs):
 _mods["homeassistant.exceptions"].ConfigEntryNotReady = type(
     "ConfigEntryNotReady", (Exception,), {}
 )
+# Phase 7 PR-2: ConfigEntryAuthFailed for OpenElectricity 401 mapping
+_mods["homeassistant.exceptions"].ConfigEntryAuthFailed = type(
+    "ConfigEntryAuthFailed", (Exception,), {}
+)
 _mods["homeassistant"].exceptions = _mods["homeassistant.exceptions"]
 
 for name, mod in _mods.items():