Skip to content

Conversation

@Artmann
Copy link
Contributor

@Artmann Artmann commented Jan 16, 2026

stale-output-detection.mp4

Summary by CodeRabbit

  • New Features

    • Notebook cells now show a "stale output" status when cell content has changed since last execution in Deepnote notebooks, with a quick "Re-run cell" action to refresh outputs.
  • Tests

    • Added unit tests covering stale-output detection and status updates across cell types, outputs, and execution events.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

📝 Walkthrough

Walkthrough

A 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
Loading
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title accurately summarizes the main change: adding a status bar indicator for stale notebook outputs.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Jan 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0%. Comparing base (9d21508) to head (645f269).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #297   +/-   ##
===========================
===========================
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 9d21508 and 2bcf71e.

📒 Files selected for processing (3)
  • src/notebooks/deepnote/staleOutputStatusBarProvider.ts
  • src/notebooks/deepnote/staleOutputStatusBarProvider.unit.test.ts
  • src/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.ts
  • src/notebooks/serviceRegistry.node.ts
  • 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
Use Uri.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., use import { 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.ts
  • src/notebooks/serviceRegistry.node.ts
  • src/notebooks/deepnote/staleOutputStatusBarProvider.ts
**/*.unit.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.unit.test.ts: Unit tests use Mocha/Chai framework with .unit.test.ts extension
Test files should be placed alongside the source files they test
Use assert.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.ts
  • 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.unit.test.ts
  • src/notebooks/serviceRegistry.node.ts
  • src/notebooks/deepnote/staleOutputStatusBarProvider.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/staleOutputStatusBarProvider.ts (1)
src/platform/deepnote/pocket.ts (1)
  • Pocket (23-34)
⏰ 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 2bcf71e and 645f269.

📒 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
Use Uri.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., use import { 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)
  • Pocket (23-34)
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.

@Artmann Artmann marked this pull request as ready for review January 16, 2026 13:53
@Artmann Artmann requested a review from a team as a code owner January 16, 2026 13:53
Comment on lines +25 to +30
function createMockCell(options: {
kind?: NotebookCellKind;
outputs?: NotebookCellOutput[];
metadata?: Record<string, unknown>;
text?: string;
}): NotebookCell {
Copy link
Contributor

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) {
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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

Copy link
Contributor Author

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!

Copy link
Contributor

@dinohamzic dinohamzic Jan 16, 2026

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants