Multiple workouts per day, the sync that never ran, and a ban on silence

feat/check-phone-runnerup-and-tray · 3 commits · 62 files · +2719 / −777 · 2026-07-17

1. Background — what this thing is

screen-locker is a discipline tool. A systemd user service runs python -m screen_locker.screen_lock --production at login and again from a timer at 08:30/09:05. If you haven't earned your workout, it throws a fullscreen Tk window over everything until you do. If you have, it calls sys.exit(0) and you never see it. That's why "the locker isn't running" is usually the healthy state, not a fault.

It decides using screen_locker/log.json — a gitignored, HMAC-signed file. Each entry is signed with a key shared across screen-locker, diet_guard and wake_alarm, so the log can't be quietly hand-edited to fake a workout. Two things read it constantly: has_weekly_minimum() (fire the lock or not?) and the reward path (how late may the PC stay on tonight?).

A workout gets in three ways. phone_verified — a StrongLifts session pulled off the phone. runnerup_verified — a real run, parsed from the actual TCX file that RunnerUp writes to /sdcard/Documents/RunnerUp/ and pulled over ADB. And manual_workout — a self-reported evidence form (squash, table tennis), rate-limited by a budget because nothing verifies it. That distinction — machine-checked versus self-reported — is the hinge the whole change turns on.

There's also a companion Flutter app on the phone, and a shared GitHub repo (kuhyx/syncs) holding a small CRDT log per device, so the two machines can agree on what happened.

The shape that caused everything

log.json was keyed by date, one entry per day. The writer said so in as many words, and did exactly that:

# _log_mixin.py — before
"""...The log is day-keyed: one entry per date."""
logs[date] = entry          # a second workout that day OVERWRITES the first

So the file physically could not hold two workouts on one day. Everything downstream inherited that: count_weekly_workouts iterated the date keys, so "4 workouts a week" and "4 days a week" were the same sentence. Nobody had to choose between them, because the data model had already chosen.

2. Intuition — one slot per day

The trigger was mundane. On Monday 13 July, kuhy played squash and went for a 9.8 km run. He logged the squash by hand. The run — a real, verified, TCX-backed run — had nowhere to go. The day already had its slot.

The trap: a list alone is a cheat code

Letting a day hold many workouts sounds harmless until you notice what the lock is for. If every entry counted, you could type four manual entries on a Monday morning, satisfy "4 workouts this week", and never be locked again until Sunday. The whole tool would be an honour system with extra steps.

So the rule isn't "count workouts". It's count what can't be faked:

Play with it. Add workouts and watch the count and the lock:

The second trap: the same workout, twice

Once a day can hold many workouts, "is this one already here?" stops being free. Every entry now carries a workout_id — deliberately the same string the sync layer uses as its record id, so a workout logged on the PC and the same workout arriving from the phone collapse to one row instead of two.

The sync had never worked. Not once.

While fixing the above, a second thing surfaced. The PC was supposed to publish its workouts so the phone could see them. It never had — screen-locker-sync/ in the repo contained a single .gitkeep. Two independent reasons:

1. No token. push_pc_manuals() opened with if token is None: return — a bare return, no log line. The PAT had been saved to the wrong app's config directory. The code knew and said nothing.
2. A side-store. It pushed from manual_sync_log.json, written only at log-time. So anything recorded before that feature existed — like the squash — could never publish, no matter how many times it ran.

The fix for (2) also fixes a category error: derive the push from log.json itself. The log is the truth; the sync is a projection of it. Then history publishes automatically, forever, with no second place to fall out of date.

Why silence was the real bug

Notice the shape of the failure. Not a crash — a return []. During the debugging session, pull_all_manual_records() returned 0 and it was impossible to tell whether the repo was empty or the call had failed, because the token guard was silent and real errors logged at INFO. The answer only came from bypassing the function and hitting the API by hand.

Worth knowing why a custom AST rule was needed at all: ruff already runs with select = ["ALL"], so S110 (except: pass) was already enforced — and the codebase had none. The lethal pattern wasn't pass. It was except: log.info(...); return [], which looks handled and no ruff rule catches.

3. Code — the change, by concern

3.1 One reader, two shapes

A new dependency-free module is the single place that reads the file. Old and new shapes both load, so a half-migrated file — or a rolled-back writer — is never a crash. Crucially, wrapping a legacy dict in a list leaves the signed bytes untouched, so every existing HMAC still verifies with zero re-signing.

+ # screen_locker/_log_io.py  (new)
+ def load_workout_log(log_file) -> dict[str, list[dict]]:
+     normalized = {}
+     for date, value in read_raw_log(log_file).items():
+         if isinstance(value, list):
+             normalized[date] = [e for e in value if isinstance(e, dict)]
+         elif isinstance(value, dict):
+             normalized[date] = [value]      # legacy: wrap, don't rewrite
+     return normalized

3.2 One writer, which appends and dedups

The overwrite becomes an append behind a single chokepoint that returns a result instead of None — so the caller can tell a genuinely new workout from a duplicate, and scale the reward accordingly.

- logs[date] = entry                       # overwrite
+ workout_id = _derive_workout_id(date, workout_data)
+ if workout_id is not None and any(
+     _entry_workout_id(date, e) == workout_id for e in entries
+ ):
+     return RecordResult(appended=False, prior_entries=prior)
+ entries.append(entry)
+ return RecordResult(appended=True, prior_entries=prior)

3.3 The counting rule

The anti-gaming asymmetry from section 2, in the one function that both the lock and the banked bonus consult:

for date_str, entries in load_workout_log(log_file).items():
    ...
    has_manual = False
    for entry in entries:
        wtype = entry.get("workout_data", {}).get("type", "")
        if wtype in VERIFIED_WORKOUT_TYPES:
            count += 1              # each verified workout counts
        elif wtype == MANUAL_WORKOUT_TYPE:
            has_manual = True
    if has_manual:
        count += 1                  # all of a day's manuals count as one

3.4 The reward, with two guards

_apply_workout_credit now scales to the day: the first counted workout earns the base +2h; an additional same-day verified one earns +1h; an additional manual earns nothing. The old "+1h when weekly_count > 4" gate was removed — weekly volume is the banked bonus's job, and paying for it twice was the bug hiding in plain sight. The credit fires only if the write actually appended (id guard) and the entry is dated today (date guard) — so a back-dated backfill raises the count but never pushes tonight's shutdown.

The tests caught a real defect here: the unlock screen still labelled the +1h #{weekly_count - 4}, which under the new rule can render negative.

3.5 The sync, derived from the log

Record id = the workout's own workout_id. HLC = derived from the entry's own timestamp — which makes a re-push byte-identical, so the repo never churns. That determinism is what lets the push run on every startup without turning the sync repo into a commit firehose.

+ hlc = Hlc.new_tick(_PC_DEVICE_ID, wall_time_ms=_entry_wall_ms(entry, date))
+ log[record_id] = Record(id=record_id, fields={_PAYLOAD_FIELD: (payload, hlc)})

And nothing about it is silent any more:

+ @dataclass(frozen=True)
+ class PushResult:
+     pushed: bool
+     record_count: int
+     reason: str        # always a sentence a human can act on

3.6 Two recoveries, one pattern

The phone kept a Keystore token and mirrored it to /sdcard — but load() only consulted the mirror when the Keystore was empty, never when its token was present-but-rejected. A stale entry could shadow a good backup forever while the UI insisted all was well. Now a 401 triggers one retry from the mirror before anyone is asked to re-authorize.

The same pattern then closed a sharper hole. active_session — which set you're on, and the reps done — lived only in app-private SQLite. A pm clear wiped it mid-workout, and backup.json didn't help because it mirrors only completed history. The state had to be re-entered by hand. The persistence granularity was already right (it saves on every set tap); only the location was wrong.

3.7 The rule that outlives the fix

+ # scripts/check_silent_failures.py — pre-commit hook `no-silent-failures`
+ def _handler_is_loud(handler) -> bool:
+     """True if the handler re-raises or logs loudly about the failure."""
+     return any(
+         isinstance(node, ast.Raise) or _is_loud_log_call(node)
+         for node in ast.walk(handler)
+     )

There's deliberately no escape hatch, because noqa is banned repo-wide. Two ironies for the record: the checker's own docstring contained the literal string # noqa and tripped the noqa-blocker, and _log_io.py — written for this very change — violated the new rule. Both were invisible while untracked, since git ls-files hides untracked files from the hooks, and only surfaced once committed.

4. Quiz

Five questions on the reasoning. Score: 0 / 0