Skip to content

Stop offering Bloom Games target images to the AI image editor (BL-16793) - #8337

Merged
andrew-polk merged 3 commits into
Version6.5from
BL-16793-OmitTargetImages
Sep 10, 2026
Merged

andrew-polk merged 3 commits into
Version6.5from
BL-16793-OmitTargetImages

Conversation

@StephenMcConnel

@StephenMcConnel StephenMcConnel commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

A Bloom Games target holds a clone of its draggable's content — image container and <img>
included — so every target looked to the AI image editor like another editable image slot. The
card's page, with a background plus three draggables, offered seven images instead of four. And
editing a clone achieved nothing: Bloom rebuilds a target's content from its draggable, so the
edit was overwritten.

EnumerateBookImages now declines any slot inside [data-target-of].

The filtering deliberately does not go in SelectImageSlotsOnPage: a slot's identity is its
index in that unfiltered list, and both slotIndexOnPage (page frame) and
applyAiImageEditorReplacements re-derive that index, so a slot we decline still has to keep its
ordinal. Live check on the Games "Drag Images to Targets" page: the editor is now offered ordinals
0, 3, 5, 7 — background plus three draggables — with 4, 6, 8 (the clones) held open but not
offered.

Because the clones are no longer editable, nothing else would repoint one when an AI edit replaces
a draggable's image on a page the user does not have open: the target would go on showing the
replaced picture, and its stale reference to the old file also stopped
DeleteSupersededAiImageFiles from reclaiming it. TryApplyReplacement now repoints those copies.
The currently-edited page needs no such work — making the swapped slot the active canvas element,
which applyAiImageEditorReplacements already does so Ctrl+Z can offer the undo, is what makes
Bloom rebuild the clone. That was verified by A/B against a running Bloom rather than assumed.

Verified against a running Bloom on a real game book, as well as by unit tests: the offered list,
the ordinal agreement between the two sides, the label renumbering, and an off-page commit
repointing both the draggable and its target's copy.

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

Devin review


This change is Reviewable

StephenMcConnel and others added 3 commits September 9, 2026 15:38
…793)

A game target holds a clone of its draggable's content -- image container
and <img> included -- so every target looked like another editable image
slot. A page with a background plus three draggables offered seven images
instead of four, and editing a clone achieved nothing: Bloom rebuilds a
target's content from its draggable, so the edit was overwritten.

EnumerateBookImages now declines any slot inside [data-target-of]. The
filtering deliberately does NOT go in SelectImageSlotsOnPage: a slot's
identity is its index in that unfiltered list, and both slotIndexOnPage in
the page frame and applyAiImageEditorReplacements re-derive that index, so
a slot we decline still has to keep its ordinal.

Because the clones are no longer editable, nothing else would repoint one
when an AI edit replaces a draggable's image on a page the user does not
have open; the target would go on showing the replaced picture, and its
reference to the old file also stopped DeleteSupersededAiImageFiles
reclaiming it. TryApplyReplacement now repoints those copies. The
currently-edited page needs no such work: making the swapped slot the
active canvas element -- which applyAiImageEditorReplacements already does
so Ctrl+Z can offer the undo -- is what makes Bloom rebuild the clone.

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

GetGameTargetImageCopiesOfSlot returned every image slot inside a matching
target, and TryApplyReplacement repoints all of them at the one replacement.
A draggable holding two pictures therefore had BOTH its target copies
repointed at whichever picture the user edited, so the other copy showed the
wrong picture.

Nothing Bloom ships builds a draggable with two pictures, so this was latent
rather than reachable -- but copyContentToTarget copies a whole bloom-canvas
when it finds one, which is exactly that shape, so leaving the pairing to
"there is only ever one" was not safe. Now the slot's position within its
draggable selects the copy at the same position within each target, which is
sound because a target's content is a copy of the draggable's.

Found by Devin's review of PR #8337.

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

Copy link
Copy Markdown
Contributor Author

[Claude Opus 5 from Steve McConnel's machine during preflight] Consulted Devin on 2026-09-09 22:16 UTC, up to commit fb741ea062e0e6bb4fd31b5561e0a553d8dcc3e4.

It raised one thing, on the first commit: a draggable holding more than one picture would have had
all of its target copies repointed at whichever single picture was edited. That is fixed in
fb741ea, with a test for it; the thread is
here. Devin's
re-review of that commit now marks the finding resolved of its own accord — "the method now finds
the source slot's position within its draggable and returns only the corresponding target slot."

Nothing else: no flags to investigate and no informational items. CI is green, and CodeRabbit's
auto-review is switched off for this repo in .coderabbit.yml, so it was not consulted.

@StephenMcConnel
StephenMcConnel marked this pull request as ready for review September 9, 2026 22:40

@andrew-polk andrew-polk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@andrew-polk reviewed 4 files and all commit messages, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on StephenMcConnel).

@andrew-polk
andrew-polk merged commit 055a2c4 into Version6.5 Sep 10, 2026
1 of 2 checks passed
@andrew-polk
andrew-polk deleted the BL-16793-OmitTargetImages branch September 10, 2026 18:05
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.

2 participants