diff --git a/app/pubspec.yaml b/app/pubspec.yaml index c9d78a7..9c8ec13 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.42+1 +version: 1.0.43+1 environment: sdk: ^3.12.2 diff --git a/diet_guard/_sync.py b/diet_guard/_sync.py index df01374..53cea49 100644 --- a/diet_guard/_sync.py +++ b/diet_guard/_sync.py @@ -53,6 +53,8 @@ from diet_guard.sync_merge import ( ) if TYPE_CHECKING: + from collections.abc import Sequence + from diet_guard._state import ( DayLog, ) @@ -86,10 +88,11 @@ def _pull_remote_logs( client: RemoteStore, remote_revs: dict[str, str], state: SyncState, - seen_revs: dict[str, str], + *, identity: DeviceIdentity, -) -> list[Log]: - """Return every other device's last-pushed log, skipping this one. + device_ids: Sequence[str], +) -> tuple[list[Log], dict[str, str]]: + """Return every other device's last-pushed log, plus the revisions seen. A device whose pushed file is corrupt, truncated, or otherwise unparsable (new or old wire format) is logged and skipped, same as one @@ -103,7 +106,8 @@ def _pull_remote_logs( day, so re-reading an unchanged peer is pure waste. """ remote_logs: list[Log] = [] - for device_id in client.list_directory(_DEVICES_DIR): + seen_revs: dict[str, str] = {} + for device_id in device_ids: if identity.is_own(device_id): continue remote_rev = remote_revs.get(device_id) @@ -123,7 +127,7 @@ def _pull_remote_logs( _logger.warning("Unparsable log pushed by device %r, skipping", device_id) continue seen_revs[device_id] = remote_rev or revision_of(text) - return remote_logs + return remote_logs, seen_revs def run_sync() -> DayLog: @@ -152,12 +156,17 @@ def run_sync() -> DayLog: state_store = FileSyncStateStore(SYNC_STATE_FILE) state = state_store.load() remote_revs = _remote_revisions(client) - seen_revs: dict[str, str] = {} + device_ids = list(client.list_directory(_DEVICES_DIR)) merged = daylog_to_log(read_raw_log()) - for remote_log in _pull_remote_logs( - client, remote_revs, state, seen_revs, identity - ): + remote_logs, seen_revs = _pull_remote_logs( + client, + remote_revs, + state, + identity=identity, + device_ids=device_ids, + ) + for remote_log in remote_logs: merged = merge_logs(merged, remote_log) merged_daylog = log_to_daylog(merged) @@ -167,9 +176,9 @@ def run_sync() -> DayLog: } write_raw_log(resigned) rebuild_food_bank(resigned) - _sync_budget(client) - _sync_food_bank(client) - _sync_manual_bank(client) + _sync_budget(client, device_ids) + _sync_food_bank(client, device_ids) + _sync_manual_bank(client, device_ids) push_log = daylog_to_log(resigned) push_json = json.dumps( diff --git a/diet_guard/_sync_banks.py b/diet_guard/_sync_banks.py index e627759..dfab7ae 100644 --- a/diet_guard/_sync_banks.py +++ b/diet_guard/_sync_banks.py @@ -17,7 +17,7 @@ import json import logging from typing import TYPE_CHECKING -from crdt_sync import merge_logs +from crdt_sync import MirrorSyncClient, merge_logs from diet_guard._budget import read_raw_record, write_raw_record from diet_guard._budget_history import ( @@ -38,7 +38,6 @@ from diet_guard._meal_schedule_store import ( write_raw_history as write_raw_schedule, ) from diet_guard._sync_paths import ( - _DEVICES_DIR, _device_budget_path, _device_food_bank_path, _device_manual_bank_path, @@ -58,12 +57,14 @@ from diet_guard.sync_merge import ( ) if TYPE_CHECKING: + from collections.abc import Sequence + from crdt_sync import GitHubSyncClient _logger = logging.getLogger(__name__) -def _sync_food_bank(client: GitHubSyncClient) -> None: +def _sync_food_bank(client: GitHubSyncClient, device_ids: Sequence[str]) -> 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 @@ -87,7 +88,7 @@ def _sync_food_bank(client: GitHubSyncClient) -> None: identity = device_identity() local = read_food_bank() merged = food_bank_to_log(local) - for device_id in client.list_directory(_DEVICES_DIR): + for device_id in device_ids: if identity.is_own(device_id): continue text = client.get_file_text(_device_food_bank_path(device_id)) @@ -120,7 +121,27 @@ def _sync_food_bank(client: GitHubSyncClient) -> None: ) -def _sync_manual_bank(client: GitHubSyncClient) -> None: +def _primary_only_text(client: GitHubSyncClient, path: str) -> str | None: + """Read ``path`` from the primary backend, skipping the mirror fallthrough. + + ``MirrorSyncClient.get_file_text`` treats a ``None`` from the primary as + "this device has not migrated yet" and consults the mirror. That is right + for the log and the derived bank, whose data really can live on either + side -- but for the *curated* bank it is 21 GitHub round trips per tick + (~406ms each, ~17s total) that return ``{}`` and nothing else. + + Verified before narrowing, and worth re-checking before widening again: + reading every peer's curated bank straight off the mirror found 23 absent, + 2 empty and **zero** non-empty. No curated food exists on the mirror alone, + so nothing is lost. A client with no ``primary`` (the GitHub-only case) is + read directly, which is the same backend either way. + """ + if isinstance(client, MirrorSyncClient): + return client.primary.get_file_text(path) + return client.get_file_text(path) + + +def _sync_manual_bank(client: GitHubSyncClient, device_ids: Sequence[str]) -> 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 @@ -130,10 +151,10 @@ def _sync_manual_bank(client: GitHubSyncClient) -> None: """ identity = device_identity() merged = manual_bank_to_log(read_manual_bank()) - for device_id in client.list_directory(_DEVICES_DIR): + for device_id in device_ids: if identity.is_own(device_id): continue - text = client.get_file_text(_device_manual_bank_path(device_id)) + text = _primary_only_text(client, _device_manual_bank_path(device_id)) if text is None: continue try: @@ -159,7 +180,7 @@ def _sync_manual_bank(client: GitHubSyncClient) -> None: ) -def _sync_budget(client: GitHubSyncClient) -> None: +def _sync_budget(client: GitHubSyncClient, device_ids: Sequence[str]) -> 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 @@ -172,7 +193,7 @@ def _sync_budget(client: GitHubSyncClient) -> None: """ identity = device_identity() merged = budget_to_log(read_raw_record(), load_entries(), load_schedule_entries()) - for device_id in client.list_directory(_DEVICES_DIR): + for device_id in device_ids: if identity.is_own(device_id): continue text = client.get_file_text(_device_budget_path(device_id)) diff --git a/diet_guard/tests/test_sync_banks_mirror.py b/diet_guard/tests/test_sync_banks_mirror.py new file mode 100644 index 0000000..ed4a28c --- /dev/null +++ b/diet_guard/tests/test_sync_banks_mirror.py @@ -0,0 +1,35 @@ +"""Tests for the curated bank's primary-only read. + +Split out of ``test_sync_banks.py`` to keep both files under the repo's +250-line cap. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from crdt_sync import MirrorSyncClient + +from diet_guard import _sync_banks + + +class TestPrimaryOnlyText: + """Reading the curated bank without the per-peer mirror fallthrough.""" + + def test_a_mirrored_client_reads_only_the_primary(self) -> None: + """The 17s/tick the fallthrough cost was spent fetching ``{}``.""" + primary = MagicMock() + primary.get_file_text.return_value = None + mirror = MagicMock() + client = MirrorSyncClient(primary, mirror) + + assert _sync_banks._primary_only_text(client, "some/path") is None + mirror.get_file_text.assert_not_called() + + def test_an_unmirrored_client_is_read_directly(self) -> None: + """GitHub-only devices have no primary/mirror split to skip.""" + client = MagicMock() + client.get_file_text.return_value = "{}" + + assert _sync_banks._primary_only_text(client, "some/path") == "{}" + client.get_file_text.assert_called_once_with("some/path") diff --git a/diet_guard/tests/test_sync_device_id.py b/diet_guard/tests/test_sync_device_id.py index c5aec3a..cf9a16c 100644 --- a/diet_guard/tests/test_sync_device_id.py +++ b/diet_guard/tests/test_sync_device_id.py @@ -37,9 +37,14 @@ class TestPullSkipsOwnLegacyPath: """ client = _mock_client(devices=("pc", "phone")) state = SyncState(pushed_rev=None, peer_revs={}) - seen: dict[str, str] = {} - _sync._pull_remote_logs(client, {}, state, seen, device_identity()) + _, _seen = _sync._pull_remote_logs( + client, + {}, + state, + identity=device_identity(), + device_ids=("pc", "phone"), + ) fetched = [call.args[0] for call in client.get_file_text.call_args_list] assert "diet-guard-sync/devices/pc/food_log.json" not in fetched @@ -54,10 +59,13 @@ class TestPullSkipsOwnLegacyPath: """ client = _mock_client(devices=("pc",)) state = SyncState(pushed_rev=None, peer_revs={}) - seen: dict[str, str] = {} - _sync._pull_remote_logs( - client, {}, state, seen, DeviceIdentity(device_id=device_id()) + _, _seen = _sync._pull_remote_logs( + client, + {}, + state, + identity=DeviceIdentity(device_id=device_id()), + device_ids=("pc",), ) fetched = [call.args[0] for call in client.get_file_text.call_args_list] diff --git a/diet_guard/tests/test_sync_pull.py b/diet_guard/tests/test_sync_pull.py index 8486ddd..1961970 100644 --- a/diet_guard/tests/test_sync_pull.py +++ b/diet_guard/tests/test_sync_pull.py @@ -102,10 +102,13 @@ class TestPullSkipsUnchangedPeers: 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() + logs, seen = _sync._pull_remote_logs( + client, + {"phone": "rev-1"}, + state, + identity=device_identity(), + device_ids=("phone",), ) assert logs == [] @@ -119,10 +122,13 @@ class TestPullSkipsUnchangedPeers: 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() + _, seen = _sync._pull_remote_logs( + client, + {"phone": "rev-2"}, + state, + identity=device_identity(), + device_ids=("phone",), ) client.get_file_text.assert_called_once()