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

github models support #600

Merged
merged 5 commits into from
Aug 2, 2024
Merged

github models support #600

merged 5 commits into from
Aug 2, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Aug 2, 2024

Based on the GIT_DIFF:

  • A new model provider from GitHub has been introduced 👍. This aims to allow the application to interact with GitHub models and make use of the functionalities provided by GitHub Marketplace.
  • New constants added for GitHub model provider and GitHub Models base ✨.
  • New environment variable GITHUB_TOKEN for GitHub models has been added in the parseTokenFromEnv function. This is used to establish a valid connection with the GitHub models.
  • Documentation links (DOCS_CONFIGURATION_GITHUB_URL) and instructions for use of GitHub Models are added in the configuration in the connection module 📚.
  • In the models test file, a new test case for github:gpt4 has been added to ensure the newly added support for GitHub models works correctly with GPT-4 model 🧪.
  • The VS Code extension part of the project has been updated to reflect the new introduction of GitHub Models functionality. This would provide users the ability to use GitHub Models while using the extension.

Please note that these changes do not have any user facing updates in the packages/core/src/prompt_template.d.ts or packages/core/src/prompt_type.ts files.

generated by pr-describe

packages/core/src/connection.ts Dismissed Show dismissed Hide dismissed
Copy link

github-actions bot commented Aug 2, 2024

The changes look mostly good. The developer has added GitHub as a new type of model provider alongside existing providers like Azure, OpenAI, etc. This includes adding the necessary configuration options, extending the parsing function to handle this new provider type, and adding related constants.

However, I have a couple of concerns:

  1. ❌ In the connection.ts file, an environment variable GITHUB_TOKEN is being used without any null check. This could lead to a runtime error if it's not set. Make sure to handle this scenario.

  2. ❌ In constants.ts, the GITHUB_MODELS_BASE is pointing to https://models.inference.ai.azure.com. This seems to be incorrect as it should point to a GitHub-related URL.

Suggested changes:

diff --git a/packages/core/src/connection.ts b/packages/core/src/connection.ts
index 7f1f6ecf..7f1f6ecf 100644
--- a/packages/core/src/connection.ts
+++ b/packages/core/src/connection.ts
@@ -102,7 +102,12 @@ export async function parseTokenFromEnv(
 
     if (provider === MODEL_PROVIDER_GITHUB) {
-        const token = env.GITHUB_TOKEN
+        const token = env.GITHUB_TOKEN;
+        if (!token) {
+          console.warn("GITHUB_TOKEN is not set. Please set it in your environment variables.");
+          return;
+        }
+        
         if (!token) throw new Error("GITHUB_TOKEN must be set")
         const type = "openai"
         const base = GITHUB_MODELS_BASE


diff --git a/packages/core/src/constants.ts b/packages/core/src/constants.ts
index 6b403147..6b403147 100644
--- a/packages/core/src/constants.ts
+++ b/packages/core/src/constants.ts
@@ -98,7 +98,7 @@ export const TRACE_FILE_PREVIEW_MAX_LENGTH = 240
 
-export const GITHUB_MODELS_BASE = "https://models.inference.ai.azure.com";
+export const GITHUB_MODELS_BASE = "https://models.github.com";  // Assuming this as the base url for github models.

Overall, with these changes, it should be good to go. LGTM 🚀 with the mentioned fixes.

generated by pr-review

GITHUB_TOKEN="${PLACEHOLDER_API_KEY}"
`,
model: `${MODEL_PROVIDER_GITHUB}:gpt-4o`,
}
Copy link

Choose a reason for hiding this comment

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

The string GITHUB_TOKEN="${PLACEHOLDER_API_KEY}" is hardcoded. This could lead to issues if the token needs to be changed or updated. Consider using a constant or a configuration file to store such values. 🔄

generated by pr-review-commit hardcoded_string


The [GitHub Models](https://github.com/marketplace/models) provider, `github`, allows running models through the GitHub Marketplace.
This provider is useful for prototyping and subject to [rate limits](https://docs.github.com/en/github-models/prototyping-with-ai-models#rate-limits)
depending on your subscription.
Copy link

Choose a reason for hiding this comment

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

The link provided for "GitHub Models" does not have a URL, it should point to the actual resource.

generated by pr-docs-review-commit broken_link


<li>

Click **Get Started** and follow the instructions to configure the github token, or start a codespace!
Copy link

Choose a reason for hiding this comment

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

The instruction "Click Get Started and follow the instructions to configure the github token, or start a codespace!" is ambiguous. It should clarify whether configuring the GitHub token and starting a codespace are two separate options or part of the same process.

generated by pr-docs-review-commit ambiguous_instruction


```js "Phi-3-mini-4k-instruct"
const modelName = "Phi-3-mini-4k-instruct";
```
Copy link

Choose a reason for hiding this comment

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

The code block is incorrectly annotated with "js" and a string "Phi-3-mini-4k-instruct" which seems to be a mistake. The code block should only specify the language (js) without the model name string.

generated by pr-docs-review-commit incorrect_code_fence

@@ -269,7 +314,7 @@ script({

</Steps>

## GitHub Copilot Models
## GitHub Copilot in Visual Studio Code
Copy link

Choose a reason for hiding this comment

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

The section title "GitHub Copilot Models" has been changed to "GitHub Copilot in Visual Studio Code" which might imply a change in content scope or focus. Ensure that the content of the section aligns with the new title.

generated by pr-docs-review-commit section_renaming

@pelikhan pelikhan merged commit b73b2e0 into main Aug 2, 2024
5 of 8 checks passed
@pelikhan pelikhan deleted the githubmodels branch August 2, 2024 14:45
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