Split brother_printer dropped-page detection out of consumables

consumables.py 279 -> 217. check_page_delivery and _snapshot_counters
move to _page_delivery.py.

The state-file primitives stay put: test_consumables.py patches
_load_consumable_state, _save_consumable_state and CONSUMABLE_STATE_FILE
on consumables, and moving them would have let a test write to the real
~/.config/brother_printer/state.json. Verified by mtime after the run,
not just by a green suite.

test_consumables_part2.py patches the same three primitives from both
sides of the seam, so it gains a second MOD_PD applied per class rather
than a blanket swap. Pass count is 402 before and after.

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

docs/superpowers/contracts/brother-printer-page-delivery-split-2026-08-16.json

@@ -0,0 +1,21 @@
+{
+  "title": "Split brother_printer/consumables.py under the 250-line cap",
+  "objective": "Bring python_pkg/brother_printer/consumables.py (279 lines) under the repo-wide 250-line cap by moving the dropped-page detection into a new _page_delivery.py, while leaving every state-file primitive in consumables where two test modules patch it.",
+  "acceptance_criteria": [
+    "consumables.py and the new _page_delivery.py are each at most 250 lines.",
+    "The brother_printer suite still reports exactly 402 passing tests -- the same count as before the split, proving no test was dropped or silently skipped.",
+    "The package keeps 100% branch coverage of the brother_printer source modules.",
+    "_load_consumable_state, _save_consumable_state, _get_cups_total_pages, CONSUMABLE_STATE_FILE and CUPS_PAGE_LOG stay defined in consumables.py, so test_consumables.py's patches still intercept every write that would otherwise reach ~/.config/brother_printer/state.json.",
+    "~/.config/brother_printer/state.json is unmodified after a full suite run, checked by mtime rather than inferred from a green run.",
+    "test_consumables.py needs no edit; test_consumables_part2.py gains a second MOD constant applied only to the two classes whose functions moved, never as a blanket substitution.",
+    "No assertion is changed and no test is deleted, skipped or weakened.",
+    "No suppression is added and no coverage exclusion is introduced."
+  ],
+  "out_of_scope": [
+    "Changing what any test asserts, or any counter-rebasing arithmetic.",
+    "Moving the state-file primitives, which would risk a test writing to the live consumable state.",
+    "Shrinking the module docstring, which records why the printer's counter is trusted over the CUPS page log.",
+    "Splitting the brother_printer test files, which are separate commits."
+  ],
+  "verifier": "PYTHONPATH=. python3 -m pytest python_pkg/brother_printer/tests -q (expects exactly 402 passed), plus ls -la ~/.config/brother_printer/state.json to confirm the live state file was not written, plus pre-commit run --files on the touched files, plus bash ~/utils/scripts/check_file_length.sh --all for the cap."
+}

docs/superpowers/evidence/brother-printer-page-delivery-split-2026-08-16.json

