BL-16822 Inline images survive a spreadsheet round trip

[hatton]

A translator's spreadsheet has one row per text block, so a picture inside a block
had nowhere to go: exporting a book with one and importing it back lost the picture.

The export now writes one [inline image] row per picture, directly after the row of
the block it belongs to, carrying the file name in the image column. Which side the
picture is docked to, how wide it is, and how far down the block it sits go into a
new hidden [details] column as self-identifying JSON, so a spreadsheet a person edits
by hand shows them a picture and its file and nothing they can break by accident.

Two things are deliberately left out of that column. The aspect ratio, because we
never stretch an image: the file named in the same row's [image source] is the
authority and the importer measures it. And the id, because it does not need to
survive: every lookup is scoped to one translation group, so all the id has to
guarantee is that a group's per-language copies of one picture share a value and two
pictures differ. The importer mints new ones to that rule, which
ImageIdsAgreeAcrossEditablesAndDifferBetweenImages pins.

The importer reads those rows back, rebuilds the wrapper in every language of the
group, and puts them in the order the export wrote. A spreadsheet made before this
change has no such rows and imports as it always did.

The [details] column is hidden and excluded from the wysiwyg-formatted columns, which
is what stops SpreadsheetIO trying to interpret its JSON as rich text.

Card: https://issues.bloomlibrary.org/youtrack/issue/BL-16822

---

🤖 Generated with Claude Code

https://claude.ai/code/session_014DCBGajN5YyAYPBenEf1yy

Devin review

---

This change is 

[hatton]

✅ Assessed, no change needed — see the outcome at the end of this comment.

[Devin] Investigate: PR target conflicts with guidance

AGENTS.md:1 -- outside the diff, so this is not resolvable as a thread.

Repository guidance targets new work at Version6.5. This stack follows master. Confirm an exception or retarget the stack.

---

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Not an issue -- same answer as on the earlier PRs of the stack. This work targets master (6.6) by the developer's decision; the Version6.5 line is in AGENTS.md's TEMPORARY section.

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin on 2026-09-07 12:46 MDT up to commit ac6ce1ba72.

It raised 6 things. Each has its own review thread with the reasoning and the outcome; in short:

• Deleted pictures return after import — real: a picture removed from the spreadsheet stayed invisibly in the block and came back the next time a language was added. Fixed.
• Bottom pictures gain blank space — real: importing a bottom-docked picture added a blank line under the text. Fixed.
• Image identity claim differs — right, and the PR description was wrong; corrected.
• Existing pictures overwritten during import — left as is: this is what import already does for every other image, and it saves a copy of the book folder first.
• Canonical picture selection differs — makes no difference in practice; every editable holds the same markup by the time a book can be exported.
• PR target conflicts with guidance — not an issue, as on the other PRs of the stack.

The fixes were amended into this branch after the review, so the commit above is not the current head; a fresh review is running against the new one. The only GitHub check on this PR is pr-automation (the review trigger), which passed; the front-end unit suite (915 tests), the C# spreadsheet tests (415) and the end-to-end suite (85) were all run locally and are green.

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin again on 2026-09-07 14:00 MDT, on commit ac0ac2fa11 (the head after the first round of fixes).

It raised 3 new things. Each has its own review thread with the reasoning and the outcome; in short:

• Imported offsets ignore layout changes — real: the export dropped the block size the offset was measured against, so an import into a different layout kept the old displacement and pushed text off the end. Fixed, with a test.
• Blank image-only blocks lose pictures — not an issue: a picture-only block exports the blank-content indicator, which counts as text, and ImageOnlyEditableIsNotDeleted covers that round trip.
• Authority marker couples unrelated details — a real coupling; the constraint is now written where the test is, since the alternatives would undo the fix for deleted pictures returning.

It also re-reported the findings the reviewed commit already fixes; those are the first round's threads, which carry their outcomes.

These fixes were amended into this branch after the review, so the commit above is not the current head. The only GitHub check on this PR is pr-automation (the review trigger). Run locally and green after these fixes: the front-end unit suite (920 tests), the end-to-end suite (85 passed, 1 skipped), and the C# spreadsheet and translation-group tests (417 and 50).

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin a third time on 2026-09-07 15:55 MDT, on commit 646e989bbb (the head after the second round of fixes).

It raised 2 new things. Each has its own review thread with the reasoning and the outcome; in short:

