Conversation
Rsdoctor Bundle Diff AnalysisFound 2 projects in monorepo, 1 project with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 rsbuild-demoPath:
📦 Download Diff Report: rsbuild-demo Bundle Diff 🤖 AI Degradation Analysis (Click to expand)📁 rsbuild-demo📦 Bundle Size Analysis Report1. Size Regression Summary
2. Root Cause AnalysisThe entire size increase is attributed to a single module change:
3. Risk AssessmentSeverity: 🟢 Low
4. Optimization Recommendations
Generated by Rsdoctor Bundle Analyzer Analysis by qwen3.5-plus 📁 rsbuild-demo2📦 Bundle Size Analysis Report✅ No Significant Regressions Detected
1. Size Regression Summary
2. Root Cause Analysis
3. Risk Assessment
4. Optimization Recommendations
Generated by Rsdoctor Bundle Analyzer Analysis by qwen3.5-plus Generated by Rsdoctor GitHub Action |
There was a problem hiding this comment.
Pull request overview
Adds optional AI-powered degradation analysis to the Rsdoctor GitHub Action, generating an additional markdown section in PR comments based on an Rsdoctor bundle-diff JSON output.
Changes:
- Add
src/ai-analysis.tsto call Anthropic/OpenAI and produce a markdown analysis from bundle-diff JSON. - Extend
src/index.tsto generatebundle-diff --json, run AI analysis whenAI_TOKENis set, and append results to the PR comment. - Add
ai_modelinput toaction.yml; update built artifact indist/(and includes a small example app change).
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.ts | Generates JSON bundle diff and appends AI analysis to PR comment when enabled. |
| src/ai-analysis.ts | Implements provider detection, prompt building, and API calls to generate markdown analysis. |
| action.yml | Adds ai_model input for configuring the AI model. |
| examples/rsbuild-demo/src/App.tsx | Comments out a duplicate button block in the demo app. |
| dist/index.js | Updates compiled distribution bundle to include new functionality. |
| // Generate JSON diff for AI analysis (requires @rsdoctor/cli >= 1.5.6-canary.0) | ||
| if (aiToken) { | ||
| try { |
There was a problem hiding this comment.
This comment says the JSON diff requires @rsdoctor/cli >= 1.5.6-canary.0, but package.json currently pins @rsdoctor/cli to 1.3.3-beta.2. With the pinned version, the --json invocation is likely to fail (and the catch block logs a misleading “CLI not found” message). Either bump the dependency to a version that supports bundle-diff --json or gate the JSON generation by detecting support (and log an accurate error when the command fails).
| } | ||
|
|
||
| const resolvedJsonPath = fs.existsSync(diffJsonPath) ? diffJsonPath : defaultDiffJsonPath; | ||
| report.aiAnalysis = await analyzeWithAI(resolvedJsonPath, aiToken, aiModel); |
There was a problem hiding this comment.
resolvedJsonPath can end up pointing to defaultDiffJsonPath even if neither JSON file was produced (e.g., rsdoctor command failed). analyzeWithAI will then log “not found” and skip, but from this call-site it’s hard to tell whether JSON generation succeeded. Consider explicitly checking existsSync(resolvedJsonPath) after running rsdoctor and only calling analyzeWithAI (or logging a clearer message) when the JSON output was actually created.
| report.aiAnalysis = await analyzeWithAI(resolvedJsonPath, aiToken, aiModel); | |
| if (fs.existsSync(resolvedJsonPath)) { | |
| report.aiAnalysis = await analyzeWithAI(resolvedJsonPath, aiToken, aiModel); | |
| } else { | |
| console.warn( | |
| `⚠️ rsdoctor JSON diff file not found for AI analysis. Expected at "${resolvedJsonPath}". ` + | |
| `Skipping AI analysis.` | |
| ); | |
| } |
| commentBody += '<details>\n<summary><b>🤖 AI Degradation Analysis</b> (Click to expand)</summary>\n\n'; | ||
| for (const report of reportsWithAI) { | ||
| if (!report.aiAnalysis) continue; | ||
| if (reportsWithAI.length > 1) { | ||
| commentBody += `#### 📁 ${report.projectName}\n\n`; | ||
| } | ||
| commentBody += report.aiAnalysis.analysis + '\n\n'; | ||
| commentBody += `<sub>Analysis by ${report.aiAnalysis.model}</sub>\n\n`; | ||
| } |
There was a problem hiding this comment.
The AI-generated markdown is appended directly into the PR comment body. Since this content comes from an external service and is influenced by repo-controlled input (prompt injection via bundle/module names), it can produce unwanted mentions (@org/@user), links, or very large output that may exceed GitHub’s comment size limits. Consider sanitizing the analysis before posting (e.g., neutralize @ to avoid mentions, optionally strip/limit links/images) and truncating to a safe maximum length with a note when truncated.
| default: '' | ||
| type: string | ||
| ai_model: | ||
| description: 'AI model to use for degradation analysis (e.g. claude-3-5-haiku-latest, gpt-4o-mini). Provider is auto-detected from the model name prefix.' |
There was a problem hiding this comment.
ai_model is configurable, but enabling the feature requires setting AI_TOKEN via environment variables; that requirement isn’t discoverable from action.yml. Consider adding an explicit ai_token input (intended for secrets.*) or at least documenting in the input description that an environment variable is required and that bundle diff JSON will be sent to a third-party AI provider when enabled.
| description: 'AI model to use for degradation analysis (e.g. claude-3-5-haiku-latest, gpt-4o-mini). Provider is auto-detected from the model name prefix.' | |
| description: 'AI model to use for degradation analysis (e.g. claude-3-5-haiku-latest, gpt-4o-mini). Provider is auto-detected from the model name prefix. Requires an AI_TOKEN environment variable (typically from secrets.*) for the provider API key, and when enabled the bundle diff JSON will be sent to the selected third-party AI provider.' |
| {/* <button onClick={handleClick} className="primary-button"> | ||
| Click Me! | ||
| </button> | ||
| <button onClick={() => console.log('Secondary button')} className="secondary-button"> | ||
| Secondary Action | ||
| </button> | ||
| </button> */} |
There was a problem hiding this comment.
This change comments out an entire button block, but the file already renders the same buttons in the next <div className="button-container">. If this was only for local debugging, it should be removed/reverted to keep the example clean; if it’s intentional, consider deleting the duplicate section rather than leaving commented JSX in the source.
| const shellCmd = `npx @rsdoctor/cli bundle-diff --json --baseline="${baselineJsonPath}" --current="${fullPath}"`; | ||
| console.log(`🛠️ Running rsdoctor --json via npx: ${shellCmd}`); | ||
| await execFileAsync('sh', ['-c', shellCmd], { cwd: tempOutDir }); |
There was a problem hiding this comment.
The fallback path uses execFileAsync('sh', ['-c', shellCmd], ...) with a command string that interpolates baselineJsonPath / fullPath. This is shell-injection prone if any path contains quotes or shell metacharacters (paths come from the repo/glob input), and it also makes quoting/escaping brittle across platforms. Prefer invoking npx (or the local node_modules/.bin/rsdoctor) via execFile with an argv array (no shell), e.g. execFileAsync('npx', ['--no-install','@rsdoctor/cli','bundle-diff','--json', ...]) or directly running the resolved bin.
| const shellCmd = `npx @rsdoctor/cli bundle-diff --json --baseline="${baselineJsonPath}" --current="${fullPath}"`; | |
| console.log(`🛠️ Running rsdoctor --json via npx: ${shellCmd}`); | |
| await execFileAsync('sh', ['-c', shellCmd], { cwd: tempOutDir }); | |
| const npxArgs = [ | |
| '@rsdoctor/cli', | |
| 'bundle-diff', | |
| '--json', | |
| `--baseline=${baselineJsonPath}`, | |
| `--current=${fullPath}`, | |
| ]; | |
| console.log(`🛠️ Running rsdoctor --json via npx: npx ${npxArgs.join(' ')}`); | |
| await execFileAsync('npx', npxArgs, { cwd: tempOutDir }); |
| const data = await response.json() as any; | ||
| return data.content[0].text as string; | ||
| } |
There was a problem hiding this comment.
callAnthropicAPI assumes data.content[0].text exists. If the API returns an error shape (or content is empty), this will throw a confusing runtime error later. Consider validating the response JSON structure and throwing a clearer error when expected fields are missing (similar for OpenAI response parsing).
| const data = await response.json() as any; | ||
| return data.choices[0].message.content as string; | ||
| } |
There was a problem hiding this comment.
callOpenAIAPI assumes data.choices[0].message.content exists. If the response is missing choices (rate limit, validation error, etc.), this will throw a non-actionable exception. Add minimal shape checks (e.g., ensure choices is an array and the content is a string) and include provider/model context in the thrown error to aid debugging.
| export async function analyzeWithAI( | ||
| diffJsonPath: string, | ||
| token: string, | ||
| model = 'claude-3-5-haiku-latest', | ||
| ): Promise<AIAnalysisResult | null> { | ||
| if (!token) { | ||
| console.log('ℹ️ No AI token provided, skipping AI analysis'); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
New AI analysis functionality is introduced here, but there are no tests covering the new module. The repo already uses @rstest/core with mocks (see src/__tests__). Please add unit tests for analyzeWithAI covering: no token, missing JSON file, provider detection from model name, and successful/failed API calls (mock fetch responses).
No description provided.