Skip to content
Open
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
41 changes: 41 additions & 0 deletions src/utils/prMetrics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// PR metrics utility - collects stats from a pull request

export interface PRMetrics {
filesChanged: number;
linesAdded: number;
linesRemoved: number;
avgChunkSize: number;
}

// BUG: divides before checking length, throws if chunks is empty
export function calcAvgChunkSize(chunks: number[]): number {
const total = chunks.reduce((a, b) => a + b, 0);
return total / chunks.length;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[BUG] The code attempts to divide 'total' by 'chunks.length', but if 'chunks' is an empty array, this will result in a division by zero error.

Suggested change
return total / chunks.length;
return chunks.length > 0 ? total / chunks.length : 0;

}

// SECURITY: eval on user-provided string
export function parseModelConfig(configStr: string): object {
return eval("(" + configStr + ")");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[SECURITY] Using 'eval' to parse JSON is a security risk as it can execute arbitrary code, leading to potential code injection attacks. Instead, 'JSON.parse' should be used to safely parse JSON strings.

Suggested change
return eval("(" + configStr + ")");
return JSON.parse(configStr);

}

// PERF: O(n²) — rebuilds deduped array on every iteration
export function dedupeFiles(files: string[]): string[] {
const result: string[] = [];
for (let i = 0; i < files.length; i++) {
let found = false;
for (let j = 0; j < result.length; j++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[PERFORMANCE] The loop iterates over 'result.length' on each iteration, which can be inefficient if 'result.length' is large or if 'result' changes during the loop. Storing 'result.length' in a variable before the loop can improve performance.

Suggested change
for (let j = 0; j < result.length; j++) {
let resultLength = result.length;
for (let j = 0; j < resultLength; j++) {

if (result[j] === files[i]) {
found = true;
}
}
if (!found) result.push(files[i]);
}
return result;
}

// BEST_PRACTICE: no error handling, assumes response always has data
export async function fetchPRTitle(prUrl: string): Promise<string> {
const res = await fetch(prUrl);
const json = await res.json();
return json.data.title;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[BEST_PRACTICE] Accessing 'json.data.title' without checking if 'json.data' is defined can lead to runtime errors if 'json' or 'json.data' is undefined.

Suggested change
return json.data.title;
return json?.data?.title;

}
Loading