From 5fcb240e8d7c33326ac06da322e084e9cc3eff7c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 24 Mar 2025 13:36:12 +1100 Subject: [PATCH] Adjust cell indexes when inserting a new cell (#244402) * Adjust cell indexes when inserting a new cell * Updates * Fixes --- .../browser/chatEditing/notebook/helpers.ts | 29 ++- .../chatEditingModifiedNotebookEntry.test.ts | 208 +++++++++++++++++- 2 files changed, 226 insertions(+), 11 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatEditing/notebook/helpers.ts b/src/vs/workbench/contrib/chat/browser/chatEditing/notebook/helpers.ts index 23a647019cb9..e2b9aed29758 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditing/notebook/helpers.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditing/notebook/helpers.ts @@ -153,15 +153,28 @@ export function adjustCellDiffAndOriginalModelBasedOnCellAddDelete(change: Noteb internalMetadata: cell.internalMetadata } satisfies ICellDto2; }); - const wasInsertedAsFirstCell = change[0] === 0; - const wasInsertedAsLastCell = change[0] === modifiedModelCellCount - 1; - const diffEntryIndex = wasInsertedAsFirstCell ? 0 : (wasInsertedAsLastCell ? cellDiffInfo.length - 1 : (cellDiffInfo.findIndex(d => d.modifiedCellIndex === change[0]))); - const indexToInsertInOriginalModel = (wasInsertedAsFirstCell || diffEntryIndex === -1) ? 0 : (wasInsertedAsLastCell ? originalModelCellCount : (((cellDiffInfo.slice(0, diffEntryIndex).reverse().find(c => typeof c.originalCellIndex === 'number')?.originalCellIndex ?? -1) + 1))); + let diffEntryIndex = -1; + let indexToInsertInOriginalModel: number | undefined = undefined; if (cells.length) { + for (let i = 0; i < cellDiffInfo.length; i++) { + const diff = cellDiffInfo[i]; + if (typeof diff.modifiedCellIndex === 'number' && diff.modifiedCellIndex === change[0]) { + diffEntryIndex = i; + + if (typeof diff.originalCellIndex === 'number') { + indexToInsertInOriginalModel = diff.originalCellIndex; + } + break; + } + if (typeof diff.originalCellIndex === 'number') { + indexToInsertInOriginalModel = diff.originalCellIndex + 1; + } + } + const edit: ICellEditOperation = { editType: CellEditType.Replace, cells, - index: indexToInsertInOriginalModel, + index: indexToInsertInOriginalModel ?? 0, count: change[1] }; applyEdits([edit], true, undefined, () => undefined, undefined, true); @@ -220,7 +233,7 @@ export function adjustCellDiffAndOriginalModelBasedOnCellAddDelete(change: Noteb cellDiffInfo = cellDiffInfo.filter(d => !itemsToRemove.has(d)); } - if (numberOfCellsInserted) { + if (numberOfCellsInserted && diffEntryIndex >= 0) { for (let i = 0; i < cellDiffInfo.length; i++) { const diff = cellDiffInfo[i]; if (i < diffEntryIndex) { @@ -244,10 +257,10 @@ export function adjustCellDiffAndOriginalModelBasedOnCellAddDelete(change: Noteb // For inserted cells, we need to ensure that we create a corresponding CellEntry. // So that any edits to the inserted cell is handled and mirrored over to the corresponding cell in original model. cells.forEach((_, i) => { - const originalCellIndex = i + indexToInsertInOriginalModel; + const originalCellIndex = i + (indexToInsertInOriginalModel ?? 0); const modifiedCellIndex = change[0] + i; const unchangedCell = createModifiedCellDiffInfo(modifiedCellIndex, originalCellIndex); - cellDiffInfo.splice((diffEntryIndex === -1 ? 0 : diffEntryIndex) + i, 0, unchangedCell); + cellDiffInfo.splice((diffEntryIndex === -1 ? cellDiffInfo.length : diffEntryIndex) + i, 0, unchangedCell); }); return cellDiffInfo; } diff --git a/src/vs/workbench/contrib/chat/test/browser/chatEditingModifiedNotebookEntry.test.ts b/src/vs/workbench/contrib/chat/test/browser/chatEditingModifiedNotebookEntry.test.ts index 0ac36cbdb452..42270fb0be7f 100644 --- a/src/vs/workbench/contrib/chat/test/browser/chatEditingModifiedNotebookEntry.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/chatEditingModifiedNotebookEntry.test.ts @@ -807,7 +807,7 @@ suite('ChatEditingModifiedNotebookEntry', function () { const cell = createICell(CellKind.Code, 'print("Hello World")'); const result = adjustCellDiffAndOriginalModelBasedOnCellAddDelete([0, 0, [cell]], - cellsDiffInfo, 2, 2, applyEdits, createModifiedCellDiffInfo); + cellsDiffInfo, 3, 2, applyEdits, createModifiedCellDiffInfo); assert.deepStrictEqual(appliedEdits, [ { editType: CellEditType.Replace, @@ -838,7 +838,7 @@ suite('ChatEditingModifiedNotebookEntry', function () { }, ]); }); - test('Insert a new cell into an notebook with 3 cells deleted', async function () { + test('Insert a new cell into a notebook with 3 cells deleted', async function () { const cellsDiffInfo: ICellDiffInfo[] = [ { diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('0'), originalCellIndex: 0, @@ -875,7 +875,7 @@ suite('ChatEditingModifiedNotebookEntry', function () { ]; const cell = createICell(CellKind.Code, 'print("Hello World")'); const result = adjustCellDiffAndOriginalModelBasedOnCellAddDelete([2, 0, [cell]], - cellsDiffInfo, 5, 7, applyEdits, createModifiedCellDiffInfo); + cellsDiffInfo, 6, 7, applyEdits, createModifiedCellDiffInfo); assert.deepStrictEqual(appliedEdits, [ { @@ -1090,6 +1090,53 @@ suite('ChatEditingModifiedNotebookEntry', function () { }, ]); }); + test('Delete the first cell, then insert a new cell at the top', async function () { + const cellsDiffInfo: ICellDiffInfo[] = [ + { + diff, keep, undo, type: 'delete', originalModel: createOriginalModel('0'), originalCellIndex: 0, + modifiedCellIndex: undefined, modifiedModel: createModifiedModel('null'), + }, + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('1'), originalCellIndex: 1, + modifiedCellIndex: 0, modifiedModel: createModifiedModel('1'), + }, + ]; + + const cell1 = createICell(CellKind.Code, 'print("Hello World")'); + const result = adjustCellDiffAndOriginalModelBasedOnCellAddDelete([0, 0, [cell1]], + cellsDiffInfo, 2, 2, applyEdits, createModifiedCellDiffInfo); + + assert.deepStrictEqual(appliedEdits, [ + { + editType: CellEditType.Replace, + index: 1, + cells: [{ + cellKind: CellKind.Code, + language: 'python', + outputs: [], + mime: undefined, + metadata: {}, + internalMetadata: {}, + source: cell1.getValue(), + }], count: 0 + } + ]); + + assert.deepStrictEqual(result, [ + { + diff, keep, undo, type: 'delete', originalModel: createOriginalModel('0'), originalCellIndex: 0, + modifiedCellIndex: undefined, modifiedModel: createModifiedModel('null'), + }, + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('InsertedOriginal:1'), originalCellIndex: 1, + modifiedCellIndex: 0, modifiedModel: createModifiedModel('InsertedModified:0'), + }, + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('1'), originalCellIndex: 2, + modifiedCellIndex: 1, modifiedModel: createModifiedModel('1'), + }, + ]); + }); test('Delete a new cell from a notebook with 3 cells deleted', async function () { const cellsDiffInfo: ICellDiffInfo[] = [ { @@ -1300,6 +1347,161 @@ suite('ChatEditingModifiedNotebookEntry', function () { }, ]); }); + + test('Insert 1 cell at the bottom via chat, then user creats a new cell just below that', async function () { + const cellsDiffInfo: ICellDiffInfo[] = [ + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('0'), originalCellIndex: 0, + modifiedCellIndex: 0, modifiedModel: createModifiedModel('0'), + }, + { + diff, keep, undo, type: 'insert', originalModel: createOriginalModel('null'), originalCellIndex: undefined, + modifiedCellIndex: 1, modifiedModel: createModifiedModel('New1'), + }, + ]; + const cell1 = createICell(CellKind.Code, 'print("Hello World")'); + const result = adjustCellDiffAndOriginalModelBasedOnCellAddDelete([2, 0, [cell1]], + cellsDiffInfo, 3, 1, applyEdits, createModifiedCellDiffInfo); + + assert.deepStrictEqual(appliedEdits, [ + { + editType: CellEditType.Replace, + index: 1, + cells: [{ + cellKind: CellKind.Code, + language: 'python', + outputs: [], + mime: undefined, + metadata: {}, + internalMetadata: {}, + source: cell1.getValue(), + }], count: 0 + } + ]); + + assert.deepStrictEqual(result, [ + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('0'), originalCellIndex: 0, + modifiedCellIndex: 0, modifiedModel: createModifiedModel('0'), + }, + { + diff, keep, undo, type: 'insert', originalModel: createOriginalModel('null'), originalCellIndex: undefined, + modifiedCellIndex: 1, modifiedModel: createModifiedModel('New1'), + }, + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('InsertedOriginal:1'), originalCellIndex: 1, + modifiedCellIndex: 2, modifiedModel: createModifiedModel('InsertedModified:2'), + }, + ]); + }); + test('Insert 1 cell at the bottom via chat, then user creats anew cells above the previous new cell', async function () { + const cellsDiffInfo: ICellDiffInfo[] = [ + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('0'), originalCellIndex: 0, + modifiedCellIndex: 0, modifiedModel: createModifiedModel('0'), + }, + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('1'), originalCellIndex: 1, + modifiedCellIndex: 1, modifiedModel: createModifiedModel('1'), + }, + { + diff, keep, undo, type: 'insert', originalModel: createOriginalModel('null'), originalCellIndex: undefined, + modifiedCellIndex: 2, modifiedModel: createModifiedModel('New1'), + }, + ]; + const cell1 = createICell(CellKind.Code, 'print("Hello World")'); + const result = adjustCellDiffAndOriginalModelBasedOnCellAddDelete([2, 0, [cell1]], + cellsDiffInfo, 3, 2, applyEdits, createModifiedCellDiffInfo); + + assert.deepStrictEqual(appliedEdits, [ + { + editType: CellEditType.Replace, + index: 2, + cells: [{ + cellKind: CellKind.Code, + language: 'python', + outputs: [], + mime: undefined, + metadata: {}, + internalMetadata: {}, + source: cell1.getValue(), + }], count: 0 + } + ]); + + assert.deepStrictEqual(result, [ + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('0'), originalCellIndex: 0, + modifiedCellIndex: 0, modifiedModel: createModifiedModel('0'), + }, + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('1'), originalCellIndex: 1, + modifiedCellIndex: 1, modifiedModel: createModifiedModel('1'), + }, + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('InsertedOriginal:2'), originalCellIndex: 2, + modifiedCellIndex: 2, modifiedModel: createModifiedModel('InsertedModified:2'), + }, + { + diff, keep, undo, type: 'insert', originalModel: createOriginalModel('null'), originalCellIndex: undefined, + modifiedCellIndex: 3, modifiedModel: createModifiedModel('New1'), + }, + ]); + }); + test('Insert 1 cell at the bottom via chat, then user inserts a new cells below the previous new cell', async function () { + const cellsDiffInfo: ICellDiffInfo[] = [ + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('0'), originalCellIndex: 0, + modifiedCellIndex: 0, modifiedModel: createModifiedModel('0'), + }, + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('1'), originalCellIndex: 1, + modifiedCellIndex: 1, modifiedModel: createModifiedModel('1'), + }, + { + diff, keep, undo, type: 'insert', originalModel: createOriginalModel('null'), originalCellIndex: undefined, + modifiedCellIndex: 2, modifiedModel: createModifiedModel('New1'), + }, + ]; + const cell1 = createICell(CellKind.Code, 'print("Hello World")'); + const result = adjustCellDiffAndOriginalModelBasedOnCellAddDelete([3, 0, [cell1]], + cellsDiffInfo, 3, 2, applyEdits, createModifiedCellDiffInfo); + + assert.deepStrictEqual(appliedEdits, [ + { + editType: CellEditType.Replace, + index: 2, + cells: [{ + cellKind: CellKind.Code, + language: 'python', + outputs: [], + mime: undefined, + metadata: {}, + internalMetadata: {}, + source: cell1.getValue(), + }], count: 0 + } + ]); + + assert.deepStrictEqual(result, [ + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('0'), originalCellIndex: 0, + modifiedCellIndex: 0, modifiedModel: createModifiedModel('0'), + }, + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('1'), originalCellIndex: 1, + modifiedCellIndex: 1, modifiedModel: createModifiedModel('1'), + }, + { + diff, keep, undo, type: 'insert', originalModel: createOriginalModel('null'), originalCellIndex: undefined, + modifiedCellIndex: 2, modifiedModel: createModifiedModel('New1'), + }, + { + diff, keep, undo, type: 'unchanged', originalModel: createOriginalModel('InsertedOriginal:2'), originalCellIndex: 2, + modifiedCellIndex: 3, modifiedModel: createModifiedModel('InsertedModified:3'), + }, + ]); + }); }); suite('Cell Movements', function () {