Phase 2: close the three P0 findings from the architecture review

P0-1 — the coordinator was constructed without config_entry. Deprecated
since Home Assistant 2024.11 and scheduled to become mandatory, at which
point setup raises and the integration stops loading. Passing it also
ties the refresh task to the entry, so an unload cancels a poll that is
still in flight. The minimum supported Home Assistant therefore moves
from 2024.7 to 2024.11, in hacs.json and the README.

The hygiene test guarding that minimum compared version strings, and
"2024.11.0" sorts below "2024.7.0" as text — it would have rejected
every release from 2024.10 onwards. It now compares numbers.

P0-2 — the inventory endpoint serialised its JSON inside the request
handler, blocking the event loop in proportion to cellar size: measured
3.3 ms at 200 bottles, 13.8 ms at 1,000 and 43.9 ms at 2,500 on a
developer-class core, and several times that on the hardware Home
Assistant usually runs on, once per dashboard load. The body is now
rendered by the executor that already runs the parse, and the view
hands over bytes it never touches. The payload is unchanged, so any
dashboard or Lovelace card reading the endpoint keeps working.

P0-3 — CellarTracker takes credentials as query parameters, so the
password travels in the request URL, and aiohttp's ClientResponseError
renders that URL in both str() and repr(). Nothing printed it, but only
by accident: the error was re-raised as a bare CannotConnect and
survived as __cause__, where nothing happened to format it. A single
_LOGGER.exception, a diagnostics dump, or debug logging on
aiohttp.client would have published it.

Transport failures are now described by a string built here — the HTTP
status, or the exception's type name — and raised after the handler has
exited, so the original is attached as neither __cause__ nor __context__.
`raise ... from None` was not enough: it clears __cause__ but leaves
__context__, which a traceback would not print but a diagnostics dump
walking the chain still could.

20 tests added, 210 passing. Each new suite was confirmed to fail
against the unfixed code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLPEGFSy3fLEuXNUPAPWR4

Author: claude

README.md

@@ -135,8 +135,9 @@ The response contains all 66 columns CellarTracker returns; the table above is t
 
 ## Installation via HACS
 
-**Requires Home Assistant 2024.7 or newer** — that is the release that added the static-path API
-the integration uses to serve its dashboard page.
+**Requires Home Assistant 2024.11 or newer** — 2024.7 added the static-path API the integration
+uses to serve its dashboard page, and 2024.11 added the `config_entry` argument its data
+coordinator now passes.
 
 1. Open **HACS** in Home Assistant.
 2. Click the **⋮** menu (top right) → **Custom repositories**.

custom_components/cellar_tracker/cellar_data.py

@@ -25,6 +25,7 @@
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryAuthFailed
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
+from homeassistant.helpers.json import json_bytes
 from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 
 from .const import (
@@ -109,13 +110,32 @@ async def async_fetch_inventory_payload(hass, username: str, password: str) -> s
         "Location": "1",
     }
 
+    # The password is a query parameter, so it travels in the request URL - and
+    # aiohttp's ClientResponseError renders that URL in both str() and repr().
+    # Nothing derived from the failed request may escape this function except a
+    # description we build ourselves.
+    failure: str | None = None
+
     try:
         async with asyncio.timeout(REQUEST_TIMEOUT):
             async with session.get(BASE_URL, params=params) as response:
                 response.raise_for_status()
                 payload = await response.text()
+    except aiohttp.ClientResponseError as err:
+        # The status is the diagnostic part and carries nothing sensitive.
+        failure = f"HTTP {err.status} from CellarTracker"
     except aiohttp.ClientError as err:
