Skip to content

Commit 61bd5bb

Browse files
committed
Give the mypy gate something to check
Reported by Codex on #19. The mypy job added with the typed coordinator passed on its first run, which should have been the tell: Home Assistant was not installed for it and ignore_missing_imports was on, so every homeassistant.* import resolved to Any. WineCellarData derives from DataUpdateCoordinator, a class deriving from Any inherits Any for every member it does not declare, and coordinator.data was therefore Any at all five sensors, both views and the diagnostics - the exact call sites DataUpdateCoordinator[CellarData] was introduced to type. Confirmed before fixing: reveal_type(coordinator.data) reported Any, and coordinator.data["definitely_not_a_key"] was accepted in silence. The job reported success over code it had never checked, which is worse than no job at all, because the PR claimed those errors were now impossible. Resolving Home Assistant's real types found four genuine bugs the Any gate had accepted: the four entity classes derived from a bare CoordinatorEntity, so their coordinator was DataUpdateCoordinator[dict[str, Any]] and CellarLastSyncSensor read a last_success that type does not have. * requirements_mypy.txt installs a pinned Home Assistant for the job; the test matrix keeps its light stub harness and its seconds-long run. * No ignore_missing_imports. disallow_any_unimported and warn_return_any make an unresolved import fail rather than degrade - without Home Assistant the job now reports 84 errors instead of success. * stubs/cellartracker for the one dependency that ships no py.typed: RateLimited derives from its CannotConnect, so an Any base there is the same hole one dependency further down. * tests/typing_gate.py asserts what the gate is supposed to guarantee in the checker's own terms - assert_type fails on Any, and the misspelled-key checks are type: ignore comments that warn_unused_ignores turns into errors if they ever stop being necessary. * tests/test_mypy_gate.py pins the wiring from pytest's side. Every reader of the payload is now typed rather than Any, which cost the four CoordinatorEntity[WineCellarData] parameterisations, a Literal key on the drink-window counters, and one honest redeclaration: Home Assistant types data as the payload and then assigns None to it before the first refresh, so the F-15 guards read as dead code until the coordinator says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLPEGFSy3fLEuXNUPAPWR4
1 parent 01d6f59 commit 61bd5bb

17 files changed

Lines changed: 387 additions & 49 deletions

File tree

.github/workflows/ci.yml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,16 @@ jobs:
6060
- uses: actions/checkout@v4
6161
- uses: actions/setup-python@v5
6262
with:
63-
python-version: "3.12"
63+
# Home Assistant 2026.2 requires 3.13; the runtime matrix above still
64+
# covers 3.12, which is what the integration has to keep working on.
65+
python-version: "3.13"
6466
cache: pip
65-
- run: pip install mypy==1.18.2 -r requirements_test.txt
67+
68+
# Home Assistant itself, not just mypy. Without it every homeassistant.*
69+
# import resolves to Any, classes deriving from Any inherit Any for
70+
# everything, and the job reports success over code it never checked.
71+
- run: pip install -r requirements_mypy.txt
72+
6673
- run: mypy
6774

6875
hassfest:

custom_components/cellar_tracker/__init__.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
from pathlib import Path
44

55
from homeassistant.components.http import StaticPathConfig
6-
from homeassistant.config_entries import ConfigEntry
76
from homeassistant.core import HomeAssistant
87
from homeassistant.helpers import config_validation as cv
8+
from homeassistant.helpers.typing import ConfigType
99

10-
from .cellar_data import WineCellarData
10+
from .cellar_data import CellarTrackerConfigEntry, WineCellarData
1111
from .const import DASHBOARD_FILENAME, DASHBOARD_URL, DOMAIN, PLATFORMS
1212
from .views import CellarTrackerInventoryView, CellarTrackerSettingsView
1313

@@ -21,7 +21,7 @@
2121
# integration was installed - HACS or a manual copy - the page is present.
2222
DASHBOARD_FILE = Path(__file__).parent / "www" / DASHBOARD_FILENAME
2323

