From 46d8405918f6797bf1dce3c80004bf838f999a88 Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Mon, 17 Aug 2026 14:33:52 +0100 Subject: [PATCH] feat: add optional comment field to ContentSheetCell One structured shape covers both comment mechanisms a real spreadsheet uses: xlsx's legacy single notes (text/author/createdAt, replies absent) and its newer threaded comments (root text plus a flat replies array -- a reply never carries replies of its own). A legacy note is simply a comment whose replies stay absent, so no union or discriminant is needed. The field is optional on ContentSheetCellSchema, existing documents validate unchanged, and CONTENT_FORMAT_VERSION stays 3. --- src/content.test.ts | 76 +++++++++++++++++++++++++++++++++++++++++++++ src/content.ts | 12 ++++++- 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/content.test.ts b/src/content.test.ts index 038ecfa..b29e46a 100644 --- a/src/content.test.ts +++ b/src/content.test.ts @@ -205,6 +205,12 @@ function spreadsheetDocument(): ContentDocument { value: { kind: 'currency', value: 125000, currency: 'USD' }, formula: '=SUM(B2:B10)', displayText: '$125,000.00', + comment: { + text: 'Excludes the late Q4 bookings.', + author: 'Joseph Mearman', + createdAt: '2026-08-17T09:30:00Z', + replies: [{ text: 'Confirmed against the ledger.', author: 'Robin Achebe' }], + }, }, { row: 1, column: 1, value: { kind: 'percentage', value: 0.235 }, displayText: '23.5%' }, { row: 2, column: 1, value: { kind: 'boolean', value: true }, displayText: 'TRUE' }, @@ -630,6 +636,76 @@ describe('frames (the FusedNode pattern)', () => { }); }); +describe('ContentSheetCell comment', () => { + it('accepts a legacy-style note -- text alone, or with author and createdAt, no replies', () => { + const bare = ContentSheetCellSchema.parse({ + row: 0, + column: 0, + value: { kind: 'string', value: 'x' }, + displayText: 'x', + comment: { text: 'Check this figure.' }, + }); + expect(bare.comment).toEqual({ text: 'Check this figure.' }); + + const attributed = ContentSheetCellSchema.parse({ + row: 0, + column: 0, + value: { kind: 'string', value: 'x' }, + displayText: 'x', + comment: { text: 'Check this figure.', author: 'Robin Achebe', createdAt: '2026-08-17T09:30:00Z' }, + }); + expect(attributed.comment?.replies).toBeUndefined(); + }); + + it('accepts a threaded comment and preserves reply order', () => { + const parsed = ContentSheetCellSchema.parse({ + row: 3, + column: 7, + value: { kind: 'number', value: 42 }, + displayText: '42', + comment: { + text: 'Excludes the late Q4 bookings.', + author: 'Joseph Mearman', + replies: [ + { text: 'Confirmed against the ledger.', author: 'Robin Achebe' }, + { text: 'Noted.', author: 'Joseph Mearman' }, + ], + }, + }); + expect(parsed.comment?.replies?.map((reply) => reply.text)).toEqual([ + 'Confirmed against the ledger.', + 'Noted.', + ]); + }); + + it('parses correctly when comment is omitted, matching every other optional field', () => { + expect( + ContentSheetCellSchema.parse({ row: 0, column: 0, value: { kind: 'empty' }, displayText: '' }).comment, + ).toBeUndefined(); + }); + + it('rejects a comment with no text, and a reply with no text', () => { + expect( + ContentSheetCellSchema.safeParse({ + row: 0, + column: 0, + value: { kind: 'empty' }, + displayText: '', + comment: { author: 'Robin Achebe' }, + }).success, + ).toBe(false); + expect( + ContentSheetCellSchema.safeParse({ + row: 0, + column: 0, + value: { kind: 'empty' }, + displayText: '', + comment: { text: 'Root', replies: [{ author: 'Robin Achebe' }] }, + }).success, + ).toBe(false); + }); +}); + describe('ContentDocumentSchema round trips', () => { it('deep-equals the original wordprocessing document after a JSON round trip', () => { const original = wordprocessingDocument(); diff --git a/src/content.ts b/src/content.ts index e349a61..d4c85ec 100644 --- a/src/content.ts +++ b/src/content.ts @@ -317,7 +317,16 @@ export const ContentCellValueSchema = z.discriminatedUnion('kind', [ ]); export type ContentCellValue = z.infer; -// row/column are the cell's own position, not implied by array index -- ContentSheetSchema.cells is sparse, since real sheets are sparse. displayText is the producer's own rendered string for `value` (its number-format/locale/currency-symbol applied already) and is required on every cell that exists in this array, since a cell with nothing to display simply isn't included; it is what makes spreadsheet-to-PDF rendering tractable without this package reimplementing a number-format/locale engine. formula, if present, is carried verbatim in whatever syntax the source format used. colSpan/rowSpan are set on the anchor cell only, matching how ContentTableCell already handles merged cells. alignment is an override of the existing value-kind default (numeric right, boolean/error centre, string left); absent means that default still applies. verticalAlignment has no value-kind default to fall back to, so its own absence means 'bottom' outright, matching a real spreadsheet's own typical default. +// A cell-anchored annotation, one shape deliberately covering both mechanisms a real spreadsheet uses for comments -- xlsx's legacy single notes and its newer threaded comments alike: a legacy note is a comment whose replies stay absent, a threaded comment is one whose replies array is populated, so no separate union or kind discriminant is needed. Replies are flat -- a reply never carries replies of its own -- matching how threaded comments nest in the source formats. createdAt is an ISO 8601 date-time in the source format's own spelling and precision (e.g. '2026-08-17T09:30:00Z'), present only when the source recorded one; a wire contract stated here rather than a validated one, for the same reason as ContentCellValueSchema's own temporal strings above -- a regex would turn a producer not yet normalised to ISO 8601 into a hard parse failure. +export const ContentSheetCellCommentSchema = z.object({ + text: z.string(), + author: z.string().optional(), + createdAt: z.string().optional(), + replies: z.array(z.object({ text: z.string(), author: z.string().optional() })).optional(), +}); +export type ContentSheetCellComment = z.infer; + +// row/column are the cell's own position, not implied by array index -- ContentSheetSchema.cells is sparse, since real sheets are sparse. displayText is the producer's own rendered string for `value` (its number-format/locale/currency-symbol applied already) and is required on every cell that exists in this array, since a cell with nothing to display simply isn't included; it is what makes spreadsheet-to-PDF rendering tractable without this package reimplementing a number-format/locale engine. formula, if present, is carried verbatim in whatever syntax the source format used. colSpan/rowSpan are set on the anchor cell only, matching how ContentTableCell already handles merged cells. alignment is an override of the existing value-kind default (numeric right, boolean/error centre, string left); absent means that default still applies. verticalAlignment has no value-kind default to fall back to, so its own absence means 'bottom' outright, matching a real spreadsheet's own typical default. comment, when present, is the cell's annotation (ContentSheetCellCommentSchema above) and never affects rendering -- it is carried for fidelity, so inspecting or round-tripping a document does not silently drop what the author pinned to that cell. export const ContentSheetCellSchema = z.object({ row: z.number().int().nonnegative(), column: z.number().int().nonnegative(), @@ -331,6 +340,7 @@ export const ContentSheetCellSchema = z.object({ borders: ContentCellBordersSchema.optional(), alignment: AlignmentSchema.optional(), // override; absent means the existing value-kind default verticalAlignment: z.enum(['top', 'middle', 'bottom']).optional(), // absent means 'bottom' + comment: ContentSheetCellCommentSchema.optional(), // a cell-anchored annotation -- a legacy note or a threaded comment; see ContentSheetCellCommentSchema above sourcePath: z.string().optional(), // deterministic, document-order-derived path assigned by the format reader frames: z.array(LayoutFrameSchema).optional(), // this cell's own rendered position(s), once a layout pass has fused one in -- see FusedNode above });