Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ jobs:
permissions:
id-token: write
contents: read
packages: read
steps:
- name: Checkout
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
Expand Down Expand Up @@ -167,6 +166,30 @@ jobs:
- name: Check Licenses
run: npm run check-licenses

spell-check:
name: Spell Check
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
- name: Checkout
uses: actions/checkout@v5

- name: Setup Node.js
uses: actions/setup-node@v5
with:
cache: 'npm'
node-version: ${{ env.NODE_VERSION }}
registry-url: 'https://npm.pkg.github.com'
scope: '@deepnote'

- name: Install dependencies
run: npm ci --prefer-offline --no-audit
Comment on lines +175 to +186
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Re-pin GitHub Actions to SHAs.

Every other job pins actions/checkout and actions/setup-node to vetted SHAs; this job drops back to floating @v5, undermining the supply-chain hardening we already enforce. Please use the same commit SHAs as the rest of the workflow.

-      - name: Checkout
-        uses: actions/checkout@v5
+      - name: Checkout
+        uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
@@
-      - name: Setup Node.js
-        uses: actions/setup-node@v5
+      - name: Setup Node.js
+        uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
.github/workflows/ci.yml lines 175-186: the job uses floating action tags
actions/checkout@v5 and actions/setup-node@v5 which weakens the repository's
supply-chain hardening; replace both with the exact commit SHAs used elsewhere
in this workflow (or the vetted SHAs used in other jobs) so they are pinned to
specific commits, i.e. locate the SHA entries for actions/checkout and
actions/setup-node in the other jobs and substitute @v5 with the corresponding
@<full-commit-sha> for each action.

env:
NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Comment on lines +175 to +188
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Pin actions to commit SHAs.
Other jobs lock actions/checkout and actions/setup-node to specific SHAs; this job floats on the v5 tag, reopening a supply-chain risk. Please pin both actions to the same commits used elsewhere in the workflow.

-      - name: Checkout
-        uses: actions/checkout@v5
+      - name: Checkout
+        uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
@@
-      - name: Setup Node.js
-        uses: actions/setup-node@v5
+      - name: Setup Node.js
+        uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
.github/workflows/ci.yml lines 175-188: the workflow uses floating tags for
actions/checkout@v5 and actions/setup-node@v5; replace both with the exact
commit SHAs used by the other jobs in this workflow (match the same commit SHAs
already pinned elsewhere) to eliminate the supply-chain risk — update the two
"uses:" lines to reference the exact SHA commits, run a quick lint/test of the
workflow, and commit the change.


- name: Run spell check
run: npm run spell-check

audit-prod:
name: Audit - Production
runs-on: ubuntu-latest
Expand Down
111 changes: 55 additions & 56 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,75 +1,74 @@
__pycache__
!build/
!src/test/pythonEnvironments/**/*.*
!yarn.lock
.DS_Store
.eslintcache
.github_token
.huskyrc.json
.nyc_output
.pytest_cache
.python-version
.qlty/cache
.qlty/logs
.qlty/out
.qlty/plugin_cachedir
.qlty/results
.vs/
.vscode-test
.vscode-test-web
dist
out
*.exe
*.i18n.json
*.nls.*.json
*.noseids
log.log
**/node_modules
*.pyc
*.vsix
*.xlf
**/.mypy_cache/**
**/.venv*/
**/.vscode test/**
**/.vscode-smoke/**
**/.vscode-test/**
**/.vscode/.ropeproject/**
**/*.esbuild.meta.json
**/node_modules
**/testFiles/**/.cache/**
# Compilation of less to css
# Esbuild files
# Qlty cache directories
# translation files
bin/**
*.noseids
.nyc_output
.vscode-test
__pycache__
npm-debug.log
**/.mypy_cache/**
!yarn.lock
coverage/
cucumber-report.json
**/.vscode-test/**
**/.vscode test/**
**/.vscode-smoke/**
**/.venv*/
port.txt
precommit.hook
pythonFiles/lib/**
debug_coverage*/**
debug*.log
debugpy*.log
dist
l10n/
languageServer.*/**
languageServer/**
log.log
languageServer.*/**
bin/**
logs/**
nodeLanguageServer.*/**
nodeLanguageServer/**
npm-debug.log
obj/**
out
port.txt
precommit.hook
.pytest_cache
temp
tmp
src/test/datascience/tmp/**
src/test/datascience/temp/**
.python-version
.vs/
test-results*.xml
testresults.json
telemetry.json
xunit-test-results.xml
!build/
debug*.log
debugpy*.log
pydevd*.log
pythonFiles/lib/**
vscode.d.ts
vscode.proposed.*.d.ts
nodeLanguageServer/**
nodeLanguageServer.*/**
src/test/datascience/.venv*
src/test/datascience/temp/**
src/test/datascience/tmp/**
!src/test/pythonEnvironments/**/*.*
.vscode-test-web
.github_token
# translation files
*.xlf
*.nls.*.json
*.i18n.json
l10n/
# Esbuild files
**/*.esbuild.meta.json
# Compilation of less to css
src/webviews/webview-side/interactive-common/variableExplorerGrid.css
src/webviews/webview-side/interactive-common/variableExplorerGrid.css.map
src/webviews/webview-side/react-common/seti/seti.css
src/webviews/webview-side/react-common/seti/seti.css.map
telemetry.json
temp
test-results*.xml
testresults.json
tmp
vscode.d.ts
vscode.proposed.*.d.ts
xunit-test-results.xml
# Qlty cache directories
.qlty/cache
.qlty/logs
.qlty/out
.qlty/plugin_cachedir
.qlty/results
1 change: 0 additions & 1 deletion .husky/post-checkout

This file was deleted.

18 changes: 17 additions & 1 deletion .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1 +1,17 @@
npm exec lint-staged
#!/bin/sh
if npm list prettier | grep -q prettier@ ; then
echo husky - prettier is installed
else
echo husky - prettier is not installed, skipping format check
exit 0
fi

# run prettier on all .ts files that are in the commit
changed=$(git diff --diff-filter=ACM --cached --name-only | grep .ts$ | tr '\n' ' ')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Strengthen the file pattern.

grep .ts$ matches .ts anywhere in the path (e.g., foo.ts.backup). Use grep '\.ts$' or grep -E '\.ts$' for an anchored extension match.

-changed=$(git diff --diff-filter=ACM --cached --name-only | grep .ts$ | tr '\n' ' ')
+changed=$(git diff --diff-filter=ACM --cached --name-only | grep '\.ts$' | tr '\n' ' ')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
changed=$(git diff --diff-filter=ACM --cached --name-only | grep .ts$ | tr '\n' ' ')
changed=$(git diff --diff-filter=ACM --cached --name-only | grep '\.ts$' | tr '\n' ' ')
🤖 Prompt for AI Agents
In .husky/pre-commit around line 10, the grep pattern grep .ts$ matches anywhere
in filenames (e.g., foo.ts.backup); replace it with an anchored, escaped
extension match such as grep '\.ts$' or grep -E '\.ts$' so only files ending
with .ts are selected (ensure the backslash is quoted so the shell passes the
pattern correctly).


if [ -z "$changed" ]; then
echo husky - No modified files to check
exit 0
fi

npx prettier $changed --check
Comment on lines +2 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Pre-commit hook checks but doesn't fix formatting.

Learnings indicate "ALWAYS run npm run format-fix before committing changes," but this hook runs prettier --check (line 17), which fails the commit without auto-fixing. Either switch to prettier --write or add guidance that developers must manually run format-fix.

Based on learnings


🏁 Script executed:

#!/bin/bash
# Check actual .husky/pre-commit file
echo "=== .husky/pre-commit ===" 
cat .husky/pre-commit 2>/dev/null || echo "File not found"

# Check npm scripts for format-fix and related commands
echo -e "\n=== package.json scripts (format-related) ===" 
jq '.scripts | with_entries(select(.key | contains("format") or contains("prettier")))' package.json 2>/dev/null || echo "package.json not found or jq failed"

# List .husky directory contents
echo -e "\n=== .husky directory ===" 
ls -la .husky/ 2>/dev/null || echo ".husky directory not found"

Length of output: 1030


Pre-commit hook enforces but doesn't fix formatting.

The hook runs prettier --check without --write, failing commits with formatting issues instead of auto-fixing them. Learnings indicate developers should always run format-fix before committing. Either switch to prettier --write or add npm run format-fix as a prior step in this hook.

🤖 Prompt for AI Agents
In .husky/pre-commit around lines 2 to 17, the hook currently runs Prettier with
--check which rejects commits instead of auto-fixing formatting; change the hook
to run Prettier in write mode or invoke the repository's format-fix script
before checking. Update the sequence to first run "npm run format-fix" (or "npx
prettier --write" on the staged .ts files), then re-stage any changed files and
finally run "npx prettier --check" to verify formatting so commits get
auto-corrected rather than failing.

5 changes: 0 additions & 5 deletions .husky/pre-push

This file was deleted.

58 changes: 2 additions & 56 deletions build/esbuild/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ import { lessLoader } from 'esbuild-plugin-less';
import fs from 'fs-extra';
import { getZeroMQPreBuildsFoldersToKeep, getBundleConfiguration, bundleConfiguration } from '../webpack/common';
import ImportGlobPlugin from 'esbuild-plugin-import-glob';
import postcss from 'postcss';
import tailwindcss from '@tailwindcss/postcss';
import autoprefixer from 'autoprefixer';
const plugin = require('node-stdlib-browser/helpers/esbuild/plugin');
const stdLibBrowser = require('node-stdlib-browser');

Expand Down Expand Up @@ -48,8 +45,6 @@ const commonExternals = [
'vscode',
'commonjs',
'node:crypto',
'node:fs/promises',
'node:path',
'vscode-jsonrpc', // Used by a few modules, might as well pull this out, instead of duplicating it in separate bundles.
// Ignore telemetry specific packages that are not required.
'applicationinsights-native-metrics',
Expand Down Expand Up @@ -91,11 +86,7 @@ const loader: { [ext: string]: Loader } = {

// https://github.com/evanw/esbuild/issues/20#issuecomment-802269745
// https://github.com/hyrious/esbuild-plugin-style
function style({
minify = true,
charset = 'utf8',
enableTailwind = false
}: StylePluginOptions & { enableTailwind?: boolean } = {}): Plugin {
function style({ minify = true, charset = 'utf8' }: StylePluginOptions = {}): Plugin {
return {
name: 'style',
setup({ onResolve, onLoad }) {
Expand Down Expand Up @@ -141,32 +132,6 @@ function style({
}));

onLoad({ filter: /.*/, namespace: 'style-content' }, async (args) => {
// Process with PostCSS/Tailwind if enabled and file exists
if (enableTailwind && args.path.includes('tailwind.css') && fs.existsSync(args.path)) {
try {
const cssContent = await fs.readFile(args.path, 'utf8');
const result = await postcss([tailwindcss, autoprefixer]).process(cssContent, {
from: args.path,
to: args.path
});

const options = { ...opt, stdin: { contents: result.css, loader: 'css' } };
options.loader = options.loader || {};
// Add the same loaders we add for other places
Object.keys(loader).forEach((key) => {
if (options.loader && !options.loader[key]) {
options.loader[key] = loader[key];
}
});
const { errors, warnings, outputFiles } = await esbuild.build(options);
return { errors, warnings, contents: outputFiles![0].text, loader: 'text' };
} catch (error) {
console.error(`PostCSS processing failed for ${args.path}:`, error);
throw error;
}
}

// Default behavior for other CSS files
const options = { entryPoints: [args.path], ...opt };
options.loader = options.loader || {};
// Add the same loaders we add for other places
Expand All @@ -175,9 +140,7 @@ function style({
options.loader[key] = loader[key];
}
});

