Before merging cells, we explicitly set the active cell and cells being deleted toidle. This prevents the split cells (created on undo) from incorrectly showing a stale execution indicator.

If the kernel is actively running, the cells will still appear asidle. Since we do not stream outputs to merged cell, or to cells restored by undo, this behavior I think is acceptable.

set-idle-status-before-merging

74bdc2a

Copy link

jupyterlab-probotbot commentedDec 5, 2025

Thanks for making a pull request to jupyterlab!
To try out this branch onbinder, follow this link:

github-actionsbot added the pkg:notebook label

Dec 5, 2025

github-actionsbot assignedDarshan808

Dec 5, 2025

Darshan808 added the bug label

Dec 5, 2025

fix-lint

ef0fe64

Darshan808 requested a review fromkrassowski

December 5, 2025 09:59

add-undo-test

c93addc

Copy link

Member

krassowski commentedDec 10, 2025

If the kernel is actively running, the cells will still appear as idle. Since we do not stream outputs to merged cell, or to cells restored by undo, this behavior I think is acceptable.

I wonder if long term we should stream outputs to merged cells though. If we wanted to get a correct state after undo, we would need to find out the kernel future object that was associated with execution request. There is a delicate balance between fixing that and avoiding memory leaks. I do not know yet what we should do here...

But in any case, a pair of unit test cases for when we expect the undo to result in[ ] vs in[*] here would be nice (even if we mark the second one as xfail for now):

jupyterlab/packages/notebook/test/actions.spec.ts

