git helpers #741

pelikhan · 2024-09-30T22:08:57Z

A new interface named Git has been introduced to the codebase in the prompt_template.d.ts file. This interface seems to allow the interaction with a git repository by finding specific files within it. The exact function, selectModifiedFiles, takes a parameter named scope and options. 🕵️‍♂️
The scope can be one of three values: "branch", "staged", or "modified", while the options parameter can contain properties endsWith and glob. The selectModifiedFiles method returns a promise of WorkspaceFile[]. ⚙️
In prompt_type.d.ts, a new variable git of type Git is declared. This allows developers to directly call Git operations in the current repository. It could potentially simplify Git related operations within the codebase. 🧪
These changes introduce a high level interface to interact with Git repositories, which with additional code, can be used for more complex source control operations. 🧩

All these changes improve the overall architecture and can lead to expanded functionality in handling Git repositories. 💡

generated by pr-describe

…ository operations

packages/core/src/types/prompt_template.d.ts

github-actions · 2024-09-30T22:10:10Z

packages/core/src/types/prompt_type.d.ts

+/**
+ * Access to Git operations for the current repository
+ */
+declare var git: Git


The git variable is declared but its type is not specified. Please provide a type for git to ensure type safety. 🛡️

generated by pr-review-commit missing_type

github-actions · 2024-09-30T22:10:11Z

The changes in GIT_DIFF involve the addition of a new Git interface in TypeScript. This new interface provides a method selectModifiedFiles, which appears to allow selection of specific files in the git repository based on the scope (branch, staged, or modified), and given options.

This method also takes optional parameters that include endsWith and glob, which could be used to filter the files based on their extensions or glob patterns respectively.

A new variable git of type Git is also declared, providing access to Git operations for the current repository.

From the presented changes, the code seems to be in good shape. It is clear and well-documented. However, without context such as knowing the other parts of the code or the actual implementation of the selectModifiedFiles function, it's hard to tell if it will perform as expected functionally.

Therefore, based on the presented changes, my response would be "LGTM 🚀".

generated by pr-review

github-actions · 2024-09-30T23:18:13Z

docs/src/content/docs/reference/scripts/git.md

+Resolve the default branch, typically `main` or `master` in the repository.
+
+```js
+const df = await git.defaultBranch()


The code snippet is missing the function call to execute the git.defaultBranch method. It should be git.defaultBranch() instead of await git.defaultBranch(). The await keyword is used with asynchronous functions, and there is no indication that git.defaultBranch is an asynchronous function in this context. If it is asynchronous, the function should be called within an async function or a thenable context.::

generated by pr-docs-review-commit missing_function_call

github-actions · 2024-09-30T23:18:23Z

packages/core/src/git.ts

+        await resolveFileContents(files)
+        return files
+    }
+}


There is no error handling in the async functions. If the git commands fail for any reason, the error will not be caught and handled properly. This could lead to unexpected behavior or crashes. Consider adding try-catch blocks around the async operations to handle potential errors. 😊

generated by pr-review-commit missing_error_handling

github-actions · 2024-09-30T23:18:24Z

packages/core/src/git.ts

+                filenames = filenames.filter(
+                    (f) => !isGlobMatch(f, excludedPaths)
+                )
+        }


There is no validation of the input parameters in the findModifiedFiles function. This could lead to unexpected behavior if invalid or unexpected input is provided. Consider adding checks to validate the input parameters before using them. 😊

generated by pr-review-commit missing_input_validation

github-actions · 2024-09-30T23:18:24Z

packages/core/src/glob.ts

 ) {
    // Convert patterns to an array and check if any pattern matches the filename
    return arrayify(patterns).some((pattern) => {
        // Perform the match using minimatch with specific options
        const match = minimatch(filename, pattern, {
            // Option to handle Windows paths correctly by preventing escape character issues
            windowsPathsNoEscape: true,
+            ...(options || {}),
        })
        return match // Return true if a match is found
    })


The isGlobMatch function does not validate its input parameters. This could lead to unexpected behavior if the function is called with invalid or unexpected input. Consider adding checks to validate the input parameters before using them. 😊

generated by pr-review-commit missing_input_validation

packages/core/src/git.ts

github-actions · 2024-09-30T23:21:53Z

packages/core/src/glob.ts

 ) {
    // Convert patterns to an array and check if any pattern matches the filename
    return arrayify(patterns).some((pattern) => {
        // Perform the match using minimatch with specific options
        const match = minimatch(filename, pattern, {
            // Option to handle Windows paths correctly by preventing escape character issues
            windowsPathsNoEscape: true,
+            ...(options || {}),
        })
        return match // Return true if a match is found
    })


The isGlobMatch function now has an optional options parameter, but there are no checks to ensure that options is defined before it's used. This could potentially lead to runtime errors. Consider adding checks to ensure options is defined before using it. 😊

generated by pr-review-commit optional_params

…mands

github-actions · 2024-09-30T23:30:23Z

packages/core/src/git.ts

+            Array.isArray(args) ? args : shellParse(args),
+            options
+        )
+        return res.stdout


The exec method does not handle errors. It's recommended to add a try-catch block to handle potential errors during the execution of the git command. 🛠️

generated by pr-review-commit exec_error_handling

github-actions · 2024-09-30T23:30:24Z

docs/src/content/docs/reference/scripts/git.md

+    order: 51
+---
+
+The `git` helper provides a thin wrapper around invoking the [git](https://git-scm.com/) executable.


Consider providing an example of how to invoke the git helper for clarity.

generated by pr-docs-review-commit documentation_content

github-actions · 2024-09-30T23:30:24Z

packages/core/src/git.ts

+                filenames = filenames.filter(
+                    (f) => !isGlobMatch(f, excludedPaths)
+                )
+        }


The findModifiedFiles method does not handle errors. It's recommended to add a try-catch block to handle potential errors during the execution of the git command. 🛠️

generated by pr-review-commit missing_error_handling

github-actions · 2024-09-30T23:30:25Z

docs/src/content/docs/reference/scripts/git.md

+Resolve the default branch, typically `main` or `master` in the repository.
+
+```js
+const df = await git.defaultBranch()


