Skip to content

Conversation

@jamesbhobbs
Copy link
Contributor

@jamesbhobbs jamesbhobbs commented Oct 9, 2025

Summary

Adds qlty code quality and security scanning configuration to vscode-deepnote, mirroring the setup from deepnote/deepnote.

Changes

  • New qlty configuration (.qlty/qlty.toml): Configures actionlint, trufflehog, and osv-scanner plugins with code smell detection thresholds
  • CI workflow: Adds new qlty job to run code quality checks and smell analysis (3min timeout)
  • Pre-commit integration: Updates .husky/pre-commit to run qlty checks alongside prettier
  • Gitignore: Excludes qlty cache directories from version control

Security & Quality Checks Added

  • actionlint: GitHub Actions workflow linting
  • trufflehog: Secret scanning
  • osv-scanner: Vulnerability scanning
  • Code smells: Detects boolean logic complexity, nested control flow, function length, cognitive complexity, etc.

Human Review Checklist

🔍 Critical items to verify:

  • qlty CI job runs successfully without errors
  • Pre-commit hook works locally without breaking developer workflow
  • Code smell thresholds are appropriate for this VS Code extension codebase
  • Security scanning tools (trufflehog, osv-scanner) work correctly and don't generate false positives
  • 3-minute timeout is sufficient for qlty checks on this repository

⚠️ Potential risks:

  • Developers may need to install qlty locally for pre-commit hooks to work
  • Configuration is copied from main deepnote repo which has different tech stack
  • Action versions are pinned to specific commits - verify they're current

Link to Devin run: https://app.devin.ai/sessions/7df2a76e10f2447faf46c3c41fd5cc8c
Requested by: James Hobbs (@jamesbhobbs)

Summary by CodeRabbit

  • Chores
    • Added a CI job to run automated quality and security checks alongside existing lint/build for earlier PR feedback.
    • Introduced project-wide code-quality configuration with rules, thresholds, and common exclusions.
    • Expanded ignore rules to avoid committing tool caches, logs, and scan results.
    • Enhanced pre-commit hook to run quality checks after formatting validation, ensuring only formatted code is analyzed.

@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 9, 2025

📝 Walkthrough

Walkthrough

Adds a new “Qlty Check” job to .github/workflows/ci.yml running on ubuntu-latest with a 3-minute timeout and steps: checkout, install qlty action (versioned), qlty check, and qlty smells. Introduces .qlty/qlty.toml configuring plugins (actionlint, trufflehog, osv-scanner), source, exclusions, and multiple smell thresholds. Updates .husky/pre-commit to run qlty check on changed TypeScript files only if the Prettier diff check passes; initial empty-change handling unchanged. Extends .gitignore to exclude .qlty cache/output directories. No exported/public API changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant GH as GitHub Actions
  participant R as Runner (ubuntu-latest)
  participant Repo as Repo
  participant Q as qlty Action

  GH->>R: Trigger workflow (Qlty Check job)
  R->>Repo: actions/checkout
  R->>Q: Install qlty action (versioned)
  R->>Q: qlty check
  R->>Q: qlty smells
  Q-->>R: Reports results (pass/fail)
  R-->>GH: Job status
Loading
sequenceDiagram
  autonumber
  participant Dev as Developer
  participant Husky as Husky pre-commit
  participant Prettier as Prettier (diff)
  participant Q as qlty

  Dev->>Husky: git commit
  Husky->>Prettier: Run on staged TS files
  alt Prettier OK
    Husky->>Q: qlty check on same files
    Q-->>Husky: Results
    Husky-->>Dev: Commit continues or fails
  else Prettier fails
    Husky-->>Dev: Abort commit (qlty skipped)
  end
Loading

Possibly related PRs

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly captures the primary work of adding Qlty configuration and integrating CI checks, aligning exactly with the changes introduced across workflows, config files, and hooks. It is concise, clear, and focused on the main objective of the pull request without extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a5ccc00 and 95087cc.

📒 Files selected for processing (1)
  • .qlty/qlty.toml (1 hunks)

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

@jamesbhobbs jamesbhobbs marked this pull request as ready for review October 9, 2025 16:15
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: 6

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f22894 and a5ccc00.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml (1 hunks)
  • .gitignore (1 hunks)
  • .husky/pre-commit (1 hunks)
  • .qlty/qlty.toml (1 hunks)
⏰ 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). (2)
  • GitHub Check: Build & Test
  • GitHub Check: Lint & Format
🔇 Additional comments (3)
.gitignore (1)

69-74: LGTM!

Standard exclusions for qlty cache and output directories.

.qlty/qlty.toml (2)

32-57: Thresholds look reasonable.

Comment mode with standard thresholds (cognitive_complexity=15, function_length=50) aligns with industry practices for TypeScript codebases.


6-13: No action needed for plugin version pinning. qlty.toml supports a version field for plugins, but per team convention omitting pins ensures you always run the latest plugin releases.

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 8f22894 and a5ccc00.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml (1 hunks)
  • .gitignore (1 hunks)
  • .husky/pre-commit (1 hunks)
  • .qlty/qlty.toml (1 hunks)
🔇 Additional comments (4)
.gitignore (1)

69-74: LGTM!

Standard qlty cache patterns.

.qlty/qlty.toml (2)

6-13: LGTM!

Appropriate security and linting plugins for this workflow.


21-29: LGTM!

Standard exclusion patterns for build artifacts.

.husky/pre-commit (1)

17-17: Pass staged files to qlty check
qlty check currently scans the entire repo. Update to only check staged files:

-npx prettier $changed --check && qlty check
+npx prettier $changed --check && qlty check $changed

Verify locally that qlty check accepts file arguments or supports a filter flag.

andyjakubowski
andyjakubowski previously approved these changes Oct 10, 2025
@jamesbhobbs jamesbhobbs enabled auto-merge (squash) October 10, 2025 09:54
@jamesbhobbs jamesbhobbs disabled auto-merge October 10, 2025 09:56
@jamesbhobbs jamesbhobbs merged commit 574e270 into main Oct 10, 2025
5 checks passed
@jamesbhobbs jamesbhobbs deleted the devin/1760026355-add-qlty-config branch October 10, 2025 09:56
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.

3 participants