Lines 358 to 518 inf74534e

	describe('#mergeCells',()=>{
	it('should merge the selected cells',()=>{
	letsource=widget.activeCell!.model.sharedModel.getSource()+'\n\n';
	letnext=widget.widgets[1];
	widget.select(next);
	source+=next.model.sharedModel.getSource()+'\n\n';
	next=widget.widgets[2];
	widget.select(next);
	source+=next.model.sharedModel.getSource();
	constcount=widget.widgets.length;
	NotebookActions.mergeCells(widget);
	expect(widget.widgets.length).toBe(count-2);
	expect(widget.activeCell!.model.sharedModel.getSource()).toBe(source);
	});

	it('should be a no-op if there is no model',()=>{
	widget.model=null;
	NotebookActions.mergeCells(widget);
	expect(widget.activeCell).toBeNull();
	});

	it('should select the next cell if there is only one cell selected',()=>{
	letsource=widget.activeCell!.model.sharedModel.getSource()+'\n\n';
	constnext=widget.widgets[1];
	source+=next.model.sharedModel.getSource();
	NotebookActions.mergeCells(widget);
	expect(widget.activeCell!.model.sharedModel.getSource()).toBe(source);
	});

	it('should select the previous cell if there is only one cell selected and mergeAbove is true',()=>{
	widget.activeCellIndex=1;
	letsource=widget.activeCell!.model.sharedModel.getSource();
	constprevious=widget.widgets[0];
	source=previous.model.sharedModel.getSource()+'\n\n'+source;
	NotebookActions.mergeCells(widget,true);
	expect(widget.activeCell!.model.sharedModel.getSource()).toBe(source);
	});

	it('should do nothing if first cell selected and mergeAbove is true',()=>{
	letsource=widget.activeCell!.model.sharedModel.getSource();
	constcellNumber=widget.widgets.length;
	NotebookActions.mergeCells(widget,true);
	expect(widget.widgets.length).toBe(cellNumber);
	expect(widget.activeCell!.model.sharedModel.getSource()).toBe(source);
	});

	it('should clear the outputs of a code cell',()=>{
	NotebookActions.mergeCells(widget);
	constcell=widget.activeCellasCodeCell;
	expect(cell.model.outputs.length).toBe(0);
	});

	it('should mark cell as trusted as cells without output are trusted',()=>{
	NotebookActions.mergeCells(widget);
	constcell=widget.activeCellasCodeCell;
	expect(cell.model.trusted).toBe(true);
	});

	it('should preserve the widget mode',()=>{
	widget.mode='edit';
	NotebookActions.mergeCells(widget);
	expect(widget.mode).toBe('edit');
	widget.mode='command';
	NotebookActions.mergeCells(widget);
	expect(widget.mode).toBe('command');
	});

	it('should be undo-able',()=>{
	constsource=widget.activeCell!.model.sharedModel.getSource();
	constcount=widget.widgets.length;
	NotebookActions.mergeCells(widget);
	NotebookActions.undo(widget);
	expect(widget.widgets.length).toBe(count);
	constcell=widget.widgets[0];
	expect(cell.model.sharedModel.getSource()).toBe(source);
	});

	it('should unrender a markdown cell',()=>{
	NotebookActions.changeCellType(widget,'markdown');
	letcell=widget.activeCellasMarkdownCell;
	cell.rendered=true;
	NotebookActions.mergeCells(widget);
	cell=widget.activeCellasMarkdownCell;
	expect(cell.rendered).toBe(false);
	expect(widget.mode).toBe('command');
	});

	it('should preserve the cell type of the active cell',()=>{
	NotebookActions.changeCellType(widget,'raw');
	NotebookActions.mergeCells(widget);
	expect(widget.activeCell).toBeInstanceOf(RawCell);
	expect(widget.mode).toBe('command');
	});

	it.each(['raw','markdown']asCellType[])(
	'should merge attachments if the last selected cell is a %s cell',
	type=>{
	for(leti=0;i<2;i++){
	NotebookActions.changeCellType(widget,type);
	constmarkdownCell=widget.widgets[i]asMarkdownCell;
	constattachment:IMimeBundle={'text/plain':'test'};
	markdownCell.model.attachments.set(UUID.uuid4(),attachment);
	widget.select(markdownCell);
	}
	NotebookActions.mergeCells(widget);
	constmodel=(widget.activeCellasMarkdownCell).model;
	expect(model.attachments.length).toBe(2);
	}
	);

	it('should drop attachments if the last selected cell is a code cell',()=>{
	NotebookActions.changeCellType(widget,'markdown');
	constmarkdownCell=widget.activeCellasMarkdownCell;
	constattachment:IMimeBundle={'text/plain':'test'};
	markdownCell.model.attachments.set(UUID.uuid4(),attachment);

	constcodeCell=widget.widgets[1];
	widget.select(codeCell);
	NotebookActions.changeCellType(widget,'code');
	NotebookActions.deselectAll(widget);

	widget.select(markdownCell);
	widget.select(codeCell);

	NotebookActions.mergeCells(widget);

	constmodel=widget.activeCell!.model.toJSON();
	expect(model.cell_type).toEqual('code');
	expect(model.attachments).toBeUndefined();
	});

	it('should merge cells with double newline when addExtraLine is true (default)',()=>{
	constfirstCell=widget.activeCell!;
	constfirstSource=firstCell.model.sharedModel.getSource();
	constsecondCell=widget.widgets[1];
	constsecondSource=secondCell.model.sharedModel.getSource();
	widget.select(secondCell);

	NotebookActions.mergeCells(widget,false,true);

	constexpectedSource=firstSource+'\n\n'+secondSource;
	expect(widget.activeCell!.model.sharedModel.getSource()).toBe(
	expectedSource
	);
	});

	it('should merge cells with single newline when addExtraLine is false',()=>{
	constfirstCell=widget.activeCell!;
	constfirstSource=firstCell.model.sharedModel.getSource();
	constsecondCell=widget.widgets[1];
	constsecondSource=secondCell.model.sharedModel.getSource();
	widget.select(secondCell);

	NotebookActions.mergeCells(widget,false,false);

	constexpectedSource=firstSource+'\n'+secondSource;
	expect(widget.activeCell!.model.sharedModel.getSource()).toBe(
	expectedSource
	);
	});
	});

try-fix-test

64a84c3

Darshan808 commented

Dec 11, 2025

View reviewed changes

packages/notebook/test/actions.spec.ts

		expect(mergedCell.model.sharedModel.getSource()).toBe(initialSource);
		expect(mergedCell.model.sharedModel.executionState).toBe('idle');
		// Undo the merge (which splits the cells again)
		NotebookActions.undo(widget);

Copy link

MemberAuthor

Darshan808Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Not sure why thisundo isn't undoing the changes!

krassowski marked this pull request as draft

December 17, 2025 12:39

Labels

bug pkg:notebook

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix stale`[*]` execution indicator on undo#18173

Are you sure you want to change the base?

Fix stale`[*]` execution indicator on undo#18173

Uh oh!