From 87c7fa0c5cab5d00f7383ca9ce15be4136e84c3d Mon Sep 17 00:00:00 2001 From: Joshua Date: Fri, 18 Sep 2026 06:56:18 -0400 Subject: [PATCH 1/3] feat: compute was_on_time at complete_instance --- custom_components/nestquest/completion.py | 65 +++++++++++-- tests/test_completion.py | 110 ++++++++++++++++++++++ 2 files changed, 169 insertions(+), 6 deletions(-) diff --git a/custom_components/nestquest/completion.py b/custom_components/nestquest/completion.py index 3a4108e..9b03108 100644 --- a/custom_components/nestquest/completion.py +++ b/custom_components/nestquest/completion.py @@ -35,6 +35,7 @@ from .dao_instances import ( EVENT_UNCOMPLETED, CompletionEventRecord, CompletionEventsDao, + QuestInstanceRecord, QuestInstancesDao, ) from .db import NestQuestDatabase @@ -64,6 +65,59 @@ def _stamp_occurred_at(now: datetime.datetime | None) -> str: ) +def _completion_moment( + now: datetime.datetime | None, + today: datetime.date | None, +) -> tuple[datetime.date, datetime.time]: + """Return the HA-local completion date and wall-clock time. + + ``today`` is the calendar date when the caller threaded one (Feature + 07 style). ``now`` supplies the clock time; a timezone-aware value + is read as HA-local wall clock (hour/minute/second as given), not + converted. When ``today`` is omitted the date comes from ``now``. + When both are omitted the UTC clock used for occurred_at is the + fallback — host ``date.today()`` is never the authority. + """ + if now is None: + moment = datetime.datetime.now(datetime.timezone.utc) + else: + if not isinstance(now, datetime.datetime): + raise ValueError(f"now must be a datetime, got {now!r}") + moment = now + if today is None: + completion_date = moment.date() + else: + completion_date = today + completion_time = datetime.time( + moment.hour, moment.minute, moment.second + ) + return completion_date, completion_time + + +def _was_on_time( + instance: QuestInstanceRecord, + now: datetime.datetime | None, + today: datetime.date | None, +) -> bool: + """Return whether the completion moment is on time for ``instance``. + + On ``due_date`` before ``due_time`` is on time; on ``due_date`` + after ``due_time``, or any later date, is late. With no + ``due_time``, on time if the completion date equals ``due_date`` + (or is earlier); late if after ``due_date``. + """ + completion_date, completion_time = _completion_moment(now, today) + due_date = datetime.date.fromisoformat(instance.due_date) + if completion_date < due_date: + return True + if completion_date > due_date: + return False + if instance.due_time is None: + return True + due_time = datetime.datetime.strptime(instance.due_time, "%H:%M").time() + return completion_time <= due_time + + class _TaskReentrantLock: """Shared connection lock the owning task may re-enter.""" @@ -278,11 +332,10 @@ async def complete_instance( append run under one connection lock so a concurrent complete cannot double-append. - ``was_on_time`` is recorded as True — a placeholder until the - on-time task computes it from ``now`` / ``today`` against the - instance due_date and optional due_time. ``today`` is accepted so - callers can thread the HA-local date through without a signature - change; it is unused here. + ``was_on_time`` is computed from the HA-local completion moment + (threaded ``now`` / ``today``) against the instance ``due_date`` + and optional ``due_time``. Host ``date.today()`` is never the + authority. """ instance_id = _validate_int_id(instance_id, "instance_id") actor_source, actor_user_id, actor_child_id = _validate_actor( @@ -312,7 +365,7 @@ async def complete_instance( actor_source=actor_source, actor_user_id=actor_user_id, actor_child_id=actor_child_id, - was_on_time=True, + was_on_time=_was_on_time(instance, now, today), now=now, ) return "done" diff --git a/tests/test_completion.py b/tests/test_completion.py index ac54dbd..eb0060d 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -687,6 +687,116 @@ def test_complete_instance_concurrent_second_call_is_noop(tmp_path) -> None: _run(_main()) +def _ha_local(day: datetime.date, hour: int, minute: int = 0) -> datetime.datetime: + return datetime.datetime( + day.year, day.month, day.day, hour, minute, 0, + tzinfo=datetime.timezone.utc, + ) + + +def test_complete_instance_on_due_date_before_due_time_is_on_time( + tmp_path, +) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + instance = await QuestInstancesDao(database).upsert( + instance.definition_id, + child.id, + instance.due_date, + _now_stamp(), + window=instance.window, + due_time="12:00", + ) + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=_ha_local(due_date, 11, 0), + today=due_date, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is True + + _with_db(tmp_path, "ontime-before-due-time.db")(_body) + + +def test_complete_instance_on_due_date_after_due_time_is_late( + tmp_path, +) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + instance = await QuestInstancesDao(database).upsert( + instance.definition_id, + child.id, + instance.due_date, + _now_stamp(), + window=instance.window, + due_time="12:00", + ) + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=_ha_local(due_date, 12, 1), + today=due_date, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is False + + _with_db(tmp_path, "late-after-due-time.db")(_body) + + +def test_complete_instance_no_due_time_on_due_date_is_on_time( + tmp_path, +) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + assert instance.due_time is None + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=_ha_local(due_date, 23, 59), + today=due_date, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is True + + _with_db(tmp_path, "ontime-no-due-time.db")(_body) + + +def test_complete_instance_no_due_time_after_due_date_is_late( + tmp_path, +) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + later = due_date + datetime.timedelta(days=1) + assert instance.due_time is None + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=_ha_local(later, 8, 0), + today=later, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is False + + _with_db(tmp_path, "late-after-due-date.db")(_body) + + # --------------------------------------------------------------------------- # uncomplete_instance: append uncompleted, no-op if already open # --------------------------------------------------------------------------- -- 2.55.0 From 39f6f9294f96fc039a6c503addb8fd9f7392e9be Mon Sep 17 00:00:00 2001 From: Joshua Date: Fri, 18 Sep 2026 07:00:19 -0400 Subject: [PATCH 2/3] fix: was_on_time uses HA-local not UTC fallback --- custom_components/nestquest/completion.py | 50 ++++++++++-------- tests/test_completion.py | 62 ++++++++++++++++++++++- 2 files changed, 89 insertions(+), 23 deletions(-) diff --git a/custom_components/nestquest/completion.py b/custom_components/nestquest/completion.py index 9b03108..a6adf35 100644 --- a/custom_components/nestquest/completion.py +++ b/custom_components/nestquest/completion.py @@ -68,30 +68,31 @@ def _stamp_occurred_at(now: datetime.datetime | None) -> str: def _completion_moment( now: datetime.datetime | None, today: datetime.date | None, -) -> tuple[datetime.date, datetime.time]: - """Return the HA-local completion date and wall-clock time. +) -> tuple[datetime.date, datetime.time | None]: + """Return the HA-local completion date and optional wall-clock time. - ``today`` is the calendar date when the caller threaded one (Feature - 07 style). ``now`` supplies the clock time; a timezone-aware value - is read as HA-local wall clock (hour/minute/second as given), not - converted. When ``today`` is omitted the date comes from ``now``. - When both are omitted the UTC clock used for occurred_at is the - fallback — host ``date.today()`` is never the authority. + ``today`` is the HA-local calendar date when threaded (Feature 07 + style). ``now`` is a timezone-aware HA-local datetime; its wall + clock is used as-is and is never converted to UTC. When ``today`` + is omitted the date comes from ``now``. When both are omitted the + host calendar date is the Feature 07 fallback — never the UTC + clock. Time-of-day is None unless ``now`` is supplied; comparing + against ``due_time`` requires ``now``. """ - if now is None: - moment = datetime.datetime.now(datetime.timezone.utc) - else: + if now is not None: if not isinstance(now, datetime.datetime): raise ValueError(f"now must be a datetime, got {now!r}") - moment = now - if today is None: - completion_date = moment.date() - else: + if now.tzinfo is None: + raise ValueError("now must be timezone-aware") + if today is not None: completion_date = today - completion_time = datetime.time( - moment.hour, moment.minute, moment.second - ) - return completion_date, completion_time + elif now is not None: + completion_date = now.date() + else: + completion_date = datetime.date.today() + if now is None: + return completion_date, None + return completion_date, datetime.time(now.hour, now.minute, now.second) def _was_on_time( @@ -104,7 +105,9 @@ def _was_on_time( On ``due_date`` before ``due_time`` is on time; on ``due_date`` after ``due_time``, or any later date, is late. With no ``due_time``, on time if the completion date equals ``due_date`` - (or is earlier); late if after ``due_date``. + (or is earlier); late if after ``due_date``. Same-day + ``due_time`` comparison requires ``now``; UTC time-of-day is never + substituted. """ completion_date, completion_time = _completion_moment(now, today) due_date = datetime.date.fromisoformat(instance.due_date) @@ -114,6 +117,8 @@ def _was_on_time( return False if instance.due_time is None: return True + if completion_time is None: + raise ValueError("now is required to compare due_time") due_time = datetime.datetime.strptime(instance.due_time, "%H:%M").time() return completion_time <= due_time @@ -334,8 +339,9 @@ async def complete_instance( ``was_on_time`` is computed from the HA-local completion moment (threaded ``now`` / ``today``) against the instance ``due_date`` - and optional ``due_time``. Host ``date.today()`` is never the - authority. + and optional ``due_time``. ``now`` is the timezone-aware HA-local + datetime; its wall clock is never converted to UTC. Same-day + ``due_time`` comparison requires ``now``. """ instance_id = _validate_int_id(instance_id, "instance_id") actor_source, actor_user_id, actor_child_id = _validate_actor( diff --git a/tests/test_completion.py b/tests/test_completion.py index eb0060d..2a76954 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -687,10 +687,13 @@ def test_complete_instance_concurrent_second_call_is_noop(tmp_path) -> None: _run(_main()) +_HA_TZ = datetime.timezone(datetime.timedelta(hours=-8)) + + def _ha_local(day: datetime.date, hour: int, minute: int = 0) -> datetime.datetime: return datetime.datetime( day.year, day.month, day.day, hour, minute, 0, - tzinfo=datetime.timezone.utc, + tzinfo=_HA_TZ, ) @@ -797,6 +800,63 @@ def test_complete_instance_no_due_time_after_due_date_is_late( _with_db(tmp_path, "late-after-due-date.db")(_body) +def test_complete_instance_ha_local_wall_clock_not_utc(tmp_path) -> None: + """HA-local 11:00 with due_time 12:00 is on time even when UTC is 19:00. + + The helper's zone is UTC-8, so 11:00 local is 19:00 UTC. Converting + to UTC before comparing would mark this late. + """ + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + instance = await QuestInstancesDao(database).upsert( + instance.definition_id, + child.id, + instance.due_date, + _now_stamp(), + window=instance.window, + due_time="12:00", + ) + now = _ha_local(due_date, 11, 0) + assert now.astimezone(datetime.timezone.utc).hour == 19 + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=now, + today=due_date, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is True + + _with_db(tmp_path, "ontime-ha-local-not-utc.db")(_body) + + +def test_complete_instance_due_time_requires_now_on_due_date(tmp_path) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + instance = await QuestInstancesDao(database).upsert( + instance.definition_id, + child.id, + instance.due_date, + _now_stamp(), + window=instance.window, + due_time="12:00", + ) + with pytest.raises(ValueError, match="now is required to compare due_time"): + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + today=due_date, + ) + + _with_db(tmp_path, "due-time-requires-now.db")(_body) + + # --------------------------------------------------------------------------- # uncomplete_instance: append uncompleted, no-op if already open # --------------------------------------------------------------------------- -- 2.55.0 From 4363ec4b300598f4d98fecc4c724fede45fd18f7 Mon Sep 17 00:00:00 2001 From: Joshua Date: Fri, 18 Sep 2026 07:01:46 -0400 Subject: [PATCH 3/3] fix: preserve microseconds in was_on_time compare --- custom_components/nestquest/completion.py | 2 +- tests/test_completion.py | 31 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/custom_components/nestquest/completion.py b/custom_components/nestquest/completion.py index a6adf35..3f22089 100644 --- a/custom_components/nestquest/completion.py +++ b/custom_components/nestquest/completion.py @@ -92,7 +92,7 @@ def _completion_moment( completion_date = datetime.date.today() if now is None: return completion_date, None - return completion_date, datetime.time(now.hour, now.minute, now.second) + return completion_date, now.time() def _was_on_time( diff --git a/tests/test_completion.py b/tests/test_completion.py index 2a76954..da2493e 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -755,6 +755,37 @@ def test_complete_instance_on_due_date_after_due_time_is_late( _with_db(tmp_path, "late-after-due-time.db")(_body) +def test_complete_instance_microsecond_after_due_time_is_late(tmp_path) -> None: + async def _body(database, child, instance): + due_date = datetime.date.fromisoformat(instance.due_date) + instance = await QuestInstancesDao(database).upsert( + instance.definition_id, + child.id, + instance.due_date, + _now_stamp(), + window=instance.window, + due_time="12:00", + ) + now = datetime.datetime( + due_date.year, due_date.month, due_date.day, 12, 0, 0, 500, + tzinfo=_HA_TZ, + ) + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=now, + today=due_date, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].was_on_time is False + + _with_db(tmp_path, "late-microsecond-after-due-time.db")(_body) + + def test_complete_instance_no_due_time_on_due_date_is_on_time( tmp_path, ) -> None: -- 2.55.0