fix: store the snapshot due_time, not the live value (NQ-75-CR-002)
This commit is contained in:
@@ -285,6 +285,7 @@ class QuestInstancesDao:
|
|||||||
generated_at: str,
|
generated_at: str,
|
||||||
*,
|
*,
|
||||||
window: str,
|
window: str,
|
||||||
|
due_time: str | None = None,
|
||||||
) -> QuestInstanceRecord | None:
|
) -> QuestInstanceRecord | None:
|
||||||
"""Insert one dated instance iff its tuple is still materializable.
|
"""Insert one dated instance iff its tuple is still materializable.
|
||||||
|
|
||||||
@@ -301,9 +302,14 @@ class QuestInstancesDao:
|
|||||||
- the (definition, child) assignment link still exists (a removed
|
- the (definition, child) assignment link still exists (a removed
|
||||||
assignment is skipped);
|
assignment is skipped);
|
||||||
- the window is still declared on the definition (a removed window
|
- the window is still declared on the definition (a removed window
|
||||||
is skipped), and its CURRENT ``due_time`` is read from the live
|
is skipped).
|
||||||
row so a due-time edit landing after the snapshot is honored,
|
|
||||||
never the stale snapshot value.
|
The live checks above ONLY decide whether the tuple is still
|
||||||
|
valid; they never read the window's ``due_time``. The instance is
|
||||||
|
written with the caller-supplied ``due_time`` — the value snapshotted
|
||||||
|
from the coherent input read — so a window-time edit landing
|
||||||
|
mid-walk cannot leak a newer time into a batch that was already
|
||||||
|
snapshotted against older rule/presence/assignment decisions.
|
||||||
|
|
||||||
Returns the stored instance, or ``None`` when a precondition
|
Returns the stored instance, or ``None`` when a precondition
|
||||||
failed — the tuple no longer materializable, which the walk SKIPS
|
failed — the tuple no longer materializable, which the walk SKIPS
|
||||||
@@ -330,8 +336,8 @@ class QuestInstancesDao:
|
|||||||
f"instances are never generated in the past: "
|
f"instances are never generated in the past: "
|
||||||
f"due_date {due_date!r} is before today {today!r}"
|
f"due_date {due_date!r} is before today {today!r}"
|
||||||
)
|
)
|
||||||
valid = await self._database.fetch_one(
|
still_valid = await self._database.fetch_one(
|
||||||
"SELECT w.due_time FROM quest_definitions d "
|
"SELECT 1 FROM quest_definitions d "
|
||||||
"JOIN quest_definition_assignees a "
|
"JOIN quest_definition_assignees a "
|
||||||
"ON a.definition_id = d.id "
|
"ON a.definition_id = d.id "
|
||||||
"JOIN children c ON c.id = a.child_id "
|
"JOIN children c ON c.id = a.child_id "
|
||||||
@@ -341,11 +347,10 @@ class QuestInstancesDao:
|
|||||||
"AND d.is_active = 1 AND c.is_active = 1 LIMIT 1",
|
"AND d.is_active = 1 AND c.is_active = 1 LIMIT 1",
|
||||||
(definition_id, child_id, window),
|
(definition_id, child_id, window),
|
||||||
)
|
)
|
||||||
if valid is None:
|
if still_valid is None:
|
||||||
# A config change invalidated the tuple since the
|
# A config change invalidated the tuple since the
|
||||||
# snapshot: skip it rather than abort the batch.
|
# snapshot: skip it rather than abort the batch.
|
||||||
return None
|
return None
|
||||||
due_time = valid[0]
|
|
||||||
existing = await self._database.fetch_one(
|
existing = await self._database.fetch_one(
|
||||||
"SELECT 1 FROM completion_events "
|
"SELECT 1 FROM completion_events "
|
||||||
"WHERE instance_id = (SELECT id FROM quest_instances "
|
"WHERE instance_id = (SELECT id FROM quest_instances "
|
||||||
|
|||||||
@@ -159,6 +159,7 @@ async def materialize(
|
|||||||
iso,
|
iso,
|
||||||
generated_at,
|
generated_at,
|
||||||
window=window.window,
|
window=window.window,
|
||||||
|
due_time=window.due_time,
|
||||||
)
|
)
|
||||||
if written is not None:
|
if written is not None:
|
||||||
count += 1
|
count += 1
|
||||||
|
|||||||
@@ -248,7 +248,7 @@ async def _materialize_with_change(
|
|||||||
seen = {"n": 0}
|
seen = {"n": 0}
|
||||||
|
|
||||||
async def _gated(self, definition_id, child_id, due_date, generated_at,
|
async def _gated(self, definition_id, child_id, due_date, generated_at,
|
||||||
*, window):
|
*, window, due_time=None):
|
||||||
seen["n"] += 1
|
seen["n"] += 1
|
||||||
if seen["n"] == 1:
|
if seen["n"] == 1:
|
||||||
seen["first"] = (definition_id, child_id, window)
|
seen["first"] = (definition_id, child_id, window)
|
||||||
@@ -256,7 +256,7 @@ async def _materialize_with_change(
|
|||||||
await release.wait()
|
await release.wait()
|
||||||
return await original_upsert(
|
return await original_upsert(
|
||||||
self, definition_id, child_id, due_date, generated_at,
|
self, definition_id, child_id, due_date, generated_at,
|
||||||
window=window,
|
window=window, due_time=due_time,
|
||||||
)
|
)
|
||||||
|
|
||||||
QuestInstancesDao.upsert_if_valid = _gated
|
QuestInstancesDao.upsert_if_valid = _gated
|
||||||
@@ -452,6 +452,44 @@ def test_materialize_uses_single_presence_snapshot_mid_walk(tmp_path) -> None:
|
|||||||
_with_db(tmp_path, "materialize-presence-snapshot.db")(_body)
|
_with_db(tmp_path, "materialize-presence-snapshot.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
def test_materialize_stores_snapshot_due_time_not_live(tmp_path) -> None:
|
||||||
|
async def _body(database):
|
||||||
|
children = ChildrenDao(database)
|
||||||
|
child = await children.create("Ada", NOW)
|
||||||
|
start = _future_monday()
|
||||||
|
start_iso = start.isoformat()
|
||||||
|
end_iso = (start + datetime.timedelta(days=2)).isoformat()
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database,
|
||||||
|
"Daily chore",
|
||||||
|
ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso),
|
||||||
|
[child.id],
|
||||||
|
[("morning", "09:00")],
|
||||||
|
)
|
||||||
|
|
||||||
|
async def _edit_due_time():
|
||||||
|
await QuestDefinitionsDao(database).upsert_window(
|
||||||
|
created.definition.id, "morning", due_time="10:15"
|
||||||
|
)
|
||||||
|
|
||||||
|
# The window's due_time is edited to 10:15 between the input
|
||||||
|
# snapshot and the insert. The walk must store the SNAPSHOT's
|
||||||
|
# 09:00, not the live 10:15 — the batch is governed entirely by
|
||||||
|
# the coherent snapshot it read.
|
||||||
|
count = await _materialize_with_change(
|
||||||
|
database, start_iso, end_iso, _edit_due_time
|
||||||
|
)
|
||||||
|
assert count == 3
|
||||||
|
records = await QuestInstancesDao(database).list_by_date_range(
|
||||||
|
child.id, start_iso, end_iso
|
||||||
|
)
|
||||||
|
assert len(records) == 3
|
||||||
|
assert {r.due_time for r in records} == {"09:00"}
|
||||||
|
return records
|
||||||
|
|
||||||
|
_with_db(tmp_path, "materialize-snapshot-due-time.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
def test_materialize_honours_overrides(tmp_path) -> None:
|
def test_materialize_honours_overrides(tmp_path) -> None:
|
||||||
async def _body(database):
|
async def _body(database):
|
||||||
from custom_components.nestquest.dao_presence import (
|
from custom_components.nestquest.dao_presence import (
|
||||||
|
|||||||
Reference in New Issue
Block a user