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

configurable default model #544

Merged
merged 8 commits into from
Jun 14, 2024
Merged

configurable default model #544

merged 8 commits into from
Jun 14, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Jun 14, 2024

Environment variable to specify the default model for all scripts

  • πŸ”„ The default model for the GenAIScript has been changed from "gpt-4" to "openai:gpt-4".
  • πŸŽ›οΈ Introduced the ability to set the default model and temperature via environment variables (GENAISCRIPT_DEFAULT_MODEL and GENAISCRIPT_DEFAULT_TEMPERATURE).
  • 🎚️ The default temperature has been changed from 0.2 to 0.8.
  • πŸ› Multiple assertions have been added to ensure that the modelId is defined before it is used.
  • πŸ“š Documentation updates have been made to reflect these changes and provide additional context and usage examples.
  • 🧰 The NodeHost.install() function has been updated to be async, and now includes a call to parse default settings from the environmental variables.
  • βœ”οΈ Extra validation has been added in the githubCreatePullRequestReview function to check the distance between lines when comparing with existing comments.
  • βš™οΈ Several functions in the expandTemplate module have been updated to use the new default settings from the host.
  • ✨ The parseTokenFromEnv function now handles parsing the default model from the environment variables.
  • πŸ”§ The .env configuration handling has been improved, with additional checks and balances.

generated by pr-describe

@pelikhan
Copy link
Member Author

/genai-describe

@pelikhan
Copy link
Member Author

/genai-review

Copy link

  • The code changes seem to primarily focus on improving model configuration and handling in the application, with specific updates to handle default model configurations.
  • The code now handles configuration defaults for the model and temperature from environment variables, which increases the flexibility of the software.
  • The code also introduces changes to error handling for model selection. Now, if a model is not specified, the code asserts for it and falls back to the default model specified in the host.
  • The code includes improvements in the generation of the .env file template.
  • There are also updates to test configurations, and several modifications to adapt to the model changes.

However, there are a couple of areas which could be improved:

  1. In the cli.ts file, the async keyword is added to the preAction hook. However, it's unclear if the parent function that calls this hook can handle promises or async operations. If the parent function can't handle promises, this could lead to unhandled promise rejections.

  2. In the constants.ts file, the GITHUB_PULLREQUEST_REVIEW_COMMENT_LINE_DISTANCE constant is added but it's not clear how this constant is used. If this is an important configuration value, it might be better to put it into a configuration file or in environment variables.

Otherwise, LGTM πŸš€.

generated by pr-review

@pelikhan pelikhan merged commit c684581 into main Jun 14, 2024
9 checks passed
@pelikhan pelikhan deleted the config_default_model branch June 14, 2024 17:41
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