Fix viewer stuck image, onefile cache prune, status bar scope (0.6.2) #172

Merged

ValleyGeek merged 14 commits from 0.6.1/v0.7.0 into main 2026-08-22 04:37:18 +00:00

Conversation 0 Commits 14 Files changed 14 +701 -198

Claude commented 2026-08-22 04:35:05 +00:00

Member

Copy link

Summary

Batches three issues into one release:

• Fixes #163 — the image viewer no longer gets stuck showing a previous directory's image after closing a directory and opening another. Two real, distinct bugs were found and fixed in the course of this: closing a directory from inside the viewer wasn't returning to the grid, and (the actual root cause of the persisting report) _release_workspace_db_locks() was calling viewer.shutdown() unconditionally — permanently deactivating the viewer's image-loader signals on the first directory close of any session, silently breaking every future image load for the rest of that session. See DR-040/DR-041 for the full investigation.
• Fixes #167 — Windows onefile builds now reliably prune unpack folders from older versions/builds under %LOCALAPPDATA%\ValleyGeek\Bulk Image Organizer\. Root cause: is_frozen_runtime()/_running_packaged() used import __compiled__ to detect a Nuitka-compiled runtime, but Nuitka injects __compiled__ as a per-module global variable, not an importable module — that import always raised ImportError, even in a real compiled build, so pruning never actually ran. See DR-038/DR-039.
• Fixes #168 — the status bar now reports only in-flight background work (scans, duplicate scans, thumbnail generation, sort into folders, confirm delete, network cache sync) plus a brief completion/cancel/fail line, instead of doubling as a general navigation caption that could stomp an in-progress background status.

Also includes a reverted speculative fix (a Windows send2trash COM-backend change for a one-time, unreproduced crash report) — reverted per direction once it was confirmed unrelated to any code on this branch; issue #170 is left open as a placeholder in case it recurs, with no code change.

VERSION bumped 0.6.1 → 0.6.2 (patch, per docs/VERSIONING.md default — none of this batch is a new capability tranche).

Test plan

• Full test suite passes locally (711 passed, 1 skipped)
• ruff check / ruff format --check clean
• ./scripts/smoke-ui-exit.sh clean (no Qt teardown errors)
• Linux Nuitka --standalone smoke build verified the __compiled__ detection fix against a real compiled binary (not just documentation)
• Issue #167 fix confirmed working by the reporter against a real Windows onefile build (log showed only the current build's own directory remaining)
• Issue #163 fix is pending the reporter's confirmation on a real Windows build (root cause confirmed via live instrumentation of the real async load pipeline + a non-mocked regression test using real files through the actual background thread pool)

🤖 Generated with Claude Code

https://claude.ai/code/session_016gxFoGEVBb1Hv3nfKwkYf7

## Summary Batches three issues into one release: - **Fixes #163** — the image viewer no longer gets stuck showing a previous directory's image after closing a directory and opening another. Two real, distinct bugs were found and fixed in the course of this: closing a directory from inside the viewer wasn't returning to the grid, and (the actual root cause of the persisting report) `_release_workspace_db_locks()` was calling `viewer.shutdown()` unconditionally — permanently deactivating the viewer's image-loader signals on the *first* directory close of any session, silently breaking every future image load for the rest of that session. See DR-040/DR-041 for the full investigation. - **Fixes #167** — Windows onefile builds now reliably prune unpack folders from older versions/builds under `%LOCALAPPDATA%\ValleyGeek\Bulk Image Organizer\`. Root cause: `is_frozen_runtime()`/`_running_packaged()` used `import __compiled__` to detect a Nuitka-compiled runtime, but Nuitka injects `__compiled__` as a per-module global variable, not an importable module — that import always raised `ImportError`, even in a real compiled build, so pruning never actually ran. See DR-038/DR-039. - **Fixes #168** — the status bar now reports only in-flight background work (scans, duplicate scans, thumbnail generation, sort into folders, confirm delete, network cache sync) plus a brief completion/cancel/fail line, instead of doubling as a general navigation caption that could stomp an in-progress background status. Also includes a reverted speculative fix (a Windows send2trash COM-backend change for a one-time, unreproduced crash report) — reverted per direction once it was confirmed unrelated to any code on this branch; issue #170 is left open as a placeholder in case it recurs, with no code change. VERSION bumped 0.6.1 → 0.6.2 (patch, per `docs/VERSIONING.md` default — none of this batch is a new capability tranche). ## Test plan - [x] Full test suite passes locally (711 passed, 1 skipped) - [x] `ruff check` / `ruff format --check` clean - [x] `./scripts/smoke-ui-exit.sh` clean (no Qt teardown errors) - [x] Linux Nuitka `--standalone` smoke build verified the `__compiled__` detection fix against a real compiled binary (not just documentation) - [x] Issue #167 fix confirmed working by the reporter against a real Windows onefile build (log showed only the current build's own directory remaining) - [x] Issue #163 fix is pending the reporter's confirmation on a real Windows build (root cause confirmed via live instrumentation of the real async load pipeline + a non-mocked regression test using real files through the actual background thread pool) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016gxFoGEVBb1Hv3nfKwkYf7

