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

Blog ideation #643

Merged
merged 5 commits into from
Aug 22, 2024
Merged

Blog ideation #643

merged 5 commits into from
Aug 22, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Aug 22, 2024

  • 💼 The compile command for GenAI scripts has been broadened to incorporate multiple folder input, enhancing its flexibility. Now you can specify which folders to compile using this command. In absence of any arguments, all scripts in the workspace will be compiled, as it functioned before.
  • 🔨 A new function, unfence, has been added to 'fence.ts'. This function removes the fence that identifies a particular language section in a text.
  • 🚀 Optimization made regarding assistant's responses in chat sessions. Now assistant unfences its response text (basically, removing any markdown language tags) if the response type is undefined. This removal of markdown language tags from the chat might improve readability and simplicity of the chat.
  • 🎯 MAX_TOOL_CALLS constant in 'constants.ts' has been ramped up from 100 to 10,000. This upgrade to higher value hints at supporting robust, resilient and complex tool interactions.
  • 🗂️ In 'PromptTest' interface (which represents a test case in the context of prompts), an optional key name has been added for storing a short name of the test.
  • 📝 The 'runTemplate' function in 'promptrunner.ts' has been modified to always set the validity of the file edit operation to true, potentially improving file handling measures.
  • ❌ Minor tweaks done to the 'Chat' section, revised assistant text parsing mechanism.
  • 📑 For 'compileScript' function, 'name' parameter has been replaced by 'folders', allowing the function to compile scripts in the specified folders.

generated by pr-describe

@@ -120,7 +120,7 @@ Commands:
list List all available scripts in workspace
create <name> Create a new script
fix fix all definition files

Choose a reason for hiding this comment

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

The usage of the 'compile' command has been updated to include '<folders...>' but the description 'Compile all script in workspace' has not been updated to reflect the change in usage. Consider revising the description to indicate that the command now accepts specific folder patterns as arguments.

generated by pr-docs-review-commit command_usage_inconsistency

.map((f) => scriptFolders.find((sf) => sf.dirname === f))
.filter((f) => f)

for (const folder of foldersToCompile) {

Choose a reason for hiding this comment

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

There is no error handling for the case when the folders argument is not provided or is an empty array. This could lead to unexpected behavior or errors. Consider adding error handling or a default value. 🚧

generated by pr-review-commit missing_error_handling


if (responseType === undefined) {
text = unfence(text, "(markdown|md)")
}

Choose a reason for hiding this comment

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

The function unfence is called only when responseType is undefined. However, there is no handling for the case when responseType is not undefined. This could lead to unexpected behavior. Consider adding handling for this case. 🚧

generated by pr-review-commit unhandled_case

@@ -323,6 +323,7 @@ export async function runTemplate(
trace.detailsFenced(`📁 file ${fn}`, content)
const fileEdit = await getFileEdit(fn)
fileEdit.after = content
fileEdit.validation = { valid: true }

Choose a reason for hiding this comment

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

The valid property of fileEdit.validation is hardcoded to true. This might not reflect the actual validation status of the file edit. Consider calculating this value based on the actual validation results. 🚧

generated by pr-review-commit hardcoded_value

Copy link

The code changes seem to be valid from the TypeScript compiler's stand point. Here are some key points:

  • The compileScript function now takes an array of folders as an argument, and only compiles scripts from those specific directories. If no folders are provided, it compiles scripts from all directories in the project. This adds flexibility to the script compilation process. 🚀
  • A new function unfence has been introduced to remove the fencing from a text, essentially unformatting it if it has been formatted in a certain language (like markdown).
  • Modifications have been made to assistantText to optionally unformat the assistant's message and the message processing now takes into account the response type.
  • The maximum number of tool calls has been increased from 100 to 10000, which is a very significant change. This could impact the performance of the application depending on the resources available. ⚠️
  • A minor update has been made to the runTemplate function to mark the file edit as valid.
  • A name field is added to the PromptTest interface which could be useful for better identifying tests.

Overall, these changes are enhancing the functionality of the project and giving users more control. However, you may want to test the change in maximum tool calls extensively to ensure it doesn't degrade performance.

So, LGTM 🚀

generated by pr-review

@@ -164,10 +164,13 @@ Options:
### `scripts compile`

```
Usage: genaiscript scripts compile [options]
Usage: genaiscript scripts compile [options] [folders...]

Compile all script in workspace

Choose a reason for hiding this comment

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

There seems to be a typo here. It should be "Compile all scripts in workspace" instead of "Compile all script in workspace".

generated by pr-docs-review-commit typo


Compile all script in workspace

Arguments:
folders Pattern to match files

Choose a reason for hiding this comment

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

The description "Pattern to match files" for the argument 'folders' is unclear. It should specify that 'folders' is a pattern to match directories containing scripts to compile.

generated by pr-docs-review-commit description_error

.option(
"-rr, --run-retry <number>",
"number of retries for the entire run"
)

Choose a reason for hiding this comment

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

Missing semicolon

generated by pr-review-commit missing_semi

@@ -205,6 +208,7 @@ export async function cli() {
scripts
.command("compile")
.description("Compile all script in workspace")
.argument("[folders...]", "Pattern to match files")
.action(compileScript)

Choose a reason for hiding this comment

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

Missing semicolon

generated by pr-review-commit missing_semi

@@ -120,7 +120,7 @@ Commands:
list List all available scripts in workspace
create <name> Create a new script
fix fix all definition files

Choose a reason for hiding this comment

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

Usage description for 'compile' command is inconsistent with the updated usage syntax.

generated by pr-docs-review-commit incorrect_usage

@@ -164,10 +164,13 @@ Options:
### `scripts compile`

```
Usage: genaiscript scripts compile [options]
Usage: genaiscript scripts compile [options] [folders...]

Compile all script in workspace

Choose a reason for hiding this comment

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

Description for 'compile' command is incomplete, missing the explanation for the new '[folders...]' argument.

generated by pr-docs-review-commit incorrect_description

.option(
"-rr, --run-retry <number>",
"number of retries for the entire run"
)

Choose a reason for hiding this comment

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

The function cli does not validate the input arguments. This could lead to unexpected behavior or errors if the function is called with invalid or unexpected arguments. It's a good practice to validate function arguments to ensure they match the expected format and type.

generated by pr-review-commit missing_argument_validation

@@ -205,6 +208,7 @@ export async function cli() {
scripts
.command("compile")
.description("Compile all script in workspace")
.argument("[folders...]", "Pattern to match files")
.action(compileScript)

Choose a reason for hiding this comment

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

The function cli does not validate the input arguments for the compile command. This could lead to unexpected behavior or errors if the function is called with invalid or unexpected arguments. It's a good practice to validate function arguments to ensure they match the expected format and type.

generated by pr-review-commit missing_argument_validation

@pelikhan pelikhan merged commit ae37058 into main Aug 22, 2024
10 checks passed
@pelikhan pelikhan deleted the blog-ideation branch August 22, 2024 22:02
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