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

Avatar for Meta-Llama-3.1-405B-Instruct model fixed #712

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Avatar for Meta-Llama-3.1-405B-Instruct model fixed #712

merged 1 commit into from
Nov 6, 2024

Conversation

gitdevjin
Copy link
Contributor

@gitdevjin gitdevjin commented Nov 6, 2024

Description

This Fixes #704

The logoUrl() function in the ChatCraftModel class checks the model’s name to determine the correct logo URL. However, some Llama model names contain uppercase letters, causing the function to fail to retrieve the correct logo for these models.

This issue affected models like sambanova/Meta-Llama-3.1-405B-Instruct and sambanova/Meta-Llama-3.1-70B-Instruct, among others. This update ensures that model names are handled in a case-insensitive manner for models

How I tested

  • I built it using pnpm build and ran it with pnpm preview --host both for Desktop version(macbook m4) and Mobile version(iPhone 15pro).
  • I ran the test code with `pnpm run test', and all passed

Screenshot with Desktop Test

Meta-Llama-3.1-70B-Instruct

Screenshot 2024-11-06 at 2 35 13 PM

Meta-Llama-3.1-405B-Instruct

Screenshot 2024-11-06 at 2 35 38 PM

Screenshot with Mobile Test

IMG_0731

@@ -43,7 +43,7 @@ export class ChatCraftModel {
return "/google-gemini-logo.png";
}

if (this.name.includes("llama")) {
if (this.name.toLowerCase().includes("llama")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a good fix, but let's do it everywhere in the function. At the top of logoUrl() can you do:

const vendor = this.vendor.toLowerCase();
const modelName = this.name.toLowerCase();

Then change all the uses of this.name to use modelName

return "/microsoft-phi-logo.png"; // Microsoft's Phi model logo
}

if (vendor === "google") {
return "/google-gemini-logo.png";
}

if (this.name.includes("llama")) {
if (modelName.toLowerCase().includes("llama")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

modelName is already lowercase

Copy link
Contributor Author

@gitdevjin gitdevjin Nov 6, 2024

Choose a reason for hiding this comment

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

I am so sorry for making you check the code many times ...

@gitdevjin
Copy link
Contributor Author

@humphd, Sure 👍 I just updated and tested it the same way I did here

All model names are checked in in a case-insensitive manner for Avatar
@humphd
Copy link
Collaborator

humphd commented Nov 6, 2024

Make sure you do these changes on topic branches, not your main.

@humphd humphd merged commit b59ae3c into tarasglek:main Nov 6, 2024
3 checks passed
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.

Fix icon for Meta-Llama-3.1-405B-Instruct
2 participants