Conversation
The screenshot tool assigned `drives.list_volumes`. That was the panel's scan until it was rewritten around `scan_batches`, after which the assignment created a new unused attribute instead of overriding anything, and nothing said so. The real scan then ran on every render: whether real drive labels and free space reached the PNGs came down to which write landed last, since the network batch arrives seconds after the local one. Patch the funnel every scan goes through, exit non-zero if that name ever disappears too, and assert before each picture that only the invented volumes are on screen — a check that reads the panel rather than trusting a monkeypatch, so the next rewrite of the scan cannot silently undo it.
The failure being guarded against was a patch that silently stopped patching, so a guard with no test of its own is the same shape of risk. Covers both halves: that the stub replaces the function the panel actually calls and yields a single final batch, leaving nothing to race; and that a missing — or merely non-callable — target stops the run instead of handing the panel back to the machine. The check itself is tested against a leaked network-share label, against several, and against an empty panel, which is the state before the first batch arrives and must not read as a leak.
owenpkent
left a comment
owenpkent
left a comment
There was a problem hiding this comment.
Reviewed the screenshot scan replacement and the pre-capture guard against the actual drive-panel path. No actionable defect found in this diff.
The stub now replaces the callable that _ScanTask consumes, refuses to continue if that callable disappears, and emits one final synthetic batch so a delayed network batch cannot overwrite the invented volumes. The guard is checked before both main-window captures, which matters because the watcher polls between captures.
Validation: 8 targeted tests passed on this head; the base/head diff passed git diff --check. No interactive screenshot session was launched for this review. The label-only guard is narrower than a complete comparison of displayed volume rows, but the current scan stub itself is correctly connected.
1 participant
Summary
tools/screenshots.pypatcheddrives.list_volumesto keep real volumes out of the README images. The panel's scan was later rewritten aroundscan_batches, so that assignment stopped overriding anything and just created an unused attribute — silently. The real scan has been running on every render since.Whether real drive labels and free space actually reached the committed PNGs came down to a race:
_rebuild(VOLUMES)forces the invented list in, but the scan delivers local drives first and network shares seconds later, overwriting it. I hit exactly that while regenerating — one render came out with my real volumes, including an SMB share's label and capacity.The currently committed images are clean; this is about the next person to run the tool.
What changed
drives.scan_batches— the funnel every scan goes through — and yield a single final batch, so there is no second delivery to race the first.SystemExitif that name is ever missing too, rather than quietly handing the panel back to the machine.assert_no_real_volumes()before each picture, reading the labels actually on screen. That is the part that survives the next rewrite of the scan; patching a function name did not.Committed images are untouched by this PR — it changes the tool, not the output.
Test plan
ruff check toolscleanpython tools/screenshots.py OUTrenders all three images, exit 0, only invented volumesdrives.scan_batchesdeleted the run aborts with the explanatory message instead of rendering