Re-encode an AI PNG that bloats the JPEG it replaces (BL-16645)

[StephenMcConnel]

The AI image editor commonly hands back a PNG for a slot the book held as a JPEG, and a photographic PNG can be several times the size of the JPEG it replaced — the same book-folder bloat that ImportImageIntoBookFolder was added to avoid.

So after importing a generated result, if the new file is a PNG substantially bigger (>1.5×) than the JPEG it supersedes, run it through ImageUtils.TryChangeFormatToJpegIfHelpful and keep the JPEG when the saving is real (that helper insists on at least 50% smaller, and cleans up after itself otherwise).

Details worth a reviewer's eye

• The JPEG gets a freshly reserved ai-image* name, rather than the PNG's name with the extension swapped. GetUnusedFilename only checked the .png name, so the matching .jpg can be another slot's live image — writing the conversion over it would destroy that image. TryChangeFormatToJpegIfHelpful also warns that a pre-existing destination file can make GraphicsMagick fail outright.
• The superseded PNG is deleted. Nothing references it, and leaving it behind would keep exactly the bulk we just converted away from.
• Only a freshly generated/uploaded result is converted. A reused book image carries its credits inside its own file and nothing re-writes them afterwards, and re-encoding runs the file through GraphicsMagick, which can drop them — the same reason the reuse path doesn't resize (see resizeIfNeeded).

The logic lives in its own internal helper so it can be tested. Five new tests in AiImageEditorApiTests cover the conversion, the PNG deletion, the name-collision case, and the three cases that must leave the PNG alone.

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

Devin review

---

This change is 

[StephenMcConnel]

[Claude Opus 5 (1M context) during preflight] Consulted Devin on 2026-08-10, four rounds, up to commit 90ba50f50.

Devin found real problems here, and one of them mattered a lot: re-encoding a picture as a JPEG destroys any see-through areas, and this code deletes the original once it has the JPEG. That would have been silent, permanent damage. Fixed in 9f5b0b1a5, along with a decode failure that could have thrown away a whole commit's worth of image replacements. Later rounds added a stray-file cleanup (414cade60) and an unguarded delete that had the same whole-commit blast radius (90ba50f50).

Two threads are deliberately still open, because both are judgement calls about the feature rather than bugs to fix, and they are in the preflight decision report for the developer:

• Transparency is detected by sampling only the top-left corner — the guard works, but it can miss a picture that is see-through only in its middle. The cheap hardening would switch the optimization off for the many AI results that are opaque RGBA, so it is a trade, not a fix.
• A picture generated into an empty slot is never re-encoded at all — the mechanism is comparative, so with no previous JPEG there is nothing to measure against. Widening it needs its own policy.

Also of note: Devin kept re-listing the decode-failure finding in every later round, including against the commit that fixed it, with text asserting there is no try/catch where there now plainly is one. Don't read those repeats as outstanding. The remaining informational items were assessed and not acted on — the JPEG-marker coverage and the image-cache/name-reuse observation are both about pre-existing shared code and fail in the safe direction; a check-then-write window on the reserved filename is not reachable because the commit handler is synchronised.

The full C# suite is green at 90ba50f50 (3070 passed, 13 skipped). CI here runs no tests — the pr-automation check exists only to open this Devin review — so that local run is the whole test gate.

[StephenMcConnel]

[Claude Opus 5 (1M context) during preflight] Consulted Devin again after the developer's decisions came back — three further rounds, up to commit f1a596fc3. Round 7 is clean: all three bugs marked resolved by Devin, and no new findings.

Implementing the chosen transparency fix drew two more real problems, which is exactly what a re-run is for:

• A severe one that predated this branch. HasTransparency was calling every palette-based picture opaque — a PNG-8 or a GIF, which keeps its transparency in its palette rather than a per-pixel alpha channel. The "no alpha channel, therefore opaque" shortcut ran ahead of the palette check, and since every indexed pixel format leaves the alpha flags clear, the palette check was unreachable dead code. Confirmed at runtime, not by reading. Harmless while the answer only picked a display format; not harmless here, where the original gets deleted. Fixed in 93e89582f, and three other callers improve with it — palette-transparent book covers now get their cover colour instead of staying see-through.
• One of my own making. I had guarded the decode and the PNG delete against a locked file aborting the whole commit, then left the cleanup delete a few lines above unguarded — in the branch where the conversion was declined, so with no benefit on the table at all. Fixed in f1a596fc3.

Two threads carry recorded developer decisions rather than fixes: the empty-slot scope is deliberately out of scope for this PR, and the ten-pixel sampling cap is the developer's explicit choice, with the alternatives and their costs written out. Devin re-argued the cap in a later round; it was recorded as decided rather than reopened. Note that indexed pictures no longer depend on sampling at all, so that residual gap covers strictly less ground than when the decision was made.

Every bug and Investigate thread on this PR is now resolved with a documented outcome. The remaining informational items were assessed and left, for the reasons in the earlier log.

The full C# suite is green at f1a596fc3 — 3076 passed, 13 skipped, 0 failed — and was re-run after every commit. As before: CI here runs no tests, so that local run is the whole test gate.