Claude added 14 commits 2026-08-22 04:35:06 +00:00

fix(viewer): reload image when reopening session with matching filename ... 073b0856a4

open_session() cleared display/size/fit caches but never reset
_current_rel_path or _current_meta, so _go_to_session_index()'s
"already showing this image" fast path could fire on the very first
navigation of a freshly opened session whenever the new directory's
image at the same session index happened to share a filename with
whatever was last displayed (e.g. reused test images or common camera
default names like IMG_0001.jpg). The stale pixmap from the previous
directory then stayed on screen instead of loading the new image.

Bumps VERSION to the dev form for issue #163 (Kind/Bug).

Threading notes: no worker changes; open_session/_go_to_session_index
run entirely on the main thread.

Fixes #163

fix(packaging): prune all stale Windows onefile unpack dirs, not just "+"-named ones ... 4535ba4122

prune_stale_onefile_caches() only removed sibling cache folders whose name
contained "+" (the current "<VERSION>+<build-id>" convention), so legacy
bare-"<VERSION>" extract dirs from older builds were never cleaned up and
AppData\Local\ValleyGeek\Bulk Image Organizer kept growing across upgrades.

Now every directory under the product cache folder other than the active
unpack is removed, regardless of naming. The active unpack is still never
touched, and a sibling that fails to delete (e.g. locked by AV) is logged
and skipped rather than aborting startup — unchanged from before. Workspace
caches and app settings live under separate AppData paths and are untouched.

Threading notes: prune_stale_onefile_caches() runs synchronously on the main
thread during startup before any Qt objects exist; no worker threads involved.

Bumps VERSION to the dev form for issue #167 (Kind/Bug).

Fixes #167

fix(ui): scope the status bar to background operations only ... 2bdbb0824f

self.status_label was doubling as a general UI caption: workbench-switch
names, viewer key-hint text, sort/tag-filter/search descriptions, dupe
group-selection captions, and instant tag/dupe-action confirmations all
wrote it, frequently stomping an in-progress background status (scan,
dupe scan, thumbnail generation, sort, delete) with unrelated navigation
text.

status_label and progress_bar now report only in-flight background work
(directory scan, dupe scan, thumbnail generation, sort into folders,
confirm delete/send2trash, network workspace cache sync, and the
rotation/crop-write thumbnail refresh) plus a brief completion/cancel/fail
line for whichever just finished. Idle state (including at startup) is
empty — no restored workbench name, viewer shortcut hints, or "Ready —
open a directory" coaching text. The centered image-count widget is
unrelated and unchanged.

Also drops a few now-dead parameters/locals left behind once their only
use (building removed status text) was gone: the unused cache_deleted
flag on _finalize_workspace_close, the removed/kind params on
_after_dupe_tags_bulk_removed (private, no external callers), the
groups_affected counter in the bulk dupe-actions handler, and
DupeManagerWorkbench.status_text() (now unused everywhere). The
_activate_viewer_sub context parameter is kept even though its only
former use (the viewer key-hint caption) is gone, since ~29 existing
tests call it positionally.

Bumps VERSION to the dev form for issue #168 (Kind/Enhancement).

Threading notes: no worker changes; every edited call site already ran
on the main thread (Qt slots / direct UI-event handlers).

Fixes #168

Merge issue-163 (stale viewer image fix) into v0.7.0 batch branch ca026163a1

Merge issue-167 (onefile cache prune fix) into v0.7.0 batch branch aeb85d02e0