-        raise CannotConnect from err
+        # Connector and payload errors name the host rather than the query
+        # string, but the same rule applies: name the failure, copy nothing.
+        failure = type(err).__name__
+
+    if failure is not None:
+        # Raised outside the handler deliberately. `raise ... from None` would
+        # clear __cause__ but leave the original on __context__, where a
+        # traceback would not print it but a diagnostics dump walking the chain
+        # still could. Once the except block has exited the exception is no
+        # longer being handled, so nothing is attached at all.
+        raise CannotConnect(failure)
 
     # An auth failure arrives as HTTP 200 with a marker in the body.
     if NOT_LOGGED_REPONSE in payload:
@@ -147,18 +167,38 @@ def __init__(self, hass: HomeAssistant, entry: ConfigEntry):
             hass,
             _LOGGER,
             name=DOMAIN,
+            # Mandatory from a future Home Assistant release, and deprecated
+            # without it since 2024.11. It also ties the refresh task to the
+            # entry, so unloading cancels a poll still in flight.
+            config_entry=entry,
             update_interval=scan_interval,
             always_update=False,
         )
 
         # Consecutive polls that reported an empty cellar after it held stock.
         self._suspicious_empty_polls = 0
 
+        # Replaced wholesale by each refresh, never mutated in place, and only
+        # one refresh runs at a time - so the loop can read it without a lock.
+        self._inventory_body: bytes = b"[]"
+
     @property
     def currency(self) -> str:
         """Return the configured currency symbol."""
         return self._currency
 
+    @property
+    def inventory_body(self) -> bytes:
+        """The bottle list as a JSON body, rendered ahead of any request.
+
+        Serialising a large cellar costs real time - tens of milliseconds at a
+        few thousand bottles, more on the hardware Home Assistant usually runs
+        on - and doing it inside a request handler spends that time on the
+        event loop. It is rendered in the executor that already runs the parse
+        instead, so the view only ever hands over bytes.
+        """
+        return self._inventory_body
+
     def _process_inventory(self, inventory: list, previous: dict | None = None) -> dict:
         """Process the raw inventory list into a structured dictionary.
 
@@ -299,4 +339,7 @@ async def _async_update_data(self) -> dict:
     def _parse_and_process(self, payload: str, previous: dict | None) -> dict:
         """Parse the tab-separated export, then summarise it. Runs in an executor."""
         rows = list(csv.DictReader(io.StringIO(payload), dialect="excel-tab"))
-        return self._process_inventory(rows, previous=previous)
+        result = self._process_inventory(rows, previous=previous)
+        # Rendered here, on the executor thread, for the HTTP views to serve.
+        self._inventory_body = json_bytes(result["bottles"])
+        return result

custom_components/cellar_tracker/views.py

@@ -77,12 +77,19 @@ class CellarTrackerInventoryView(_CellarTrackerView):
     name = "api:cellartracker:inventory"
 
     async def get(self, request):
-        """Handle GET request for inventory."""
+        """Handle GET request for inventory.
+
+        The body was rendered by the coordinator when it last refreshed, so a
+        thousand-bottle cellar costs this handler nothing: serialising it here
+        would block the event loop for every dashboard load.
+        """
         coordinator = self._coordinator(request)
         if coordinator is None or not coordinator.data:
             return web.json_response([])
 
-        return web.json_response(coordinator.data.get("bottles", []))
+        return web.Response(
+            body=coordinator.inventory_body, content_type="application/json"
+        )
 
 
 class CellarTrackerSettingsView(_CellarTrackerView):

hacs.json

@@ -1,5 +1,5 @@
 {
   "name": "CellarTracker",
   "render_readme": true,
-  "homeassistant": "2024.7.0"
+  "homeassistant": "2024.11.0"
 }

tests/conftest.py

@@ -13,6 +13,7 @@
 from __future__ import annotations
 
 import asyncio
+import json as _json
 import pathlib
 import sys
 import types
@@ -45,13 +46,22 @@ class UpdateFailed(Exception):
 
 # --- homeassistant.helpers.update_coordinator ---------------------------------
 class DataUpdateCoordinator:
-    """Minimal stand-in that records what the real base class would receive."""
+    """Minimal stand-in that records what the real base class would receive.
 
