Review fixes: specific sync exceptions, clearer names

Address code-review feedback on the sync-before-lock change:

- pull_shared_log now catches (SyncError, GitHubSyncError, OSError) instead
  of a blanket `except Exception`. crdt_sync wraps every requests transport
  error in GitHubSyncError, so those three are the whole realistic failure
  surface; the gate still fails closed on all of them, and a genuine bug now
  surfaces instead of being hidden as "sync unavailable". Drops the
  broad-exception pylint disable and the per-file BLE001 ignore in pyproject.
- Rename _cli_gate._is_due -> _should_lock: too easily confused with the pure
  local predicate gate_is_due it wraps. The new name says what it decides.
- Rename the list `satisfied` -> `satisfied_slots` in _reconcile_after_fetch;
  it's a list of slots, not a boolean.

No behaviour change; 100% coverage retained.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FtSmzACzJSGyEatgK9exkd

diet_guard/_cli_gate.py

@@ -19,12 +19,15 @@ if TYPE_CHECKING:
     from collections.abc import Callable
 
 
-def _is_due(emit: Callable[[str], None]) -> bool:
-    """Return the gate decision, refreshing from sync only when a lock is due.
+def _should_lock(emit: Callable[[str], None]) -> bool:
+    """Decide whether to lock, pulling from sync first if a lock looks due.
 
-    The cheap local check runs first; the network pull happens solely on the
-    rare tick a lock would otherwise fire (via :func:`pull_shared_log`, which
-    fails closed), then the decision is re-read from the freshly written log.
+    This wraps the pure, local :func:`~diet_guard._gate.gate_is_due` predicate
+    with one network step: the cheap local check runs first, and only when it
+    says a lock is due do we pull the shared log (via :func:`pull_shared_log`,
+    which fails closed) and re-read the freshly written log.  So a meal just
+    logged on the phone can clear the lock, while a not-due tick never touches
+    the network at all.
     """
     if not gate_is_due():
         return False
@@ -40,8 +43,8 @@ def cmd_gate(emit: Callable[[str], None], *, check: bool, demo: bool) -> int:
     Three modes: ``--check`` is a headless decision (no window) whose exit code
     a timer reads; ``--demo`` always shows a safe demo window; bare ``gate``
     shows the real lock only when one is due.  Both real modes route through
-    :func:`_is_due`, which -- only when a lock is otherwise due -- pulls the
-    shared log first (see :func:`_refresh_from_sync`) so a meal just logged on
+    :func:`_should_lock`, which -- only when a lock is otherwise due -- pulls
+    the shared log first (via :func:`pull_shared_log`) so a meal just logged on
     the phone unlocks without a manual re-entry.  A flock guard stops a second
     window from stacking on top of the first, and a window-opening mode first
     waits for the X display so a session-start launch never crashes unshown.
@@ -56,10 +59,10 @@ def cmd_gate(emit: Callable[[str], None], *, check: bool, demo: bool) -> int:
         For ``--check``: 0 if not due, 1 if a lock is due.  Otherwise 0.
     """
     if check:
-        due = _is_due(emit)
+        due = _should_lock(emit)
         emit("due (a lock is warranted)" if due else "ok (no lock needed)")
         return 1 if due else 0
-    if not demo and not _is_due(emit):
+    if not demo and not _should_lock(emit):
         emit("ok - no lock needed right now.")
         return 0
     handle = acquire_gate_lock()

diet_guard/_gatelock_mealflow.py

@@ -283,9 +283,9 @@ class _GateMealFlow(_GateNutrition):
     def _reconcile_after_fetch(self) -> None:
         """Drop pending slots a freshly pulled meal now covers; unlock if none."""
         still_due = set(due_slots())
-        satisfied = [slot for slot in self._pending if slot not in still_due]
+        satisfied_slots = [slot for slot in self._pending if slot not in still_due]
         self._refresh_dashboard()
-        if not satisfied:
+        if not satisfied_slots:
             self._set_status("No new meals found in sync.")
             return
         self._pending = [slot for slot in self._pending if slot in still_due]
@@ -294,7 +294,7 @@ class _GateMealFlow(_GateNutrition):
             return
         self._clear_inputs()
         self._refresh_slot_header()
-        count = len(satisfied)
+        count = len(satisfied_slots)
         meal_word = "meal" if count == 1 else "meals"
         self._set_status(f"Pulled {count} {meal_word} — next meal, please.")
         self._widgets.desc_text.focus_set()

diet_guard/_sync.py

@@ -13,7 +13,7 @@ from __future__ import annotations
 import json
 import logging
 
-from crdt_sync import GitHubSyncClient, Log, merge_logs
+from crdt_sync import GitHubSyncClient, GitHubSyncError, Log, merge_logs
 
 from diet_guard._constants import (
     SYNC_DEVICE_ID,
@@ -138,12 +138,18 @@ def pull_shared_log() -> str | None:
     A thin wrapper over :func:`run_sync` for callers that must never crash on a
     sync error: the gate's automatic pre-lock refresh and the lock screen's
     manual "Fetch from sync" button.  Returns ``None`` on success, or a short
-    human-readable reason when the pull could not complete (no token, network
-    down, a disk error writing the merged log back), so the caller keeps its
-    own lock decision rather than failing open.
+    human-readable reason when the pull could not complete, so the caller keeps
+    its own lock decision rather than failing open.
+
+    The three caught types are the whole realistic failure surface of a run:
+    :class:`SyncError` (no/empty token), :class:`~crdt_sync.GitHubSyncError`
+    (the client wraps every ``requests`` transport error in this), and
+    :class:`OSError` (reading the token or writing the merged log back). A bug
+    outside these is deliberately *not* swallowed -- it should surface, not be
+    silently reported as a sync outage.
     """
     try:
         run_sync()
-    except Exception as exc:  # pylint: disable=broad-exception-caught
+    except (SyncError, GitHubSyncError, OSError) as exc:
         return f"sync unavailable ({exc})"
     return None

diet_guard/tests/test_cli_gate.py

@@ -8,7 +8,7 @@ from datetime import datetime, timezone
 from unittest.mock import MagicMock, patch
 
 from diet_guard import _cli_gate
-from diet_guard._cli_gate import _is_due, cmd_gate
+from diet_guard._cli_gate import _should_lock, cmd_gate
 from diet_guard._estimator import Nutrition
 from diet_guard._gate import gate_is_due
 from diet_guard._state import log_meal
@@ -109,7 +109,7 @@ class TestCmdGate:
         assert "display not ready" in lines[0]
 
 
-class TestIsDue:
+class TestShouldLock:
     """The decision helper only pays the network cost when a lock is due."""
 
     def test_not_due_skips_sync(self) -> None:
@@ -119,7 +119,7 @@ class TestIsDue:
             patch.object(_cli_gate, "gate_is_due", return_value=False),
             patch.object(_cli_gate, "pull_shared_log", pull),
         ):
-            assert _is_due([].append) is False
+            assert _should_lock([].append) is False
         pull.assert_not_called()
 
     def test_due_then_cleared_by_sync(self) -> None:
@@ -129,7 +129,7 @@ class TestIsDue:
             patch.object(_cli_gate, "gate_is_due", side_effect=[True, False]),
             patch.object(_cli_gate, "pull_shared_log", pull),
         ):
-            assert _is_due([].append) is False
+            assert _should_lock([].append) is False
         pull.assert_called_once()
 
     def test_due_and_still_due_after_sync(self) -> None:
@@ -139,7 +139,7 @@ class TestIsDue:
             patch.object(_cli_gate, "gate_is_due", side_effect=[True, True]),
             patch.object(_cli_gate, "pull_shared_log", pull),
         ):
-            assert _is_due([].append) is True
+            assert _should_lock([].append) is True
         pull.assert_called_once()
 
     def test_pull_failure_is_reported(self) -> None:
@@ -151,7 +151,7 @@ class TestIsDue:
                 _cli_gate, "pull_shared_log", return_value="sync unavailable (x)"
             ),
         ):
-            assert _is_due(lines.append) is True
+            assert _should_lock(lines.append) is True
         assert "sync unavailable (x)" in lines[0]
 
 

diet_guard/tests/test_sync.py

@@ -213,9 +213,17 @@ class TestPullSharedLog:
             assert _sync.pull_shared_log() is None
         run_sync.assert_called_once_with()
 
-    def test_returns_reason_on_any_failure(self) -> None:
-        """Any run_sync error becomes a reason string, never an exception."""
-        with patch.object(_sync, "run_sync", side_effect=RuntimeError("boom")):
+    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
+
+    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()

pyproject.toml

@@ -42,10 +42,6 @@ unfixable = []
 [tool.ruff.lint.per-file-ignores]
 "**/tests/**/*.py" = ["ARG", "D", "PLC0415", "PLR2004", "S101", "S107", "SLF001"]
 "**/test_*.py" = ["ARG", "D", "PLC0415", "PLR2004", "S101", "S107", "SLF001"]
-# pull_shared_log must fail closed: any run_sync error (no token, network, disk
-# write-back) is returned as a reason string so the gate keeps locking rather
-# than failing open. A deliberately broad catch, so BLE001 does not apply here.
-"**/_sync.py" = ["BLE001"]
 
 [tool.ruff.lint.pydocstyle]
 convention = "google"