Merge issue-168 (status bar scope fix) into v0.7.0 batch branch a123ed16a4

fix(core): force send2trash's legacy backend on Windows, never COM IFileOperation ... a434c9be87

Reported (in chat, not yet reproduced by the user on retry): on Windows,
confirming a delete (specifically File Duplicate Bulk Actions' send-to-trash
path) sent files to the recycle bin but then crashed the app before
_on_delete_finished could show a summary or return control to the UI.

send2trash auto-selects a COM-based IFileOperation backend
(send2trash.win.modern) on Windows whenever pywin32 happens to be
importable — not a direct dependency here, but nothing guarantees its
absence in a given build environment. DeleteWorker calls this once per
file entirely inside a QThreadPool worker thread; the COM backend's
per-call CoInitialize()/CoCreateInstance()/IFileOperationProgressSink
sequence expects a Windows message pump that QThreadPool worker threads
never provide — a well-known native-instability hazard that would bypass
DeleteWorker.run()'s except Exception entirely, matching the report and
this project's own DR-027/DR-028 precedent (native crash, no Python
traceback).

delete_worker.py now forces the ctypes/SHFileOperationW legacy backend
on sys.platform == "win32" instead of send2trash's own auto-selection.
macOS/Linux unaffected. See DR-037 for full reasoning; this is grounded
in reading the installed send2trash source and the single call site in
this codebase, not a confirmed Windows crash log — awaiting the user's
confirmation on a subsequent Windows session.

Threading notes: no change to what DeleteWorker does off the main thread
(still IO + plain-data signals only) — only which underlying send2trash
implementation it calls into on Windows.

Fixes #170

diag(core): log every onefile prune decision at INFO, no behavior change ... eb144463b8

Reported: %LOCALAPPDATA%\ValleyGeek\Bulk Image Organizer still held 11
extract directories spanning 0.4.0 through two consecutive 0.6.1/v0.7.0
builds three hours apart. Every listed folder name already contains "+",
including ones from before issue #167's fix (which only required "+" in
the name) — so that fix cannot have been the operative bug. Pruning has
evidently never deleted anything on a real Windows run, before or after.

prune_stale_onefile_caches() has several early-return guards ahead of its
deletion loop, and packaged builds log at INFO while most of these guards
previously logged at DEBUG or not at all — so the existing log file could
not distinguish "a guard short-circuited" from "ran but every deletion
failed" from "never called." Patching a guess at one of several guards
(or the bundle_root() tie-break when multiple never-pruned siblings all
still satisfy _windows_onefile_unpack_roots()) risks repeating issue #167:
a plausible-looking fix that ships without confirming it addressed the
actual cause.

Every branch of _is_onefile_windows_runtime(), _is_managed_cache_parent(),
and prune_stale_onefile_caches() now logs at INFO: the sys.argv[0]-derived
exe_parent vs bundle_root()-derived unpack_root used for onefile detection,
which _is_managed_cache_parent() sub-check failed and why, the
BIO_SKIP_ONEFILE_CACHE_PRUNE short-circuit, and — when the deletion loop
is reached — the resolved current dir, cache parent, and full sibling
list before attempting deletion. No behavior change; this is purely
diagnostic, matching the DR-027 checkpoint-logging pattern this project
already uses for platform-specific failures that can't be reproduced
locally. See DR-038.

Root cause not yet confirmed — next step is the user sharing the
"Onefile cache prune" log lines from bulk-image-organizer.log after a
subsequent Windows run.

Refs #171

docs: reference issue #167 instead of a separate #171 for prune diagnostics ... eab99c6749

The previous commit's issue references pointed at #171, opened in error —
this diagnostic work is a continuation of the still-open #167 (the user's
fresh Windows test showed that issue's fix didn't work, not a new bug).
Fixes issue numbers in the diagnostic log docstring, the new tests, and
DR-038 to point at #167. #171 is being closed as a duplicate.

No functional change.

Revert "fix(core): force send2trash's legacy backend on Windows, never COM IFileOperation" ... 3ccdad046d

This reverts commit a434c9be87.

