From 9131c8086e7670ae02202fd54de12fe4cabe6d67 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Thu, 27 Aug 2026 20:50:52 +0200 Subject: [PATCH] Log manual workouts headlessly, with the form's rules intact The phone form was the only way to log a manual workout, so logging one depended on the phone's foreground. Another app taking focus mid-entry could interleave a field (seen: "Played 6 games 1 did nPlayed 6 games...") or drop one, which is a bad property for a record that spends budget. `--log-manual-workout` takes every field as a flag and needs no UI, no phone and no app. It is emphatically NOT a way to log a workout without evidence: the draft goes through the same `validate_manual_workout`, so a missing or too-short field refuses the whole run, writes nothing and exits non-zero with the concrete reason. Sport-specific rules survive too -- table tennis still demands its match counts, racket and balls. Routed through `ingest_manual_records` rather than writing `log.json` directly, so validation, the rolling 2/week budget, HMAC signing and workout_id dedup are the shared implementation with the sync path instead of a second copy that can drift. That is the difference from the deleted `log_manual_workout.py`, which bypassed the checks: here the budget refuses a third entry in a week exactly as the phone does, and re-running an identical command is a no-op rather than a duplicate. "Today" is UTC, matching how the budget windows compute it -- a local date near midnight would file a workout in a different window than it is counted in. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AWq9MRemX9ZWvNmnhNWUP2 --- screen_locker/_cli.py | 9 ++ screen_locker/_manual_cli.py | 162 +++++++++++++++++++++++ screen_locker/tests/test_manual_cli.py | 176 +++++++++++++++++++++++++ 3 files changed, 347 insertions(+) create mode 100644 screen_locker/_manual_cli.py create mode 100644 screen_locker/tests/test_manual_cli.py diff --git a/screen_locker/_cli.py b/screen_locker/_cli.py index f77746a..771c397 100644 --- a/screen_locker/_cli.py +++ b/screen_locker/_cli.py @@ -14,6 +14,7 @@ import sys from typing import TYPE_CHECKING from screen_locker._decision_log import record_no_decision +from screen_locker._manual_cli import run_manual_log from screen_locker._status import run_status if TYPE_CHECKING: @@ -22,6 +23,7 @@ if TYPE_CHECKING: __all__ = ["main"] _LOG_FILE_NAME = "log.json" +_LOG_MANUAL_FLAG = "--log-manual-workout" def _headless_locker(locker_cls: type[ScreenLocker]) -> ScreenLocker: @@ -52,6 +54,13 @@ def main(locker_cls: type[ScreenLocker], argv: list[str]) -> None: if "--status" in argv: run_status(_headless_locker(locker_cls)) + if _LOG_MANUAL_FLAG in argv: + # Headless manual logging: no Tk, no phone, no app in the foreground. + # Everything after the flag is the workout's evidence, validated by + # the same rules the phone form applies. + rest = argv[argv.index(_LOG_MANUAL_FLAG) + 1 :] + sys.exit(run_manual_log(_headless_locker(locker_cls).log_file, rest)) + if "--sync-only" in argv: # Headless sync for the timer unit: pull other devices' workouts and # apply their credit, with no Tk and no lock screen. Sync used to run diff --git a/screen_locker/_manual_cli.py b/screen_locker/_manual_cli.py new file mode 100644 index 0000000..479d15e --- /dev/null +++ b/screen_locker/_manual_cli.py @@ -0,0 +1,162 @@ +"""Headless manual-workout logging: same evidence rules, no UI. + +The phone form was the only way to log a manual workout, which made logging +one depend on the phone's foreground: another app stealing focus mid-entry +could interleave or drop a field. That is a bad property for a record that +consumes budget, so this offers the same thing as a command. + +What it deliberately is NOT: a way to log a workout without evidence. Every +field the form demands is demanded here, checked by the SAME +``validate_manual_workout``, and a missing or too-short field refuses the +whole run and writes nothing. ``log_manual_workout.py`` was deleted because +it bypassed those checks; this routes through the ordinary sync-ingest path +(``ingest_manual_records``), so validation, the rolling budget, HMAC signing +and workout_id dedup are the shared implementation rather than a second copy +that can drift. +""" + +from __future__ import annotations + +import argparse +from datetime import datetime, timezone +import logging +from typing import TYPE_CHECKING + +from screen_locker._manual_sync import ingest_manual_records +from screen_locker._manual_workout import ( + SPORT_CHOICES, + SPORT_OTHER, + ManualWorkoutDraft, + build_sync_payload, + manual_sync_record_id, + validate_manual_workout, +) + +if TYPE_CHECKING: + from collections.abc import Sequence + from pathlib import Path + +_logger = logging.getLogger(__name__) + +__all__ = ["add_manual_arguments", "run_manual_log"] + +# (flag, payload key, default). Defaults mirror ManualWorkoutDraft's own, so +# "not supplied" means the same thing headlessly as it does on the form -- +# pain_or_injury is "none" (an answer) rather than "" (never asked). +_OPTIONAL_TEXT = ( + ("reservation_phone", "reservation_phone", ""), + ("techniques", "techniques_practiced", ""), + ("warm_up", "warm_up_minutes", ""), + ("pain_or_injury", "pain_or_injury", "none"), + ("racket", "racket", ""), + ("balls", "balls", ""), + ("equipment", "equipment", ""), +) +_OPTIONAL_INT = ( + ("matches_won", "matches_won"), + ("matches_lost", "matches_lost"), + ("sets_won", "sets_won"), + ("sets_lost", "sets_lost"), +) + + +def add_manual_arguments(parser: argparse.ArgumentParser) -> None: + """Register every manual-workout field as a flag on ``parser``. + + No flag carries a substantive default: the point is that the caller + supplies the evidence, so an omitted field must fail validation rather + than be quietly filled in with something plausible. + """ + parser.add_argument("--sport", choices=sorted(SPORT_CHOICES), required=True) + parser.add_argument( + "--activity", + default="", + help=f"what the activity was; required when --sport {SPORT_OTHER}", + ) + parser.add_argument("--date", default="", help="ISO date; defaults to today") + parser.add_argument("--start", required=True, help="HH:MM") + parser.add_argument("--end", required=True, help="HH:MM") + parser.add_argument("--location", required=True) + parser.add_argument("--transport", required=True, help="how you got there") + parser.add_argument("--cost", required=True, help='e.g. "60 PLN" or "none"') + parser.add_argument("--rpe", required=True, type=int, help="1-10") + parser.add_argument("--details", required=True, help="what was done") + parser.add_argument("--went-well", required=True) + parser.add_argument("--to-improve", required=True) + parser.add_argument("--feeling", required=True, help="overall feeling") + for flag, _key, default in _OPTIONAL_TEXT: + parser.add_argument(f"--{flag.replace('_', '-')}", default=default) + for flag, _key in _OPTIONAL_INT: + parser.add_argument(f"--{flag.replace('_', '-')}", default=0, type=int) + + +def _draft(args: argparse.Namespace) -> ManualWorkoutDraft: + """Build the draft from parsed flags, with no substantive defaults. + + Optional fields fall back to the dataclass's own defaults so the headless + path and the form agree on what "not supplied" means. + """ + return ManualWorkoutDraft( + sport=args.sport, + start_time=args.start, + end_time=args.end, + location_name=args.location, + transport_method=args.transport, + cost=args.cost, + rpe=args.rpe, + went_well=args.went_well, + to_improve=args.to_improve, + overall_feeling=args.feeling, + activity_type_other=args.activity if args.sport == SPORT_OTHER else "", + activity_details=args.details, + **{key: getattr(args, flag) for flag, key, _default in _OPTIONAL_TEXT}, + **{key: getattr(args, flag) for flag, key in _OPTIONAL_INT}, + ) + + +def run_manual_log(log_file: Path, argv: Sequence[str]) -> int: + """Log one manual workout headlessly. Returns a process exit code. + + Non-zero means nothing was written: ``ingest_manual_records`` refuses a + draft that fails validation or that the rolling budget has no room for, + and it already logs the concrete reason, so a caller never has to guess + between "rejected" and "already logged". + """ + parser = argparse.ArgumentParser( + prog="screen-locker --log-manual-workout", + description="Log a manual workout without opening any app.", + ) + add_manual_arguments(parser) + args = parser.parse_args(list(argv)) + + # UTC, matching how the budget windows compute "today" -- a local date + # near midnight would file the workout in a different window than the one + # it is then counted in. + date = args.date or datetime.now(tz=timezone.utc).strftime("%Y-%m-%d") + draft = _draft(args) + # Validated here as well as inside the ingest, purely so a person running + # this by hand gets the concrete field error on stderr and a non-zero exit + # instead of only a log line. The ingest re-checks it regardless. + error = validate_manual_workout(draft) + if error is not None: + _logger.error("Manual workout REFUSED — %s. Nothing was written.", error) + return 1 + + record_id = manual_sync_record_id(date, args.start) + payload = build_sync_payload(draft, date) + + ingested = ingest_manual_records(log_file, [(record_id, payload)], today=date) + if not ingested: + _logger.error( + "Manual workout %s was NOT logged — see the reason logged above " + "(invalid field, exhausted budget, or already logged)", + record_id, + ) + return 1 + _logger.info( + "Logged manual workout %s. It publishes to the other devices on the " + "next sync pass (workout-sync.timer, every 15 minutes); run " + "--sync-only to push it now.", + record_id, + ) + return 0 diff --git a/screen_locker/tests/test_manual_cli.py b/screen_locker/tests/test_manual_cli.py new file mode 100644 index 0000000..5487f7e --- /dev/null +++ b/screen_locker/tests/test_manual_cli.py @@ -0,0 +1,176 @@ +"""Tests for headless manual-workout logging (``--log-manual-workout``). + +The point of these is the REFUSALS. A headless entry point is only safe if it +cannot log a workout the phone form would have rejected, so each way of being +invalid is asserted to write nothing and exit non-zero. +""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING + +import pytest + +from screen_locker._manual_cli import run_manual_log + +if TYPE_CHECKING: + from pathlib import Path + +_VALID = ( + "--sport", + "other", + "--activity", + "padel", + "--date", + "2026-08-27", + "--start", + "17:30", + "--end", + "19:00", + "--location", + "Padel PGE Narodowy", + "--transport", + "By bus, 2 przesiadki", + "--cost", + "60 PLN", + "--rpe", + "4", + "--details", + "Played 6 games 1 did not finish won 3 lost 2", + "--went-well", + "my serves got much better, with good speed and accuracy", + "--to-improve", + "first game was sluggish, I was tired from work", + "--feeling", + "after the tiredness went away the game went very well indeed", +) + + +def _args(**overrides: str) -> list[str]: + """Return the valid arg list with ``--flag`` values replaced.""" + args = list(_VALID) + for flag, value in overrides.items(): + idx = args.index(f"--{flag.replace('_', '-')}") + args[idx + 1] = value + return args + + +def _entries(log_file: Path) -> list[dict]: + """Return every logged entry, or [] when nothing was written.""" + if not log_file.exists(): + return [] + return [e for day in json.loads(log_file.read_text()).values() for e in day] + + +def test_logs_a_valid_workout(tmp_path: Path) -> None: + """A complete draft is signed, filed under its date and reported.""" + log_file = tmp_path / "log.json" + + assert run_manual_log(log_file, _args()) == 0 + + (entry,) = _entries(log_file) + data = entry["workout_data"] + assert data["type"] == "manual_workout" + assert data["activity_type"] == "padel" + assert data["duration_minutes"] == "90.0" + assert data["rpe"] == 4 + assert entry["workout_id"] == "manual:2026-08-27T17:30" + assert entry["hmac"] + + +def test_unsupplied_optional_matches_the_form_default(tmp_path: Path) -> None: + """``pain_or_injury`` defaults to "none" -- an answer, not a blank. + + An empty string would read as "never asked", which is a different claim + from the one the form records. + """ + log_file = tmp_path / "log.json" + run_manual_log(log_file, _args()) + + (entry,) = _entries(log_file) + assert entry["workout_data"]["pain_or_injury"] == "none" + + +@pytest.mark.parametrize( + ("field", "value", "why"), + [ + ("details", "too short", "under the 40-char activity-details bar"), + ("went_well", "brief", "under the 20-char reflection bar"), + ("to_improve", "brief", "under the 20-char reflection bar"), + ("feeling", "brief", "under the 20-char reflection bar"), + ("location", "", "required field left blank"), + ("transport", "", "required field left blank"), + ("cost", "", "required field left blank"), + ("rpe", "11", "outside the 1-10 RPE range"), + ("end", "17:35", "under the minimum workout duration"), + ], +) +def test_refuses_and_writes_nothing( + tmp_path: Path, field: str, value: str, why: str +) -> None: + """Every way of being invalid refuses the whole run. + + Asserted per-field rather than once, because a headless path that logged + even one of these would be the deleted ``log_manual_workout.py`` again. + """ + log_file = tmp_path / "log.json" + + assert run_manual_log(log_file, _args(**{field: value})) == 1, why + assert _entries(log_file) == [] + + +def test_rerunning_the_same_workout_logs_it_once(tmp_path: Path) -> None: + """The record id is derived from (date, start), so a repeat is a no-op.""" + log_file = tmp_path / "log.json" + + assert run_manual_log(log_file, _args()) == 0 + assert run_manual_log(log_file, _args()) == 1 + assert len(_entries(log_file)) == 1 + + +def test_the_rolling_budget_still_caps_headless_entries(tmp_path: Path) -> None: + """The third workout in a week is refused, exactly as on the phone.""" + log_file = tmp_path / "log.json" + + assert run_manual_log(log_file, _args(start="07:00", end="08:30")) == 0 + assert run_manual_log(log_file, _args(start="10:00", end="11:30")) == 0 + assert run_manual_log(log_file, _args(start="17:30", end="19:00")) == 1 + assert len(_entries(log_file)) == 2 + + +def test_date_defaults_to_today(tmp_path: Path) -> None: + """Omitting --date files the workout under today rather than failing.""" + log_file = tmp_path / "log.json" + args = [a for a in _args() if a not in {"--date", "2026-08-27"}] + + assert run_manual_log(log_file, args) == 0 + assert _entries(log_file) + + +def test_table_tennis_needs_no_activity_name(tmp_path: Path) -> None: + """``--activity`` is only meaningful for the open-ended "other" sport. + + Table tennis names itself, but it does demand its own evidence (a match + count), so the sport-specific rules survive the headless path too. + """ + log_file = tmp_path / "log.json" + args = _args(sport="table_tennis") + args[args.index("--activity") + 1] = "" + args += [ + "--matches-won", + "3", + "--matches-lost", + "1", + "--sets-won", + "7", + "--sets-lost", + "4", + "--racket", + "Butterfly", + "--balls", + "Nittaku", + ] + + assert run_manual_log(log_file, args) == 0 + assert _entries(log_file)[0]["workout_data"]["activity_type"] == "table tennis"