Skip to content

Preserve image credits when editing with the AI Image Editor (BL-16603) - #8101

Draft
hatton wants to merge 12 commits into
masterfrom
BL-16603-preserve-ai-edited-image-credits
Draft

Preserve image credits when editing with the AI Image Editor (BL-16603)#8101
hatton wants to merge 12 commits into
masterfrom
BL-16603-preserve-ai-edited-image-credits

Conversation

@hatton

@hatton hatton commented Jul 23, 2026

Copy link
Copy Markdown
Member

Editing a book image with the AI Image Editor lost the image's credits (copyright / creator / license).

Why: a result comes back as a fresh ai-image*.png with no embedded (EXIF/XMP) metadata. The credits live on the DOM (data-copyright / data-creator / data-license), but Bloom rebuilds those attributes from the file's embedded metadata via ImageUpdater.UpdateImgMetadataAttributesToMatchImage (run on book load and data-div sync), so they were overwritten with empty strings on the next sync.

Fix — the AI image editor decides, Bloom embeds. Credits now travel in both directions over the host protocol:

  • Outbound: each bookImages entry in the launch payload carries that image's current credits, read from its file metadata (GetCreditsForImageFile).
  • Inbound: every commit replacement returns an explicit credits value, and EmbedCreditsInNewImageFile writes exactly that into the new file (before the current-page early-return, so it covers both the off-page book-DOM path and the live current-page path).

Bloom never guesses. When the AI image editor sends no credits — null, or an object whose every field is empty — that is its answer, and the generated file is left with none. It has to work that way: a user can open an existing illustration, generate something entirely new while in the tool, and click OK, and that new image is not entitled to the old illustration's credits. The editor is the only side that knows which happened. It already implements this contract (bloom-ai-image-tools c2cc9b9, shipped in the pinned dist-v0.1.1): a result inherits credits only when the tool actually edited the target image, and reference-only generations, text-to-image, and in-editor uploads never inherit them.

Returning without touching the file also means a user-uploaded result keeps its own embedded metadata, which is the only credit information anyone has for it. (That closes the Greptile P1 on this PR.)

Also in here: the current-page save fix (a replacement on the open page is now persisted, so a re-commit's oldSrc still matches the live page), the dev/alpha gate on the editor's Local Dummy model, and a bloom-automation recipe for driving the editor over CDP.

Terminology: inside Bloom, "editor" is ambiguous, because Bloom's own edit tab is an editor too. Comments and private member names in AiImageEditorApi, its tests, and aiEditorLauncher now say "AI image editor" throughout. Wire and environment names (editorUrl, BLOOM_AI_EDITOR_URL, the aiImageEditor routes, the .ai-image-editor folder) are unchanged.

Found while processing review comments on the merged PR #8033.

Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16603

Devin review


This change is Reviewable

Editing a book image with the AI Image Editor lost the image's credits
(copyright/creator/license). The editor writes the result to a fresh
ai-image*.png that has no embedded metadata; although the credits are
preserved on the DOM (data-copyright/creator/license), Bloom rebuilds
those attributes from the file's embedded metadata
(ImageUpdater.UpdateImgMetadataAttributesToMatchImage, run on book load
and data-div sync), so they were overwritten with empty strings.

Fix: in HandleReplacement, after writing the replacement file, copy the
replaced image's intellectual-property metadata into the new file (new
CarryCreditsToNewImageFile). It runs before the current-page early return
so it covers both the off-page book-DOM path and the live current-page
path, and is gated to freshly generated/uploaded results (reused existing
images keep their own file metadata). Adds unit tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Preserves image-credit metadata across AI Image Editor replacements and persists current-page edits.

  • Adds bidirectional image-credit data to the editor protocol and embeds selected credits into newly generated or uploaded replacement files.
  • Persists live current-page replacements through Bloom’s save-and-rethink flow.
  • Updates the AI image editor dependency and adds unit tests for metadata copying and license reconstruction.
  • Adds automation guidance and a CDP driver for exercising the AI editor with the local dummy model.

