From 1e201acbdcecaf1f836d865c2958dcd9c50c958d Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sat, 15 Aug 2026 18:51:18 +0200 Subject: [PATCH] Split _sync.py and its tests; extract the gate fixtures from conftest _sync.py 498 -> 165, into siblings that each own one concern: _sync_client.py picking a backend and building the remote store _sync_banks.py pull/merge/push for the food bank and the budget _sync_paths.py the kuhyx/syncs wire layout, shared by both _sync_errors.py SyncError, so _sync and _sync_client need not import each other The patch targets moved with the code they guard, which is the whole risk of this commit. unittest.mock.patch and monkeypatch.setattr both bind to a *module object*, so a patch left pointing at _sync after CONFIG_FILE and SYNC_TOKEN_FILE moved to _sync_client redirects nothing. That is not hypothetical here: the first run of the suite took 9m18s instead of 7s and raised 401s, because tests were reaching the real GitHub API with a fake token. Repointed: conftest SYNC_TOKEN_FILE, CONFIG_FILE -> _sync_client test_sync* GitHubSyncClient (30 patch sites) -> _sync_client test_backend mirror/firebase_client_for -> _sync_client The repo's own _test_guard caught the same leak independently, which is the behaviour worth keeping: two guards, both fail-closed. test_sync.py 482 -> 205, splitting the budget and pull/revision classes into siblings and the shared fakes into _sync_fakes.py. conftest.py 284 -> 155, moving the Tk/gatelock fixtures to _gate_fixtures.py. Importing them back is what registers them (pytest_plugins is an error in a non-root conftest), and they are named in __all__ because ruff's autofix otherwise deletes the "unused" import and silently disables Tk blocking -- it did exactly that once here. Verified afterwards with a canary asserting _gatelock.tk is still a MagicMock, not merely that the suite passes. Verified: 692 tests / 100% coverage unchanged; ~/.local/share/diet_guard byte-identical before and after the run; cd /tmp && python3 -m diet_guard sync -> 402 entries, exit 0. Gate 47 -> 44. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MK3HCZrM75sRHL1gL8usve --- .pre-commit-config.yaml | 6 +- app/pubspec.yaml | 2 +- diet_guard/_budget_history.py | 2 +- diet_guard/_device.py | 2 +- diet_guard/_sync.py | 298 +------------ diet_guard/_sync_banks.py | 199 +++++++++ diet_guard/_sync_client.py | 131 ++++++ diet_guard/_sync_errors.py | 17 + diet_guard/_sync_merge.py | 470 --------------------- diet_guard/_sync_paths.py | 39 ++ diet_guard/sync_merge/__init__.py | 71 ++++ diet_guard/sync_merge/_banks.py | 137 ++++++ diet_guard/sync_merge/_budget.py | 186 ++++++++ diet_guard/sync_merge/_clock.py | 30 ++ diet_guard/sync_merge/_daylog.py | 170 ++++++++ diet_guard/tests/_gate_fixtures.py | 182 ++++++++ diet_guard/tests/_sync_fakes.py | 59 +++ diet_guard/tests/conftest.py | 187 ++------ diet_guard/tests/test_sync.py | 321 +------------- diet_guard/tests/test_sync_backend.py | 50 ++- diet_guard/tests/test_sync_banks.py | 30 +- diet_guard/tests/test_sync_budget.py | 125 ++++++ diet_guard/tests/test_sync_merge.py | 216 +--------- diet_guard/tests/test_sync_merge_banks.py | 2 +- diet_guard/tests/test_sync_merge_budget.py | 216 ++++++++++ diet_guard/tests/test_sync_pull.py | 153 +++++++ 26 files changed, 1824 insertions(+), 1477 deletions(-) create mode 100644 diet_guard/_sync_banks.py create mode 100644 diet_guard/_sync_client.py create mode 100644 diet_guard/_sync_errors.py delete mode 100644 diet_guard/_sync_merge.py create mode 100644 diet_guard/_sync_paths.py create mode 100644 diet_guard/sync_merge/__init__.py create mode 100644 diet_guard/sync_merge/_banks.py create mode 100644 diet_guard/sync_merge/_budget.py create mode 100644 diet_guard/sync_merge/_clock.py create mode 100644 diet_guard/sync_merge/_daylog.py create mode 100644 diet_guard/tests/_gate_fixtures.py create mode 100644 diet_guard/tests/_sync_fakes.py create mode 100644 diet_guard/tests/test_sync_budget.py create mode 100644 diet_guard/tests/test_sync_merge_budget.py create mode 100644 diet_guard/tests/test_sync_pull.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 892b7c8..265a493 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -26,9 +26,9 @@ repos: - id: debug-statements - id: name-tests-test args: [--pytest-test-first] - # Shared fake widgets, not a test module. Split out of conftest.py only - # because that file passed the 250-line cap. - exclude: ^diet_guard/tests/_(tk_fakes|gate_layout_probe|gate_keyboard_probe)\.py$ + # Shared fakes and fixtures, not test modules. Split out of conftest.py + # and test_sync.py only because those files passed the 250-line cap. + exclude: ^diet_guard/tests/_(tk_fakes|gate_layout_probe|gate_keyboard_probe|gate_fixtures|sync_fakes)\.py$ - id: check-ast - id: mixed-line-ending args: [--fix=lf] diff --git a/app/pubspec.yaml b/app/pubspec.yaml index 58a51da..559fbf6 100644 --- a/app/pubspec.yaml +++ b/app/pubspec.yaml @@ -2,7 +2,7 @@ name: diet_guard_app description: "Companion phone app for diet_guard: log meals on the go." publish_to: 'none' -version: 1.0.14+1 +version: 1.0.15+1 environment: sdk: ^3.12.2 diff --git a/diet_guard/_budget_history.py b/diet_guard/_budget_history.py index d7a54e0..feb2a22 100644 --- a/diet_guard/_budget_history.py +++ b/diet_guard/_budget_history.py @@ -16,7 +16,7 @@ Storage and sync are deliberately split: * the entries live in their own file (``.budget_history``), leaving :mod:`diet_guard._budget`'s ``.budget`` schema untouched at v2; * they travel as extra ``hist:`` *fields* on the existing - ``budget`` CRDT record (see :mod:`diet_guard._sync_merge`). + ``budget`` CRDT record (see :mod:`diet_guard.sync_merge`). That second point is the load-bearing one. ``crdt_sync``'s ``merge_record`` does per-field last-writer-wins over the *union* of field names, and both diff --git a/diet_guard/_device.py b/diet_guard/_device.py index 17286c8..6483e46 100644 --- a/diet_guard/_device.py +++ b/diet_guard/_device.py @@ -1,7 +1,7 @@ """This machine's persisted sync identity. Its own module rather than living in :mod:`diet_guard._sync`, because -:mod:`diet_guard._sync_merge` needs it too for HLC stamping and importing +:mod:`diet_guard.sync_merge` needs it too for HLC stamping and importing ``_sync`` from there would be a cycle. """ diff --git a/diet_guard/_sync.py b/diet_guard/_sync.py index 6813310..4b6231c 100644 --- a/diet_guard/_sync.py +++ b/diet_guard/_sync.py @@ -2,7 +2,7 @@ Pulls every other device's pushed log from GitHub-backed dumb storage (``crdt_sync.GitHubSyncClient``), merges with the local log via -``crdt_sync``'s shared CRDT scheme (:mod:`diet_guard._sync_merge` adapts +``crdt_sync``'s shared CRDT scheme (:mod:`diet_guard.sync_merge` adapts diet_guard's entries to/from ``crdt_sync.Record``), re-signs every persisted entry, rebuilds the food bank, and pushes this device's own merged log back up in the new Record-based wire format. @@ -20,176 +20,38 @@ import json import logging from crdt_sync import ( - CONFIG_FILE, - ConfigError, DeviceIdentity, FileSyncStateStore, - FirebaseAuthError, - GitHubSyncClient, GitHubSyncError, Log, RemoteStore, RemoteSyncError, SyncState, - firebase_client_for, merge_logs, - mirror_client_for, revision_of, ) -from diet_guard._budget import read_raw_record, write_raw_record -from diet_guard._budget_history import ( - history_to_json, - load_entries, - write_raw_history, -) from diet_guard._constants import ( - SYNC_REPO_NAME, - SYNC_REPO_OWNER, SYNC_STATE_FILE, - SYNC_TIMEOUT_SECONDS, - SYNC_TOKEN_FILE, ) from diet_guard._device import device_identity -from diet_guard._foodbank import read_food_bank, rebuild_food_bank, write_food_bank -from diet_guard._foodbank_manual import read_manual_bank, write_manual_bank +from diet_guard._foodbank import rebuild_food_bank from diet_guard._state import DayLog, read_raw_log, resign_entry, write_raw_log -from diet_guard._sync_merge import ( - budget_to_log, +from diet_guard._sync_banks import _sync_budget, _sync_food_bank, _sync_manual_bank +from diet_guard._sync_client import _client_for_run +from diet_guard._sync_errors import SyncError +from diet_guard._sync_paths import _DEVICES_DIR, _REVS_DIR, _device_log_path +from diet_guard.sync_merge import ( daylog_to_log, - food_bank_to_log, - log_to_budget, log_to_daylog, - log_to_food_bank, - log_to_history, - log_to_manual_bank, - manual_bank_to_log, - parse_remote_budget, - parse_remote_food_bank, parse_remote_log, - parse_remote_manual_bank, ) _logger = logging.getLogger(__name__) -_DEVICES_DIR = "diet-guard-sync/devices" # One small text node per device, each written only by its owner. Deliberately # not one shared map: a whole-map write would erase every other device's entry, # after which those peers would look permanently unchanged and never be -# fetched again. -_REVS_DIR = "diet-guard-sync/revs" - - -class SyncError(Exception): - """Raised when a sync run cannot even start (no usable PAT).""" - - -def _remote_client(github: GitHubSyncClient) -> RemoteStore: - """Return the backend to sync against. - - Firebase when ``~/.config/crdt-sync/`` is set up, with GitHub kept as a - mirror so a device that has not moved yet still converges; GitHub alone - otherwise. An unconfigured machine keeps syncing exactly as before. - - Every sub-sync here (log, budget, food bank, manual bank) takes the client - as a parameter, so this one swap covers all of them. - - The config file is checked before constructing anything, so an - unconfigured machine never reaches the network -- otherwise a suite that - blocks real sockets fails here rather than in the code under test. - - Rolling back is deleting this function and passing ``github`` straight - through: no data moves either way. - """ - if not CONFIG_FILE.is_file(): - return github - try: - return mirror_client_for("diet_guard", github) - except (ConfigError, FirebaseAuthError, RemoteSyncError) as exc: - _logger.warning("Firebase unavailable, syncing via GitHub only: %s", exc) - return github - - -def _client_for_run() -> RemoteStore: - """Return the backend for one sync run, requiring only *one* to be set up. - - The PAT is no longer mandatory. Firebase is the primary backend now, and - requiring a GitHub token before ever constructing it meant a - Firebase-configured machine with no PAT could not sync at all -- the - commits that fixed exactly this ("Auto-sync on a Firebase-only device") - changed only the Dart side, leaving the Python half behind. - - A missing PAT is therefore fatal only when Firebase is *also* unconfigured, - which is the genuine "nothing is set up" case the caller must report. - """ - try: - token = _read_token() - except SyncError: - if not CONFIG_FILE.is_file(): - raise - # Firebase-only: there is no GitHub client to mirror to, so use the - # primary directly rather than failing the whole run. - # - # ConfigError is translated rather than propagated: it is *not* a - # RemoteSyncError (it subclasses Exception directly), so letting it out - # would escape every caller's catch tuple and raise a traceback out of - # the gate's fail-closed "Fetch from sync" button -- the exact failure - # the RemoteSyncError swap was made to stop. A config file that exists - # but is unusable is reached by precisely this branch. - try: - return firebase_client_for("diet_guard") - except ConfigError as exc: - message = f"Firebase config at {CONFIG_FILE} is unusable: {exc}" - raise SyncError(message) from exc - return _remote_client( - GitHubSyncClient( - SYNC_REPO_OWNER, - SYNC_REPO_NAME, - token, - timeout_seconds=SYNC_TIMEOUT_SECONDS, - ) - ) - - -def _device_log_path(device_id: str) -> str: - """Return the repo-relative path a device's full log is pushed to.""" - return f"{_DEVICES_DIR}/{device_id}/food_log.json" - - -def _device_food_bank_path(device_id: str) -> str: - """Return one device's pushed derived-food-bank path.""" - return f"{_DEVICES_DIR}/{device_id}/food_bank.json" - - -def _device_manual_bank_path(device_id: str) -> str: - """Return one device's pushed curated-food-bank path.""" - return f"{_DEVICES_DIR}/{device_id}/food_bank_manual.json" - - -def _device_budget_path(device_id: str) -> str: - """Return the repo-relative path a device's budget is pushed to.""" - return f"{_DEVICES_DIR}/{device_id}/budget.json" - - -def _read_token() -> str: - """Return the saved sync PAT, stripped of trailing whitespace. - - Raises: - SyncError: If the token file is missing or empty -- the user has not - completed the one-time github.com setup step yet. - """ - if not SYNC_TOKEN_FILE.exists(): - message = ( - f"no sync token at {SYNC_TOKEN_FILE} -- create a fine-grained " - "GitHub PAT scoped to the syncs repo's contents and " - f"save it there (mode 600), then re-run sync" - ) - raise SyncError(message) - token = SYNC_TOKEN_FILE.read_text().strip() - if not token: - msg = f"{SYNC_TOKEN_FILE} is empty" - raise SyncError(msg) - return token def _remote_revisions(client: RemoteStore) -> dict[str, str]: @@ -254,152 +116,6 @@ def _pull_remote_logs( return remote_logs -def _sync_food_bank(client: GitHubSyncClient) -> None: - """Pull, merge, persist and push the log-derived food bank. - - Runs *after* the local rebuild in :func:`run_sync`, so this device's own - records already reflect the merged log; the merge then unions in whatever - another device knows and max-count wins per food (see - :func:`diet_guard._sync_merge.food_bank_to_log`). - - Strictly speaking the bank is derivable from the already-synced log, so - both devices converge on their own eventually. Syncing it makes them - agree *now*, and publishes the bank so a fresh device has autocomplete - before it has replayed anything. - - **The log stays authoritative for which foods exist.** A CRDT union - never shrinks, so without this the max-count merge would resurrect a food - whose entries were all undone -- a peer's stale copy would out-clock the - local absence and be written back and re-pushed forever, un-deletable. - Restricting the result to the foods the freshly-rebuilt local bank still - contains fixes that, and stays identical across devices because that - rebuild comes from the *merged* log both devices share. - """ - identity = device_identity() - local = read_food_bank() - merged = food_bank_to_log(local) - for device_id in client.list_directory(_DEVICES_DIR): - if identity.is_own(device_id): - continue - text = client.get_file_text(_device_food_bank_path(device_id)) - if text is None: - continue - try: - merged = merge_logs(merged, parse_remote_food_bank(text)) - except (TypeError, KeyError, ValueError, json.JSONDecodeError): - _logger.warning( - "Unparsable food bank pushed by device %r, skipping", - device_id, - ) - - if not merged: - return - resolved = { - name: record - for name, record in log_to_food_bank(merged).items() - if name in local - } - write_food_bank(resolved) - merged = food_bank_to_log(resolved) - client.put_file_text( - _device_food_bank_path(identity.device_id), - json.dumps( - {record_id: record.to_dict() for record_id, record in merged.items()}, - indent=2, - ), - message="diet_guard sync", - ) - - -def _sync_manual_bank(client: GitHubSyncClient) -> None: - """Pull, merge, persist and push the hand-curated food bank. - - Curated entries are the one part of the bank that is not derivable from - the food log (see :mod:`diet_guard._foodbank_manual`), so unlike - ``food_bank.json`` they need a real merge: last-writer-wins per food name - by edit time, union across devices. - """ - identity = device_identity() - merged = manual_bank_to_log(read_manual_bank()) - for device_id in client.list_directory(_DEVICES_DIR): - if identity.is_own(device_id): - continue - text = client.get_file_text(_device_manual_bank_path(device_id)) - if text is None: - continue - try: - merged = merge_logs(merged, parse_remote_manual_bank(text)) - except (TypeError, KeyError, ValueError, json.JSONDecodeError): - _logger.warning( - "Unparsable curated food bank pushed by device %r, skipping", - device_id, - ) - - if not merged: - # No device has curated anything: nothing to persist, and pushing an - # empty object every tick would be pure churn. - return - write_manual_bank(log_to_manual_bank(merged)) - client.put_file_text( - _device_manual_bank_path(identity.device_id), - json.dumps( - {record_id: record.to_dict() for record_id, record in merged.items()}, - indent=2, - ), - message="diet_guard sync", - ) - - -def _sync_budget(client: GitHubSyncClient) -> None: - """Pull other devices' budgets, merge, write locally, push this device's. - - Runs in the same tick as the food-log sync, reusing the already - authenticated ``client``. Merging is last-writer-wins by edit time (see - :mod:`diet_guard._sync_merge`'s budget adapters), not the food log's - union-of-immutable-entries -- a budget can be edited repeatedly. A - device that has never run ``init`` neither contributes a local record - to the merge nor overwrites a real budget pulled from elsewhere, and if - *no* device has ever set one, nothing is written or pushed. - """ - identity = device_identity() - merged = budget_to_log(read_raw_record(), load_entries()) - for device_id in client.list_directory(_DEVICES_DIR): - if identity.is_own(device_id): - continue - text = client.get_file_text(_device_budget_path(device_id)) - if text is None: - continue - try: - merged = merge_logs(merged, parse_remote_budget(text)) - except (TypeError, KeyError, ValueError, json.JSONDecodeError): - _logger.warning( - "Unparsable budget pushed by device %r, skipping", - device_id, - ) - - merged_record = log_to_budget(merged) - if merged_record is None: - return - write_raw_record(merged_record) - merged_history = log_to_history(merged) - # Only write back when the merge actually carried history. A pre-feature - # peer contributes none, and persisting an empty document would look like - # "history already exists" to any presence-based check and stop the local - # seed from ever running. - if merged_history: - write_raw_history(history_to_json(merged_history)) - - push_json = json.dumps( - {record_id: record.to_dict() for record_id, record in merged.items()}, - indent=2, - ) - client.put_file_text( - _device_budget_path(identity.device_id), - push_json, - message="diet_guard sync", - ) - - def run_sync() -> DayLog: """Run one full sync tick: pull, merge, re-sign, persist, push. diff --git a/diet_guard/_sync_banks.py b/diet_guard/_sync_banks.py new file mode 100644 index 0000000..53a864a --- /dev/null +++ b/diet_guard/_sync_banks.py @@ -0,0 +1,199 @@ +"""Pull/merge/push for the food bank and the budget. + +Split out of ``_sync.py`` for file size. These three run inside the same tick +as the food-log sync but are independent of it: each owns one remote document, +merges it with ``crdt_sync``'s LWW, writes the merged result back locally and +pushes it. + +The derived bank merges by ``count`` (max-count-wins, idempotent -- the right +merge for a counter derived from the log), the curated bank by ``editedAt``, +and the budget by its ``t`` edit stamp. All three are per-field LWW over the +*union* of field names, so a device predating a field relays it untouched. +""" + +from __future__ import annotations + +import json +import logging +from typing import TYPE_CHECKING + +from crdt_sync import merge_logs + +from diet_guard._budget import read_raw_record, write_raw_record +from diet_guard._budget_history import ( + history_to_json, + load_entries, + write_raw_history, +) +from diet_guard._device import device_identity +from diet_guard._foodbank import read_food_bank, write_food_bank +from diet_guard._foodbank_manual import read_manual_bank, write_manual_bank +from diet_guard._sync_paths import ( + _DEVICES_DIR, + _device_budget_path, + _device_food_bank_path, + _device_manual_bank_path, +) +from diet_guard.sync_merge import ( + budget_to_log, + food_bank_to_log, + log_to_budget, + log_to_food_bank, + log_to_history, + log_to_manual_bank, + manual_bank_to_log, + parse_remote_budget, + parse_remote_food_bank, + parse_remote_manual_bank, +) + +if TYPE_CHECKING: + from crdt_sync import GitHubSyncClient + +_logger = logging.getLogger(__name__) + + +def _sync_food_bank(client: GitHubSyncClient) -> None: + """Pull, merge, persist and push the log-derived food bank. + + Runs *after* the local rebuild in :func:`run_sync`, so this device's own + records already reflect the merged log; the merge then unions in whatever + another device knows and max-count wins per food (see + :func:`diet_guard.sync_merge.food_bank_to_log`). + + Strictly speaking the bank is derivable from the already-synced log, so + both devices converge on their own eventually. Syncing it makes them + agree *now*, and publishes the bank so a fresh device has autocomplete + before it has replayed anything. + + **The log stays authoritative for which foods exist.** A CRDT union + never shrinks, so without this the max-count merge would resurrect a food + whose entries were all undone -- a peer's stale copy would out-clock the + local absence and be written back and re-pushed forever, un-deletable. + Restricting the result to the foods the freshly-rebuilt local bank still + contains fixes that, and stays identical across devices because that + rebuild comes from the *merged* log both devices share. + """ + identity = device_identity() + local = read_food_bank() + merged = food_bank_to_log(local) + for device_id in client.list_directory(_DEVICES_DIR): + if identity.is_own(device_id): + continue + text = client.get_file_text(_device_food_bank_path(device_id)) + if text is None: + continue + try: + merged = merge_logs(merged, parse_remote_food_bank(text)) + except (TypeError, KeyError, ValueError, json.JSONDecodeError): + _logger.warning( + "Unparsable food bank pushed by device %r, skipping", + device_id, + ) + + if not merged: + return + resolved = { + name: record + for name, record in log_to_food_bank(merged).items() + if name in local + } + write_food_bank(resolved) + merged = food_bank_to_log(resolved) + client.put_file_text( + _device_food_bank_path(identity.device_id), + json.dumps( + {record_id: record.to_dict() for record_id, record in merged.items()}, + indent=2, + ), + message="diet_guard sync", + ) + + +def _sync_manual_bank(client: GitHubSyncClient) -> None: + """Pull, merge, persist and push the hand-curated food bank. + + Curated entries are the one part of the bank that is not derivable from + the food log (see :mod:`diet_guard._foodbank_manual`), so unlike + ``food_bank.json`` they need a real merge: last-writer-wins per food name + by edit time, union across devices. + """ + identity = device_identity() + merged = manual_bank_to_log(read_manual_bank()) + for device_id in client.list_directory(_DEVICES_DIR): + if identity.is_own(device_id): + continue + text = client.get_file_text(_device_manual_bank_path(device_id)) + if text is None: + continue + try: + merged = merge_logs(merged, parse_remote_manual_bank(text)) + except (TypeError, KeyError, ValueError, json.JSONDecodeError): + _logger.warning( + "Unparsable curated food bank pushed by device %r, skipping", + device_id, + ) + + if not merged: + # No device has curated anything: nothing to persist, and pushing an + # empty object every tick would be pure churn. + return + write_manual_bank(log_to_manual_bank(merged)) + client.put_file_text( + _device_manual_bank_path(identity.device_id), + json.dumps( + {record_id: record.to_dict() for record_id, record in merged.items()}, + indent=2, + ), + message="diet_guard sync", + ) + + +def _sync_budget(client: GitHubSyncClient) -> None: + """Pull other devices' budgets, merge, write locally, push this device's. + + Runs in the same tick as the food-log sync, reusing the already + authenticated ``client``. Merging is last-writer-wins by edit time (see + :mod:`diet_guard.sync_merge`'s budget adapters), not the food log's + union-of-immutable-entries -- a budget can be edited repeatedly. A + device that has never run ``init`` neither contributes a local record + to the merge nor overwrites a real budget pulled from elsewhere, and if + *no* device has ever set one, nothing is written or pushed. + """ + identity = device_identity() + merged = budget_to_log(read_raw_record(), load_entries()) + for device_id in client.list_directory(_DEVICES_DIR): + if identity.is_own(device_id): + continue + text = client.get_file_text(_device_budget_path(device_id)) + if text is None: + continue + try: + merged = merge_logs(merged, parse_remote_budget(text)) + except (TypeError, KeyError, ValueError, json.JSONDecodeError): + _logger.warning( + "Unparsable budget pushed by device %r, skipping", + device_id, + ) + + merged_record = log_to_budget(merged) + if merged_record is None: + return + write_raw_record(merged_record) + merged_history = log_to_history(merged) + # Only write back when the merge actually carried history. A pre-feature + # peer contributes none, and persisting an empty document would look like + # "history already exists" to any presence-based check and stop the local + # seed from ever running. + if merged_history: + write_raw_history(history_to_json(merged_history)) + + push_json = json.dumps( + {record_id: record.to_dict() for record_id, record in merged.items()}, + indent=2, + ) + client.put_file_text( + _device_budget_path(identity.device_id), + push_json, + message="diet_guard sync", + ) diff --git a/diet_guard/_sync_client.py b/diet_guard/_sync_client.py new file mode 100644 index 0000000..4f8cffa --- /dev/null +++ b/diet_guard/_sync_client.py @@ -0,0 +1,131 @@ +"""Choosing and building the remote store one sync tick will use. + +Split out of ``_sync.py`` for file size. + +Two backends, and the tick needs exactly one: Firebase (primary, configured +at ``~/.config/crdt-sync/``) or a GitHub PAT mirror at +``~/.config/diet_guard/sync_token``. Neither configured is not an error -- +:func:`_client_for_run` raises :class:`~diet_guard._sync.SyncError` and the +caller logs ``sync not configured`` and no-ops, which is what a machine that +has never been set up should do on a 15-minute timer. + +``ConfigError`` subclasses ``Exception`` directly rather than +``RemoteSyncError``, so it is caught explicitly here and translated -- a +sibling-not-subclass relationship that has bitten this file before. +""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING + +from crdt_sync import ( + CONFIG_FILE, + ConfigError, + FirebaseAuthError, + GitHubSyncClient, + RemoteSyncError, + firebase_client_for, + mirror_client_for, +) + +from diet_guard._constants import ( + SYNC_REPO_NAME, + SYNC_REPO_OWNER, + SYNC_TIMEOUT_SECONDS, + SYNC_TOKEN_FILE, +) +from diet_guard._sync_errors import SyncError + +if TYPE_CHECKING: + from crdt_sync import RemoteStore + +_logger = logging.getLogger(__name__) + + +def _remote_client(github: GitHubSyncClient) -> RemoteStore: + """Return the backend to sync against. + + Firebase when ``~/.config/crdt-sync/`` is set up, with GitHub kept as a + mirror so a device that has not moved yet still converges; GitHub alone + otherwise. An unconfigured machine keeps syncing exactly as before. + + Every sub-sync here (log, budget, food bank, manual bank) takes the client + as a parameter, so this one swap covers all of them. + + The config file is checked before constructing anything, so an + unconfigured machine never reaches the network -- otherwise a suite that + blocks real sockets fails here rather than in the code under test. + + Rolling back is deleting this function and passing ``github`` straight + through: no data moves either way. + """ + if not CONFIG_FILE.is_file(): + return github + try: + return mirror_client_for("diet_guard", github) + except (ConfigError, FirebaseAuthError, RemoteSyncError) as exc: + _logger.warning("Firebase unavailable, syncing via GitHub only: %s", exc) + return github + + +def _client_for_run() -> RemoteStore: + """Return the backend for one sync run, requiring only *one* to be set up. + + The PAT is no longer mandatory. Firebase is the primary backend now, and + requiring a GitHub token before ever constructing it meant a + Firebase-configured machine with no PAT could not sync at all -- the + commits that fixed exactly this ("Auto-sync on a Firebase-only device") + changed only the Dart side, leaving the Python half behind. + + A missing PAT is therefore fatal only when Firebase is *also* unconfigured, + which is the genuine "nothing is set up" case the caller must report. + """ + try: + token = _read_token() + except SyncError: + if not CONFIG_FILE.is_file(): + raise + # Firebase-only: there is no GitHub client to mirror to, so use the + # primary directly rather than failing the whole run. + # + # ConfigError is translated rather than propagated: it is *not* a + # RemoteSyncError (it subclasses Exception directly), so letting it out + # would escape every caller's catch tuple and raise a traceback out of + # the gate's fail-closed "Fetch from sync" button -- the exact failure + # the RemoteSyncError swap was made to stop. A config file that exists + # but is unusable is reached by precisely this branch. + try: + return firebase_client_for("diet_guard") + except ConfigError as exc: + message = f"Firebase config at {CONFIG_FILE} is unusable: {exc}" + raise SyncError(message) from exc + return _remote_client( + GitHubSyncClient( + SYNC_REPO_OWNER, + SYNC_REPO_NAME, + token, + timeout_seconds=SYNC_TIMEOUT_SECONDS, + ) + ) + + +def _read_token() -> str: + """Return the saved sync PAT, stripped of trailing whitespace. + + Raises: + SyncError: If the token file is missing or empty -- the user has not + completed the one-time github.com setup step yet. + """ + if not SYNC_TOKEN_FILE.exists(): + message = ( + f"no sync token at {SYNC_TOKEN_FILE} -- create a fine-grained " + "GitHub PAT scoped to the syncs repo's contents and " + f"save it there (mode 600), then re-run sync" + ) + raise SyncError(message) + token = SYNC_TOKEN_FILE.read_text().strip() + if not token: + msg = f"{SYNC_TOKEN_FILE} is empty" + raise SyncError(msg) + return token diff --git a/diet_guard/_sync_errors.py b/diet_guard/_sync_errors.py new file mode 100644 index 0000000..9fbeb70 --- /dev/null +++ b/diet_guard/_sync_errors.py @@ -0,0 +1,17 @@ +"""The one exception a sync tick raises before it can start. + +Its own module so that :mod:`diet_guard._sync_client`, which raises it, and +:mod:`diet_guard._sync`, which catches it and re-exports it for callers, do +not have to import each other. + +Deliberately *not* a subclass of ``crdt_sync.RemoteSyncError``: this means +"never got far enough to talk to a remote" (no PAT, no usable Firebase +config), which callers treat as a quiet no-op rather than as a sync failure +worth surfacing. +""" + +from __future__ import annotations + + +class SyncError(Exception): + """Raised when a sync run cannot even start (no usable PAT).""" diff --git a/diet_guard/_sync_merge.py b/diet_guard/_sync_merge.py deleted file mode 100644 index e9d6d0c..0000000 --- a/diet_guard/_sync_merge.py +++ /dev/null @@ -1,470 +0,0 @@ -"""Entry <-> crdt_sync.Record adapters for diet_guard's cross-device sync. - -diet_guard's own on-disk ``food_log.json`` format is unchanged (a -:class:`~diet_guard._state.DayLog`: date string -> list of entry dicts) -- -only the GitHub-synced wire format and the cross-device merge algorithm now -go through ``crdt_sync``'s ``Record``/``Log``/``Hlc`` primitives, the same -ones every other kuhy app that syncs this way uses (see ``~/crdt-sync``). - -Each diet_guard entry maps to one ``Record`` with a single opaque ``body`` -field holding everything except ``id``/``deleted``: entries are immutable -after creation (only ``deleted`` ever changes post-write, see -:func:`diet_guard._state.resign_entry`), so there is no benefit to -``crdt_sync``'s per-field LWW granularity here -- the whole body shares one -derived ``Hlc``. ``hmac`` travels inside ``body`` like any other field but -is never trusted on read; :func:`diet_guard._sync.run_sync` always re-signs -after merging, exactly as before this migration. - -Backward compatible with devices not yet migrated (the phone app, for now): -:func:`parse_remote_log` tries the new Record-based wire format first and -falls back to the old plain-DayLog format, converting old-format entries -through the same adapters used for the local log. Push always writes the -new format -- there is no code path left that ever *writes* the old one. - -The budget adapters at the bottom of this module (:func:`budget_to_log` -etc.) follow the same ``Record``/``Log`` shape, but differ in one important -way: a budget record is edited repeatedly (not immutable-after-creation -like a food-log entry), so its ``Hlc`` is derived from a ``t`` edit -timestamp that :func:`diet_guard._budget.write_budget` bumps on every -write, rather than from a birth time that never changes. There is no -legacy plain-``DayLog``-shaped fallback for budgets -- ``budget.json`` is a -brand-new sync payload, so every device pushing it already speaks the -Record-based wire format. -""" - -from __future__ import annotations - -from datetime import datetime, timezone -import hashlib -import json -from typing import TYPE_CHECKING - -from crdt_sync import Hlc, Record - -from diet_guard._budget_history import BudgetEntry -from diet_guard._device import device_id -from diet_guard._foodbank_manual import record_edit_time - -if TYPE_CHECKING: - from collections.abc import Mapping - - from crdt_sync import Log - - from diet_guard._state import DayLog - -_EPOCH = datetime(1970, 1, 1, tzinfo=timezone.utc) - - -def _wall_time_ms(stamp: str) -> int: - """Return ``stamp``'s epoch milliseconds, or the epoch when unparsable.""" - try: - moment = datetime.fromisoformat(stamp) - except ValueError: - moment = _EPOCH - return int(moment.timestamp() * 1000) - - -def _entry_hlc(entry: dict[str, object]) -> Hlc: - """Derive a deterministic Hlc for ``entry`` from its own ``time`` field. - - The same entry always yields the same Hlc regardless of when this runs - -- entries are immutable after creation, so there's no real "now" to - stamp, just the birth-time already recorded on the entry itself. - Malformed/missing ``time`` still gets a valid (if early-sorting) Hlc - rather than raising -- this only affects tie-breaking between - otherwise-identical copies of the same id, never whether the entry - survives a merge. - """ - try: - dt = datetime.fromisoformat(str(entry.get("time", ""))) - except ValueError: - dt = _EPOCH - wall_time_ms = int(dt.timestamp() * 1000) - return Hlc.new_tick(device_id(), wall_time_ms=wall_time_ms) - - -def _legacy_entry_id(entry: dict[str, object]) -> str: - """Deterministic id for a pre-``id`` legacy entry, from ``(time, desc)``. - - Two devices holding the same legacy entry independently derive the same - id without communicating, so they merge as one record instead of two -- - the same guarantee the old ``(time, desc)`` dedup key gave, just - expressed as a real id going forward. - """ - key = f"{entry.get('time')}|{entry.get('desc')}" - digest = hashlib.sha256(key.encode()).hexdigest()[:32] - return f"legacy-{digest}" - - -def entry_to_record(entry: dict[str, object]) -> Record: - """Convert one diet_guard log entry to a ``crdt_sync.Record``.""" - entry_id = entry.get("id") - if not isinstance(entry_id, str) or not entry_id: - entry_id = _legacy_entry_id(entry) - hlc = _entry_hlc(entry) - body = {k: v for k, v in entry.items() if k not in ("id", "deleted")} - deleted = bool(entry.get("deleted", False)) - return Record( - id=entry_id, - fields={"body": (body, hlc)}, - deleted=deleted, - deleted_hlc=hlc if deleted else None, - ) - - -def record_to_entry(record: Record) -> dict[str, object]: - """Convert one ``crdt_sync.Record`` back to a diet_guard log entry.""" - body_value, _hlc = record.fields.get("body", ({}, None)) - entry: dict[str, object] = dict(body_value) if isinstance(body_value, dict) else {} - entry["id"] = record.id - if record.deleted: - entry["deleted"] = True - return entry - - -def daylog_to_log(daylog: DayLog) -> Log: - """Convert a full local/remote DayLog into a ``crdt_sync.Log``.""" - log: Log = {} - for entries in daylog.values(): - for entry in entries: - record = entry_to_record(entry) - log[record.id] = record - return log - - -def log_to_daylog(log: Log) -> DayLog: - """Convert a merged ``crdt_sync.Log`` back into DayLog shape. - - Each entry is re-bucketed under its own ``time``'s date rather than - whatever date key it might have arrived under, and each day's entries - are sorted oldest-first -- matching the existing on-disk convention. - """ - daylog: DayLog = {} - for record in log.values(): - entry = record_to_entry(record) - date_key = str(entry.get("time", ""))[:10] - daylog.setdefault(date_key, []).append(entry) - for entries in daylog.values(): - entries.sort(key=lambda entry: str(entry.get("time", ""))) - return daylog - - -def _looks_like_new_format(raw: dict[str, object]) -> bool: - """Return True if ``raw`` is shaped like a crdt_sync Record-keyed Log. - - An empty object is ambiguous but harmless either way (no entries to - convert), so it's treated as new format to skip the old-format - conversion pass for nothing. - """ - return all( - isinstance(value, dict) and "fields" in value and "id" in value - for value in raw.values() - ) - - -def parse_remote_log(text: str) -> Log: - """Parse one device's pushed log text into a ``crdt_sync.Log``. - - Tries the new Record-based wire format first; falls back to the old - plain-DayLog format (today's on-the-wire shape) for devices not yet - migrated onto crdt_sync, converting their entries through the same - adapter the local log uses. Raises on genuinely malformed data, same as - the pre-migration behavior -- the caller - (:func:`diet_guard._sync._pull_remote_logs`) already logs-and-skips on - that. - - Raises: - TypeError: If the top-level JSON isn't an object, or a "new format" - value or an old-format day's entries aren't shaped as expected. - KeyError: Via ``Record.from_dict``, if a "new format" record is - missing an expected key. - ValueError: Via ``json.loads`` on invalid JSON, or ``Hlc.from_str`` - on a malformed clock string. - """ - raw = json.loads(text) - if not isinstance(raw, dict): - msg = f"top-level sync payload is not a JSON object: {raw!r}" - raise TypeError(msg) - if _looks_like_new_format(raw): - return {record_id: Record.from_dict(data) for record_id, data in raw.items()} - - daylog: DayLog = {} - for date_key, entries in raw.items(): - if not isinstance(entries, list): - msg = f"day {date_key!r} is not a JSON array: {entries!r}" - raise TypeError(msg) - for entry in entries: - if not isinstance(entry, dict): - msg = f"entry under day {date_key!r} is not a JSON object: {entry!r}" - raise TypeError(msg) - daylog[date_key] = entries - return daylog_to_log(daylog) - - -# Stable id: exactly one budget record per device-pushed budget.json. -_BUDGET_RECORD_ID = "budget" -# Field-name prefix for the effective-from budget history, one field per date. -# A separate *field* rather than a separate document, because merge_record -# unions field names -- see budget_to_log. -_HISTORY_FIELD_PREFIX = "hist:" -# Body weight, as its own field rather than a key inside ``value``. It is -# shared state like everything else -- both devices must see one value -- but -# it needs its own Hlc so a device that never sets it (the phone) cannot -# delete it merely by pushing a budget edit. -_WEIGHT_FIELD = "weight" - - -def _budget_hlc(record: dict[str, object]) -> Hlc: - """Derive a deterministic Hlc for a raw budget record from its ``t`` field. - - Mirrors :func:`_entry_hlc`'s determinism -- the same unedited record - always yields the same Hlc, so re-syncing an unchanged budget is a - no-op -- but reads ``t`` (bumped on every :func:`diet_guard._budget. - write_budget` call) rather than a fixed birth time, since a budget can - be edited repeatedly and the *edit* time is what last-writer-wins must - compare. - """ - try: - dt = datetime.fromisoformat(str(record.get("t", ""))) - except ValueError: - dt = _EPOCH - wall_time_ms = int(dt.timestamp() * 1000) - return Hlc.new_tick(device_id(), wall_time_ms=wall_time_ms) - - -def _history_hlc(entry: BudgetEntry) -> Hlc: - """Derive a deterministic Hlc for one history entry from its edit time. - - Same trick as :func:`_budget_hlc`: identical inputs always yield the same - clock, so re-syncing unchanged history is a no-op. Two devices that seed - the history independently derive the same ``wall_time_ms`` and the same - value and differ only in node id, so whichever side wins the field-level - LWW, the value is identical and the merge converges in one round. - """ - try: - moment = datetime.fromisoformat(entry.edited_at) - except ValueError: - moment = _EPOCH - return Hlc.new_tick(device_id(), wall_time_ms=int(moment.timestamp() * 1000)) - - -def budget_to_log( - record: dict[str, object] | None, - entries: tuple[BudgetEntry, ...] = (), -) -> Log: - """Convert a raw local budget record plus its history into a Log. - - Returns an empty ``Log`` when ``record`` is None (this device has never - run ``init``), so an uninitialized device contributes nothing to the - merge rather than clobbering another device's real value. - - The history rides along as one ``hist:`` field per entry on - the *same* record. ``crdt_sync``'s ``merge_record`` is per-field LWW over - the union of field names, so a device that predates the history neither - clobbers those fields nor blocks them: it merges them in from the remote - and pushes them straight back out (both sides push the *merged* log). - That is what makes this safe to roll out without a coordinated release. - - ``w`` (body weight) travels the same way, as its own :data:`_WEIGHT_FIELD` - rather than as a key inside ``value``. It is shared state -- both devices - must agree on it -- but inside ``value`` it was collateral damage of the - whole-map LWW: the phone rebuilds that map without ``w``, so any phone - budget edit silently deleted the stored weight and with it the protein - target. As its own field it is protected by per-field LWW, and a device - that never sets it relays it untouched instead of dropping it. - """ - if record is None: - return {} - hlc = _budget_hlc(record) - value = {k: v for k, v in record.items() if k not in {"t", "w"}} - fields: dict[str, tuple[object, Hlc]] = {"value": (value, hlc)} - weight = record.get("w") - if isinstance(weight, (int, float)) and not isinstance(weight, bool): - fields[_WEIGHT_FIELD] = (float(weight), hlc) - for entry in entries: - fields[f"{_HISTORY_FIELD_PREFIX}{entry.effective_from}"] = ( - entry.kcal, - _history_hlc(entry), - ) - rec = Record(id=_BUDGET_RECORD_ID, fields=fields) - return {rec.id: rec} - - -def log_to_history(log: Log) -> tuple[BudgetEntry, ...]: - """Extract the effective-from history from a merged budget ``Log``. - - Each entry's ``edited_at`` is reconstructed from its field Hlc rather than - carried separately, so the stored timestamp and the clock the merge - compared can never drift apart. - """ - record = log.get(_BUDGET_RECORD_ID) - if record is None: - return () - entries = [] - for name, (value, hlc) in record.fields.items(): - if not name.startswith(_HISTORY_FIELD_PREFIX): - continue - if isinstance(value, bool) or not isinstance(value, int): - continue - edited = datetime.fromtimestamp(hlc.wall_time_ms / 1000, tz=timezone.utc) - entries.append( - BudgetEntry( - effective_from=name[len(_HISTORY_FIELD_PREFIX) :], - kcal=value, - edited_at=edited.astimezone().isoformat(timespec="seconds"), - ), - ) - return tuple(sorted(entries, key=lambda e: e.effective_from)) - - -def log_to_budget(log: Log) -> dict[str, object] | None: - """Convert a merged budget ``Log`` back into a raw budget record. - - Returns None when the log has no budget record at all (neither device - has ever run ``init`` yet) -- callers treat that as "nothing to write - locally", not an error. - """ - record = log.get(_BUDGET_RECORD_ID) - if record is None: - return None - value, hlc = record.fields.get("value", ({}, None)) - result: dict[str, object] = dict(value) if isinstance(value, dict) else {} - if hlc is not None: - winning_time = datetime.fromtimestamp(hlc.wall_time_ms / 1000, tz=timezone.utc) - result["t"] = winning_time.astimezone().isoformat(timespec="seconds") - weight, _ = record.fields.get(_WEIGHT_FIELD, (None, None)) - if isinstance(weight, (int, float)) and not isinstance(weight, bool): - result["w"] = float(weight) - return result - - -def food_bank_to_log(bank: Mapping[str, object]) -> Log: - """Convert the log-derived food bank into a ``crdt_sync.Log``. - - The bank is *derived* -- both devices replay the same synced log and - compute the same records -- so this exists to make them agree - **immediately** rather than only after each has replayed, and to let a - device that is missing part of the log still autocomplete the other's - foods. - - The clock is the record's own ``count``, not a wall time. That makes - last-writer-wins mean *max-count-wins*, which is the right merge for a - derived counter: a device that has seen more of the log has the higher - count, and re-merging is idempotent because the count does not move - unless the log did. It also avoids inventing an edit timestamp for a - record nobody edits. - """ - log: Log = {} - for name, record in bank.items(): - if not isinstance(record, dict): - continue - count = record.get("count") - ticks = int(count) if isinstance(count, (int, float)) else 0 - hlc = Hlc.new_tick(device_id(), wall_time_ms=ticks) - log[name] = Record(id=name, fields={"body": (dict(record), hlc)}) - return log - - -def log_to_food_bank(log: Log) -> dict[str, dict[str, object]]: - """Convert a merged food-bank ``Log`` back into on-disk bank shape.""" - bank: dict[str, dict[str, object]] = {} - for name, record in log.items(): - if record.deleted: - continue - body, _hlc = record.fields.get("body", ({}, None)) - if isinstance(body, dict): - bank[name] = dict(body) - return bank - - -def parse_remote_food_bank(text: str) -> Log: - """Parse one device's pushed ``food_bank.json`` into a ``crdt_sync.Log``. - - Raises: - TypeError: If the top-level JSON isn't an object. - KeyError: Via ``Record.from_dict``, on a record missing a key. - ValueError: Via ``json.loads``/``Hlc.from_str`` on malformed input. - """ - raw = json.loads(text) - if not isinstance(raw, dict): - msg = f"top-level food-bank payload is not a JSON object: {raw!r}" - raise TypeError(msg) - return {record_id: Record.from_dict(data) for record_id, data in raw.items()} - - -def manual_bank_to_log(bank: Mapping[str, object]) -> Log: - """Convert the hand-curated food bank into a ``crdt_sync.Log``. - - One record per normalized food name, carrying the whole bank record as an - opaque ``body`` -- the same shape food-log entries use. Unlike an entry - a curated food is editable, so its Hlc comes from the record's own ``t`` - edit stamp (like the budget's) rather than a fixed birth time. - """ - log: Log = {} - for name, record in bank.items(): - if not isinstance(record, dict): - continue - hlc = Hlc.new_tick( - device_id(), - wall_time_ms=_wall_time_ms(record_edit_time(record)), - ) - body = {k: v for k, v in record.items() if k != "t"} - log[name] = Record(id=name, fields={"body": (body, hlc)}) - return log - - -def log_to_manual_bank(log: Log) -> dict[str, dict[str, object]]: - """Convert a merged curated-bank ``Log`` back into on-disk bank shape.""" - bank: dict[str, dict[str, object]] = {} - for name, record in log.items(): - if record.deleted: - continue - body, hlc = record.fields.get("body", ({}, None)) - if not isinstance(body, dict): - continue - stored = dict(body) - if hlc is not None: - winning = datetime.fromtimestamp(hlc.wall_time_ms / 1000, tz=timezone.utc) - stored["t"] = winning.astimezone().isoformat(timespec="seconds") - bank[name] = stored - return bank - - -def parse_remote_manual_bank(text: str) -> Log: - """Parse one device's pushed curated-bank text into a ``crdt_sync.Log``. - - Raises on malformed data; the caller logs-and-skips, matching every other - remote parser here. - - Raises: - TypeError: If the top-level JSON isn't an object. - KeyError: Via ``Record.from_dict``, on a record missing a key. - ValueError: Via ``json.loads``/``Hlc.from_str`` on malformed input. - """ - raw = json.loads(text) - if not isinstance(raw, dict): - msg = f"top-level curated-bank payload is not a JSON object: {raw!r}" - raise TypeError(msg) - return {record_id: Record.from_dict(data) for record_id, data in raw.items()} - - -def parse_remote_budget(text: str) -> Log: - """Parse one device's pushed ``budget.json`` text into a ``crdt_sync.Log``. - - Raises on malformed data; the caller - (:func:`diet_guard._sync._pull_remote_budgets`) logs-and-skips on that, - matching :func:`parse_remote_log`'s tolerance for a bad device file. - - Raises: - TypeError: If the top-level JSON isn't an object, or a record value - isn't shaped as expected. - KeyError: Via ``Record.from_dict``, if a record is missing an - expected key. - ValueError: Via ``json.loads`` on invalid JSON, or ``Hlc.from_str`` - on a malformed clock string. - """ - raw = json.loads(text) - if not isinstance(raw, dict): - msg = f"top-level budget payload is not a JSON object: {raw!r}" - raise TypeError(msg) - return {record_id: Record.from_dict(data) for record_id, data in raw.items()} diff --git a/diet_guard/_sync_paths.py b/diet_guard/_sync_paths.py new file mode 100644 index 0000000..1d6d21d --- /dev/null +++ b/diet_guard/_sync_paths.py @@ -0,0 +1,39 @@ +"""Repo-relative paths every device pushes its state to. + +Split out of ``_sync.py`` for file size. These are the wire layout of the +`kuhyx/syncs` repo, so they are shared by the tick that pushes the food log +(:mod:`diet_guard._sync`) and the one that pushes the banks and budget +(:mod:`diet_guard._sync_banks`) -- a second copy of the layout in either +module is how one of them starts writing where the other does not look. + +Every device pushes under its own persisted per-install uuid, never a role +constant; see :mod:`diet_guard._device`. +""" + +from __future__ import annotations + +_DEVICES_DIR = "diet-guard-sync/devices" + +#: Where per-device revision markers live, used to skip re-reading an +#: unchanged remote log. +_REVS_DIR = "diet-guard-sync/revs" + + +def _device_log_path(device_id: str) -> str: + """Return the repo-relative path a device's full log is pushed to.""" + return f"{_DEVICES_DIR}/{device_id}/food_log.json" + + +def _device_food_bank_path(device_id: str) -> str: + """Return one device's pushed derived-food-bank path.""" + return f"{_DEVICES_DIR}/{device_id}/food_bank.json" + + +def _device_manual_bank_path(device_id: str) -> str: + """Return one device's pushed curated-food-bank path.""" + return f"{_DEVICES_DIR}/{device_id}/food_bank_manual.json" + + +def _device_budget_path(device_id: str) -> str: + """Return the repo-relative path a device's budget is pushed to.""" + return f"{_DEVICES_DIR}/{device_id}/budget.json" diff --git a/diet_guard/sync_merge/__init__.py b/diet_guard/sync_merge/__init__.py new file mode 100644 index 0000000..86faed2 --- /dev/null +++ b/diet_guard/sync_merge/__init__.py @@ -0,0 +1,71 @@ +"""Entry <-> ``crdt_sync.Record`` adapters for diet_guard's cross-device sync. + +diet_guard's own on-disk ``food_log.json`` format is unchanged (a +:class:`~diet_guard._state.DayLog`: date string -> list of entry dicts) -- +only the GitHub-synced wire format and the cross-device merge algorithm go +through ``crdt_sync``'s ``Record``/``Log``/``Hlc`` primitives, the same ones +every other kuhy app that syncs this way uses (see ``~/crdt-sync``). + +Split into three submodules for file size, along the same seams the tests +already use: + +* :mod:`._daylog` -- the food log itself, including the legacy-format fallback +* :mod:`._budget` -- the budget record and its ``hist:`` history fields +* :mod:`._banks` -- the derived and curated halves of the food bank + +Everything public is re-exported here, so ``from diet_guard.sync_merge import +parse_remote_log`` reaches the same object the flat module exposed. (The +``merge_logs`` these adapters feed is ``crdt_sync``'s, imported directly by +:mod:`diet_guard._sync` -- it was never defined here.) +""" + +from __future__ import annotations + +from diet_guard.sync_merge._banks import ( + food_bank_to_log, + log_to_food_bank, + log_to_manual_bank, + manual_bank_to_log, + parse_remote_food_bank, + parse_remote_manual_bank, +) +from diet_guard.sync_merge._budget import ( + _budget_hlc, + budget_to_log, + log_to_budget, + log_to_history, + parse_remote_budget, +) +from diet_guard.sync_merge._daylog import ( + _entry_hlc, + _legacy_entry_id, + daylog_to_log, + entry_to_record, + log_to_daylog, + parse_remote_log, + record_to_entry, +) + +__all__ = [ + # Private helpers the tests reach for directly. Re-exported deliberately: + # they were importable from the flat module, and dropping them here would + # break those tests for a naming reason rather than a behavioural one. + "_budget_hlc", + "_entry_hlc", + "_legacy_entry_id", + "budget_to_log", + "daylog_to_log", + "entry_to_record", + "food_bank_to_log", + "log_to_budget", + "log_to_daylog", + "log_to_food_bank", + "log_to_history", + "log_to_manual_bank", + "manual_bank_to_log", + "parse_remote_budget", + "parse_remote_food_bank", + "parse_remote_log", + "parse_remote_manual_bank", + "record_to_entry", +] diff --git a/diet_guard/sync_merge/_banks.py b/diet_guard/sync_merge/_banks.py new file mode 100644 index 0000000..b617243 --- /dev/null +++ b/diet_guard/sync_merge/_banks.py @@ -0,0 +1,137 @@ +"""Food-bank <-> ``crdt_sync.Record`` adapters, for both halves of the bank. + +The derived bank's merge clock is the record's own ``count``, not a wall time: +last-writer-wins therefore means *max-count-wins*, which is the correct merge +for a derived counter (the device that has seen more of the log has the higher +count) and is idempotent, since the count only moves when the log does. + +The curated bank is one record per normalized name, LWW by an ``editedAt`` +stamp -- those foods were never eaten, so they are not derivable from any log. +""" + +from __future__ import annotations + +from datetime import datetime, timezone +import json +from typing import TYPE_CHECKING + +from crdt_sync import Hlc, Record + +from diet_guard._device import device_id +from diet_guard._foodbank_manual import record_edit_time +from diet_guard.sync_merge._clock import _wall_time_ms + +if TYPE_CHECKING: + from collections.abc import Mapping + + from crdt_sync import Log + + +def food_bank_to_log(bank: Mapping[str, object]) -> Log: + """Convert the log-derived food bank into a ``crdt_sync.Log``. + + The bank is *derived* -- both devices replay the same synced log and + compute the same records -- so this exists to make them agree + **immediately** rather than only after each has replayed, and to let a + device that is missing part of the log still autocomplete the other's + foods. + + The clock is the record's own ``count``, not a wall time. That makes + last-writer-wins mean *max-count-wins*, which is the right merge for a + derived counter: a device that has seen more of the log has the higher + count, and re-merging is idempotent because the count does not move + unless the log did. It also avoids inventing an edit timestamp for a + record nobody edits. + """ + log: Log = {} + for name, record in bank.items(): + if not isinstance(record, dict): + continue + count = record.get("count") + ticks = int(count) if isinstance(count, (int, float)) else 0 + hlc = Hlc.new_tick(device_id(), wall_time_ms=ticks) + log[name] = Record(id=name, fields={"body": (dict(record), hlc)}) + return log + + +def log_to_food_bank(log: Log) -> dict[str, dict[str, object]]: + """Convert a merged food-bank ``Log`` back into on-disk bank shape.""" + bank: dict[str, dict[str, object]] = {} + for name, record in log.items(): + if record.deleted: + continue + body, _hlc = record.fields.get("body", ({}, None)) + if isinstance(body, dict): + bank[name] = dict(body) + return bank + + +def parse_remote_food_bank(text: str) -> Log: + """Parse one device's pushed ``food_bank.json`` into a ``crdt_sync.Log``. + + Raises: + TypeError: If the top-level JSON isn't an object. + KeyError: Via ``Record.from_dict``, on a record missing a key. + ValueError: Via ``json.loads``/``Hlc.from_str`` on malformed input. + """ + raw = json.loads(text) + if not isinstance(raw, dict): + msg = f"top-level food-bank payload is not a JSON object: {raw!r}" + raise TypeError(msg) + return {record_id: Record.from_dict(data) for record_id, data in raw.items()} + + +def manual_bank_to_log(bank: Mapping[str, object]) -> Log: + """Convert the hand-curated food bank into a ``crdt_sync.Log``. + + One record per normalized food name, carrying the whole bank record as an + opaque ``body`` -- the same shape food-log entries use. Unlike an entry + a curated food is editable, so its Hlc comes from the record's own ``t`` + edit stamp (like the budget's) rather than a fixed birth time. + """ + log: Log = {} + for name, record in bank.items(): + if not isinstance(record, dict): + continue + hlc = Hlc.new_tick( + device_id(), + wall_time_ms=_wall_time_ms(record_edit_time(record)), + ) + body = {k: v for k, v in record.items() if k != "t"} + log[name] = Record(id=name, fields={"body": (body, hlc)}) + return log + + +def log_to_manual_bank(log: Log) -> dict[str, dict[str, object]]: + """Convert a merged curated-bank ``Log`` back into on-disk bank shape.""" + bank: dict[str, dict[str, object]] = {} + for name, record in log.items(): + if record.deleted: + continue + body, hlc = record.fields.get("body", ({}, None)) + if not isinstance(body, dict): + continue + stored = dict(body) + if hlc is not None: + winning = datetime.fromtimestamp(hlc.wall_time_ms / 1000, tz=timezone.utc) + stored["t"] = winning.astimezone().isoformat(timespec="seconds") + bank[name] = stored + return bank + + +def parse_remote_manual_bank(text: str) -> Log: + """Parse one device's pushed curated-bank text into a ``crdt_sync.Log``. + + Raises on malformed data; the caller logs-and-skips, matching every other + remote parser here. + + Raises: + TypeError: If the top-level JSON isn't an object. + KeyError: Via ``Record.from_dict``, on a record missing a key. + ValueError: Via ``json.loads``/``Hlc.from_str`` on malformed input. + """ + raw = json.loads(text) + if not isinstance(raw, dict): + msg = f"top-level curated-bank payload is not a JSON object: {raw!r}" + raise TypeError(msg) + return {record_id: Record.from_dict(data) for record_id, data in raw.items()} diff --git a/diet_guard/sync_merge/_budget.py b/diet_guard/sync_merge/_budget.py new file mode 100644 index 0000000..cce36ef --- /dev/null +++ b/diet_guard/sync_merge/_budget.py @@ -0,0 +1,186 @@ +"""Budget and budget-history <-> ``crdt_sync.Record`` adapters. + +Same ``Record``/``Log`` shape as the food-log adapters, with one important +difference: a budget record is edited repeatedly rather than being immutable +after creation, so its ``Hlc`` derives from a ``t`` edit timestamp that +:func:`diet_guard._budget.write_budget` bumps on every write, not from a birth +time that never changes. + +History rides along as ``hist:`` *fields* on the same record +rather than as a second document, so a device predating the feature relays +those fields untouched. There is no legacy fallback -- ``budget.json`` is a +brand-new sync payload, so every device pushing it already speaks this format. +""" + +from __future__ import annotations + +from datetime import datetime, timezone +import json +from typing import TYPE_CHECKING + +from crdt_sync import Hlc, Record + +from diet_guard._budget_history import BudgetEntry +from diet_guard._device import device_id +from diet_guard.sync_merge._clock import _EPOCH + +if TYPE_CHECKING: + from crdt_sync import Log + + +_BUDGET_RECORD_ID = "budget" +# Field-name prefix for the effective-from budget history, one field per date. +# A separate *field* rather than a separate document, because merge_record +# unions field names -- see budget_to_log. +_HISTORY_FIELD_PREFIX = "hist:" +# Body weight, as its own field rather than a key inside ``value``. It is +# shared state like everything else -- both devices must see one value -- but +# it needs its own Hlc so a device that never sets it (the phone) cannot +# delete it merely by pushing a budget edit. +_WEIGHT_FIELD = "weight" + + +def _budget_hlc(record: dict[str, object]) -> Hlc: + """Derive a deterministic Hlc for a raw budget record from its ``t`` field. + + Mirrors :func:`_entry_hlc`'s determinism -- the same unedited record + always yields the same Hlc, so re-syncing an unchanged budget is a + no-op -- but reads ``t`` (bumped on every :func:`diet_guard._budget. + write_budget` call) rather than a fixed birth time, since a budget can + be edited repeatedly and the *edit* time is what last-writer-wins must + compare. + """ + try: + dt = datetime.fromisoformat(str(record.get("t", ""))) + except ValueError: + dt = _EPOCH + wall_time_ms = int(dt.timestamp() * 1000) + return Hlc.new_tick(device_id(), wall_time_ms=wall_time_ms) + + +def _history_hlc(entry: BudgetEntry) -> Hlc: + """Derive a deterministic Hlc for one history entry from its edit time. + + Same trick as :func:`_budget_hlc`: identical inputs always yield the same + clock, so re-syncing unchanged history is a no-op. Two devices that seed + the history independently derive the same ``wall_time_ms`` and the same + value and differ only in node id, so whichever side wins the field-level + LWW, the value is identical and the merge converges in one round. + """ + try: + moment = datetime.fromisoformat(entry.edited_at) + except ValueError: + moment = _EPOCH + return Hlc.new_tick(device_id(), wall_time_ms=int(moment.timestamp() * 1000)) + + +def budget_to_log( + record: dict[str, object] | None, + entries: tuple[BudgetEntry, ...] = (), +) -> Log: + """Convert a raw local budget record plus its history into a Log. + + Returns an empty ``Log`` when ``record`` is None (this device has never + run ``init``), so an uninitialized device contributes nothing to the + merge rather than clobbering another device's real value. + + The history rides along as one ``hist:`` field per entry on + the *same* record. ``crdt_sync``'s ``merge_record`` is per-field LWW over + the union of field names, so a device that predates the history neither + clobbers those fields nor blocks them: it merges them in from the remote + and pushes them straight back out (both sides push the *merged* log). + That is what makes this safe to roll out without a coordinated release. + + ``w`` (body weight) travels the same way, as its own :data:`_WEIGHT_FIELD` + rather than as a key inside ``value``. It is shared state -- both devices + must agree on it -- but inside ``value`` it was collateral damage of the + whole-map LWW: the phone rebuilds that map without ``w``, so any phone + budget edit silently deleted the stored weight and with it the protein + target. As its own field it is protected by per-field LWW, and a device + that never sets it relays it untouched instead of dropping it. + """ + if record is None: + return {} + hlc = _budget_hlc(record) + value = {k: v for k, v in record.items() if k not in {"t", "w"}} + fields: dict[str, tuple[object, Hlc]] = {"value": (value, hlc)} + weight = record.get("w") + if isinstance(weight, (int, float)) and not isinstance(weight, bool): + fields[_WEIGHT_FIELD] = (float(weight), hlc) + for entry in entries: + fields[f"{_HISTORY_FIELD_PREFIX}{entry.effective_from}"] = ( + entry.kcal, + _history_hlc(entry), + ) + rec = Record(id=_BUDGET_RECORD_ID, fields=fields) + return {rec.id: rec} + + +def log_to_history(log: Log) -> tuple[BudgetEntry, ...]: + """Extract the effective-from history from a merged budget ``Log``. + + Each entry's ``edited_at`` is reconstructed from its field Hlc rather than + carried separately, so the stored timestamp and the clock the merge + compared can never drift apart. + """ + record = log.get(_BUDGET_RECORD_ID) + if record is None: + return () + entries = [] + for name, (value, hlc) in record.fields.items(): + if not name.startswith(_HISTORY_FIELD_PREFIX): + continue + if isinstance(value, bool) or not isinstance(value, int): + continue + edited = datetime.fromtimestamp(hlc.wall_time_ms / 1000, tz=timezone.utc) + entries.append( + BudgetEntry( + effective_from=name[len(_HISTORY_FIELD_PREFIX) :], + kcal=value, + edited_at=edited.astimezone().isoformat(timespec="seconds"), + ), + ) + return tuple(sorted(entries, key=lambda e: e.effective_from)) + + +def log_to_budget(log: Log) -> dict[str, object] | None: + """Convert a merged budget ``Log`` back into a raw budget record. + + Returns None when the log has no budget record at all (neither device + has ever run ``init`` yet) -- callers treat that as "nothing to write + locally", not an error. + """ + record = log.get(_BUDGET_RECORD_ID) + if record is None: + return None + value, hlc = record.fields.get("value", ({}, None)) + result: dict[str, object] = dict(value) if isinstance(value, dict) else {} + if hlc is not None: + winning_time = datetime.fromtimestamp(hlc.wall_time_ms / 1000, tz=timezone.utc) + result["t"] = winning_time.astimezone().isoformat(timespec="seconds") + weight, _ = record.fields.get(_WEIGHT_FIELD, (None, None)) + if isinstance(weight, (int, float)) and not isinstance(weight, bool): + result["w"] = float(weight) + return result + + +def parse_remote_budget(text: str) -> Log: + """Parse one device's pushed ``budget.json`` text into a ``crdt_sync.Log``. + + Raises on malformed data; the caller + (:func:`diet_guard._sync._pull_remote_budgets`) logs-and-skips on that, + matching :func:`parse_remote_log`'s tolerance for a bad device file. + + Raises: + TypeError: If the top-level JSON isn't an object, or a record value + isn't shaped as expected. + KeyError: Via ``Record.from_dict``, if a record is missing an + expected key. + ValueError: Via ``json.loads`` on invalid JSON, or ``Hlc.from_str`` + on a malformed clock string. + """ + raw = json.loads(text) + if not isinstance(raw, dict): + msg = f"top-level budget payload is not a JSON object: {raw!r}" + raise TypeError(msg) + return {record_id: Record.from_dict(data) for record_id, data in raw.items()} diff --git a/diet_guard/sync_merge/_clock.py b/diet_guard/sync_merge/_clock.py new file mode 100644 index 0000000..a38fce6 --- /dev/null +++ b/diet_guard/sync_merge/_clock.py @@ -0,0 +1,30 @@ +"""Wall-clock helpers shared by every adapter in this package. + +All three adapter modules derive an ``Hlc`` from some ISO timestamp -- an +entry's birth ``time``, a budget's ``t`` edit stamp, a curated food's +``editedAt``. They therefore share one parsing rule, and it lives here rather +than in whichever module happened to be written first: a second copy is how +two adapters start disagreeing about what an unparsable stamp means, which +would silently change merge outcomes rather than raising. +""" + +from __future__ import annotations + +from datetime import datetime, timezone + +_EPOCH = datetime(1970, 1, 1, tzinfo=timezone.utc) + + +def _wall_time_ms(stamp: str) -> int: + """Return ``stamp``'s epoch milliseconds, or the epoch when unparsable. + + Falling back to the epoch rather than raising keeps a malformed stamp from + dropping the record entirely: it sorts early in a last-writer-wins merge, + so a well-formed copy from any other device wins, which is the outcome the + user wants. + """ + try: + moment = datetime.fromisoformat(stamp) + except ValueError: + moment = _EPOCH + return int(moment.timestamp() * 1000) diff --git a/diet_guard/sync_merge/_daylog.py b/diet_guard/sync_merge/_daylog.py new file mode 100644 index 0000000..9b86916 --- /dev/null +++ b/diet_guard/sync_merge/_daylog.py @@ -0,0 +1,170 @@ +"""Food-log entry <-> ``crdt_sync.Record`` adapters. + +Each entry maps to one ``Record`` with a single opaque ``body`` field holding +everything except ``id``/``deleted``: entries are immutable after creation +(only ``deleted`` ever changes post-write, see +:func:`diet_guard._state.resign_entry`), so the whole body shares one derived +``Hlc``. ``hmac`` travels inside ``body`` but is never trusted on read -- +:func:`diet_guard._sync.run_sync` always re-signs after merging. + +Backward compatible with devices not yet migrated: +:func:`parse_remote_log` tries the Record-based wire format first and falls +back to the old plain-``DayLog`` format. Push always writes the new format. +""" + +from __future__ import annotations + +from datetime import datetime +import hashlib +import json +from typing import TYPE_CHECKING + +from crdt_sync import Hlc, Record + +from diet_guard._device import device_id +from diet_guard.sync_merge._clock import _EPOCH + +if TYPE_CHECKING: + from crdt_sync import Log + + from diet_guard._state import DayLog + + +def _entry_hlc(entry: dict[str, object]) -> Hlc: + """Derive a deterministic Hlc for ``entry`` from its own ``time`` field. + + The same entry always yields the same Hlc regardless of when this runs + -- entries are immutable after creation, so there's no real "now" to + stamp, just the birth-time already recorded on the entry itself. + Malformed/missing ``time`` still gets a valid (if early-sorting) Hlc + rather than raising -- this only affects tie-breaking between + otherwise-identical copies of the same id, never whether the entry + survives a merge. + """ + try: + dt = datetime.fromisoformat(str(entry.get("time", ""))) + except ValueError: + dt = _EPOCH + wall_time_ms = int(dt.timestamp() * 1000) + return Hlc.new_tick(device_id(), wall_time_ms=wall_time_ms) + + +def _legacy_entry_id(entry: dict[str, object]) -> str: + """Deterministic id for a pre-``id`` legacy entry, from ``(time, desc)``. + + Two devices holding the same legacy entry independently derive the same + id without communicating, so they merge as one record instead of two -- + the same guarantee the old ``(time, desc)`` dedup key gave, just + expressed as a real id going forward. + """ + key = f"{entry.get('time')}|{entry.get('desc')}" + digest = hashlib.sha256(key.encode()).hexdigest()[:32] + return f"legacy-{digest}" + + +def entry_to_record(entry: dict[str, object]) -> Record: + """Convert one diet_guard log entry to a ``crdt_sync.Record``.""" + entry_id = entry.get("id") + if not isinstance(entry_id, str) or not entry_id: + entry_id = _legacy_entry_id(entry) + hlc = _entry_hlc(entry) + body = {k: v for k, v in entry.items() if k not in ("id", "deleted")} + deleted = bool(entry.get("deleted", False)) + return Record( + id=entry_id, + fields={"body": (body, hlc)}, + deleted=deleted, + deleted_hlc=hlc if deleted else None, + ) + + +def record_to_entry(record: Record) -> dict[str, object]: + """Convert one ``crdt_sync.Record`` back to a diet_guard log entry.""" + body_value, _hlc = record.fields.get("body", ({}, None)) + entry: dict[str, object] = dict(body_value) if isinstance(body_value, dict) else {} + entry["id"] = record.id + if record.deleted: + entry["deleted"] = True + return entry + + +def daylog_to_log(daylog: DayLog) -> Log: + """Convert a full local/remote DayLog into a ``crdt_sync.Log``.""" + log: Log = {} + for entries in daylog.values(): + for entry in entries: + record = entry_to_record(entry) + log[record.id] = record + return log + + +def log_to_daylog(log: Log) -> DayLog: + """Convert a merged ``crdt_sync.Log`` back into DayLog shape. + + Each entry is re-bucketed under its own ``time``'s date rather than + whatever date key it might have arrived under, and each day's entries + are sorted oldest-first -- matching the existing on-disk convention. + """ + daylog: DayLog = {} + for record in log.values(): + entry = record_to_entry(record) + date_key = str(entry.get("time", ""))[:10] + daylog.setdefault(date_key, []).append(entry) + for entries in daylog.values(): + entries.sort(key=lambda entry: str(entry.get("time", ""))) + return daylog + + +def _looks_like_new_format(raw: dict[str, object]) -> bool: + """Return True if ``raw`` is shaped like a crdt_sync Record-keyed Log. + + An empty object is ambiguous but harmless either way (no entries to + convert), so it's treated as new format to skip the old-format + conversion pass for nothing. + """ + return all( + isinstance(value, dict) and "fields" in value and "id" in value + for value in raw.values() + ) + + +def parse_remote_log(text: str) -> Log: + """Parse one device's pushed log text into a ``crdt_sync.Log``. + + Tries the new Record-based wire format first; falls back to the old + plain-DayLog format (today's on-the-wire shape) for devices not yet + migrated onto crdt_sync, converting their entries through the same + adapter the local log uses. Raises on genuinely malformed data, same as + the pre-migration behavior -- the caller + (:func:`diet_guard._sync._pull_remote_logs`) already logs-and-skips on + that. + + Raises: + TypeError: If the top-level JSON isn't an object, or a "new format" + value or an old-format day's entries aren't shaped as expected. + KeyError: Via ``Record.from_dict``, if a "new format" record is + missing an expected key. + ValueError: Via ``json.loads`` on invalid JSON, or ``Hlc.from_str`` + on a malformed clock string. + """ + raw = json.loads(text) + if not isinstance(raw, dict): + msg = f"top-level sync payload is not a JSON object: {raw!r}" + raise TypeError(msg) + if _looks_like_new_format(raw): + return {record_id: Record.from_dict(data) for record_id, data in raw.items()} + + daylog: DayLog = {} + for date_key, entries in raw.items(): + if not isinstance(entries, list): + msg = f"day {date_key!r} is not a JSON array: {entries!r}" + raise TypeError(msg) + for entry in entries: + if not isinstance(entry, dict): + msg = f"entry under day {date_key!r} is not a JSON object: {entry!r}" + raise TypeError(msg) + daylog[date_key] = entries + return daylog_to_log(daylog) + + +# Stable id: exactly one budget record per device-pushed budget.json. diff --git a/diet_guard/tests/_gate_fixtures.py b/diet_guard/tests/_gate_fixtures.py new file mode 100644 index 0000000..a4c0a07 --- /dev/null +++ b/diet_guard/tests/_gate_fixtures.py @@ -0,0 +1,182 @@ +"""Fixtures that keep the gate's Tk widgets fake and its outputs hermetic. + +Split out of ``conftest.py`` for file size, and imported back into it by name +-- pytest only discovers fixtures defined *or imported* in a conftest, and +``pytest_plugins`` is an error in a non-root conftest, so the import in +``conftest.py`` is what registers these. Do not "tidy" it away. + +Every module that builds gate widgets is patched as one set: hand-picking a +subset is how a test ends up mixing a real ``tk.Frame`` into a fake notebook, +which fails far from the cause. +""" + +from __future__ import annotations + +from contextlib import ExitStack, contextmanager +from typing import TYPE_CHECKING +from unittest.mock import MagicMock, patch + +from gatelock import Output, OutputRect +from gatelock import _scrollable as _gatelock_scrollable +from gatelock import widgets as _gatelock_widgets +import pytest + +from diet_guard import ( + _gatelock, + _gatelock_calendar, + _gatelock_calendar_ui, + _gatelock_core, + _gatelock_mealflow, + _gatelock_nutrition, + _gatelock_ui, +) +from diet_guard._gatelock import MealGate +from diet_guard.tests._tk_fakes import ( + _FAKE_TK, + _FAKE_TTK, +) + +if TYPE_CHECKING: + from collections.abc import Iterator + from pathlib import Path + + +@pytest.fixture(autouse=True) +def _block_real_tk() -> Iterator[None]: + """Replace tk + the window class in _gatelock so no real window can open.""" + with ( + patch("diet_guard._gatelock.tk", MagicMock()), + patch("diet_guard._gatelock.GateRoot", MagicMock()), + ): + yield + + +FAKE_OUTPUTS = ( + Output("DP-0", connected=True, rect=OutputRect(0, 0, 1920, 1080), primary=True), +) +"""One live output by default, so the Tk-screen fallback (which would call +int() on a MagicMock) is never reached.""" + +TWO_OUTPUTS = ( + Output("DP-0", connected=True, rect=OutputRect(0, 0, 3840, 2160), primary=True), + Output("HDMI-0", connected=True, rect=OutputRect(3840, 0, 2560, 1440)), +) +"""The real desk. Opt in with ``dual_output`` for anything asserting that the +gate is really built on every monitor.""" + + +def _make_toplevel(_parent: object = None, **_kwargs: object) -> MagicMock: + """A Toplevel stand-in for gatelock's per-output surfaces.""" + window = MagicMock() + window.winfo_children.return_value = [] + return window + + +@pytest.fixture(autouse=True) +def _hermetic_gatelock( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> Iterator[None]: + """Keep gatelock v0.2.0's per-output machinery off the real machine. + + ``_block_real_tk`` patches ``tk`` inside *diet_guard*, which is no longer + enough: ``LockWindow.setup()`` builds one real ``tk.Toplevel`` per live + output through *gatelock's* own ``tk``. Over a MagicMock root that sends + real tkinter into unbounded mock recursion, so the suite **hangs** rather + than failing. The runtime dir is redirected too, so no test can stand a + production locker down through the arbiter. + """ + monkeypatch.setenv("GATELOCK_RUNTIME_DIR", str(tmp_path / "gatelock-runtime")) + with ( + patch("gatelock._surfaces.tk.Toplevel", side_effect=_make_toplevel), + patch("gatelock._outputs.RandrBackend.create", return_value=None), + patch("gatelock._outputs.scan_xrandr", return_value=FAKE_OUTPUTS), + patch("gatelock._detect._RandrEventSource.start", return_value=False), + ): + yield + + +@pytest.fixture +def dual_output(_hermetic_gatelock: None) -> Iterator[None]: + """Re-scan as a two-monitor desk, layered over the single-output default.""" + with patch("gatelock._outputs.scan_xrandr", return_value=TWO_OUTPUTS): + yield + + +@pytest.fixture(autouse=True) +def _block_real_vt() -> Iterator[None]: + """Make gatelock's VT-switch disable a no-op for every test. + + Belt-and-suspenders alongside ``_block_real_tk``: VT-disable now lives in + ``gatelock``, independent of the (mocked) root, so a test that builds a + real prod-mode (``demo_mode=False``) gate would otherwise run a genuine + ``setxkbmap`` against whatever X session the test happens to run under. + """ + with patch("gatelock._vt.shutil.which", return_value=None): + yield + + +@pytest.fixture(autouse=True) +def _hmac_key(tmp_path: Path) -> Iterator[None]: + """Point the shared HMAC key at a deterministic temp file. + + Makes signing/verification work the same in any environment (including CI, + which has no ``/etc/workout-locker/hmac.key``). Tests that need the + no-key path patch ``compute_entry_hmac`` to return None locally. + """ + key = tmp_path / "hmac.key" + key.write_bytes(b"diet-guard-test-key-0123456789ab") + with patch("gatelock.log_integrity.DEFAULT_HMAC_KEY_FILE", key): + yield + + +# -------------------------------------------------------------------------- +# Gate fixture and its functional tk fakes +# -------------------------------------------------------------------------- +# +# A functional fake ``tk`` (stateful Entry/Text/Listbox/StringVar widgets and a +# real, catchable ``TclError``) replaces the blanket MagicMock above for the +# duration of each gate test, so the window's *logic* runs for real against +# in-memory widgets without ever opening a window or grabbing the keyboard. + + +_GATE_TK_MODULES = ( + _gatelock, + # The gate's buttons are built by gatelock now, so its widget module + # needs the fake tk too -- otherwise the gate tests open real windows. + _gatelock_widgets, + _gatelock_calendar, + _gatelock_calendar_ui, + _gatelock_core, + _gatelock_nutrition, + _gatelock_mealflow, + _gatelock_ui, + # The gate's scroll viewport lives in gatelock, not here, and imports + # tkinter independently -- so it needs the fake too, or a real tk.Frame + # ends up parented to a FakeNotebook. + _gatelock_scrollable, +) + + +@contextmanager +def fake_tk() -> Iterator[None]: + """Patch every module that builds gate widgets to use the fakes. + + Exposed so tests that construct a gate outside the :func:`gate` fixture + patch the *same* set. Hand-picking a subset is how a test ends up mixing a + real ``tk.Label`` with a fake parent: the widget tree spans several modules + plus ``gatelock``, and any module left real will try to talk to a fake + master. + """ + with ExitStack() as stack: + for module in _GATE_TK_MODULES: + stack.enter_context(patch.object(module, "tk", _FAKE_TK)) + stack.enter_context(patch.object(_gatelock_calendar, "ttk", _FAKE_TTK)) + stack.enter_context(patch.object(_gatelock_calendar_ui, "ttk", _FAKE_TTK)) + yield + + +@pytest.fixture +def gate() -> Iterator[MealGate]: + """Build a demo gate whose widgets are functional fakes.""" + with fake_tk(): + yield MealGate(demo_mode=True) diff --git a/diet_guard/tests/_sync_fakes.py b/diet_guard/tests/_sync_fakes.py new file mode 100644 index 0000000..6488f09 --- /dev/null +++ b/diet_guard/tests/_sync_fakes.py @@ -0,0 +1,59 @@ +"""Shared fakes for the sync tests. + +Not a ``test_`` module: these are fixtures-by-hand imported by +``test_sync.py``, ``test_sync_banks.py`` and ``test_sync_pull.py``, and +pytest must not collect them as tests. The repo already excludes this +naming pattern from the ``name-tests-test`` hook. +""" + +from __future__ import annotations + +import json +from unittest.mock import MagicMock + +from diet_guard import _sync_client +from diet_guard._estimator import Nutrition +from diet_guard.sync_merge import budget_to_log + + +def _remote_budget_json(*, kcal: int, t: str, weight_kg: float | None = None) -> str: + """Build the wire text a remote device would push for a given budget edit.""" + record: dict[str, object] = {"v": 2, "b": kcal, "t": t} + if weight_kg is not None: + record["w"] = weight_kg + log = budget_to_log(record) + return json.dumps({rid: rec.to_dict() for rid, rec in log.items()}, indent=2) + + +def _nutrition(kcal: float = 200.0) -> Nutrition: + return Nutrition( + kcal=kcal, + protein_g=10.0, + carbs_g=20.0, + fat_g=5.0, + grams=100.0, + source="manual", + ) + + +def _write_token(token: str = "fake-token") -> None: + _sync_client.SYNC_TOKEN_FILE.parent.mkdir(parents=True, exist_ok=True) + _sync_client.SYNC_TOKEN_FILE.write_text(token) + + +def _mock_client( + *, + devices: tuple[str, ...] = (), + files: dict[str, str] | None = None, +) -> MagicMock: + """Build a mock ``GitHubSyncClient`` covering the methods sync calls.""" + client = MagicMock() + client.list_directory.return_value = list(devices) + resolved_files = files or {} + client.get_file_text.side_effect = resolved_files.get + # The real GitHubSyncClient has no bulk-map read, and a MagicMock would + # otherwise auto-create one returning a Mock -- which then fails to + # serialise. Deleting it also exercises the documented degrade path: + # no revision map means every peer is fetched, exactly as before. + del client.get_string_map + return client diff --git a/diet_guard/tests/conftest.py b/diet_guard/tests/conftest.py index f320f90..bb01fcd 100644 --- a/diet_guard/tests/conftest.py +++ b/diet_guard/tests/conftest.py @@ -20,26 +20,27 @@ are functional in-memory stand-ins, shared by ``test_gatelock.py`` and from __future__ import annotations -from contextlib import ExitStack, contextmanager from typing import TYPE_CHECKING -from unittest.mock import MagicMock, patch +from unittest.mock import patch -from gatelock import Output, OutputRect -from gatelock import _scrollable as _gatelock_scrollable -from gatelock import widgets as _gatelock_widgets import pytest -from diet_guard import ( - _gatelock, - _gatelock_calendar, - _gatelock_calendar_ui, - _gatelock_core, - _gatelock_mealflow, - _gatelock_nutrition, - _gatelock_ui, -) from diet_guard._estimator import Nutrition -from diet_guard._gatelock import MealGate + +# Importing these fixtures IS their registration: pytest discovers fixtures +# defined *or imported* in a conftest, and ``pytest_plugins`` is an error in a +# non-root conftest. They look unused and are not -- do not "tidy" them away. +from diet_guard.tests._gate_fixtures import ( + _GATE_TK_MODULES, + FAKE_OUTPUTS, + TWO_OUTPUTS, + _block_real_tk, + _block_real_vt, + _hermetic_gatelock, + dual_output, + fake_tk, + gate, +) from diet_guard.tests._tk_fakes import ( _FAKE_TK, _FAKE_TTK, @@ -57,12 +58,15 @@ from diet_guard.tests._tk_fakes import ( ) # Re-exported: the fake widgets moved into the package so this file stays -# under the 500-line cap, but tests import them from conftest by name. +# under the 250-line cap, but tests import them from conftest by name. __all__ = [ "FAKE_OUTPUTS", "TWO_OUTPUTS", "_FAKE_TK", "_FAKE_TTK", + # The autouse fixtures: importing them here is what registers them with + # pytest, and naming them here is what stops a linter deleting the import. + "_GATE_TK_MODULES", "FakeCanvas", "FakeEntry", "FakeListbox", @@ -74,8 +78,14 @@ __all__ = [ "FakeVar", "FakeWidget", "_FakeTclError", + "_block_real_tk", + "_block_real_vt", + "_hermetic_gatelock", + "dual_output", "dual_output", "fake_tk", + "fake_tk", + "gate", "gate", ] @@ -113,7 +123,7 @@ def _isolate_state(tmp_path: Path) -> Iterator[None]: tmp_path / ".gate.lock", ), patch( - "diet_guard._sync.SYNC_TOKEN_FILE", + "diet_guard._sync_client.SYNC_TOKEN_FILE", tmp_path / "sync_token", ), patch( @@ -131,154 +141,13 @@ def _isolate_state(tmp_path: Path) -> Iterator[None]: # mirror. On a developer machine the real file exists, so without this # every sync test would sign in and push to the live database. patch( - "diet_guard._sync.CONFIG_FILE", + "diet_guard._sync_client.CONFIG_FILE", tmp_path / "nonexistent-firebase.json", ), ): yield -@pytest.fixture(autouse=True) -def _block_real_tk() -> Iterator[None]: - """Replace tk + the window class in _gatelock so no real window can open.""" - with ( - patch("diet_guard._gatelock.tk", MagicMock()), - patch("diet_guard._gatelock.GateRoot", MagicMock()), - ): - yield - - -FAKE_OUTPUTS = ( - Output("DP-0", connected=True, rect=OutputRect(0, 0, 1920, 1080), primary=True), -) -"""One live output by default, so the Tk-screen fallback (which would call -int() on a MagicMock) is never reached.""" - -TWO_OUTPUTS = ( - Output("DP-0", connected=True, rect=OutputRect(0, 0, 3840, 2160), primary=True), - Output("HDMI-0", connected=True, rect=OutputRect(3840, 0, 2560, 1440)), -) -"""The real desk. Opt in with ``dual_output`` for anything asserting that the -gate is really built on every monitor.""" - - -def _make_toplevel(_parent: object = None, **_kwargs: object) -> MagicMock: - """A Toplevel stand-in for gatelock's per-output surfaces.""" - window = MagicMock() - window.winfo_children.return_value = [] - return window - - -@pytest.fixture(autouse=True) -def _hermetic_gatelock( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch -) -> Iterator[None]: - """Keep gatelock v0.2.0's per-output machinery off the real machine. - - ``_block_real_tk`` patches ``tk`` inside *diet_guard*, which is no longer - enough: ``LockWindow.setup()`` builds one real ``tk.Toplevel`` per live - output through *gatelock's* own ``tk``. Over a MagicMock root that sends - real tkinter into unbounded mock recursion, so the suite **hangs** rather - than failing. The runtime dir is redirected too, so no test can stand a - production locker down through the arbiter. - """ - monkeypatch.setenv("GATELOCK_RUNTIME_DIR", str(tmp_path / "gatelock-runtime")) - with ( - patch("gatelock._surfaces.tk.Toplevel", side_effect=_make_toplevel), - patch("gatelock._outputs.RandrBackend.create", return_value=None), - patch("gatelock._outputs.scan_xrandr", return_value=FAKE_OUTPUTS), - patch("gatelock._detect._RandrEventSource.start", return_value=False), - ): - yield - - -@pytest.fixture -def dual_output(_hermetic_gatelock: None) -> Iterator[None]: - """Re-scan as a two-monitor desk, layered over the single-output default.""" - with patch("gatelock._outputs.scan_xrandr", return_value=TWO_OUTPUTS): - yield - - -@pytest.fixture(autouse=True) -def _block_real_vt() -> Iterator[None]: - """Make gatelock's VT-switch disable a no-op for every test. - - Belt-and-suspenders alongside ``_block_real_tk``: VT-disable now lives in - ``gatelock``, independent of the (mocked) root, so a test that builds a - real prod-mode (``demo_mode=False``) gate would otherwise run a genuine - ``setxkbmap`` against whatever X session the test happens to run under. - """ - with patch("gatelock._vt.shutil.which", return_value=None): - yield - - -@pytest.fixture(autouse=True) -def _hmac_key(tmp_path: Path) -> Iterator[None]: - """Point the shared HMAC key at a deterministic temp file. - - Makes signing/verification work the same in any environment (including CI, - which has no ``/etc/workout-locker/hmac.key``). Tests that need the - no-key path patch ``compute_entry_hmac`` to return None locally. - """ - key = tmp_path / "hmac.key" - key.write_bytes(b"diet-guard-test-key-0123456789ab") - with patch("gatelock.log_integrity.DEFAULT_HMAC_KEY_FILE", key): - yield - - -# -------------------------------------------------------------------------- -# Gate fixture and its functional tk fakes -# -------------------------------------------------------------------------- -# -# A functional fake ``tk`` (stateful Entry/Text/Listbox/StringVar widgets and a -# real, catchable ``TclError``) replaces the blanket MagicMock above for the -# duration of each gate test, so the window's *logic* runs for real against -# in-memory widgets without ever opening a window or grabbing the keyboard. - - -_GATE_TK_MODULES = ( - _gatelock, - # The gate's buttons are built by gatelock now, so its widget module - # needs the fake tk too -- otherwise the gate tests open real windows. - _gatelock_widgets, - _gatelock_calendar, - _gatelock_calendar_ui, - _gatelock_core, - _gatelock_nutrition, - _gatelock_mealflow, - _gatelock_ui, - # The gate's scroll viewport lives in gatelock, not here, and imports - # tkinter independently -- so it needs the fake too, or a real tk.Frame - # ends up parented to a FakeNotebook. - _gatelock_scrollable, -) - - -@contextmanager -def fake_tk() -> Iterator[None]: - """Patch every module that builds gate widgets to use the fakes. - - Exposed so tests that construct a gate outside the :func:`gate` fixture - patch the *same* set. Hand-picking a subset is how a test ends up mixing a - real ``tk.Label`` with a fake parent: the widget tree spans several modules - plus ``gatelock``, and any module left real will try to talk to a fake - master. - """ - with ExitStack() as stack: - for module in _GATE_TK_MODULES: - stack.enter_context(patch.object(module, "tk", _FAKE_TK)) - stack.enter_context(patch.object(_gatelock_calendar, "ttk", _FAKE_TTK)) - stack.enter_context(patch.object(_gatelock_calendar_ui, "ttk", _FAKE_TTK)) - yield - - -@pytest.fixture -def gate() -> Iterator[MealGate]: - """Build a demo gate whose widgets are functional fakes.""" - with fake_tk(): - yield MealGate(demo_mode=True) - - def _nutrition(kcal: float = 100, grams: float = 100) -> Nutrition: """A simple reference nutrition for driving the gate form.""" return Nutrition(kcal, 10, 20, 5, grams, "food bank") diff --git a/diet_guard/tests/test_sync.py b/diet_guard/tests/test_sync.py index 8d5a803..16fae66 100644 --- a/diet_guard/tests/test_sync.py +++ b/diet_guard/tests/test_sync.py @@ -8,90 +8,36 @@ The GitHub layer is mocked (no network access); conftest.py's from __future__ import annotations import json -from unittest.mock import MagicMock, patch +from unittest.mock import patch -from crdt_sync import ( - FirebaseAuthError, - FirebaseSyncError, - GitHubSyncError, - SyncState, -) import pytest -from diet_guard import _sync -from diet_guard._budget import budget_weight, daily_budget, write_budget -from diet_guard._budget_history import load_entries -from diet_guard._device import device_id, device_identity -from diet_guard._estimator import Nutrition +from diet_guard import _sync, _sync_client +from diet_guard._device import device_id from diet_guard._foodbank import lookup_food from diet_guard._state import load_log, log_meal -from diet_guard._sync_merge import ( - budget_to_log, -) - - -def _remote_budget_json(*, kcal: int, t: str, weight_kg: float | None = None) -> str: - """Build the wire text a remote device would push for a given budget edit.""" - record: dict[str, object] = {"v": 2, "b": kcal, "t": t} - if weight_kg is not None: - record["w"] = weight_kg - log = budget_to_log(record) - return json.dumps({rid: rec.to_dict() for rid, rec in log.items()}, indent=2) - - -def _nutrition(kcal: float = 200.0) -> Nutrition: - return Nutrition( - kcal=kcal, - protein_g=10.0, - carbs_g=20.0, - fat_g=5.0, - grams=100.0, - source="manual", - ) - - -def _write_token(token: str = "fake-token") -> None: - _sync.SYNC_TOKEN_FILE.parent.mkdir(parents=True, exist_ok=True) - _sync.SYNC_TOKEN_FILE.write_text(token) - - -def _mock_client( - *, - devices: tuple[str, ...] = (), - files: dict[str, str] | None = None, -) -> MagicMock: - """Build a mock ``GitHubSyncClient`` covering the methods sync calls.""" - client = MagicMock() - client.list_directory.return_value = list(devices) - resolved_files = files or {} - client.get_file_text.side_effect = resolved_files.get - # The real GitHubSyncClient has no bulk-map read, and a MagicMock would - # otherwise auto-create one returning a Mock -- which then fails to - # serialise. Deleting it also exercises the documented degrade path: - # no revision map means every peer is fetched, exactly as before. - del client.get_string_map - return client +from diet_guard.tests._sync_fakes import _mock_client, _nutrition, _write_token class TestReadToken: def test_missing_token_file_raises_sync_error(self) -> None: with pytest.raises(_sync.SyncError): - _sync._read_token() + _sync_client._read_token() def test_empty_token_file_raises_sync_error(self) -> None: _write_token(" ") with pytest.raises(_sync.SyncError): - _sync._read_token() + _sync_client._read_token() def test_present_token_is_read_and_stripped(self) -> None: _write_token(" abc123 \n") - assert _sync._read_token() == "abc123" + assert _sync_client._read_token() == "abc123" class TestRunSync: def test_raises_before_touching_github_when_no_token(self) -> None: with ( - patch.object(_sync, "GitHubSyncClient") as client_cls, + patch.object(_sync_client, "GitHubSyncClient") as client_cls, pytest.raises(_sync.SyncError), ): _sync.run_sync() @@ -101,7 +47,7 @@ class TestRunSync: _write_token() log_meal("oatmeal", _nutrition(), slot=8) client = _mock_client(devices=()) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): merged = _sync.run_sync() assert sum(len(entries) for entries in merged.values()) == 1 @@ -131,7 +77,7 @@ class TestRunSync: devices=("pc", "phone"), files={"diet-guard-sync/devices/phone/food_log.json": "{}"}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() # Every pull -- food log, budget, curated bank -- skips "pc" (this # device) and only ever reads "phone"'s files. @@ -146,7 +92,7 @@ class TestRunSync: def test_skips_a_device_with_no_pushed_file_yet(self) -> None: _write_token() client = _mock_client(devices=("phone",), files={}) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): merged = _sync.run_sync() assert merged == {} @@ -156,7 +102,7 @@ class TestRunSync: devices=("phone",), files={"diet-guard-sync/devices/phone/food_log.json": "[]"}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): merged = _sync.run_sync() assert merged == {} @@ -169,7 +115,7 @@ class TestRunSync: devices=("phone",), files={"diet-guard-sync/devices/phone/food_log.json": "{not valid json"}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): merged = _sync.run_sync() assert merged == {} @@ -196,7 +142,7 @@ class TestRunSync: devices=("phone",), files={"diet-guard-sync/devices/phone/food_log.json": remote_log_json}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): merged = _sync.run_sync() descs = {entry["desc"] for entries in merged.values() for entry in entries} assert "phone meal" in descs @@ -231,7 +177,7 @@ class TestRunSync: devices=("phone",), files={"diet-guard-sync/devices/phone/food_log.json": remote_log_json}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() reloaded = load_log() @@ -242,241 +188,6 @@ class TestRunSync: _write_token() log_meal("oatmeal", _nutrition(), slot=8) client = _mock_client(devices=()) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() assert lookup_food("oatmeal") is not None - - -class TestSyncBudget: - """The daily budget's last-writer-wins sync, folded into run_sync().""" - - def test_pushes_local_budget_when_no_other_devices_have_synced(self) -> None: - _write_token() - write_budget(2000) - client = _mock_client(devices=()) - with patch.object(_sync, "GitHubSyncClient", return_value=client): - _sync.run_sync() - pushed_paths = {call.args[0] for call in client.put_file_text.call_args_list} - assert f"diet-guard-sync/devices/{device_id()}/budget.json" in pushed_paths - - def test_nothing_pushed_when_no_budget_ever_set(self) -> None: - """An uninitialized device contributes nothing -- no push, no crash.""" - _write_token() - client = _mock_client(devices=()) - with patch.object(_sync, "GitHubSyncClient", return_value=client): - _sync.run_sync() - pushed_paths = {call.args[0] for call in client.put_file_text.call_args_list} - assert f"diet-guard-sync/devices/{device_id()}/budget.json" not in pushed_paths - - def test_remote_only_budget_is_adopted_locally(self) -> None: - """Only the phone has ever set a budget -- the PC adopts it.""" - _write_token() - remote_json = _remote_budget_json(kcal=1800, t="2026-01-01T09:00:00") - client = _mock_client( - devices=("phone",), - files={"diet-guard-sync/devices/phone/budget.json": remote_json}, - ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): - _sync.run_sync() - assert daily_budget() == 1800 - - def test_local_edit_later_than_remote_wins(self) -> None: - """A fresh local write beats a much older remote edit.""" - _write_token() - write_budget(1500) # stamped with "now" - remote_json = _remote_budget_json(kcal=1800, t="2020-01-01T09:00:00") - client = _mock_client( - devices=("phone",), - files={"diet-guard-sync/devices/phone/budget.json": remote_json}, - ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): - _sync.run_sync() - assert daily_budget() == 1500 - - def test_remote_edit_later_than_local_wins(self) -> None: - """A remote edit far in the future beats a stale local write. - - Confirms this is genuinely edit-time (not sync-time or push-order) - LWW: whichever side has the later ``t`` wins regardless of which - device happens to run its sync tick first. - """ - _write_token() - write_budget(1500) # stamped with "now" - remote_json = _remote_budget_json(kcal=1800, t="2999-01-01T09:00:00") - client = _mock_client( - devices=("phone",), - files={"diet-guard-sync/devices/phone/budget.json": remote_json}, - ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): - _sync.run_sync() - assert daily_budget() == 1800 - - def test_a_remote_edit_does_not_delete_the_local_body_weight(self) -> None: - """The w-loss bug: ``w`` is PC-local and must survive a phone edit. - - ``w`` never travels (budget_to_log strips it), so before this fix a - winning remote record replaced the whole stored map and silently took - the weight -- and the protein target -- with it. - """ - _write_token() - write_budget(1500, weight_kg=78.5) - remote_json = _remote_budget_json(kcal=1800, t="2999-01-01T09:00:00") - client = _mock_client( - devices=("phone",), - files={"diet-guard-sync/devices/phone/budget.json": remote_json}, - ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): - _sync.run_sync() - assert daily_budget() == 1800 - assert budget_weight() == 78.5 - - def test_budget_history_survives_a_round_trip(self) -> None: - """The history is written back locally after every merge.""" - _write_token() - write_budget(2000) - client = _mock_client(devices=()) - with patch.object(_sync, "GitHubSyncClient", return_value=client): - _sync.run_sync() - assert load_entries()[-1].kcal == 2000 - - def test_malformed_remote_budget_is_skipped(self) -> None: - """A corrupt remote budget.json is skipped, not a crash.""" - _write_token() - write_budget(2000) - client = _mock_client( - devices=("phone",), - files={"diet-guard-sync/devices/phone/budget.json": "{not valid json"}, - ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): - _sync.run_sync() - assert daily_budget() == 2000 - - -class TestPullSharedLog: - """The fail-closed wrapper the gate and the lock-screen button share.""" - - def test_returns_none_on_success(self) -> None: - """A clean pull returns None (no reason to report).""" - with patch.object(_sync, "run_sync") as run_sync: - assert _sync.pull_shared_log() is None - run_sync.assert_called_once_with() - - def test_returns_reason_on_expected_failure(self) -> None: - """A real sync failure (here a network error) becomes a reason string.""" - with patch.object(_sync, "run_sync", side_effect=_sync.GitHubSyncError("boom")): - reason = _sync.pull_shared_log() - assert reason is not None - assert "boom" in reason - - @pytest.mark.parametrize( - "error", - [ - FirebaseAuthError("token refresh rejected"), - FirebaseSyncError("RTDB unavailable"), - ], - ) - def test_firebase_failure_fails_closed(self, error: Exception) -> None: - """A Firebase failure must return a reason, never raise. - - Regression guard: ``FirebaseAuthError``/``FirebaseSyncError`` are - *siblings* of ``GitHubSyncError`` under ``RemoteSyncError``, not - subclasses. Catching only the GitHub type let every per-request - failure of the *primary* backend escape, so the gate's "Fetch from - sync" button raised a traceback instead of reporting a reason and - leaving the lock up -- on the exact path this helper exists to - fail closed. - """ - with patch.object(_sync, "run_sync", side_effect=error): - reason = _sync.pull_shared_log() - - assert reason is not None - assert str(error) in reason - - def test_unexpected_error_is_not_swallowed(self) -> None: - """A bug outside the known failure surface surfaces, not hidden.""" - with ( - patch.object(_sync, "run_sync", side_effect=KeyError("bug")), - pytest.raises(KeyError), - ): - _sync.pull_shared_log() - - -class TestRemoteRevisions: - """Reading the peers' revision map, which gates the big downloads.""" - - def test_is_empty_without_a_bulk_map_read(self) -> None: - """GitHub has none; correctness must not depend on the optimisation.""" - client = MagicMock() - del client.get_string_map - - assert _sync._remote_revisions(client) == {} - - def test_returns_the_published_revisions(self) -> None: - client = MagicMock() - client.get_string_map.return_value = {"phone": "abc"} - - assert _sync._remote_revisions(client) == {"phone": "abc"} - - def test_an_unreadable_map_degrades_to_fetching_everything(self) -> None: - """Not worth failing a sync over; every peer is simply fetched.""" - client = MagicMock() - client.get_string_map.side_effect = GitHubSyncError("boom") - - assert _sync._remote_revisions(client) == {} - - -class TestPullSkipsUnchangedPeers: - """The single largest traffic saving in the fleet.""" - - def test_skips_a_peer_whose_revision_is_unchanged(self) -> None: - client = _mock_client(devices=("phone",)) - state = SyncState(pushed_rev=None, peer_revs={"phone": "rev-1"}) - seen: dict[str, str] = {} - - logs = _sync._pull_remote_logs( - client, {"phone": "rev-1"}, state, seen, device_identity() - ) - - assert logs == [] - assert seen == {"phone": "rev-1"} - client.get_file_text.assert_not_called() - - def test_downloads_a_peer_whose_revision_moved(self) -> None: - text = json.dumps({}) - client = _mock_client( - devices=("phone",), - files={"diet-guard-sync/devices/phone/food_log.json": text}, - ) - state = SyncState(pushed_rev=None, peer_revs={"phone": "rev-1"}) - seen: dict[str, str] = {} - - _sync._pull_remote_logs( - client, {"phone": "rev-2"}, state, seen, device_identity() - ) - - client.get_file_text.assert_called_once() - assert seen == {"phone": "rev-2"} - - -class TestNoOpPushSuppression: - """88% of the old GitHub history was byte-identical no-op pushes.""" - - def test_a_second_unchanged_sync_pushes_no_log(self) -> None: - """The saving the free-tier budget depends on, at 96 ticks a day.""" - _write_token() - client = _mock_client() - - with patch.object(_sync, "GitHubSyncClient", return_value=client): - _sync.run_sync() - pushed_first = [ - call.args[0] for call in client.put_file_text.call_args_list - ] - client.put_file_text.reset_mock() - _sync.run_sync() - - pushed_second = [call.args[0] for call in client.put_file_text.call_args_list] - assert f"diet-guard-sync/devices/{device_id()}/food_log.json" in pushed_first - assert ( - f"diet-guard-sync/devices/{device_id()}/food_log.json" not in pushed_second - ) - assert f"diet-guard-sync/revs/{device_id()}" not in pushed_second diff --git a/diet_guard/tests/test_sync_backend.py b/diet_guard/tests/test_sync_backend.py index 7c7a6c9..bde8241 100644 --- a/diet_guard/tests/test_sync_backend.py +++ b/diet_guard/tests/test_sync_backend.py @@ -12,7 +12,7 @@ from pathlib import Path from crdt_sync import ConfigError import pytest -from diet_guard import _sync +from diet_guard import _sync, _sync_client class TestRemoteClient: @@ -22,10 +22,12 @@ class TestRemoteClient: self, monkeypatch: pytest.MonkeyPatch ) -> None: """An unconfigured machine must not reach the network at all.""" - monkeypatch.setattr(_sync, "CONFIG_FILE", Path("/nonexistent/firebase.json")) + monkeypatch.setattr( + _sync_client, "CONFIG_FILE", Path("/nonexistent/firebase.json") + ) github = object() - assert _sync._remote_client(github) is github + assert _sync_client._remote_client(github) is github def test_mirrors_to_github_when_configured( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -33,13 +35,13 @@ class TestRemoteClient: """Configured: Firebase is primary, GitHub keeps receiving writes.""" config = tmp_path / "firebase.json" config.write_text("{}", encoding="utf-8") - monkeypatch.setattr(_sync, "CONFIG_FILE", config) + monkeypatch.setattr(_sync_client, "CONFIG_FILE", config) monkeypatch.setattr( - _sync, "mirror_client_for", lambda _app, client: ("mirror", client) + _sync_client, "mirror_client_for", lambda _app, client: ("mirror", client) ) github = object() - assert _sync._remote_client(github) == ("mirror", github) + assert _sync_client._remote_client(github) == ("mirror", github) def test_falls_back_when_firebase_is_unusable( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -47,16 +49,16 @@ class TestRemoteClient: """A broken Firebase must degrade to GitHub, never fail the tick.""" config = tmp_path / "firebase.json" config.write_text("{}", encoding="utf-8") - monkeypatch.setattr(_sync, "CONFIG_FILE", config) + monkeypatch.setattr(_sync_client, "CONFIG_FILE", config) def _boom(*_args: object, **_kwargs: object) -> None: message = "no password" raise ConfigError(message) - monkeypatch.setattr(_sync, "mirror_client_for", _boom) + monkeypatch.setattr(_sync_client, "mirror_client_for", _boom) github = object() - assert _sync._remote_client(github) is github + assert _sync_client._remote_client(github) is github class TestClientForRun: @@ -73,11 +75,15 @@ class TestClientForRun: """ config = tmp_path / "firebase.json" config.write_text("{}", encoding="utf-8") - monkeypatch.setattr(_sync, "CONFIG_FILE", config) - monkeypatch.setattr(_sync, "SYNC_TOKEN_FILE", tmp_path / "absent_sync_token") - monkeypatch.setattr(_sync, "firebase_client_for", lambda app: ("firebase", app)) + monkeypatch.setattr(_sync_client, "CONFIG_FILE", config) + monkeypatch.setattr( + _sync_client, "SYNC_TOKEN_FILE", tmp_path / "absent_sync_token" + ) + monkeypatch.setattr( + _sync_client, "firebase_client_for", lambda app: ("firebase", app) + ) - assert _sync._client_for_run() == ("firebase", "diet_guard") + assert _sync_client._client_for_run() == ("firebase", "diet_guard") def test_unusable_firebase_config_becomes_a_sync_error( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -94,24 +100,28 @@ class TestClientForRun: """ config = tmp_path / "firebase.json" config.write_text("{}", encoding="utf-8") - monkeypatch.setattr(_sync, "CONFIG_FILE", config) - monkeypatch.setattr(_sync, "SYNC_TOKEN_FILE", tmp_path / "absent_token") + monkeypatch.setattr(_sync_client, "CONFIG_FILE", config) + monkeypatch.setattr(_sync_client, "SYNC_TOKEN_FILE", tmp_path / "absent_token") def _boom(*_args: object, **_kwargs: object) -> None: message = "no password" raise ConfigError(message) - monkeypatch.setattr(_sync, "firebase_client_for", _boom) + monkeypatch.setattr(_sync_client, "firebase_client_for", _boom) with pytest.raises(_sync.SyncError): - _sync._client_for_run() + _sync_client._client_for_run() def test_still_fails_when_neither_backend_is_configured( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """No PAT *and* no Firebase is the genuine "nothing is set up" case.""" - monkeypatch.setattr(_sync, "CONFIG_FILE", tmp_path / "absent_firebase.json") - monkeypatch.setattr(_sync, "SYNC_TOKEN_FILE", tmp_path / "absent_sync_token") + monkeypatch.setattr( + _sync_client, "CONFIG_FILE", tmp_path / "absent_firebase.json" + ) + monkeypatch.setattr( + _sync_client, "SYNC_TOKEN_FILE", tmp_path / "absent_sync_token" + ) with pytest.raises(_sync.SyncError): - _sync._client_for_run() + _sync_client._client_for_run() diff --git a/diet_guard/tests/test_sync_banks.py b/diet_guard/tests/test_sync_banks.py index 319c74f..478478b 100644 --- a/diet_guard/tests/test_sync_banks.py +++ b/diet_guard/tests/test_sync_banks.py @@ -11,14 +11,14 @@ from __future__ import annotations import json from unittest.mock import patch -from diet_guard import _sync +from diet_guard import _sync, _sync_client from diet_guard._device import device_id from diet_guard._estimator import Nutrition from diet_guard._foodbank import lookup_food, read_food_bank from diet_guard._foodbank_manual import add_manual_entry, read_manual_bank from diet_guard._state import log_meal -from diet_guard._sync_merge import food_bank_to_log, manual_bank_to_log -from diet_guard.tests.test_sync import _mock_client, _write_token +from diet_guard.sync_merge import food_bank_to_log, manual_bank_to_log +from diet_guard.tests._sync_fakes import _mock_client, _write_token class TestSyncFoodBank: @@ -31,7 +31,7 @@ class TestSyncFoodBank: # behind it is (correctly) discarded before the push. log_meal("apple", Nutrition(95, 0.5, 25, 0.3, 180, "manual"), 8) client = _mock_client(devices=()) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() pushed = [call.args[0] for call in client.put_file_text.call_args_list] assert f"diet-guard-sync/devices/{device_id()}/food_bank.json" in pushed @@ -65,7 +65,7 @@ class TestSyncFoodBank: devices=("phone",), files={"diet-guard-sync/devices/phone/food_log.json": remote_log_json}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() found = lookup_food("Skyr") assert found is not None @@ -93,7 +93,7 @@ class TestSyncFoodBank: devices=("phone",), files={"diet-guard-sync/devices/phone/food_bank.json": remote}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() assert lookup_food("Skyr") is None @@ -121,7 +121,7 @@ class TestSyncFoodBank: devices=("phone",), files={"diet-guard-sync/devices/phone/food_bank.json": remote}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() assert read_food_bank()["apple"]["count"] == 99 @@ -132,14 +132,14 @@ class TestSyncFoodBank: devices=("phone",), files={"diet-guard-sync/devices/phone/food_bank.json": "{not json"}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() assert lookup_food("apple") is not None def test_nothing_pushed_when_no_device_has_a_bank(self) -> None: _write_token() client = _mock_client(devices=()) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() pushed = [call.args[0] for call in client.put_file_text.call_args_list] assert f"diet-guard-sync/devices/{device_id()}/food_bank.json" not in pushed @@ -151,7 +151,7 @@ class TestSyncManualBank: def test_nothing_pushed_when_no_device_has_curated_anything(self) -> None: _write_token() client = _mock_client(devices=()) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() pushed = [call.args[0] for call in client.put_file_text.call_args_list] assert not any("food_bank_manual" in path for path in pushed) @@ -160,7 +160,7 @@ class TestSyncManualBank: _write_token() add_manual_entry("Skyr", {"desc": "Skyr", "kcal": 120.0, "count": 0}) client = _mock_client(devices=()) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() pushed = [call.args[0] for call in client.put_file_text.call_args_list] assert f"diet-guard-sync/devices/{device_id()}/food_bank_manual.json" in pushed @@ -192,7 +192,7 @@ class TestSyncManualBank: devices=("phone",), files={"diet-guard-sync/devices/phone/food_bank_manual.json": remote}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() assert "skyr" in read_manual_bank() found = lookup_food("Skyr") @@ -221,7 +221,7 @@ class TestSyncManualBank: devices=("phone",), files={"diet-guard-sync/devices/phone/food_bank_manual.json": remote}, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() assert read_manual_bank()["skyr"]["kcal"] == 999.0 @@ -234,7 +234,7 @@ class TestSyncManualBank: "diet-guard-sync/devices/phone/food_bank_manual.json": "{not json", }, ) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() assert read_manual_bank()["skyr"]["kcal"] == 120.0 @@ -242,6 +242,6 @@ class TestSyncManualBank: _write_token() add_manual_entry("Skyr", {"desc": "Skyr", "kcal": 120.0, "count": 0}) client = _mock_client(devices=("phone",)) - with patch.object(_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): _sync.run_sync() assert read_manual_bank()["skyr"]["kcal"] == 120.0 diff --git a/diet_guard/tests/test_sync_budget.py b/diet_guard/tests/test_sync_budget.py new file mode 100644 index 0000000..6c21d0a --- /dev/null +++ b/diet_guard/tests/test_sync_budget.py @@ -0,0 +1,125 @@ +"""Tests for the budget's half of the sync tick. + +Split out of ``test_sync.py`` to keep both files under the repo's 250-line +cap, along the same seam as the module they cover +(``diet_guard.sync_merge._budget``). +""" + +from __future__ import annotations + +from unittest.mock import patch + +from diet_guard import _sync, _sync_client +from diet_guard._budget import budget_weight, daily_budget, write_budget +from diet_guard._budget_history import load_entries +from diet_guard._device import device_id +from diet_guard.tests._sync_fakes import ( + _mock_client, + _remote_budget_json, + _write_token, +) + + +class TestSyncBudget: + """The daily budget's last-writer-wins sync, folded into run_sync().""" + + def test_pushes_local_budget_when_no_other_devices_have_synced(self) -> None: + _write_token() + write_budget(2000) + client = _mock_client(devices=()) + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): + _sync.run_sync() + pushed_paths = {call.args[0] for call in client.put_file_text.call_args_list} + assert f"diet-guard-sync/devices/{device_id()}/budget.json" in pushed_paths + + def test_nothing_pushed_when_no_budget_ever_set(self) -> None: + """An uninitialized device contributes nothing -- no push, no crash.""" + _write_token() + client = _mock_client(devices=()) + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): + _sync.run_sync() + pushed_paths = {call.args[0] for call in client.put_file_text.call_args_list} + assert f"diet-guard-sync/devices/{device_id()}/budget.json" not in pushed_paths + + def test_remote_only_budget_is_adopted_locally(self) -> None: + """Only the phone has ever set a budget -- the PC adopts it.""" + _write_token() + remote_json = _remote_budget_json(kcal=1800, t="2026-01-01T09:00:00") + client = _mock_client( + devices=("phone",), + files={"diet-guard-sync/devices/phone/budget.json": remote_json}, + ) + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): + _sync.run_sync() + assert daily_budget() == 1800 + + def test_local_edit_later_than_remote_wins(self) -> None: + """A fresh local write beats a much older remote edit.""" + _write_token() + write_budget(1500) # stamped with "now" + remote_json = _remote_budget_json(kcal=1800, t="2020-01-01T09:00:00") + client = _mock_client( + devices=("phone",), + files={"diet-guard-sync/devices/phone/budget.json": remote_json}, + ) + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): + _sync.run_sync() + assert daily_budget() == 1500 + + def test_remote_edit_later_than_local_wins(self) -> None: + """A remote edit far in the future beats a stale local write. + + Confirms this is genuinely edit-time (not sync-time or push-order) + LWW: whichever side has the later ``t`` wins regardless of which + device happens to run its sync tick first. + """ + _write_token() + write_budget(1500) # stamped with "now" + remote_json = _remote_budget_json(kcal=1800, t="2999-01-01T09:00:00") + client = _mock_client( + devices=("phone",), + files={"diet-guard-sync/devices/phone/budget.json": remote_json}, + ) + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): + _sync.run_sync() + assert daily_budget() == 1800 + + def test_a_remote_edit_does_not_delete_the_local_body_weight(self) -> None: + """The w-loss bug: ``w`` is PC-local and must survive a phone edit. + + ``w`` never travels (budget_to_log strips it), so before this fix a + winning remote record replaced the whole stored map and silently took + the weight -- and the protein target -- with it. + """ + _write_token() + write_budget(1500, weight_kg=78.5) + remote_json = _remote_budget_json(kcal=1800, t="2999-01-01T09:00:00") + client = _mock_client( + devices=("phone",), + files={"diet-guard-sync/devices/phone/budget.json": remote_json}, + ) + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): + _sync.run_sync() + assert daily_budget() == 1800 + assert budget_weight() == 78.5 + + def test_budget_history_survives_a_round_trip(self) -> None: + """The history is written back locally after every merge.""" + _write_token() + write_budget(2000) + client = _mock_client(devices=()) + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): + _sync.run_sync() + assert load_entries()[-1].kcal == 2000 + + def test_malformed_remote_budget_is_skipped(self) -> None: + """A corrupt remote budget.json is skipped, not a crash.""" + _write_token() + write_budget(2000) + client = _mock_client( + devices=("phone",), + files={"diet-guard-sync/devices/phone/budget.json": "{not valid json"}, + ) + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): + _sync.run_sync() + assert daily_budget() == 2000 diff --git a/diet_guard/tests/test_sync_merge.py b/diet_guard/tests/test_sync_merge.py index d1af867..be2426b 100644 --- a/diet_guard/tests/test_sync_merge.py +++ b/diet_guard/tests/test_sync_merge.py @@ -11,21 +11,15 @@ from __future__ import annotations import json -from crdt_sync import Record, merge_logs +from crdt_sync import merge_logs import pytest -from diet_guard._budget_history import BudgetEntry -from diet_guard._sync_merge import ( - _budget_hlc, +from diet_guard.sync_merge import ( _entry_hlc, _legacy_entry_id, - budget_to_log, daylog_to_log, entry_to_record, - log_to_budget, log_to_daylog, - log_to_history, - parse_remote_budget, parse_remote_log, record_to_entry, ) @@ -187,13 +181,13 @@ class TestParseRemoteLog: def test_parses_new_format_wire_content(self) -> None: entry = _entry(id="x") pushed = {"x": entry_to_record(entry).to_dict()} - log = parse_remote_log(_dumps(pushed)) + log = parse_remote_log(json.dumps(pushed)) assert log["x"].id == "x" def test_parses_old_daylog_format_for_backward_compatibility(self) -> None: entry = _entry(id="x") old_format = {"2026-06-22": [entry]} - log = parse_remote_log(_dumps(old_format)) + log = parse_remote_log(json.dumps(old_format)) assert log["x"].id == "x" def test_empty_object_parses_as_empty_log(self) -> None: @@ -205,210 +199,12 @@ class TestParseRemoteLog: def test_old_format_day_not_a_list_raises_type_error(self) -> None: with pytest.raises(TypeError): - parse_remote_log(_dumps({"2026-06-22": "not-a-list"})) + parse_remote_log(json.dumps({"2026-06-22": "not-a-list"})) def test_old_format_entry_not_an_object_raises_type_error(self) -> None: with pytest.raises(TypeError): - parse_remote_log(_dumps({"2026-06-22": ["not-a-dict"]})) + parse_remote_log(json.dumps({"2026-06-22": ["not-a-dict"]})) def test_invalid_json_raises(self) -> None: with pytest.raises(json.JSONDecodeError): parse_remote_log("not json{{{") - - -def _budget_record(**overrides: object) -> dict[str, object]: - """Build a minimal valid raw budget record, overriding what a test needs.""" - record: dict[str, object] = { - "v": 2, - "b": 2000, - "t": "2026-06-22T08:00:00+02:00", - } - record.update(overrides) - return record - - -_HIST_SEED = BudgetEntry("1970-01-01", 2200, "2026-07-13T21:15:09+02:00") -_HIST_CUT = BudgetEntry("2026-07-26", 2000, "2026-07-26T10:00:00+02:00") - - -class TestBudgetHlc: - def test_same_record_always_yields_the_same_hlc(self) -> None: - record = _budget_record() - assert _budget_hlc(record) == _budget_hlc(dict(record)) - - def test_malformed_t_still_yields_a_valid_hlc(self) -> None: - record = _budget_record(t="not-a-timestamp") - assert _budget_hlc(record).wall_time_ms == 0 - - def test_a_later_t_yields_a_greater_hlc(self) -> None: - earlier = _budget_record(t="2020-01-01T00:00:00+00:00") - later = _budget_record(t="2030-01-01T00:00:00+00:00") - assert _budget_hlc(later) > _budget_hlc(earlier) - - -class TestBudgetLogRoundTrip: - def test_none_record_yields_an_empty_log(self) -> None: - assert budget_to_log(None) == {} - - def test_round_trip_preserves_the_budget(self) -> None: - record = _budget_record() - round_tripped = log_to_budget(budget_to_log(record)) - assert round_tripped is not None - assert round_tripped["b"] == 2000 - - def test_weight_travels_as_its_own_field(self) -> None: - """``w`` is shared state, carried as a field of its own. - - Inside ``value`` it was collateral damage of whole-map LWW: the phone - rebuilds that map without ``w``, so any phone budget edit silently - deleted the stored weight and the protein target with it. - """ - log = budget_to_log(_budget_record(w=80.0)) - assert "w" not in log["budget"].fields["value"][0] - assert log["budget"].fields["weight"][0] == 80.0 - - def test_weight_round_trips(self) -> None: - round_tripped = log_to_budget(budget_to_log(_budget_record(w=80.0))) - assert round_tripped is not None - assert round_tripped["w"] == 80.0 - - def test_no_weight_round_trips_to_no_weight(self) -> None: - round_tripped = log_to_budget(budget_to_log(_budget_record())) - assert round_tripped is not None - assert "w" not in round_tripped - - def test_a_non_numeric_weight_is_not_emitted(self) -> None: - assert "weight" not in budget_to_log(_budget_record(w="heavy"))["budget"].fields - - def test_a_boolean_weight_is_not_emitted(self) -> None: - assert "weight" not in budget_to_log(_budget_record(w=True))["budget"].fields - - def test_a_non_numeric_weight_field_is_ignored_on_read(self) -> None: - log = { - "budget": Record( - id="budget", - fields={"weight": ("heavy", _budget_hlc({}))}, - ), - } - assert "w" not in (log_to_budget(log) or {}) - - def test_a_weightless_peer_cannot_delete_the_weight(self) -> None: - """The whole point: the phone never sets ``w`` and must not drop it.""" - ours = budget_to_log(_budget_record(w=80.0)) - phone = budget_to_log( - {"v": 2, "b": 1800, "t": "2099-01-01T00:00:00+00:00"}, - ) - merged = log_to_budget(merge_logs(ours, phone)) - assert merged is not None - assert merged["b"] == 1800 - assert merged["w"] == 80.0 - - def test_round_tripped_t_reflects_the_winning_hlc(self) -> None: - record = _budget_record(t="2026-06-22T08:00:00+02:00") - round_tripped = log_to_budget(budget_to_log(record)) - assert round_tripped is not None - assert round_tripped["t"] != "" - - def test_empty_log_has_no_budget(self) -> None: - assert log_to_budget({}) is None - - def test_record_with_no_value_field_has_no_hlc_in_result(self) -> None: - """A record present but missing the "value" field is a defensive - edge case (should not occur from budget_to_log itself) -- the - result still comes back without crashing, and with no ``t``. - """ - log = {"budget": Record(id="budget", fields={})} - round_tripped = log_to_budget(log) - assert round_tripped == {} - - -class TestBudgetHistoryFields: - """The effective-from history riding along on the budget record.""" - - def test_history_becomes_one_field_per_date(self) -> None: - log = budget_to_log(_budget_record(), (_HIST_SEED, _HIST_CUT)) - fields = log["budget"].fields - assert "value" in fields - assert fields["hist:1970-01-01"][0] == 2200 - assert fields["hist:2026-07-26"][0] == 2000 - - def test_round_trips_back_to_entries(self) -> None: - entries = log_to_history( - budget_to_log(_budget_record(), (_HIST_SEED, _HIST_CUT)) - ) - assert [e.effective_from for e in entries] == ["1970-01-01", "2026-07-26"] - assert [e.kcal for e in entries] == [2200, 2000] - - def test_no_history_round_trips_to_nothing(self) -> None: - assert log_to_history(budget_to_log(_budget_record())) == () - - def test_empty_log_has_no_history(self) -> None: - assert log_to_history({}) == () - - def test_non_history_fields_are_ignored(self) -> None: - log = { - "budget": Record( - id="budget", fields={"value": ({"b": 2000}, _budget_hlc({}))} - ) - } - assert log_to_history(log) == () - - def test_non_int_history_value_is_skipped(self) -> None: - log = { - "budget": Record( - id="budget", - fields={"hist:2026-07-26": ("nonsense", _budget_hlc({}))}, - ), - } - assert log_to_history(log) == () - - def test_unparsable_edit_time_still_yields_a_field(self) -> None: - broken = BudgetEntry("2026-07-26", 2000, "not a timestamp") - log = budget_to_log(_budget_record(), (broken,)) - assert "hist:2026-07-26" in log["budget"].fields - - def test_history_survives_a_merge_with_a_history_free_peer(self) -> None: - """The rollout guarantee: an old device cannot clobber the history. - - merge_record is per-field LWW over the *union* of field names, so a - peer that only pushes ``value`` leaves ``hist:*`` untouched -- which - is why this needed no coordinated release. - """ - ours = budget_to_log(_budget_record(), (_HIST_SEED, _HIST_CUT)) - legacy = { - "budget": Record( - id="budget", - fields={ - "value": ( - {"v": 2, "b": 2200}, - _budget_hlc({"t": "2099-01-01T00:00:00+00:00"}), - ) - }, - ), - } - merged = merge_logs(ours, legacy) - assert len(log_to_history(merged)) == 2 - - -class TestParseRemoteBudget: - def test_parses_pushed_budget_wire_content(self) -> None: - record = _budget_record() - pushed = budget_to_log(record) - wire = json.dumps({rid: rec.to_dict() for rid, rec in pushed.items()}) - log = parse_remote_budget(wire) - assert log["budget"].id == "budget" - - def test_empty_object_parses_as_empty_log(self) -> None: - assert parse_remote_budget("{}") == {} - - def test_non_object_top_level_raises_type_error(self) -> None: - with pytest.raises(TypeError): - parse_remote_budget("[1, 2, 3]") - - def test_invalid_json_raises(self) -> None: - with pytest.raises(json.JSONDecodeError): - parse_remote_budget("not json{{{") - - -def _dumps(data: object) -> str: - return json.dumps(data) diff --git a/diet_guard/tests/test_sync_merge_banks.py b/diet_guard/tests/test_sync_merge_banks.py index 2ad12f7..6e4d763 100644 --- a/diet_guard/tests/test_sync_merge_banks.py +++ b/diet_guard/tests/test_sync_merge_banks.py @@ -12,7 +12,7 @@ import json from crdt_sync import Record, merge_logs import pytest -from diet_guard._sync_merge import ( +from diet_guard.sync_merge import ( _budget_hlc, food_bank_to_log, log_to_food_bank, diff --git a/diet_guard/tests/test_sync_merge_budget.py b/diet_guard/tests/test_sync_merge_budget.py new file mode 100644 index 0000000..af02c3d --- /dev/null +++ b/diet_guard/tests/test_sync_merge_budget.py @@ -0,0 +1,216 @@ +"""Tests for the budget and budget-history adapters. + +Split out of ``test_sync_merge.py`` to keep both files under the repo's +250-line cap, along the same seam as the module they cover +(``diet_guard.sync_merge._budget``). +""" + +from __future__ import annotations + +import json + +from crdt_sync import Record, merge_logs +import pytest + +from diet_guard._budget_history import BudgetEntry +from diet_guard.sync_merge import ( + _budget_hlc, + budget_to_log, + log_to_budget, + log_to_history, + parse_remote_budget, +) + + +def _budget_record(**overrides: object) -> dict[str, object]: + """Build a minimal valid raw budget record, overriding what a test needs.""" + record: dict[str, object] = { + "v": 2, + "b": 2000, + "t": "2026-06-22T08:00:00+02:00", + } + record.update(overrides) + return record + + +_HIST_SEED = BudgetEntry("1970-01-01", 2200, "2026-07-13T21:15:09+02:00") +_HIST_CUT = BudgetEntry("2026-07-26", 2000, "2026-07-26T10:00:00+02:00") + + +class TestBudgetHlc: + def test_same_record_always_yields_the_same_hlc(self) -> None: + record = _budget_record() + assert _budget_hlc(record) == _budget_hlc(dict(record)) + + def test_malformed_t_still_yields_a_valid_hlc(self) -> None: + record = _budget_record(t="not-a-timestamp") + assert _budget_hlc(record).wall_time_ms == 0 + + def test_a_later_t_yields_a_greater_hlc(self) -> None: + earlier = _budget_record(t="2020-01-01T00:00:00+00:00") + later = _budget_record(t="2030-01-01T00:00:00+00:00") + assert _budget_hlc(later) > _budget_hlc(earlier) + + +class TestBudgetLogRoundTrip: + def test_none_record_yields_an_empty_log(self) -> None: + assert budget_to_log(None) == {} + + def test_round_trip_preserves_the_budget(self) -> None: + record = _budget_record() + round_tripped = log_to_budget(budget_to_log(record)) + assert round_tripped is not None + assert round_tripped["b"] == 2000 + + def test_weight_travels_as_its_own_field(self) -> None: + """``w`` is shared state, carried as a field of its own. + + Inside ``value`` it was collateral damage of whole-map LWW: the phone + rebuilds that map without ``w``, so any phone budget edit silently + deleted the stored weight and the protein target with it. + """ + log = budget_to_log(_budget_record(w=80.0)) + assert "w" not in log["budget"].fields["value"][0] + assert log["budget"].fields["weight"][0] == 80.0 + + def test_weight_round_trips(self) -> None: + round_tripped = log_to_budget(budget_to_log(_budget_record(w=80.0))) + assert round_tripped is not None + assert round_tripped["w"] == 80.0 + + def test_no_weight_round_trips_to_no_weight(self) -> None: + round_tripped = log_to_budget(budget_to_log(_budget_record())) + assert round_tripped is not None + assert "w" not in round_tripped + + def test_a_non_numeric_weight_is_not_emitted(self) -> None: + assert "weight" not in budget_to_log(_budget_record(w="heavy"))["budget"].fields + + def test_a_boolean_weight_is_not_emitted(self) -> None: + assert "weight" not in budget_to_log(_budget_record(w=True))["budget"].fields + + def test_a_non_numeric_weight_field_is_ignored_on_read(self) -> None: + log = { + "budget": Record( + id="budget", + fields={"weight": ("heavy", _budget_hlc({}))}, + ), + } + assert "w" not in (log_to_budget(log) or {}) + + def test_a_weightless_peer_cannot_delete_the_weight(self) -> None: + """The whole point: the phone never sets ``w`` and must not drop it.""" + ours = budget_to_log(_budget_record(w=80.0)) + phone = budget_to_log( + {"v": 2, "b": 1800, "t": "2099-01-01T00:00:00+00:00"}, + ) + merged = log_to_budget(merge_logs(ours, phone)) + assert merged is not None + assert merged["b"] == 1800 + assert merged["w"] == 80.0 + + def test_round_tripped_t_reflects_the_winning_hlc(self) -> None: + record = _budget_record(t="2026-06-22T08:00:00+02:00") + round_tripped = log_to_budget(budget_to_log(record)) + assert round_tripped is not None + assert round_tripped["t"] != "" + + def test_empty_log_has_no_budget(self) -> None: + assert log_to_budget({}) is None + + def test_record_with_no_value_field_has_no_hlc_in_result(self) -> None: + """A record present but missing the "value" field is a defensive + edge case (should not occur from budget_to_log itself) -- the + result still comes back without crashing, and with no ``t``. + """ + log = {"budget": Record(id="budget", fields={})} + round_tripped = log_to_budget(log) + assert round_tripped == {} + + +class TestBudgetHistoryFields: + """The effective-from history riding along on the budget record.""" + + def test_history_becomes_one_field_per_date(self) -> None: + log = budget_to_log(_budget_record(), (_HIST_SEED, _HIST_CUT)) + fields = log["budget"].fields + assert "value" in fields + assert fields["hist:1970-01-01"][0] == 2200 + assert fields["hist:2026-07-26"][0] == 2000 + + def test_round_trips_back_to_entries(self) -> None: + entries = log_to_history( + budget_to_log(_budget_record(), (_HIST_SEED, _HIST_CUT)) + ) + assert [e.effective_from for e in entries] == ["1970-01-01", "2026-07-26"] + assert [e.kcal for e in entries] == [2200, 2000] + + def test_no_history_round_trips_to_nothing(self) -> None: + assert log_to_history(budget_to_log(_budget_record())) == () + + def test_empty_log_has_no_history(self) -> None: + assert log_to_history({}) == () + + def test_non_history_fields_are_ignored(self) -> None: + log = { + "budget": Record( + id="budget", fields={"value": ({"b": 2000}, _budget_hlc({}))} + ) + } + assert log_to_history(log) == () + + def test_non_int_history_value_is_skipped(self) -> None: + log = { + "budget": Record( + id="budget", + fields={"hist:2026-07-26": ("nonsense", _budget_hlc({}))}, + ), + } + assert log_to_history(log) == () + + def test_unparsable_edit_time_still_yields_a_field(self) -> None: + broken = BudgetEntry("2026-07-26", 2000, "not a timestamp") + log = budget_to_log(_budget_record(), (broken,)) + assert "hist:2026-07-26" in log["budget"].fields + + def test_history_survives_a_merge_with_a_history_free_peer(self) -> None: + """The rollout guarantee: an old device cannot clobber the history. + + merge_record is per-field LWW over the *union* of field names, so a + peer that only pushes ``value`` leaves ``hist:*`` untouched -- which + is why this needed no coordinated release. + """ + ours = budget_to_log(_budget_record(), (_HIST_SEED, _HIST_CUT)) + legacy = { + "budget": Record( + id="budget", + fields={ + "value": ( + {"v": 2, "b": 2200}, + _budget_hlc({"t": "2099-01-01T00:00:00+00:00"}), + ) + }, + ), + } + merged = merge_logs(ours, legacy) + assert len(log_to_history(merged)) == 2 + + +class TestParseRemoteBudget: + def test_parses_pushed_budget_wire_content(self) -> None: + record = _budget_record() + pushed = budget_to_log(record) + wire = json.dumps({rid: rec.to_dict() for rid, rec in pushed.items()}) + log = parse_remote_budget(wire) + assert log["budget"].id == "budget" + + def test_empty_object_parses_as_empty_log(self) -> None: + assert parse_remote_budget("{}") == {} + + def test_non_object_top_level_raises_type_error(self) -> None: + with pytest.raises(TypeError): + parse_remote_budget("[1, 2, 3]") + + def test_invalid_json_raises(self) -> None: + with pytest.raises(json.JSONDecodeError): + parse_remote_budget("not json{{{") diff --git a/diet_guard/tests/test_sync_pull.py b/diet_guard/tests/test_sync_pull.py new file mode 100644 index 0000000..8486ddd --- /dev/null +++ b/diet_guard/tests/test_sync_pull.py @@ -0,0 +1,153 @@ +"""Tests for pulling peers' logs: revisions, skips and push suppression. + +Split out of ``test_sync.py`` to keep both files under the repo's 250-line +cap. Covers the revision cache that lets a tick skip an unchanged peer, and +the no-op push suppression that keeps an idle tick from rewriting the remote. +""" + +from __future__ import annotations + +import json +from unittest.mock import MagicMock, patch + +from crdt_sync import ( + FirebaseAuthError, + FirebaseSyncError, + GitHubSyncError, + SyncState, +) +import pytest + +from diet_guard import _sync, _sync_client +from diet_guard._device import device_id, device_identity +from diet_guard.tests._sync_fakes import _mock_client, _write_token + + +class TestPullSharedLog: + """The fail-closed wrapper the gate and the lock-screen button share.""" + + def test_returns_none_on_success(self) -> None: + """A clean pull returns None (no reason to report).""" + with patch.object(_sync, "run_sync") as run_sync: + assert _sync.pull_shared_log() is None + run_sync.assert_called_once_with() + + def test_returns_reason_on_expected_failure(self) -> None: + """A real sync failure (here a network error) becomes a reason string.""" + with patch.object(_sync, "run_sync", side_effect=_sync.GitHubSyncError("boom")): + reason = _sync.pull_shared_log() + assert reason is not None + assert "boom" in reason + + @pytest.mark.parametrize( + "error", + [ + FirebaseAuthError("token refresh rejected"), + FirebaseSyncError("RTDB unavailable"), + ], + ) + def test_firebase_failure_fails_closed(self, error: Exception) -> None: + """A Firebase failure must return a reason, never raise. + + Regression guard: ``FirebaseAuthError``/``FirebaseSyncError`` are + *siblings* of ``GitHubSyncError`` under ``RemoteSyncError``, not + subclasses. Catching only the GitHub type let every per-request + failure of the *primary* backend escape, so the gate's "Fetch from + sync" button raised a traceback instead of reporting a reason and + leaving the lock up -- on the exact path this helper exists to + fail closed. + """ + with patch.object(_sync, "run_sync", side_effect=error): + reason = _sync.pull_shared_log() + + assert reason is not None + assert str(error) in reason + + def test_unexpected_error_is_not_swallowed(self) -> None: + """A bug outside the known failure surface surfaces, not hidden.""" + with ( + patch.object(_sync, "run_sync", side_effect=KeyError("bug")), + pytest.raises(KeyError), + ): + _sync.pull_shared_log() + + +class TestRemoteRevisions: + """Reading the peers' revision map, which gates the big downloads.""" + + def test_is_empty_without_a_bulk_map_read(self) -> None: + """GitHub has none; correctness must not depend on the optimisation.""" + client = MagicMock() + del client.get_string_map + + assert _sync._remote_revisions(client) == {} + + def test_returns_the_published_revisions(self) -> None: + client = MagicMock() + client.get_string_map.return_value = {"phone": "abc"} + + assert _sync._remote_revisions(client) == {"phone": "abc"} + + def test_an_unreadable_map_degrades_to_fetching_everything(self) -> None: + """Not worth failing a sync over; every peer is simply fetched.""" + client = MagicMock() + client.get_string_map.side_effect = GitHubSyncError("boom") + + assert _sync._remote_revisions(client) == {} + + +class TestPullSkipsUnchangedPeers: + """The single largest traffic saving in the fleet.""" + + def test_skips_a_peer_whose_revision_is_unchanged(self) -> None: + client = _mock_client(devices=("phone",)) + state = SyncState(pushed_rev=None, peer_revs={"phone": "rev-1"}) + seen: dict[str, str] = {} + + logs = _sync._pull_remote_logs( + client, {"phone": "rev-1"}, state, seen, device_identity() + ) + + assert logs == [] + assert seen == {"phone": "rev-1"} + client.get_file_text.assert_not_called() + + def test_downloads_a_peer_whose_revision_moved(self) -> None: + text = json.dumps({}) + client = _mock_client( + devices=("phone",), + files={"diet-guard-sync/devices/phone/food_log.json": text}, + ) + state = SyncState(pushed_rev=None, peer_revs={"phone": "rev-1"}) + seen: dict[str, str] = {} + + _sync._pull_remote_logs( + client, {"phone": "rev-2"}, state, seen, device_identity() + ) + + client.get_file_text.assert_called_once() + assert seen == {"phone": "rev-2"} + + +class TestNoOpPushSuppression: + """88% of the old GitHub history was byte-identical no-op pushes.""" + + def test_a_second_unchanged_sync_pushes_no_log(self) -> None: + """The saving the free-tier budget depends on, at 96 ticks a day.""" + _write_token() + client = _mock_client() + + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): + _sync.run_sync() + pushed_first = [ + call.args[0] for call in client.put_file_text.call_args_list + ] + client.put_file_text.reset_mock() + _sync.run_sync() + + pushed_second = [call.args[0] for call in client.put_file_text.call_args_list] + assert f"diet-guard-sync/devices/{device_id()}/food_log.json" in pushed_first + assert ( + f"diet-guard-sync/devices/{device_id()}/food_log.json" not in pushed_second + ) + assert f"diet-guard-sync/revs/{device_id()}" not in pushed_second