feat: Show an indicator for stale outputs#297
Conversation
📝 WalkthroughWalkthroughAdds two VS Code notebook status bar providers for Deepnote: StaleOutputStatusBarProvider (detects stale code-cell outputs by tracking and persisting per-cell SHA‑256 content hashes) and DirtyInputBlockStatusBarProvider (detects stale input-block values by tracking last-executed input hashes). Both register as activation services, listen for notebook/document/execution events, maintain in-memory maps of executed hashes, emit status bar refresh events, and persist/read per-cell metadata. Adds test helpers for mocking notebooks/cells, updates numerous unit tests to use them, and registers the new providers in the notebooks service registry. Sequence Diagram(s)sequenceDiagram
participant User
participant Execution as Cell Execution System
participant Provider as StaleOutputStatusBarProvider
participant Metadata as Cell Metadata
participant Workspace as WorkspaceEdit
participant StatusBar as Status Bar UI
User->>Execution: Execute cell
Execution->>Provider: Notify execution complete (state -> Idle)
Provider->>Provider: Compute content hash (SHA-256)
Provider->>Workspace: Apply WorkspaceEdit to persist __deepnotePocket.contentHash
Workspace->>Metadata: Update cell metadata
Provider->>StatusBar: Emit onDidChangeCellStatusBarItems
User->>User: Edit cell content
StatusBar->>Provider: provideCellStatusBarItems called
Provider->>Metadata: Read stored contentHash
Provider->>Provider: Compute current content hash
alt Hashes match
Provider->>StatusBar: No status item
else Hashes differ
Provider->>StatusBar: Return "Stale output" warning with "Re-run cell" command
StatusBar->>User: Display stale-output indicator
end
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #297 +/- ##
===========================
===========================
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/notebooks/deepnote/staleOutputStatusBarProvider.ts`:
- Around line 32-172: Reorder the class members so all public members come
before private ones and each group is alphabetized: place public members in this
order: activate, constructor, onDidChangeCellStatusBarItems,
provideCellStatusBarItems; then place private members alphabetically:
_onDidChangeCellStatusBarItems, checkAndCreateStatusBarItem,
executedContentHashes, getCellKey, handleCellExecutionComplete. Ensure you move
the corresponding fields and EventEmitter/property declarations (e.g.,
_onDidChangeCellStatusBarItems, executedContentHashes) into the private block
and keep their implementations intact.
- Around line 45-47: The getCellKey function currently builds keys using
cell.index which shifts on insert/delete/reorder; change it to use a stable cell
identifier such as the cell's document URI (e.g. use
cell.document.uri.toString() or include notebook + cell.document.uri) so the
cache maps consistently to the same cell; update the getCellKey implementation
to return the stable URI-based string instead of using cell.index.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/notebooks/deepnote/staleOutputStatusBarProvider.tssrc/notebooks/deepnote/staleOutputStatusBarProvider.unit.test.tssrc/notebooks/serviceRegistry.node.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Order method, fields and properties first by accessibility (public/private/protected) and then by alphabetical order
Files:
src/notebooks/deepnote/staleOutputStatusBarProvider.unit.test.tssrc/notebooks/serviceRegistry.node.tssrc/notebooks/deepnote/staleOutputStatusBarProvider.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Don't add the Microsoft copyright header to new files
UseUri.joinPath()for constructing file paths instead of string concatenation with/to ensure platform-correct path separators
Follow established patterns when importing new packages, using helper imports rather than direct imports (e.g., useimport { generateUuid } from '../platform/common/uuid'instead of importing uuid directly)
Add blank lines after const groups and before return statements for readability
Separate third-party and local file imports
Files:
src/notebooks/deepnote/staleOutputStatusBarProvider.unit.test.tssrc/notebooks/serviceRegistry.node.tssrc/notebooks/deepnote/staleOutputStatusBarProvider.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.unit.test.ts: Unit tests use Mocha/Chai framework with.unit.test.tsextension
Test files should be placed alongside the source files they test
Useassert.deepStrictEqual()for object comparisons instead of checking individual properties
Files:
src/notebooks/deepnote/staleOutputStatusBarProvider.unit.test.ts
src/notebooks/deepnote/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration located in
src/notebooks/deepnote/with refactored architecture:deepnoteTypes.ts(type definitions),deepnoteNotebookManager.ts(state management),deepnoteNotebookSelector.ts(UI selection logic),deepnoteDataConverter.ts(data transformations),deepnoteSerializer.ts(main serializer/orchestration),deepnoteActivationService.ts(VSCode activation)
Files:
src/notebooks/deepnote/staleOutputStatusBarProvider.unit.test.tssrc/notebooks/deepnote/staleOutputStatusBarProvider.ts
🧠 Learnings (1)
📚 Learning: 2026-01-12T09:12:12.830Z
Learnt from: Artmann
Repo: deepnote/vscode-deepnote PR: 256
File: src/notebooks/deepnote/deepnoteNotebookCommandListener.ts:549-562
Timestamp: 2026-01-12T09:12:12.830Z
Learning: IConfigurationService.updateSetting() automatically prefixes the key with the 'deepnote' namespace. When calling updateSetting, always pass the unprefixed key (e.g., use 'snapshots.enabled' instead of 'deepnote.snapshots.enabled'). This guideline applies to all TypeScript source files under src where updateSetting is used.
Applied to files:
src/notebooks/deepnote/staleOutputStatusBarProvider.unit.test.tssrc/notebooks/serviceRegistry.node.tssrc/notebooks/deepnote/staleOutputStatusBarProvider.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/staleOutputStatusBarProvider.ts (1)
src/platform/deepnote/pocket.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint & Format
- GitHub Check: Build & Test
🔇 Additional comments (4)
src/notebooks/deepnote/staleOutputStatusBarProvider.unit.test.ts (3)
25-69: Helpers look solid for deterministic mocks.
71-170: Good coverage of stale detection scenarios.
172-244: Activation/event wiring tests are thorough.src/notebooks/serviceRegistry.node.ts (1)
183-186: Provider registration matches existing activation pattern.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/notebooks/deepnote/staleOutputStatusBarProvider.ts`:
- Around line 89-120: Replace the magic numeric alignment value in
checkAndCreateStatusBarItem with the enum NotebookCellStatusBarAlignment.Left
and add NotebookCellStatusBarAlignment to the vscode imports; specifically,
update the returned object’s alignment from 1 to
NotebookCellStatusBarAlignment.Left and ensure the import list (where vscode
symbols are imported) includes NotebookCellStatusBarAlignment so the symbol
resolves.
♻️ Duplicate comments (1)
src/notebooks/deepnote/staleOutputStatusBarProvider.ts (1)
32-37: Reorder members by accessibility/alphabetical.Private members appear before public members and public members aren’t alphabetized. Please reorder so public members come first (alphabetized), then private fields/methods. As per coding guidelines, keep public members first and alphabetized.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/notebooks/deepnote/staleOutputStatusBarProvider.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Order method, fields and properties first by accessibility (public/private/protected) and then by alphabetical order
Files:
src/notebooks/deepnote/staleOutputStatusBarProvider.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Don't add the Microsoft copyright header to new files
UseUri.joinPath()for constructing file paths instead of string concatenation with/to ensure platform-correct path separators
Follow established patterns when importing new packages, using helper imports rather than direct imports (e.g., useimport { generateUuid } from '../platform/common/uuid'instead of importing uuid directly)
Add blank lines after const groups and before return statements for readability
Separate third-party and local file imports
Files:
src/notebooks/deepnote/staleOutputStatusBarProvider.ts
src/notebooks/deepnote/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration located in
src/notebooks/deepnote/with refactored architecture:deepnoteTypes.ts(type definitions),deepnoteNotebookManager.ts(state management),deepnoteNotebookSelector.ts(UI selection logic),deepnoteDataConverter.ts(data transformations),deepnoteSerializer.ts(main serializer/orchestration),deepnoteActivationService.ts(VSCode activation)
Files:
src/notebooks/deepnote/staleOutputStatusBarProvider.ts
🧠 Learnings (1)
📚 Learning: 2026-01-12T09:12:12.830Z
Learnt from: Artmann
Repo: deepnote/vscode-deepnote PR: 256
File: src/notebooks/deepnote/deepnoteNotebookCommandListener.ts:549-562
Timestamp: 2026-01-12T09:12:12.830Z
Learning: IConfigurationService.updateSetting() automatically prefixes the key with the 'deepnote' namespace. When calling updateSetting, always pass the unprefixed key (e.g., use 'snapshots.enabled' instead of 'deepnote.snapshots.enabled'). This guideline applies to all TypeScript source files under src where updateSetting is used.
Applied to files:
src/notebooks/deepnote/staleOutputStatusBarProvider.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/staleOutputStatusBarProvider.ts (2)
src/platform/deepnote/pocket.ts (1)
src/test/mocks/vsc/extHostedTypes.ts (2)
WorkspaceEdit(650-834)NotebookEdit(2468-2531)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint & Format
- GitHub Check: Build & Test
🔇 Additional comments (4)
src/notebooks/deepnote/staleOutputStatusBarProvider.ts (4)
38-60: Looks good.Scoped listeners + disposables are handled cleanly.
62-87: LGTM.Guard conditions and hash gating are straightforward.
122-131: LGTM.Cache and key helper are clean.
133-172: Looks good.Hash update + metadata persistence are well contained.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| function createMockCell(options: { | ||
| kind?: NotebookCellKind; | ||
| outputs?: NotebookCellOutput[]; | ||
| metadata?: Record<string, unknown>; | ||
| text?: string; | ||
| }): NotebookCell { |
There was a problem hiding this comment.
It seems this function is duplicated (or very similar) across the codebase. Would this be a good time to extract and reuse?
| * This ensures the stale indicator is removed after re-running a cell. | ||
| */ | ||
| private async handleCellExecutionComplete(cell: NotebookCell): Promise<void> { | ||
| if (cell.kind !== NotebookCellKind.Code) { |
There was a problem hiding this comment.
Question: why only code blocks? would it make sense to also apply this behavior to eg. input blocks?
There was a problem hiding this comment.
This is a VS Code notebook cell that can only be Code or Markdown. We translate our block types into Code cells.
There was a problem hiding this comment.
This is what I meant:
Screen.Recording.2026-01-16.at.15.58.02.mov
There was a problem hiding this comment.
Right now we cant do that because we don't have the relationship between cells. But once we have reactivity in place, we should do this!
There was a problem hiding this comment.
Curious, why does this require reactivity? isn't it simply about being able to tell if the value in the input has changed after a partial of full run has been executed?
There was a problem hiding this comment.
No, just marking the input block as stale as soon as the user changes its value (like you are doing with code blocks).
This is just to highlight to the user that if the input block itself is not rerun, other blocks will still rely on its old value.
Hopefully I'm making sense. 😄
There was a problem hiding this comment.
But input blocks doesn't have outputs to be marked as stale 😕
There was a problem hiding this comment.
But I see what you mean, I can figure something out
There was a problem hiding this comment.
Oh yeah, true, if we are talking about outputs exclusively I agree.
If you think this is out of scope for this PR we can think about this separately.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/notebooks/deepnote/deepnoteTestHelpers.ts`:
- Around line 78-88: createMockCell currently always normalizes the
options.metadata to {} which prevents callers from explicitly passing undefined;
change the normalization in createMockCell to preserve an explicit undefined by
checking whether the caller provided the metadata property (e.g. use
Object.prototype.hasOwnProperty.call(options || {}, 'metadata') ? metadata : {})
instead of applying a default in the destructuring. Apply the same pattern to
the other metadata normalization block referenced around lines 119-126 so
explicit undefined is preserved there as well.
Change metadata normalization to check if the property was explicitly provided rather than applying a default in destructuring. This allows tests to pass explicit undefined for metadata and notebookMetadata.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/notebooks/deepnote/dirtyInputBlockStatusBarProvider.ts`:
- Around line 120-159: cleanupHashes is failing because getCellKey uses
cell.document.uri.toString() (vscode-notebook-cell: URIs) while cleanupHashes
compares keys with notebook.uri.toString() (file: URIs), so keys never match and
stale entries remain; update cleanupHashes to normalize or derive the notebook
identity from the cell keys (or convert notebook.uri to the notebook-cell
scheme) before comparing. Specifically, in cleanupHashes (and any related lookup
code) use the same key format as getCellKey — e.g., compute a prefix by
converting notebook.uri to the corresponding notebook-cell URI base or iterate
executedContentHashes and parse each key's notebook component (split or parse
URI with vscode.Uri) to compare the underlying notebook URI, then delete
matching entries from executedContentHashes; refer to cleanupHashes, getCellKey,
executedContentHashes, and cell.document.uri when making the change.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/notebooks/deepnote/deepnoteTestHelpers.ts`:
- Around line 58-64: The mock NotebookCellOutput returned by createMockOutput
lacks the required id field so it fails NotebookCellOutput.isNotebookCellOutput
validation; update createMockOutput to generate and include an id: string on the
returned object (use generateUuid() to produce the id) and add the import for
generateUuid from ../../platform/common/uuid alongside the existing vscode
imports; ensure the returned shape still has items and metadata but includes id
so tests mirror VS Code's NotebookCellOutput shape.
♻️ Duplicate comments (1)
src/notebooks/deepnote/deepnoteTestHelpers.ts (1)
42-49: Preserve explicit undefined notebook metadata.
createMockNotebookstill normalizesmetadatato{}even when the caller explicitly passesundefined, socreateMockCellcan’t model “undefined notebookMetadata”. Mirror the explicit-undefined handling used increateMockCell.🔧 Proposed fix
export function createMockNotebook(options?: CreateMockNotebookOptions): NotebookDocument { - const { notebookType = 'deepnote', uri = Uri.file('/test/notebook.deepnote'), metadata = {} } = options ?? {}; + const opts = options ?? {}; + const { notebookType = 'deepnote', uri = Uri.file('/test/notebook.deepnote') } = opts; + const metadata = Object.prototype.hasOwnProperty.call(opts, 'metadata') ? opts.metadata : {}; return { uri, notebookType, metadata
The mock NotebookCellOutput was missing the required id field, causing NotebookCellOutput.isNotebookCellOutput validation to fail. Generate the id using generateUuid() to match VS Code's NotebookCellOutput shape.
cleanupHashes was comparing notebook.uri (file: scheme) against cell keys which use cell.document.uri (vscode-notebook-cell: scheme). These never matched, leaving stale entries in the cache. Now parse each key as a URI and compare the path portion with the notebook's path.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/notebooks/deepnote/dirtyInputBlockStatusBarProvider.ts`:
- Around line 33-44: The class members are out of order per project conventions;
move public members before private ones and alphabetize private members by name:
ensure onDidChangeCellStatusBarItems (public) appears before
_onDidChangeCellStatusBarItems and executedContentHashes (private), and keep the
constructor positioned according to the convention (public-facing API first),
then list private fields/methods in alphabetical order (e.g.,
_onDidChangeCellStatusBarItems, executedContentHashes) to satisfy the reordering
rule in dirtyInputBlockStatusBarProvider.ts.
♻️ Duplicate comments (1)
src/notebooks/deepnote/deepnoteTestHelpers.ts (1)
44-51: Preserve explicitundefinednotebook metadata.Destructuring with
metadata = {}converts an explicitundefinedinto{}, which defeats the “explicit undefined” behavior you preserve increateMockCell. Use a has‑own check here too.🔧 Proposed fix
export function createMockNotebook(options?: CreateMockNotebookOptions): NotebookDocument { - const { notebookType = 'deepnote', uri = Uri.file('/test/notebook.deepnote'), metadata = {} } = options ?? {}; + const opts = options ?? {}; + const { notebookType = 'deepnote', uri = Uri.file('/test/notebook.deepnote') } = opts; + const metadata = Object.prototype.hasOwnProperty.call(opts, 'metadata') ? opts.metadata : {}; return { uri, notebookType, metadata } as NotebookDocument; }
2 participants
stale-output-detection.mp4
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.