-
Notifications
You must be signed in to change notification settings - Fork 7
feat: Show an indicator for stale outputs #297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughA new VSCode extension component StaleOutputStatusBarProvider was added to detect potentially stale outputs in Deepnote notebook cells. It registers as a notebook cell status bar provider, tracks per-cell execution hashes in memory, and persists a contentHash to cell metadata on successful execution. provideCellStatusBarItems compares the current content hash with stored and in-memory hashes and returns a warning status item with a "Re-run cell" action when they differ. The provider is registered as a singleton activation service and unit tests cover its behavior and event-driven updates. Sequence Diagram(s)sequenceDiagram
participant User
participant Execution as Cell Execution System
participant Provider as StaleOutputStatusBarProvider
participant Metadata as Cell Metadata
participant StatusBar as Status Bar UI
User->>Execution: Execute cell
Execution->>Provider: Notify execution complete (Idle)
Provider->>Provider: Compute content hash (sha256)
Provider->>Metadata: Persist contentHash in __deepnotePocket
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 (stale)
Provider->>StatusBar: Return warning item 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:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I meant:
Screen.Recording.2026-01-16.at.15.58.02.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
stale-output-detection.mp4
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.