@@ -0,0 +1,52 @@
+{
+  "intent": "Bring brother_printer/consumables.py under the 250-line cap by moving the dropped-page detection into its own module, with no behaviour change.",
+  "scope": [
+    "python_pkg/brother_printer/_page_delivery.py (new)",
+    "python_pkg/brother_printer/consumables.py",
+    "python_pkg/brother_printer/_page_count.py (import path only)",
+    "python_pkg/brother_printer/tests/test_consumables_part2.py (second MOD constant, applied per class)",
+    "Non-goal: no assertion changed, no test deleted, skipped or weakened"
+  ],
+  "changes": [
+    "Moved check_page_delivery and _snapshot_counters verbatim into python_pkg/brother_printer/_page_delivery.py, which imports the three state-file primitives from consumables.",
+    "The primitives themselves stay in consumables.py: test_consumables.py patches _load_consumable_state, _save_consumable_state, _get_cups_total_pages, CONSUMABLE_STATE_FILE and CUPS_PAGE_LOG on that module, and moving any of them would have let a test write to the real state file at ~/.config/brother_printer/state.json.",
+    "consumables.py dropped the now-unused PAGE_DROP_WARN_THRESHOLD and PageDeliveryCheck imports; 279 -> 217 lines.",
+    "test_consumables_part2.py gained a second constant MOD_PD alongside MOD, applied only to TestCheckPageDelivery and TestSnapshotCounters. Its other two classes test functions that did not move and keep MOD. This is the per-name discipline docs/python-split-recipes.md requires, not a blanket swap: the same three primitives are patched from both modules, so which MOD is correct depends on which caller the test exercises.",
+    "_page_count.py imports check_page_delivery from _page_delivery now; test_page_count.py patches that name on _page_count, so it needed no edit."
+  ],
+  "verification": [
+    {
+      "command": "PYTHONPATH=. python3 -m pytest python_pkg/brother_printer/tests -q",
+      "result": "pass",
+      "evidence": "402 passed before the split and 402 passed after it - identical pass count, not merely a green run."
+    },
+    {
+      "command": "PYTHONPATH=. python3 -c \"from python_pkg.brother_printer import _page_delivery as pd, consumables as c, _page_count as pc; pd._load_consumable_state; pd._save_consumable_state; pd._get_cups_total_pages; c.CONSUMABLE_STATE_FILE; c.CUPS_PAGE_LOG; c._migrate_state_to_printer_scale; pc.check_page_delivery\"",
+      "result": "pass",
+      "evidence": "runtime probe OK - both MOD and MOD_PD resolve every name their classes patch."
+    },
+    {
+      "command": "ls -la ~/.config/brother_printer/state.json",
+      "result": "pass",
+      "evidence": "mtime still Jul 17 14:42 after the full suite run, so no test escaped its mock and wrote to the live consumable state. A green pass count alone would not have caught that."
+    },
+    {
+      "command": "wc -l python_pkg/brother_printer/consumables.py python_pkg/brother_printer/_page_delivery.py",
+      "result": "pass",
+      "evidence": "217 and 76 lines, both under the 250-line cap."
+    },
+    {
+      "command": "pre-commit run --files <the five touched files>",
+      "result": "pass",
+      "evidence": "ruff, ruff format, mypy, pylint, bandit and pytest-with-coverage all pass on the touched files."
+    }
+  ],
+  "risks": [
+    "test_consumables_part2.py now carries two module constants; a future edit that patches a state primitive in the wrong one fails loudly with an AttributeError rather than silently, because each name exists in only one of the two modules under its own MOD.",
+    "consumables <- _page_delivery <- _page_count is a three-module chain; consumables must never import from _page_delivery or it becomes a cycle."
+  ],
+  "rollback": [
+    "git revert the commit; consumables.py returns to 279 lines and _page_delivery.py disappears.",
+    "After rollback, re-run the package suite and confirm it still reports 402 passed."
+  ]
+}

python_pkg/brother_printer/_page_count.py

@@ -8,6 +8,7 @@ than through ``display``.
 
 from __future__ import annotations
 
