-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add PR metrics utility #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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; | ||||||||
| } | ||||||||
|
|
||||||||
| // SECURITY: eval on user-provided string | ||||||||
| export function parseModelConfig(configStr: string): object { | ||||||||
| return eval("(" + configStr + ")"); | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||||
| } | ||||||||
|
|
||||||||
| // 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++) { | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||||
| 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; | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||||
| } | ||||||||
There was a problem hiding this comment.
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.