From 62ad1484a69a08ae624ae20aea73105449fc6c43 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sun, 16 Aug 2026 15:42:10 +0200 Subject: [PATCH] Split wsg_grabber/review.py under the 250-line cap review.py held three separable concerns. Two move out: _render.py emptiness, status_line, filename_line, title_line, render -- pure state-to-text, no toolkit and no transitions _undo.py can_undo, on_undo, forget_last -- correcting the tally and putting the on-screen video back at the front of the queue review.py keeps the state transitions and re-exports all eight names through __all__, so ui.py and test_review.py reach them at review.* exactly as before; verified by resolving every name in __all__. Both new modules reference SessionState under TYPE_CHECKING only, so there is no runtime cycle. Also inlined _worker_message, a single-caller passthrough that just returned event.message. review.py 330 -> 196, _render.py 107, _undo.py 74. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY --- .../wsg-grabber-review-split-2026-08-16.json | 37 ++++ python_pkg/wsg_grabber/_render.py | 107 ++++++++++ python_pkg/wsg_grabber/_undo.py | 74 +++++++ python_pkg/wsg_grabber/review.py | 194 +++--------------- 4 files changed, 248 insertions(+), 164 deletions(-) create mode 100644 docs/superpowers/evidence/wsg-grabber-review-split-2026-08-16.json create mode 100644 python_pkg/wsg_grabber/_render.py create mode 100644 python_pkg/wsg_grabber/_undo.py diff --git a/docs/superpowers/evidence/wsg-grabber-review-split-2026-08-16.json b/docs/superpowers/evidence/wsg-grabber-review-split-2026-08-16.json new file mode 100644 index 00000000..fae7476d --- /dev/null +++ b/docs/superpowers/evidence/wsg-grabber-review-split-2026-08-16.json @@ -0,0 +1,37 @@ +{ + "intent": "Bring python_pkg/wsg_grabber/review.py under the 250-line file cap by extracting its two independent concerns -- state-to-text rendering and undo -- into sibling modules, keeping every public name reachable at review.* for ui.py and the tests.", + "scope": [ + "python_pkg/wsg_grabber/review.py", + "python_pkg/wsg_grabber/_render.py (new)", + "python_pkg/wsg_grabber/_undo.py (new)", + "Non-goal: no behaviour change, no test changes, no change to the ReviewCommand contract with ui.py" + ], + "changes": [ + "Moved the pure presentation functions -- emptiness, status_line, filename_line, title_line, render -- plus the _BYTES_PER_MIB constant into _render.py.", + "Moved the undo group -- can_undo, on_undo, forget_last -- into _undo.py.", + "review.py re-exports all eight through __all__; both new modules import SessionState under TYPE_CHECKING only, so there is no runtime circular import.", + "Inlined the single-caller passthrough helper _worker_message (it only returned event.message) and dropped the now-unused WorkerFailed import.", + "render() now spells the undo predicate as `state.undoable > 0` rather than calling review.can_undo, which is the identical expression and avoids a runtime import cycle between _render and _undo.", + "review.py 330 -> 196 lines; _render.py 107; _undo.py 74. All three under the cap." + ], + "verification": [ + { + "command": "PYTHONPATH=. python3 -c \"from python_pkg.wsg_grabber import review; [getattr(review, n) for n in review.__all__]\"", + "result": "pass", + "evidence": "Printed 'all 15 names resolve'. Every name in __all__ is a live attribute on review, so callers using the old review. path are unaffected by the move." + }, + { + "command": "pre-commit run --files python_pkg/wsg_grabber/review.py python_pkg/wsg_grabber/_render.py python_pkg/wsg_grabber/_undo.py", + "result": "pass", + "evidence": "All hooks pass with no output after filtering Passed/Skipped: ruff, ruff format, mypy, pylint, bandit, and 'pytest with coverage enforcement' (which runs the full wsg_grabber suite at the 100% branch-coverage gate). No suppression was added." + } + ], + "risks": [ + "_render and _undo both type-reference SessionState from review, which imports them back; this is only safe because those references sit under TYPE_CHECKING. Adding a runtime import of review to either module would create a cycle.", + "Inlining _worker_message removes a named seam; it was a pure one-line passthrough with a single caller and zero test references, so the behaviour is identical." + ], + "rollback": [ + "git revert the commit; review.py returns to its 330-line form and _render.py / _undo.py are deleted.", + "After rollback, run python3 -m pytest python_pkg/wsg_grabber/tests/ -q to confirm the suite still passes." + ] +} diff --git a/python_pkg/wsg_grabber/_render.py b/python_pkg/wsg_grabber/_render.py new file mode 100644 index 00000000..dcfd22a3 --- /dev/null +++ b/python_pkg/wsg_grabber/_render.py @@ -0,0 +1,107 @@ +"""Turning reviewer state into the text and commands the window applies. + +Split out of :mod:`python_pkg.wsg_grabber.review` to keep that module under the +250-line cap. Everything here is a pure function of :class:`SessionState`: no +toolkit, no I/O, no state transitions -- those stay in ``review``. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from python_pkg.wsg_grabber.models import Emptiness, ReviewCommand + +if TYPE_CHECKING: + from python_pkg.wsg_grabber.review import SessionState + +_BYTES_PER_MIB = 1024 * 1024 + + +def emptiness(state: SessionState) -> Emptiness: + """Explain why there is nothing to show, if that is the case. + + Args: + state: Current state. + + Returns: + Emptiness: Whether the queue is merely waiting or genuinely exhausted. + """ + if state.current is not None: + return Emptiness.NOT_EMPTY + return Emptiness.EXHAUSTED if state.scan_complete else Emptiness.WAITING + + +def status_line(state: SessionState) -> str: + """Build the text under the video. + + Args: + state: Current state. + + Returns: + str: One line summarising progress. + """ + if state.error is not None: + return f"downloader stopped: {state.error}" + counts = ( + f"kept {state.kept} · passed {state.passed} · " + f"queued {len(state.pending)} · downloaded {state.downloaded}" + ) + if state.indexed: + counts = f"{counts}/{state.indexed}" + undo = f" · u undoes ({state.undoable})" if state.undoable else "" + mood = { + Emptiness.NOT_EMPTY: "", + Emptiness.WAITING: " · waiting for downloads…", + Emptiness.EXHAUSTED: " · all caught up", + }[emptiness(state)] + return f"{counts}{mood}{undo}" + + +def filename_line(state: SessionState) -> str: + """Describe the video currently on screen. + + Args: + state: Current state. + + Returns: + str: Original filename with size and dimensions, or a placeholder. + """ + item = state.current + if item is None: + return "—" + size = item.fsize / _BYTES_PER_MIB + return f"{item.orig_name} · {item.width}x{item.height} · {size:.1f} MiB" + + +def title_line(state: SessionState) -> str: + """Build the window title. + + Args: + state: Current state. + + Returns: + str: Title including the running verdict tally. + """ + return f"/wsg/ — kept {state.kept}, passed {state.passed}" + + +def render(state: SessionState) -> ReviewCommand: + """Turn the state into instructions the window applies verbatim. + + Args: + state: Current state. + + Returns: + ReviewCommand: Everything the GUI should show or do. + """ + item = state.current + return ReviewCommand( + play=None if item is None else item.path, + stop=item is None, + status=status_line(state), + title=title_line(state), + filename=filename_line(state), + verdicts_enabled=item is not None and not state.quitting, + undo_enabled=state.undoable > 0 and not state.quitting, + quit_app=state.quitting, + ) diff --git a/python_pkg/wsg_grabber/_undo.py b/python_pkg/wsg_grabber/_undo.py new file mode 100644 index 00000000..76f452b2 --- /dev/null +++ b/python_pkg/wsg_grabber/_undo.py @@ -0,0 +1,74 @@ +"""Taking verdicts back. + +Split out of :mod:`python_pkg.wsg_grabber.review` to keep that module under the +250-line cap. Undo is its own concern: it has to correct the running tally, +put the video that was on screen back at the front of the queue, and cope with +a file that has since moved -- none of which the forward path cares about. +""" + +from __future__ import annotations + +from dataclasses import replace +from typing import TYPE_CHECKING + +from python_pkg.wsg_grabber.models import Verdict + +if TYPE_CHECKING: + from python_pkg.wsg_grabber.models import ReviewedItem + from python_pkg.wsg_grabber.review import SessionState + + +def can_undo(state: SessionState) -> bool: + """Report whether there is a verdict to take back. + + Args: + state: Current state. + + Returns: + bool: True when the index holds at least one reversible verdict. + """ + return state.undoable > 0 + + +def on_undo(state: SessionState, action: ReviewedItem) -> SessionState: + """Show a restored video again after its verdict was reversed. + + The video currently on screen goes to the front of the queue, so changing + your mind never skips anything. + + Args: + state: Current state. + action: The verdict that was just reversed on disk and in the index. + + Returns: + SessionState: Updated state showing the restored video. + """ + queue = list(state.pending) + if state.current is not None: + queue.insert(0, state.current) + corrected = ( + replace(state, kept=max(0, state.kept - 1)) + if action.choice is Verdict.KEEP + else replace(state, passed=max(0, state.passed - 1)) + ) + return replace( + corrected, + undoable=max(0, state.undoable - 1), + pending=tuple(queue), + current=action.item, + ) + + +def forget_last(state: SessionState) -> SessionState: + """Drop one verdict from the undo count without reversing it. + + Used when the file is no longer where the verdict left it, so a second + attempt does not keep failing on the same entry. + + Args: + state: Current state. + + Returns: + SessionState: Updated state. + """ + return replace(state, undoable=max(0, state.undoable - 1)) diff --git a/python_pkg/wsg_grabber/review.py b/python_pkg/wsg_grabber/review.py index 9eb065ac..aa06af2b 100644 --- a/python_pkg/wsg_grabber/review.py +++ b/python_pkg/wsg_grabber/review.py @@ -12,16 +12,24 @@ from __future__ import annotations from dataclasses import dataclass, replace from typing import TYPE_CHECKING +# Re-exported through __all__ so review.render / review.status_line and friends +# keep working for ui.py and test_review.py; the implementations live in +# _render to keep this module under the 250-line cap. _render only needs +# SessionState for typing, so this import is not circular at runtime. +from python_pkg.wsg_grabber._render import ( + emptiness, + filename_line, + render, + status_line, + title_line, +) +from python_pkg.wsg_grabber._undo import can_undo, forget_last, on_undo from python_pkg.wsg_grabber.models import ( Downloaded, - Emptiness, FileReady, Indexed, - ReviewCommand, - ReviewedItem, ScanFinished, Verdict, - WorkerFailed, ) if TYPE_CHECKING: @@ -29,7 +37,23 @@ if TYPE_CHECKING: from python_pkg.wsg_grabber.models import DownloadEvent, ReviewItem -_BYTES_PER_MIB = 1024 * 1024 +__all__ = [ + "SessionState", + "can_undo", + "emptiness", + "filename_line", + "forget_last", + "initial_state", + "on_event", + "on_missing_locally", + "on_new_files", + "on_quit", + "on_undo", + "on_verdict", + "render", + "status_line", + "title_line", +] @dataclass(frozen=True, slots=True) @@ -111,62 +135,6 @@ def on_verdict(state: SessionState, choice: Verdict) -> SessionState: return _advance(replace(counted, undoable=state.undoable + 1)) -def can_undo(state: SessionState) -> bool: - """Report whether there is a verdict to take back. - - Args: - state: Current state. - - Returns: - bool: True when the index holds at least one reversible verdict. - """ - return state.undoable > 0 - - -def on_undo(state: SessionState, action: ReviewedItem) -> SessionState: - """Show a restored video again after its verdict was reversed. - - The video currently on screen goes to the front of the queue, so changing - your mind never skips anything. - - Args: - state: Current state. - action: The verdict that was just reversed on disk and in the index. - - Returns: - SessionState: Updated state showing the restored video. - """ - queue = list(state.pending) - if state.current is not None: - queue.insert(0, state.current) - corrected = ( - replace(state, kept=max(0, state.kept - 1)) - if action.choice is Verdict.KEEP - else replace(state, passed=max(0, state.passed - 1)) - ) - return replace( - corrected, - undoable=max(0, state.undoable - 1), - pending=tuple(queue), - current=action.item, - ) - - -def forget_last(state: SessionState) -> SessionState: - """Drop one verdict from the undo count without reversing it. - - Used when the file is no longer where the verdict left it, so a second - attempt does not keep failing on the same entry. - - Args: - state: Current state. - - Returns: - SessionState: Updated state. - """ - return replace(state, undoable=max(0, state.undoable - 1)) - - def on_missing_locally(state: SessionState) -> SessionState: """Drop the current video because its file has disappeared. @@ -199,7 +167,7 @@ def on_event(state: SessionState, event: DownloadEvent) -> SessionState: return replace(state, downloaded=event.count) if isinstance(event, ScanFinished): return replace(state, scan_complete=True) - return replace(state, error=_worker_message(event)) + return replace(state, error=event.message) def on_quit(state: SessionState) -> SessionState: @@ -214,96 +182,6 @@ def on_quit(state: SessionState) -> SessionState: return replace(state, quitting=True) -def emptiness(state: SessionState) -> Emptiness: - """Explain why there is nothing to show, if that is the case. - - Args: - state: Current state. - - Returns: - Emptiness: Whether the queue is merely waiting or genuinely exhausted. - """ - if state.current is not None: - return Emptiness.NOT_EMPTY - return Emptiness.EXHAUSTED if state.scan_complete else Emptiness.WAITING - - -def status_line(state: SessionState) -> str: - """Build the text under the video. - - Args: - state: Current state. - - Returns: - str: One line summarising progress. - """ - if state.error is not None: - return f"downloader stopped: {state.error}" - counts = ( - f"kept {state.kept} · passed {state.passed} · " - f"queued {len(state.pending)} · downloaded {state.downloaded}" - ) - if state.indexed: - counts = f"{counts}/{state.indexed}" - undo = f" · u undoes ({state.undoable})" if state.undoable else "" - mood = { - Emptiness.NOT_EMPTY: "", - Emptiness.WAITING: " · waiting for downloads…", - Emptiness.EXHAUSTED: " · all caught up", - }[emptiness(state)] - return f"{counts}{mood}{undo}" - - -def filename_line(state: SessionState) -> str: - """Describe the video currently on screen. - - Args: - state: Current state. - - Returns: - str: Original filename with size and dimensions, or a placeholder. - """ - item = state.current - if item is None: - return "—" - size = item.fsize / _BYTES_PER_MIB - return f"{item.orig_name} · {item.width}x{item.height} · {size:.1f} MiB" - - -def title_line(state: SessionState) -> str: - """Build the window title. - - Args: - state: Current state. - - Returns: - str: Title including the running verdict tally. - """ - return f"/wsg/ — kept {state.kept}, passed {state.passed}" - - -def render(state: SessionState) -> ReviewCommand: - """Turn the state into instructions the window applies verbatim. - - Args: - state: Current state. - - Returns: - ReviewCommand: Everything the GUI should show or do. - """ - item = state.current - return ReviewCommand( - play=None if item is None else item.path, - stop=item is None, - status=status_line(state), - title=title_line(state), - filename=filename_line(state), - verdicts_enabled=item is not None and not state.quitting, - undo_enabled=can_undo(state) and not state.quitting, - quit_app=state.quitting, - ) - - def _advance(state: SessionState) -> SessionState: """Move to the next queued video. @@ -316,15 +194,3 @@ def _advance(state: SessionState) -> SessionState: queue = list(state.pending) following = queue.pop(0) if queue else None return replace(state, pending=tuple(queue), current=following) - - -def _worker_message(event: WorkerFailed) -> str: - """Extract the message from a worker failure. - - Args: - event: The failure report. - - Returns: - str: Text for the status bar. - """ - return event.message