+from python_pkg.brother_printer._page_delivery import check_page_delivery
 from python_pkg.brother_printer._supply import render_life_bar
 from python_pkg.brother_printer.constants import (
     BOLD,
@@ -19,10 +20,7 @@ from python_pkg.brother_printer.constants import (
     YELLOW,
     _out,
 )
-from python_pkg.brother_printer.consumables import (
-    check_page_delivery,
-    estimate_consumable_life,
-)
+from python_pkg.brother_printer.consumables import estimate_consumable_life
 
 
 def _display_page_delivery_warning(printer_total: int, *, queue_idle: bool) -> None:

python_pkg/brother_printer/_page_delivery.py

@@ -0,0 +1,76 @@
+"""Dropped-page detection: CUPS's page log versus the printer's own counter.
+
+Split out of :mod:`python_pkg.brother_printer.consumables` to keep it under
+the 250-line cap. The state-file I/O stays in ``consumables`` because
+``tests/test_consumables.py`` patches it there; it is imported here so
+``tests/test_consumables_part2.py`` can patch it on this module for the two
+functions that moved.
+"""
+
+from __future__ import annotations
+
+from python_pkg.brother_printer.constants import PAGE_DROP_WARN_THRESHOLD
+from python_pkg.brother_printer.consumables import (
+    _get_cups_total_pages,
+    _load_consumable_state,
+    _save_consumable_state,
+)
+from python_pkg.brother_printer.data_classes import PageDeliveryCheck
+
+
+def check_page_delivery(printer_total: int, *, queue_idle: bool) -> PageDeliveryCheck:
+    """Compare pages CUPS logged against pages the printer actually counted.
+
+    Only meaningful between jobs: mid-job, CUPS has logged pages the printer has
+    not yet pulled off the wire, which would look identical to dropping them.
+    Records a fresh snapshot of both counters whenever it runs cleanly.
+
+    Args:
+        printer_total: Lifetime count from the printer's own counter.
+        queue_idle: False when a job is queued or printing, which makes any
+            comparison meaningless.
+
+    Returns:
+        The comparison. suspected is True only when CUPS claims materially more
+        pages than the printer recorded.
+    """
+    check = PageDeliveryCheck()
+    if printer_total <= 0 or not queue_idle:
+        return check
+    state = _load_consumable_state()
+    cups_total = _get_cups_total_pages()
+    last_printer = state["last_printer_count"]
+    last_cups = state["last_cups_total"]
+
+    _snapshot_counters(state, printer_total, cups_total)
+
+    if last_printer <= 0 or last_cups <= 0:
+        # No baseline yet: this run establishes one.
+        return check
+    printer_delta = printer_total - last_printer
+    cups_delta = cups_total - last_cups
+    if printer_delta < 0 or cups_delta < 0:
+        # Counter reset or the page log rotated; nothing to conclude.
+        return check
+    check.cups_pages = cups_delta
+    check.printer_pages = printer_delta
+    check.dropped = cups_delta - printer_delta
+    check.suspected = check.dropped >= PAGE_DROP_WARN_THRESHOLD
+    return check
+
+
+def _snapshot_counters(
+    state: dict[str, int],
+    printer_total: int,
+    cups_total: int,
+) -> None:
+    """Persist where both counters stood, for the next run to compare against."""
+    if (
+        state["last_printer_count"] == printer_total
+        and state["last_cups_total"] == cups_total
+    ):
+        return
+    updated = dict(state)
+    updated["last_printer_count"] = printer_total
+    updated["last_cups_total"] = cups_total
+    _save_consumable_state(updated)

python_pkg/brother_printer/consumables.py

@@ -20,15 +20,11 @@ from python_pkg.brother_printer.constants import (
     CUPS_PAGE_LOG_PATH,
     DRUM_RATED_PAGES,
     GREEN,
-    PAGE_DROP_WARN_THRESHOLD,
     RESET,
     TONER_RATED_PAGES,
     _out,
 )
-from python_pkg.brother_printer.data_classes import (
-    PageCountEstimate,
-    PageDeliveryCheck,
-)
+from python_pkg.brother_printer.data_classes import PageCountEstimate
 
 logger = logging.getLogger(__name__)
 
@@ -186,64 +182,6 @@ def _cups_total_on_printer_scale(state: dict[str, int]) -> int:
     return cups_total + offset
 
 
-def check_page_delivery(printer_total: int, *, queue_idle: bool) -> PageDeliveryCheck:
-    """Compare pages CUPS logged against pages the printer actually counted.
-
-    Only meaningful between jobs: mid-job, CUPS has logged pages the printer has
-    not yet pulled off the wire, which would look identical to dropping them.
-    Records a fresh snapshot of both counters whenever it runs cleanly.
-
-    Args:
-        printer_total: Lifetime count from the printer's own counter.
-        queue_idle: False when a job is queued or printing, which makes any
-            comparison meaningless.
-
-    Returns:
-        The comparison. suspected is True only when CUPS claims materially more
-        pages than the printer recorded.
-    """
-    check = PageDeliveryCheck()
-    if printer_total <= 0 or not queue_idle:
-        return check
-    state = _load_consumable_state()
-    cups_total = _get_cups_total_pages()
-    last_printer = state["last_printer_count"]
-    last_cups = state["last_cups_total"]
-
-    _snapshot_counters(state, printer_total, cups_total)
-
-    if last_printer <= 0 or last_cups <= 0:
-        # No baseline yet: this run establishes one.
-        return check
-    printer_delta = printer_total - last_printer
-    cups_delta = cups_total - last_cups
-    if printer_delta < 0 or cups_delta < 0:
-        # Counter reset or the page log rotated; nothing to conclude.
-        return check
-    check.cups_pages = cups_delta
-    check.printer_pages = printer_delta
-    check.dropped = cups_delta - printer_delta
-    check.suspected = check.dropped >= PAGE_DROP_WARN_THRESHOLD
-    return check
-
-
-def _snapshot_counters(
-    state: dict[str, int],
-    printer_total: int,
-    cups_total: int,
-) -> None:
-    """Persist where both counters stood, for the next run to compare against."""
-    if (
-        state["last_printer_count"] == printer_total
-        and state["last_cups_total"] == cups_total
-    ):
-        return
-    updated = dict(state)
-    updated["last_printer_count"] = printer_total
-    updated["last_cups_total"] = cups_total
-    _save_consumable_state(updated)
-
-
 def estimate_consumable_life(printer_total: int = 0) -> PageCountEstimate:
     """Estimate toner/drum life from pages printed since the last replacement.
 

python_pkg/brother_printer/tests/test_consumables_part2.py

@@ -9,16 +9,21 @@ from __future__ import annotations
 
 from unittest.mock import MagicMock, patch
 
+from python_pkg.brother_printer._page_delivery import (
+    _snapshot_counters,
+    check_page_delivery,
+)
 from python_pkg.brother_printer.consumables import (
     STATE_SCHEMA_CUPS_SCALE,
     STATE_SCHEMA_PRINTER_SCALE,
     _cups_total_on_printer_scale,
     _migrate_state_to_printer_scale,
-    _snapshot_counters,
-    check_page_delivery,
 )
 
 MOD = "python_pkg.brother_printer.consumables"
+# check_page_delivery and _snapshot_counters moved to _page_delivery, so the
+# state-file helpers they call resolve through that module's globals instead.
+MOD_PD = "python_pkg.brother_printer._page_delivery"
 
 
 class TestMigrateStateToPrinterScale:
@@ -113,9 +118,9 @@ class TestCheckPageDelivery:
     never actually came out.
     """
 
-    @patch(f"{MOD}._snapshot_counters")
-    @patch(f"{MOD}._get_cups_total_pages", return_value=1841)
-    @patch(f"{MOD}._load_consumable_state")
+    @patch(f"{MOD_PD}._snapshot_counters")
+    @patch(f"{MOD_PD}._get_cups_total_pages", return_value=1841)
+    @patch(f"{MOD_PD}._load_consumable_state")
     def test_detects_dropped_pages(
         self,
         mock_load: MagicMock,
@@ -133,9 +138,9 @@ class TestCheckPageDelivery:
         assert check.dropped == 63
         assert check.suspected is True
 
-    @patch(f"{MOD}._snapshot_counters")
-    @patch(f"{MOD}._get_cups_total_pages", return_value=1841)
-    @patch(f"{MOD}._load_consumable_state")
+    @patch(f"{MOD_PD}._snapshot_counters")
+    @patch(f"{MOD_PD}._get_cups_total_pages", return_value=1841)
+    @patch(f"{MOD_PD}._load_consumable_state")
     def test_healthy_job_does_not_warn(
         self,
         mock_load: MagicMock,
@@ -150,9 +155,9 @@ class TestCheckPageDelivery:
         assert check.dropped == 0
         assert check.suspected is False
 
-    @patch(f"{MOD}._snapshot_counters")
-    @patch(f"{MOD}._get_cups_total_pages", return_value=1780)
-    @patch(f"{MOD}._load_consumable_state")
+    @patch(f"{MOD_PD}._snapshot_counters")
+    @patch(f"{MOD_PD}._get_cups_total_pages", return_value=1780)
+    @patch(f"{MOD_PD}._load_consumable_state")
     def test_small_gap_is_not_a_warning(
         self,
         mock_load: MagicMock,
@@ -175,9 +180,9 @@ class TestCheckPageDelivery:
     def test_skipped_without_printer_count(self) -> None:
         assert check_page_delivery(0, queue_idle=True).suspected is False
 
-    @patch(f"{MOD}._snapshot_counters")
-    @patch(f"{MOD}._get_cups_total_pages", return_value=1841)
-    @patch(f"{MOD}._load_consumable_state")
+    @patch(f"{MOD_PD}._snapshot_counters")
+    @patch(f"{MOD_PD}._get_cups_total_pages", return_value=1841)
+    @patch(f"{MOD_PD}._load_consumable_state")
     def test_first_run_only_establishes_baseline(
         self,
         mock_load: MagicMock,
@@ -189,9 +194,9 @@ class TestCheckPageDelivery:
         assert check.suspected is False
         mock_snap.assert_called_once()
 
-    @patch(f"{MOD}._snapshot_counters")
-    @patch(f"{MOD}._get_cups_total_pages", return_value=10)
-    @patch(f"{MOD}._load_consumable_state")
+    @patch(f"{MOD_PD}._snapshot_counters")
+    @patch(f"{MOD_PD}._get_cups_total_pages", return_value=10)
+    @patch(f"{MOD_PD}._load_consumable_state")
     def test_log_rotation_draws_no_conclusion(
         self,
         mock_load: MagicMock,
@@ -206,9 +211,9 @@ class TestCheckPageDelivery:
         check = check_page_delivery(2087, queue_idle=True)
         assert check.suspected is False
 
-    @patch(f"{MOD}._snapshot_counters")
-    @patch(f"{MOD}._get_cups_total_pages", return_value=1841)
-    @patch(f"{MOD}._load_consumable_state")
+    @patch(f"{MOD_PD}._snapshot_counters")
+    @patch(f"{MOD_PD}._get_cups_total_pages", return_value=1841)
+    @patch(f"{MOD_PD}._load_consumable_state")
     def test_printer_counter_reset_draws_no_conclusion(
         self,
         mock_load: MagicMock,
@@ -224,7 +229,7 @@ class TestCheckPageDelivery:
 
 
 class TestSnapshotCounters:
-    @patch(f"{MOD}._save_consumable_state")
+    @patch(f"{MOD_PD}._save_consumable_state")
     def test_writes_new_snapshot(self, mock_save: MagicMock) -> None:
         state = {"last_printer_count": 2000, "last_cups_total": 1700}
         _snapshot_counters(state, 2087, 1778)
@@ -232,7 +237,7 @@ class TestSnapshotCounters:
         assert saved["last_printer_count"] == 2087
         assert saved["last_cups_total"] == 1778
 
-    @patch(f"{MOD}._save_consumable_state")
+    @patch(f"{MOD_PD}._save_consumable_state")
     def test_unchanged_counters_skip_the_write(self, mock_save: MagicMock) -> None:
         """Do not rewrite the file on every run when nothing moved."""
         state = {"last_printer_count": 2087, "last_cups_total": 1778}