diff --git a/.cursor/rules/agents.mdc b/.cursor/rules/agents.mdc new file mode 100644 index 000000000..34381d2aa --- /dev/null +++ b/.cursor/rules/agents.mdc @@ -0,0 +1,6 @@ +--- +description: Repository routing, workflow, and code style guide for the Deepnote extension +alwaysApply: true +--- + +@AGENTS.md diff --git a/AGENTS.md b/AGENTS.md index 292cfb760..cc51afda7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,84 +1,180 @@ -# Code Style & Organization +# Agent Development Guide -- Order method, fields and properties, first by accessibility and then by alphabetical order. +This document provides guidelines for AI coding agents working on the Deepnote extension for VS Code, Cursor, Windsurf, and Antigravity. + +## Repository Overview + +This repository is a fork of Microsoft's `vscode-jupyter` extension. Most of `src/` is inherited upstream Jupyter/notebook code that should change rarely and deliberately. Deepnote-specific work is concentrated in four directories: + +- `src/notebooks/deepnote/` - `.deepnote` file parsing/serialization, block converters, the sidebar explorer, environment snapshots, and integrations wiring (the bulk of Deepnote-specific code) +- `src/kernels/deepnote/` - kernel auto-selection, the SQL language server client, and server startup for Deepnote projects +- `src/platform/deepnote/` and `src/platform/notebooks/deepnote/` - shared Deepnote types, telemetry, and integration config used across the platform layer +- `src/webviews/webview-side/integrations/` - the React UI for the database integrations panel + +If a file isn't under one of these paths, assume it's inherited upstream code — read it for context, but don't restructure it as a side effect of a Deepnote change. + +### Repository Routing + +Start with the owning directory and its colocated tests before searching broadly. Avoid traversing unrelated inherited Jupyter code. + +| When working on | Start with | +| -------------------------------------------------------- | ------------------------------------------------------------------------------------------ | +| `.deepnote` parsing, serialization, block conversion | `src/notebooks/deepnote/deepnoteSerializer.ts`, `src/notebooks/deepnote/converters/` | +| Explorer sidebar / tree view | `src/notebooks/deepnote/deepnoteExplorerView.ts`, `src/notebooks/deepnote/deepnoteTreeDataProvider.ts` | +| Environment snapshots & `requirements.txt` generation | `src/notebooks/deepnote/snapshots/` | +| Database integrations (credentials, env refresh, webview) | `src/notebooks/deepnote/integrations/`, `src/webviews/webview-side/integrations/` | +| Kernel selection, SQL LSP, server startup | `src/kernels/deepnote/` | +| Shared Deepnote types & platform config | `src/platform/deepnote/`, `src/platform/notebooks/deepnote/` | +| Core serializer architecture & data flow | `specs/architecture.md` | +| Kernel management internals | `specs/DEBUGGING_KERNEL_MANAGEMENT.md`, `specs/DEEPNOTE_KERNEL_IMPLEMENTATION.md` | +| Integration credentials & live env refresh | `specs/INTEGRATIONS_CREDENTIALS.md`, `specs/INTEGRATION_ENV_LIVE_REFRESH.md` | +| SQL language server behavior | `specs/LSP.md` | +| End-to-end tests | `test/e2e/` | + +## Development Workflow + +Always run commands from the repository root, using the Node version pinned in `.nvmrc`. + +### Setup + +```bash +npm install +``` + +`postinstall` downloads the VS Code API typings and runs `build/ci/postInstall.js`. + +### Testing + +```bash +# Unit tests run against compiled JS, so build first +npm run compile-tsc +npm test # same as npm run test:unittests + +# Filter by suite/test name +npm run test:unittests -- --grep "SuiteName" + +# Run a single compiled test file +npx mocha --config ./build/.mocha.unittests.js.json ./out/path/to/file.unit.test.js + +# End-to-end tests (extest-driven VS Code + chromedriver) +npm run setup:e2e # one-time: fetches VS Code, chromedriver, mock LLM server +npm run test:e2e +``` + +- Unit tests use Mocha/Chai with the `.unit.test.ts` extension, colocated with the source they test. +- Use `assert.deepStrictEqual()` for object comparisons instead of checking individual properties. + +### Type Checking + +```bash +npm run typecheck +``` + +### Linting, Formatting & Spelling + +```bash +npm run lint # ESLint check +npm run lint-fix # ESLint autofix +npm run format # Prettier check +npm run format-fix # Prettier write +npm run spell-check # cspell +``` + +### Building + +```bash +npm run compile # tsc + esbuild, dev +npm run build # production bundle +``` + +## Code Quality Standards + +### After making changes + +Always run `npm run format-fix`. + +### Before committing + +1. **Tests** - `npm test` - all tests must pass +2. **Type check** - `npm run typecheck` - no TypeScript errors +3. **Lint** - `npm run lint` - must pass ESLint +4. **Format** - `npm run format` - must pass Prettier + +The `pre-commit` hook already runs `lint-staged` (ESLint + Prettier on staged files) and `pre-push` blocks direct pushes to `main` — running the full checks yourself catches issues earlier. + +### TypeScript & Code Style + +- Order methods, fields, and properties first by accessibility, then alphabetically. - 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 `npm run format-fix` after making changes to the code - -## Testing - -- Unit tests use Mocha/Chai framework with `.unit.test.ts` extension -- Test files should be placed alongside the source files they test -- Tests run against compiled JavaScript files in `out/` directory — build first with `npm run compile-tsc` -- Run all tests: `npm test` or `npm run test:unittests` -- Run tests matching a pattern: `npm run test:unittests -- --grep "SuiteName"` (forwards `--grep` to mocha, matches suite and test names) -- Run single test file: `npx mocha --config ./build/.mocha.unittests.js.json ./out/path/to/file.unit.test.js` -- 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 +- Use `Uri.joinPath()` for file paths instead of string concatenation, so path separators stay platform-correct. +- Reuse existing helpers instead of importing packages directly (e.g. `generateUuid` from `platform/common/uuid` instead of the `uuid` package). +- User-facing strings must go through constants in `src/platform/common/utils/localize.ts`, not inline literals. +- Separate third-party imports from local imports; add a blank line after const groups and before return statements. + +## Deepnote-Specific Invariants + +These aren't derivable from reading a single file — they're constraints that span the serializer and its callers: + +- Snapshot mode must not persist execution-time metadata (e.g. `contentHash`); doing so causes false-dirty state. Track it in memory instead. +- `DeepnoteNotebookSerializer.detectContentChanges` must compare notebook-level fields (`name`, `executionMode`, `isModule`, `workingDirectory`) and detect removed notebooks, not just block-level diffs. ## Best Practices ### Resource Cleanup -- Always dispose `CancellationTokenSource` - never create inline without storing/disposing -- Use try/finally to ensure cleanup: +Always dispose `CancellationTokenSource` - never create one inline without storing/disposing it. Use try/finally: - ```typescript - const cts = new CancellationTokenSource(); - try { - await fn(cts.token); - } finally { - cts.dispose(); - } - ``` +```typescript +const cts = new CancellationTokenSource(); +try { + await fn(cts.token); +} finally { + cts.dispose(); +} +``` +Use real cancellation tokens tied to lifecycle events (e.g. notebook close, cell cancel) instead of fake/never-cancelled tokens. -### DRY Principle +### DRY -- 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 +Extract duplicate logic into helper methods to prevent drift — e.g. when similar setup logic (placeholder controllers, interpreter validation) appears in multiple places, consolidate it. ### Magic Numbers -- Extract magic numbers (retry counts, delays, timeouts) as named constants near the top of the module +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 +- 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 the user cancels, so wrap in try/catch and return an 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) +- Verify state after async setup - methods can return early without throwing, so check that the expected state was actually created. +- Validate cached state before early returns - before returning "already configured," confirm the cached state is still valid (e.g. interpreter paths match, controllers aren't stale). + +## Common Tasks + +### Adding Tests + +1. Create a `.unit.test.ts` file next to the source file. +2. Group related tests with Mocha `describe()`/`it()`. +3. Build and run: `npm run compile-tsc && npm test`. + +### Fixing Lint or Format Issues + +1. `npm run lint` / `npm run format` to see issues. +2. `npm run lint-fix` / `npm run format-fix` to autofix most of them. +3. Fix the rest manually, following the linter's suggestions. + +### Fixing Type Errors + +1. `npm run typecheck` to see all errors. +2. Add proper type annotations, use type guards for conditional access, and ensure function signatures match implementations. -### Cancellation Tokens +## File Structure Conventions -- 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) +- Source code: `src/` +- Tests: colocated with source as `*.unit.test.ts` (unit), `*.vscode.test*.ts` under `src/test/` (integration), or under `test/e2e/` (end-to-end) +- Reference docs for agents and contributors: `specs/` +- Build output: `out/` (compiled TS) and `dist/`/bundled output from esbuild (gitignored) diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 120000 index 47dc3e3d8..000000000 --- a/CLAUDE.md +++ /dev/null @@ -1 +0,0 @@ -AGENTS.md \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..43c994c2d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ff08bd661..f798fd674 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -227,7 +227,7 @@ Note: Integration tests are those in files with extension `*.vscode.test*.ts`. You can also run the tests from the command-line (after compiling): ```shell -npm run testVSCode # will launch the VSC UI +npm run test:integration # will launch the VSC UI ``` #### Customising the test run @@ -290,7 +290,7 @@ on your system, however most systems support a syntax like the following for setting a single variable for a subprocess: ```shell -VSC_JUPYTER_CI_TEST_GREP=Sorting npm run testVSCode +VSC_JUPYTER_CI_TEST_GREP=Sorting npm run test:integration ``` ### Testing Python scripts