const { errors, warnings, outputFiles } = await esbuild.build(options);

return { errors, warnings, contents: outputFiles![0].text, loader: 'text' };
});
}
Expand All @@ -195,9 +158,7 @@ function createConfig(
const plugins: Plugin[] = [];
let define: SameShape<BuildOptions, BuildOptions>['define'] = undefined;
if (target === 'web') {
// Enable Tailwind processing for dataframe renderer
const enableTailwind = source.includes(path.join('dataframe-renderer', 'index.ts'));
plugins.push(style({ enableTailwind }));
plugins.push(style());
plugins.push(lessLoader());

Comment on lines +161 to 163
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Align CSS minify with dev/prod.

style() always minifies; degrade DX in dev. Pass minify based on isDevbuild.

-        plugins.push(style());
+        plugins.push(style({ minify: !isDevbuild }));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
plugins.push(style());
plugins.push(lessLoader());
plugins.push(style({ minify: !isDevbuild }));
plugins.push(lessLoader());
🤖 Prompt for AI Agents
In build/esbuild/build.ts around lines 161 to 163, the call
plugins.push(style()) always enables CSS minification which hurts DX in
development; update the call to pass a minify option tied to the build mode
(e.g., plugins.push(style({ minify: !isDevbuild }))) so minification is disabled
when isDevbuild is true and enabled for production builds; leave lessLoader()
unchanged.

define = {
Expand Down Expand Up @@ -326,16 +287,6 @@ async function buildAll() {
),
{ target: 'web', watch: watchAll }
),
build(
path.join(extensionFolder, 'src', 'webviews', 'webview-side', 'dataframe-renderer', 'index.ts'),
path.join(extensionFolder, 'dist', 'webviews', 'webview-side', 'dataframeRenderer', 'dataframeRenderer.js'),
{ target: 'web', watch: isWatchMode }
),
build(
path.join(extensionFolder, 'src', 'webviews', 'webview-side', 'vega-renderer', 'index.ts'),
path.join(extensionFolder, 'dist', 'webviews', 'webview-side', 'vegaRenderer', 'vegaRenderer.js'),
{ target: 'web', watch: isWatchMode }
),
build(
path.join(extensionFolder, 'src', 'webviews', 'webview-side', 'variable-view', 'index.tsx'),
path.join(extensionFolder, 'dist', 'webviews', 'webview-side', 'viewers', 'variableView.js'),
Expand All @@ -350,11 +301,6 @@ async function buildAll() {
path.join(extensionFolder, 'src', 'webviews', 'webview-side', 'data-explorer', 'index.tsx'),
path.join(extensionFolder, 'dist', 'webviews', 'webview-side', 'viewers', 'dataExplorer.js'),
{ target: 'web', watch: watchAll }
),
build(
path.join(extensionFolder, 'src', 'webviews', 'webview-side', 'integrations', 'index.tsx'),
path.join(extensionFolder, 'dist', 'webviews', 'webview-side', 'integrations', 'index.js'),
{ target: 'web', watch: watchAll }
)
);

Expand Down
54 changes: 54 additions & 0 deletions cspell.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
{
"version": "0.2",
"files": [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls check if we should add to here when reviewing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add src/webviews/** and package.json

"src/notebooks/deepnote/**",
"src/kernels/deepnote/**",
"src/platform/errors/deepnoteServerNotFoundError.ts"
],
"ignorePaths": [
"node_modules",
"package-lock.json",
"pnpm-lock.yaml",
"dist",
"out",
"build",
"coverage",
".vscode",
".git",
"*.min.js",
"*.min.css",
"*.map"
],
"language": "en",
"words": [
"dataframe",
"deepnote",
"deepnoteserver",
"dont",
"DONT",
"ename",
"evalue",
"findstr",
"getsitepackages",
"IMAGENAME",
"ipykernel",
"ipynb",
"jupyter",
"jupyterlab",
"JVSC",
"millis",
"numpy",
"pids",
"Pids",
"PYTHONHOME",
"Reselecting",
"taskkill",
"unittests",
"venv",
"venv's",
"Venv",
"venvs",
"vscode"
],
"useGitignore": true
}
Loading
Loading