When a category fetch is about to fire (Rule34/Safebooru.org/
Moebooru on first click), the info panel was rendering the full
flat tag list, then ~200ms later re-rendering with categorized
tags. The re-layout from flat→categorized looked like a visual
hitch.
Fix: new _categories_pending flag on InfoPanel. When True, the
flat-tag fallback branch is skipped — the tags area stays empty
until categories arrive and render in one clean pass.
_ensure_post_categories_async sets _categories_pending = True
before scheduling the fetch (or False if no fetcher = Danbooru)
_on_categories_updated clears _categories_pending = False
Visual result:
Danbooru/e621: instant (inline, no flag)
Gelbooru with auth: instant (background prefetch beat the click)
Rule34/SB.org/Moebooru: empty ~200ms → categories appear cleanly
(no flat→categorized re-layout)
When _batch_api_works is True (Gelbooru proper with auth, persisted
from a prior session's probe), search() fires prefetch_batch in the
background. The batch tag API covers the entire page's tags in 1-2
requests during the time between grid render and user click — the
cache is warm before the info panel opens, so categories appear
instantly with no flash of flat tags.
Gated on _batch_api_works is True (not None, not False):
- Gelbooru proper: prefetches (batch API known good)
- Rule34: skips (batch_api_works = False, persisted)
- Safebooru.org: skips (no auth → fetcher skips batch capability)
Rule34 / Safebooru.org / Moebooru stay on-demand: the ~200ms
per-click HTML scrape is unavoidable for those sites because their
only path is per-post page fetching, which can't be batched.
The Library tab's single-delete and multi-delete context menu
actions called .unlink() directly, bypassing delete_from_library
entirely. They only extracted post_id from digit-stem filenames
(int(stem) if stem.isdigit()), so templated files like
artist_12345.jpg got deleted from disk but left orphan
library_meta rows that made get_saved_post_ids lie forever.
Fix: resolve post_id via db.get_library_post_id_by_filename first
(handles templated filenames), fall back to int(stem) for legacy
digit-stem files, then call db.remove_library_meta(post_id) after
unlinking. Both single-delete and multi-delete paths are fixed.
This was the last source of orphan library_meta rows. With this
fix + the earlier delete_from_library cleanup, every deletion
path in the app now cleans up its meta row:
- Library tab single delete (this commit)
- Library tab multi delete (this commit)
- Browse/preview "Unsave from Library" (via delete_from_library)
- Browse multi-select "Unsave All" (via delete_from_library)
- Bookmarks "Unsave from Library" (via delete_from_library)
- Bookmarks multi-select "Unsave All" (via delete_from_library)
The primary search result handler (_on_search_done) was still using
the old filesystem walk + stem.isdigit() filter to build the saved-
post-id set. The two other call sites (_on_load_more and the
blacklist rebuild) were fixed in the earlier saved-dot sweep but
this one was missed. Templated filenames like artist_12345.jpg
were invisible, so the saved-dot disappeared after any grid
rebuild (new search, page change, etc).
Fix: use self._db.get_saved_post_ids() (one indexed SELECT,
format-agnostic) like the other two sites already do. Also drops
the saved_dir import that was only needed for the filesystem walk.
_do_ensure only tried the batch API when _batch_api_works was True,
but after removing the search-time prefetch (where the probe used
to run), _batch_api_works stayed None forever. Gelbooru's only
viable path IS the batch API (its post-view HTML has no tag links),
so clicks on Gelbooru posts produced zero categories.
Fix: _do_ensure now tries the batch API when _batch_api_works is
not False (i.e., both True and None). When None, the call doubles
as an inline probe: if the batch produced categories, save True;
if nothing useful came back, save False and fall to HTML.
This is simpler than the old prefetch_batch probe because it runs
on ONE post at a time — no batch/HTML mixing concerns, no "single
path per invocation" rule. The probe result is persisted to DB so
it only fires once per site ever.
Dispatch matrix in _do_ensure:
_batch_api_works True + auth → batch API (Gelbooru proper)
_batch_api_works None + auth → batch as probe → True or False
_batch_api_works False → HTML scrape (Rule34)
no auth → HTML scrape (Safebooru.org)
transient error → stays None, retry next click
Verified all three sites from clean cache: Gelbooru 55/56+49/50
(batch), Rule34 40/40+38/38 (HTML), Safebooru.org 47/47+47/47
(HTML).
Removes the asyncio.create_task(prefetch_batch) calls from
search() and get_post() in both clients. Tags are now fetched
ONLY when the user actually clicks a post (via ensure_categories
in the info panel path) or saves with a category-token template.
The background prefetch was the source of most of the complexity:
probe timing, early-exit bugs from partial composes racing with
on-click ensures, Rule34's slow probe blocking the prefetch
window. All gone.
New flow:
search() → fast, returns posts with flat tags only
click → ensure_categories fires, ~200ms HTML scrape or
batch API, categories arrive, signal re-renders
re-click → instant (cache compose, no HTTP)
save → ensure in save_post_file, same path
The ~200ms per first-click is invisible during the image load.
The cache compounds across posts and sessions. The prefetch_batch
method stays in CategoryFetcher for potential future use but
nothing calls it from the hot path anymore.
The probe that detects whether a site's batch tag API works
(Gelbooru proper: yes, Rule34: no) now persists its result in the
tag_types table using a sentinel key (__batch_api_probe__). On
subsequent app launches, the fetcher reads the saved result at
construction time and skips the probe entirely.
Before: every session with Rule34 wasted ~0.6s on a probe request
that always fails (Rule34 returns garbage for names=). During that
time the background prefetch couldn't start HTML scraping, so the
first few post clicks paid ~0.3s each.
After: first ever session probes Rule34 once, stores False. Every
subsequent session reads False from DB, skips the probe, and the
background prefetch immediately starts HTML scraping. By the time
the user clicks any post, the scrape is usually done.
Gelbooru proper: probe succeeds on first session, stores True.
Future sessions use the batch API without probing. No change in
speed (already fast), just saves the probe roundtrip.
Persisted per site_id so different Gelbooru-shaped sites get their
own probe result. The clear_tag_cache method wipes probe results
along with tag data (the sentinel key lives in the same table).
Two places checked `if post.tag_categories: return` before doing
a full cache-coverage check, causing posts with partial cache
composes (e.g. 5/40 tags from the background prefetch) to get
stuck at low coverage forever:
ensure_categories: removed the post.tag_categories early exit.
Now ALWAYS runs try_compose_from_cache first. Only the 100%
coverage return (True) is trusted as "done." Partial composes
return False and fall through to the fetch path.
_ensure_post_categories_async: removed the post.tag_categories
guard. Danbooru/e621 are filtered by the client.category_fetcher
is None check instead (they categorize inline, no fetcher).
For Gelbooru-style sites, always schedules ensure_categories
regardless of current post state.
Root cause: the partial-compose fix (try_compose_from_cache
populates tag_categories even when cache coverage is <100%)
conflicted with the early-exit guards that assumed non-empty
tag_categories = fully categorized. Now the only "fully done"
signal is try_compose_from_cache returning True (100% coverage).
try_compose_from_cache was returning True on ANY partial cache hit
(even 1/38 tags). ensure_categories then saw non-empty
tag_categories and returned immediately, leaving the post stuck at
1/38 coverage. The bug showed on Rule34: post 1 got fully scraped
(40/40), its tags got cached, then post 2's compose found one
matching tag and declared victory.
Fix: try_compose_from_cache now returns True ONLY when 100% of
unique tags have cached labels (no fetch needed). It STILL
populates post.tag_categories with whatever IS cached (for
immediate partial display), but returning False signals
ensure_categories to continue to the fetch path.
This is the correct semantic split:
- populate → always (for display)
- return True → only when complete (for dispatch)
Verified:
Rule34: 40/40 + 38/38 (was 40/40 + 1/38)
Gelbooru: 55/56 + 49/50 (batch API, one rare tag)
Safebooru.org: 47/47 + 47/47 (HTML scrape, full)
Three fixes:
1. HTML parser completely rewritten with two-pass approach:
- Pass 1: regex finds each tag-type element and its full inner
content (up to closing </li|span|td|div>)
- Pass 2: within the content, extracts the tag name from the
tags=NAME URL parameter in the search link
The old single-pass regex captured the ? wiki-link (first <a>)
instead of the tag name (second <a>). The URL-param extraction
works on Rule34 (40 tags), Safebooru.org (47 tags), and
yande.re (3 tags). Gelbooru proper returns 0 (post page only
has ? links with no tags= param) which is correct — Gelbooru
uses the batch tag API instead.
2. prefetch_batch is now truly fire-and-forget:
gelbooru.py and moebooru.py use asyncio.create_task instead of
await for prefetch_batch. search() returns immediately. The
probe + batch/HTML fetch runs in the background. Previously
search() blocked on the probe, which made Rule34 searches take
5+ seconds (slow/broken Rule34 API response time).
3. Partial cache compose already fixed in the previous commit
complements this: posts with 49/50 cached tags now show all
available categories instead of nothing.
try_compose_from_cache previously required 100% cache coverage —
every tag in the post had to have a cached label or it returned
False and populated nothing. One rare uncached tag out of 50
blocked the entire composition, leaving the post with zero
categories even though 49/50 labels were available.
Fix: compose whatever IS cached, return True when at least one
tag got categorized. Tags not in the cache are simply absent from
the categories dict (they stay in the flat tags string). The
return value now means "the post has usable categories" rather
than "the post has complete categories." This distinction matters
because the dispatch logic uses the return value to decide
whether to skip the fetch path — partial coverage is better than
no coverage, and the missing tags get cached eventually when
other posts that contain them get fetched.
Verified against Gelbooru: post with 50 tags where 49 were cached
now gets 49/50 categorized (Artist, Character, Copyright, General,
Meta) instead of 0/50.
Two bookmark save sites updated for save_post_file's sync→async
signature change:
_save_bookmark_to_library: wraps the save in an async closure
and schedules via run_on_app_loop (already imported for the
thumbnail download path). Fire-and-forget; the source file is
already cached so the save is near-instant.
Save As action: same async wrapper pattern. The dialog runs
synchronously (user picks destination), then the actual file
copy is scheduled on the async loop.
Neither site passes a category_fetcher — bookmarks don't have a
direct reference to the active BooruClient. The save flow's
ensure_categories check in library_save.py short-circuits (the
fetcher is None), so template rendering uses whatever categories
are already on the post object. For bookmark→library saves, the
user typically hasn't clicked the post in the browse grid, so
categories may be empty — the template falls back to %id% for
category tokens, same as before. Full categorization on the
bookmark save path is a future enhancement (would require passing
the client through from main_window).
Four save call sites updated to await save_post_file (now async)
and pass category_fetcher so the template-render ensure check can
fire when needed.
_bulk_save: creates fetcher once at the top of the async closure,
shared across all posts in the batch. Probe state persists
within the batch.
_save_to_library: creates fetcher per invocation (single post).
_save_as: wrapped in an async closure (was sync before) since
save_post_file is now async. Uses bookmark_done/error signals
for status instead of direct showMessage.
_batch_download_to: creates fetcher once at the top, shared
across the batch.
New _get_category_fetcher helper returns the fetcher from a fresh
client (lightweight — shares the global httpx pool) or None if no
site is active.
save_post_file is now async and gains an optional
category_fetcher parameter. When the template uses any category
token (%artist%, %character%, %copyright%, %general%, %meta%,
%species%) AND the post's tag_categories is empty AND a fetcher
is available, it awaits ensure_categories(post) before calling
render_filename_template. This guarantees the filename is
correct even when saving a post the user hasn't clicked
(bypassing the info panel's on-display trigger).
When the template uses only non-category tokens (%id%, %md5%,
%score%, %rating%, %ext%) or is empty, the ensure check is
skipped entirely — no HTTP overhead for the common case.
Every existing caller already runs from _run_async closures,
so the sync→async signature change is mechanical. The callers
are updated in the next two commits to pass category_fetcher.
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.
client_for_type gains optional db + site_id kwargs. When both are
passed and api_type is gelbooru or moebooru, a CategoryFetcher is
constructed and assigned to client.category_fetcher. The fetcher
owns the per-tag cache, the batch tag API fast path, and the
per-post HTML scrape fallback.
Danbooru and e621 never get a fetcher — their inline JSON
categorization is already optimal.
Test Connection dialog and scripts don't pass db/site_id, so they
get fetcher-less clients with the existing search behavior.
Override _post_view_url to return /post/show/{id} for the per-post
HTML scrape path. No _tag_api_url override — Moebooru has no batch
tag DAPI; the CategoryFetcher dispatch goes straight to per-post
HTML for these sites.
search() and get_post() now call prefetch_batch when a fetcher is
attached, same fire-and-forget pattern as gelbooru.py.
Overrides both URL methods from the base class:
_post_view_url(post) -> /index.php?page=post&s=view&id={id}
Universal HTML scrape path — works on Gelbooru proper, Rule34,
Safebooru.org without auth.
_tag_api_url() -> {base_url}/index.php
Batch tag DAPI fast path. The CategoryFetcher's probe-and-cache
determines at runtime whether the endpoint actually honors
names=. Gelbooru proper: probe succeeds. Rule34: probe fails
(garbage response), falls back to HTML. Safebooru.org: no auth,
dispatch skips batch entirely.
search() and get_post() now call
await self.category_fetcher.prefetch_batch(posts)
after building the post list, when a fetcher is attached. The
prefetch is fire-and-forget — search returns immediately and the
background tasks fill categories as the user reads. When no
fetcher is attached (Test Connection dialog, scripts), this is a
no-op and behavior is unchanged.
Three additions to the base class, all default-inactive:
_post_view_url(post) -> str | None
Override to provide the post-view HTML URL for the per-post
category scrape path. Default None (Danbooru/e621 skip it).
_tag_api_url() -> str | None
Override to provide the batch tag DAPI base URL for the fast
path in CategoryFetcher. Default None. Only Gelbooru proper
benefits — the fetcher's probe-and-cache determines at runtime
whether the endpoint actually honors the names= parameter.
self.category_fetcher = None
Set externally by the factory (client_for_type) when db and
site_id are available. Gelbooru-shape and Moebooru clients use
it; Danbooru/e621 leave it None.
No behavior change at this commit. Existing clients inherit the
defaults and continue working identically.
New module core/api/category_fetcher.py — the unified tag-category
fetcher for boorus that don't return categories inline.
Public surface:
try_compose_from_cache(post) — instant, no HTTP. Builds
post.tag_categories from cached (site_id, name) -> label
entries. Returns True if every tag in the post is cached.
fetch_via_tag_api(posts) — batch fast path. Collects uncached
tags across posts, chunks into 500-name batches, GETs the
tag DAPI. Only available when the client declares _tag_api_url
AND has credentials (Gelbooru proper). Includes JSON/XML
sniffing parser ported from the reverted code.
fetch_post(post) — universal fallback. HTTP GETs the post-view
HTML page, regex-extracts class="tag-type-X">name</a>
markup. Works on every Gelbooru fork and every Moebooru
deployment. Does NOT require auth.
ensure_categories(post) — idempotent dispatch: cache compose ->
batch API (if available) -> HTML scrape. Coalesces concurrent
calls for the same post.id via an in-flight task dict.
prefetch_batch(posts) — fire-and-forget background prefetch.
ONE fetch path per invocation (no mixing batch + HTML).
Probe-and-cache for the batch tag API:
_batch_api_works = None -> not yet probed OR transient error
(retry next call)
_batch_api_works = True -> batch works (Gelbooru proper)
_batch_api_works = False -> clean 200 + zero matching names
(Rule34's broken names= filter)
Transition to True/False is permanent per instance. Transient
errors (HTTP error, timeout, parse exception) leave None so the
next search retries the probe.
HTML regex handles both standard tag-type-artist and combined-
class forms like tag-link tag-type-artist (Konachan). Tag names
normalized to underscore-separated lowercase.
Canonical category order: Artist > Character > Copyright >
Species > General > Meta > Lore (matches danbooru/e621 inline).
Dead code at this commit — no integration yet.
Per-site tag-type cache for boorus that don't return categories
inline. Uses string labels ("Artist", "Character", "Copyright",
"General", "Meta") instead of the integer codes the reverted
version used — the labels come directly from HTML class names,
no mapping step needed.
Schema: tag_types(site_id, name, label TEXT, fetched_at)
PRIMARY KEY (site_id, name)
Methods:
get_tag_labels(site_id, names) — chunked 500-name SELECT
set_tag_labels(site_id, mapping) — bulk INSERT OR REPLACE,
auto-prunes oldest entries when the table exceeds 50k rows
clear_tag_cache(site_id=None) — manual wipe, for future
Settings UI "Clear tag cache" button
The 50k row cap prevents unbounded growth over months of
browsing multiple boorus. Normal usage (a few thousand unique
tags per site) never reaches it. When exceeded, the oldest
entries by fetched_at are pruned first — these are the tags the
user hasn't encountered recently and would be re-fetched cheaply
if needed.
Migration: CREATE TABLE IF NOT EXISTS in _migrate(), non-breaking
for existing databases.
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
The dot on bookmark thumbnails uses set_saved_locally(...) and was
driven by find_library_files(post_id) — a digit-stem filesystem
walk that silently failed for any save with a templated filename
(e.g. 12345_hatsune_miku.jpg). The user reported it broken right
after templating landed.
Switch to db.get_saved_post_ids() for the grid refresh: one indexed
SELECT, set membership in O(1) per thumb. Format-agnostic, sees
both digit-stem and templated saves.
The "Unsave from Library" context menu used the same broken
find_library_files check for visibility. Switched to
db.is_post_in_library(post_id), which is the same idea via a
single-row SELECT 1.
Both delete_from_library call sites (single + bulk Unsave All)
now pass db so templated filenames are matched and the meta row
gets cleaned up. Refresh always runs after Unsave so the dot
clears whether the file was on disk or just an orphan meta row.
Two related fixes that the old delete flow was missing:
1. delete_from_library now accepts an optional `db` parameter which
it forwards to find_library_files. Without `db`, only digit-stem
files match (the old behavior — preserved as a fallback). With
`db`, templated filenames stored in library_meta also match,
so post-refactor saves like 12345_hatsune_miku.jpg get unlinked
too. Without this fix, "Unsave from Library" on a templated
save was a silent no-op.
2. Always cleans up the library_meta row when called with `db`, not
just when files were unlinked. Two cases this matters for:
a. Files were on disk and unlinked → meta is now stale.
b. Files were already gone but the meta lingered (orphan from
a previous broken delete) → user asked to "unsave," meta
should reflect that.
This is the missing half of the cleanup that left some libraries
with pathologically more meta rows than actual files.
The old delete_from_library deleted files from disk but never
cleaned up the matching library_meta row. Result: pathologically
the meta table can have many more rows than there are files on
disk. This was harmless when the only consumer was tag-search (the
meta would just match nothing useful), but it becomes a real
problem the moment is_post_in_library / get_saved_post_ids start
driving UI state — the saved-dot indicator would light up for
posts whose files have been gone for ages.
reconcile_library_meta() walks saved_dir() shallowly (root + one
level of subdirs), collects every present post_id (digit-stem
files plus templated filenames looked up via library_meta.filename),
and DELETEs every meta row whose post_id isn't in that set.
Returns the count of removed rows.
Defensive: if saved_dir() exists but has zero files (e.g. removable
drive temporarily unmounted), the method refuses to reconcile and
returns 0. The cost of a false positive — wiping every meta row
for a perfectly intact library — is higher than the cost of
leaving stale rows around for one more session.
The cache.py fix in the next commit makes future delete_from_library
calls clean up after themselves. This method is the one-time
catch-up for libraries that were already polluted before that fix.
When given an optional db handle, find_library_files queries
library_meta for templated filenames belonging to the post and
matches them alongside the legacy digit-stem stem == str(post_id)
heuristic. Without db it degrades to the legacy-only behavior, so
existing callers don't break — but every caller in the gui layer
has a Database instance and will be updated to pass it.
This is the foundation for the bookmark/browse saved-dot indicator
fix and the delete_from_library fix in the next three commits.
The pre-template world used find_library_files(post_id) — a
filesystem walk matching files whose stem equals str(post_id) — for
"is this post saved?" checks across the bookmark dot indicator,
browse dot indicator, Unsave menu visibility, etc. With templated
filenames (e.g. 12345_hatsune_miku.jpg) the stem no longer equals
the post id and the dots silently stop lighting up.
Two new helpers, both indexed:
- is_post_in_library(post_id) -> bool single check, SELECT 1
- get_saved_post_ids() -> set[int] batch fetch for grid scans
Both go through library_meta which is keyed by post_id, so they're
format-agnostic — they don't care whether the on-disk filename is
12345.jpg, mon3tr_(arknights).jpg, or anything else, as long as the
save flow wrote a meta row. Every save site does this since the
unified save_post_file refactor landed.
The danbooru and e621 API clients store tag_categories with
Capitalized keys ("Artist", "Character", "Copyright", "General",
"Meta", "Species") — that's the convention info_panel and
preview_pane already iterate against. render_filename_template was
looking up lowercase keys, so every category token rendered empty
even on Danbooru posts where the data was right there. Templates
like "%id%_%character%" silently collapsed back to "{id}.{ext}".
Fix: look up the Capitalized form, with a fallback chain (exact ->
.lower() -> .capitalize()) so future drift between API clients in
either direction won't silently break templates again.
Verified against a real Danbooru save in the user's library: post
11122211 with tag_categories containing Artist=["yun_ze"],
Character=["mon3tr_(arknights)"], etc. now renders
"%id%_%character%" -> "11122211_mon3tr_(arknights).jpg" instead of
"11122211.jpg".
Sixth and final Phase 2 site migration. The bookmarks context-menu
Save As action now mirrors main_window._save_as: render the template
to populate the dialog default name, then route the actual save
through save_post_file with explicit_name set to whatever the user
typed. Same behavior change as the browse-side Save As — Save As
into saved_dir() now registers library_meta where v0.2.3 didn't.
After this commit the eight save sites in main_window.py and
bookmarks.py all share one implementation. The net diff of Phase 1 +
Phase 2 (excluding the Phase 0 scaffolding) is a deletion in
main_window.py + bookmarks.py even after adding library_save.py,
which is the test for whether the refactor was the right call.
Fifth Phase 2 site migration. _copy_to_library_unsorted and
_copy_to_library now both delegate to a private
_save_bookmark_to_library helper that walks through save_post_file.
A small _bookmark_to_post adapter constructs a Post from a Bookmark
for the renderer — Bookmark already carries every field the renderer
reads, this is just one place to maintain if Post's shape drifts.
Fixes the latent v0.2.3 bug where bookmark→library copies wrote
files but never registered library_meta rows — those files were on
disk but invisible to Library tag-search until you also re-saved
from the browse side.
Picks up filename templates and sequential collision suffixes for
bookmark→library saves for free, same as the browse-side migrations.
Net add (+32 lines) is from the new helper docstrings + the explicit
_bookmark_to_post adapter; the actual save logic shrinks to a one-
liner per public method.
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.
New module core/library_save.py with one public function and two
private helpers. Dead code at this commit — Phase 2 commits route the
eight save sites through it one at a time.
save_post_file(src, post, dest_dir, db, in_flight=None, explicit_name=None)
- Renders the basename from library_filename_template, or uses
explicit_name when set (Save As path).
- Resolves collisions: same-post-on-disk hits return the basename
unchanged so re-saves are idempotent; different-post collisions get
sequential _1, _2, _3 suffixes. in_flight is consulted alongside
on-disk state for batch members claimed earlier in the same call.
- Conditionally writes library_meta when the resolved destination is
inside saved_dir(), regardless of which save path called us.
- Returns the resolved Path so callers can build status messages.
_same_post_on_disk uses get_library_post_id_by_filename, falling back
to the legacy v0.2.3 digit-stem heuristic for rows whose filename
column is empty. Mirrors the digit-stem checks already in gui/library.py.
Boundary rule: imports core.cache, core.config, core.db only. No gui/
imports — that's how main_window.py and bookmarks.py will both call in
without circular imports.
Adds the foundation that the unified save flow refactor builds on. No
behavior change at this commit — empty default template means every save
site still produces {id}{ext} like v0.2.3.
- core/db.py: library_meta.filename column with non-breaking migration
for legacy databases. Index on filename. New
get_library_post_id_by_filename() lookup. filename kwarg on
save_library_meta (defaults to "" for legacy callers).
library_filename_template added to _DEFAULTS.
- core/config.py: render_filename_template() with %id% %md5% %ext%
%rating% %score% %artist% %character% %copyright% %general% %meta%
%species% tokens. Sanitizes filesystem-reserved chars, collapses
whitespace, strips leading dots/.., caps the rendered stem at 200
characters, falls back to post id when sanitization yields empty.
- gui/settings.py: Library filename template input field next to the
Library directory row, with a help label listing tokens and noting
that Gelbooru/Moebooru can only resolve the basic ones.
The previous FlowLayout._do_layout walked each thumb summing
`widget.width() + THUMB_SPACING` and wrapped on `x + item_w >
self.width()`. This was vulnerable to two issues that conspired to
produce the "grid collapses by a column when switching to a post"
bug:
1. **Per-widget width drift**: ThumbnailWidget calls
`setFixedSize(THUMB_SIZE, THUMB_SIZE)` in __init__, capturing the
constant at construction time. If `THUMB_SIZE` is later mutated
via `_apply_settings` (main_window.py:2953 writes
`grid_mod.THUMB_SIZE = new_size`), existing thumbs keep their old
fixed size while new ones (e.g. from infinite-scroll backfill via
`append_posts`) get the new value. Mixed widths break the
width-summing wrap loop.
2. **Off-by-one in the columns property**: `w // (THUMB_SIZE +
THUMB_SPACING)` overcounted by 1 at column boundaries because it
omitted the leading THUMB_SPACING margin. A row that fits N
thumbs needs `THUMB_SPACING + N * step` pixels, not `N * step`.
For width=1135 with step=188, the formula returned 6 columns
while `_do_layout` only fit 5 — the two diverged whenever the
grid sat in the boundary range.
Both are fixed by using a deterministic position formula:
cols = max(1, (width - THUMB_SPACING) // step)
for each thumb i:
col = i % cols
row = i // cols
x = THUMB_SPACING + col * step
y = THUMB_SPACING + row * step
The layout is now a function of `self.width()` and the constants
only — no per-widget reads, no width-summing accumulator. The
columns property uses the EXACT same formula so callers (e.g.
main_window's keyboard Up/Down nav step) always get the value the
visual layout actually used.
Standardizing on the constant means existing thumbs that were
created with an old `THUMB_SIZE` value still position correctly
(they sit in the cells positioned by the new step), and any future
mutation of THUMB_SIZE only affects newly-created thumbs without
breaking the layout of the surviving ones.
Affects all three tabs (Browse / Bookmarks / Library) since they
all use ThumbnailGrid from grid.py.
Verification:
- Phase A test suite (16 tests) still passes
- Popout state machine tests (65 tests) still pass
- Total: 81 / 81 automated tests green
- Imports clean
- Manual: open the popout to a column boundary (resize window
width such that the grid is exactly N columns wide), switch
between posts — column count should NOT flip to N-1 anymore.
Also verify keyboard Up/Down nav steps by exactly the column
count visible on screen (was off-by-one before at boundaries).
The ThumbnailGrid was setting horizontal scrollbar to AlwaysOff
explicitly but leaving the vertical scrollbar at the default
AsNeeded. When content first overflowed enough to summon the
vertical scrollbar, the viewport width dropped by ~14-16px
(scrollbar width), and FlowLayout's column count flipped down by 1
because the integer-division formula sat right at a boundary.
columns = max(1, w // (THUMB_SIZE + THUMB_SPACING))
For THUMB_SIZE=180 + THUMB_SPACING=6 (per-column step = 186):
- viewport 1122 → 6 columns
- viewport 1108 (1122 - 14 scrollbar) → 5 columns
If the popout/main window happened to sit anywhere in the range
where `viewport_width % 186 < scrollbar_width`, the column count
flipped when the scrollbar appeared. The user saw "the grid
collapses by a column when switching to a post" — the actual
trigger isn't post selection, it's the grid scrolling enough to
bring the selected thumbnail into view, which makes content
visibly overflow and summons the scrollbar. From the user's
perspective the two events looked correlated.
Fix: setVerticalScrollBarPolicy(Qt.ScrollBarAlwaysOn). The
scrollbar is now always visible, its width is always reserved in
the viewport, and FlowLayout's column count is stable across the
scrollbar visibility transition.
Trade-off: a slim grey scrollbar strip is always visible on the
right edge of the grid, even when content fits on one screen and
would otherwise have no scrollbar. For an image grid that almost
always overflows in practice, this is the standard behavior (most
file browsers / image viewers do the same) and the cost is
invisible after the first few thumbnails load.
Affects all three tabs (Browse / Bookmarks / Library) since they
all use ThumbnailGrid from grid.py.
Verification:
- Phase A test suite (16 tests) still passes
- Popout state machine tests (65 tests) still pass
- Total: 81 / 81 automated tests green
- Imports clean
- Manual: open the popout to a column boundary (resize window
width such that the grid is exactly N columns wide before any
scrolling), then scroll down — column count should NOT flip to
N-1 anymore.
The signal-connection lambdas in __init__ added by commit 14a only
called _fsm_dispatch — they never followed up with _apply_effects.
Commit 14b added the apply layer and updated the keyboard event
handlers in eventFilter to dispatch+apply, but missed the lambdas.
Result: every effect produced by an mpv-driven signal was silently
dropped.
Two user-visible regressions:
1. Video auto-fit (and aspect ratio lock) broken in popout. The
mpv `video-params` observer fires when mpv reports video
dimensions, and the chain is:
_on_video_params (mpv thread) → _pending_video_size set
→ _poll → video_size.emit(w, h)
→ connected lambda → dispatch VideoSizeKnown(w, h)
→ state machine emits FitWindowToContent(w, h)
→ adapter SHOULD apply by calling _fit_to_content
The lambda dropped the effects, so _fit_to_content never ran
for video loads. Image loads were unaffected because they go
through set_media's ContentArrived dispatch (which DOES apply
via _dispatch_and_apply in this commit) with API-known
dimensions.
2. Loop=Next play_next broken. The mpv eof → VideoPlayer.play_next
→ connected lambda → dispatch VideoEofReached chain produces an
EmitPlayNextRequested effect in PlayingVideo + Loop=Next, but
the lambda dropped the effect, so self.play_next_requested was
never emitted, and main_window's _on_video_end_next never fired.
The user reported the auto-fit breakage; the play_next breakage
was the silent twin that no one noticed because Loop=Next isn't
the default.
Both bugs landed in commit 14b. The seek pin removal in d48435d
didn't cause them but exposed the auto-fit one because the user
was paying attention to popout sizing during the slider verification.
Fix:
- Add `_dispatch_and_apply(event)` helper. The single line of
documentation in its docstring tells future-pax: "if you're
going to dispatch an event, go through this helper, not bare
_fsm_dispatch." This makes the apply step impossible to forget
for any new wire-point.
- Update all 6 signal-connection lambdas to call _dispatch_and_apply:
play_next → VideoEofReached
video_size → VideoSizeKnown
clicked_position → SeekRequested
_mute_btn.clicked → MuteToggleRequested
_vol_slider.valueChanged → VolumeSet
_loop_btn.clicked → LoopModeSet
- Update the rest of the dispatch sites (keyboard event handlers in
eventFilter, the wheel-tilt navigation, the wheel-vertical volume
scroll, _on_video_playback_restart, set_media, closeEvent, the
Open dispatch in __init__, and the WindowResized/WindowMoved
dispatches in resizeEvent/moveEvent) to use _dispatch_and_apply
for consistency. The keyboard handlers were already calling
dispatch+apply via the two-line `effects = ...; self._apply_effects(effects)`
pattern; switching to the helper is just deduplication. The
Open / Window* dispatches were bare _fsm_dispatch but their
handlers return [] anyway so the apply was a no-op.
After this commit, every dispatch site in the popout adapter goes
through one helper. The only remaining `self._fsm_dispatch(...)` call
is inside the helper itself (line 437) and one reference in the
helper's docstring.
Verification:
- Phase A test suite (16 tests) still passes
- State machine tests (65 tests) still pass — none of them touch
the adapter wiring
- 81 / 81 tests green at HEAD
Manual verification needed:
- Click an uncached video in browse → popout opens, video loads,
popout auto-fits to video aspect, Hyprland aspect lock applies
- Click cached video → same
- Loop=Next mode + video reaches EOF → popout advances to next post
(was silently broken since 14b)
- Image load still auto-fits (regression check — image path was
already working via ContentArrived's immediate FitWindowToContent)
The 500ms `_seek_pending_until` pin window in `VideoPlayer._poll`
became redundant after `609066c` switched the slider seek from
`'absolute'` to `'absolute+exact'`. With exact seek, mpv decodes
from the previous keyframe forward to the click position before
reporting it via `time_pos`, so `_poll`'s read-and-write loop
naturally lands the slider at the click position without any
pinning. The pin was defense in depth for keyframe-rounding latency
that no longer exists.
Removed:
- `_seek_target_ms`, `_seek_pending_until`, `_seek_pin_window_secs`
fields from `__init__`
- The `_time.monotonic() < _seek_pending_until` branch in `_poll`
(now unconditionally `setValue(pos_ms)` after the isSliderDown
check)
- The pin-arming logic from `_seek` (now just calls `mpv.seek`
directly)
Net diff: ~30 lines removed, ~10 lines of explanatory comments
added pointing future-pax at the `609066c` commit body for the
"why" of the cleanup.
The popout's state machine SeekingVideo state continues to track
seeks via the dispatch path (seek_target_ms is held on the state
machine, not on VideoPlayer) for the future when the adapter's
SeekVideoTo apply handler grows past its current no-op. The
removal here doesn't affect that — it only drops dead defense-in-
depth code from the legacy slider rendering path.
Verification:
- Phase A test suite (16 tests) still passes
- State machine tests (65 tests) still pass — none of them touch
VideoPlayer fields
- Both surfaces (embedded preview + popout) still seek correctly
per the post-609066c verification (commit 14a/14b sweep)
Followup target from docs/POPOUT_FINAL.md "What's NOT done"
section. The other listed followup (replace self._viewport with
self._state_machine.viewport in popout/window.py) is bigger and
filed for a future session.
Removes the tests/ folder from git tracking and adds it to .gitignore.
The 81 tests (16 Phase A core + 65 popout state machine) stay on
disk as local-only working notes, the same way docs/ and project.md
are gitignored. Running them is `pytest tests/` from the project
root inside .venv as before — nothing about the tests themselves
changed, just whether they're version-controlled.
Reverts the related additions in pyproject.toml and README.md from
commit bf14466 (Phase A baseline) so the public surface doesn't
reference a tests/ folder that no longer ships:
- pyproject.toml: drops [project.optional-dependencies] test extra
and [tool.pytest.ini_options]. pytest + pytest-asyncio are still
installed in the local .venv via the previous pip install -e ".[test]"
so the suite keeps running locally; new clones won't get them
automatically.
- README.md: drops the "Run tests:" section from the Linux install
block. The README's install instructions return to their pre-
Phase-A state.
- .gitignore: adds `tests/` alongside the existing `docs/` and
`project.md` lines (the same convention used for the refactor
inventory / plan / notes / final report docs).
The 12 test files removed from tracking (`git rm -r --cached`):
tests/__init__.py
tests/conftest.py
tests/core/__init__.py
tests/core/test_cache.py
tests/core/test_concurrency.py
tests/core/test_config.py
tests/core/test_db.py
tests/core/api/__init__.py
tests/core/api/test_base.py
tests/gui/__init__.py
tests/gui/popout/__init__.py
tests/gui/popout/test_state.py
Verification:
- tests/ still exists on disk
- `pytest tests/` still runs and passes 81 / 81 in 0.11s
- `git ls-files tests/` returns nothing
- `git status` is clean
