Conversation
…lection matching Three fixes/improvements to the Lightroom Classic plug-in, found and fixed while dogfooding it against a real LrC instance: - Ship the publish service's icon (icon.png/icon@2x.png). It was referenced in code but never added to the plug-in folder, so it showed up blank/broken in Lightroom's Publishing Services panel. - Stop duplicate files piling up on re-publish. uploadOnePhoto always created a new Lumio file and never removed the previous one, so editing a photo and republishing (or a manual "Republish") left several versions of the same photo in the online gallery. The plug-in now deletes the old remote file (via rendition.publishedPhotoId) before uploading the new one. - Make Selection-Import matching survive renames and same-basename ambiguity. The Publish-Service now embeds the MD5 of the original master file into a custom XMP segment of the uploaded JPEG (JpegXmp.lua). The worker extracts it via the existing exiftool call and stores it under the already-present (and previously unused) exif.lumio.originalMd5 JSON field -- no schema migration needed. Selection-Import uses it to auto-resolve ambiguous filename matches and, with a new opt-in option, to recover files renamed after publishing. Also: bump the plug-in to v0.3, translate all plug-in comments to English (previously a mix of German/Finnish/English), and update the plug-in's own README/known-limitations docs to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness/documentation issues that should be addressed before merge (JSONB merge semantics for exif.lumio.*, misleading changelog wording, and JPEG XMP segment insertion position compatibility risk).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the Lightroom Classic plug-in and supporting server/worker plumbing to (1) ship the missing publish-service icon, (2) prevent duplicate remote files on re-publish by deleting the previously published remote file first, and (3) make Selection-Import matching robust to renames/ambiguous basenames by embedding + round-tripping an original-master MD5 via a custom XMP tag.
Changes:
- Lightroom plug-in: add XMP-stamping (
JpegXmp.lua) + hash-based disambiguation/rename recovery; delete old remote file on re-publish; bump plug-in version. - Worker/API: extract/surface
originalMd5(stored inFile.exif.lumio.originalMd5) through plugin endpoints; update processing tasks to persist it. - Docs/changelog/readmes: document the new behavior and plug-in structure; translate plug-in comments to English in touched files.
File summaries
| File | Description |
|---|---|
| CHANGELOG.md | Adds Unreleased notes for the plug-in icon, re-publish behavior, and hash-based matching. |
| apps/worker/tasks/process_raw.py | Switches EXIF extraction to extract_metadata() and persists original_md5 when available. |
| apps/worker/tasks/process_file.py | Switches EXIF extraction to extract_metadata() and persists original_md5 when available. |
| apps/worker/exif_meta.py | Adds original-master MD5 extraction from custom XMP and a combined extract_metadata() helper. |
| apps/worker/db.py | Extends mark_file_ready() to persist original_md5 into the existing exif JSONB field. |
| apps/lightroom-plugin/README.md | Updates known limitations + folder structure to reflect hash matching and new files. |
| apps/lightroom-plugin/README.it.md | Updates Italian README with the new hash-matching behavior and file list. |
| apps/lightroom-plugin/lumio.lrdevplugin/PluginManager.lua | Translates/clarifies Plug-in Manager UI comments. |
| apps/lightroom-plugin/lumio.lrdevplugin/LumioPublishService.lua | Embeds original MD5 into JPEG XMP and deletes previous remote file on re-publish. |
| apps/lightroom-plugin/lumio.lrdevplugin/LumioApi.lua | Translates comments/messages and keeps retry logic; supports delete call used by re-publish. |
| apps/lightroom-plugin/lumio.lrdevplugin/Logger.lua | Translates header comments. |
| apps/lightroom-plugin/lumio.lrdevplugin/Json.lua | Translates header comments for embedded JSON helper. |
| apps/lightroom-plugin/lumio.lrdevplugin/JpegXmp.lua | New module to inject an APP1 XMP segment containing OriginalMD5 into rendered JPEGs. |
| apps/lightroom-plugin/lumio.lrdevplugin/Info.lua | Bumps plug-in version to 0.3 and updates documentation comments. |
| apps/lightroom-plugin/lumio.lrdevplugin/ImportSelectionTask.lua | Adds hash-based disambiguation and opt-in rename recovery while applying selection to the catalog. |
| apps/lightroom-plugin/lumio.lrdevplugin/ImportSelectionDialog.lua | Adds opt-in checkbox to enable hash-based recovery of renamed files. |
| apps/api/src/routes/plugin.ts | Extracts and returns originalMd5 from exif.lumio.originalMd5 on plugin endpoints. |
| apps/api/prisma/schema.prisma | Documents the exif.lumio.originalMd5 convention in the Prisma schema comments. |
Review details
Suppressed comments (1)
CHANGELOG.md:43
- This changelog bullet duplicates the same English sentence twice (plain + italic after
·). It looks like an accidental double entry and makes the section noisy.
- Lightroom plug-in: the Publish-Service now embeds the MD5 hash of the original master file into a custom XMP field of the uploaded JPEG, and the Selection-Import side uses it to automatically resolve ambiguous filename matches (e.g. the same shot published as both `.NEF` and `.DNG`) and, with a new opt-in option, to recover files that were renamed in Lightroom after publishing. Only available for files published from this plug-in version onward. · *The Lightroom plug-in's Publish-Service now embeds the original master file's MD5 hash into a custom XMP field of the uploaded JPEG, and Selection-Import uses it to automatically resolve ambiguous filename matches (e.g. the same shot published as both `.NEF` and `.DNG`) and, with a new opt-in option, to recover files renamed in Lightroom after publishing. Only available for files published from this plug-in version onward.*
- Files reviewed: 18/20 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| A pull + regular redeploy is enough for the server/worker (the new | ||
| `originalMd5` column is picked up by the existing `prisma migrate deploy` | ||
| step). **Anyone using the Lightroom plug-in has to reinstall it:** the |
| SET status = 'ready', width = %s, height = %s, | ||
| sha256 = COALESCE(%s, sha256), | ||
| exif = COALESCE(exif, '{}'::jsonb) || COALESCE(%s::jsonb, '{}'::jsonb), | ||
| "updatedAt" = NOW() |
| def extract_original_md5(src_path: str) -> str | None: | ||
| """MD5 of the ORIGINAL master file, as embedded by the Lightroom | ||
| plug-in's Publish-Service into a custom XMP field of the uploaded | ||
| JPEG. Only present for files published via that plug-in from this | ||
| feature onward; None for everything else -- never throws. |
| Original-Masters im hochgeladenen JPEG hinterlegt. Damit werden | ||
| mehrdeutige Treffer automatisch aufgelöst, und mit der Option | ||
| „Rinominati per hash" lassen sich auch umbenannte Files wiederfinden. | ||
| Für älter veröffentlichte Files (oder Uploads über Browser/Upload-Link) |
| end | ||
| local segment = string.char(0xFF, 0xE1) .. u16be(segmentLen) .. payload | ||
|
|
||
| local newData = data:sub(1, 2) .. segment .. data:sub(3) |
| local targets = {} -- md5 -> file | ||
| for _, file in ipairs(missingCandidates) do | ||
| if file.originalMd5 then targets[file.originalMd5] = file end | ||
| end | ||
| if next(targets) then | ||
| progress:setCaption("Recomputing hashes to find renamed files…") | ||
| for i, photo in ipairs(pool) do | ||
| if progress:isCanceled() then canceled = true break end | ||
| if i % 20 == 0 then progress:setPortionComplete(i, #pool) end | ||
| local okPath, path = LrTasks.pcall(function() return photo:getRawMetadata("path") end) | ||
| if okPath and path then | ||
| local okHash, hash = LrTasks.pcall(fileMd5, path) | ||
| if okHash and hash and targets[hash] then | ||
| local file = targets[hash] | ||
| resolved[file] = { photo } | ||
| targets[hash] = nil -- consumed, keep scanning for the rest | ||
| resolvedByHashCount = resolvedByHashCount + 1 | ||
| end | ||
| end | ||
| end | ||
| end |
… public gallery goToPublishedCollection built the public customer-facing link (host + /g/<slug>). A photographer clicking "Show in Lumio" from Lightroom wants to manage the gallery, not see the customer's view, so it now opens the Studio management page (host + /studio/<galleryId>) instead. The Studio route is keyed by gallery id, not slug, so this also drops the now-unneeded slug-from-cache lookup and the gallerySlug tracking in processRenderedPhotos that only existed to support it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Big one, thanks. The XMP route for the hash is a good call — solving the rename problem without a schema migration is better than the column I'd have reached for. Two of the Copilot findings I'd want fixed before merge: APP1 before APP0. Inserting the XMP segment directly after SOI when a JFIF APP0 follows can trip strict readers. This one touches real customer files that go on to print labs and third-party software, so the conservative order seems worth it: if the first segment is APP0, insert after it, otherwise keep the current behaviour. Unless you've already checked how the labs' intake handles it and it's a non-issue — you'd know better than me there.
The third one I read differently, and I'd like your view. Copilot says the Three smaller things:
A worker test for On the "Show in Lumio" change: I follow the reasoning and mostly agree a photographer clicking it wants the management view. One thing gives me pause — the studio link needs an active studio session in the browser, where the public link worked regardless, and there are moments where you genuinely want to see what the client sees. Not asking you to revert it; what did you weigh there? If you considered keeping the customer link reachable and decided against, I'm happy to take your read on it. The end-to-end check in a real Lightroom Classic is the part I don't want to skip. @canja006, you were in the plug-in loading path in #13 — any chance you can run the icon, re-publish and Selection-Import flows against your install? |
|
Happy to — I'll run the end-to-end flows against my install (Lightroom Classic on macOS + self-hosted instance): plugin icon, publish → re-publish (anti-duplicate), and Selection-Import both normally and against a renamed file to exercise the XMP hash path. I'll wait for the two fixes above to land first and expect to have results within a day of the push. |
- JpegXmp.lua: insert the XMP hash segment after a leading APP0/JFIF
segment when present, instead of unconditionally right after SOI.
Verified byte-for-byte against a real JPEG with and without a
leading APP0.
- ImportSelectionTask.lua: the rename-recovery hash map was
hash -> file (single), silently dropping every file after the first
one sharing a hash -- normal when virtual copies of one master were
all published. Now hash -> list of files, all resolved to the same
matched photo.
- LumioPublishService.lua: add goToPublishedPhoto (per-photo "Show
public gallery"), opening the public customer-facing gallery link.
Complements the collection-level "Show in Lumio" now pointing at the
Studio view, for when you do want to see the client's side -- and
doesn't need an active Studio session the way the Studio link does.
Restores the gallerySlug tracking this needs.
- db.py: mark_file_ready no longer touches the exif column at all when
there's no original_md5 to write (previously flipped NULL to {} on
every file). When there is one, it's written via jsonb_set over the
existing 'lumio' object instead of a flat merge, so a future second
field under exif.lumio survives being written alongside originalMd5.
Verified both the previous silent-{} behaviour and the new jsonb_set
merge against a throwaway Postgres instance.
- README.md (DE): fixed an Italian checkbox label ("Rinominati per
hash") accidentally left in the German option description.
- Added worker tests for extract_original_md5/extract_metadata,
including one asserting a single exiftool call for both values.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for the thorough read-through. Pushed APP1/APP0 ordering — fixed:
CHANGELOG wording — this one was already fixed in README Italian label — fixed, replaced with the actual German option text.
Worker test — added "Show in Lumio" -> Studio — to answer directly: this was an explicit request from the project owner, not something I inferred on my own. Your underlying concern (needs an active Studio session; sometimes you want the client's view) is fair though, so rather than just explain it away I added @canja006 the two blocking fixes are in, whenever you get a chance to run the LrC flows. |
|
Further explanation for the Show in Lumio:
This is just my workflow, for sure I'm bringing some influence from Pixieset |
|
Ran the end-to-end flows against my install — Lightroom Classic 15.5.1 on macOS, self-hosted Lumio on v0.74.1 with this PR at Five of the six work. The sixth could not have worked on any machine. Icon — renders in the Publishing Services panel. Anti-duplicate re-publish — republished a 70-file gallery. Per photo the order is The XMP hash round trip — all 70 files carry Selection import by filename — 70/70, both with and without the hash option. Selection import after a rename — renamed a published photo in the catalog, then: hash off → "Show in Lumio" → "Show public gallery" never reaches the collection settings
There is no Worth stressing that this is not a slug-availability problem: the gallery cache held the right entry (
function exportServiceProvider.goToPublishedPhoto(publishSettings, info)
local LrHttp = import "LrHttp"
info = info or {}
-- Reading the catalog yields, and yielding is not allowed inside a plain
-- pcall (a C call), so this runs in an async task with LrTasks.pcall.
LrTasks.startAsyncTask(function()
-- publishedCollectionInfo carries only {isDefaultCollection, name,
-- parents}, and LrPublishedPhoto has no getPublishedCollection(),
-- so the settings have to come from the photo.
local wantedName = (type(info.publishedCollectionInfo) == "table")
and info.publishedCollectionInfo.name or nil
local collSettings = {}
if info.photo then
local ok, colls = LrTasks.pcall(function()
return info.photo:getContainedPublishedCollections()
end)
if ok and type(colls) == "table" then
local fallback
for _, coll in ipairs(colls) do
local okn, nm = LrTasks.pcall(function() return coll:getName() end)
local oks, summary = LrTasks.pcall(function()
return coll:getCollectionInfoSummary()
end)
local st = oks and summary and summary.collectionSettings or nil
if type(st) == "table" and st.galleryId and st.galleryId ~= "" then
if okn and wantedName and nm == wantedName then
collSettings = st
break
end
fallback = fallback or st
end
end
if next(collSettings) == nil and fallback then collSettings = fallback end
end
end
local host = (LrPrefs.prefsForPlugin().host or ""):gsub("/+$", "")
local slug = resolveGallerySlug(collSettings)
if not slug or slug == "" or host == "" then
LrDialogs.message("Lumio",
"Gallery slug or host is missing. Publish the collection once, " ..
"or set the server address in Plug-in Manager.", "warning")
return
end
LrHttp.openUrlInBrowser(host .. "/g/" .. slug)
end)
endWith that, the log reads The hash pass is expensive by defaultSame import, same result, two scopes:
Lightroom became visibly sluggish during the second one and memory climbed throughout. Two causes:
Photos already resolved by filename in pass 1 are hashed again too. Since Two small things
|
… speed up rename recovery Addresses the real end-to-end test report from @canja006 on PR markusthiel#27 (LrC 15.5.1, 2774-photo catalog). - goToPublishedPhoto ("Show public gallery") always failed: it assumed info.publishedCollectionInfo had a .collectionSettings field like getCollectionInfoSummary() elsewhere in this file. On a real install it only ever carries {isDefaultCollection, name, parents}, and LrPublishedPhoto has no getPublishedCollection() either. Adopts @canja006's tested fix: fetch the collection through photo:getContainedPublishedCollections() instead, wrapped in LrTasks.startAsyncTask since that call yields. - gallerySlug was only ever captured when a NEW gallery was created from the plug-in dialog; a collection bound to an EXISTING gallery (picked from the dropdown) never got one, leaving the public-gallery link dependent on a cache that goes stale as soon as the gallery is edited in Studio. processRenderedPhotos now self-heals it (same proven catalog:withWriteAccessDo pattern already used for gallery creation) on the next publish to any such collection. - Rename-recovery hash pass took 4m12s on the full 2774-photo catalog (the default scope) vs 3.8s on a 70-photo collection, with Lightroom visibly sluggish and memory climbing. Three fixes: stop scanning once every target hash is found (it never exited early before), skip photos pass 1 already matched by filename (they were hashed again for nothing), and -- the structural fix -- embed the original master's byte size alongside its MD5 (free: the file is already fully in RAM to hash it) so a rename-recovery candidate can be ruled out with a plain LrFileUtils.fileAttributes stat instead of being fully read into memory. Exact byte-size collisions between different photos are rare; identity is still always confirmed by the real hash. exif_meta.py/db.py/plugin.ts extended to carry originalSize alongside originalMd5 through the same pipeline, same reasoning as the original hash (no schema change, jsonb_set-based merge). - Logger.lua/README: corrected the macOS log path for current Lightroom Classic (confirmed on 15.5: ~/Library/Logs/Adobe/Lightroom/ LrClassicLogs/, not ~/Documents/LrClassicLogs/). Verified: XMP round-trip with the new OriginalSize field against a real JPEG + exiftool (with and without a size, matching the existing OriginalMD5 test pattern); worker db.py's extended jsonb_set query against a throwaway Postgres instance including the actual mark_file_ready() function end-to-end (exif=NULL populated with both fields when given, an existing exif with sibling keys left completely untouched when no hash is given); 4 new + all 19 existing worker tests pass; tsc --noEmit clean; luac -p clean on all 9 plug-in files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@canja006 thank you for this — an exceptionally thorough report, and exactly the kind of real-device signal I can't produce here. Pushed
The hash-pass performance — went with a structural fix rather than just the two obvious patches. Now:
This does mean Logger.lua/README log path — corrected to Verified from this end: XMP round-trip with the new Whenever you get a chance: would appreciate a re-run of |
Scoped to the Python files touched by this PR (db.py, process_file.py, process_raw.py, test_exif_meta.py) -- exif_meta.py was already in English. Comments only, no functional changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Re-ran everything against All three hold up.
Rename-recovery timing — same catalog, same
Both correct:
One thing worth knowing for anyone else testing this on a real install: the plug-in file was in place on disk at 17:55, but Lightroom was still executing the previously loaded copy at 20:00 — I could only tell because my own build had logging in it that yours doesn't. A full Lightroom restart at 20:06 picked up the new file. Two of my earlier results were against stale in-memory Lua before I caught it. Nothing outstanding from my side. |
|
Thanks @canja006 for all the tests. |
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of correctness issues in the new hash-recovery path and tests that should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 19/21 changed files
- Comments generated: 3
- Review effort level: Lite
| local sizesToCheck = {} | ||
| for _, file in ipairs(missingCandidates) do | ||
| if file.originalMd5 then | ||
| targets[file.originalMd5] = targets[file.originalMd5] or {} | ||
| table.insert(targets[file.originalMd5], file) | ||
| if file.originalSize then | ||
| sizesToCheck[file.originalSize] = true | ||
| end | ||
| end | ||
| end | ||
| if next(targets) then | ||
| progress:setCaption("Recomputing hashes to find renamed files…") | ||
| for i, photo in ipairs(pool) do | ||
| if progress:isCanceled() then canceled = true break end | ||
| if not usedPhotos[photo] then | ||
| if i % 20 == 0 then progress:setPortionComplete(i, #pool) end | ||
| local okPath, path = LrTasks.pcall(function() return photo:getRawMetadata("path") end) | ||
| if okPath and path then | ||
| local attrs = LrFileUtils.fileAttributes(path) | ||
| local size = attrs and attrs.fileSize | ||
| -- Cheap pre-filter (see PERFORMANCE above): | ||
| -- only hash candidates whose size could | ||
| -- possibly match a hash we're still after. | ||
| if size and sizesToCheck[size] then | ||
| local okHash, hash = LrTasks.pcall(fileMd5, path) |
| - Lightroom plug-in: the Publish-Service now embeds the MD5 hash of the original master file into a custom XMP field of the uploaded JPEG, and the Selection-Import side uses it to automatically resolve ambiguous filename matches (e.g. the same shot published as both `.NEF` and `.DNG`) and, with a new opt-in option, to recover files that were renamed in Lightroom after publishing. Only available for files published from this plug-in version onward. · *The Lightroom plug-in's Publish-Service now embeds the original master file's MD5 hash into a custom XMP field of the uploaded JPEG, and Selection-Import uses it to automatically resolve ambiguous filename matches (e.g. the same shot published as both `.NEF` and `.DNG`) and, with a new opt-in option, to recover files renamed in Lightroom after publishing. Only available for files published from this plug-in version onward.* | ||
|
|
||
| ### Changed | ||
|
|
||
| - Lightroom plug-in: "Show in Lumio" (right-click a published collection) now opens the gallery's Studio management view instead of the public customer-facing gallery link. A new per-photo "Show public gallery" entry (right-click a published photo) covers the case where you do want the customer's view. · *"Show in Lumio" (right-click a published collection) now opens the gallery's Studio management view instead of the public customer-facing gallery link. A new per-photo "Show public gallery" entry (right-click a published photo) covers the case where you do want the customer's view.* | ||
|
|
||
| ### Fixed | ||
|
|
||
| - Lightroom plug-in: the publish service's icon was referenced but never shipped, so it showed up blank/broken in Lightroom's Publishing Services panel. · *The publish service's icon was referenced but never shipped, so it showed up blank/broken in Lightroom's Publishing Services panel.* | ||
| - Lightroom plug-in: re-publishing a photo (after an edit, or via "Republish") uploaded a new file without ever removing the previous one, so the online gallery accumulated several versions of the same photo. The plug-in now deletes the old remote file before uploading the new one. · *Re-publishing a photo (after an edit, or via "Republish") uploaded a new file without ever removing the previous one, so the online gallery accumulated several versions of the same photo. The plug-in now deletes the old remote file before uploading the new one.* | ||
| - Lightroom plug-in: the embedded original-file hash could land before a leading JFIF (APP0) segment in the uploaded JPEG, which some strict readers (print lab intake, some third-party tools) don't tolerate. It's now inserted after APP0 when one is present. · *The embedded original-file hash could land before a leading JFIF (APP0) segment in the uploaded JPEG, which some strict readers (print lab intake, some third-party tools) don't tolerate. It's now inserted after APP0 when one is present.* | ||
| - Lightroom plug-in: the renamed-file recovery pass could silently skip files when several Lumio uploads shared the same original-file hash (e.g. virtual copies of one master) — only one of them was ever attempted. All of them are now resolved. · *The renamed-file recovery pass could silently skip files when several Lumio uploads shared the same original-file hash (e.g. virtual copies of one master) — only one of them was ever attempted. All of them are now resolved.* | ||
| - Worker: processing a file with no original-file hash to report (i.e. almost every file, since only the Lightroom plug-in ever produces one) was quietly turning `exif` from `NULL` into `{}`. The `exif` column is now left untouched when there's nothing to write into it. · *Processing a file with no original-file hash to report (i.e. almost every file, since only the Lightroom plug-in ever produces one) was quietly turning `exif` from `NULL` into `{}`. The `exif` column is now left untouched when there's nothing to write into it.* | ||
| - Lightroom plug-in: the per-photo "Show public gallery" entry always failed ("Gallery slug or host is missing"), because the collection info Lightroom actually hands to that hook has no `collectionSettings` field to read from. Fixed by fetching the collection's settings through the photo instead (credit: [@canja006](https://github.com/canja006), tested against a real Lightroom Classic install). · *The per-photo "Show public gallery" entry always failed ("Gallery slug or host is missing"), because the collection info Lightroom actually hands to that hook has no `collectionSettings` field to read from. Fixed by fetching the collection's settings through the photo instead (credit: [@canja006](https://github.com/canja006), tested against a real Lightroom Classic install).* | ||
| - Lightroom plug-in: a published collection bound to an *existing* Lumio gallery (picked from the dropdown, rather than created from the plug-in) never had its gallery slug saved, leaving the public-gallery link dependent on a cache that goes stale as soon as the gallery is edited in Studio. It now self-heals on the next publish. · *A published collection bound to an existing Lumio gallery (picked from the dropdown, rather than created from the plug-in) never had its gallery slug saved, leaving the public-gallery link dependent on a cache that goes stale as soon as the gallery is edited in Studio. It now self-heals on the next publish.* | ||
| - Lightroom plug-in: recovering renamed files by content hash took over 4 minutes on a 2774-photo catalog (the default search scope), with Lightroom visibly sluggish throughout — measured on real hardware. The pass now stops as soon as every renamed file has been found, skips photos already matched by filename, and, most importantly, embeds the original file's byte size alongside its hash so most catalog photos can be ruled out with a plain file-size check instead of being fully read into memory just to be hashed. · *Recovering renamed files by content hash took over 4 minutes on a 2774-photo catalog (the default search scope), with Lightroom visibly sluggish throughout — measured on real hardware. The pass now stops as soon as every renamed file has been found, skips photos already matched by filename, and, most importantly, embeds the original file's byte size alongside its hash so most catalog photos can be ruled out with a plain file-size check instead of being fully read into memory just to be hashed.* | ||
| - Lightroom plug-in: the log-file location documented for macOS was wrong for current Lightroom Classic versions (confirmed on 15.5: `~/Library/Logs/Adobe/Lightroom/LrClassicLogs/Lumio.log`, not `~/Documents/LrClassicLogs/`). · *The log-file location documented for macOS was wrong for current Lightroom Classic versions (confirmed on 15.5: `~/Library/Logs/Adobe/Lightroom/LrClassicLogs/Lumio.log`, not `~/Documents/LrClassicLogs/`).* |
| data = open(path, "rb").read() | ||
| with open(path, "wb") as f: | ||
| f.write(data[:2] + segment + data[2:]) |
|
Thanks both — @canja006, that report was worth more than any amount of review from my side, and the Copilot re-reviewed on the 8th, five days after the last push, so three things are still open. One of them matters. The size pre-filter breaks recovery for targets without Small blast radius, since Worth flagging how this slipped through: @canja006 had the exact case in hand (60 files, one hash, no sizes) and reported it as behaving as described — but that was reading the data, not running a recovery in that gallery. Easy trap, and my own review missed it too. CHANGELOG is meant to be German + English. The entries here are English, then Third, trivial: On the restart question — yes, please put it in the plug-in README. It cost two of @canja006's results here, and the next person testing a plug-in change will hit the same thing. Worth being blunt about it: a full Lightroom restart, "Reload" in the Plug-in Manager isn't enough. Once those are in and @canja006 has had a look at the recovery case specifically, this is good to merge from my side. |
|
Fair point — that was read off the data, not run. I have the exact case here (one file with |
Three findings from Copilot's 2026-09-08 review on PR markusthiel#27, left open by the previous fix commit: - ImportSelectionTask.lua: the rename-recovery size pre-filter added in the last commit skipped hashing any candidate whose size didn't match sizesToCheck. But sizesToCheck is only populated from targets that carry an originalSize -- a target with originalMd5 but no originalSize (older embeddings, from before this PR's last commit added size) was silently unrecoverable, since no candidate size would ever match it. Now tracked via sizelessRemaining: the size filter only excludes a candidate once every size-less target has already been resolved. - CHANGELOG.md: every [Unreleased] bullet duplicated its English sentence a second time in italics after " · ". Every other version section in this file (0.74.1, 0.74.0, 0.73.5, ...) uses a single plain-English sentence -- the bilingual " · *...*" format is only ever used in the file's header/preamble, never in a version's own entries. Stripped the duplicate from all 9 affected bullets. - test_exif_meta.py: _stamp_original_md5 read the JPEG via open(path, "rb").read() without closing the handle. Wrapped in a context manager. Verified: luac -p on ImportSelectionTask.lua; full worker test suite (23/23) passes unmodified. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Addressed the remaining findings from Copilot's 2026-09-08 review:
Verified: |
|
Ran it, before and after, on the exact case. Setup: the gallery of 63 files, one of them (
So it reproduced exactly as described — silently unrecoverable, and fast precisely because it never hashed anything — and The 38.8 s is worth recording next to the 1.44 s from the all-sizes case: while a size-less target is outstanding the pre-filter is off, so the scan is back to reading candidates in full, and it ends as soon as that target is found. Correct-then-fast rather than fast-then-wrong, which is the right way round, and it only costs anything for files published by an intermediate build. Two things about my test case rather than your code, for the record:
Nothing else outstanding from me on this one. |
|
@canja006 thank you for running the exact case, before and after — that table is exactly the confirmation this needed. The byte-identical-second-master note is a fair callout on your test setup rather than the code: pass/fail didn't depend on which of the two identical files got matched, and that's the "one hash, several candidates" situation the Nothing outstanding from my side either. @markusthiel this one's fully closed out now — two rounds of Copilot findings addressed, three rounds of real-device testing from canja006, all clean. Ready whenever you get to it. |
Closes #26
Summary
icon.png/icon@2x.png, generated from the existing Lumio brand mark). It was referenced inLumioPublishService.luabut never added to the plug-in folder, so it showed up blank/broken in Lightroom's Publishing Services panel.uploadOnePhotonow deletes the old remote file (viarendition.publishedPhotoId, set by LR from the previous publish) before uploading the new one, instead of only ever adding new files.JpegXmp.lua). The worker extracts it via the existingexiftoolcall and stores it under the already-present (and previously unused)exif.lumio.originalMd5JSON field — no schema migration.ImportSelectionTask.luauses it to auto-resolve ambiguous filename matches and, with a new opt-in dialog option, to recover files renamed after publishing./studio/<galleryId>) instead of the public customer-facing gallery link (/g/<slug>) — a photographer clicking this wants to manage the gallery, not see the customer's view. This also drops the slug-from-cache lookup that only existed to support the old public link.Test plan
.luafiles passluac -psyntax checkexiftool -G1correctly reads backXMP-lumio:OriginalMD5with no custom config neededapps/worker/exif_meta.py's newextract_original_md5/extract_metadataverified against that same stamped JPEG; existing 13-testtest_exif_meta.pysuite still passes unmodifiedexifJSONB merge (COALESCE(exif,'{}') || COALESCE(new,'{}')) against a throwaway Postgres instance: sets the value, leaves otherexifkeys untouched when no hash is given, and correctly overwrites only thelumiokey on a second writetsc --noEmitpasses onapps/apiwith the updatedplugin.tsLrMD5.digestreturn format, re-publish flow, Selection-Import with a renamed file, the new Studio deep link) — I don't have a full LrC install in this environment, would appreciate a second pair of eyes here🤖 Generated with Claude Code