No code was ever confirmed changed by this branch in the delete-completion
path (verified by diffing #168 against it), and the crash has not recurred
or been reproduced — treating it as a one-time issue on the reporter's
system rather than an application bug. Reverting the speculative send2trash
backend change, its regression test, and DR-037 accordingly.

Issue #170 stays open as a placeholder to track recurrence, but carries no
code change on this branch.

fix(core): detect Nuitka __compiled__ as a module global, not an import ... ed41f3a21d

Root cause confirmed via the reporter's own Windows log line (issue #167):
"Onefile cache prune skipped: is_frozen_runtime() is False" — on a real
packaged onefile .exe. That single line explains every symptom seen across
both rounds of this issue regardless of folder naming, since it means the
deletion loop was never reached at all.

is_frozen_runtime() (resource_paths.py) and its independently-duplicated
twin _running_packaged() (logging_config.py) both fell back to
`try: import __compiled__ except ImportError: return False` whenever
sys.frozen isn't set — and this project's real Windows builds evidently
never set sys.frozen (the docstrings already said as much). Nuitka does
not register a real importable module named __compiled__; it injects
__compiled__ as a plain global variable into the namespace of every module
it compiles. The documented idiom is `"__compiled__" in globals()`.
`import __compiled__` as a statement therefore raises ImportError even
inside a genuine compiled build, which is exactly why this always
returned False. The existing test suite never caught this because every
test simulating a frozen run set sys.frozen = True directly (bypassing the
buggy branch) or mocked __compiled__ as a sys.modules entry — the tests
encoded the same wrong mental model as the code.

Both functions now check `"__compiled__" in globals()` instead.
_nuitka_compiled_anchors() has the same mistake but is deliberately left
unfixed this round: it isn't implicated by any confirmed symptom (the
other bundle_root() anchors already resolve resources correctly in
production), and its `.containing_dir`/`.static_src` attribute assumptions
aren't independently confirmed the way the `globals()` idiom is documented.

Verified against a real compiled binary, not just documentation: built a
Linux Nuitka --standalone smoke build (./scripts/build-linux-nuitka-smoke.sh)
and ran it headlessly. It wrote to the packaged log path
(~/.config/bulk-image-organizer/logs/, INFO-only, no dev fallback) instead
of the dev temp log — confirming _running_packaged() (same fix pattern)
genuinely returns True in a real compiled process.

Existing tests that mocked __compiled__ via sys.modules were updated to
also inject it as a real per-module global (matching actual Nuitka
behavior). New regression tests simulate the exact confirmed failure shape
(no sys.frozen, no sys.modules entry, __compiled__ only as a module
global) and were verified to fail without this fix and pass with it.

See DR-039 for full reasoning.

Refs #167

fix(ui): close directory while in viewer must return to the grid ... f834e80fbf

Reported: "Closing a directory and deleting cache is keeping the last
viewed image in the viewer" — a different shape of issue #163 than the
one already fixed on this branch (commit 073b085).

_finalize_workspace_close() force-set self._in_viewer_sub = False directly,
then called _reset_workspace_to_grid() — which reads that exact flag to
decide whether workbench_stack needs to switch away from the viewer. Since
the flag was already cleared, _reset_workspace_to_grid()'s was_in_viewer
was always False, so its _activate_workbench("grid") call (the only thing
that actually swaps the visible widget) never fired whenever
self._current_mode was already "grid" — the common case, since entering
the viewer doesn't change _current_mode. The internal flag said "not in
viewer" but workbench_stack was still showing the viewer widget with
whatever image was last displayed.

open_directory() (the open-a-new-directory path) calls the same
_reset_workspace_to_grid() without touching _in_viewer_sub first, which is
why it was never broken (test_open_directory_closes_viewer already passed).

Removed the premature assignment so _reset_workspace_to_grid() observes
the true prior state. New regression test (modeled on
test_open_directory_closes_viewer) verified to fail without this fix
(currentWidget() was still ImageViewerSub) and pass with it. See DR-040.

Threading notes: no worker changes; this is main-thread widget-stack
bookkeeping only.

Refs #163

fix(ui): stop closing a directory from permanently killing viewer loads ... 7c4d718550

Confirmed via live instrumentation of the real async load pipeline, not a
guess. The user retested DR-040's fix with a real Windows log and reported
the viewer stayed frozen on a prior directory's image indefinitely, even
across many navigation attempts in a second directory with zero filename
overlap with the first -- ruling out every cache-key-collision theory
already fixed on this branch.

