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

bug: Mismatch ID between model.json and folder path #3476

Closed
4 tasks done
imtuyethan opened this issue Aug 27, 2024 · 7 comments
Closed
4 tasks done

bug: Mismatch ID between model.json and folder path #3476

imtuyethan opened this issue Aug 27, 2024 · 7 comments
Assignees
Labels
P1: important Important feature / fix type: bug Something isn't working

Comments

@imtuyethan
Copy link
Contributor

imtuyethan commented Aug 27, 2024

  • I have searched the existing issues

Current behavior

Manually added models are broken due to a mismatched ID between the model.json file and the folder path. The model_id in the JSON file doesn't match the folder name where the model is stored, causing issues with model recognition and functionality.

Minimum reproduction step

  1. Manually add a model to Jan
  2. Create a model.json file with an "id" that doesn't match the folder name
  3. Place the model.json file in a folder with a different name
  4. Attempt to use the manually added model in Jan

Expected behavior

Jan should consistently recognize and use manually added models, regardless of minor discrepancies between the model.json "id" field and the folder name. Ideally, Jan should use the folder name as the authoritative model_id to ensure consistency.

Screenshots / Logs

Screenshot 2024-08-27 at 9 40 07 PM

Jan version

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Environment details

@imtuyethan imtuyethan added type: bug Something isn't working P1: important Important feature / fix labels Aug 27, 2024
@imtuyethan imtuyethan changed the title bug: bug: Mismatch ID between model.json and folder path Aug 27, 2024
@louis-jan louis-jan removed their assignment Aug 28, 2024
@louis-jan
Copy link
Contributor

There is an edge case where a customized OpenAI-compatible remote model could have a / in its model ID. So auto generated ID would not work for the case unless users create nested folder to correct this. (Which is not a good workaround)

@louis-jan
Copy link
Contributor

Ref: https://discord.com/channels/1107178041848909847/1110389363809976361/1278650955214229534

@dan-homebrew
Copy link
Contributor

@louis-jan @marknguyen1302 I think Model Folder detection and maintenance is going to important for Jan + Cortex to get right. We currently have 3 main model sources:

Nitro

Cortex

  • Cortex Models: tag-based variants
  • Huggingface Models
  • ...

What is our current logic that we currently use for Model detection, in the Model Folder? We should be really careful - I probably screwed up asking for folder names to be part of the design, and this will break in all sorts of ways cross-OS.

@dan-homebrew
Copy link
Contributor

dan-homebrew commented Aug 29, 2024

@louis-jan @marknguyen1302 Additionally, to prepare for our upcoming Cortex migration, can we write test coverage for our existing Model Folder (i.e. all the variants we have, local, remote, etc).

I think we need to make sure we don't break for existing users, and whatever patchwork of assumptions, edge cases are accounted for, before we make changes.

We will likely need to do extensive Upgrade Testing, i.e. to make sure we don't break existing functionality for users.

Jan is pretty much a single feature tool now: i.e. download models to our folder structure and run them. Investing in test coverage for this critical functionality will give us a stable base to build off in the longer term

@dan-homebrew
Copy link
Contributor

@louis-jan @imtuyethan @urmauur Quick check: will we do a hotfix for this in v0.5.4?

@louis-jan
Copy link
Contributor

Yeah, we should right? There still a long way to do a full cortex.cpp migration. I can take this.

@imtuyethan imtuyethan added this to the v0.5.4 milestone Sep 11, 2024
@louis-jan
Copy link
Contributor

louis-jan commented Sep 12, 2024

After investigation, both the model's ID generation and retrieving a folder from the model ID would not work and pose a high risk of breaking.

One of the better fixes is to work with ModelFile (a type alias of Model & File), where:

  • It contains Model metadata, which is used widely in the app.
  • It contains File metadata, which is used for file path retrieval.

There is no need to constrain the model ID and the JSON file path

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1: important Important feature / fix type: bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

6 participants
@Van-QA @imtuyethan @dan-homebrew @louis-jan @marknguyen1302 and others