Skip to content

chore(dedupe): collapse low-risk redundancies from code review#244

Merged
lstein merged 1 commit into
masterfrom
lstein/chore/dedupe-batch
May 20, 2026
Merged

chore(dedupe): collapse low-risk redundancies from code review#244
lstein merged 1 commit into
masterfrom
lstein/chore/dedupe-batch

Conversation

@lstein
Copy link
Copy Markdown
Owner

@lstein lstein commented May 20, 2026

Summary

Low-risk batch of redundancy fixes from the code review. Net +172 / −263 across 6 files.

Backend:

  • Removed the duplicate /filetree/home endpoint from routers/album.pyrouters/filetree.py already serves the canonical one. Whichever router was included last had been silently winning.
  • Extracted _compute_curation() and _validate_curation_request(). /curate (async via background task) and /curate_sync were carrying ~150 lines of identical Monte Carlo / mapping / selection logic. The async path passes a tiny on_iteration callback to drive its ProgressTracker.
  • Routed delete_image, move_images, copy_images in routers/index.py through the existing get_embeddings_for_album helper instead of three near-identical inline Embeddings(embeddings_path=..., encoder_spec=...) constructions.
  • Added _l2_normalize(x, axis, eps=1e-12) in embeddings.py and used it at all three numpy-normalize sites — also fixes the no-epsilon NaN risk in find_duplicate_clusters that the review flagged.
  • Added _normalized_filtered_embeddings() shared loader for FPS / KMeans. get_fps_indices_global and get_kmeans_indices_global shared a ~30-line "open npz / build valid_mask / normalize" preamble — that part is now in one place.

Frontend:

  • Deleted the duplicate createPathField in album-manager.js (defined twice, byte-for-byte identical — the second shadowed the first).
  • Replaced new ScoreDisplay() in seek-slider.js with the exported scoreDisplay singleton from score-display.js. The second instance was double-binding the star click handler on #scoreText, so bookmark toggles fired twice and immediately cancelled themselves.

Deferred to follow-up PRs

The bigger structural items from the review (FastAPI Depends() for validate_album_exists/get_embeddings_for_album/check_album_lock, state.js setter generator, fetchJson() helper, makeDraggable() extraction, encoder base-class close(), update_index/update_index_async unification, InvokeMetadataView strategy class) are intentionally left out of this batch — each is its own reviewable change.

Test plan

  • ruff check photomap tests — clean
  • npm run lint + npm run format:check — clean
  • pytest tests/backend — 257 passed (same count as before)
  • npm test — 288 passed across 18 suites
  • Manual: /curate (async) and /curate_sync both return identical-shaped responses for the same request
  • Manual: bookmark a search result via the seek-slider's star — confirm exactly one toggle (the prior double-bind would cancel itself)

🤖 Generated with Claude Code

Backend:

* drop the duplicate `/filetree/home` endpoint from `routers/album.py`;
  `routers/filetree.py` already serves the canonical one (whichever
  router was included last had been winning)
* extract `_compute_curation()` and `_validate_curation_request()` —
  `/curate` (async background task) and `/curate_sync` were carrying
  ~150 lines of identical Monte Carlo / mapping / selection logic.
  The async path passes a small `on_iteration` callback to drive its
  ProgressTracker
* route the three index endpoints (`delete_image`, `move_images`,
  `copy_images`) through the existing `get_embeddings_for_album`
  helper instead of each re-instantiating `Embeddings` directly
* add `_l2_normalize(x, axis, eps=1e-12)` to `embeddings.py` and use
  it in all three numpy-normalize sites — also closes the no-epsilon
  NaN risk in `find_duplicate_clusters`
* add `_normalized_filtered_embeddings()` shared loader for FPS /
  KMeans (npz open + valid mask + normalize); each function is now
  ~15 lines shorter and the "load" half is provably identical

Frontend:

* delete the duplicate `createPathField` in `album-manager.js`
  (defined twice with identical bodies — the second shadowed the
  first)
* share the exported `scoreDisplay` singleton in `seek-slider.js`
  instead of constructing a second `ScoreDisplay`. The duplicate
  instance was double-binding the star click handler on `#scoreText`,
  so a bookmark toggle fired twice and immediately cancelled itself

Net: +172 / −263 across 6 files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lstein lstein merged commit e29f3da into master May 20, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant