add logging service to notebook text model to trace edit activity (#270786)

* add logging service to text model and small cleanup

* log output edits

* less redundant category

* Update src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Aaron Munger
2025-10-10 11:35:41 -07:00
committed by GitHub
parent 1414a15c9a
commit 63173cdabc
6 changed files with 92 additions and 27 deletions

View File

@@ -24,6 +24,10 @@ export class NotebookLoggingService extends Disposable implements INotebookLoggi
this._logger = this._register(loggerService.createLogger(logChannelId, { name: nls.localize('renderChannelName', "Notebook"), group: windowLogGroup }));
}
trace(category: string, output: string): void {
this._logger.trace(`[${category}] ${output}`);
}
debug(category: string, output: string): void {
this._logger.debug(`[${category}] ${output}`);
}

View File

@@ -15,12 +15,13 @@ import { createTextBuffer, TextModel } from '../../../../../editor/common/model/
import { PLAINTEXT_LANGUAGE_ID } from '../../../../../editor/common/languages/modesRegistry.js';
import { ILanguageService } from '../../../../../editor/common/languages/language.js';
import { NotebookCellOutputTextModel } from './notebookCellOutputTextModel.js';
import { CellInternalMetadataChangedEvent, CellKind, ICell, ICellDto2, ICellOutput, IOutputDto, IOutputItemDto, NotebookCellCollapseState, NotebookCellInternalMetadata, NotebookCellMetadata, NotebookCellOutputsSplice, TransientCellMetadata, TransientOptions } from '../notebookCommon.js';
import { CellInternalMetadataChangedEvent, CellKind, ICell, ICellDto2, ICellOutput, IOutputItemDto, NotebookCellCollapseState, NotebookCellDefaultCollapseConfig, NotebookCellInternalMetadata, NotebookCellMetadata, NotebookCellOutputsSplice, TransientCellMetadata, TransientOptions } from '../notebookCommon.js';
import { ThrottledDelayer } from '../../../../../base/common/async.js';
import { ILanguageDetectionService } from '../../../../services/languageDetection/common/languageDetectionWorkerService.js';
import { toFormattedString } from '../../../../../base/common/jsonFormatter.js';
import { IModelContentChangedEvent } from '../../../../../editor/common/textModelEvents.js';
import { splitLines } from '../../../../../base/common/strings.js';
import { INotebookLoggingService } from '../notebookLoggingService.js';
export class NotebookCellTextModel extends Disposable implements ICell {
private readonly _onDidChangeTextModel = this._register(new Emitter<void>());
@@ -190,26 +191,34 @@ export class NotebookCellTextModel extends Disposable implements ICell {
private _hasLanguageSetExplicitly: boolean = false;
get hasLanguageSetExplicitly(): boolean { return this._hasLanguageSetExplicitly; }
private _source: string;
private _language: string;
private _mime: string | undefined;
public readonly cellKind: CellKind;
public readonly collapseState: NotebookCellCollapseState | undefined;
constructor(
readonly uri: URI,
public readonly handle: number,
private readonly _source: string,
private _language: string,
private _mime: string | undefined,
public readonly cellKind: CellKind,
outputs: IOutputDto[],
metadata: NotebookCellMetadata | undefined,
internalMetadata: NotebookCellInternalMetadata | undefined,
public readonly collapseState: NotebookCellCollapseState | undefined,
cell: ICellDto2,
public readonly transientOptions: TransientOptions,
private readonly _languageService: ILanguageService,
private readonly _defaultEOL: model.DefaultEndOfLine,
defaultCollapseConfig: NotebookCellDefaultCollapseConfig | undefined,
private readonly _languageDetectionService: ILanguageDetectionService | undefined = undefined,
private readonly _notebookLoggingService: INotebookLoggingService
) {
super();
this._outputs = outputs.map(op => new NotebookCellOutputTextModel(op));
this._metadata = metadata ?? {};
this._internalMetadata = internalMetadata ?? {};
this._source = cell.source;
this._language = cell.language;
this._mime = cell.mime;
this.cellKind = cell.cellKind;
// Compute collapse state: use cell's state if provided, otherwise use default config for this cell kind
const defaultConfig = cell.cellKind === CellKind.Code ? defaultCollapseConfig?.codeCell : defaultCollapseConfig?.markupCell;
this.collapseState = cell.collapseState ?? (defaultConfig ?? undefined);
this._outputs = cell.outputs.map(op => new NotebookCellOutputTextModel(op));
this._metadata = cell.metadata ?? {};
this._internalMetadata = cell.internalMetadata ?? {};
}
enableAutoLanguageDetection() {
@@ -322,6 +331,7 @@ export class NotebookCellTextModel extends Disposable implements ICell {
}
spliceNotebookCellOutputs(splice: NotebookCellOutputsSplice): void {
this._notebookLoggingService.trace('textModelEdits', `splicing outputs at ${splice.start} length: ${splice.deleteCount} with ${splice.newOutputs.length} new outputs`);
if (splice.deleteCount > 0 && splice.newOutputs.length > 0) {
const commonLen = Math.min(splice.deleteCount, splice.newOutputs.length);
// update
@@ -349,6 +359,7 @@ export class NotebookCellTextModel extends Disposable implements ICell {
return false;
}
this._notebookLoggingService.trace('textModelEdits', `replacing an output item at index ${outputIndex}`);
const output = this.outputs[outputIndex];
// convert to dto and dispose the cell output model
output.replaceData({
@@ -369,6 +380,7 @@ export class NotebookCellTextModel extends Disposable implements ICell {
}
const output = this.outputs[outputIndex];
this._notebookLoggingService.trace('textModelEdits', `${append ? 'appending' : 'replacing'} ${items.length} output items to for output index ${outputIndex}`);
if (append) {
output.appendData(items);
} else {

View File

@@ -23,8 +23,9 @@ import { IModelContentChangedEvent } from '../../../../../editor/common/textMode
import { IResourceUndoRedoElement, IUndoRedoElement, IUndoRedoService, IWorkspaceUndoRedoElement, UndoRedoElementType, UndoRedoGroup } from '../../../../../platform/undoRedo/common/undoRedo.js';
import { ILanguageDetectionService } from '../../../../services/languageDetection/common/languageDetectionWorkerService.js';
import { SnapshotContext } from '../../../../services/workingCopy/common/fileWorkingCopy.js';
import { CellEditType, CellKind, CellUri, diff, ICell, ICellDto2, ICellEditOperation, ICellOutput, INotebookSnapshotOptions, INotebookTextModel, IOutputDto, IOutputItemDto, ISelectionState, NotebookCellCollapseState, NotebookCellDefaultCollapseConfig, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookCellMetadata, NotebookCellOutputsSplice, NotebookCellsChangeType, NotebookCellTextModelSplice, NotebookData, NotebookDocumentMetadata, NotebookTextModelChangedEvent, NotebookTextModelWillAddRemoveEvent, NullablePartialNotebookCellInternalMetadata, NullablePartialNotebookCellMetadata, TransientOptions } from '../notebookCommon.js';
import { CellEditType, CellUri, diff, ICell, ICellDto2, ICellEditOperation, ICellOutput, INotebookSnapshotOptions, INotebookTextModel, IOutputDto, IOutputItemDto, ISelectionState, NotebookCellDefaultCollapseConfig, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookCellMetadata, NotebookCellOutputsSplice, NotebookCellsChangeType, NotebookCellTextModelSplice, NotebookData, NotebookDocumentMetadata, NotebookTextModelChangedEvent, NotebookTextModelWillAddRemoveEvent, NullablePartialNotebookCellInternalMetadata, NullablePartialNotebookCellMetadata, TransientOptions } from '../notebookCommon.js';
import { INotebookExecutionStateService } from '../notebookExecutionStateService.js';
import { INotebookLoggingService } from '../notebookLoggingService.js';
import { CellMetadataEdit, MoveCellEdit, SpliceCellsEdit } from './cellEdit.js';
import { NotebookCellOutputTextModel } from './notebookCellOutputTextModel.js';
import { NotebookCellTextModel } from './notebookCellTextModel.js';
@@ -252,6 +253,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
@ILanguageService private readonly _languageService: ILanguageService,
@ILanguageDetectionService private readonly _languageDetectionService: ILanguageDetectionService,
@INotebookExecutionStateService private readonly _notebookExecutionStateService: INotebookExecutionStateService,
@INotebookLoggingService private readonly _notebookLoggingService: INotebookLoggingService
) {
super();
this.transientOptions = options;
@@ -309,6 +311,8 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
this._overwriteAlternativeVersionId(alternativeVersionId);
}
);
this._notebookLoggingService.trace('notebookTextModel', `Initialized notebook text model for ${uri.toString()}`);
}
setCellCollapseDefault(collapseConfig: NotebookCellDefaultCollapseConfig | undefined) {
@@ -323,9 +327,17 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
const mainCells = cells.map(cell => {
const cellHandle = this._cellhandlePool++;
const cellUri = CellUri.generate(this.uri, cellHandle);
const collapseState = this._getDefaultCollapseState(cell);
return new NotebookCellTextModel(cellUri, cellHandle, cell.source, cell.language, cell.mime, cell.cellKind, cell.outputs, cell.metadata, cell.internalMetadata, collapseState, this.transientOptions, this._languageService,
this._modelService.getCreationOptions(cell.language, cellUri, false).defaultEOL, this._languageDetectionService);
return new NotebookCellTextModel(
cellUri,
cellHandle,
cell,
this.transientOptions,
this._languageService,
this._modelService.getCreationOptions(cell.language, cellUri, false).defaultEOL,
this._defaultCollapseConfig,
this._languageDetectionService,
this._notebookLoggingService
);
});
for (let i = 0; i < mainCells.length; i++) {
@@ -621,6 +633,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
}
applyEdits(rawEdits: ICellEditOperation[], synchronous: boolean, beginSelectionState: ISelectionState | undefined, endSelectionsComputer: () => ISelectionState | undefined, undoRedoGroup: UndoRedoGroup | undefined, computeUndoRedo: boolean): boolean {
this._notebookLoggingService.trace('textModelEdits', `Begin applying ${rawEdits.length} raw edits`);
this._pauseableEmitter.pause();
try {
this._operationManager.pushStackElement(this._alternativeVersionId, undefined);
@@ -648,6 +661,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
// Broadcast changes
this._pauseableEmitter.fire({ rawEvents: [], versionId: this.versionId, synchronous: synchronous, endSelectionState: endSelections });
this._notebookLoggingService.trace('textModelEdits', `End applying ${rawEdits.length} raw edits`);
}
}
} finally {
@@ -821,11 +835,6 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
return mergedEdits;
}
private _getDefaultCollapseState(cellDto: ICellDto2): NotebookCellCollapseState | undefined {
const defaultConfig = cellDto.cellKind === CellKind.Code ? this._defaultCollapseConfig?.codeCell : this._defaultCollapseConfig?.markupCell;
return cellDto.collapseState ?? (defaultConfig ?? undefined);
}
private _replaceCells(index: number, count: number, cellDtos: ICellDto2[], synchronous: boolean, computeUndoRedo: boolean, beginSelectionState: ISelectionState | undefined, undoRedoGroup: UndoRedoGroup | undefined): void {
if (count === 0 && cellDtos.length === 0) {
@@ -849,13 +858,19 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
const cells = cellDtos.map(cellDto => {
const cellHandle = this._cellhandlePool++;
const cellUri = CellUri.generate(this.uri, cellHandle);
const collapseState = this._getDefaultCollapseState(cellDto);
if (!cellDto.outputs) {
cellDto.outputs = [];
}
const cell = new NotebookCellTextModel(
cellUri, cellHandle,
cellDto.source, cellDto.language, cellDto.mime, cellDto.cellKind, cellDto.outputs || [], cellDto.metadata, cellDto.internalMetadata, collapseState, this.transientOptions,
cellUri,
cellHandle,
cellDto,
this.transientOptions,
this._languageService,
this._modelService.getCreationOptions(cellDto.language, cellUri, false).defaultEOL,
this._languageDetectionService
this._defaultCollapseConfig,
this._languageDetectionService,
this._notebookLoggingService
);
const textModel = this._modelService.getModel(cellUri);
if (textModel && textModel instanceof TextModel) {

View File

@@ -13,4 +13,5 @@ export interface INotebookLoggingService {
warn(category: string, output: string): void;
error(category: string, output: string): void;
debug(category: string, output: string): void;
trace(category: string, output: string): void;
}

View File

@@ -32,6 +32,7 @@ import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/tes
import { mainWindow } from '../../../../../base/browser/window.js';
import { ICodeEditorService } from '../../../../../editor/browser/services/codeEditorService.js';
import { ILanguageDetectionService } from '../../../../services/languageDetection/common/languageDetectionWorkerService.js';
import { INotebookLoggingService } from '../../common/notebookLoggingService.js';
suite('NotebookViewModel', () => {
ensureNoDisposablesAreLeakedInTestSuite();
@@ -45,6 +46,7 @@ suite('NotebookViewModel', () => {
let languageService: ILanguageService;
let languageDetectionService: ILanguageDetectionService;
let notebookExecutionStateService: INotebookExecutionStateService;
let notebookLogger: INotebookLoggingService;
suiteSetup(() => {
disposables = new DisposableStore();
@@ -56,6 +58,7 @@ suite('NotebookViewModel', () => {
languageService = instantiationService.get(ILanguageService);
languageDetectionService = instantiationService.get(ILanguageDetectionService);
notebookExecutionStateService = instantiationService.get(INotebookExecutionStateService);
notebookLogger = instantiationService.get(INotebookLoggingService);
instantiationService.stub(IConfigurationService, new TestConfigurationService());
instantiationService.stub(IThemeService, new TestThemeService());
@@ -64,7 +67,7 @@ suite('NotebookViewModel', () => {
suiteTeardown(() => disposables.dispose());
test('ctor', function () {
const notebook = new NotebookTextModel('notebook', URI.parse('test'), [], {}, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false, cellContentMetadata: {} }, undoRedoService, modelService, languageService, languageDetectionService, notebookExecutionStateService);
const notebook = new NotebookTextModel('notebook', URI.parse('test'), [], {}, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false, cellContentMetadata: {} }, undoRedoService, modelService, languageService, languageDetectionService, notebookExecutionStateService, notebookLogger);
const model = new NotebookEditorTestModel(notebook);
const options = new NotebookOptions(mainWindow, false, undefined, instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService), instantiationService.get(ICodeEditorService));
const eventDispatcher = new NotebookEventDispatcher();

View File

@@ -71,6 +71,16 @@ import { INotebookOutlineEntryFactory, NotebookOutlineEntryFactory } from '../..
import { IOutlineService } from '../../../../services/outline/browser/outline.js';
import { DefaultEndOfLine } from '../../../../../editor/common/model.js';
import { ITextResourcePropertiesService } from '../../../../../editor/common/services/textResourceConfiguration.js';
import { INotebookLoggingService } from '../../common/notebookLoggingService.js';
class NullNotebookLoggingService implements INotebookLoggingService {
_serviceBrand: undefined;
info(category: string, output: string): void { }
warn(category: string, output: string): void { }
error(category: string, output: string): void { }
debug(category: string, output: string): void { }
trace(category: string, message: string): void { }
}
export class TestCell extends NotebookCellTextModel {
constructor(
@@ -82,7 +92,26 @@ export class TestCell extends NotebookCellTextModel {
outputs: IOutputDto[],
languageService: ILanguageService,
) {
super(CellUri.generate(URI.parse('test:///fake/notebook'), handle), handle, source, language, Mimes.text, cellKind, outputs, undefined, undefined, undefined, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false, cellContentMetadata: {} }, languageService, DefaultEndOfLine.LF);
super(
CellUri.generate(URI.parse('test:///fake/notebook'), handle),
handle,
{
source,
language,
mime: Mimes.text,
cellKind,
outputs,
metadata: undefined,
internalMetadata: undefined,
collapseState: undefined
},
{ transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false, cellContentMetadata: {} },
languageService,
DefaultEndOfLine.LF,
undefined, // defaultCollapseConfig
undefined, // languageDetectionService
new NullNotebookLoggingService()
);
}
}
@@ -207,6 +236,7 @@ export function setupInstantiationService(disposables: Pick<DisposableStore, 'ad
instantiationService.stub(IOutlineService, new class extends mock<IOutlineService>() { override registerOutlineCreator() { return { dispose() { } }; } });
instantiationService.stub(INotebookCellOutlineDataSourceFactory, instantiationService.createInstance(NotebookCellOutlineDataSourceFactory));
instantiationService.stub(INotebookOutlineEntryFactory, instantiationService.createInstance(NotebookOutlineEntryFactory));
instantiationService.stub(INotebookLoggingService, new NullNotebookLoggingService());
instantiationService.stub(ILanguageDetectionService, new class MockLanguageDetectionService implements ILanguageDetectionService {
_serviceBrand: undefined;