• Image-description pictures disappear — real: an image description is exported as a cell on its image's row, so no picture rows can follow it, and the importer read that as "this group has none" and cleared what the book had. Fixed on both sides, with a test — and the editor no longer offers Add Image inside an image description at all (PR 8321).
• Picture transparency settings disappear — real: whether a picture's white is transparent is held as a class on the img, and the round trip dropped it. Fixed, with a test.

It also re-reported the findings the reviewed commit already fixes; those are the earlier rounds' threads, which carry their outcomes.

These fixes were amended into this branch after the review, so the commit above is not the current head. The only GitHub check on this PR is pr-automation (the review trigger). Run locally and green after these fixes: the front-end unit suite (927 tests), the C# spreadsheet and translation-group tests (470), and the end-to-end suite.

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin a fourth time on 2026-09-07 16:20 MDT, on commit f61e11e6b9 (the head after the third round of fixes).

It raised 3 new things. Each has its own review thread with the reasoning and the outcome; in short:

• Retained markup overrides image rows — real, and worse than it sounds: the language cell carried the wrapper markup, RetainMarkup wrote it back verbatim, and StampInlineImages will not stamp over pictures already there — so every change made through the picture rows, deletions included, was ignored. The export no longer puts wrapper markup in the language cell. Fixed, with a test.
• Geometry values bypass format validation — fair: the [details] cell is hidden but editable, and its width and offset go straight into the wrapper's style. Both are now checked against a length, with a warning naming the row. Fixed, with a test.
• Inline-image rows expose their identifiers — they are hidden already: InternalSpreadsheet.HiddenColumns covers [image source] and [details], and what shows is the [row type] cell, as on every existing image row.

It also re-reported the findings the reviewed commit already fixes; those are the earlier rounds' threads, which carry their outcomes.

These fixes were amended into this branch after the review, so the commit above is not the current head. The only GitHub check on this PR is pr-automation (the review trigger). Run locally and green after these fixes: the inline-image unit tests (47), the C# spreadsheet tests (422), and the nine end-to-end specs the cleanup touched.

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin a fifth time on 2026-09-07 16:50 MDT, on commit 11a810bf32 (the head after the fourth round of fixes).

It raised 2 new things. Each has its own review thread with the reasoning and the outcome; in short:

• Description pictures vanish across books — the premise does not hold, and checking it corrected something I said last round: a spreadsheet cell holds MarkedUpText (paragraphs, bold, italic, underline), so a wrapper div cannot survive the xlsx at all. Markup in a cell therefore cannot move a picture to another book — and an import from a file was never at risk of writing old wrappers into an editable either, which is the harm I claimed for "Retained markup overrides image rows". Keeping the markup out of the language cells is still right, for a consumer that reads a sheet without going through a file (the importer's own tests do), and the code comment and test now say that instead. Pinned by PictureMarkupCannotTravelInACell.
• Focused tests were unavailable — noted: SpreadsheetInlineImageTests runs locally (423 spreadsheet tests, green). This repo runs no C# suite per PR.

It also re-reported the findings the reviewed commit already fixes; those are the earlier rounds' threads, which carry their outcomes.

These fixes were amended into this branch after the review, so the commit above is not the current head. The only GitHub check on this PR is pr-automation (the review trigger). Run locally and green after these fixes: the whole front-end unit suite, the C# spreadsheet tests (423), and the inline-image end-to-end specs.

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin a sixth time on 2026-09-07 17:30 MDT, on commit 2b5c56f819 (the head after the fifth round of fixes).

It raised 1 new thing. Each has its own review thread with the reasoning and the outcome; in short:

• CLI image metadata needs review — real: the ordinary [image] import mirrors the file's copyright, creator and license onto the img as it copies it, and the inline-image path did not. A book opened in the editor would have them filled in by the next up-to-date pass, but a command-line import gets no such pass, and image credits read those attributes. The importer now makes the same call. Fixed, with a test.

It also re-reported the findings the reviewed commit already fixes; those are the earlier rounds' threads, which carry their outcomes.

These fixes were amended into this branch after the review, so the commit above is not the current head. The only GitHub check on this PR is pr-automation (the review trigger). Run locally and green after these fixes: the whole front-end unit suite, the C# spreadsheet tests (424), and the inline-image end-to-end specs.