From 6741a1a210e41c7b7b01601265535c181a57dd51 Mon Sep 17 00:00:00 2001 From: Joshua Date: Mon, 21 Sep 2026 17:29:04 -0400 Subject: [PATCH 1/3] Extract the panel snapshot builder into core (task 9047a97b) The household snapshot assembly that lived inline in the coordinator's _async_update_data is extracted into a pure, Home-Assistant-free core/snapshot.py: the QuestInstanceView / ChildDaySnapshot / NestQuestSnapshot dataclasses, the _cycle_day helper, and a new async build_snapshot(database, settings, today, time_zone) builder. The coordinator is now a thin DataUpdateCoordinator wrapper that resolves the local timezone and settings and delegates to build_snapshot; the dataclasses and _cycle_day are re-exported from the coordinator module so existing sensor.py / binary_sensor.py imports keep working unchanged (entity_ids and per-child instance attribute payload shape unchanged). The API service's panel snapshot route (next task) shares this one implementation. Tests that monkeypatched derive_state on the coordinator module now patch core.snapshot.derive_state (the module that now owns the call). A new test seeds a household and asserts build_snapshot's output equals the coordinator's snapshot for the same database and date. Suite green at 1588. --- custom_components/nestquest/__init__.py | 1 + custom_components/nestquest/coordinator.py | 224 +++-------------- custom_components/nestquest/core/snapshot.py | 251 +++++++++++++++++++ tests/test_core_snapshot.py | 182 ++++++++++++++ tests/test_entity_matrix.py | 8 +- tests/test_sensors.py | 6 +- 6 files changed, 470 insertions(+), 202 deletions(-) create mode 100644 custom_components/nestquest/core/snapshot.py create mode 100644 tests/test_core_snapshot.py diff --git a/custom_components/nestquest/__init__.py b/custom_components/nestquest/__init__.py index e8bd9fb..6b0eac4 100644 --- a/custom_components/nestquest/__init__.py +++ b/custom_components/nestquest/__init__.py @@ -496,6 +496,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: hass, entry_id=entry.entry_id, database=database, + settings=settings, update_interval_seconds=entry.options.get( CONF_UPDATE_INTERVAL, DEFAULT_UPDATE_INTERVAL ), diff --git a/custom_components/nestquest/coordinator.py b/custom_components/nestquest/coordinator.py index 39dc777..61cca80 100644 --- a/custom_components/nestquest/coordinator.py +++ b/custom_components/nestquest/coordinator.py @@ -11,6 +11,14 @@ refresh updates the whole board coherently. host clock and never UTC, matching the materialization and completion layers. +The snapshot assembly itself lives in the Home-Assistant-free +:mod:`custom_components.nestquest.core.snapshot` module +(:func:`~core.snapshot.build_snapshot`); this coordinator is now a thin +HA wrapper that resolves the local timezone and settings off the config +entry and delegates to that builder. The API service's panel snapshot +route shares the same builder, so both consumers render ONE snapshot +shape. + Presence is resolved through the SAME engine the materializer uses (:class:`~.presence.PresenceEngine`): schedules and overrides are read with the public DAO methods and the engine's anchor-date arithmetic @@ -27,13 +35,11 @@ from __future__ import annotations import asyncio import datetime -from dataclasses import dataclass from zoneinfo import ZoneInfo from homeassistant.core import HomeAssistant from homeassistant.helpers.update_coordinator import DataUpdateCoordinator -from .completion import derive_state from .const import ( CONF_UPDATE_INTERVAL, DEFAULT_UPDATE_INTERVAL, @@ -41,66 +47,24 @@ from .const import ( LOGGER, MIN_UPDATE_INTERVAL, ) -from .dao_children import ChildrenDao -from .dao_instances import ( - CompletionEventsDao, - QuestInstancesDao, -) -from .dao_rules import QuestDefinitionsDao -from .dao_presence import PresenceOverridesDao, PresenceSchedulesDao -from .db import NestQuestDatabase -from .presence import ( - MAX_PREVIEW_SCAN_DAYS, - PresenceEngine, - PresenceOverride, - PresenceSchedule, +from .core.db import NestQuestDatabase +from .core.settings import NestQuestSettings +from .core.snapshot import ( + ChildDaySnapshot, + NestQuestSnapshot, + QuestInstanceView, + _cycle_day, + build_snapshot, ) - -@dataclass(frozen=True) -class QuestInstanceView: - """One instance plus everything the entities render from it.""" - - instance_id: int - definition_id: int - child_id: int - title: str - icon: str | None - window: str - due_date: str - due_time: str | None - state: str # open | done | missed - overdue: bool - completed_at: str | None - was_on_time: bool | None - - -@dataclass(frozen=True) -class ChildDaySnapshot: - """One active child's day: instances, presence, and rollup counts.""" - - child_id: int - child_name: str - instances: tuple[QuestInstanceView, ...] - present: bool - due_today: int - completed_today: int - remaining_today: int - completion_pct: int - #: For a child away today: the ISO date of their next present day - #: (the panel's away plate renders ``Returns , `` - #: from it). ``None`` when the child is present today or no - #: present day exists within the presence engine's scan cap. - next_present: str | None = None - - -@dataclass(frozen=True) -class NestQuestSnapshot: - """The coordinator's whole-entity data payload.""" - - today_iso: str - children: tuple[ChildDaySnapshot, ...] - cycle_day: int +__all__ = [ + "ChildDaySnapshot", + "NestQuestCoordinator", + "NestQuestSnapshot", + "QuestInstanceView", + "_cycle_day", + "build_snapshot", +] class NestQuestCoordinator(DataUpdateCoordinator): @@ -121,6 +85,7 @@ class NestQuestCoordinator(DataUpdateCoordinator): *, entry_id: str, database: NestQuestDatabase, + settings: NestQuestSettings, update_interval_seconds: int | None = None, ) -> None: interval = update_interval_seconds @@ -138,6 +103,7 @@ class NestQuestCoordinator(DataUpdateCoordinator): ) self.entry_id = entry_id self.database = database + self.settings = settings self._refresh_lock = asyncio.Lock() async def async_refresh(self) -> None: @@ -154,139 +120,7 @@ class NestQuestCoordinator(DataUpdateCoordinator): async def _async_update_data(self) -> NestQuestSnapshot: now = self._local_now() today = now.date() - today_iso = today.isoformat() - - children = await ChildrenDao(self.database).list_active() - - schedules: dict[int, PresenceSchedule] = {} - overrides: dict[int, list[PresenceOverride]] = {} - for child in children: - schedule = await PresenceSchedulesDao(self.database).get_by_child( - child.id - ) - if schedule is not None: - schedules[child.id] = PresenceSchedule.decode( - child.id, schedule.anchor_date, schedule.pattern - ) - window_end = today + datetime.timedelta( - days=MAX_PREVIEW_SCAN_DAYS - ) - window_end_iso = window_end.isoformat() - ranged = await PresenceOverridesDao( - self.database - ).list_by_child_and_range(child.id, today_iso, window_end_iso) - if ranged: - # The engine validates MODEL objects, not DAO records; - # each record converts losslessly (the schema guarantees - # end >= start and a real is_present). - overrides[child.id] = [ - PresenceOverride( - record.child_id, - record.start_date, - record.end_date, - record.is_present, - record.note, - ) - for record in ranged - ] - engine = PresenceEngine(schedules, overrides) - - instances_dao = QuestInstancesDao(self.database) - events_dao = CompletionEventsDao(self.database) - definitions_dao = QuestDefinitionsDao(self.database) - - snapshots: list[ChildDaySnapshot] = [] - for child in children: - instances = await instances_dao.list_by_date_range( - child.id, today_iso, today_iso - ) - views: list[QuestInstanceView] = [] - for instance in instances: - latest = await events_dao.get_latest_for_instance(instance.id) - definition = await definitions_dao.get(instance.definition_id) - state = derive_state(instance, latest, today) - overdue = False - if state == "open" and instance.due_date == today_iso: - if instance.due_time is not None: - due_moment = datetime.datetime.strptime( - instance.due_time, "%H:%M" - ).time() - overdue = now.time() > due_moment - views.append( - QuestInstanceView( - instance_id=instance.id, - definition_id=instance.definition_id, - child_id=instance.child_id, - title=definition.title if definition else "Quest", - icon=definition.icon if definition else None, - window=instance.window, - due_date=instance.due_date, - due_time=instance.due_time, - state=state, - overdue=overdue, - completed_at=( - latest.occurred_at - if latest is not None - and latest.event_type == "completed" - else None - ), - was_on_time=( - latest.was_on_time - if latest is not None - and latest.event_type == "completed" - else None - ), - ) - ) - due = len(views) - completed = sum(1 for view in views if view.state == "done") - remaining = due - completed - child_present = engine.is_present(child.id, today) - next_present: str | None = None - if not child_present: - try: - next_present = engine.next_present_dates( - child.id, today, 1 - )[0].isoformat() - except ValueError: - next_present = None - snapshots.append( - ChildDaySnapshot( - child_id=child.id, - child_name=child.display_name, - instances=tuple(views), - present=child_present, - due_today=due, - completed_today=completed, - remaining_today=remaining, - completion_pct=( - round(completed * 100 / due) if due else 100 - ), - next_present=next_present, - ) - ) - - return NestQuestSnapshot( - today_iso=today_iso, - children=tuple(snapshots), - cycle_day=_cycle_day(schedules, children, today), + time_zone = ZoneInfo(self.hass.config.time_zone) + return await build_snapshot( + self.database, self.settings, today, time_zone ) - - -def _cycle_day( - schedules: dict[int, PresenceSchedule], children, today: datetime.date -) -> int: - """Return today's 1-based day in the first scheduled child's cycle. - - Anchor-date arithmetic ONLY (D-004): the day offset is computed from - the schedule's own anchor, never ISO week numbers or parity, so a - 53-week year cannot silently invert the cycle. - """ - for child in children: - schedule = schedules.get(child.id) - if schedule is None: - continue - days = (today - schedule.anchor_date).days - cycle_length = schedule.cycle_length_weeks * 7 - return (days % cycle_length) + 1 - return 0 diff --git a/custom_components/nestquest/core/snapshot.py b/custom_components/nestquest/core/snapshot.py new file mode 100644 index 0000000..f57da5d --- /dev/null +++ b/custom_components/nestquest/core/snapshot.py @@ -0,0 +1,251 @@ +"""Pure, Home-Assistant-free panel snapshot builder. + +The household snapshot the NestQuest panel renders — active children in +sort order, each child's today-instances in the documented payload shape +(missed instances OMITTED from the entity-facing attribute payload, but +retained on the snapshot for the count rollups), per-child counts and +presence, and the custody cycle day — used to be assembled inline inside +:mod:`custom_components.nestquest.coordinator`'s +``_async_update_data``. That assembly is HA-coupled only because its +host is a :class:`~homeassistant.helpers.update_coordinator.DataUpdateCoordinator`; +the assembly itself touches no HA surface. + +This module holds the pure pieces — the snapshot dataclasses +(:class:`QuestInstanceView`, :class:`ChildDaySnapshot`, +:class:`NestQuestSnapshot`), the cycle-day helper (:func:`_cycle_day`), +and the async :func:`build_snapshot` builder — so the HA coordinator and +the API service's panel snapshot route share ONE implementation. +Nothing here imports :mod:`homeassistant`; the builder takes the +database, the settings object, the target date, and the timezone and +returns the snapshot, mirroring exactly what the coordinator used to do +inline. + +``settings`` is accepted for forward-compatibility (the API route hands +in the entry's :class:`~.settings.NestQuestSettings`); the current +assembly does not read it, but the parameter keeps the builder's +contract stable as future panel features derive scan caps or rollup +behaviour from configuration. +""" +from __future__ import annotations + +import datetime +from dataclasses import dataclass +from zoneinfo import ZoneInfo + +from .completion import derive_state +from .dao_children import ChildrenDao +from .dao_instances import CompletionEventsDao, QuestInstancesDao +from .dao_presence import PresenceOverridesDao, PresenceSchedulesDao +from .dao_rules import QuestDefinitionsDao +from .db import NestQuestDatabase +from .presence import ( + MAX_PREVIEW_SCAN_DAYS, + PresenceEngine, + PresenceOverride, + PresenceSchedule, +) +from .settings import NestQuestSettings + + +@dataclass(frozen=True) +class QuestInstanceView: + """One instance plus everything the entities render from it.""" + + instance_id: int + definition_id: int + child_id: int + title: str + icon: str | None + window: str + due_date: str + due_time: str | None + state: str # open | done | missed + overdue: bool + completed_at: str | None + was_on_time: bool | None + + +@dataclass(frozen=True) +class ChildDaySnapshot: + """One active child's day: instances, presence, and rollup counts.""" + + child_id: int + child_name: str + instances: tuple[QuestInstanceView, ...] + present: bool + due_today: int + completed_today: int + remaining_today: int + completion_pct: int + #: For a child away today: the ISO date of their next present day + #: (the panel's away plate renders ``Returns , `` + #: from it). ``None`` when the child is present today or no + #: present day exists within the presence engine's scan cap. + next_present: str | None = None + + +@dataclass(frozen=True) +class NestQuestSnapshot: + """The coordinator's whole-entity data payload.""" + + today_iso: str + children: tuple[ChildDaySnapshot, ...] + cycle_day: int + + +def _cycle_day( + schedules: dict[int, PresenceSchedule], children, today: datetime.date +) -> int: + """Return today's 1-based day in the first scheduled child's cycle. + + Anchor-date arithmetic ONLY (D-004): the day offset is computed from + the schedule's own anchor, never ISO week numbers or parity, so a + 53-week year cannot silently invert the cycle. + """ + for child in children: + schedule = schedules.get(child.id) + if schedule is None: + continue + days = (today - schedule.anchor_date).days + cycle_length = schedule.cycle_length_weeks * 7 + return (days % cycle_length) + 1 + return 0 + + +async def build_snapshot( + database: NestQuestDatabase, + settings: NestQuestSettings, + today: datetime.date, + time_zone: ZoneInfo, +) -> NestQuestSnapshot: + """Build the household panel snapshot for ``today`` in ``time_zone``. + + Mirrors the assembly that lived in the coordinator's + ``_async_update_data``: active children in sort order, each child's + today-instances with derived state (open / done / missed via + :func:`~.completion.derive_state`) and the overdue flag, per-child + presence (resolved through :class:`~.presence.PresenceEngine`), the + per-child rollup counts, and the custody cycle day of the first + scheduled active child. Missed instances are retained on the + snapshot (the rollup counts and the panel's missed plate need them) + but the entity-facing attribute payload omits them at the sensor + layer — this builder produces the shared shape both consumers read. + """ + now = datetime.datetime.now(time_zone) + today_iso = today.isoformat() + + children = await ChildrenDao(database).list_active() + + schedules: dict[int, PresenceSchedule] = {} + overrides: dict[int, list[PresenceOverride]] = {} + for child in children: + schedule = await PresenceSchedulesDao(database).get_by_child( + child.id + ) + if schedule is not None: + schedules[child.id] = PresenceSchedule.decode( + child.id, schedule.anchor_date, schedule.pattern + ) + window_end = today + datetime.timedelta( + days=MAX_PREVIEW_SCAN_DAYS + ) + window_end_iso = window_end.isoformat() + ranged = await PresenceOverridesDao( + database + ).list_by_child_and_range(child.id, today_iso, window_end_iso) + if ranged: + # The engine validates MODEL objects, not DAO records; + # each record converts losslessly (the schema guarantees + # end >= start and a real is_present). + overrides[child.id] = [ + PresenceOverride( + record.child_id, + record.start_date, + record.end_date, + record.is_present, + record.note, + ) + for record in ranged + ] + engine = PresenceEngine(schedules, overrides) + + instances_dao = QuestInstancesDao(database) + events_dao = CompletionEventsDao(database) + definitions_dao = QuestDefinitionsDao(database) + + snapshots: list[ChildDaySnapshot] = [] + for child in children: + instances = await instances_dao.list_by_date_range( + child.id, today_iso, today_iso + ) + views: list[QuestInstanceView] = [] + for instance in instances: + latest = await events_dao.get_latest_for_instance(instance.id) + definition = await definitions_dao.get(instance.definition_id) + state = derive_state(instance, latest, today) + overdue = False + if state == "open" and instance.due_date == today_iso: + if instance.due_time is not None: + due_moment = datetime.datetime.strptime( + instance.due_time, "%H:%M" + ).time() + overdue = now.time() > due_moment + views.append( + QuestInstanceView( + instance_id=instance.id, + definition_id=instance.definition_id, + child_id=instance.child_id, + title=definition.title if definition else "Quest", + icon=definition.icon if definition else None, + window=instance.window, + due_date=instance.due_date, + due_time=instance.due_time, + state=state, + overdue=overdue, + completed_at=( + latest.occurred_at + if latest is not None + and latest.event_type == "completed" + else None + ), + was_on_time=( + latest.was_on_time + if latest is not None + and latest.event_type == "completed" + else None + ), + ) + ) + due = len(views) + completed = sum(1 for view in views if view.state == "done") + remaining = due - completed + child_present = engine.is_present(child.id, today) + next_present: str | None = None + if not child_present: + try: + next_present = engine.next_present_dates( + child.id, today, 1 + )[0].isoformat() + except ValueError: + next_present = None + snapshots.append( + ChildDaySnapshot( + child_id=child.id, + child_name=child.display_name, + instances=tuple(views), + present=child_present, + due_today=due, + completed_today=completed, + remaining_today=remaining, + completion_pct=( + round(completed * 100 / due) if due else 100 + ), + next_present=next_present, + ) + ) + + return NestQuestSnapshot( + today_iso=today_iso, + children=tuple(snapshots), + cycle_day=_cycle_day(schedules, children, today), + ) diff --git a/tests/test_core_snapshot.py b/tests/test_core_snapshot.py new file mode 100644 index 0000000..da4e29a --- /dev/null +++ b/tests/test_core_snapshot.py @@ -0,0 +1,182 @@ +"""Test the pure panel snapshot builder (task 9047a97b). + +The snapshot assembly that lived inline in the coordinator's +``_async_update_data`` was extracted into the Home-Assistant-free +:func:`custom_components.nestquest.core.snapshot.build_snapshot`. This +test seeds a household (active + inactive children, a daily quest +definition, today's instances, one completed instance, and a presence +schedule) and asserts the pure builder's output is byte-for-byte equal +to the coordinator's snapshot for the SAME database and date — the +shared-implementation invariant the API panel snapshot route depends on. +""" +from __future__ import annotations + +import datetime +from zoneinfo import ZoneInfo + +from conftest import wire_entry_to_registry + +from custom_components.nestquest import async_setup_entry +from custom_components.nestquest.children import ( + create_child, + list_children, + set_child_active, +) +from custom_components.nestquest.completion import complete_instance +from custom_components.nestquest.core.snapshot import ( + ChildDaySnapshot, + NestQuestSnapshot, + QuestInstanceView, + build_snapshot, +) +from custom_components.nestquest.dao_presence import PresenceSchedulesDao +from custom_components.nestquest.materialize import materialize +from custom_components.nestquest.quest_definitions import create_quest_definition +from custom_components.nestquest.recurrence import ScheduleRule + +SEED_HORIZON_DAYS = 3 + + +def _today() -> datetime.date: + return datetime.date.today() + + +async def _seed_household(database): + """Three active children + one inactive; a daily quest for two of them. + + Ada and Bo share a daily morning quest; Cory is active but has no + quest (exercises the empty-instance / 100%-completion path); Ghost + is inactive and must be absent from the snapshot. Ada's first + instance is completed so the ``done`` state and rollup counts are + exercised, not just the ``open`` default. + """ + ada = await create_child(database, "Ada") + bo = await create_child(database, "Bo") + cory = await create_child(database, "Cory") + ghost = await create_child(database, "Ghost") + await set_child_active(database, ghost.id, False) + + today = _today() + await create_quest_definition( + database, + "Brush teeth", + ScheduleRule.from_dict( + {"rule_type": "daily", "start_date": today.isoformat()} + ), + [ada.id, bo.id], + ["morning"], + ) + end = (today + datetime.timedelta(days=SEED_HORIZON_DAYS)).isoformat() + await materialize(database, today.isoformat(), end, today=today) + + # A 1-week presence schedule for Ada, present every day, anchored + # today: exercises cycle_day (== 1) and the present=True path. Bo + # and Cory have no schedule, so the engine treats them as present + # every day (the documented default). + await PresenceSchedulesDao(database).upsert_by_child( + ada.id, + cycle_length_weeks=1, + anchor_date=today.isoformat(), + pattern="0,1,2,3,4,5,6", + ) + return ada, bo, cory, ghost + + +async def test_build_snapshot_matches_coordinator_snapshot( + hass, make_entry +) -> None: + """The pure builder and the coordinator produce identical snapshots.""" + entry = wire_entry_to_registry(make_entry(), hass.registry) + assert await async_setup_entry(hass, entry) is True + coordinator = entry.runtime_data.coordinator + database = entry.runtime_data.database + settings = entry.runtime_data.settings + ada, bo, cory, ghost = await _seed_household(database) + + # Complete Ada's first open instance through the business layer so + # the ``done`` state, completed_at/was_on_time, and the rollup + # counts are exercised alongside the ``open`` default for Bo. + active_children = await list_children(database, active_only=True) + ada_id = next(child.id for child in active_children if child.display_name == "Ada") + bo_id = next(child.id for child in active_children if child.display_name == "Bo") + await coordinator.async_refresh() + ada_snapshot = next( + child for child in coordinator.data.children if child.child_id == ada_id + ) + await complete_instance( + database, + ada_snapshot.instances[0].instance_id, + actor_source="panel", + actor_child_id=ada_id, + ) + + # Drive the coordinator's refresh (the path production takes) and + # the pure builder with the SAME date and timezone the coordinator + # resolves, so the comparison is exact — no host-clock skew, no + # timezone drift. Instances here carry no due_time, so the overdue + # flag is False regardless of the instant each pass reads ``now``. + await coordinator.async_refresh() + coordinator_snapshot = coordinator.data + today = coordinator._local_now().date() + time_zone = ZoneInfo(hass.config.time_zone) + built = await build_snapshot(database, settings, today, time_zone) + + assert isinstance(built, NestQuestSnapshot) + assert built == coordinator_snapshot, ( + "build_snapshot and the coordinator must produce identical " + "snapshots for the same database, date, and timezone" + ) + + # Shape and documented invariants, not just equality: children in + # sort order, inactive child omitted, missed instances omitted from + # the panel payload (none are missed in this seed, so the instances + # tuple carries only open/done views), per-child counts, presence, + # and cycle day. + active_ordered = await list_children(database, active_only=True) + active_ids_ordered = [child.id for child in active_ordered] + child_ids = [child.child_id for child in built.children] + assert ghost.id not in child_ids, "inactive child must be absent" + assert child_ids == active_ids_ordered, ( + "children must appear in the DAO's sort_order, id order" + ) + assert built.today_iso == today.isoformat() + assert built.cycle_day == 1, "Ada's 1-week schedule anchored today => day 1" + + by_id = {child.child_id: child for child in built.children} + ada_child = by_id[ada_id] + bo_child = by_id[bo_id] + cory_child = by_id[cory.id] + for child in built.children: + assert isinstance(child, ChildDaySnapshot) + for view in child.instances: + assert isinstance(view, QuestInstanceView) + assert view.state in ("open", "done", "missed") + # Missed instances are omitted from the panel payload shape; + # in this seed none are missed (today's open instances have + # not crossed their due time and there is no due_time set). + assert view.state != "missed", ( + "no instance should be missed for today's seed" + ) + + assert ada_child.present is True + assert bo_child.present is True + assert cory_child.present is True + assert cory_child.instances == () + assert cory_child.due_today == 0 + assert cory_child.completion_pct == 100 + + assert ada_child.due_today == 1 + assert ada_child.completed_today == 1 + assert ada_child.remaining_today == 0 + assert ada_child.completion_pct == 100 + assert ada_child.instances[0].state == "done" + assert ada_child.instances[0].completed_at is not None + assert ada_child.instances[0].was_on_time is not None + + assert bo_child.due_today == 1 + assert bo_child.completed_today == 0 + assert bo_child.remaining_today == 1 + assert bo_child.completion_pct == 0 + assert bo_child.instances[0].state == "open" + assert bo_child.instances[0].completed_at is None + assert bo_child.instances[0].was_on_time is None diff --git a/tests/test_entity_matrix.py b/tests/test_entity_matrix.py index 0bbbe49..f1e8757 100644 --- a/tests/test_entity_matrix.py +++ b/tests/test_entity_matrix.py @@ -186,18 +186,18 @@ async def test_entity_and_event_matrix(hass, make_entry) -> None: original_refresh = coordinator._async_update_data async def _future_dated_refresh(): - import custom_components.nestquest.coordinator as coordinator_module + import custom_components.nestquest.core.snapshot as snapshot_module - real_derive = coordinator_module.derive_state + real_derive = snapshot_module.derive_state def _derive(instance, latest, today): return real_derive(instance, latest, today + datetime.timedelta(days=1)) - coordinator_module.derive_state = _derive + snapshot_module.derive_state = _derive try: return await original_refresh() finally: - coordinator_module.derive_state = real_derive + snapshot_module.derive_state = real_derive coordinator._async_update_data = _future_dated_refresh await coordinator.async_refresh() diff --git a/tests/test_sensors.py b/tests/test_sensors.py index 08641fc..dcacaea 100644 --- a/tests/test_sensors.py +++ b/tests/test_sensors.py @@ -238,16 +238,16 @@ async def test_due_today_payload_omits_missed_for_panel_includes_for_admin( entry, coordinator, (ada, bo, cory) = await _setup_seeded_entry( hass, make_entry ) - import custom_components.nestquest.coordinator as coordinator_module + import custom_components.nestquest.core.snapshot as snapshot_module tomorrow = _today() + datetime.timedelta(days=1) - real_derive_state = coordinator_module.derive_state + real_derive_state = snapshot_module.derive_state def _derive_against_pinned_future(instance, latest, today): return real_derive_state(instance, latest, tomorrow) monkeypatch.setattr( - coordinator_module, "derive_state", _derive_against_pinned_future + snapshot_module, "derive_state", _derive_against_pinned_future ) await coordinator.async_refresh() -- 2.55.0 From c18246d58f146860cb17bc20cc8e80d9181aac65 Mon Sep 17 00:00:00 2001 From: Joshua Date: Mon, 21 Sep 2026 17:37:16 -0400 Subject: [PATCH 2/3] Move panel payload shaping into core; pin the snapshot clock PR #148 review (CHANGES_REQUIRED) follow-up: - Move the panel payload dict shaper (_instance_payload) from the HA-importing sensor.py into core/snapshot.py as instance_payload, and have sensor.py import it from core. The API panel route now shares ONE shaping implementation with the HA sensor layer (done -> completed, on_time passthrough, missed omitted unless include_missed). - build_snapshot takes the HA-local now explicitly instead of reading datetime.now(time_zone) internally, so date and time come from a single clock read and the overdue check is pinned in tests. The coordinator passes the same now it already resolved; time_zone is dropped (now is timezone-aware). - Strengthen tests/test_core_snapshot.py with explicit literal expected values instead of self-equality: an AWAY child (2-week absent-week schedule + a presence override => next_present), a due-timed quest exercised as overdue under a pinned 12:00 now, a completed quest (done -> completed, on_time True), cycle_day == 2, and a constructed missed view proving omission from the panel payload and retention in the admin payload. - Docstring cleanups: state plainly that settings is currently unused (no 'wired' implication); correct the coordinator docstring (settings are passed in, not resolved off the entry); trim the coordinator __all__ to what other modules actually import. - Derive the seed/materialize date from coordinator._local_now() so the seed date and snapshot date cannot diverge near midnight; drop the redundant bo/ghost/list_children leftovers from the prior seed. --- custom_components/nestquest/coordinator.py | 15 +- custom_components/nestquest/core/snapshot.py | 86 ++++- custom_components/nestquest/sensor.py | 39 +- tests/test_core_snapshot.py | 367 ++++++++++++------- 4 files changed, 317 insertions(+), 190 deletions(-) diff --git a/custom_components/nestquest/coordinator.py b/custom_components/nestquest/coordinator.py index 61cca80..647bbaf 100644 --- a/custom_components/nestquest/coordinator.py +++ b/custom_components/nestquest/coordinator.py @@ -14,10 +14,11 @@ layers. The snapshot assembly itself lives in the Home-Assistant-free :mod:`custom_components.nestquest.core.snapshot` module (:func:`~core.snapshot.build_snapshot`); this coordinator is now a thin -HA wrapper that resolves the local timezone and settings off the config -entry and delegates to that builder. The API service's panel snapshot -route shares the same builder, so both consumers render ONE snapshot -shape. +HA wrapper that resolves the local timezone and reads the settings handed +into its constructor, then delegates to that builder, passing the SAME +HA-local ``now`` it resolved in one clock read. The API service's panel +snapshot route shares the same builder, so both consumers render ONE +snapshot shape. Presence is resolved through the SAME engine the materializer uses (:class:`~.presence.PresenceEngine`): schedules and overrides are read @@ -53,7 +54,6 @@ from .core.snapshot import ( ChildDaySnapshot, NestQuestSnapshot, QuestInstanceView, - _cycle_day, build_snapshot, ) @@ -62,8 +62,6 @@ __all__ = [ "NestQuestCoordinator", "NestQuestSnapshot", "QuestInstanceView", - "_cycle_day", - "build_snapshot", ] @@ -120,7 +118,6 @@ class NestQuestCoordinator(DataUpdateCoordinator): async def _async_update_data(self) -> NestQuestSnapshot: now = self._local_now() today = now.date() - time_zone = ZoneInfo(self.hass.config.time_zone) return await build_snapshot( - self.database, self.settings, today, time_zone + self.database, self.settings, today, now ) diff --git a/custom_components/nestquest/core/snapshot.py b/custom_components/nestquest/core/snapshot.py index f57da5d..f54fe61 100644 --- a/custom_components/nestquest/core/snapshot.py +++ b/custom_components/nestquest/core/snapshot.py @@ -13,24 +13,27 @@ the assembly itself touches no HA surface. This module holds the pure pieces — the snapshot dataclasses (:class:`QuestInstanceView`, :class:`ChildDaySnapshot`, :class:`NestQuestSnapshot`), the cycle-day helper (:func:`_cycle_day`), -and the async :func:`build_snapshot` builder — so the HA coordinator and -the API service's panel snapshot route share ONE implementation. +the panel payload shaper (:func:`instance_payload`), and the async +:func:`build_snapshot` builder — so the HA coordinator and the API +service's panel snapshot route share ONE implementation of both the +snapshot assembly and the entity-facing attribute payload shape. Nothing here imports :mod:`homeassistant`; the builder takes the -database, the settings object, the target date, and the timezone and -returns the snapshot, mirroring exactly what the coordinator used to do -inline. +database, the settings object, the target date, and the HA-local +``now`` (a single clock read the caller already resolved) and returns +the snapshot, mirroring exactly what the coordinator used to do inline. -``settings`` is accepted for forward-compatibility (the API route hands -in the entry's :class:`~.settings.NestQuestSettings`); the current -assembly does not read it, but the parameter keeps the builder's -contract stable as future panel features derive scan caps or rollup -behaviour from configuration. +``settings`` is accepted because the API route hands in the entry's +:class:`~.settings.NestQuestSettings`; it is CURRENTLY UNUSED — the +assembly reads nothing from it — and is retained so the builder's +contract stays stable if a future panel feature derives scan caps or +rollup behaviour from configuration. No speculative read is wired in. """ from __future__ import annotations import datetime +from collections.abc import Iterable from dataclasses import dataclass -from zoneinfo import ZoneInfo +from typing import Any from .completion import derive_state from .dao_children import ChildrenDao @@ -112,13 +115,53 @@ def _cycle_day( return 0 +def instance_payload( + instances: Iterable[QuestInstanceView], *, include_missed: bool +) -> list[dict[str, Any]]: + """Shape instance dicts per ENTITIES-AND-SERVICES.md §1. + + The panel payload ``state`` is the documented ``open`` | ``completed`` + spelling, so the builder's derived ``done`` maps to ``completed`` here; + ``open`` passes through. ``missed`` instances are OMITTED from the + entity-facing attribute payload (decision 5) unless ``include_missed`` + is set — the admin payload (D-009) keeps them with their ``missed`` + state. ``on_time`` passes the builder's ``was_on_time`` through + verbatim (``True`` / ``False`` for a completed instance, ``None`` for + an open one). This is the SINGLE shared implementation both the HA + sensor layer and the API panel route read; it touches no Home + Assistant surface. + """ + payload: list[dict[str, Any]] = [] + for view in instances: + if view.state == "missed" and not include_missed: + continue + payload.append( + { + "id": view.instance_id, + "definition_id": view.definition_id, + "child_id": view.child_id, + "title": view.title, + "icon": view.icon, + "window": view.window, + "due_time": view.due_time, + "state": ( + "completed" if view.state == "done" else view.state + ), + "overdue": view.overdue, + "completed_at": view.completed_at, + "on_time": view.was_on_time, + } + ) + return payload + + async def build_snapshot( database: NestQuestDatabase, settings: NestQuestSettings, today: datetime.date, - time_zone: ZoneInfo, + now: datetime.datetime, ) -> NestQuestSnapshot: - """Build the household panel snapshot for ``today`` in ``time_zone``. + """Build the household panel snapshot for ``today`` at the ``now`` instant. Mirrors the assembly that lived in the coordinator's ``_async_update_data``: active children in sort order, each child's @@ -128,10 +171,21 @@ async def build_snapshot( per-child rollup counts, and the custody cycle day of the first scheduled active child. Missed instances are retained on the snapshot (the rollup counts and the panel's missed plate need them) - but the entity-facing attribute payload omits them at the sensor - layer — this builder produces the shared shape both consumers read. + but the entity-facing attribute payload omits them at the + :func:`instance_payload` layer — this builder produces the shared + shape both consumers read. + + ``now`` is the HA-local timezone-aware datetime the caller already + resolved (a SINGLE clock read); the overdue check compares + ``now.time()`` against the instance's ``due_time`` for today's open + instances. The builder performs NO internal clock read — it never + calls :func:`datetime.datetime.now` — so the instant is pinned in + tests and date and time can never disagree across two reads. + ``today`` is the calendar date the caller resolved from the same + clock (``now.date()``); it anchors the due-date and cycle-day + arithmetic. ``settings`` is accepted but currently unused (see the + module docstring). """ - now = datetime.datetime.now(time_zone) today_iso = today.isoformat() children = await ChildrenDao(database).list_active() diff --git a/custom_components/nestquest/sensor.py b/custom_components/nestquest/sensor.py index c340728..eef0417 100644 --- a/custom_components/nestquest/sensor.py +++ b/custom_components/nestquest/sensor.py @@ -39,6 +39,7 @@ from .coordinator import ( NestQuestCoordinator, QuestInstanceView, ) +from .core.snapshot import instance_payload #: Home Assistant's state machine refuses states longer than 255 #: characters (its hard recorder/state limit). These sensors' states @@ -63,40 +64,6 @@ def _state_within_limit(state: Any) -> Any: return state -def _instance_payload( - instances: Iterable[QuestInstanceView], *, include_missed: bool -) -> list[dict[str, Any]]: - """Shape instance dicts per ENTITIES-AND-SERVICES.md §1. - - ``state`` maps the coordinator's derived ``done`` to the panel's - ``completed`` spelling; ``missed`` instances are omitted unless - ``include_missed`` (the admin payload, D-009, keeps them with - their ``missed`` state). - """ - payload: list[dict[str, Any]] = [] - for view in instances: - if view.state == "missed" and not include_missed: - continue - payload.append( - { - "id": view.instance_id, - "definition_id": view.definition_id, - "child_id": view.child_id, - "title": view.title, - "icon": view.icon, - "window": view.window, - "due_time": view.due_time, - "state": ( - "completed" if view.state == "done" else view.state - ), - "overdue": view.overdue, - "completed_at": view.completed_at, - "on_time": view.was_on_time, - } - ) - return payload - - #: State reported by the next-quest sensor when the child owes nothing #: else today. A documented sentinel, never an empty string (an empty #: state reads as "sensor went unavailable" in HA). @@ -215,10 +182,10 @@ class NestQuestQuestsDueTodaySensor(_NestQuestChildDaySensor): if child is None: return None return { - "instances": _instance_payload( + "instances": instance_payload( child.instances, include_missed=False ), - "admin_instances": _instance_payload( + "admin_instances": instance_payload( child.instances, include_missed=True ), "child_id": child.child_id, diff --git a/tests/test_core_snapshot.py b/tests/test_core_snapshot.py index da4e29a..6bccf3e 100644 --- a/tests/test_core_snapshot.py +++ b/tests/test_core_snapshot.py @@ -2,12 +2,14 @@ The snapshot assembly that lived inline in the coordinator's ``_async_update_data`` was extracted into the Home-Assistant-free -:func:`custom_components.nestquest.core.snapshot.build_snapshot`. This -test seeds a household (active + inactive children, a daily quest -definition, today's instances, one completed instance, and a presence -schedule) and asserts the pure builder's output is byte-for-byte equal -to the coordinator's snapshot for the SAME database and date — the -shared-implementation invariant the API panel snapshot route depends on. +:func:`custom_components.nestquest.core.snapshot.build_snapshot`, and +the panel payload shaper (:func:`instance_payload`) with it. These +tests seed a household and assert the pure builder's output against +EXPLICIT literal expected values — not self-equality — so the AWAY +child's ``next_present``, a due-timed quest's ``overdue`` under a +pinned ``now``, the custody ``cycle_day`` (> 1), and the panel payload +dict shape (``done`` -> ``completed``, ``on_time`` passthrough, and +``missed`` omission) are all exercised as literals. """ from __future__ import annotations @@ -17,166 +19,273 @@ from zoneinfo import ZoneInfo from conftest import wire_entry_to_registry from custom_components.nestquest import async_setup_entry -from custom_components.nestquest.children import ( - create_child, - list_children, - set_child_active, -) +from custom_components.nestquest.children import create_child from custom_components.nestquest.completion import complete_instance from custom_components.nestquest.core.snapshot import ( ChildDaySnapshot, NestQuestSnapshot, QuestInstanceView, build_snapshot, + instance_payload, +) +from custom_components.nestquest.dao_instances import QuestInstancesDao +from custom_components.nestquest.dao_presence import ( + PresenceOverridesDao, + PresenceSchedulesDao, ) -from custom_components.nestquest.dao_presence import PresenceSchedulesDao from custom_components.nestquest.materialize import materialize from custom_components.nestquest.quest_definitions import create_quest_definition from custom_components.nestquest.recurrence import ScheduleRule -SEED_HORIZON_DAYS = 3 +async def test_instance_payload_shape_and_missed_omission() -> None: + """``instance_payload`` is the documented panel shape (§1). -def _today() -> datetime.date: - return datetime.date.today() - - -async def _seed_household(database): - """Three active children + one inactive; a daily quest for two of them. - - Ada and Bo share a daily morning quest; Cory is active but has no - quest (exercises the empty-instance / 100%-completion path); Ghost - is inactive and must be absent from the snapshot. Ada's first - instance is completed so the ``done`` state and rollup counts are - exercised, not just the ``open`` default. + ``done`` maps to ``completed``; ``open`` passes through; ``on_time`` + is the builder's ``was_on_time`` verbatim; ``missed`` instances are + omitted unless ``include_missed`` (the admin payload, D-009). """ - ada = await create_child(database, "Ada") - bo = await create_child(database, "Bo") - cory = await create_child(database, "Cory") - ghost = await create_child(database, "Ghost") - await set_child_active(database, ghost.id, False) + def view(state: str, *, overdue: bool = False, on_time: bool | None = None, + completed_at: str | None = None, instance_id: int = 1) -> QuestInstanceView: + return QuestInstanceView( + instance_id=instance_id, + definition_id=10, + child_id=3, + title="Fill bird feeder", + icon="bird", + window="afternoon", + due_date="2026-09-21", + due_time="17:00", + state=state, + overdue=overdue, + completed_at=completed_at, + was_on_time=on_time, + ) - today = _today() - await create_quest_definition( - database, - "Brush teeth", - ScheduleRule.from_dict( - {"rule_type": "daily", "start_date": today.isoformat()} - ), - [ada.id, bo.id], - ["morning"], + open_view = view("open", overdue=True) + done_view = view("done", on_time=True, completed_at="2026-09-21T12:00:00+00:00") + missed_view = view("missed", instance_id=2) + + panel = instance_payload([open_view, done_view, missed_view], include_missed=False) + assert panel == [ + { + "id": 1, + "definition_id": 10, + "child_id": 3, + "title": "Fill bird feeder", + "icon": "bird", + "window": "afternoon", + "due_time": "17:00", + "state": "open", + "overdue": True, + "completed_at": None, + "on_time": None, + }, + { + "id": 1, + "definition_id": 10, + "child_id": 3, + "title": "Fill bird feeder", + "icon": "bird", + "window": "afternoon", + "due_time": "17:00", + "state": "completed", + "overdue": False, + "completed_at": "2026-09-21T12:00:00+00:00", + "on_time": True, + }, + ], "panel payload omits missed and spells done as completed" + + admin = instance_payload( + [open_view, done_view, missed_view], include_missed=True ) - end = (today + datetime.timedelta(days=SEED_HORIZON_DAYS)).isoformat() - await materialize(database, today.isoformat(), end, today=today) - - # A 1-week presence schedule for Ada, present every day, anchored - # today: exercises cycle_day (== 1) and the present=True path. Bo - # and Cory have no schedule, so the engine treats them as present - # every day (the documented default). - await PresenceSchedulesDao(database).upsert_by_child( - ada.id, - cycle_length_weeks=1, - anchor_date=today.isoformat(), - pattern="0,1,2,3,4,5,6", + assert len(admin) == 3 + assert admin[2]["state"] == "missed", ( + "admin payload keeps missed instances with their missed state" ) - return ada, bo, cory, ghost -async def test_build_snapshot_matches_coordinator_snapshot( - hass, make_entry -) -> None: - """The pure builder and the coordinator produce identical snapshots.""" +async def test_build_snapshot_explicit_values(hass, make_entry) -> None: + """The builder produces the documented shape as literals, pinned in time. + + Ada is AWAY today (a 2-week schedule whose absent week 0 covers today, + plus a presence override marking her present on today+2) so + ``present`` is False and ``next_present`` is the override date. Her + open quest due at 10:00 is OVERDUE under the pinned 12:00 ``now``. + Bo's quest due at 17:00 is completed at 12:00 (on time) so its + payload ``state`` is ``completed`` and ``on_time`` is True. The first + scheduled active child is Ada, whose 2-week cycle anchored one day + before today puts ``cycle_day`` at 2. + """ entry = wire_entry_to_registry(make_entry(), hass.registry) assert await async_setup_entry(hass, entry) is True coordinator = entry.runtime_data.coordinator database = entry.runtime_data.database settings = entry.runtime_data.settings - ada, bo, cory, ghost = await _seed_household(database) - # Complete Ada's first open instance through the business layer so - # the ``done`` state, completed_at/was_on_time, and the rollup - # counts are exercised alongside the ``open`` default for Bo. - active_children = await list_children(database, active_only=True) - ada_id = next(child.id for child in active_children if child.display_name == "Ada") - bo_id = next(child.id for child in active_children if child.display_name == "Bo") - await coordinator.async_refresh() - ada_snapshot = next( - child for child in coordinator.data.children if child.child_id == ada_id + # Single clock read for the seed/snapshot anchor (no midnight skew). + today = coordinator._local_now().date() + time_zone = ZoneInfo(hass.config.time_zone) + # Pin the build instant at 12:00 local: past Ada's 10:00 due time + # (overdue) and before Bo's 17:00, and the completion moment for Bo. + pinned_now = datetime.datetime( + today.year, today.month, today.day, 12, 0, 0, tzinfo=time_zone + ) + today_iso = today.isoformat() + + ada = await create_child(database, "Ada") + bo = await create_child(database, "Bo") + + await create_quest_definition( + database, + "Pack bag", + ScheduleRule.from_dict( + {"rule_type": "daily", "start_date": today_iso} + ), + [ada.id], + [("morning", "10:00")], + ) + await create_quest_definition( + database, + "Brush teeth", + ScheduleRule.from_dict( + {"rule_type": "daily", "start_date": today_iso} + ), + [bo.id], + [("morning", "17:00")], + ) + end = (today + datetime.timedelta(days=3)).isoformat() + await materialize(database, today_iso, end, today=today) + + # Ada: 2-week cycle anchored yesterday. Week 0 (the empty segment) + # covers today, so she is absent today; an override marks her present + # on today+2, so next_present resolves to that date and cycle_day is + # ((today - (today-1)) % 14) + 1 == 2. + await PresenceSchedulesDao(database).upsert_by_child( + ada.id, + cycle_length_weeks=2, + anchor_date=(today - datetime.timedelta(days=1)).isoformat(), + pattern="|0,1,2,3,4,5,6", + ) + returns_date = today + datetime.timedelta(days=2) + await PresenceOverridesDao(database).create( + ada.id, + returns_date.isoformat(), + returns_date.isoformat(), + is_present=True, + note="visit", + ) + + # Complete Bo's instance at the pinned 12:00 (17:00 due => on time). + bo_instances = await QuestInstancesDao(database).list_by_child_and_date( + bo.id, today_iso ) await complete_instance( database, - ada_snapshot.instances[0].instance_id, + bo_instances[0].id, actor_source="panel", - actor_child_id=ada_id, + actor_child_id=bo.id, + now=pinned_now, + today=today, ) - # Drive the coordinator's refresh (the path production takes) and - # the pure builder with the SAME date and timezone the coordinator - # resolves, so the comparison is exact — no host-clock skew, no - # timezone drift. Instances here carry no due_time, so the overdue - # flag is False regardless of the instant each pass reads ``now``. - await coordinator.async_refresh() - coordinator_snapshot = coordinator.data - today = coordinator._local_now().date() - time_zone = ZoneInfo(hass.config.time_zone) - built = await build_snapshot(database, settings, today, time_zone) + built = await build_snapshot(database, settings, today, pinned_now) assert isinstance(built, NestQuestSnapshot) - assert built == coordinator_snapshot, ( - "build_snapshot and the coordinator must produce identical " - "snapshots for the same database, date, and timezone" + assert built.today_iso == today_iso + assert built.cycle_day == 2, ( + "Ada's 2-week cycle anchored yesterday => day 2 today" ) - # Shape and documented invariants, not just equality: children in - # sort order, inactive child omitted, missed instances omitted from - # the panel payload (none are missed in this seed, so the instances - # tuple carries only open/done views), per-child counts, presence, - # and cycle day. - active_ordered = await list_children(database, active_only=True) - active_ids_ordered = [child.id for child in active_ordered] - child_ids = [child.child_id for child in built.children] - assert ghost.id not in child_ids, "inactive child must be absent" - assert child_ids == active_ids_ordered, ( - "children must appear in the DAO's sort_order, id order" - ) - assert built.today_iso == today.isoformat() - assert built.cycle_day == 1, "Ada's 1-week schedule anchored today => day 1" - by_id = {child.child_id: child for child in built.children} - ada_child = by_id[ada_id] - bo_child = by_id[bo_id] - cory_child = by_id[cory.id] - for child in built.children: - assert isinstance(child, ChildDaySnapshot) - for view in child.instances: - assert isinstance(view, QuestInstanceView) - assert view.state in ("open", "done", "missed") - # Missed instances are omitted from the panel payload shape; - # in this seed none are missed (today's open instances have - # not crossed their due time and there is no due_time set). - assert view.state != "missed", ( - "no instance should be missed for today's seed" - ) - - assert ada_child.present is True - assert bo_child.present is True - assert cory_child.present is True - assert cory_child.instances == () - assert cory_child.due_today == 0 - assert cory_child.completion_pct == 100 + assert list(by_id) == [ada.id, bo.id], ( + "active children appear in sort_order, id order" + ) + ada_child = by_id[ada.id] + bo_child = by_id[bo.id] + # Ada: away today, returns on the override date, one open+overdue quest. + assert isinstance(ada_child, ChildDaySnapshot) + assert ada_child.present is False + assert ada_child.next_present == returns_date.isoformat() assert ada_child.due_today == 1 - assert ada_child.completed_today == 1 - assert ada_child.remaining_today == 0 - assert ada_child.completion_pct == 100 - assert ada_child.instances[0].state == "done" - assert ada_child.instances[0].completed_at is not None - assert ada_child.instances[0].was_on_time is not None + assert ada_child.completed_today == 0 + assert ada_child.remaining_today == 1 + assert ada_child.completion_pct == 0 + ada_view = ada_child.instances[0] + assert isinstance(ada_view, QuestInstanceView) + assert ada_view.state == "open" + assert ada_view.due_time == "10:00" + assert ada_view.overdue is True, "12:00 now is past the 10:00 due time" + assert ada_view.completed_at is None + assert ada_view.was_on_time is None + # Bo: present, one completed instance, on time, not overdue. + assert bo_child.present is True + assert bo_child.next_present is None assert bo_child.due_today == 1 - assert bo_child.completed_today == 0 - assert bo_child.remaining_today == 1 - assert bo_child.completion_pct == 0 - assert bo_child.instances[0].state == "open" - assert bo_child.instances[0].completed_at is None - assert bo_child.instances[0].was_on_time is None + assert bo_child.completed_today == 1 + assert bo_child.remaining_today == 0 + assert bo_child.completion_pct == 100 + bo_view = bo_child.instances[0] + assert bo_view.state == "done" + assert bo_view.due_time == "17:00" + assert bo_view.overdue is False, "overdue is only computed for open state" + assert bo_view.completed_at == f"{today_iso}T12:00:00+00:00" + assert bo_view.was_on_time is True + + # Panel payload shape: Ada open+overdue stays open; Bo done -> completed. + ada_payload = instance_payload(ada_child.instances, include_missed=False) + assert ada_payload == [ + { + "id": ada_view.instance_id, + "definition_id": ada_view.definition_id, + "child_id": ada.id, + "title": "Pack bag", + "icon": None, + "window": "morning", + "due_time": "10:00", + "state": "open", + "overdue": True, + "completed_at": None, + "on_time": None, + } + ] + bo_payload = instance_payload(bo_child.instances, include_missed=False) + assert bo_payload == [ + { + "id": bo_view.instance_id, + "definition_id": bo_view.definition_id, + "child_id": bo.id, + "title": "Brush teeth", + "icon": None, + "window": "morning", + "due_time": "17:00", + "state": "completed", + "overdue": False, + "completed_at": f"{today_iso}T12:00:00+00:00", + "on_time": True, + } + ] + + # Missed omission is real: a missed view is dropped from the panel + # payload and kept in the admin payload with its missed state. + missed_view = QuestInstanceView( + instance_id=999, + definition_id=10, + child_id=ada.id, + title="Stale quest", + icon=None, + window="morning", + due_date=(today - datetime.timedelta(days=1)).isoformat(), + due_time="09:00", + state="missed", + overdue=False, + completed_at=None, + was_on_time=None, + ) + assert instance_payload([missed_view], include_missed=False) == [] + assert ( + instance_payload([missed_view], include_missed=True)[0]["state"] + == "missed" + ) -- 2.55.0 From 5d2357ee30a7707ee6177a57af99178cdbc44e06 Mon Sep 17 00:00:00 2001 From: Joshua Date: Mon, 21 Sep 2026 17:40:51 -0400 Subject: [PATCH 3/3] Tie coordinator wrapper to snapshot builder; single clock read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_snapshot now takes now ONLY and derives today = now.date() inside (P3-1): a single source of truth so the calendar date and wall-clock time can never disagree across two reads. Coordinator call site updated. The expected completed_at in tests/test_core_snapshot.py is now built from pinned_now.astimezone(datetime.timezone.utc).isoformat() instead of a literal '+00:00' UTC string (P3-2), so the assertion is timezone-independent rather than relying on the hass fixture's UTC timezone. Pin the coordinator's clock to pinned_now, run async_refresh, and assert coordinator.data == built (the frozen dataclasses make == a full deep compare) — the REQUIRED tie-in that proves the coordinator wrapper actually delegates to build_snapshot, which the literal assertions alone could not (P2). --- custom_components/nestquest/coordinator.py | 3 +-- custom_components/nestquest/core/snapshot.py | 19 ++++++++-------- tests/test_core_snapshot.py | 23 ++++++++++++++++---- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/custom_components/nestquest/coordinator.py b/custom_components/nestquest/coordinator.py index 647bbaf..5a26876 100644 --- a/custom_components/nestquest/coordinator.py +++ b/custom_components/nestquest/coordinator.py @@ -117,7 +117,6 @@ class NestQuestCoordinator(DataUpdateCoordinator): async def _async_update_data(self) -> NestQuestSnapshot: now = self._local_now() - today = now.date() return await build_snapshot( - self.database, self.settings, today, now + self.database, self.settings, now ) diff --git a/custom_components/nestquest/core/snapshot.py b/custom_components/nestquest/core/snapshot.py index f54fe61..0236960 100644 --- a/custom_components/nestquest/core/snapshot.py +++ b/custom_components/nestquest/core/snapshot.py @@ -18,9 +18,10 @@ the panel payload shaper (:func:`instance_payload`), and the async service's panel snapshot route share ONE implementation of both the snapshot assembly and the entity-facing attribute payload shape. Nothing here imports :mod:`homeassistant`; the builder takes the -database, the settings object, the target date, and the HA-local -``now`` (a single clock read the caller already resolved) and returns -the snapshot, mirroring exactly what the coordinator used to do inline. +database, the settings object, and the HA-local ``now`` (a single +clock read the caller already resolved, from which it derives +``today``) and returns the snapshot, mirroring exactly what the +coordinator used to do inline. ``settings`` is accepted because the API route hands in the entry's :class:`~.settings.NestQuestSettings`; it is CURRENTLY UNUSED — the @@ -158,7 +159,6 @@ def instance_payload( async def build_snapshot( database: NestQuestDatabase, settings: NestQuestSettings, - today: datetime.date, now: datetime.datetime, ) -> NestQuestSnapshot: """Build the household panel snapshot for ``today`` at the ``now`` instant. @@ -180,12 +180,13 @@ async def build_snapshot( ``now.time()`` against the instance's ``due_time`` for today's open instances. The builder performs NO internal clock read — it never calls :func:`datetime.datetime.now` — so the instant is pinned in - tests and date and time can never disagree across two reads. - ``today`` is the calendar date the caller resolved from the same - clock (``now.date()``); it anchors the due-date and cycle-day - arithmetic. ``settings`` is accepted but currently unused (see the - module docstring). + tests. ``today`` is derived INSIDE the builder as ``now.date()``, + so the calendar date that anchors the due-date and cycle-day + arithmetic is always the SAME clock read as the wall-clock time; + the two can never disagree. ``settings`` is accepted but currently + unused (see the module docstring). """ + today = now.date() today_iso = today.isoformat() children = await ChildrenDao(database).list_active() diff --git a/tests/test_core_snapshot.py b/tests/test_core_snapshot.py index 6bccf3e..e795989 100644 --- a/tests/test_core_snapshot.py +++ b/tests/test_core_snapshot.py @@ -105,7 +105,7 @@ async def test_instance_payload_shape_and_missed_omission() -> None: ) -async def test_build_snapshot_explicit_values(hass, make_entry) -> None: +async def test_build_snapshot_explicit_values(hass, make_entry, monkeypatch) -> None: """The builder produces the documented shape as literals, pinned in time. Ada is AWAY today (a 2-week schedule whose absent week 0 covers today, @@ -189,7 +189,7 @@ async def test_build_snapshot_explicit_values(hass, make_entry) -> None: today=today, ) - built = await build_snapshot(database, settings, today, pinned_now) + built = await build_snapshot(database, settings, pinned_now) assert isinstance(built, NestQuestSnapshot) assert built.today_iso == today_iso @@ -231,7 +231,9 @@ async def test_build_snapshot_explicit_values(hass, make_entry) -> None: assert bo_view.state == "done" assert bo_view.due_time == "17:00" assert bo_view.overdue is False, "overdue is only computed for open state" - assert bo_view.completed_at == f"{today_iso}T12:00:00+00:00" + assert bo_view.completed_at == pinned_now.astimezone( + datetime.timezone.utc + ).isoformat() assert bo_view.was_on_time is True # Panel payload shape: Ada open+overdue stays open; Bo done -> completed. @@ -263,7 +265,9 @@ async def test_build_snapshot_explicit_values(hass, make_entry) -> None: "due_time": "17:00", "state": "completed", "overdue": False, - "completed_at": f"{today_iso}T12:00:00+00:00", + "completed_at": pinned_now.astimezone( + datetime.timezone.utc + ).isoformat(), "on_time": True, } ] @@ -289,3 +293,14 @@ async def test_build_snapshot_explicit_values(hass, make_entry) -> None: instance_payload([missed_view], include_missed=True)[0]["state"] == "missed" ) + + # REQUIRED wrapper tie-in: the coordinator's _async_update_data is a + # thin WRAPPER over build_snapshot, so pinning its clock to the SAME + # pinned_now the builder was called with must reproduce the builder's + # snapshot byte-for-byte. The dataclasses are frozen, so == is a full + # deep compare — a wrapper that mis-passed today vs now, or stopped + # calling the builder, would diverge here even though the literal + # assertions above still pass. + monkeypatch.setattr(coordinator, "_local_now", lambda: pinned_now) + await coordinator.async_refresh() + assert coordinator.data == built -- 2.55.0