-    def __init__(self, hass, logger, *, name=None, update_interval=None, **kwargs):
+    ``config_entry`` is captured explicitly rather than absorbed into
+    ``**kwargs``: Home Assistant 2024.11 added it and is making it mandatory,
+    so a test has to be able to see whether we passed it.
+    """
+
+    def __init__(
+        self, hass, logger, *, name=None, config_entry=None, update_interval=None, **kwargs
+    ):
         self.hass = hass
         self.logger = logger
         self.name = name
+        self.config_entry = config_entry
         self.update_interval = update_interval
+        self.init_kwargs = kwargs
         self.data = None
 
 
@@ -174,6 +184,12 @@ class OptionsFlow(_FlowBase):
     EntityCategory=types.SimpleNamespace(DIAGNOSTIC="diagnostic"),
 )
 _module("homeassistant.helpers.entity_platform", AddEntitiesCallback=object)
+_module(
+    "homeassistant.helpers.json",
+    # The real helper is orjson-backed; stdlib json is equivalent for our
+    # payload, which is only str/int/float.
+    json_bytes=lambda data: _json.dumps(data).encode("utf-8"),
+)
 
 class FakeRequest:
     """Minimal aiohttp request exposing only the query string."""
@@ -231,19 +247,27 @@ def __init__(self, coordinator):
 
 
 class FakeCoordinator:
-    """Stands in for WineCellarData in view tests."""
+    """Stands in for WineCellarData in view tests.
+
+    Renders ``inventory_body`` the same way the real coordinator does: the
+    views serve those bytes directly rather than encoding per request, so a
+    double without them would not exercise the code path that runs in
+    production.
+    """
 
     def __init__(self, *, currency="USD", bottles=None, data=True):
         self.currency = currency
         if not data:
             self.data = None
+            self.inventory_body = b"[]"
         else:
             bottles = bottles or []
             self.data = {
                 "total_bottles": len(bottles),
                 "total_value": 0.0,
                 "bottles": bottles,
             }
+            self.inventory_body = _json.dumps(bottles).encode("utf-8")
 
 
 class FakeHttp:

tests/test_coordinator_config_entry.py

@@ -0,0 +1,41 @@
+"""P0-1: the coordinator must be constructed with its config entry.
+
+Home Assistant 2024.11 added ``config_entry`` to ``DataUpdateCoordinator``.
+Omitting it logs a deprecation warning today and is scheduled to become
+mandatory; when it does, setup raises and the integration stops loading.
+
+Passing it is not only about silencing the warning. The coordinator registers
+its refresh task against the entry, so unloading the entry cancels a poll that
+is still in flight instead of leaving it running against a torn-down entry.
+
+The entry is already the constructor's own argument, so nothing has to be
+threaded through to make this work.
+"""
+
+from __future__ import annotations
+
+from cellar_tracker.cellar_data import WineCellarData
+from conftest import ConfigEntry, FakeHass, FakeSession
+
+ENTRY_DATA = {"username": "alice", "password": "s3cret", "currency": "USD"}
+
+
+def build() -> tuple[WineCellarData, ConfigEntry]:
+    hass = FakeHass()
+    hass.session = FakeSession()
+    entry = ConfigEntry(entry_id="entry-abc", data=ENTRY_DATA)
+    return WineCellarData(hass, entry), entry
+
+
+def test_the_entry_reaches_the_base_coordinator():
+    coordinator, entry = build()
+    assert coordinator.config_entry is entry, (
+        "DataUpdateCoordinator must receive config_entry= or Home Assistant "
+        "will refuse to construct it once the deprecation lands"
+    )
+
+
+def test_the_entry_is_not_smuggled_through_kwargs():
+    """It has to be the named parameter, not an extra the base class ignores."""
+    coordinator, _ = build()
+    assert "config_entry" not in coordinator.init_kwargs