Spreadsheet: a hidden [details] column and a registration point for objects the ordinary columns cannot carry - #8313
Draft
hatton wants to merge 6 commits into
Draft
Spreadsheet: a hidden [details] column and a registration point for objects the ordinary columns cannot carry#8313hatton wants to merge 6 commits into
hatton wants to merge 6 commits into
Conversation
things on a page that the ordinary columns cannot carry Some things on a page cannot survive a spreadsheet round trip through the text, image, video and widget columns: putting them back needs state that none of those columns holds. This adds the generic mechanism for them, so that a feature which needs it registers one object and gets export, import, placement and reporting for free. It adds no such feature itself. The parts: - InternalSpreadsheet gains a hidden [details] column. A row's cell in it holds a JSON object whose "kind" property says what the row is for, so a blob identifies itself even apart from its row. HiddenColumns now tolerates optional columns being absent instead of reporting -1 as a column to hide. The column's presence anywhere in a spreadsheet marks that spreadsheet as the authority on the objects whose rows use it; a spreadsheet without it (any made by an older Bloom) can say nothing about them, so import leaves whatever the book has alone and behaves exactly as before. - SpreadsheetIO keeps [details] out of the WYSIWYG formatting and out of XML escaping, so the JSON comes back byte for byte. Escaping an ampersand in it would corrupt what we are copying verbatim. - ISpreadsheetObjectKind (SpreadsheetObjectKind.cs) is the registration point: a selector saying which page elements the kind owns, so the generic collectors of translation groups, bloom-canvases, video containers and widget containers leave their innards alone; an export method producing the object's rows; and an import method consuming the row family. Registering a kind is all it takes. - SpreadsheetExporter writes each object's rows at the object's own position among the page's [page content] rows, worked out from document order, and makes the [details] column before it calls the kind. WriteTranslationGroup, WriteVideo, ImagePath and CopyImageFileToSpreadsheetFolder become internal so a kind can put its own text, video and pictures into its rows -- text then reaching a translator in the same language columns as any other text. - SpreadsheetImporter treats such an object as a page block in its own right, so a lead row advances onto the object's page and picks the object there just as a [page content] row picks its translation group. That is the only thing that could reach a page whose sole content is such an object. The importer collects the row family, and skips it with a row-numbered warning when the sheet has no [details] column or the page has nowhere to put the object; it never invents one, since that would be an object of a shape nothing in the spreadsheet asked for. A continuation row stranded from its lead row is reported too. Warn, CurrentRowIndexForMessages, PutRowInGroupAsync, PutRowInImageAsync, PutRowInVideo and DestinationDom become internal for the use of a kind. Tests: SpreadsheetDetailsTests registers a stub kind of its own and covers the [details] column being present and hidden, the lead row carrying the kind and its state, an object's rows landing at the object's position, the object's inner text not also being exported as page content, a round trip restoring both the text and the state only [details] carries, an import into a page with nowhere to put the object reporting and skipping while still importing the rest of the page, a stranded continuation row being reported, and a spreadsheet with no [details] column importing as it always did. 11 tests; BloomTests.Spreadsheet 405/405; full BloomTests 3357 passed, 13 skipped, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… has no object When an object's lead row names a page type whose template page holds no object of the kind, InsertDefaultPageIfNeeded warned that the page type "contains no data suitable for that page type", found no default page for the object, and then recursed with nothing left needed. The recursive call ran the same page-type branch again and repeated the warning for the same row. Now, when the object was all the row needed, it returns false at once and uses the page it has; ImportObjectAsync then reports and skips the row as before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The [details] column holds JSON that must come back byte for byte, and the
read path already skipped XML escaping for it. But it still ran the cell
through the decoding of Excel's _xNNNN_ character escapes, so text in the
JSON that merely looked like such an escape ("_x0041_" in a label, say)
came back as the character it named. Excel needs those escapes only for
characters XML cannot hold, and JSON never holds one raw, so the details
cell now gets neither treatment. A test writes such a cell to a real .xlsx
and reads it back unchanged.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
hatton
commented
Sep 4, 2026
hatton
commented
Sep 4, 2026
hatton
commented
Sep 4, 2026
hatton
commented
Sep 4, 2026
When a lead row's object cannot be found in the book, the importer used to skip the family without moving, so every row after it landed a page early and the page the family was meant for was thrown away as unused at the end of the import. Now the importer looks for the object the way it looks for any other block, before deciding to skip: a lead row that starts a page (it carries a page type, as export writes on the first row of every page) moves onto a new page whether or not an object is found there, so later rows still land on their own pages and no page is lost. A lead row in the middle of a page is still skipped in place, so that page keeps its layout. This removes the look-ahead that decided whether any object of the kind was still available; it is no longer needed. Tests cover an object-only page followed by a text page, an object-only spreadsheet, and the mid-page case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The previous commit moved the importer past a skipped object family only when its lead row carried a page type. A page with no label gets no page type on export, so an object-only page of that shape was still skipped in place and thrown away at the end of the import. Now the lead row always looks for its object the way a [page content] row looks for its group, before deciding to skip; the importer therefore moves the same way for every row. A lead row in the middle of a page whose book has no object now moves on too, so the rows after it land on a page of their own, as they would after any row the page had no room for. Also make SpreadsheetObjectKinds.Register reject a kind whose Kind name is already taken, as it already did for the lead row label, so that two kinds can never claim the same [details] kind. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
hatton
commented
Sep 4, 2026
hatton
commented
Sep 4, 2026
Member
Author
|
[Claude Fable 5.1 from Hatton's machine during preflight] Consulted Devin on 2026-09-04 up to commit 543e2ff. Across its three passes it raised four bugs and two investigate flags, all mirrored as review threads:
CI ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Some things on a page cannot survive a round trip through a Bloom spreadsheet. The spreadsheet has columns for text, images, video and widgets, and import puts each row back by position on the page. An object that is made of several of those pieces, or that needs state none of those columns holds, comes back wrong or not at all. Bloom has no place in the spreadsheet for that state, and no hook by which a feature could add one.
What the PR does
This is the generic mechanism only. It adds no feature that uses it.
[details]column. A row's cell in it holds a JSON object whosekindproperty says what the row is for. The column is excluded from WYSIWYG formatting, from XML escaping, and from the decoding of Excel's_xNNNN_character escapes, so the JSON round-trips byte for byte.ISpreadsheetObjectKind. A kind says which page elements it owns, writes its own rows for each object on export, and consumes that row family on import. Registering a kind is all a feature has to do.[page content]rows, and keeps the object's inner text, pictures and video out of the ordinary rows.[details]column, or the page has no place for the object, import reports the row number and skips the row family. A skipped family that starts a page still holds its place in the page order, so the rows after it land on their own pages and no page is dropped; past the end of the book this adds a copy of the last page, as any row does. A skipped family in the middle of a page leaves that page as it was. Import never invents an object, and it leaves an older spreadsheet's behavior unchanged.[details]cell, and the unchanged import of a spreadsheet with no[details]column.Devin review
This change is
Devin review