Important Files Changed

Filename Overview
src/BloomExe/web/controllers/AiImageEditorApi.cs Adds image-credit serialization, metadata reconstruction, and embedding for AI-generated or uploaded replacement files.
src/BloomBrowserUI/bookEdit/toolbox/canvas/aiEditorLauncher.ts Saves and rethinks the current page after applying live AI image replacements.
src/BloomTests/web/controllers/AiImageEditorApiTests.cs Adds coverage for credit copying, editor-provided metadata precedence, empty credits, and custom or Creative Commons licenses.
src/BloomBrowserUI/package.json Updates bloom-ai-image-tools to dist-v0.1.1, with the resolved commit and integrity recorded in the lockfile.
.github/skills/bloom-automation/driveAiImageEditor.mjs Adds a CDP automation helper for inspecting and exercising the AI Image Editor flow.

Reviews (3): Last reviewed commit: "Docs: dummy model is offered on dev AND ..." | Re-trigger Greptile

Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs Outdated
Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs Outdated
Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs Outdated
@hatton

hatton commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-23 21:32 UTC up to commit 9cb92e5f77d563b6e15765a93acbfcc1bb04558d.

Result: 0 bugs, 2 Investigate flags (no Informational). Both on the new CarryCreditsToNewImageFile:

  • webp results may still lose credits (gated, non-regression) — left open for a developer decision.
  • current-page path relies on front-end changeImage preserving file metadata — assessed as a non-issue (changeImage doesn't rewrite the file; re-processing preserves IP metadata) and resolved.

CI/other bots tracked separately in this preflight run.

hatton and others added 2 commits July 23, 2026 18:46
…ummy model (BL-16603)

- Protocol: send each book image's IP credits to the editor and embed the credits
  it returns on commit (AiImageEditorApi: GetCreditsForImageFile,
  CommitReplacement.credits, MetadataFromCredits/BuildLicense). License travels as a
  single string (CC URL, else custom rights statement) to match the editor's
  ImageCredits. Adds unit tests.
- Fix lost credits / stale editor on the current page: after a current-page
  replacement, save+rethink the page so storage matches the live DOM
  (aiEditorLauncher.ts). Previously the swap only hit the live DOM, so reopening the
  editor showed the pre-edit image and a re-commit's oldSrc no longer matched
  ("0 of N could be updated").
- Offer the free "Local Dummy (No AI)" editor model on developer AND alpha builds
  (showDeveloperTools = ApplicationUpdateSupport.IsDevOrAlpha) so human testers can
  exercise the flow without spending AI credits.
- Fix undefined toPowerShellLiteral in processTree.mjs that crashed every
  ./go.sh --with run.
- Bump bloom-ai-image-tools to dist-v0.1.1 (editor side of the same protocol).
- Add bloom-automation docs + CDP driver for driving/testing the AI editor.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs Outdated
…L-16603)