24-
async def async_setup(hass: HomeAssistant, config: dict) -> bool:
24+
async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
2525
"""Set up the component.
2626
2727
Home Assistant calls this once, before the first config entry. The views
@@ -62,7 +62,7 @@ async def _async_register_dashboard(hass: HomeAssistant) -> None:
6262
[StaticPathConfig(DASHBOARD_URL, str(DASHBOARD_FILE), False)]
6363
)
6464

65-
async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
65+
async def async_setup_entry(hass: HomeAssistant, entry: CellarTrackerConfigEntry) -> bool:
6666
"""Set up CellarTracker from a config entry."""
6767
coordinator = WineCellarData(hass, entry)
6868
await coordinator.async_config_entry_first_refresh()
@@ -78,14 +78,19 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
7878

7979
return True
8080

81-
async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
81+
async def async_unload_entry(hass: HomeAssistant, entry: CellarTrackerConfigEntry) -> bool:
8282
"""Unload a config entry."""
8383
unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
8484
if unload_ok:
8585
# Cleared so the views stop seeing this entry immediately, and so a
8686
# setup that failed before assigning it unloads without a KeyError.
87-
entry.runtime_data = None
87+
# The ignore is the price of the alias: runtime_data is typed as the
88+
# coordinator because that is what every reader wants it to be, and
89+
# this is the one line where it is deliberately not one. Home Assistant
90+
# deletes the attribute itself once unload returns, so this only
91+
# narrows the window in which a request could still find the entry.
92+
entry.runtime_data = None # type: ignore[assignment]
8893
return unload_ok
8994

90-
async def update_listener(hass: HomeAssistant, entry: ConfigEntry) -> None:
95+
async def update_listener(hass: HomeAssistant, entry: CellarTrackerConfigEntry) -> None:
9196
await hass.config_entries.async_reload(entry.entry_id)

custom_components/cellar_tracker/cellar_data.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,17 @@ async def async_fetch_inventory_payload(
256256
class WineCellarData(DataUpdateCoordinator[CellarData]):
257257
"""Fetch and process CellarTracker inventory data."""
258258

259+
# Home Assistant declares `data` as the payload itself and then assigns
260+
# None to it until the first refresh completes - a white lie the framework
261+
# tells with a `type: ignore` of its own. Every reader here already guards
262+
# for that (see F-15: a state read must survive a coordinator with no data
263+
# yet), and without this redeclaration mypy calls those guards dead code,
264+
# because a TypedDict with required keys can never be falsy.
265+
#
266+
# Stated once here rather than narrowed at each of the five call sites. If
267+
# Home Assistant ever types it honestly, warn_unused_ignores will say so.
268+
data: CellarData | None # type: ignore[assignment]
269+
259270
def __init__(self, hass: HomeAssistant, entry: ConfigEntry) -> None:
260271
"""Initialize the data coordinator."""
261272
self._hass = hass
@@ -350,10 +361,6 @@ def compact_body(self) -> bytes:
350361
"""
351362
return self._compact_body
352363

353-
# Declared because the base class is unresolved without Home Assistant
354-
# installed, so mypy has nothing to infer this from.
355-
update_interval: timedelta | None
356-
357364
def _backoff_for(self, retry_after: int | None) -> timedelta:
358365
"""How long to wait after being throttled.
359366

custom_components/cellar_tracker/config_flow.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
REAUTH_SCHEMA = vol.Schema({vol.Required(CONF_PASSWORD): str})
4242

4343

44-
class CellarTrackerConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): # type: ignore[call-arg]
44+
class CellarTrackerConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
4545
"""Handle a config flow for CellarTracker."""
4646

4747
VERSION = 1

custom_components/cellar_tracker/diagnostics.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@
1818
from typing import Any
1919

2020
from homeassistant.components.diagnostics import async_redact_data
21-
from homeassistant.config_entries import ConfigEntry
2221
from homeassistant.const import CONF_PASSWORD, CONF_USERNAME
2322
from homeassistant.core import HomeAssistant
2423

24+
from .cellar_data import CellarTrackerConfigEntry
25+
2526
# "title" as well as the credential keys: async_step_user names the entry
2627
# after the account, so the title *is* the username for every entry this
2728
# integration creates. Redacting only the username field would have published
@@ -48,12 +49,12 @@
4849

4950

5051
async def async_get_config_entry_diagnostics(
51-
hass: HomeAssistant, entry: ConfigEntry
52+
hass: HomeAssistant, entry: CellarTrackerConfigEntry
5253
) -> dict[str, Any]:
5354
"""Return redacted diagnostics for a config entry."""
5455
coordinator = entry.runtime_data
55-
data = coordinator.data or {}
56-
bottles = data.get("bottles") or []
56+
data = coordinator.data
57+
bottles = [] if data is None else data.get("bottles") or []
5758

5859
return {
5960
"entry": async_redact_data(entry.as_dict(), TO_REDACT),
@@ -64,8 +65,8 @@ async def async_get_config_entry_diagnostics(
6465
"currency": coordinator.currency,
6566
},
6667
"totals": {
67-
"total_bottles": data.get("total_bottles"),
68-
"total_value": data.get("total_value"),
68+
"total_bottles": None if data is None else data.get("total_bottles"),
69+
"total_value": None if data is None else data.get("total_value"),
6970
},
7071
# Sorted so two reports can be diffed when a schema change is suspected.
7172
"columns": sorted(bottles[0]) if bottles else [],

custom_components/cellar_tracker/sensor.py

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import annotations
22

3-
from typing import Any
3+
from datetime import datetime
4+
from typing import Literal
45

56
from homeassistant.components.sensor import (
67
SensorDeviceClass,
@@ -14,7 +15,7 @@
1415
from homeassistant.helpers.entity_platform import AddEntitiesCallback
1516
from homeassistant.helpers.update_coordinator import CoordinatorEntity
1617

17-
from .cellar_data import CellarTrackerConfigEntry, WineCellarData
18+
from .cellar_data import CellarData, CellarTrackerConfigEntry, WineCellarData
1819
from .const import CONF_CURRENCY, DEFAULT_CURRENCY, DOMAIN, normalize_currency
1920

2021
# One typed place for what five constructors used to spell out. The key is
@@ -61,6 +62,21 @@
6162
DESCRIPTIONS_BY_KEY = {description.key: description for description in SENSOR_DESCRIPTIONS}
6263

6364

65+
def _count(
66+
data: CellarData | None,
67+
key: Literal["total_bottles", "ready_to_drink", "past_drink_window"],
68+
) -> int:
69+
"""Read a count from the payload, defaulting rather than raising.
70+
71+
Both defences are deliberate and covered by F-15. ``data`` is None until
72+
the first refresh succeeds, which is the state a sensor is in if its
73+
entity is read during a failed setup; and a payload that predates a key -
74+
the drink-window counters were added after the totals - has no such key at
75+
all. Neither should turn a state read into a traceback.
76+
"""
77+
return 0 if data is None else data.get(key, 0)
78+
79+
6480
async def async_setup_entry(
6581
hass: HomeAssistant,
6682
entry: CellarTrackerConfigEntry,
@@ -99,7 +115,7 @@ async def async_setup_entry(
99115
async_add_entities(sensors)
100116

101117

102-
class TotalBottlesSensor(CoordinatorEntity, SensorEntity):
118+
class TotalBottlesSensor(CoordinatorEntity[WineCellarData], SensorEntity):
103119
"""How many bottles the cellar currently holds."""
104120

105121
# Home Assistant composes the friendly name as "<device> <entity>", so the
@@ -116,12 +132,11 @@ def __init__(
116132
self._attr_device_info = device_info
117133

118134
@property
119-
def native_value(self) -> Any:
120-
# `or {}`: coordinator.data is None until the first successful refresh.
121-
return (self.coordinator.data or {}).get("total_bottles", 0)
135+
def native_value(self) -> int:
136+
return _count(self.coordinator.data, "total_bottles")
122137

123138

124-
class TotalValueSensor(CoordinatorEntity, SensorEntity):
139+
class TotalValueSensor(CoordinatorEntity[WineCellarData], SensorEntity):
125140
"""What the cellar is worth, in the configured currency.
126141
127142
MONETARY with state_class TOTAL, so Home Assistant keeps a long-term
@@ -147,11 +162,12 @@ def __init__(
147162
self._attr_native_unit_of_measurement = currency
148163

149164
@property
150-
def native_value(self) -> Any:
151-
return (self.coordinator.data or {}).get("total_value", 0.0)
165+
def native_value(self) -> float:
166+
data = self.coordinator.data
167+
return 0.0 if data is None else data.get("total_value", 0.0)
152168

153169

154-
class _BottleCountSensor(CoordinatorEntity, SensorEntity):
170+
class _BottleCountSensor(CoordinatorEntity[WineCellarData], SensorEntity):
155171
"""Shared shape for the drink-window counters.
156172
157173
Both are plain counts the coordinator computed during the parse, so they
@@ -161,7 +177,9 @@ class _BottleCountSensor(CoordinatorEntity, SensorEntity):
161177
"""
162178

163179
_attr_has_entity_name = True
164-
_data_key: str
180+
# A literal union rather than a bare str: it is used to index CellarData,
181+
# and only a literal lets the checker confirm the key exists at all.
182+
_data_key: Literal["ready_to_drink", "past_drink_window"]
165183

166184
def __init__(
167185
self, coordinator: WineCellarData, device_info: DeviceInfo, entry_id: str
@@ -172,11 +190,8 @@ def __init__(
172190
self._attr_device_info = device_info
173191

174192
@property
175-
def native_value(self) -> Any:
176-
# Defaulted rather than indexed: coordinator.data is None before the
177-
# first refresh, and a payload cached by an older version has no such
178-
# key at all.
179-
return (self.coordinator.data or {}).get(self._data_key, 0)
193+
def native_value(self) -> int:
194+
return _count(self.coordinator.data, self._data_key)
180195

181196

182197
class ReadyToDrinkSensor(_BottleCountSensor):
@@ -191,7 +206,7 @@ class PastDrinkWindowSensor(_BottleCountSensor):
191206
_data_key = "past_drink_window"
192207

193208

194-
class CellarLastSyncSensor(CoordinatorEntity, SensorEntity):
209+
class CellarLastSyncSensor(CoordinatorEntity[WineCellarData], SensorEntity):
195210
"""When the cellar last synchronised with CellarTracker.
196211
197212
This replaces a status sensor that reported "Connected" or "Empty". After
@@ -219,6 +234,6 @@ def __init__(
219234
self._attr_device_info = device_info
220235

221236
@property
222-
def native_value(self) -> Any:
237+
def native_value(self) -> datetime | None:
223238
"""The last successful refresh, or None if none has happened yet."""
224239
return self.coordinator.last_success

custom_components/cellar_tracker/views.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from .const import CURRENCY_SYMBOLS, DEFAULT_CURRENCY, DOMAIN
1313

1414
if TYPE_CHECKING:
15-
from .cellar_data import WineCellarData
15+
from .cellar_data import CellarTrackerConfigEntry, WineCellarData
1616

1717
_LOGGER = logging.getLogger(__name__)
1818

@@ -51,9 +51,16 @@ def _coordinator(self, request: web.Request) -> WineCellarData | None:
5151
and so still forwards the parameter. Serving the lowest entry id there
5252
would answer for an account the caller did not ask for.
5353
"""
54+
# async_entries is typed for any integration, so it hands back
55+
# ConfigEntry[Any]; naming the type here is what stops runtime_data
56+
# laundering Any into everything this method returns. Only entries of
57+
# our own domain are asked for, so the claim holds.
58+
entries: list[CellarTrackerConfigEntry] = self.hass.config_entries.async_entries(
59+
DOMAIN
60+
)
5461
coordinators = {
5562
entry.entry_id: entry.runtime_data
56-
for entry in self.hass.config_entries.async_entries(DOMAIN)
63+
for entry in entries
5764
# runtime_data is assigned at setup and cleared at unload, so its
5865
# presence is what "this entry is serving requests" means here.
5966
if getattr(entry, "runtime_data", None) is not None

pyproject.toml

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,24 @@ testpaths = ["tests"]
1515
known-first-party = ["cellar_tracker", "conftest"]
1616

1717
[tool.mypy]
18-
python_version = "3.12"
19-
files = ["custom_components/cellar_tracker"]
20-
# Home Assistant itself is not installed here - pytest-homeassistant-custom-component
21-
# cannot build its wheels in this environment - so its imports are unresolved by
22-
# design. Everything about *our* code is still checked.
23-
ignore_missing_imports = true
24-
follow_imports = "silent"
18+
# 3.13 because that is what Home Assistant 2026.2 requires; the runtime matrix
19+
# in CI still covers 3.12 as well.
20+
python_version = "3.13"
21+
22+
# custom_components so that `cellar_tracker` resolves as a top-level package,
23+
# stubs/ for the one dependency that ships no types of its own.
24+
mypy_path = ["custom_components", "stubs"]
25+
files = ["custom_components/cellar_tracker", "tests/typing_gate.py"]
26+
27+
# No ignore_missing_imports. Home Assistant is installed for this job
28+
# (requirements_mypy.txt) precisely so that it is not waved through: an
29+
# unresolved `homeassistant.*` makes DataUpdateCoordinator `Any`, a class
30+
# deriving from `Any` inherits `Any` for everything it does not declare, and
31+
# `coordinator.data` is then unchecked at every call site - the generic
32+
# parameter reduced to decoration. tests/typing_gate.py asserts, in the
33+
# checker's own terms, that this has not happened.
34+
disallow_any_unimported = true
35+
warn_return_any = true
2536

2637
warn_unused_ignores = true
2738
warn_redundant_casts = true

requirements_mypy.txt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# The type gate's dependencies, pinned and separate from requirements_test.txt.
2+
#
3+
# Home Assistant is here for one reason: without it installed, every
4+
# `homeassistant.*` import resolves to `Any`, subclassing `Any` makes every
5+
# inherited member `Any`, and the checker then accepts a misspelled coordinator
6+
# key or an invalid framework call in silence. See tests/typing_gate.py.
7+
#
8+
# It is deliberately absent from requirements_test.txt: the unit tests stub the
9+
# handful of symbols they need (tests/conftest.py) and run in seconds on both
10+
# supported Python versions, which is worth keeping.
11+
#
12+
# 2026.2.3 is the last release that runs on Python 3.13; 2026.3 requires 3.14.
13+
# Bump both this pin and the mypy job's python-version together.
14+
homeassistant==2026.2.3
15+
mypy==1.18.2

stubs/cellartracker/README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# Type stubs for `cellartracker`
2+
3+
`cellartracker` 1.1.1 ships no `py.typed` marker, so mypy resolves everything
4+
imported from it to `Any`. That matters more than it looks: `RateLimited`
5+
subclasses `CannotConnect`, and a class whose base is `Any` has `Any` for every
6+
member it did not declare itself - the same hole that made the coordinator's
7+
own `data` untyped before this.
8+
9+
Waving it through with `ignore_missing_imports` would leave that hole open and
10+
make `disallow_any_unimported` unusable, so the four symbols the integration
11+
actually imports are declared here instead. The surface is deliberately narrow:
12+
two constants, two enums and two exception types, pinned by
13+
`cellartracker==1.1.1` in `requirements_test.txt`. If the library grows a
14+
`py.typed` of its own, delete this directory and the `mypy_path` entry.

0 commit comments

Comments
 (0)