diff --git a/screen_locker/_status_data.py b/screen_locker/_status_data.py index 20d6a97..9901ccb 100644 --- a/screen_locker/_status_data.py +++ b/screen_locker/_status_data.py @@ -67,9 +67,8 @@ from screen_locker._sick_tracker import ( from screen_locker._wake_state import has_workout_skip_today from screen_locker._weekly_check import ( COUNTED_WORKOUT_TYPES, - MANUAL_WORKOUT_TYPE, - VERIFIED_WORKOUT_TYPES, WEEKLY_WORKOUT_MINIMUM, + count_day_credits, is_relaxed_day, ) @@ -82,9 +81,14 @@ _DEFAULT_LOG_FILE = Path(__file__).resolve().parent / "log.json" def _day_status(day_date: date, entries: list[dict], sick_days: set[str]) -> DayStatus: """Build a :class:`DayStatus` for one calendar day from its entry list. - ``day_count`` mirrors the weekly rule (each verified workout counts, all of - a day's manual entries count once) so the week total sums to - :func:`~screen_locker._weekly_check.count_weekly_workouts`. + ``day_count`` comes from + :func:`~screen_locker._weekly_check.count_day_credits`, the same rule + enforcement uses, so the week total sums to + :func:`~screen_locker._weekly_check.count_weekly_workouts` by construction. + It used to re-derive the rule as ``verified + (1 if has_manual else 0)``, + which disagreed on any day with two manual workouts -- and since this value + feeds the status view's ``weekly_minimum_met`` explanation, the window could + explain a different decision than the locker actually made. """ iso = day_date.isoformat() label = day_date.strftime("%a %b %d") @@ -93,8 +97,6 @@ def _day_status(day_date: date, entries: list[dict], sick_days: set[str]) -> Day for e in entries if isinstance(e, dict) ) - verified = sum(1 for t in types if t in VERIFIED_WORKOUT_TYPES) - has_manual = any(t == MANUAL_WORKOUT_TYPE for t in types) sources = [ str(e.get("workout_data", {}).get("source", "")) for e in entries @@ -106,7 +108,7 @@ def _day_status(day_date: date, entries: list[dict], sick_days: set[str]) -> Day entry_types=tuple(t for t in types if t), source=" · ".join(s for s in sources if s), counted=any(t in COUNTED_WORKOUT_TYPES for t in types), - day_count=verified + (1 if has_manual else 0), + day_count=count_day_credits(iso, [e for e in entries if isinstance(e, dict)]), is_sick_day=iso in sick_days, ) diff --git a/screen_locker/_weekly_check.py b/screen_locker/_weekly_check.py index c141ff7..af44d7a 100644 --- a/screen_locker/_weekly_check.py +++ b/screen_locker/_weekly_check.py @@ -11,9 +11,10 @@ current ISO week (Mon-Sun). from __future__ import annotations +from collections.abc import Iterable, Mapping from datetime import UTC, datetime, timedelta import logging -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from screen_locker._log_io import load_workout_log @@ -52,6 +53,21 @@ MANUAL_WORKOUT_TYPE: str = "manual_workout" # of truth instead of duplicating the type check. COUNTED_WORKOUT_TYPES: frozenset[str] = VERIFIED_WORKOUT_TYPES | {MANUAL_WORKOUT_TYPE} +# The two ingestion paths for ONE StrongLifts session. The phone sync writes +# ``phone_verified``; the PC session sync writes ``pc_workout_verified``. Both +# are built from the same WorkoutSession payload, so a session done once is +# recorded twice whenever both paths run -- and ``workout_id`` is +# ``":"``, so the two copies get different IDs and neither dedups +# the other away. They must therefore share ONE credit slot per day. +# +# Getting this wrong is not theoretical: on 2026-08-29 the double count pushed +# two consecutive ISO weeks to 5/5 when the real totals were 4 and 3, and +# ``has_weekly_minimum`` waived enforcement for both. Nothing reported a +# problem, because 5/5 is exactly what a compliant week looks like. +STRONGLIFTS_INGESTION_TYPES: frozenset[str] = frozenset( + {"phone_verified", PC_WORKOUT_TYPE}, +) + # Marks a relaxed day (Tue/Wed/Thu) as dismissed via "Skip — No Penalty" for # the rest of the calendar day. Deliberately NOT in COUNTED_WORKOUT_TYPES — # it's a UI-dismissal marker, not a workout, same treatment as heat_skip. @@ -71,19 +87,75 @@ def is_relaxed_day(*, today: datetime | None = None) -> bool: return dt.weekday() in _RELAXED_WEEKDAYS +def credit_key( + date_str: str, + entry_index: int, + entry: Mapping[str, Any], +) -> tuple[str, str] | None: + """Return the credit slot a log entry occupies, or None if it earns none. + + This is THE rule for "does this entry earn a workout credit, and is it the + same credit as another entry". Every counter in the package must go through + it rather than re-deriving the rule, because a second copy of the rule that + drifts from this one is precisely the bug this function was written to fix. + + Args: + date_str: The ``YYYY-MM-DD`` key the entry is filed under. + entry_index: Position of the entry within that date's list, used to + keep independently-earned credits distinct. + entry: The log entry. + + Returns: + A hashable key shared by entries that represent the same workout, or + ``None`` when the entry earns no credit at all (a ``relaxed_day_skip`` + marker, a heat skip, an unknown type). + """ + workout_data = entry.get("workout_data") + wtype = workout_data.get("type", "") if isinstance(workout_data, Mapping) else "" + if wtype not in COUNTED_WORKOUT_TYPES: + return None + if wtype in STRONGLIFTS_INGESTION_TYPES: + # One session, however many paths ingested it. + return ("stronglifts", date_str) + # Verified runs and manual workouts are genuinely separate sessions, so + # several on one day all count -- the manual-workout rate budget (see + # screen_locker._manual_workout) is the anti-gaming limiter, not a collapse. + return (wtype, f"{date_str}#{entry_index}") + + +def count_day_credits(date_str: str, entries: Iterable[Mapping[str, Any]]) -> int: + """Count the distinct workout credits earned on one day. + + Args: + date_str: The ``YYYY-MM-DD`` key the entries are filed under. + entries: That date's log entries. + + Returns: + The number of distinct credits, after collapsing the two StrongLifts + ingestion paths onto one slot. + """ + keys = { + key + for index, entry in enumerate(entries) + if (key := credit_key(date_str, index, entry)) is not None + } + return len(keys) + + def count_weekly_workouts( log_file: Path, *, today: datetime | None = None, ) -> int: - """Count counted workouts in the current ISO week. + """Count distinct workout credits in the current ISO week. - Every entry whose type is in :data:`COUNTED_WORKOUT_TYPES` — verified or - manual — counts individually, so multiple workouts on one day all count. - Manual workouts are rate-limited separately by the manual-workout budget - (:mod:`screen_locker._manual_workout`), not by a once-per-day collapse - here. Feeds both the weekly lock minimum (:func:`has_weekly_minimum`) and - the banked bonus. + Credits are decided by :func:`credit_key`: verified runs and manual + workouts count individually, so multiple on one day all count, while the + two StrongLifts ingestion paths share one slot per day so a single session + recorded twice is credited once. Manual workouts are rate-limited + separately by the manual-workout budget + (:mod:`screen_locker._manual_workout`), not by a collapse here. Feeds both + the weekly lock minimum (:func:`has_weekly_minimum`) and the banked bonus. Args: log_file: Path to ``log.json``. @@ -112,10 +184,7 @@ def count_weekly_workouts( continue if not (week_start <= entry_date <= today_date): continue - for entry in entries: - wtype = entry.get("workout_data", {}).get("type", "") - if wtype in COUNTED_WORKOUT_TYPES: - count += 1 # each counted workout — verified or manual — counts + count += count_day_credits(date_str, entries) return count diff --git a/screen_locker/tests/test_status_data_part2.py b/screen_locker/tests/test_status_data_part2.py index 3f8fa49..a6fd31b 100644 --- a/screen_locker/tests/test_status_data_part2.py +++ b/screen_locker/tests/test_status_data_part2.py @@ -189,8 +189,8 @@ class TestDayStatus: assert day.day_count == 0 assert day.source == "" - def test_multiple_entries_count_verified_individually_manual_once(self) -> None: - """Two verified + two manual on one day → day_count 3, all types listed.""" + def test_multiple_entries_each_count_individually(self) -> None: + """Two verified + two manual on one day → day_count 4, all types listed.""" day = _day_status( date(2024, 1, 5), [ @@ -208,6 +208,12 @@ class TestDayStatus: "manual_workout", ) assert day.counted is True - # 2 verified counted individually + all manual entries count once = 3. - assert day.day_count == 3 + # Every distinct session counts: 2 verified + 2 manual = 4. This used to + # assert 3, because _day_status re-derived the rule as + # "all of a day's manual entries count once" -- a rule + # count_weekly_workouts never had. The two disagreed on exactly this + # input, and _day_status feeds the status view's weekly_minimum_met + # explanation, so the window could explain a decision the locker did + # not make. Both now go through _weekly_check.count_day_credits. + assert day.day_count == 4 assert day.source == "run · gym · tt · squash" diff --git a/screen_locker/tests/test_status_data_part3.py b/screen_locker/tests/test_status_data_part3.py index e2a2a68..88784ac 100644 --- a/screen_locker/tests/test_status_data_part3.py +++ b/screen_locker/tests/test_status_data_part3.py @@ -104,9 +104,14 @@ class TestGatherStatusLockSuppression: "2024-01-01": {"workout_data": {"type": "phone_verified"}}, "2024-01-02": {"workout_data": {"type": "phone_verified"}}, "2024-01-03": {"workout_data": {"type": "phone_verified"}}, + # A gym session and a run on the same day: two genuinely + # separate workouts, so two credits. Deliberately NOT two + # phone_verified entries -- since the 2026-08-29 fix those + # share one StrongLifts credit slot per day, which is the + # whole point of the dedup. "2024-01-04": [ {"workout_data": {"type": "phone_verified"}}, - {"workout_data": {"type": "phone_verified"}}, + {"workout_data": {"type": "runnerup_verified"}}, ], } ) @@ -127,7 +132,9 @@ class TestGatherStatusLockSuppression: # Jan 1-4 one each, Jan 5 (today, Friday — the last day in-week without # going into the future) holds two, for 6 counted total: 1 above the # new minimum of 5. Also exercises multiple same-day entries each - # counting individually. + # counting individually — which is why the pair is a gym session plus a + # run rather than two phone_verified entries: those are the two + # ingestion paths for ONE StrongLifts session and share a credit slot. files["log_file"].write_text( json.dumps( { @@ -137,7 +144,7 @@ class TestGatherStatusLockSuppression: "2024-01-04": {"workout_data": {"type": "phone_verified"}}, "2024-01-05": [ {"workout_data": {"type": "phone_verified"}}, - {"workout_data": {"type": "phone_verified"}}, + {"workout_data": {"type": "runnerup_verified"}}, ], } ) diff --git a/screen_locker/tests/test_weekly_check_dedup.py b/screen_locker/tests/test_weekly_check_dedup.py new file mode 100644 index 0000000..5bbd254 --- /dev/null +++ b/screen_locker/tests/test_weekly_check_dedup.py @@ -0,0 +1,163 @@ +"""Tests for the StrongLifts credit-key dedup in _weekly_check. + +Regression cover for 2026-08-29: one StrongLifts session is ingested through +two independent paths -- the phone sync (``phone_verified``) and the PC session +sync (``pc_workout_verified``) -- and ``workout_id`` is ``":"``, so +the two copies of one workout got different IDs and both survived into +``log.json``. ``count_weekly_workouts`` then counted each entry individually, +the week reached 5/5 two workouts early, and ``has_weekly_minimum`` disabled +enforcement for two consecutive weeks without anything reporting a problem. + +The fixtures below are the real log entries from those days, not invented ones: +the point of the bug is that the two halves look like separate workouts unless +you notice the matching durations. +""" + +from __future__ import annotations + +from datetime import UTC, datetime +import json +from typing import TYPE_CHECKING, Any + +from screen_locker._weekly_check import ( + count_weekly_workouts, + has_weekly_minimum, +) + +if TYPE_CHECKING: + from pathlib import Path + +# Saturday of the ISO week beginning Monday 2026-08-24 -- the day the locker +# was expected to fire and did not. +_SATURDAY_2026_08_29 = datetime(2026, 8, 29, 21, 0, 0, tzinfo=UTC) + + +def _entry(wtype: str, *, timestamp: str, source: str = "") -> dict[str, Any]: + """Build one log entry of the given workout type. + + Args: + wtype: The ``workout_data.type`` value. + timestamp: ISO-8601 timestamp for the entry. + source: Human-readable provenance string, as the real log carries. + + Returns: + A log entry dict in the current on-disk shape. + """ + return { + "timestamp": timestamp, + "workout_data": {"type": wtype, "source": source}, + "workout_id": f"{wtype}:{timestamp[:10]}", + } + + +def _write_log(days: dict[str, list[dict[str, Any]]], log_file: Path) -> Path: + """Write a log.json in the current ``{date: [entry, ...]}`` shape. + + Args: + days: Mapping of ``YYYY-MM-DD`` to its list of entries. + log_file: Destination path. + + Returns: + ``log_file``, for convenience at call sites. + """ + log_file.write_text(json.dumps(days, indent=2), encoding="utf-8") + return log_file + + +def _real_week_2026_08_24() -> dict[str, list[dict[str, Any]]]: + """Return the real log contents for the ISO week of 2026-08-24. + + Three genuine workouts -- a StrongLifts session on Monday, padel on + Thursday, a StrongLifts session on Friday -- recorded as five entries, + because both StrongLifts sessions were ingested twice. + + Returns: + The date-keyed log fragment for that week. + """ + return { + "2026-08-24": [ + _entry( + "pc_workout_verified", + timestamp="2026-08-24T10:55:29.565673+00:00", + source="StrongLifts workout A (118 min, all succeeded)", + ), + _entry( + "phone_verified", + timestamp="2026-08-24T10:55:38.274973+00:00", + source="Workout verified! (118 min, all succeeded)", + ), + ], + "2026-08-27": [ + _entry( + "manual_workout", + timestamp="2026-08-27T18:45:32.328067+00:00", + source="padel at Padel PGE Narodowy", + ), + ], + "2026-08-28": [ + _entry( + "phone_verified", + timestamp="2026-08-28T09:24:00.798562+00:00", + source="Workout verified! (113 min, all succeeded)", + ), + _entry( + "pc_workout_verified", + timestamp="2026-08-28T09:30:07.715751+00:00", + source="StrongLifts workout B (113 min, all succeeded)", + ), + ], + } + + +class TestStrongliftsDoubleCountRegression: + """The 2026-08-29 outage: five entries, three workouts, no lock.""" + + def test_real_week_counts_three_not_five(self, tmp_path: Path) -> None: + """The week of 2026-08-24 holds three workouts, not five entries.""" + log = _write_log(_real_week_2026_08_24(), tmp_path / "log.json") + assert count_weekly_workouts(log, today=_SATURDAY_2026_08_29) == 3 + + def test_real_week_does_not_meet_the_minimum(self, tmp_path: Path) -> None: + """Three workouts is short of five, so enforcement must not be waived.""" + log = _write_log(_real_week_2026_08_24(), tmp_path / "log.json") + assert has_weekly_minimum(log, today=_SATURDAY_2026_08_29) is False + + def test_phone_and_pc_on_one_day_share_one_credit( + self, + tmp_path: Path, + ) -> None: + """Both ingestion paths for one session count once between them.""" + log = _write_log( + { + "2026-08-24": [ + _entry("phone_verified", timestamp="2026-08-24T10:55:38+00:00"), + _entry( + "pc_workout_verified", + timestamp="2026-08-24T10:55:29+00:00", + ), + ], + }, + tmp_path / "log.json", + ) + assert count_weekly_workouts(log, today=_SATURDAY_2026_08_29) == 1 + + def test_phone_and_pc_on_different_days_count_separately( + self, + tmp_path: Path, + ) -> None: + """The collapse is per-day: two days of StrongLifts are two workouts.""" + log = _write_log( + { + "2026-08-24": [ + _entry("phone_verified", timestamp="2026-08-24T10:00:00+00:00"), + ], + "2026-08-25": [ + _entry( + "pc_workout_verified", + timestamp="2026-08-25T10:00:00+00:00", + ), + ], + }, + tmp_path / "log.json", + ) + assert count_weekly_workouts(log, today=_SATURDAY_2026_08_29) == 2 diff --git a/screen_locker/tests/test_weekly_check_dedup_part2.py b/screen_locker/tests/test_weekly_check_dedup_part2.py new file mode 100644 index 0000000..edcd9bb --- /dev/null +++ b/screen_locker/tests/test_weekly_check_dedup_part2.py @@ -0,0 +1,115 @@ +"""Tests that the StrongLifts dedup does not collapse unrelated workouts. + +Split out of test_weekly_check_dedup.py for the 250-line cap; the shared +fixture helpers stay in that module and are imported here. + +These are the guard rail on the other side of the 2026-08-29 fix: collapsing +the two StrongLifts ingestion paths must not turn into "one workout per day", +which would quietly undercount a genuine double session and lock on a day that +was actually earned. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from screen_locker._weekly_check import count_weekly_workouts +from screen_locker.tests.test_weekly_check_dedup import ( + _SATURDAY_2026_08_29, + _entry, + _write_log, +) + +if TYPE_CHECKING: + from pathlib import Path + + +class TestDedupDoesNotOverCollapse: + """Only the two StrongLifts ingestion paths share a slot -- nothing else.""" + + def test_two_runs_on_one_day_both_count(self, tmp_path: Path) -> None: + """Verified runs are machine-checked, so multiple per day still count.""" + log = _write_log( + { + "2026-08-24": [ + _entry( + "runnerup_verified", + timestamp="2026-08-24T07:00:00+00:00", + ), + _entry( + "runnerup_verified", + timestamp="2026-08-24T19:00:00+00:00", + ), + ], + }, + tmp_path / "log.json", + ) + assert count_weekly_workouts(log, today=_SATURDAY_2026_08_29) == 2 + + def test_manual_and_phone_on_one_day_both_count(self, tmp_path: Path) -> None: + """A manual workout is a different session from a StrongLifts one.""" + log = _write_log( + { + "2026-08-24": [ + _entry("manual_workout", timestamp="2026-08-24T08:00:00+00:00"), + _entry("phone_verified", timestamp="2026-08-24T18:00:00+00:00"), + ], + }, + tmp_path / "log.json", + ) + assert count_weekly_workouts(log, today=_SATURDAY_2026_08_29) == 2 + + def test_two_manual_workouts_on_one_day_both_count( + self, + tmp_path: Path, + ) -> None: + """The manual-workout budget is the limiter, not a per-day collapse.""" + log = _write_log( + { + "2026-08-24": [ + _entry("manual_workout", timestamp="2026-08-24T08:00:00+00:00"), + _entry("manual_workout", timestamp="2026-08-24T18:00:00+00:00"), + ], + }, + tmp_path / "log.json", + ) + assert count_weekly_workouts(log, today=_SATURDAY_2026_08_29) == 2 + + def test_lone_phone_verified_still_counts(self, tmp_path: Path) -> None: + """A phone session with no PC twin is one workout, not zero.""" + log = _write_log( + { + "2026-08-24": [ + _entry("phone_verified", timestamp="2026-08-24T10:00:00+00:00"), + ], + }, + tmp_path / "log.json", + ) + assert count_weekly_workouts(log, today=_SATURDAY_2026_08_29) == 1 + + def test_lone_pc_workout_still_counts(self, tmp_path: Path) -> None: + """A PC session with no phone twin is one workout, not zero.""" + log = _write_log( + { + "2026-08-24": [ + _entry( + "pc_workout_verified", + timestamp="2026-08-24T10:00:00+00:00", + ), + ], + }, + tmp_path / "log.json", + ) + assert count_weekly_workouts(log, today=_SATURDAY_2026_08_29) == 1 + + def test_relaxed_day_skip_never_counts(self, tmp_path: Path) -> None: + """A skip marker is a UI dismissal, not a workout, twin or not.""" + log = _write_log( + { + "2026-08-25": [ + _entry("relaxed_day_skip", timestamp="2026-08-25T18:00:00+00:00"), + ], + }, + tmp_path / "log.json", + ) + assert count_weekly_workouts(log, today=_SATURDAY_2026_08_29) == 0