The variable df is not descriptive; consider using a more descriptive variable name like defaultBranch.

generated by pr-docs-review-commit variable_naming

github-actions · 2024-09-30T23:30:25Z

packages/core/src/glob.ts

 ) {
    // Convert patterns to an array and check if any pattern matches the filename
    return arrayify(patterns).some((pattern) => {
        // Perform the match using minimatch with specific options
        const match = minimatch(filename, pattern, {
            // Option to handle Windows paths correctly by preventing escape character issues
            windowsPathsNoEscape: true,
+            ...(options || {}),
        })
        return match // Return true if a match is found
    })


The isGlobMatch function does not handle errors. It's recommended to add a try-catch block to handle potential errors during the execution of the minimatch function. 🛠️

generated by pr-review-commit missing_error_handling

github-actions · 2024-09-30T23:52:43Z

packages/core/src/git.ts

+            Array.isArray(args) ? args : shellParse(args),
+            options
+        )
+        return res.stdout


The exec function does not handle errors. Consider adding a try-catch block to handle potential exceptions. 🛠️

generated by pr-review-commit missing_error_handling

github-actions · 2024-09-30T23:52:44Z

packages/core/src/git.ts

+        const files = filenames.map((filename) => ({ filename }))
+        await resolveFileContents(files)
+        return files
+    }


The findModifiedFiles function does not handle errors. Consider adding a try-catch block to handle potential exceptions. 🛠️

generated by pr-review-commit missing_error_handling

…specific paths

…orrectly

github-actions · 2024-10-01T00:04:15Z

packages/core/src/diff.ts

+    if (!diff) return diff
+
+    const parsed = tryParseDiff(diff)
+    if (!parsed) return undefined



The function tryParseDiff does not handle errors properly. It returns undefined when an error occurs, but it does not log or throw the error, which could make debugging difficult.

generated by pr-review-commit missing_error_handling

github-actions · 2024-10-01T00:04:16Z

packages/core/src/globals.ts

