-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add cspell configuration and CI spell-check #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
|
📝 WalkthroughWalkthroughAdds a cspell.json (v0.2) providing ignorePaths, English language, useGitignore: true, a global words list of domain/project terms, and Markdown-specific overrides. Adds a GitHub Actions workflow at .github/workflows/ci.yaml that triggers on pushes to main and PRs, uses concurrency, checks out the repo, runs jupyterlab/maintainer-tools base setup, installs JupyterLab, runs jlpm install (with GITHUB_TOKEN), and runs cspell over common file patterns to report spelling issues. Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub
participant Runner as Actions Runner
participant Repo as Repository
participant Tools as Maintainer Tools
participant JL as JupyterLab Env
participant JLPM as jlpm
participant CSpell as cspell
Dev->>GH: Push to main / Open PR
GH-->>Runner: Trigger CI (spell-check)
Runner->>Repo: actions/checkout
Runner->>Tools: Run maintainer-tools base setup
Runner->>JL: Install JupyterLab
Runner->>JLPM: jlpm install (uses GITHUB_TOKEN)
Runner->>CSpell: Run cspell with cspell.json across repo files
CSpell-->>Runner: Return findings (errors/warnings)
Runner-->>GH: Report job status
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
Comment |
There was a problem hiding this 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build.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.
…jlpm The previous commit attempted to add a 'Install dependencies' step using jlpm, but jlpm is not available after Base Setup alone. The correct fix requires updating yarn.lock locally with the cspell dependency, which is blocked by missing GITHUB_TOKEN in the development environment. This commit reverts the incorrect build.yml changes. The spell-check job still needs yarn.lock to be updated with the cspell@9.2.1 dependency.
9ac36f7
d61f6b4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml(1 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.
🔇 Additional comments (1)
.github/workflows/ci.yaml (1)
13-27: CI‑only spell‑check approach looks goodRunning CSpell in CI (not pre‑commit) aligns with performance guidance.
Based on learnings
The spell-check job was failing with 'jlpm: command not found' because the Base Setup action provides the jlpm binary but doesn't install node_modules. This adds the missing install step with GITHUB_TOKEN.
There was a problem hiding this 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
♻️ Duplicate comments (4)
.github/workflows/ci.yaml (4)
3-8: Add workflow_dispatch for manual runs
Enable ad‑hoc runs from Actions UI.on: push: branches: main pull_request: branches: '*' + workflow_dispatch: {}
14-17: Harden job: least‑privilege token + timeout
Add explicit permissions and a timeout.spell-check: name: Spell Check + permissions: + contents: read + timeout-minutes: 10 runs-on: ubuntu-latest
19-19: Pin GitHub Actions to immutable SHAs (supply‑chain)
Replace floating tags with commit SHAs.- uses: actions/checkout@v4 + uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4 - - name: Base Setup - uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1 + - name: Base Setup + uses: jupyterlab/maintainer-tools/.github/actions/base-setup@0340588c71254f7cdf872941134910deeb292875 # v1Also applies to: 22-22
29-32: Remove unnecessary GITHUB_TOKEN from spell check step
CSpell doesn’t need it; reduce secret exposure.- name: Run spell check run: jlpm run spell-check - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml(1 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.
The jlpm command without arguments automatically runs yarn install. This matches the pattern used in build.yml and fixes the 'jlpm: command not found' error.
The Base Setup action provides Python and yarn but not jlpm. Installing jupyterlab via pip provides the jlpm command, following the pattern used in build.yml.
There was a problem hiding this 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
♻️ Duplicate comments (5)
.github/workflows/ci.yaml (5)
3-8: Add manual trigger via workflow_dispatch
Handy for ad-hoc runs.on: push: branches: main pull_request: branches: '*' + workflow_dispatch: {}
14-17: Lock down job permissions and add timeout
Least privilege and guard against hung jobs.spell-check: name: Spell Check + permissions: + contents: read + timeout-minutes: 10 runs-on: ubuntu-latest
27-31: Make dependency install reproducible; drop token
Enforce lockfile and avoid passing secrets unless needed.- - name: Install dependencies - run: jlpm - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Install dependencies + run: jlpm install --immutableIf using Yarn Classic, use: jlpm install --frozen-lockfile
32-35: Drop GITHUB_TOKEN from spell-check step
CSpell doesn’t use it.- name: Run spell check run: jlpm run spell-check - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
19-19: Pin GitHub Actions to immutable SHAs (supply‑chain hardening)
Avoid mutable tags.- uses: actions/checkout@v4 + uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4 - - name: Base Setup - uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1 + - name: Base Setup + uses: jupyterlab/maintainer-tools/.github/actions/base-setup@0340588c71254f7cdf872941134910deeb292875 # v1Also applies to: 22-22
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml(1 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.
🔇 Additional comments (1)
.github/workflows/ci.yaml (1)
9-12: Good use of concurrency cancellationPrevents redundant runs on updates to the same ref.
- Avoids yarn.lock sync issue that causes CI failure - npx downloads cspell on-the-fly without requiring it in package.json - Fixes 'YN0028: The lockfile would have been modified' error
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11 +/- ##
=======================================
Coverage 18.50% 18.50%
=======================================
Files 13 13
Lines 200 200
Branches 27 27
=======================================
Hits 37 37
Misses 163 163 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add 22 technical terms found in spell check CI logs: - JupyterLab packages: apputils, coreutils, settingregistry, lumino, testutils - Notebook formats: nbformat, ipynb - Build tools: jlpm, pyproject, sdist, bdist, lintcache, stylelintcache, csstree - Test tools: pytest, Pytest, codegen - Library names: chakra, ydoc - Config: jpserver, venv, labextensions - Author names: Jakubowski, rseidelsohn
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Ensures dependencies are installed exactly as specified in yarn.lock without modifications during CI runs.
The spell check uses npx cspell which doesn't require project dependencies to be installed. The jlpm install step was causing authentication failures when trying to fetch @deepnote/blocks.
Add CSpell configuration and CI spell checking
Summary
Adds consistent spell checking to jupyterlab-deepnote by implementing CSpell configuration and CI automation. This introduces a new required CI check that validates spelling across markdown, code, and configuration files using a curated dictionary of technical terms.
Key changes:
.github/workflows/ci.yamlwith spell check job that runs on PRs and main branchcspell.jsonconfiguration with JupyterLab-specific technical vocabulary (64 custom words)**/*.{md,txt,json,js,ts,tsx,jsx,yml,yaml}Review & Testing Checklist for Human
This is a medium risk change that adds new CI infrastructure. Please verify:
npx cspell@9.2.1 "**/*.{md,txt,json,js,ts,tsx,jsx,yml,yaml}"in the repo root to confirm it works as expected and doesn't flag legitimate termscspell.jsonare legitimate technical terms (e.g., "jupyterlab", "labextension", "settingregistry") and not typos being incorrectly allowed"**/*.{md,txt,json,js,ts,tsx,jsx,yml,yaml}"catches appropriate files without false positives in build/vendor directoriesNotes
Link to Devin run: https://app.devin.ai/sessions/f834d045e38442e88bff0556346e3a2b
Requested by: @jamesbhobbs