Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .jules/bolt.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## 2024-05-18 - Optimize asynchronous file operations and child processes in `services/ReviewGenerator.js`
**Learning:** `generateSplitReviews` and `generateUnifiedReview` previously executed Git operations (e.g. `GitService.getDiffForFile`) and file write operations (`fs.writeFileSync`) sequentially within a `for...of` loop. For large PRs or repos with many changed files, this sequential execution significantly bottlenecks review generation time.
**Action:** Replace `for...of` loops over `includedFiles` with an array of Promises processed via `Promise.all`. This allows Git diffs to be requested and files to be written in parallel, substantially cutting down end-to-end execution time for large payloads. It is critical to use asynchronous I/O (`fs.promises.writeFile`) instead of synchronous alternatives.
**Action:** Replace `for...of` loops over `includedFiles` with an array of Promises processed via `Promise.all`. This allows Git diffs to be requested and files to be written in parallel, substantially cutting down end-to-end execution time for large payloads. It is critical to use asynchronous I/O (`fs.promises.writeFile`) instead of synchronous alternatives.
## 2026-03-04 - Parallelize independent Git CLI commands
**Learning:** Sequential execution of Git CLI commands (via `child_process.execFile`) adds unnecessary latency to endpoints like `/api/staged-files` and `/api/health`. These commands are independent and can be safely parallelized.
**Action:** Always look for opportunities to run independent asynchronous operations concurrently using `Promise.all` to reduce response times, especially for operations that wrap CLI calls.
44 changes: 25 additions & 19 deletions server.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@

// Additional checks for suspicious patterns
const suspiciousPatterns = [
/\x00/, // Null bytes

Check failure on line 181 in server.js

View workflow job for this annotation

GitHub Actions / test (18.x)

Unexpected control character(s) in regular expression: \x00

Check failure on line 181 in server.js

View workflow job for this annotation

GitHub Actions / test (20.x)

Unexpected control character(s) in regular expression: \x00
/[<>"|*?]/, // Dangerous file characters
/^\//, // Absolute paths
/^[a-zA-Z]:\\/, // Windows absolute paths
Expand Down Expand Up @@ -281,21 +281,27 @@
let stagedCount = 0;
let unstagedCount = 0;

try {
const stagedFiles = await GitService.execute('diff-cached-names');
stagedCount = stagedFiles.trim() ? stagedFiles.trim().split('\n').filter(f => f.length > 0).length : 0;
} catch (error) {
console.warn('Failed to get staged files count:', error.message);
// Run git commands in parallel
const [stagedFilesOutput, unstagedFilesOutput] = await Promise.all([
GitService.execute('diff-cached-names').catch(error => {
console.warn('Failed to get staged files count:', error.message);
return '';
}),
GitService.execute('status-porcelain').catch(error => {
console.warn('Failed to get unstaged files count:', error.message);
return '';
})
]);

if (stagedFilesOutput) {
stagedCount = stagedFilesOutput.trim() ? stagedFilesOutput.trim().split('\n').filter(f => f.length > 0).length : 0;
}

try {
const unstagedFiles = await GitService.execute('status-porcelain');
const unstagedLines = unstagedFiles.trim().split('\n').filter(line =>
if (unstagedFilesOutput) {
const unstagedLines = unstagedFilesOutput.trim().split('\n').filter(line =>
line.length > 0 && (line.startsWith(' M') || line.startsWith('??'))
);
unstagedCount = unstagedLines.length;
} catch (error) {
console.warn('Failed to get unstaged files count:', error.message);
}

const totalChanges = stagedCount + unstagedCount;
Expand Down Expand Up @@ -329,34 +335,34 @@

app.get('/api/staged-files', handleAsyncRoute(async (req, res) => {
try {
const output = await GitService.execute('diff-cached-names');
// Run git commands in parallel to optimize response time
const [output, statusOutput, deletedOutput] = await Promise.all([
GitService.execute('diff-cached-names'),
GitService.execute('diff-cached', ['--name-status']).catch(() => ''),
GitService.execute('status-porcelain').catch(() => '')
Comment on lines +341 to +342
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid silent fallback when Git status commands fail.

Returning '' without logging can mask operational failures and misreport status/deletion data as “empty” instead of “unavailable”.

🔧 Suggested fix
-      GitService.execute('diff-cached', ['--name-status']).catch(() => ''),
-      GitService.execute('status-porcelain').catch(() => '')
+      GitService.execute('diff-cached', ['--name-status']).catch((error) => {
+        console.warn('Failed to get staged file statuses:', error.message);
+        return '';
+      }),
+      GitService.execute('status-porcelain').catch((error) => {
+        console.warn('Failed to get unstaged status output:', error.message);
+        return '';
+      })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
GitService.execute('diff-cached', ['--name-status']).catch(() => ''),
GitService.execute('status-porcelain').catch(() => '')
GitService.execute('diff-cached', ['--name-status']).catch((error) => {
console.warn('Failed to get staged file statuses:', error.message);
return '';
}),
GitService.execute('status-porcelain').catch((error) => {
console.warn('Failed to get unstaged status output:', error.message);
return '';
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server.js` around lines 341 - 342, The two GitService.execute calls for
'diff-cached' and 'status-porcelain' are swallowing errors and returning empty
strings which masks failures; update their .catch handlers (the
GitService.execute calls) to log the caught error with context (including which
command failed: 'diff-cached' or 'status-porcelain') using the project logger
(or console) and either rethrow or return a sentinel (e.g., null/undefined)
instead of '' so callers can distinguish "unavailable" from an empty result and
handle failures appropriately.

]);

const files = output.trim() ? output.trim().split('\n').filter(f => f.length > 0) : [];

// Get file statuses to determine if files are deleted
let fileStatuses = {};
try {
const statusOutput = await GitService.execute('diff-cached', ['--name-status']);
if (statusOutput) {
const statusLines = statusOutput.trim().split('\n').filter(line => line.length > 0);
statusLines.forEach(line => {
const [status, filename] = line.split('\t');
if (filename) {
fileStatuses[filename] = status;
}
});
} catch (error) {
// Ignore error, fileStatuses will remain empty
}

// Check for unstaged deleted files
let deletedFiles = [];
try {
const deletedOutput = await GitService.execute('status-porcelain');
if (deletedOutput) {
const deletedLines = deletedOutput.trim().split('\n').filter(line =>
line.length > 0 && (line.startsWith(' D') || line.startsWith('AD'))
);
deletedFiles = deletedLines.map(line => line.substring(line.startsWith('AD') ? 3 : 3));
} catch (error) {
// Ignore error, deletedFiles will remain empty
}

res.json({
Expand Down
Loading