From 0b658f55f62bb50a95e6730b3d20b8b7fe55cab0 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Fri, 21 Aug 2026 13:38:34 +0200 Subject: [PATCH] fix(sync): prune the revision marker too, not just the device directory Pruning 20 dead devices made the narrow pull *slower* -- ~90ms to ~15s -- which is the opposite of the point, and I caught it only because I measured after running rather than assuming the win. `_device_paths` listed the four data files a device publishes but not its entry under `revs/`. `_sync_refresh._candidate_peers` deliberately enumerates peers from the revision map instead of calling `list_directory` (that call is ~445ms mirrored), so every orphaned marker resurrected a deleted device as a candidate and bought a round trip to discover it was gone. 19 orphans, 19 wasted probes, every tick. The marker is now part of the device's path set, so it is backed up and deleted with everything else, and a regression test pins it by name rather than by count. The 19 already-orphaned markers on the remote were cleaned up separately. Verified after the cleanup: narrow pull ~84ms, full tick 8.9s (was ~27s), gate --check ~105ms, and the log is untouched at 441 entries / 117 foods with peer_revs correctly down to 3. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LJ5YnA3CERitQ57HjMu2Qz --- app/pubspec.yaml | 2 +- diet_guard/_prune_peers.py | 12 +++++++++++- diet_guard/tests/test_prune_peers.py | 21 ++++++++++++++++++--- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/app/pubspec.yaml b/app/pubspec.yaml index 0dda167..47ddf99 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.45+1 +version: 1.0.46+1 environment: sdk: ^3.12.2 diff --git a/diet_guard/_prune_peers.py b/diet_guard/_prune_peers.py index 59bf1f8..42c9218 100644 --- a/diet_guard/_prune_peers.py +++ b/diet_guard/_prune_peers.py @@ -34,6 +34,7 @@ from diet_guard._device import device_identity from diet_guard._sync_client import _client_for_run from diet_guard._sync_paths import ( _DEVICES_DIR, + _REVS_DIR, _device_budget_path, _device_food_bank_path, _device_log_path, @@ -69,12 +70,21 @@ class PrunePlan: def _device_paths(device_id: str) -> tuple[str, ...]: - """Return every path a device publishes under.""" + """Return every path a device publishes under. + + The revision marker is one of them, and forgetting it is not cosmetic: + ``_sync_refresh._candidate_peers`` enumerates peers from the revision map, + so a marker left behind for a deleted device makes every tick propose a + peer that no longer exists and pay a round trip discovering that. Pruning + without this made the narrow pull *slower* (~90ms -> ~15s) rather than + faster. + """ return ( _device_log_path(device_id), _device_food_bank_path(device_id), _device_manual_bank_path(device_id), _device_budget_path(device_id), + f"{_REVS_DIR}/{device_id}", ) diff --git a/diet_guard/tests/test_prune_peers.py b/diet_guard/tests/test_prune_peers.py index 5c6e103..ced591d 100644 --- a/diet_guard/tests/test_prune_peers.py +++ b/diet_guard/tests/test_prune_peers.py @@ -102,7 +102,7 @@ class TestApply: written = _prune_peers.backup_peers(client, ["dead"], tmp_path) - assert written == 4 # log, food bank, curated bank, budget + assert written == 5 # log, food bank, curated bank, budget, rev assert (tmp_path / "dead" / "food_log.json").read_text() == "{}" def test_backup_skips_paths_a_device_never_published(self, tmp_path: Path) -> None: @@ -111,11 +111,26 @@ class TestApply: assert _prune_peers.backup_peers(client, ["dead"], tmp_path) == 0 + def test_apply_deletes_the_revision_marker_too(self) -> None: + """Leaving it behind makes the narrow pull slower, not faster. + + ``_sync_refresh._candidate_peers`` enumerates peers from the revision + map, so an orphaned marker resurrects a deleted device as a candidate + and costs a round trip every tick. Observed for real: pruning without + this took the narrow pull from ~90ms to ~15s. + """ + client = MagicMock() + + list(_prune_peers.apply_prune(client, ["dead"])) + + deleted = [call.args[0] for call in client.delete_file.call_args_list] + assert "diet-guard-sync/revs/dead" in deleted + def test_apply_deletes_all_four_paths(self) -> None: client = MagicMock() assert list(_prune_peers.apply_prune(client, ["dead"])) == ["dead"] - assert client.delete_file.call_count == 4 + assert client.delete_file.call_count == 5 class TestCommand: @@ -180,7 +195,7 @@ class TestCommand: code, out = self._run(monkeypatch, client, apply=True, backup_dir=str(tmp_path)) assert code == 0 - assert client.delete_file.call_count == 4 + assert client.delete_file.call_count == 5 assert any("pruned 1 device" in line for line in out) def test_main_dispatches_the_subcommand(