Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a “Repository Hygiene Analysis” feature to the Usage Analysis webview, backed by a new extension-side handler that invokes Copilot via the VS Code Language Model API and renders a structured hygiene report per workspace/repo.
Changes:
- Adds repo hygiene UI (single-repo + multi-repo list/details) and result rendering to the Usage Analysis webview.
- Implements extension handlers to analyze one or all repositories and send results back to the webview.
- Introduces a repo hygiene “skill” prompt template used to drive the AI analysis; adds a new
.prettierignore.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/webview/usage/styles.css | Adds styling for the new repo hygiene panels/buttons. |
| src/webview/usage/main.ts | Adds repo hygiene section UI, state management, and message handling for analysis results. |
| src/extension.ts | Adds commands to run repo analysis (single + batch), git info gathering, file-tree scanning, and Copilot LM invocation. |
| src/backend/repoHygieneSkill.ts | Adds the skill/prompt instructions that define expected checks and JSON schema guidance. |
| .prettierignore | Disables Prettier formatting by ignoring all files. |
| # Ignore all files - formatting disabled for this repository | ||
| **/* |
There was a problem hiding this comment.
.prettierignore currently ignores **/*, which effectively disables Prettier formatting for the entire repository (including in the devcontainer where Prettier is configured as the default formatter). If the goal is only to skip generated artifacts, narrow this to build/output directories instead of ignoring everything.
| # Ignore all files - formatting disabled for this repository | |
| **/* | |
| # Ignore generated and build artifacts; allow Prettier to format source files | |
| node_modules/ | |
| dist/ | |
| out/ | |
| coverage/ | |
| *.log |
There was a problem hiding this comment.
Meaning is to ignore, it will be enabled in a separate pr. For it just brings a lot of noise
| if (workspacePath) { | ||
| // Use the provided workspace path | ||
| workspaceRoot = workspacePath; | ||
| } else { | ||
| // Fall back to the first open workspace folder | ||
| const firstFolder = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath; | ||
| if (!firstFolder) { | ||
| throw new Error('No workspace folder open'); | ||
| } | ||
| workspaceRoot = firstFolder; | ||
| } |
There was a problem hiding this comment.
runRepoHygieneAnalysis accepts workspacePath directly from the webview (which can include pseudo-paths like <unresolved:...> from the customization matrix). This will produce meaningless scans and can trigger unexpected errors. Consider rejecting unresolved/non-existent paths (e.g., detect <unresolved: and/or verify fs.existsSync(workspaceRoot)), and surface a clear error back to the webview.
| const branch = childProcess.execSync('git rev-parse --abbrev-ref HEAD', { | ||
| cwd: workspaceRoot, | ||
| encoding: 'utf8', | ||
| timeout: 5000, | ||
| stdio: ['pipe', 'pipe', 'pipe'] | ||
| }).trim(); | ||
| branchName = branch; | ||
|
|
||
| try { | ||
| const remote = childProcess.execSync('git remote get-url origin', { | ||
| cwd: workspaceRoot, | ||
| encoding: 'utf8', | ||
| timeout: 5000, | ||
| stdio: ['pipe', 'pipe', 'pipe'] | ||
| }).trim(); |
There was a problem hiding this comment.
Using childProcess.execSync for git commands can block the extension host thread (up to the 5s timeout each time) and will scale poorly when analyzing multiple repos. Prefer an async approach (e.g., execFile/spawn wrapped in a Promise) so the UI remains responsive during analysis.
| const configPatterns = [ | ||
| '.git', '.gitignore', '.env.example', '.env.sample', '.editorconfig', | ||
| '.eslintrc', 'eslint.config', '.prettierrc', 'prettier.config', | ||
| 'tsconfig.json', 'jsconfig.json', 'package.json', 'Makefile', | ||
| 'Dockerfile', 'docker-compose', '.github/workflows', '.devcontainer', | ||
| 'LICENSE', '.nvmrc', '.node-version' | ||
| ]; | ||
|
|
||
| try { | ||
| const files: string[] = []; | ||
| const maxDepth = 3; | ||
|
|
||
| const scanDir = (dir: string, depth: number = 0) => { | ||
| if (depth > maxDepth) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const entries = fs.readdirSync(dir, { withFileTypes: true }); | ||
| for (const entry of entries) { | ||
| const fullPath = path.join(dir, entry.name); | ||
| const relativePath = path.relative(workspaceRoot, fullPath); | ||
|
|
||
| // Skip node_modules and other large directories | ||
| if (entry.name === 'node_modules' || entry.name === '.git' || | ||
| entry.name === 'dist' || entry.name === 'build' || entry.name === 'out') { | ||
| continue; | ||
| } | ||
|
|
||
| // Check if this file matches any config pattern | ||
| const isConfig = configPatterns.some(pattern => relativePath.includes(pattern)); | ||
|
|
There was a problem hiding this comment.
getWorkspaceFileTree uses relativePath.includes(pattern) with patterns like .git and .github/workflows. This will (1) over-match (e.g. .git matches .github, .gitignore, etc.) and (2) fail to match forward-slash patterns on Windows where path.relative returns backslashes. Consider normalizing relativePath to POSIX separators and matching by path segment / exact basename rather than substring includes.
| The skill performs 17 automated checks across 5 categories: | ||
|
|
||
| - **Version Control**: Git setup, ignore files, environment templates | ||
| - **Code Quality**: Linters, formatters, type safety configuration | ||
| - **CI/CD & Automation**: Continuous integration, standard scripts, task runners | ||
| - **Environment**: Dev containers, Docker, runtime version pinning | ||
| - **Documentation**: License files, commit message quality |
There was a problem hiding this comment.
The skill text claims “17 automated checks across 5 categories” including a Documentation category, but the checklist below only enumerates 16 checks and does not define any Documentation checks/weights. This mismatch is likely to produce responses that don’t satisfy the JSON schema (e.g., wrong totalChecks, missing documentation category scores).
| The skill performs 17 automated checks across 5 categories: | |
| - **Version Control**: Git setup, ignore files, environment templates | |
| - **Code Quality**: Linters, formatters, type safety configuration | |
| - **CI/CD & Automation**: Continuous integration, standard scripts, task runners | |
| - **Environment**: Dev containers, Docker, runtime version pinning | |
| - **Documentation**: License files, commit message quality | |
| The skill performs 16 automated checks across 4 categories: | |
| - **Version Control**: Git setup, ignore files, environment templates | |
| - **Code Quality**: Linters, formatters, type safety configuration | |
| - **CI/CD & Automation**: Continuous integration, standard scripts, task runners | |
| - **Environment**: Dev containers, Docker, runtime version pinning |
| const response = await model.sendRequest(messages, {}, new vscode.CancellationTokenSource().token); | ||
|
|
||
| let fullResponse = ''; | ||
| for await (const chunk of response.text) { | ||
| fullResponse += chunk; | ||
| } | ||
|
|
||
| this.log(`📋 Copilot analysis response length: ${fullResponse.length} characters`); | ||
|
|
||
| // Extract JSON from response (in case it's wrapped in markdown code blocks) | ||
| let jsonText = fullResponse.trim(); | ||
| const jsonMatch = jsonText.match(/```(?:json)?\s*\n?([\s\S]*?)\n?```/); | ||
| if (jsonMatch) { | ||
| jsonText = jsonMatch[1].trim(); | ||
| } | ||
|
|
||
| // Parse the JSON response | ||
| const results = JSON.parse(jsonText); | ||
|
|
||
| // Validate the structure | ||
| if (!results.summary || !results.checks || !results.metadata) { | ||
| throw new Error('Invalid response structure from Copilot'); | ||
| } | ||
|
|
||
| return results; |
There was a problem hiding this comment.
model.sendRequest(...) creates a new CancellationTokenSource inline and never disposes it, and the request can’t be cancelled from the UI. Create a CTS, pass its token, and ensure it’s disposed (and ideally expose cancellation via the webview) to avoid resource leaks for longer analyses.
| const response = await model.sendRequest(messages, {}, new vscode.CancellationTokenSource().token); | |
| let fullResponse = ''; | |
| for await (const chunk of response.text) { | |
| fullResponse += chunk; | |
| } | |
| this.log(`📋 Copilot analysis response length: ${fullResponse.length} characters`); | |
| // Extract JSON from response (in case it's wrapped in markdown code blocks) | |
| let jsonText = fullResponse.trim(); | |
| const jsonMatch = jsonText.match(/```(?:json)?\s*\n?([\s\S]*?)\n?```/); | |
| if (jsonMatch) { | |
| jsonText = jsonMatch[1].trim(); | |
| } | |
| // Parse the JSON response | |
| const results = JSON.parse(jsonText); | |
| // Validate the structure | |
| if (!results.summary || !results.checks || !results.metadata) { | |
| throw new Error('Invalid response structure from Copilot'); | |
| } | |
| return results; | |
| const cts = new vscode.CancellationTokenSource(); | |
| try { | |
| const response = await model.sendRequest(messages, {}, cts.token); | |
| let fullResponse = ''; | |
| for await (const chunk of response.text) { | |
| fullResponse += chunk; | |
| } | |
| this.log(`📋 Copilot analysis response length: ${fullResponse.length} characters`); | |
| // Extract JSON from response (in case it's wrapped in markdown code blocks) | |
| let jsonText = fullResponse.trim(); | |
| const jsonMatch = jsonText.match(/```(?:json)?\s*\n?([\s\S]*?)\n?```/); | |
| if (jsonMatch) { | |
| jsonText = jsonMatch[1].trim(); | |
| } | |
| // Parse the JSON response | |
| const results = JSON.parse(jsonText); | |
| // Validate the structure | |
| if (!results.summary || !results.checks || !results.metadata) { | |
| throw new Error('Invalid response structure from Copilot'); | |
| } | |
| return results; | |
| } finally { | |
| cts.dispose(); | |
| } |
| ## Total Weights: 76 points | ||
| - Version Control: 13 points (17%) | ||
| - Code Quality: 17 points (22%) | ||
| - CI/CD & Automation: 10 points (13%) | ||
| - Environment: 9 points (12%) |
There was a problem hiding this comment.
The “Total Weights” section totals 76 points but omits the Documentation category entirely, while the prompt/schema in runRepoHygieneAnalysis expects documentation.maxScore = 5 and totalChecks = 17. The weights/categories in the skill should align with the enforced schema so the model can return consistent totals.
| ## Total Weights: 76 points | |
| - Version Control: 13 points (17%) | |
| - Code Quality: 17 points (22%) | |
| - CI/CD & Automation: 10 points (13%) | |
| - Environment: 9 points (12%) | |
| ## Total Weights: 81 points | |
| - Version Control: 13 points (16%) | |
| - Code Quality: 17 points (21%) | |
| - CI/CD & Automation: 10 points (12%) | |
| - Environment: 9 points (11%) | |
| - Documentation: 5 points (6%) |
| .repo-hygiene-list { | ||
| max-height: none; | ||
| overflow: visible; | ||
| } | ||
|
|
There was a problem hiding this comment.
.repo-hygiene-list is defined but not referenced anywhere in the usage webview markup/scripts. If it’s leftover from an earlier iteration, consider removing it to avoid accumulating dead CSS.
| .repo-hygiene-list { | |
| max-height: none; | |
| overflow: visible; | |
| } |
| function getScoreLabel(workspacePath: string): string { | ||
| const record = repoAnalysisState.get(workspacePath); | ||
| if (record?.data?.summary) { | ||
| return `${Math.round(record.data.summary.percentage)}%`; |
There was a problem hiding this comment.
getScoreLabel() rounds record.data.summary.percentage directly; if the AI returns this as a string (or any non-number), this will render NaN% in the repo list. Consider reusing toFiniteNumber() here (as you do elsewhere) to make the UI resilient to minor schema deviations.
| return `${Math.round(record.data.summary.percentage)}%`; | |
| const percentage = toFiniteNumber(record.data.summary.percentage); | |
| return `${Math.round(percentage)}%`; |
Scan repo for setup that makes copilot perform better, scanned by AI
Initial setup with multi repo scan, and details view. Can improve from here