Repository navigation
Isolate copied notebook output items - #347117
Open
Sameer (sameerihs) wants to merge 1 commit into
Open
Sameer (sameerihs) wants to merge 1 commit into
Sameer (sameerihs) wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the change isolating copied notebook output-item arrays.
No open issues 🎉
What changed in this PR
Fixes #347116 by giving copied notebook outputs independent item arrays, preventing later stream appends from changing the unexecuted copy’s saved output. Existing item buffers remain shared.
Changes:
- Shallow-copy output-item arrays when cloning notebook cells.
- Add command-level regressions for appends to both the source and copied cell.
- Add an API regression that copies a running cell, appends output, and checks the saved bytes.
| File | Description |
|---|---|
| src/vs/workbench/contrib/notebook/test/browser/cellOperations.test.ts | Tests output isolation in both append directions. |
| src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts | Creates a separate output-item array for each clone. |
| extensions/vscode-api-tests/src/singlefolder-tests/notebook.kernel.test.ts | Verifies copied output remains unchanged through serialization and saving. |
Review effort: Balanced
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #347116
Copying a running notebook cell shares its mutable output-item array. Later stream appends can therefore change the saved output of the unexecuted copy. Give each cloned output its own array while retaining the existing item buffers.
Two model/command regressions and a public API regression cover appending and saving. The API test uses a synthetic controller, Copy Cell Down and the existing serializer, then checks the written bytes. It fails on unchanged main and packaged stable 1.145.2 and passes with the fix.
Validation on macOS: 111 neighbouring unit tests pass with five existing pending cases; all 11 Notebook Kernel API tests pass. Client/API compilation, client typechecking and changed-file lint pass.
This touches the clone helper also used by #319694, whose change preserves user metadata rather than output-array ownership.