@@ -107,6 +108,8 @@ export function installGlobals() {

    glb.github = new GitHubClient()

+    glb.git = new GitClient()
+


The GitClient is instantiated and assigned to glb.git without checking if the git command is available in the system. This could lead to runtime errors if the git command is not available.

generated by pr-review-commit missing_dependency

… readability

github-actions · 2024-10-01T00:12:12Z

docs/src/content/docs/reference/scripts/git.md

+Resolve the default branch, typically `main` or `master` in the repository.
+
+```js
+const df = await git.defaultBranch()


The await keyword is used without an async function context. The code snippet should be inside an async function or the documentation should mention that it needs to be used within one.

generated by pr-docs-review-commit await_usage

github-actions · 2024-10-01T00:56:12Z

packages/core/src/chat.ts

@@ -647,6 +647,6 @@ export async function executeChatSession(

 export function tracePromptResult(trace: MarkdownTrace, resp: RunPromptResult) {
    const { json, text } = resp
-    trace.details(`🔠 output`, text, { expanded: true })
+    trace.details(`🔠 output`, prettifyMarkdown(text), { expanded: true })


The function prettifyMarkdown is called but it is not defined in the ./markdown module.

generated by pr-review-commit undefined_function

packages/core/src/diff.ts

…n script

github-actions · 2024-10-01T00:59:39Z

packages/core/src/git.ts

+        }
+        return res
+    }
+}


The classes Git, WorkspaceFile, ElementOrArray, and host are used but they are not imported.

generated by pr-review-commit missing_import

…anch method

github-actions · 2024-10-01T01:45:25Z

packages/core/src/git.ts

+        }
+        return res
+    }
+}


The new file git.ts is missing corresponding unit tests.

generated by pr-review-commit missing_tests

…r filtering

github-actions · 2024-10-01T01:48:41Z

docs/src/content/docs/reference/scripts/git.md

+Resolve the default branch, typically `main` or `master` in the repository.
+
+```js
+const df = await git.defaultBranch()


Missing 'await' keyword before 'git.defaultBranch()' function call.

generated by pr-docs-review-commit missing_await

….diff method

github-actions · 2024-10-01T02:11:54Z

docs/src/content/docs/getting-started/automating-scripts.mdx

@@ -10,7 +10,7 @@ import { Steps } from "@astrojs/starlight/components"
 import { FileTree } from "@astrojs/starlight/components"
 import { Image } from "astro:assets"
 import { Code } from "@astrojs/starlight/components"
-import prDescribeSrc from "../../../../../packages/sample/genaisrc/pr-describe.genai.js?raw"
+import prDescribeSrc from "../../../../../packages/sample/genaisrc/pr-describe.genai.mjs?raw"


The import statement for 'prDescribeSrc' has an incorrect file extension. It should be '.js' instead of '.mjs'.

generated by pr-docs-review-commit incorrect_extension

github-actions · 2024-10-01T02:11:54Z

docs/src/content/docs/reference/scripts/git.md

+
+```js
+const df = await git.defaultBranch()
+```


The code snippet is missing the declaration for the 'git' variable. It should show how to import or require the 'git' helper module.

generated by pr-docs-review-commit missing_variable_declaration

packages/core/src/diff.ts

… packages

github-actions · 2024-10-01T04:19:49Z

docs/src/content/docs/getting-started/automating-scripts.mdx

@@ -10,7 +10,7 @@ import { Steps } from "@astrojs/starlight/components"
 import { FileTree } from "@astrojs/starlight/components"
 import { Image } from "astro:assets"
 import { Code } from "@astrojs/starlight/components"
-import prDescribeSrc from "../../../../../packages/sample/genaisrc/pr-describe.genai.js?raw"
+import prDescribeSrc from "../../../../../packages/sample/genaisrc/pr-describe.genai.mjs?raw"


The file extension for the imported script should be .js instead of .mjs to maintain consistency with the existing codebase.

generated by pr-docs-review-commit incorrect_file_extension

github-actions · 2024-10-01T04:19:50Z

docs/src/content/docs/reference/scripts/git.md

+
+```js
+const df = await git.defaultBranch()
+```


The code snippet is missing the function declaration for git.defaultBranch. It should be preceded by an import statement or a function declaration to provide context.

generated by pr-docs-review-commit missing_function_declaration

github-actions · 2024-10-01T04:19:54Z

packages/core/src/diff.ts

-export function llmifyDiff(diff: string) {
-    if (!diff) return diff
-
+/**


The function llmifyDiff is called without arguments. This will result in an error as the function expects a string argument.

generated by pr-review-commit missing_argument

github-actions · 2024-10-01T04:19:55Z

packages/core/src/diff.ts

+ * @returns The LLMDiff formatted string, or undefined if parsing fails.
+ */
+export function llmifyDiff(diff: string) {
+    if (!diff) return diff


The function tryParseDiff is called without arguments. This function expects a string argument.

generated by pr-review-commit missing_argument

github-actions · 2024-10-01T04:19:56Z

packages/core/src/liner.ts

@@ -12,7 +12,7 @@ import { llmifyDiff } from "./diff"
 * @returns The text with line numbers added, or processed diff text if applicable.
 */
 export function addLineNumbers(text: string, language?: string) {
-    if (language === "diff") {
+    if (language === "diff" || tryParseDiff(text)) {
        const diffed = llmifyDiff(text) // Process the text with a special function for diffs
        if (diffed !== undefined) return diffed // Return processed text if diff handling was successful
    }


The function tryParseDiff is called without arguments. This function expects a string argument.

generated by pr-review-commit missing_argument

github-actions · 2024-10-01T04:22:45Z

packages/cli/src/run.ts

@@ -179,12 +180,13 @@ export async function runScript(
        if (removeOut) await emptyDir(out)
        await ensureDir(out)
    }
+    let outTraceFilename
    if (outTrace && !/^false$/i.test(outTrace) && trace)


Missing semicolon

generated by pr-review-commit missing_semi

github-actions · 2024-10-01T04:22:46Z

packages/core/src/diff.ts

            if (isNaN(diffln) && !isNaN(currentLine)) {
                currentLine++
                diffln = currentLine
                if (op === "-") currentLine--
            } else {
                currentLine = diffln
            }
+
+            // Handle added lines
            if (op === "+") {
                const l = line.substring(diffM[0].length)


Unused variable 'l'

generated by pr-review-commit unused_variable

…finition in checkModifications function.

github-actions · 2024-10-01T04:25:59Z

docs/src/content/docs/reference/scripts/git.md

+
+```js
+const df = await git.defaultBranch()
+```


The code example for 'git.defaultBranch' is incomplete. It should include the import statement and possibly more context for clarity.

generated by pr-docs-review-commit missing_code_example

github-actions · 2024-10-01T04:26:03Z

packages/core/src/diff.ts

            if (isNaN(diffln) && !isNaN(currentLine)) {
                currentLine++
                diffln = currentLine
                if (op === "-") currentLine--
            } else {
                currentLine = diffln
            }
+
+            // Handle added lines
            if (op === "+") {
                const l = line.substring(diffM[0].length)


Missing semicolon

generated by pr-review-commit missing_semi

github-actions · 2024-10-01T04:26:04Z

packages/core/src/diff.ts

@@ -54,6 +64,7 @@ export function parseLLMDiffs(text: string): Chunk[] {
                    chunks.push(chunk)
                }
            } else {
+                // Handle deleted lines
                assert(op === "-")


Missing semicolon

generated by pr-review-commit missing_semi

github-actions · 2024-10-01T04:27:00Z

Investigator report

run failure
, run last success
, 9a3211d
, diff 682c2d3...9a3211d

Analysis

The root cause of the failure seems to be related to a TypeScript compilation error. The error log indicates:

genaisrc/sc.genai.mts(14,45): error TS2345: Argument of type '"branch"' is not assignable to parameter of type '"base" | "staged" | "modified"'.

This error occurs because there is a mismatch between the type being passed and the expected parameter types of a function or method.

Suggested Fix

To fix the issue, you need to ensure that the argument passed matches one of the expected types. Here's a suggested change in the code:

- someFunction("branch")
+ someFunction("base") // or "staged" or "modified" as needed

Make sure to adjust the argument based on the intended functionality. If "branch" is needed, you may need to update the function's parameter type definition to include "branch".

generated by gai

Add Git interface for file selection and declare git variable for rep…

574fb12

…ository operations

github-actions bot reviewed Sep 30, 2024

View reviewed changes

packages/core/src/types/prompt_template.d.ts Show resolved Hide resolved

github-actions bot reviewed Sep 30, 2024

View reviewed changes

packages/core/src/types/prompt_template.d.ts Show resolved Hide resolved

github-actions bot reviewed Sep 30, 2024

View reviewed changes

pelikhan added 2 commits September 30, 2024 22:16

wiring up globlas

3b19e95

Add git helper functions for branch and file modifications detection

adcdd8c

github-actions bot reviewed Sep 30, 2024

View reviewed changes

Use --porcelain format in git status to ignore deleted files

81f2cb5

github-actions bot reviewed Sep 30, 2024

View reviewed changes

packages/core/src/git.ts Show resolved Hide resolved

github-actions bot reviewed Sep 30, 2024

View reviewed changes

Refactor GitClient to use a unified exec method for executing git com…

682c2d3

…mands

github-actions bot reviewed Sep 30, 2024

View reviewed changes

Refactor GitClient for scope handling and add diff method with filters

6897762

github-actions bot reviewed Sep 30, 2024

View reviewed changes

pelikhan added 2 commits September 30, 2024 23:53

Refactor git diff command to use object-based parameters and exclude …

c074fad

…specific paths

Refactor diff parsing and update GitClient to handle excluded paths c…

7b9a8e2

…orrectly