Skip to content
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

Cli test list #647

Merged
merged 3 commits into from
Aug 23, 2024
Merged

Cli test list #647

merged 3 commits into from
Aug 23, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Aug 23, 2024

  • A new command list is added to GenAIScript CLI, which lists available tests in the workspace. This is a user facing change.📝
  • Also, a function scriptTestList has been implemented to display the listed tests' id and filename. 📄
  • Description field in FileOutput interface and defFileOutput function in "core/src/prompt_template.d.ts" and "core/src/prompt_type.ts" optional now. This is a user facing change as well. 🔄
  • In "promptdom.ts", it checks if the "description" of FileOutput is available before displaying it. 💫
  • There's a new prompt script added, which utilizes git diff and test list command for commit review. 🆕

generated by pr-describe

Copy link

The pull request introduces several changes:

  1. It adds a new command list under test to list available tests in the workspace. This command calls the scriptTestList function.

  2. It introduces a new function listTests that filters templates based on whether they have tests and whether they match specified ids or groups. This function is used by both scriptTestList and runPromptScriptTests.

  3. It modifies the condition for file output descriptions in promptdom.ts, and the descriptions are now optional.

  4. It updates the interfaces FileOutput and ChatGenerationContext to make description optional in defFileOutput.

  5. It adds a comment to promptrunner.ts indicating that it will show the trace if the expansion failed or no message was generated.

Overall, the changes look fine. However, since description is now optional in several places, make sure to handle cases where it is not provided properly to avoid unexpected behavior or crashes.

So, LGTM 🚀.

generated by pr-review

@@ -72,7 +72,7 @@ export async function cli() {
program.on("option:quiet", () => setQuiet(true))

program
.command("run", { isDefault: true })
.command("run")

Choose a reason for hiding this comment

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

The default command for the CLI has been removed. This could lead to unexpected behavior for users who rely on the default command.

generated by pr-review-commit missing_default

export async function scriptTestList(options: {}) {
const scripts = await listTests(options)
console.log(scripts.map((s) => toStringList(s.id, s.filename)).join("\n"))
}

Choose a reason for hiding this comment

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

Direct use of console.log is found in the scriptTestList function. It's recommended to use a proper logging library or mechanism for better control over log levels and output formatting.

generated by pr-review-commit console_log

@@ -669,13 +669,14 @@ ${trimNewlines(schemaText)}
},
})

if (fileOutputs.length > 0) {
const fods = fileOutputs?.filter((f) => !!f.description)
if (fods?.length > 0) {

Choose a reason for hiding this comment

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

Optional chaining is used on fileOutputs. If fileOutputs is undefined, it could lead to unexpected behavior. Please ensure fileOutputs is always defined or handle the case where it might be undefined.

generated by pr-review-commit optional_chaining

@pelikhan pelikhan merged commit 20483b2 into main Aug 23, 2024
11 checks passed
@pelikhan pelikhan deleted the cli-test-list branch August 23, 2024 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant