Three changes:
1. _make_client passes db=self._db, site_id=s.id so Gelbooru and
Moebooru clients get a CategoryFetcher attached via the factory.
2. _on_post_activated calls _ensure_post_categories_async(post)
after setting up the preview. If the post has empty categories
(background prefetch hasn't reached it yet, or cache miss),
this schedules ensure_categories on the async loop. When it
completes, it emits categories_updated via the Qt signal.
3. _on_categories_updated slot re-renders the info panel and
preview pane tag display when the currently-selected post's
categories arrive. Stale updates (user clicked a different post
before the fill completed) are silently dropped by the post.id
check.
Two more digit-stem-only callsites I missed in the saved-dot fix
sweep. _set_library_info and _show_library_post both did
'if not stem.isdigit(): return' before consulting library_meta or
building the toolbar Post. Templated files (post-template-refactor
saves like 12345_hatsune_miku.jpg) bailed out silently — clicking
one in the Library tab left the info panel showing the previous
selection's data and the toolbar actions did nothing.
Extracted a small helper _post_id_from_library_path that resolves
either layout: look up library_meta.filename first (templated),
fall back to int(stem) for legacy digit-stem files. Both call sites
go through the helper now.
Same pattern as the find_library_files / _is_post_in_library
fixes from the earlier saved-dot bug. With this commit there are
no remaining "is templated file in the library?" callsites that
fall back to digit-stem matching alone — every check is
format-agnostic via the DB.
The browse grid had the same digit-stem-only bug as the bookmark
grid: _saved_ids in two places used a root-only iterdir + isdigit
filter, missing both subfolder saves and templated filenames. The
user only reported the bookmark side, but this side has been
silently broken for any save into a subfolder for a while.
Six changes, all driven by the new db-backed helpers:
_on_load_more (browse grid append):
_saved_ids = self._db.get_saved_post_ids()
After-blacklist rebuild:
_saved_ids = self._db.get_saved_post_ids()
_is_post_saved:
return self._db.is_post_in_library(post_id)
Bookmark preview lookup find_library_files:
pass db=self._db so templated names also match
_unsave_from_preview delete_from_library:
pass db=self._db so templated names get unlinked AND meta cleaned
_bulk_unsave delete_from_library:
same fix
Fourth Phase 2 site migration. Extracts a shared _batch_download_to
helper that owns the async loop with a per-batch in_flight set, then
makes both _batch_download (the dialog-driven entry) and
_batch_download_posts (the multi-select entry) thin wrappers that
delegate to it.
Fixes the latent v0.2.3 bug where batch downloads landing inside
saved_dir() never wrote library_meta rows — _on_batch_done painted
saved-dots from disk but the search index stayed empty. The
library_meta write is now automatic via save_post_file's
is_relative_to(saved_dir()) check, so any batch into a library folder
gets indexed for free.
Also picks up filename templates and sequential collision suffixes
across batch downloads — collision-prone templates like %artist% on a
page of same-artist posts now produce someartist.jpg, someartist_1.jpg,
someartist_2.jpg instead of clobbering.
Third Phase 2 site migration. Default filename in the dialog now
comes from rendering the library_filename_template against the post,
so users see their templated name and can edit if they want. Drops
the legacy hardcoded "post_" prefix on the default — anyone who wants
the prefix can put it in the template.
The actual save still routes through save_post_file with
explicit_name set to whatever the user typed, so collision resolution
runs even on user-chosen filenames (sequential _1/_2 if the picked
name already belongs to a different post in the library).
behavior change from v0.2.3: Save As into saved_dir() now registers
library_meta. Previously Save As never wrote meta regardless of
destination. If a file is in the library it should be searchable —
this fixes that.
Second Phase 2 site migration. Hoists destination resolution out of
the per-iteration loop, uses a shared in_flight set so collision-prone
templates (%artist% on a page of same-artist posts) get sequential
suffixes instead of clobbering each other, and finally calls
_copy_library_thumb so multi-select bulk saves get library thumbnails
just like single-post saves do.
Drops the dead site_id assignment that nothing read.
Fixes the latent bug where _bulk_save left library thumbnails uncopied
even though _save_to_library always copied them — multi-select saves
were missing thumbnails in the Library tab until you re-saved one at
a time.
First Phase 2 site migration. _save_to_library shrinks from ~80 lines
to ~30 by delegating to core.library_save.save_post_file. The
"find existing copy and rename across folders" block is gone — same-
post idempotency is now handled by the DB-backed filename column via
_same_post_on_disk inside save_post_file. The thumbnail-copy block is
extracted as a new _copy_library_thumb helper so _bulk_save (Phase
2.2) can call it too.
behavior change from v0.2.3: cross-folder re-save is now copy, not
move. Old folder's copy is preserved. The atomic-rename-move was a
workaround for not having a DB-backed filename column; with
_same_post_on_disk the workaround is unnecessary. Users who want
move semantics can manually delete the old copy.
Net diff: -52 lines.
Drops every direct popout._underscore access from main_window in favor
of nine new public methods on FullscreenPreview. The legacy private
fields (_video, _viewer, _stack, _bookmark_btn, etc.) stay in place —
this is a clean public wrapper layer, not a re-architecture. Going
through public methods makes the popout's interface explicit and
prevents future code from reaching into popout internals.
New public methods on FullscreenPreview:
is_video_active() -> bool
Replaces popout._stack.currentIndex() == 1 checks. Used to gate
video-only operations.
set_toolbar_visibility(*, bookmark, save, bl_tag, bl_post)
Replaces 4-line popout._bookmark_btn.setVisible(...) etc block.
Per-tab toolbar gating.
sync_video_state(*, volume, mute, autoplay, loop_state)
Replaces 4-line popout._video.volume = ... etc block. Called by
main_window's _open_fullscreen_preview to push embedded preview
state into the popout.
get_video_state() -> dict
Returns volume / mute / autoplay / loop_state / position_ms in
one read. Replaces 5 separate popout._video.* attribute reads
in main_window's _on_fullscreen_closed reverse sync.
seek_video_to(ms)
Wraps VideoPlayer.seek_to_ms (which uses 'absolute+exact' since
the 609066c drag-back fix). Used by the seek-after-load pattern.
connect_media_ready_once(callback)
One-shot callback wiring with auto-disconnect. Replaces the
manual lambda + try/except disconnect dance in main_window.
pause_media()
Wraps VideoPlayer.pause(). Replaces 3 sites of direct
popout._video.pause() calls in privacy-screen / external-open
paths.
force_mpv_pause()
Direct mpv.pause = True without button text update. Replaces
the legacy popout._video._mpv.pause = True deep attribute access
in main_window's _on_post_activated. Used to prevent the OLD
video from reaching natural EOF during the new post's async
download.
stop_media()
Stops the video and clears the image viewer. Replaces 2 sites
of the popout._viewer.clear() + popout._video.stop() sequence
in blacklist-removal flow.
main_window.py call sites updated:
Line 1122-1130 (_on_post_activated):
popout._video._mpv.pause = True → popout.force_mpv_pause()
Line 1339-1342 (_update_fullscreen):
4 popout._*.setVisible(...) → popout.set_toolbar_visibility(...)
Line 1798, 1811, 2731:
popout._video.pause() → popout.pause_media()
Line 2151-2166 (_open_fullscreen_preview sync block):
sv = popout._video; sv.volume = ...; ...
+ manual seek-when-ready closure
→ popout.sync_video_state(...) + popout.connect_media_ready_once(...)
Line 2196-2207 (_on_fullscreen_closed reverse sync):
sv = popout._video; pv.volume = sv.volume; ...; popout._stack.currentIndex...
→ popout.get_video_state() returning a dict
Line 2393-2394, 2421-2423 (blacklist removal):
popout._viewer.clear() + popout._video.stop()
→ popout.stop_media()
After this commit, main_window has ZERO direct popout._underscore
accesses. The popout's public method surface is the only way for
main_window to interact with the popout's internals.
The popout's public method surface is now:
Lifecycle:
- set_media (existing — keeps the kind, info, width, height contract)
- update_state (existing — bookmarked/saved button labels)
- close (Qt builtin — triggers closeEvent)
Wiring:
- set_post_tags
- set_bookmark_folders_callback
- set_folders_callback
Privacy:
- privacy_hide / privacy_show (existing)
New in commit 15:
- is_video_active
- set_toolbar_visibility
- sync_video_state
- get_video_state
- seek_video_to
- connect_media_ready_once
- pause_media
- force_mpv_pause
- stop_media
Outbound signals (unchanged from refactor start):
- navigate / play_next_requested / closed
- bookmark_requested / bookmark_to_folder
- save_to_folder / unsave_requested
- blacklist_tag_requested / blacklist_post_requested
- privacy_requested
Tests passing after this commit: 81 / 81 (16 Phase A + 65 state).
Phase A still green.
Verification:
- Imports clean
- Pure-Python state machine + tests unchanged
- main_window's popout interaction goes through public methods only
Test cases for commit 16 (final shim cleanup):
- Drop the hyprland re-export shim methods from popout/window.py
- Have callers use popout.hyprland directly
Note #3 in REFACTOR_NOTES.md (search result count + end-of-results
flag mismatch) reproduced once during the refactor verification sweep
and not again at later commits, so it's intermittent — likely
scenario-dependent (specific tag, blacklist hit rate, page-size /
limit interaction). The bug is real but not reliably repro-able, so
the right move is to add logging now and capture real data on the
next reproduction instead of guessing at a fix.
Both _do_search (paginated) and _on_reached_bottom (infinite scroll
backfill) now log a `do_search:` / `on_reached_bottom:` line with the
following fields:
- limit the configured page_size
- api_returned_total raw count of posts the API returned across all
fetched pages (sum of every batch the loop saw)
- kept post-filter, post-clamp count actually emitted
- drops_bl_tags posts dropped by the blacklist-tags filter
- drops_bl_posts posts dropped by the blacklist-posts filter
- drops_dedup posts dropped by the dedup-against-seen filter
- api_short_signal (do_search only) whether the LAST batch came
back smaller than limit — the implicit "API ran
out" hint
- api_exhausted (on_reached_bottom only) the explicit
api_exhausted flag the loop sets when len(batch)
falls short
- last_page (on_reached_bottom only) the highest page index
the backfill loop touched
_on_search_done also gets a one-liner with displayed_count, limit,
and the at_end decision so the user-visible "(end)" flag can be
correlated with the upstream numbers.
Implementation note: the per-filter drop counters live in a closure-
captured `drops` dict that the `_filter` closure mutates as it walks
its three passes (bl_tags → bl_posts → dedup). Same dict shape in
both `_do_search` and `_on_reached_bottom` so the two log lines are
directly comparable. Both async closures also accumulate `raw_total`
across the loop iterations to capture the API's true return count,
since the existing locals only kept the last batch's length.
All logging is `log.debug` so it's off at default INFO level. To
capture: bump booru_viewer logger level (or run with debug logging
enabled in main_window.py:440 — already DEBUG by default per the
existing setLevel call).
This commit DOES NOT fix#3 — the symptom is still intermittent and
the root cause is unknown. It just makes the next reproduction
diagnosable in one shot instead of requiring a second instrumented
run.
Two related improvements to the Ctrl+P privacy screen flow.
1. Resume video on un-hide
Pre-fix: Ctrl+P paused any playing video in the embedded preview and
the popout, but the second Ctrl+P only hid the privacy overlay — the
videos stayed paused. The user had to manually click Play to resume.
Fix: in _toggle_privacy's privacy-off branch, mirror the privacy-on
pause logic with resume() calls on the embedded preview's video player
and the popout's video. Unconditional resume — if the user manually
paused before Ctrl+P, the auto-resume on un-hide is a tiny annoyance,
but the common case (privacy hides → user comes back → video should
be playing again) wins.
2. Popout privacy uses an in-place overlay instead of hide()
Pre-fix attempt: privacy-on called self._fullscreen_window.hide() and
privacy-off called .show(). On Wayland (Hyprland) the hide→show round
trip drops the window's position because the compositor unmaps the
window on hide and remaps it at the default tile position on show.
A first attempt at restoring the position via a deferred
hyprctl_resize_and_move dispatch in privacy_show didn't take — by
the time the dispatch landed, the window had already been re-tiled
and the move was gated by `if not win.get("floating"): return`.
Cleaner fix: don't hide the popout window at all. FullscreenPreview
gains its own _privacy_overlay (a black QWidget child of central,
parallel to the existing toolbar / controls bar children) that
privacy_hide raises over the media stack. The popout window stays
mapped, position is preserved automatically because nothing moves,
and the overlay covers the content visually.
privacy_hide / privacy_show methods live in FullscreenPreview, not
in main_window — popout-internal state belongs to the popout module.
_toggle_privacy in main_window just calls them. This also makes
adding more popout-side privacy state later (e.g. fullscreen save)
a one-method change inside the popout class.
Also added a _popout_was_visible flag in BooruApp._toggle_privacy so
privacy-off only restores the popout if it was actually visible at
privacy-on time. Without the gate, privacy-off would inappropriately
re-show a popout the user had closed before triggering privacy.
Verified manually:
- popout open + drag to non-default pos + Ctrl+P + Ctrl+P → popout
still at the dragged position, content visible again
- popout open + video playing + Ctrl+P + Ctrl+P → video resumes
- popout closed + Ctrl+P + Ctrl+P → popout stays closed
- embedded preview video + Ctrl+P + Ctrl+P → resumes
- Ctrl+P with no video on screen → no errors
Pax requested the keyboard shortcut be removed — too easy to fat-finger
when navigating with the keyboard, and "Open in Default App" still
ships an external process that may steal focus from the app. The
right-click menu's Open in Default App action stays, both on browse
thumbnails and in the preview pane right-click — only the bare-key
shortcut goes away.
The deleted block was the only Key_O handler in BooruApp.keyPressEvent,
so no other behavior changes.
Verified manually:
- press O on a selected thumbnail → nothing happens
- right-click thumbnail → "Open in Default App" still present and opens
- right-click preview pane → same
Two related fixes for the File → Batch Download Page (Ctrl+D) flow.
1. Saved-dot refresh
Pre-fix: when the user picked a destination inside the library, the
batch wrote files to disk but the browse grid's saved-dots stayed dark
until the next refresh. The grid was lying about local state.
Fix: stash the chosen destination as self._batch_dest at the dispatch
site, then in _on_batch_progress (which already fires per-file via
the existing batch_progress signal) check whether dest is inside
saved_dir(); if so, find the just-finished post in self._posts by id
and light its grid thumb's saved-locally dot. Dots appear incrementally
as each file lands, not all at once at the end.
The batch_progress signal grew a third int param (post_id of the
just-finished item). It's a single-consumer signal — only
_on_batch_progress connects to it — so the shape change is local.
Both batch download paths (the file menu's _batch_download and the
multi-select menu's _batch_download_posts) pass post.id through.
When the destination is OUTSIDE the library, dots stay dark — the
saved-dot means "in library", not "downloaded somewhere". The check
uses Path.is_relative_to (Python 3.11+).
self._batch_dest is cleared in _on_batch_done after the batch finishes
so a subsequent non-batch save doesn't accidentally see a stale dest.
2. Tab gating
Pre-fix: File → Batch Download Page... was enabled on Bookmarks and
Library tabs, where it makes no sense (those tabs already show local
files). Ctrl+D fired regardless of active tab.
Fix: store the QAction as self._batch_action instead of a local var
in _setup_menu, then toggle setEnabled(index == 0) from _switch_view.
Disabling the QAction also disables its keyboard shortcut, so Ctrl+D
becomes a no-op on non-browse tabs without a separate guard.
Verified manually:
- Browse tab → menu enabled, Ctrl+D works
- Bookmarks/Library tabs → menu grayed out, Ctrl+D no-op
- Batch dl into ~/.local/share/booru-viewer/saved → dots light up
one-by-one as files land
- Batch dl into /tmp → files written, dots stay dark
The infinite-scroll backfill loop in _on_reached_bottom accumulates
results from up to 9 follow-up API pages until len(collected) >= limit,
but the break condition is >= not ==, so the very last full batch
would push collected past the configured page_size. The non-infinite
search path in _do_search already slices collected[:limit] before
emitting search_done at line 805 — the infinite path was emitting the
unclamped list. Result: a single backfill round occasionally appended
more than page_size posts, producing irregular batch sizes the user
could see.
Fix: one-character change at the search_append.emit call site to mirror
the non-infinite path's slice.
Why collected[:limit] over the alternative break-early-with-clamp:
1. Consistency — the non-infinite path in _do_search already does
the same slice before emit. One pattern, both branches.
2. Trivially fewer lines than restructuring the loop break.
3. The slight wasted download work (the over-fetched final batch is
already on disk by the time we slice) is acceptable. It's at most
one extra page's worth, only happens at the boundary, only on
infinite scroll, and the next backfill round picks up from where
the visible slice ends — nothing is *lost*, just briefly redundant.
Verified manually on a high-volume tag with infinite scroll enabled
and page_size=40: pre-fix appended >40 posts in one round, post-fix
appended exactly 40.
The browse grid's multi-select right-click menu collapsed library and
bookmark actions into a single "Remove All Bookmarks" entry that did
*both* — it called delete_from_library and remove_bookmark per post,
and was unconditionally visible regardless of selection state. Two
problems:
1. There was no way to bulk-unsave files from the library without
also stripping the bookmarks. Saved-but-not-bookmarked posts had
no bulk-unsave path at all.
2. The single misleadingly-named action didn't match the single-post
right-click menu's clean separation of "Save to Library / Unsave
from Library" vs. "Bookmark as / Remove Bookmark".
Reshape: split into four distinct actions, each with symmetric
conditional visibility:
- Save All to Library → shown only if any post is unsaved
- Unsave All from Library → shown only if any post is saved (NEW)
- Bookmark All → shown only if any post is unbookmarked
- Remove All Bookmarks → shown only if any post is bookmarked
Mixed selections show whichever subset of the four is relevant. The
new Unsave All from Library calls a new _bulk_unsave method that
mirrors the _bulk_save shape but synchronously (delete_from_library
is a filesystem op, no httpx round-trip). Remove All Bookmarks now
*only* removes bookmarks — it no longer touches the library, matching
the single-post Remove Bookmark action's scope.
Always-shown actions (Download All, Copy All URLs) stay below a
separator at the bottom.
Verified:
- Multi-select unbookmarked+unsaved posts → only Save All / Bookmark All
- Multi-select saved-not-bookmarked → only Unsave All / Bookmark All
- Multi-select bookmarked+saved → only Unsave All / Remove All Bookmarks
- Mixed selection → all four appear
- Unsave All from Library removes files, leaves bookmarks
- Remove All Bookmarks removes bookmarks, leaves files
Final commit of the gui/app.py + gui/preview.py structural refactor.
Updates the four call sites that were importing through the
preview.py / app.py shims to import from each entity's canonical
sibling module instead, then deletes the now-empty shim files.
Edits:
- main_gui.py:38 from booru_viewer.gui.app import run
→ from booru_viewer.gui.app_runtime import run
- main_window.py:44 from .preview import ImagePreview
→ from .preview_pane import ImagePreview
- main_window.py:1133 from .preview import VIDEO_EXTENSIONS
→ from .media.constants import VIDEO_EXTENSIONS
- main_window.py:2061 from .preview import FullscreenPreview
→ from .popout.window import FullscreenPreview
- main_window.py:2135 from .preview import FullscreenPreview
→ from .popout.window import FullscreenPreview
Deleted:
- booru_viewer/gui/app.py
- booru_viewer/gui/preview.py
Final gui/ tree:
gui/
__init__.py (unchanged, empty)
app_runtime.py entry point + style loader
main_window.py BooruApp QMainWindow
preview_pane.py ImagePreview embedded preview
info_panel.py InfoPanel widget
log_handler.py LogHandler (Qt-aware logger adapter)
async_signals.py AsyncSignals signal hub
search_state.py SearchState dataclass
media/
__init__.py
constants.py VIDEO_EXTENSIONS, _is_video
image_viewer.py ImageViewer (zoom/pan)
mpv_gl.py _MpvGLWidget, _MpvOpenGLSurface
video_player.py VideoPlayer + _ClickSeekSlider
popout/
__init__.py
viewport.py Viewport NamedTuple, _DRIFT_TOLERANCE
window.py FullscreenPreview popout window
grid.py, bookmarks.py, library.py, search.py, sites.py,
settings.py, dialogs.py (all untouched)
Net result for the refactor: 2 god-files (app.py 3608 lines +
preview.py 2273 lines = 5881 lines mixing every concern) replaced
by 12 small clean modules + 2 oversize-by-design god-class files
(main_window.py and popout/window.py — see docs/REFACTOR_PLAN.md
for the indivisible-class rationale).
Followups discovered during execution are recorded in
docs/REFACTOR_NOTES.md (gitignored, local-only).
Step 12 of the gui/app.py + gui/preview.py structural refactor — the
biggest single move out of app.py. The entire ~3020-line BooruApp
QMainWindow class moves to its own module under gui/. The class body
is byte-identical: every method, every signal connection, every
private attribute access stays exactly as it was.
main_window.py imports the helper classes that already moved out of
app.py (SearchState, LogHandler, AsyncSignals, InfoPanel) directly
from their canonical sibling modules at the top of the file, so the
bare-name lookups inside BooruApp method bodies (`SearchState(...)`,
`LogHandler(self._log_text)`, `AsyncSignals()`, `InfoPanel()`) keep
resolving to the same class objects. Same package depth as app.py
was, so no relative-import depth adjustment is needed for any of
the lazy `..core.X` or `.preview` imports inside method bodies —
they keep working through the preview.py shim until commit 14
swaps them to canonical paths.
app.py grows the BooruApp re-export shim line. After this commit
app.py is just imports + log + the four helpers (run,
_apply_windows_dark_mode, _load_user_qss, _BASE_POPOUT_OVERLAY_QSS)
+ the shim block. Commit 13 carves the helpers out, commit 14
deletes the shims and the file.
VERIFICATION: full method-cluster sweep (see docs/REFACTOR_PLAN.md
"Commit 12 expanded verification" section), not the 7-item smoke test.
