Skip to content

Commit b15eb5b

Browse files
authored
Merge branch 'main' into jk/feat/input-block-ux
2 parents f59aad4 + c1b5526 commit b15eb5b

File tree

10 files changed

+417
-53
lines changed

10 files changed

+417
-53
lines changed

package.json

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,12 @@
129129
"category": "Deepnote",
130130
"icon": "$(graph)"
131131
},
132+
{
133+
"command": "deepnote.addChartBlock",
134+
"title": "%deepnote.commands.addChartBlock.title%",
135+
"category": "Deepnote",
136+
"icon": "$(graph-line)"
137+
},
132138
{
133139
"command": "deepnote.addInputTextBlock",
134140
"title": "%deepnote.commands.addInputTextBlock.title%",
@@ -850,6 +856,51 @@
850856
"group": "navigation@1",
851857
"when": "notebookType == 'deepnote'"
852858
},
859+
{
860+
"command": "deepnote.addChartBlock",
861+
"group": "navigation@2",
862+
"when": "notebookType == 'deepnote'"
863+
},
864+
{
865+
"command": "deepnote.addBigNumberChartBlock",
866+
"group": "navigation@3",
867+
"when": "notebookType == 'deepnote'"
868+
},
869+
{
870+
"command": "deepnote.addInputTextBlock",
871+
"group": "navigation@4",
872+
"when": "notebookType == 'deepnote'"
873+
},
874+
{
875+
"command": "deepnote.addInputTextareaBlock",
876+
"group": "navigation@5",
877+
"when": "notebookType == 'deepnote'"
878+
},
879+
{
880+
"command": "deepnote.addInputSelectBlock",
881+
"group": "navigation@6",
882+
"when": "notebookType == 'deepnote'"
883+
},
884+
{
885+
"command": "deepnote.addInputSliderBlock",
886+
"group": "navigation@7",
887+
"when": "notebookType == 'deepnote'"
888+
},
889+
{
890+
"command": "deepnote.addInputCheckboxBlock",
891+
"group": "navigation@8",
892+
"when": "notebookType == 'deepnote'"
893+
},
894+
{
895+
"command": "deepnote.addInputDateBlock",
896+
"group": "navigation@9",
897+
"when": "notebookType == 'deepnote'"
898+
},
899+
{
900+
"command": "deepnote.addInputDateRangeBlock",
901+
"group": "navigation@10",
902+
"when": "notebookType == 'deepnote'"
903+
},
853904
{
854905
"command": "jupyter.restartkernel",
855906
"group": "navigation/execute@5",

package.nls.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,8 @@
255255
"deepnote.commands.importNotebook.title": "Import Notebook",
256256
"deepnote.commands.importJupyterNotebook.title": "Import Jupyter Notebook",
257257
"deepnote.commands.addSqlBlock.title": "Add SQL Block",
258-
"deepnote.commands.addBigNumberChartBlock.title": "Add Big Number Chart Block",
258+
"deepnote.commands.addBigNumberChartBlock.title": "Add Big Number Block",
259+
"deepnote.commands.addChartBlock.title": "Add Chart Block",
259260
"deepnote.commands.addInputTextBlock.title": "Add Input Text Block",
260261
"deepnote.commands.addInputTextareaBlock.title": "Add Input Textarea Block",
261262
"deepnote.commands.addInputSelectBlock.title": "Add Input Select Block",

src/commands.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ export interface ICommandNameArgumentTypeMapping {
188188
[DSCommands.ContinueEditSessionInCodespace]: [];
189189
[DSCommands.AddSqlBlock]: [];
190190
[DSCommands.AddBigNumberChartBlock]: [];
191+
[DSCommands.AddChartBlock]: [];
191192
[DSCommands.AddInputTextBlock]: [];
192193
[DSCommands.AddInputTextareaBlock]: [];
193194
[DSCommands.AddInputSelectBlock]: [];

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { IExtensionSyncActivationService } from '../../platform/activation/types
1717
import { IDisposableRegistry } from '../../platform/common/types';
1818
import { Commands } from '../../platform/common/constants';
1919
import { chainWithPendingUpdates } from '../../kernels/execution/notebookUpdater';
20+
import { WrappedError } from '../../platform/errors/types';
2021
import { formatInputBlockCellContent, getInputBlockLanguage } from './inputBlockContentFormatter';
2122
import {
2223
DeepnoteBigNumberMetadataSchema,
@@ -31,6 +32,7 @@ import {
3132
DeepnoteButtonMetadataSchema,
3233
DeepnoteSqlMetadata
3334
} from './deepnoteSchemas';
35+
import { DATAFRAME_SQL_INTEGRATION_ID } from '../../platform/notebooks/deepnote/integrationTypes';
3436

3537
export type InputBlockType =
3638
| 'input-text'
@@ -76,7 +78,10 @@ export function getInputBlockMetadata(blockType: InputBlockType, variableName: s
7678

7779
export function safeParseDeepnoteVariableNameFromContentJson(content: string): string | undefined {
7880
try {
79-
const variableNameResult = z.string().safeParse(JSON.parse(content)['deepnote_variable_name']);
81+
const parsed = JSON.parse(content);
82+
// Chart blocks use 'variable' key, other blocks use 'deepnote_variable_name'
83+
const variableName = parsed['variable'] ?? parsed['deepnote_variable_name'];
84+
const variableNameResult = z.string().safeParse(variableName);
8085
return variableNameResult.success ? variableNameResult.data : undefined;
8186
} catch (error) {
8287
logger.error('Error parsing deepnote variable name from content JSON', error);
@@ -148,6 +153,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation
148153
this.disposableRegistry.push(
149154
commands.registerCommand(Commands.AddBigNumberChartBlock, () => this.addBigNumberChartBlock())
150155
);
156+
this.disposableRegistry.push(commands.registerCommand(Commands.AddChartBlock, () => this.addChartBlock()));
151157
this.disposableRegistry.push(
152158
commands.registerCommand(Commands.AddInputTextBlock, () => this.addInputBlock('input-text'))
153159
);
@@ -190,7 +196,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation
190196
const defaultMetadata: DeepnoteSqlMetadata = {
191197
deepnote_variable_name: deepnoteVariableName,
192198
deepnote_return_variable_type: 'dataframe',
193-
sql_integration_id: 'deepnote-dataframe-sql'
199+
sql_integration_id: DATAFRAME_SQL_INTEGRATION_ID
194200
};
195201

196202
// Determine the index where to insert the new cell (below current selection or at the end)
@@ -262,6 +268,61 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation
262268
await commands.executeCommand('notebook.cell.edit');
263269
}
264270

271+
public async addChartBlock(): Promise<void> {
272+
const editor = window.activeNotebookEditor;
273+
274+
if (!editor) {
275+
throw new WrappedError(l10n.t('No active notebook editor found'));
276+
}
277+
278+
const document = editor.notebook;
279+
const selection = editor.selection;
280+
const insertIndex = selection ? selection.end : document.cellCount;
281+
282+
const defaultVisualizationSpec = {
283+
mark: 'line',
284+
$schema: 'https://vega.github.io/schema/vega-lite/v5.json',
285+
data: { values: [] },
286+
encoding: {
287+
x: { field: 'x', type: 'quantitative' },
288+
y: { field: 'y', type: 'quantitative' }
289+
}
290+
};
291+
292+
const cellContent = {
293+
variable: 'df_1',
294+
spec: defaultVisualizationSpec,
295+
filters: []
296+
};
297+
298+
const metadata = {
299+
__deepnotePocket: {
300+
type: 'visualization'
301+
}
302+
};
303+
304+
const result = await chainWithPendingUpdates(document, (edit) => {
305+
const newCell = new NotebookCellData(NotebookCellKind.Code, JSON.stringify(cellContent, null, 2), 'json');
306+
307+
newCell.metadata = metadata;
308+
309+
const nbEdit = NotebookEdit.insertCells(insertIndex, [newCell]);
310+
311+
edit.set(document.uri, [nbEdit]);
312+
});
313+
314+
if (result !== true) {
315+
throw new WrappedError(l10n.t('Failed to insert chart block'));
316+
}
317+
318+
const notebookRange = new NotebookRange(insertIndex, insertIndex + 1);
319+
320+
editor.revealRange(notebookRange, NotebookEditorRevealType.Default);
321+
editor.selection = notebookRange;
322+
323+
await commands.executeCommand('notebook.cell.edit');
324+
}
325+
265326
public async addInputBlock(blockType: InputBlockType): Promise<void> {
266327
const editor = window.activeNotebookEditor;
267328
if (!editor) {

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts

Lines changed: 189 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import { formatInputBlockCellContent, getInputBlockLanguage } from './inputBlock
2222
import { IDisposable } from '../../platform/common/types';
2323
import * as notebookUpdater from '../../kernels/execution/notebookUpdater';
2424
import { createMockedNotebookDocument } from '../../test/datascience/editor-integration/helpers';
25+
import { WrappedError } from '../../platform/errors/types';
26+
import { DATAFRAME_SQL_INTEGRATION_ID } from '../../platform/notebooks/deepnote/integrationTypes';
2527

2628
suite('DeepnoteNotebookCommandListener', () => {
2729
let commandListener: DeepnoteNotebookCommandListener;
@@ -739,7 +741,7 @@ suite('DeepnoteNotebookCommandListener', () => {
739741
);
740742
assert.equal(
741743
newCell.metadata.sql_integration_id,
742-
'deepnote-dataframe-sql',
744+
DATAFRAME_SQL_INTEGRATION_ID,
743745
'Should have correct sql integration id'
744746
);
745747

@@ -980,5 +982,191 @@ suite('DeepnoteNotebookCommandListener', () => {
980982
);
981983
});
982984
});
985+
986+
suite('addChartBlock', () => {
987+
test('should add chart block at the end when no selection exists', async () => {
988+
// Setup mocks
989+
const { editor, document } = createMockEditor([], undefined);
990+
const { chainStub, executeCommandStub, getCapturedNotebookEdits } =
991+
mockNotebookUpdateAndExecute(editor);
992+
993+
// Call the method
994+
await commandListener.addChartBlock();
995+
996+
const capturedNotebookEdits = getCapturedNotebookEdits();
997+
998+
// Verify chainWithPendingUpdates was called
999+
assert.isTrue(chainStub.calledOnce, 'chainWithPendingUpdates should be called once');
1000+
assert.equal(chainStub.firstCall.args[0], document, 'Should be called with correct document');
1001+
1002+
// Verify the edits were captured
1003+
assert.isNotNull(capturedNotebookEdits, 'Notebook edits should be captured');
1004+
assert.isDefined(capturedNotebookEdits, 'Notebook edits should be defined');
1005+
1006+
const editsArray = capturedNotebookEdits!;
1007+
assert.equal(editsArray.length, 1, 'Should have one notebook edit');
1008+
1009+
const notebookEdit = editsArray[0] as any;
1010+
assert.equal(notebookEdit.newCells.length, 1, 'Should insert one cell');
1011+
1012+
const newCell = notebookEdit.newCells[0];
1013+
assert.equal(newCell.kind, NotebookCellKind.Code, 'Should be a code cell');
1014+
assert.equal(newCell.languageId, 'json', 'Should have json language');
1015+
1016+
// Verify cell content is valid JSON with correct structure
1017+
const content = JSON.parse(newCell.value);
1018+
assert.equal(content.variable, 'df_1', 'Should have correct variable name');
1019+
assert.property(content, 'spec', 'Should have spec property');
1020+
assert.property(content, 'filters', 'Should have filters property');
1021+
1022+
// Verify the spec has the correct Vega-Lite structure
1023+
assert.equal(content.spec.mark, 'line', 'Should be a line chart');
1024+
assert.equal(
1025+
content.spec.$schema,
1026+
'https://vega.github.io/schema/vega-lite/v5.json',
1027+
'Should have Vega-Lite schema'
1028+
);
1029+
assert.deepStrictEqual(content.spec.data, { values: [] }, 'Should have empty data array');
1030+
assert.property(content.spec, 'encoding', 'Should have encoding property');
1031+
assert.property(content.spec.encoding, 'x', 'Should have x encoding');
1032+
assert.property(content.spec.encoding, 'y', 'Should have y encoding');
1033+
1034+
// Verify metadata structure
1035+
assert.property(newCell.metadata, '__deepnotePocket', 'Should have __deepnotePocket metadata');
1036+
assert.equal(newCell.metadata.__deepnotePocket.type, 'visualization', 'Should have visualization type');
1037+
1038+
// Verify reveal and selection were set
1039+
assert.isTrue((editor.revealRange as sinon.SinonStub).calledOnce, 'Should reveal the new cell range');
1040+
const revealCall = (editor.revealRange as sinon.SinonStub).firstCall;
1041+
assert.equal(revealCall.args[0].start, 0, 'Should reveal correct range start');
1042+
assert.equal(revealCall.args[0].end, 1, 'Should reveal correct range end');
1043+
assert.equal(revealCall.args[1], 0, 'Should use NotebookEditorRevealType.Default (value 0)');
1044+
1045+
// Verify notebook.cell.edit command was executed
1046+
assert.isTrue(
1047+
executeCommandStub.calledWith('notebook.cell.edit'),
1048+
'Should execute notebook.cell.edit command'
1049+
);
1050+
});
1051+
1052+
test('should add chart block after selection when selection exists', async () => {
1053+
// Setup mocks
1054+
const existingCells = [createMockCell('{}'), createMockCell('{}')];
1055+
const selection = new NotebookRange(0, 1);
1056+
const { editor } = createMockEditor(existingCells, selection);
1057+
const { chainStub, getCapturedNotebookEdits } = mockNotebookUpdateAndExecute(editor);
1058+
1059+
// Call the method
1060+
await commandListener.addChartBlock();
1061+
1062+
const capturedNotebookEdits = getCapturedNotebookEdits();
1063+
1064+
// Verify chainWithPendingUpdates was called
1065+
assert.isTrue(chainStub.calledOnce, 'chainWithPendingUpdates should be called once');
1066+
1067+
// Verify a cell was inserted
1068+
assert.isNotNull(capturedNotebookEdits, 'Notebook edits should be captured');
1069+
const notebookEdit = capturedNotebookEdits![0] as any;
1070+
assert.equal(notebookEdit.newCells.length, 1, 'Should insert one cell');
1071+
assert.equal(notebookEdit.newCells[0].languageId, 'json', 'Should be JSON cell');
1072+
});
1073+
1074+
test('should use hardcoded variable name df_1', async () => {
1075+
// Setup mocks with existing df variables
1076+
const existingCells = [
1077+
createMockCell('{ "deepnote_variable_name": "df_1" }'),
1078+
createMockCell('{ "variable": "df_2" }')
1079+
];
1080+
const { editor } = createMockEditor(existingCells, undefined);
1081+
const { getCapturedNotebookEdits } = mockNotebookUpdateAndExecute(editor);
1082+
1083+
// Call the method
1084+
await commandListener.addChartBlock();
1085+
1086+
const capturedNotebookEdits = getCapturedNotebookEdits();
1087+
const notebookEdit = capturedNotebookEdits![0] as any;
1088+
const newCell = notebookEdit.newCells[0];
1089+
1090+
// Verify variable name is always df_1
1091+
const content = JSON.parse(newCell.value);
1092+
assert.equal(content.variable, 'df_1', 'Should always use df_1');
1093+
});
1094+
1095+
test('should always use df_1 regardless of existing variables', async () => {
1096+
// Setup mocks with various existing variables
1097+
const existingCells = [
1098+
createMockCell('{ "deepnote_variable_name": "input_10" }'),
1099+
createMockCell('{ "deepnote_variable_name": "df_5" }'),
1100+
createMockCell('{ "variable": "df_2" }')
1101+
];
1102+
const { editor } = createMockEditor(existingCells, undefined);
1103+
const { getCapturedNotebookEdits } = mockNotebookUpdateAndExecute(editor);
1104+
1105+
// Call the method
1106+
await commandListener.addChartBlock();
1107+
1108+
const capturedNotebookEdits = getCapturedNotebookEdits();
1109+
const notebookEdit = capturedNotebookEdits![0] as any;
1110+
const newCell = notebookEdit.newCells[0];
1111+
1112+
// Verify variable name is always df_1
1113+
const content = JSON.parse(newCell.value);
1114+
assert.equal(content.variable, 'df_1', 'Should always use df_1');
1115+
});
1116+
1117+
test('should insert at correct position in the middle of notebook', async () => {
1118+
// Setup mocks
1119+
const existingCells = [createMockCell('{}'), createMockCell('{}'), createMockCell('{}')];
1120+
const selection = new NotebookRange(1, 2);
1121+
const { editor } = createMockEditor(existingCells, selection);
1122+
const { chainStub, getCapturedNotebookEdits } = mockNotebookUpdateAndExecute(editor);
1123+
1124+
// Call the method
1125+
await commandListener.addChartBlock();
1126+
1127+
const capturedNotebookEdits = getCapturedNotebookEdits();
1128+
1129+
// Verify chainWithPendingUpdates was called
1130+
assert.isTrue(chainStub.calledOnce, 'chainWithPendingUpdates should be called once');
1131+
1132+
// Verify a cell was inserted
1133+
assert.isNotNull(capturedNotebookEdits, 'Notebook edits should be captured');
1134+
const notebookEdit = capturedNotebookEdits![0] as any;
1135+
assert.equal(notebookEdit.newCells.length, 1, 'Should insert one cell');
1136+
assert.equal(notebookEdit.newCells[0].languageId, 'json', 'Should be JSON cell');
1137+
});
1138+
1139+
test('should throw error when no active editor exists', async () => {
1140+
// Setup: no active editor
1141+
Object.defineProperty(window, 'activeNotebookEditor', {
1142+
value: undefined,
1143+
configurable: true,
1144+
writable: true
1145+
});
1146+
1147+
// Call the method and expect rejection
1148+
await assert.isRejected(
1149+
commandListener.addChartBlock(),
1150+
WrappedError,
1151+
'No active notebook editor found'
1152+
);
1153+
});
1154+
1155+
test('should throw error when chainWithPendingUpdates fails', async () => {
1156+
// Setup mocks
1157+
const { editor } = createMockEditor([], undefined);
1158+
Object.defineProperty(window, 'activeNotebookEditor', {
1159+
value: editor,
1160+
configurable: true,
1161+
writable: true
1162+
});
1163+
1164+
// Mock chainWithPendingUpdates to return false
1165+
sandbox.stub(notebookUpdater, 'chainWithPendingUpdates').resolves(false);
1166+
1167+
// Call the method and expect rejection
1168+
await assert.isRejected(commandListener.addChartBlock(), WrappedError, 'Failed to insert chart block');
1169+
});
1170+
});
9831171
});
9841172
});

0 commit comments

Comments
 (0)