Correct the AI-editor testing docs and driver comment, which still said the free
"Local Dummy (No AI)" model appears only on developer builds and that shipped
builds never expose it. The gate is ApplicationUpdateSupport.IsDevOrAlpha, so
alpha/unstable testers get it too. (Caught by Devin review.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hatton

hatton commented Jul 24, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-24 01:1x UTC up to commit 0f17c3dcb5.

1 Bug — "docs describe the wrong build gate": this was correct on the prior commit and is fixed in 0f17c3dcb5 (the docs + driver comment now say IsDevOrAlpha / dev+alpha). Devin's re-review still quotes the superseded text, so it's a false positive against the current HEAD.

3 Investigate flags (all assessed, none blocking):

  • webp metadata may silently fail — replied & resolved: not a regression, logged-not-fatal, PNG/JPG verified end-to-end.
  • current-page relies on changeImage preserving file metadata — verified end-to-end via a dummy-model edit (committed file kept its embedded credits); thread resolved.
  • empty editor credits don't fall back to the replaced file — intentional per design (editor owns the credit decision) and covered by CarryCreditsToNewImageFile_EmptyEditorCredits_LeavesGeneratedFileClean.

6 Informational — not mirrored (low signal).

Greptile (Greptile Review check ✓) raised one P1 ("uploaded credits still overwritten") now routed to the developer. CI checks green.

hatton and others added 2 commits July 30, 2026 14:40
Bloom no longer falls back to the replaced image's file metadata when the AI
image editor sends no credits for a committed result. That fallback was wrong:
a user can open an existing illustration, generate something entirely new while
in the tool, and click OK, and the new image is not entitled to the old one's
credits. Null credits and an all-empty credits object now mean the same thing,
"this result has no credits", and the generated file is left with none.

The AI image editor already implements this contract (bloom-ai-image-tools
c2cc9b9, shipped in the pinned dist-v0.1.1): every commit replacement returns an
explicit credits value, and a result inherits credits only when the tool
actually edited the target image. Bloom was the side still guessing.

This also fixes the Greptile P1 on this PR about uploaded results: because we
now return before touching the file, a user-uploaded result keeps its own
embedded metadata instead of having the replaced slot's credits written over it.

CarryCreditsToNewImageFile becomes EmbedCreditsInNewImageFile and loses its
replacedImageSrc parameter; the ReadReplacedImageCredits helper is deleted. The
two fallback tests are replaced by a TestCase pair covering null and all-empty,
each with a credit-bearing replaced image in place to prove nothing leaks.

Terminology: inside Bloom, "editor" is ambiguous, since Bloom's own edit tab is
an editor too. Comments and private member names in AiImageEditorApi, its tests,
and aiEditorLauncher now say "AI image editor" throughout, and the class header
states the term. Wire and environment names (editorUrl, BLOOM_AI_EDITOR_URL, the
aiImageEditor routes, the .ai-image-editor folder) are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hatton
hatton marked this pull request as ready for review July 30, 2026 20:48
hatton and others added 4 commits July 30, 2026 14:57
… (BL-16603)

Devin flagged this on the PR. The current-page save was gated on
currentPageApplied > 0, but that counter was only assigned after
applyCurrentPageReplacements returned. If changeImageByElement threw part-way
through the loop, control jumped to the catch, the counter stayed at 0, and the
save never fired -- while the live page DOM already held the swaps made before
the throw. The user's AI edit then silently disappeared on the next page load,
and re-editing that image reported "0 of N could be updated", which is exactly
the stale-storage condition that save call was added to prevent.

The count now happens per swap as it lands, via an onApplied callback, so it
survives an exception and the save fires for whatever actually got applied. The
ack to the AI image editor still reports the failure; this only stops us
discarding the work that succeeded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rough (BL-16603)

Devin noted (Informational) that the front-end's type for the `commit` message
omits the new `credits` field. Harmless today: the handler forwards the
`replacements` array to C# by reference, so every field the AI image editor sent
arrives intact whether or not TypeScript knows about it.

It is a trap for later, though. The declared type said the wire carried only
incomingId and resultId, so the obvious-looking refactor -- rebuild the array
field by field on the way to C# -- would silently drop credits and put us right
back in the bug this whole feature exists to prevent. Declaring the fields makes
the contract visible at the one place someone would touch it. `sourceUrl` was
missing for the same reason and is now declared too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e (BL-16603)

Bloom mirrors an image file's IP metadata on the element as data-copyright /
data-creator / data-license, and the edit tab shows its "missing information"
indicator when data-copyright is empty. When the AI editor committed a
replacement for the CURRENTLY-OPEN page, the front-end read those three
attributes off the element it was replacing and carried them forward, so a
result with no credits kept the old illustration's credits in the DOM: the
indicator said the credits were fine while the file -- and the credits dialog
-- had none. They then vanished silently at the next book-up-to-date pass,
which re-derives the attributes from the file.

Off-page slots had the same gap: HandleCommit set the new src but left the
attributes describing the old file.

Now the attributes come from the new file in both cases. A new
ReadCreditAttributes reads that file's embedded metadata into the three
attribute values (matching what ImageUpdater writes, so data-license is the
short "cc-by" token rather than the wire URL), and the commit reply carries
them for current-page slots; off-page slots call Bloom's own
ImageUpdater.UpdateImgMetadataAttributesToMatchImage once the new src is in
place. The front-end takes the values from the reply instead of the element.

Verified end-to-end against a running Bloom, both directions: a dummy-model
edit of a credit-less source now shows the missing-credits indicator, and one
of a credited image keeps its credits in the DOM, the file, and the launch
payload alike.

Also fixes three bits of drift in the checked-in driveAiImageEditor.mjs helper
that made this verification harder than it should have been (it only
recognized the editor at its dev-server URL, two editor selectors had gone
stale, and a screenshot timeout could swallow the run's report), and logs a
papercut about that cross-repo selector contract.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hatton
hatton marked this pull request as draft July 30, 2026 22:51
… (BL-16603)

Two fixes from Devin's review of the previous commit.

GetCreditsForImageFile resolved the image with a plain Path.Combine, but a
book's image src can still be percent-encoded (BL-3901), which the rest of
Bloom handles with UrlPathString.GetFullyDecodedPath. For such an image the
lookup missed, so the launch payload said credits:null -- the AI editor then
believed the picture had no credits and any result derived from it correctly
got none. That is the very loss this branch exists to prevent, just narrowed
to filenames with characters that get encoded. Now it decodes the same way
Bloom does everywhere else, with a test that pins it (a name with a space,
looked up in its %20 form).

Also replaces a stacked ternary in the driveAiImageEditor.mjs test helper with
an if/else chain, which AGENTS.md asks for explicitly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@@ -355,6 +394,21 @@ export const launchAiImageEditor = (
if (finalOk) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Opus 5 from Hatton's machine during preflight] [Devin] Bug: AI image editor window can become unclosable when a commit only partly succeeds

When a commit partly succeeds — say one picture on another page couldn't be replaced, while a picture on the page you're looking at was — this finally block does two things: it leaves the AI editor panel on screen (so you can see the error), and it asks Bloom to save and re-think the current page.

Those two don't mix. The editor panel is attached to Bloom's top-level window, but all the code that runs it — the ✕ button's handler, and the listener that receives messages from the editor — belongs to the page iframe. Saving and re-thinking the page reloads that iframe, which throws away the code while leaving the panel itself on screen. The result is a full-screen panel that no longer responds to its own close button, so the user has to restart Bloom.

Before this branch no save was issued here, so a partial failure left a panel that still worked.

This has been routed to the developer as a decision, because the fix involves a real trade-off: closing the panel before the reload is the smallest change but throws away the error message the user was meant to read, whereas keeping the panel usable across the reload means moving its ownership out of the page iframe, which is a bigger structural change.

/// otherwise the free-text rights statement for a custom license. Null when there is
/// no license.
/// </summary>
private static string LicenseToWireString(LicenseInfo license)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Opus 5 from Hatton's machine during preflight] [Devin] Bug: Extra license notes on a picture are dropped when it is edited with the AI Image Editor

Bloom lets a picture carry both a license type (say CC-BY) and free-text license notes — wording the user typed about how the picture may be used. When we hand a picture's credits to the AI editor, LicenseToWireString collapses the whole license down to a single string, and for any license that has a URL it sends only the URL. The notes never travel. On the way back, BuildLicense rebuilds a bare CC license from that URL, so the new picture's file is written without them.

What the user sees: they typed notes about how an illustration may be used, ran an AI edit on it, and the notes are gone.

There is a second variant. A picture whose license is "contact the copyright holder" (a NullLicense) plus notes has no URL, so the notes text itself becomes the wire string. Coming back, that text isn't a creativecommons.org URL, so it is rebuilt as a custom license — quietly changing the license type.

This has been routed to the developer as a decision, because the fix is a change to the wire format Bloom and the editor share: the rights statement needs its own field (and probably a license token alongside the URL), which means a matching change in the bloom-ai-image-tools repo. That is a cross-repo contract call, not something to change unilaterally from this side.

Comment thread .github/skills/bloom-automation/driveAiImageEditor.mjs
Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs
Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs
Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs
@hatton

hatton commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin on 2026-07-30 23:2x UTC up to commit 69dc1398da.

Devin ran twice this preflight — once on 120bb21f67 and again after the fixes, on 69dc1398da. On the final commit it reports 6 bugs (4 of which it now considers fixed), 6 Investigate flags, and 11 Informational items (not mirrored — low signal).

Two things it raised are fixed in 69dc1398da, each with its own thread:

  • A picture whose filename needs percent-encoding had its credits reported as absent at launch, so an AI edit of it correctly produced a result with no credits — the very loss this branch prevents, narrowed to those filenames. Now decoded the way the rest of Bloom does, with a test.
  • A stacked ternary in the new test-automation helper, which AGENTS.md asks us not to write.

Three Investigate flags were assessed as non-issues and their threads replied to and resolved: the live-page credit attributes (fixed by this branch — Devin's own bug list agrees), the deliberate no-fallback for empty editor credits (confirmed against a running Bloom that the editor really does echo the source's credits back), and the encoding one above. Two more — the webp metadata limitation and whether the current-page changeImage flow preserves the file's metadata — were already assessed and documented on resolved threads earlier in this PR, so they weren't duplicated.

Two findings are left open for the developer, both genuine and both needing a call I shouldn't make alone:

CI is green (pr-automation, track). Greptile stayed quiet on both commits.

Two decisions from the preflight review of this branch.

1. The editor panel could become unclosable after a partly-failed commit.
   When a commit succeeded for a slot on the current page but failed for one on
   some other page, the launcher deliberately kept the panel up so the user
   could read the error -- and at the same time fired
   common/saveChangesAndRethinkPageEvent to persist the live-DOM swap that had
   landed. Those two fight each other: the save reloads the page frame, and
   everything that operates the panel (its message listener, the close button's
   handler, the cleanup function itself) is code belonging to that frame, even
   though the overlay div lives in the top window. So the reload left a
   full-screen overlay with nothing able to close it, escapable only by
   restarting Bloom.

   The save is now deferred to cleanup(): on success it happens as soon as the
   overlay comes down, exactly as before, and on a partial failure it waits for
   the user to close the overlay they are reading the error in. The new
   aiEditorLauncher.test.ts covers both paths, plus a commit that changed
   nothing on this page and so must never save (an unnecessary page reload
   would discard unsaved text edits).

2. A picture's license notes were lost when it was AI-edited. ClearShare lets
   an image carry a Creative Commons license AND free-text "license notes"
   spelling out extra restrictions, but the credits we handed the AI image
   editor flattened the license into a single string, which for a CC license
   meant just the URL. The notes never travelled, so they could not come back,
   and the edited copy lost them. The wire now carries licenseUrl and
   licenseRightsStatement as separate fields -- which is what an image file
   actually holds (cc:license plus dc:rights).

   BuildLicense deliberately mirrors ClearShare's own LicenseUtils.FromXmp, the
   code that reads a license back out of a file: a CC URL becomes a CC license,
   failing that a rights statement becomes a CustomLicense, and nothing at all
   becomes a NullLicense. Matching FromXmp is what makes the round trip
   faithful, and it is also why no license-TYPE token rides on the wire: a
   token could only express licenses a file cannot store. In particular, an
   "ask the copyright holder" license that also has notes reads back from any
   image file as a custom license, so that is not something this code can or
   should preserve. A test pins that reasoning, and will fail if ClearShare
   ever starts distinguishing more license types in the file.

The matching bloom-ai-image-tools change is types and fixtures only: that
editor carries the credits object verbatim (it never reads a field, and
serializes the object whole), so the new fields already survive the published
dist-v0.1.1 build and no dependency bump is needed here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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