fix(xlsx): keep table filters on their source columns

src/core/xlsx/edit/table-edits.ts

@@ -214,7 +214,13 @@ export function updateTable(ctx: EditContext, s: number, ref: TableRef, patch: T
 			if (patch.columns) {
 				const width = table.range.end.col - table.range.start.col + 1;
 				if (patch.columns.length !== width) throw new Error('Give one column per table column.');
+				const before = table.columns;
 				table.columns = structuredClone(patch.columns);
+				// Replacing properties updates the existing columns, not their source identity.
+				for (const [index, column] of table.columns.entries()) {
+					const sourceId = before[index]?.sourceId;
+					if (column.sourceId === undefined && sourceId !== undefined) column.sourceId = sourceId;
+				}
 			}
 			if (patch.headerRow !== undefined) setHeaderRow(workbook, sheet, id, patch.headerRow);
 			if (patch.totalsRow !== undefined) setTotalsRow(workbook, sheet, id, patch.totalsRow);

src/core/xlsx/model.ts

@@ -480,6 +480,8 @@ export interface AutoFilterColumn {
 
 export interface TableColumn {
 	name: string;
+	/** Original table-column id, keeping source metadata attached through renames and shifts. */
+	sourceId?: number;
 	totalsRowFunction?: string;
 	totalsRowLabel?: string;
 	calculatedColumnFormula?: string;

src/core/xlsx/read/tables.ts

@@ -13,6 +13,8 @@ export function parseTable(xml: string, partName: string): Table | undefined {
 	const columns: TableColumn[] = xChildren(xFirst(root, 'tableColumns') ?? root, 'tableColumn').map(
 		(node) => {
 			const column: TableColumn = { name: att(node, 'name') ?? '' };
+			const id = numAttr(node, 'id');
+			if (id !== undefined && Number.isInteger(id) && id >= 0) column.sourceId = id;
 			const fn = att(node, 'totalsRowFunction');
 			if (fn && fn !== 'none') column.totalsRowFunction = fn;
 			const label = att(node, 'totalsRowLabel');

src/core/xlsx/write/save.test.ts

@@ -38,6 +38,8 @@ export function comparable(value: unknown): unknown {
 			(key, value) => {
 				// Sheet titles in app.xml are derived from the sheets on save (openpyxl omits them).
 				if (key === 'headingPairs' || key === 'titlesOfParts') return undefined;
+				// Table-column ids record read provenance; newly created columns have none.
+				if (key === 'sourceId') return undefined;
 				if (key === 'preserved' && value instanceof Map)
 					return [...value].filter(([name]) => !String(name).startsWith('source:'));
 				if (value instanceof Map) return [...value].sort((a, b) => Number(a[0]) - Number(b[0]));

src/core/xlsx/write/table-filter-shift.test.ts

@@ -0,0 +1,132 @@
+import { describe, expect, it } from 'vitest';
+import JSZip from 'jszip';
+import { parseRange } from '../address';
+import { loadXlsx } from '../read/load';
+import { saveXlsx } from './save';
+import { HEAD, R, X, miniPackage, ws } from './mini-package-fixtures';
+import { createEditSession } from '../edit/session';
+import type { Workbook } from '../model';
+
+/** Loads a complete table package with a criterion on one original column. */
+async function filteredTable(filterCol = 2) {
+	const headers = ['Item', 'Qty', 'Amt'];
+	const rows =
+		'<row r="1">' +
+		headers
+			.map(
+				(text, c) =>
+					`<c r="${String.fromCharCode(65 + c)}1" t="inlineStr"><is><t>${text}</t></is></c>`,
+			)
+			.join('') +
+		'</row>' +
+		'<row r="2"><c r="A2" t="inlineStr"><is><t>a</t></is></c><c r="B2"><v>2</v></c><c r="C2"><v>20</v></c></row>' +
+		'<row r="3"><c r="A3" t="inlineStr"><is><t>b</t></is></c><c r="B3"><v>3</v></c><c r="C3"><v>30</v></c></row>';
+	const table = `${HEAD}<table xmlns="${X}" id="1" name="Sales" displayName="Sales" ref="A1:C3"><autoFilter ref="A1:C3"><filterColumn colId="${filterCol}"><filters><filter val="20"/></filters></filterColumn></autoFilter><tableColumns count="3">${headers.map((name, i) => `<tableColumn id="${i + 1}" name="${name}"/>`).join('')}</tableColumns></table>`;
+	return loadXlsx(
+		await miniPackage({
+			sheets: [
+				{
+					name: 'Data',
+					xml: ws(rows, '', '<tableParts count="1"><tablePart r:id="rId1"/></tableParts>'),
+					rels: `${HEAD}<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="${R}/table" Target="../tables/table1.xml"/></Relationships>`,
+				},
+			],
+			parts: { 'xl/tables/table1.xml': table },
+			contentTypes:
+				'<Override PartName="/xl/tables/table1.xml" ContentType="application/vnd.openxmlformats-officedocument.spreadsheetml.table+xml"/>',
+		}),
+	);
+}
+/** Reads the actual saved table part so criteria are checked after serialization. */
+async function tableXml(wb: Workbook) {
+	const zip = await JSZip.loadAsync(await saveXlsx(wb));
+	return (await zip.file('xl/tables/table1.xml')?.async('string')) ?? '';
+}
+/** Extracts the saved zero-based filter-column indexes. */
+const filterIds = (xml: string) =>
+	[...xml.matchAll(/<filterColumn[^>]*colId="(\d+)"/g)].map((m) => Number(m[1]));
+
+describe('table filter identity after structural edits', () => {
+	it('keeps the original filter on an unedited save', async () => {
+		expect(filterIds(await tableXml(await filteredTable()))).toEqual([2]);
+	});
+	it('keeps the Amt filter on Amt when a middle column is inserted', async () => {
+		const wb = await filteredTable();
+		createEditSession(wb).insertColumns(0, 1, 1);
+		expect(wb.sheets[0]?.tables[0]?.columns.map((c) => c.name)).toEqual([
+			'Item',
+			'Column1',
+			'Qty',
+			'Amt',
+		]);
+		expect(filterIds(await tableXml(wb))).toEqual([3]);
+	});
+	it('keeps the Amt filter when an earlier column is deleted', async () => {
+		const wb = await filteredTable();
+		createEditSession(wb).deleteColumns(0, 0, 1);
+		expect(wb.sheets[0]?.tables[0]?.columns.map((c) => c.name)).toEqual(['Qty', 'Amt']);
+		expect(filterIds(await tableXml(wb))).toEqual([1]);
+	});
+	it('removes the Qty filter when Qty itself is deleted', async () => {
+		const wb = await filteredTable(1);
+		createEditSession(wb).deleteColumns(0, 1, 1);
+		expect(wb.sheets[0]?.tables[0]?.columns.map((c) => c.name)).toEqual(['Item', 'Amt']);
+		expect(filterIds(await tableXml(wb))).toEqual([]);
+	});
+});
+
+describe('source column identity', () => {
+	it('keeps filters and column metadata through a header rename and insertion', async () => {
+		const wb = await filteredTable();
+		const s = createEditSession(wb);
+		s.setCellValue(0, 0, 2, 'Revenue');
+		s.insertColumns(0, 1, 1);
+		const xml = await tableXml(wb);
+		expect(filterIds(xml)).toEqual([3]);
+		expect(xml).toContain('id="3" name="Revenue"');
+		s.undo();
+		expect(filterIds(await tableXml(wb))).toEqual([2]);
+		s.redo();
+		expect(filterIds(await tableXml(wb))).toEqual([3]);
+	});
+	it('does not transfer a removed filter to a replacement column of the same width', async () => {
+		const wb = await filteredTable(1);
+		const s = createEditSession(wb);
+		s.deleteColumns(0, 1, 1);
+		s.insertColumns(0, 1, 1);
+		expect(filterIds(await tableXml(wb))).toEqual([]);
+	});
+	it('remaps a filter when resize removes the first column', async () => {
+		const wb = await filteredTable();
+		createEditSession(wb).resizeTable(0, 'Sales', parseRange('B1:C3')!);
+		expect(filterIds(await tableXml(wb))).toEqual([1]);
+	});
+	it('keeps source identity when updateTable replaces column property objects', async () => {
+		const wb = await filteredTable();
+		const s = createEditSession(wb);
+		s.updateTable(0, 'Sales', {
+			columns: wb.sheets[0]!.tables[0]!.columns.map((column) => ({
+				name: column.name === 'Amt' ? 'Revenue' : column.name,
+			})),
+		});
+		expect(filterIds(await tableXml(wb))).toEqual([2]);
+		s.insertColumns(0, 1, 1);
+		expect(filterIds(await tableXml(wb))).toEqual([3]);
+	});
+	it('does not revive filters after all original columns have been replaced', async () => {
+		const wb = await filteredTable();
+		const s = createEditSession(wb);
+		s.insertColumns(0, 1, 3);
+		s.deleteColumns(0, 0, 1);
+		s.deleteColumns(0, 3, 2);
+		expect(wb.sheets[0]?.tables[0]?.columns).toHaveLength(3);
+		expect(filterIds(await tableXml(wb))).toEqual([]);
+	});
+	it('retains the remapped filter after save, reload and a second insertion', async () => {
+		const wb = await filteredTable();
+		createEditSession(wb).insertColumns(0, 1, 1);
+		const again = await loadXlsx(await saveXlsx(wb));
+		createEditSession(again).insertColumns(0, 1, 1);
+		expect(filterIds(await tableXml(again))).toEqual([4]);
+	});
+});

src/core/xlsx/write/table-patch.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, it } from 'vitest';
+import { createEditSession } from '../edit/session';
 import { HEAD, R, X, miniPackage, roundTrip, ws } from './mini-package-fixtures';
 
 const TABLE_SHEET = ws(
@@ -12,7 +13,8 @@ const TABLE_SHEET = ws(
 // What Excel writes for a table named "T1" (which reads as a cell reference).
 const TABLE_XML = `${HEAD}<table xmlns="${X}" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" mc:Ignorable="xr xr3" xmlns:xr="http://schemas.microsoft.com/office/spreadsheetml/2014/revision" xmlns:xr3="http://schemas.microsoft.com/office/spreadsheetml/2016/revision3" id="1" xr:uid="{9AC660E2-940A-42C8-9458-CF5E3EE4F94D}" name="T1" displayName="T1_" ref="A1:B4" totalsRowCount="1"><autoFilter ref="A1:B3" xr:uid="{9AC660E2-940A-42C8-9458-CF5E3EE4F94D}"><filterColumn colId="1"><customFilters><customFilter operator="greaterThan" val="8"/></customFilters></filterColumn><sortState ref="A2:B3"><sortCondition descending="1" ref="B2:B3"/></sortState></autoFilter><tableColumns count="2"><tableColumn id="1" xr3:uid="{281A89E3-63C6-46BA-B9A9-525F4BDCF0B3}" name="Item" totalsRowLabel="Total"/><tableColumn id="2" xr3:uid="{3C30B12E-A50A-422B-A962-AB4E95350E84}" name="Amt" totalsRowFunction="custom" dataDxfId="0"><totalsRowFormula>SUBTOTAL(109,T1_[Amt])*2</totalsRowFormula></tableColumn></tableColumns><tableStyleInfo name="TableStyleMedium2" showFirstColumn="0" showLastColumn="0" showRowStripes="1" showColumnStripes="0"/></table>`;
 
-const tablePackage = () =>
+/** Builds a workbook around source table XML to exercise preservation through saving. */
+const tablePackage = (xml = TABLE_XML) =>
 	miniPackage({
 		sheets: [
 			{
@@ -21,7 +23,7 @@ const tablePackage = () =>
 				rels: `${HEAD}<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="${R}/table" Target="../tables/table1.xml"/></Relationships>`,
 			},
 		],
-		parts: { 'xl/tables/table1.xml': TABLE_XML },
+		parts: { 'xl/tables/table1.xml': xml },
 		contentTypes:
 			'<Override PartName="/xl/tables/table1.xml" ContentType="application/vnd.openxmlformats-officedocument.spreadsheetml.table+xml"/>',
 	});
@@ -52,6 +54,60 @@ describe('table round trip', () => {
 		expect(xml.startsWith('<?xml')).toBe(true);
 	});
 
+	it('keeps custom filter criteria and column metadata after a rename and insertion', async () => {
+		const { part } = await roundTrip(await tablePackage(), (wb) => {
+			const session = createEditSession(wb);
+			session.setCellValue(0, 0, 1, 'Revenue');
+			session.insertColumns(0, 1, 1);
+		});
+		const xml = (await part('xl/tables/table1.xml')) ?? '';
+		expect(xml).toContain('<filterColumn colId="2"><customFilters>');
+		expect(xml).toContain('<customFilter operator="greaterThan" val="8"/>');
+		expect(xml).toMatch(/<tableColumn[^>]*id="2"[^>]*name="Revenue"[^>]*dataDxfId="0"/);
+		expect(xml).toContain('xr3:uid="{3C30B12E-A50A-422B-A962-AB4E95350E84}"');
+	});
+
+	it('keeps a zero source column id, filter and metadata after a style change', async () => {
+		const source = TABLE_XML.replace('<tableColumn id="2"', '<tableColumn id="0"');
+		const { wb, again, part } = await roundTrip(await tablePackage(source), (wb) => {
+			createEditSession(wb).updateTable(0, 'T1', { styleName: 'TableStyleLight1' });
+		});
+		expect(wb.sheets[0]?.tables[0]?.columns[1]?.sourceId).toBe(0);
+		expect(again.sheets[0]?.tables[0]?.columns[1]?.sourceId).toBe(0);
+		const xml = (await part('xl/tables/table1.xml')) ?? '';
+		expect(xml).toContain('<filterColumn colId="1"><customFilters>');
+		expect(xml).toContain('<customFilter operator="greaterThan" val="8"/>');
+		expect(xml).toMatch(/<tableColumn[^>]*id="0"[^>]*name="Amt"[^>]*dataDxfId="0"/);
+		expect(xml).toContain('xr3:uid="{3C30B12E-A50A-422B-A962-AB4E95350E84}"');
+		expect(xml).toContain('<totalsRowFormula>SUBTOTAL(109,T1_[Amt])*2</totalsRowFormula>');
+	});
+
+	it('moves a zero-id column filter with its source metadata after rename and insertion', async () => {
+		const source = TABLE_XML.replace('<tableColumn id="2"', '<tableColumn id="0"');
+		const { again, part } = await roundTrip(await tablePackage(source), (wb) => {
+			const session = createEditSession(wb);
+			session.setCellValue(0, 0, 1, 'Revenue');
+			session.insertColumns(0, 1, 1);
+		});
+		expect(again.sheets[0]?.tables[0]?.columns.map((column) => column.sourceId)).toEqual([1, 2, 0]);
+		const xml = (await part('xl/tables/table1.xml')) ?? '';
+		expect(xml).toContain('<filterColumn colId="2"><customFilters>');
+		expect(xml).toContain('<customFilter operator="greaterThan" val="8"/>');
+		expect(xml).toMatch(/<tableColumn[^>]*id="0"[^>]*name="Revenue"[^>]*dataDxfId="0"/);
+		expect(xml).toContain('xr3:uid="{3C30B12E-A50A-422B-A962-AB4E95350E84}"');
+	});
+
+	it('removes the filter and metadata when the zero-id source column is deleted', async () => {
+		const source = TABLE_XML.replace('<tableColumn id="2"', '<tableColumn id="0"');
+		const { again, part } = await roundTrip(await tablePackage(source), (wb) => {
+			createEditSession(wb).deleteColumns(0, 1, 1);
+		});
+		expect(again.sheets[0]?.tables[0]?.columns.map((column) => column.sourceId)).toEqual([1]);
+		const xml = (await part('xl/tables/table1.xml')) ?? '';
+		expect(xml).not.toContain('<filterColumn');
+		expect(xml).not.toContain('xr3:uid="{3C30B12E-A50A-422B-A962-AB4E95350E84}"');
+	});
+
 	it('stretches the sort state when rows are added and drops filters past the width', async () => {
 		const { part } = await roundTrip(await tablePackage(), (wb) => {
 			const table = wb.sheets[0]!.tables[0]!;

src/core/xlsx/write/table-patch.ts

@@ -47,7 +47,43 @@ export function tableFilterRange(table: Table): CellRange {
 	};
 }
 
-function patchAutoFilter(doc: Document, root: XmlElement, table: Table, before: CellRange): void {
+/** Matches current columns to their original XML nodes without relying on mutable names. */
+function sourceColumnsForPatch(
+	root: XmlElement,
+	table: Table,
+	names: readonly string[],
+): (XmlElement | undefined)[] {
+	const list = xKid(root, 'tableColumns');
+	const source = list ? xKids(list, 'tableColumn') : [];
+	const byId = new Map(source.map((c) => [Number(c.getAttribute('id')), c]));
+	const byName = new Map(source.map((c) => [(c.getAttribute('name') ?? '').toLowerCase(), c]));
+	const tracked =
+		table.partName !== undefined || table.columns.some((c) => c.sourceId !== undefined);
+	const used = new Set<XmlElement>();
+	return names.map((name, index) => {
+		const id = table.columns[index]?.sourceId;
+		const match =
+			id !== undefined
+				? byId.get(id)
+				: tracked
+					? undefined
+					: source.length === names.length
+						? source[index]
+						: byName.get(name.toLowerCase());
+		if (!match || used.has(match)) return undefined;
+		used.add(match);
+		return match;
+	});
+}
+
+/** Moves criteria with their source columns while preserving unmodelled filter markup. */
+function patchAutoFilter(
+	doc: Document,
+	root: XmlElement,
+	table: Table,
+	before: CellRange,
+	names: readonly string[],
+): void {
 	const existing = xKid(root, 'autoFilter');
 	if (!table.headerRow) {
 		if (existing) root.removeChild(existing);
@@ -56,10 +92,14 @@ function patchAutoFilter(doc: Document, root: XmlElement, table: Table, before:
 	const filter = existing ?? doc.createElementNS(NS.x, 'autoFilter');
 	const range = tableFilterRange(table);
 	filter.setAttribute('ref', formatRange(range));
-	const width = range.end.col - range.start.col + 1;
+	const list = xKid(root, 'tableColumns');
+	const original = list ? xKids(list, 'tableColumn') : [];
+	const current = new Map(sourceColumnsForPatch(root, table, names).map((c, index) => [c, index]));
 	for (const column of xKids(filter, 'filterColumn')) {
-		const colId = Number(column.getAttribute('colId') ?? '0');
-		if (!(colId < width)) filter.removeChild(column);
+		const old = original[Number(column.getAttribute('colId') ?? '0')];
+		const index = old ? current.get(old) : undefined;
+		if (index === undefined) filter.removeChild(column);
+		else column.setAttribute('colId', num(index));
 	}
 	if (!existing) insertBefore(root, filter, ['sortState', 'tableColumns']);
 	const sort = xKid(filter, 'sortState') ?? xKid(root, 'sortState');
@@ -92,6 +132,7 @@ function patchAutoFilter(doc: Document, root: XmlElement, table: Table, before:
 
 const COLUMN_CHILD_ORDER = ['calculatedColumnFormula', 'totalsRowFormula', 'xmlColumnPr', 'extLst'];
 
+/** Reuses surviving source column nodes to retain metadata through renames and axis edits. */
 function patchColumns(
 	doc: Document,
 	root: XmlElement,
@@ -105,18 +146,11 @@ function patchColumns(
 		insertBefore(root, list, ['tableStyleInfo', 'extLst']);
 	}
 	const sourceColumns = xKids(list, 'tableColumn');
-	const byName = new Map(
-		sourceColumns.map((c) => [(c.getAttribute('name') ?? '').toLowerCase(), c]),
-	);
-	const sameCount = sourceColumns.length === names.length;
-	const used = new Set<XmlElement>();
 	const ids = new Set<number>();
-	const picked = names.map((name, index) => {
-		const match = sameCount ? sourceColumns[index] : byName.get(name.toLowerCase());
-		if (!match || used.has(match)) return undefined;
-		used.add(match);
+	const picked = sourceColumnsForPatch(root, table, names).map((match) => {
+		if (!match) return undefined;
 		const id = Number(match.getAttribute('id'));
-		if (Number.isInteger(id) && id > 0 && !ids.has(id)) ids.add(id);
+		if (Number.isInteger(id) && id >= 0 && !ids.has(id)) ids.add(id);
 		else match.removeAttribute('id');
 		return match;
 	});
@@ -203,7 +237,7 @@ export function patchTableXml(
 		root.removeAttribute('totalsRowCount');
 		if (!root.hasAttribute('totalsRowShown')) root.setAttribute('totalsRowShown', '0');
 	}
-	patchAutoFilter(doc, root, table, before);
+	patchAutoFilter(doc, root, table, before, names);
 	patchColumns(doc, root, sheet, table, names);
 	patchStyleInfo(doc, root, table);
 	return buildXml(doc);