perf: fix slow duplicate-group navigation and related redundant calls (#118) #133

Merged

ValleyGeek merged 8 commits from 0.2.0/issue-118-dupe-group-nav-perf into main 2026-06-23 05:37:22 +00:00

Conversation 0 Commits 8 Files changed 16 +758 -124

Claude commented 2026-06-23 05:33:16 +00:00

Member

Copy link

Summary

Fixes #118 (slow duplicate-group navigation), then expanded into a general audit for the same redundant-call bug classes elsewhere in the app.

Root causes in the Duplicates workbench (#118):

• ImageFilterProxy.set_dupe_group_filter (and a redundant second call in main_window._apply_dupe_group_filter) forced a full invalidate() — refilter and resort of the entire image list — on every group navigation click, even though the dupe-group filter never affects sort order.
• DupeMultiGrid destroyed and recreated every comparison tile (each owning its own QGraphicsView/QGraphicsScene) on every group switch, instead of reusing them.
• Three redundant per-click DB round trips (get_all_tags() queried separately for the current group + 2 preloaded neighbors; count_distinct_dupe_tags() duplicating a count already available).

Generalized to the rest of the app (same bug classes):

• All 10 ImageFilterProxy filter setters (tag filter, search, hide-deleted, orientation, etc.) shared the same over-broad invalidate() — fixed for all of them, not just the dupe-group filter.
• TagPanel fully rebuilt every tag swatch button on every grid selection change and every viewer navigation step. Now updates existing buttons in place.
• Bulk tag/DELETE actions (tag_manager.py) opened one SQLite connection per image for planning and applying changes — a 200-image selection meant ~800-1000 connections for one keypress. Added bulk WorkingDB helpers (get_images_by_paths, get_tags_for_images, add_tags_to_images, remove_tags_from_images) and routed every per-path loop through them.
• Same per-path DB loop pattern in two main_window.py selection/tag-sync helpers, fixed identically.
• ImageListModel (grid data model) did an O(n) linear scan to find a row by path on every upsert/thumbnail-batch/remove/relocate call — hit repeatedly during scan and thumbnail-generation progress. Added a path→row index for O(1) lookups, and batched dataChanged emissions per call instead of one per item.

Measured ~8x faster for the dupe-grid widget-rebuild step alone in an isolated offscreen benchmark (15.7ms → 1.9ms per switch, excluding image decode/IO).

Test plan

• Full test suite passes (536 tests, 1 skip)
• ruff check / ruff format --check clean
• New regression tests added for each fix, including SQLite connection-count assertions proving the N+1 patterns are gone and path-index consistency checks for ImageListModel
• scripts/smoke-ui-exit.sh clean (no Qt teardown errors) — relevant since several fixes change widget pooling/lifecycle
• scripts/check-version-bump.sh passes (VERSION bumped 0.2.0 → 0.2.1, compiled inputs changed)
• Manual verification on a large real-world directory (hundreds of images/duplicates) on Windows, where the original report came from, is still recommended before release

🤖 Generated with Claude Code

## Summary Fixes #118 (slow duplicate-group navigation), then expanded into a general audit for the same redundant-call bug classes elsewhere in the app. **Root causes in the Duplicates workbench (#118):** - `ImageFilterProxy.set_dupe_group_filter` (and a redundant second call in `main_window._apply_dupe_group_filter`) forced a full `invalidate()` — refilter **and** resort of the entire image list — on every group navigation click, even though the dupe-group filter never affects sort order. - `DupeMultiGrid` destroyed and recreated every comparison tile (each owning its own `QGraphicsView`/`QGraphicsScene`) on every group switch, instead of reusing them. - Three redundant per-click DB round trips (`get_all_tags()` queried separately for the current group + 2 preloaded neighbors; `count_distinct_dupe_tags()` duplicating a count already available). **Generalized to the rest of the app (same bug classes):** - All 10 `ImageFilterProxy` filter setters (tag filter, search, hide-deleted, orientation, etc.) shared the same over-broad `invalidate()` — fixed for all of them, not just the dupe-group filter. - `TagPanel` fully rebuilt every tag swatch button on *every* grid selection change and *every* viewer navigation step. Now updates existing buttons in place. - Bulk tag/DELETE actions (`tag_manager.py`) opened one SQLite connection per image for planning **and** applying changes — a 200-image selection meant ~800-1000 connections for one keypress. Added bulk `WorkingDB` helpers (`get_images_by_paths`, `get_tags_for_images`, `add_tags_to_images`, `remove_tags_from_images`) and routed every per-path loop through them. - Same per-path DB loop pattern in two `main_window.py` selection/tag-sync helpers, fixed identically. - `ImageListModel` (grid data model) did an O(n) linear scan to find a row by path on every upsert/thumbnail-batch/remove/relocate call — hit repeatedly during scan and thumbnail-generation progress. Added a path→row index for O(1) lookups, and batched `dataChanged` emissions per call instead of one per item. Measured ~8x faster for the dupe-grid widget-rebuild step alone in an isolated offscreen benchmark (15.7ms → 1.9ms per switch, excluding image decode/IO). ## Test plan - [x] Full test suite passes (536 tests, 1 skip) - [x] `ruff check` / `ruff format --check` clean - [x] New regression tests added for each fix, including SQLite connection-count assertions proving the N+1 patterns are gone and path-index consistency checks for `ImageListModel` - [x] `scripts/smoke-ui-exit.sh` clean (no Qt teardown errors) — relevant since several fixes change widget pooling/lifecycle - [x] `scripts/check-version-bump.sh` passes (VERSION bumped 0.2.0 → 0.2.1, compiled inputs changed) - [ ] Manual verification on a large real-world directory (hundreds of images/duplicates) on Windows, where the original report came from, is still recommended before release 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Claude added 8 commits 2026-06-23 05:33:17 +00:00

fix(dupes): avoid full re-sort+filter on duplicate group navigation (#118) ... c6e1aa079e

Switching between adjacent duplicate groups invalidated the entire image
filter proxy twice per click — once in set_dupe_group_filter, again in
_apply_dupe_group_filter's refresh — each forcing a full filter+sort pass
over every image in the directory even though group filtering never
affects sort order. In directories with hundreds of images this produced
the ~1s stall reported in #118. Use invalidateFilter() for the group
filter and skip the redundant second invalidate.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

perf(dupes): pool comparison-grid cells and dedupe per-click DB calls (#118) ... 69a4b44b2c

Switching duplicate groups still felt slow after the filter-proxy fix because
DupeMultiGrid destroyed and recreated every comparison tile (each owning its
own QGraphicsView/QGraphicsScene) on every navigation, even when the grid
shape was unchanged. Pool cells by grid position and reuse them in place
(content + size only) instead of tearing the widget tree down each click;
measured ~8x faster per switch in an offscreen benchmark.

Also cut three redundant per-click DB round trips: get_all_tags() was queried
once per enriched group (current + 2 preloaded neighbors) for an identical
tag-color map, and count_distinct_dupe_tags() duplicated a count already
available from the summaries refresh_from_db() just fetched.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

perf(filter): apply issue #118's invalidateFilter() fix to all filter setters ... 96447924a4

ImageFilterProxy's other 10 filter setters (tag filter, search, hide-deleted,
orientation, dupe-group, pinned path) shared an _invalidate() helper that
called the full invalidate() -- forcing Qt to refilter AND resort the entire
source model on every tag-filter click or search keystroke, even though none
of these fields are read in lessThan(). Generalizes the fix already applied
to the dupe-group filter to every other setter, and consolidates the
now-identical _invalidate()/_invalidate_filter() helpers into one.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

perf(tags): stop rebuilding the tag panel on every selection/nav step ... 9ffbeef0d7

TagPanel deleteLater()'d and recreated every TagSwatchButton on every grid
selection change AND every viewer navigation step (A/D key), via
TagContainer.refresh_from_db() calling set_tags() then
set_selection_tag_state() back-to-back -- two full rebuilds per click for
state that's usually just a highlight change. set_tags() now skips the
rebuild when the tag set is unchanged, and set_selection_tag_state() updates
existing TagSwatchButton instances in place via a new set_active_state()
setter instead of rebuilding.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

perf(tags): batch bulk tag/DELETE DB calls instead of one connection per path ... 75d30924d9

tag_manager.py's plan_*/apply_assignment_changes/set_delete_on_paths helpers
each opened a fresh SQLite connection per image (get_image_by_path +
get_tags_for_image, then again to write) -- a 200-image bulk tag/DELETE
action meant roughly 800-1000 separate connection cycles for one keypress.
plan_apply_tags_on_paths was worse: O(tag_names * paths) round trips.

Adds bulk WorkingDB helpers (get_images_by_paths, get_tags_for_images,
add_tags_to_images, remove_tags_from_images) and routes every per-path loop
in tag_manager.py through them, so a bulk action now costs a small constant
number of round trips regardless of selection size.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

perf(tags): batch DB calls in main_window selection/tag-sync helpers ... 2356a88b74

_selection_tag_state() and _sync_model_tags_for_paths() had the same
per-path get_image_by_path()+get_tags_for_image() loop already fixed in
tag_manager.py -- the former runs on every grid selection change and viewer
nav step. Adds TagManager.image_dicts_with_tags() (bulk variant of
image_dict_with_tags()) and routes both helpers through the bulk
WorkingDB.get_images_by_paths()/get_tags_for_images() queries.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

perf(grid): O(1) path-to-row lookups in ImageListModel, batch dataChanged ... 80501ba5e1

upsert_image, apply_thumbnail_batch, remove_images, relocate_image, and
row_for_relative_path each did a linear scan over every row to find a match
by relative_path -- called repeatedly during scan progress and
thumbnail-generation batches, this was effectively quadratic over a large
scan. Adds a _row_by_path index maintained by every mutator for O(1) lookups.

Also batches upsert_images()/apply_thumbnail_batch() into one combined
dataChanged emission per call instead of one per item (matching the existing
issue #94 batching pattern in patch_tag_display), and routes
_sync_model_tags_for_paths() through the now-batched upsert_images().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

chore: PR-prep — bump to 0.2.1 for dupe-nav and redundant-call perf fixes ... 7aada89bc1

Compiled inputs changed on this branch (src/**), so VERSION advances from
the 0.2.0-bug.118 dev form to the next canonical release per VERSIONING.md.
CHANGELOG.md documents the user-visible effect (issue #118).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request 2026-06-23 05:33:33 +00:00

[Bug]: Slow Dupe Group Change #118

ValleyGeek merged commit c199d2c031 into main 2026-06-23 05:37:22 +00:00

ValleyGeek deleted branch 0.2.0/issue-118-dupe-group-nav-perf 2026-06-23 05:37:22 +00:00

ValleyGeek referenced this pull request from a commit 2026-06-23 05:37:23 +00:00

perf: fix slow duplicate-group navigation and related redundant calls (#118) (#133)

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!133

No description provided.