Skip to content

Conversation

@jamesbhobbs
Copy link
Contributor

@jamesbhobbs jamesbhobbs commented Oct 20, 2025

Summary

Adds CSpell spell-checking configuration to vscode-deepnote repository, focusing only on Deepnote-specific code rather than the forked Microsoft vscode-jupyter codebase.

Changes Made

Spell-check configuration:

  • cspell.json: Configured to only check Deepnote-specific files:
    • src/notebooks/deepnote/**
    • src/kernels/deepnote/**
    • src/platform/errors/deepnoteServerNotFoundError.ts
  • Custom dictionary: 37 technical terms (jupyter, deepnote, ipykernel, vscode, etc.)
  • Ignore patterns: Standard build artifacts, node_modules, lockfiles

CI integration:

  • .github/workflows/ci.yml: Added spell-check job that runs npm run spell-check
  • Timeout: 15 minutes with GitHub npm registry authentication

Package updates:

  • Added dependency: cspell@9.2.1 as dev dependency
  • Added script: "spell-check": "cspell --config cspell.json"
  • License exclusion: Added @cspell/dict-en-common-misspellings@2.1.6 to license checker exclusions

Key Design Decisions

  1. Limited scope: Only checks Deepnote-authored code, excludes forked Microsoft vscode-jupyter code
  2. Minimal word list: 37 words vs. thousands to avoid over-permissive spell-checking
  3. License workaround: Excluded problematic dictionary package from license checks

Human Review Checklist

  • License exclusion safety: Verify excluding @cspell/dict-en-common-misspellings@2.1.6 from license checks is acceptable for compliance
  • Word list accuracy: Review the 37 custom words in cspell.json to ensure no actual misspellings were whitelisted instead of being fixed
  • File scope appropriateness: Confirm the limited file patterns appropriately target Deepnote-specific code
  • CI functionality: Test that the spell-check job runs successfully and catches spelling issues

Link to Devin run: https://app.devin.ai/sessions/f834d045e38442e88bff0556346e3a2b
Requested by: @jamesbhobbs

- Add cspell.json with Deepnote-specific file patterns
- Configure spell-check to only check Deepnote code (not forked Microsoft code)
- Add minimal 35-word technical dictionary
- Add spell-check CI job to GitHub Actions workflow
- Update package.json scripts to use cspell config

Result: 49 files checked (Deepnote-specific only), 0 spelling errors
@devin-ai-integration
Copy link

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 20, 2025

📝 Walkthrough

Walkthrough

Adds cspell-based spell-checking: introduces cspell.json config targeting specific source paths and ignore patterns; adds cspell dependency and a spell-check npm script in package.json; and updates CI (.github/workflows/ci.yml) with a new "Spell Check" job and an additional spell-check step in the existing "Check Licenses" workflow path.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer (push)
  participant GH as GitHub Actions
  participant Runner as CI Runner
  participant Node as Node/npm (cspell)

  Dev->>GH: push PR
  GH->>Runner: start CI
  alt existing checks
    Runner->>Runner: Checkout repo, Run "Check Licenses" job
    Runner->>Node: npm ci\nnpm run spell-check (new step added)
    Node-->>Runner: spell-check results
  end
  alt new standalone job
    Runner->>Runner: Run "Spell Check" job
    Runner->>Node: npm ci\nnpm run spell-check
    Node-->>Runner: spell-check results
  end
  Runner->>GH: report status (pass/fail)
Loading

Possibly related PRs

Suggested reviewers

  • saltenasl
  • Artmann
  • andyjakubowski

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "feat: add cspell configuration and CI spell-check" accurately maps to all three main components of the changeset: the new cspell.json configuration file, the GitHub Actions CI job in .github/workflows/ci.yml, and the updates to package.json (dependency and scripts). The title is specific and descriptive rather than vague, clearly communicating that this PR adds spell-checking infrastructure to the project. A teammate reviewing the history would immediately understand the purpose and scope of these changes.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

@codecov
Copy link

codecov bot commented Oct 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71%. Comparing base (3c6e419) to head (b5e3e46).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@          Coverage Diff          @@
##            main     #87   +/-   ##
=====================================
  Coverage     71%     71%           
=====================================
  Files        523     523           
  Lines      39001   39001           
  Branches    4905    4905           
=====================================
  Hits       27834   27834           
  Misses      9542    9542           
  Partials    1625    1625           
🚀 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: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7271532 and 476009e.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • .github/workflows/ci.yml (1 hunks)
  • cspell.json (1 hunks)
  • package.json (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: jamesbhobbs
PR: deepnote/deepnote#1
File: package.json:24-28
Timestamp: 2025-09-30T13:33:12.669Z
Learning: In the deepnote repository, do not suggest adding spell-check to lint-staged pre-commit hooks due to performance concerns. Spell-check runs in CI instead.
⏰ 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). (1)
  • GitHub Check: Build & Test
🔇 Additional comments (3)
cspell.json (3)

3-7: Scoping looks correct. File patterns target Deepnote-specific directories and avoid the forked Jupyter extension code. ✓


30-31: Validate "dont" and "DONT" — likely misspellings of "don't" / "DON'T".

Both appear in the custom words list but look like typos rather than intentional terms. Verify they exist in your codebase before allowlisting them.


23-59: Verification passed—all spot-checked terms are legitimate code identifiers.

The checked terms ("Reselecting", "DONT"/"dont") appear as actual identifiers and comments in the codebase, not misspelled words masked by the allowlist. Combined with the known technical terms (jupyter, ipykernel, numpy, venv), the custom words list is appropriate.

- Remove spell-check:fix script (didn't actually fix, just showed suggestions)
- Add explicit --config cspell.json to spell-check script for clarity
@jamesbhobbs jamesbhobbs marked this pull request as ready for review October 20, 2025 16:08
@jamesbhobbs jamesbhobbs merged commit 9ad31d9 into main Oct 20, 2025
10 of 11 checks passed
@jamesbhobbs jamesbhobbs deleted the devin/1760974113-clean-spell-check-config branch October 20, 2025 16:12
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.

2 participants