fix: Prevent main file changes when running blocks with snapshots enabled (#312)

* fix: Prevent main file changes when running blocks with snapshots enabled

- Strip executionCount from blocks in snapshot mode (was only stripping
  outputs and execution timestamps)
- Only update modifiedAt when actual content changes occur in snapshot mode,
  not when running cells
- Add detectContentChanges helper to compare block content, type, and IDs
  while ignoring outputs and execution metadata

Author: Artmann

AGENTS.md

@@ -0,0 +1,83 @@
+# Code Style & Organization
+
+- Order method, fields and properties, first by accessibility and then by alphabetical order.
+- Don't add the Microsoft copyright header to new files.
+- Use `Uri.joinPath()` for constructing file paths to ensure platform-correct path separators (e.g., `Uri.joinPath(venvPath, 'share', 'jupyter', 'kernels')` instead of string concatenation with `/`)
+- Follow established patterns, especially when importing new packages (e.g. instead of importing uuid directly, use the helper `import { generateUuid } from '../platform/common/uuid';`)
+
+## Code conventions
+
+- Always run `npx prettier` before committing
+
+## Testing
+
+- Unit tests use Mocha/Chai framework with `.unit.test.ts` extension
+- Test files should be placed alongside the source files they test
+- Run all tests: `npm test` or `npm run test:unittests`
+- Run single test file: `npx mocha --config ./build/.mocha.unittests.js.json ./out/path/to/file.unit.test.js`
+- Tests run against compiled JavaScript files in `out/` directory
+- Use `assert.deepStrictEqual()` for object comparisons instead of checking individual properties
+
+## Project Structure
+
+- VSCode extension for Jupyter notebooks
+- Uses dependency injection with inversify
+- Follows separation of concerns pattern
+- TypeScript codebase that compiles to `out/` directory
+
+## Deepnote Integration
+
+- Located in `src/notebooks/deepnote/`
+- 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
+- Whitespace is good for readability, add a blank line after const groups and before return statements
+- Separate third-party and local file imports
+- How the extension works is described in @specs/architecture.md
+- Snapshot mode: avoid persisting execution-time metadata (e.g., `contentHash`) to prevent dirty state; rely on in-memory tracking when needed
+- `DeepnoteNotebookSerializer.detectContentChanges` should consider notebook-level fields (e.g., `name`, `executionMode`, `isModule`, `workingDirectory`) and detect removed notebooks, in addition to block-level comparisons
+
+## Best Practices
+
+### Resource Cleanup
+
+- Always dispose `CancellationTokenSource` - never create inline without storing/disposing
+- Use try/finally to ensure cleanup:
+
+  ```typescript
+  const cts = new CancellationTokenSource();
+  try {
+    await fn(cts.token);
+  } finally {
+    cts.dispose();
+  }
+  ```
+
+
+### DRY Principle
+
+- Extract duplicate logic into helper methods to prevent drift
+- When similar logic appears in multiple places (e.g., placeholder controller setup, interpreter validation), consolidate it
+
+### Magic Numbers
+
+- Extract magic numbers (retry counts, delays, timeouts) as named constants near the top of the module
+
+### Error Handling
+
+- Use per-iteration error handling in loops - wrap each iteration in try/catch so one failure doesn't stop the rest
+- Handle `withProgress` cancellation gracefully - it throws when user cancels, so wrap in try/catch and return appropriate value
+
+### State Validation
+
+- Verify state after async setup operations - methods can return early without throwing, so check expected state was created
+- Validate cached state before early returns - before returning "already configured", verify the state is still valid (e.g., interpreter paths match, controllers aren't stale)
+
+### Cancellation Tokens
+
+- Use real cancellation tokens tied to lifecycle events instead of fake/never-cancelled tokens
+- Create `CancellationTokenSource` tied to relevant events (e.g., notebook close, cell cancel)

CLAUDE.md

@@ -0,0 +1 @@
+AGENTS.md

CLAUDE.md

@@ -1,5 +1,5 @@
-import { injectable, inject, optional } from 'inversify';
-import { workspace } from 'vscode';
+import { inject, injectable, optional } from 'inversify';
+import { commands, l10n, workspace, window, type Disposable, type NotebookDocumentContentOptions } from 'vscode';
 
 import { IExtensionSyncActivationService } from '../../platform/activation/types';
 import { IExtensionContext } from '../../platform/common/types';
@@ -17,13 +17,17 @@ import { SnapshotService } from './snapshots/snapshotService';
  */
 @injectable()
 export class DeepnoteActivationService implements IExtensionSyncActivationService {
+    private editProtection: DeepnoteInputBlockEditProtection;
+
     private explorerView: DeepnoteExplorerView;
 
     private integrationManager: IIntegrationManager;
 
     private serializer: DeepnoteNotebookSerializer;
 
-    private editProtection: DeepnoteInputBlockEditProtection;
+    private serializerRegistration?: Disposable;
+
+    private snapshotsEnabled = false;
 
     constructor(
         @inject(IExtensionContext) private extensionContext: IExtensionContext,
@@ -43,11 +47,87 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic
         this.serializer = new DeepnoteNotebookSerializer(this.notebookManager, this.snapshotService);
         this.explorerView = new DeepnoteExplorerView(this.extensionContext, this.notebookManager, this.logger);
         this.editProtection = new DeepnoteInputBlockEditProtection(this.logger);
+        this.snapshotsEnabled = this.isSnapshotsEnabled();
 
-        this.extensionContext.subscriptions.push(workspace.registerNotebookSerializer('deepnote', this.serializer));
+        this.registerSerializer();
         this.extensionContext.subscriptions.push(this.editProtection);
+        this.extensionContext.subscriptions.push(
+            workspace.onDidChangeConfiguration((event) => {
+                if (event.affectsConfiguration('deepnote.snapshots.enabled')) {
+                    const snapshotsEnabled = this.isSnapshotsEnabled();
+
+                    if (!this.snapshotsEnabled && snapshotsEnabled) {
+                        this.promptReloadForSnapshots();
+                    }
+
+                    this.snapshotsEnabled = snapshotsEnabled;
+                    this.registerSerializer();
+                }
+            })
+        );
 
         this.explorerView.activate();
         this.integrationManager.activate();
     }
+
+    private isSnapshotsEnabled(): boolean {
+        if (this.snapshotService) {
+            return this.snapshotService.isSnapshotsEnabled();
+        }
+
+        const config = workspace.getConfiguration('deepnote');
+
+        return config.get<boolean>('snapshots.enabled', false);
+    }
+
+    private promptReloadForSnapshots(): void {
+        const hasOpenDeepnoteNotebooks = workspace.notebookDocuments.some(
+            (notebook) => notebook.notebookType === 'deepnote'
+        );
+
+        if (!hasOpenDeepnoteNotebooks) {
+            void window.showInformationMessage(l10n.t('Snapshots enabled for this workspace.'));
+
+            return;
+        }
+
+        const reloadOption = l10n.t('Reload Window');
+        const laterOption = l10n.t('Later'); // Dismisses the dialog without action
+
+        void window
+            .showInformationMessage(
+                l10n.t('Snapshots enabled. Reload the window to apply to open notebooks.'),
+                reloadOption,
+                laterOption
+            )
+            .then((selection) => {
+                if (selection === reloadOption) {
+                    void commands.executeCommand('workbench.action.reloadWindow');
+                }
+                // "Later" or dialog dismissal: no action needed
+            });
+    }
+
+    private registerSerializer(): void {
+        // When snapshots are enabled, treat outputs as transient so VS Code
+        // doesn't mark the document dirty when outputs change during execution
+        const contentOptions: NotebookDocumentContentOptions = {};
+
+        if (this.isSnapshotsEnabled()) {
+            contentOptions.transientOutputs = true;
+        }
+
+        if (this.serializerRegistration) {
+            this.serializerRegistration.dispose();
+
+            const idx = this.extensionContext.subscriptions.indexOf(this.serializerRegistration);
+
+            if (idx >= 0) {
+                this.extensionContext.subscriptions.splice(idx, 1);
+            }
+        }
+
+        this.serializerRegistration = workspace.registerNotebookSerializer('deepnote', this.serializer, contentOptions);
+        this.extensionContext.subscriptions.push(this.serializerRegistration);
+    }
 }

src/notebooks/deepnote/deepnoteActivationService.ts

@@ -1,10 +1,12 @@
 import { assert } from 'chai';
+import { anything, verify, when } from 'ts-mockito';
 
 import { DeepnoteActivationService } from './deepnoteActivationService';
 import { DeepnoteNotebookManager } from './deepnoteNotebookManager';
 import { IExtensionContext } from '../../platform/common/types';
 import { ILogger } from '../../platform/logging/types';
 import { IIntegrationManager } from './integrations/types';
+import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock';
 
 function createMockLogger(): ILogger {
     return {
@@ -71,6 +73,97 @@ suite('DeepnoteActivationService', () => {
                 assert.isTrue(true, 'activate() method exists and attempts to initialize components');
             }
         });
+
+        test('should re-register serializer when snapshots are enabled in-session', () => {
+            resetVSCodeMocks();
+
+            const registrations: Array<{ transientOutputs?: boolean }> = [];
+            let configHandler: ((event: { affectsConfiguration: (section: string) => boolean }) => void) | undefined;
+
+            when(
+                mockedVSCodeNamespaces.workspace.registerNotebookSerializer(anything(), anything(), anything())
+            ).thenCall((_type, _serializer, options) => {
+                registrations.push(options);
+
+                return { dispose: () => undefined } as any;
+            });
+            const onDidChangeConfiguration = (
+                handler: (event: { affectsConfiguration: (section: string) => boolean }) => void
+            ) => {
+                configHandler = handler;
+
+                return { dispose: () => undefined } as any;
+            };
+            when(mockedVSCodeNamespaces.workspace.onDidChangeConfiguration).thenReturn(onDidChangeConfiguration as any);
+
+            let snapshotsEnabled = false;
+            const mockSnapshotService = { isSnapshotsEnabled: () => snapshotsEnabled } as any;
+            activationService = new DeepnoteActivationService(
+                mockExtensionContext,
+                manager,
+                mockIntegrationManager,
+                mockLogger,
+                mockSnapshotService
+            );
+
+            try {
+                activationService.activate();
+            } catch {
+                // Activation may fail in the test environment, but registrations should still occur.
+            }
+
+            assert.strictEqual(registrations.length, 1);
+            assert.isUndefined(registrations[0].transientOutputs);
+
+            snapshotsEnabled = true;
+            configHandler?.({ affectsConfiguration: (section) => section === 'deepnote.snapshots.enabled' });
+
+            assert.strictEqual(registrations.length, 2);
+            assert.strictEqual(registrations[1].transientOutputs, true);
+            verify(
+                mockedVSCodeNamespaces.workspace.registerNotebookSerializer(anything(), anything(), anything())
+            ).twice();
+        });
+
+        test('should prompt to reload when snapshots are enabled with open notebooks', () => {
+            resetVSCodeMocks();
+
+            let configHandler: ((event: { affectsConfiguration: (section: string) => boolean }) => void) | undefined;
+
+            when(
+                mockedVSCodeNamespaces.workspace.registerNotebookSerializer(anything(), anything(), anything())
+            ).thenReturn({ dispose: () => undefined } as any);
+            const onDidChangeConfiguration = (
+                handler: (event: { affectsConfiguration: (section: string) => boolean }) => void
+            ) => {
+                configHandler = handler;
+
+                return { dispose: () => undefined } as any;
+            };
+            when(mockedVSCodeNamespaces.workspace.onDidChangeConfiguration).thenReturn(onDidChangeConfiguration as any);
+            when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([{ notebookType: 'deepnote' } as any]);
+
+            let snapshotsEnabled = false;
+            const mockSnapshotService = { isSnapshotsEnabled: () => snapshotsEnabled } as any;
+            activationService = new DeepnoteActivationService(
+                mockExtensionContext,
+                manager,
+                mockIntegrationManager,
+                mockLogger,
+                mockSnapshotService
+            );
+
+            try {
+                activationService.activate();
+            } catch {
+                // Activation may fail in the test environment, but prompts should still be attempted.
+            }
+
+            snapshotsEnabled = true;
+            configHandler?.({ affectsConfiguration: (section) => section === 'deepnote.snapshots.enabled' });
+
+            verify(mockedVSCodeNamespaces.window.showInformationMessage(anything(), anything(), anything())).once();
+        });
     });
 
     suite('component initialization', () => {