_release_workspace_db_locks() unconditionally called self.viewer.shutdown(),
which sets ViewerImageSignals._active = False on the viewer's loader
signals with no way to ever reactivate them. shutdown()'s own docstring
says "before window teardown", and it's correctly called from
MainWindow.closeEvent() for that -- but _release_workspace_db_locks() is
*also* called from _finalize_workspace_close(), which runs on every
routine "Close Directory and Delete Cache", not just app exit.
ViewerImageLoader.run()'s first line is `if not self._signals.is_active():
return` -- once flipped, every future image load, for any future
directory, silently no-ops for the rest of the session. The first
directory close in any session permanently kills the viewer.

Removed the shutdown() call from _release_workspace_db_locks() and added
it directly to closeEvent() instead, after that helper already drains
QThreadPool via waitForDone() -- real app teardown behaves identically,
routine directory closes no longer touch it.

Also hardened ImageViewerSub._on_image_failed() to clear the displayed
pixmap on a failed load rather than silently leaving whatever was on
screen before (defense in depth, found while narrowing this down).

New regression test uses two real, distinctly-sized JPEG files through the
actual QThreadPool-backed load pipeline (not a synchronous stand-in) --
verified to fail on the pre-fix code with the literal reported symptom and
pass with the fix. smoke-ui-exit.sh re-confirmed real app close still
tears down cleanly. See DR-041 for the full investigation, including why
two prior fixes on this branch (073b085, DR-040) were each individually
correct but insufficient.

Threading notes: no worker-side changes; shutdown() still runs on the main
thread, just from one caller instead of two, after QThreadPool.waitForDone()
already guarantees no worker is still using the signals it deactivates.

Refs #163

docs: PR-prep for 0.6.2 (issues #163, #167, #168) ... 1cfe8edbcb

Bump VERSION to the next release per compiled-inputs-changed.sh (patch,
per docs/VERSIONING.md default -- none of this batch's changes are a new
feature capability tranche, they're bug fixes plus a status-bar scope
cleanup). Add the 0.6.2 CHANGELOG entry. USER_GUIDE.md and README.md were
already updated for the onefile cache and status bar changes in their
respective commits on this branch and needed no further changes.

ValleyGeek merged commit ca51bc34b8 into main 2026-08-22 04:37:18 +00:00

ValleyGeek deleted branch 0.6.1/v0.7.0 2026-08-22 04:37:19 +00:00

ValleyGeek referenced this pull request from a commit 2026-08-22 04:37:21 +00:00

Fix viewer stuck image, onefile cache prune, status bar scope (0.6.2) (#172)

Claude referenced this pull request from a commit 2026-08-22 05:44:49 +00:00

Correct release version: 0.6.2 -> 0.7.0

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

Kind/Bug

Something isn't working as expected

  

Kind/Feature

New capability or user-requested feature

  

Priority/High

High priority

  

Priority/Medium

Medium priority

  

Reviewed/Confirmed

Bug or feature report confirmed valid

  

Compat/Breaking

Breaking change that won't be backward compatible

  

Kind/Bug

Something is not working

  

Kind/Documentation

Documentation changes

  

Kind/Enhancement

Improve existing functionality

  

Kind/Feature

New functionality

  

Kind/Security

This is security issue

  

Kind/Testing

Issue or pull request related to testing

  

Priority

Critical

The priority is critical

  

Priority

High

The priority is high

  

Priority

Low

The priority is low

  

Priority

Medium

The priority is medium

  

Reviewed

Confirmed

Issue has been confirmed

  

Reviewed

Duplicate

This issue or pull request already exists

  

Reviewed

Invalid

Invalid issue

  

Reviewed

Won't Fix

This issue won't be fixed

  

Status

Abandoned

Somebody has started to work on this but abandoned work

  

Status

Blocked

Something is blocking this issue or pull request

  

Status

Need More Info

Feedback is required to reproduce issue or to continue work

No labels

Kind/Bug

Kind/Feature

Priority/High

Priority/Medium

Reviewed/Confirmed

Compat/Breaking

Kind/Bug

Kind/Documentation

Kind/Enhancement

Kind/Feature

Kind/Security

Kind/Testing

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Reviewed

Confirmed

Reviewed

Duplicate

Reviewed

Invalid

Reviewed

Won't Fix

Status

Abandoned

Status

Blocked

Status

Need More Info

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

ai-collab/bulk-image-organizer!172

No description provided.