OpenModelDB / open-model-database

An open and free database for AI models
https://openmodeldb.info
GNU General Public License v3.0
308 stars 33 forks source link

Add ability to add models from the deployed site #407

Closed joeyballentine closed 6 months ago

joeyballentine commented 6 months ago

I'll admit this is a little jank, but it works. Please keep in mind that having this and it not being perfect is better than not having it at all.

This uses the existing unused MapCollection to store local changes to the site. I also wrapped it in a way that stores the maps in session storage, so that changes stay on refresh. This collection is used for the webApi when the site is deployed.

I realized after working on this for a while that it's kinda impossible to make a new page on an entirely statically generated site. I figure this is what you ran into as well and it's why this never got completed in the past.

The workaround I came up with was to just have a dummy page that is always present (so the route exists), that can be used to edit the model when deployed. While not perfect, this does get around the routing limitation and provides a blank slate to edit on.

I also added three new buttons at the bottom of the models page: "Load Model from clipboard", "Copy Model to Clipboard", and "Submit Model on GitHub". The first two are self explanatory, and I figure this way people can backup and restore their models as they are working on them. The third button opens a GitHub issue with the model's name in the title and the json in the body. While this obviously can't be auto converted to a PR, it will at least let people submit their models to us for someone to go in and make a PR. It'll be a lot easier for us to add models when we can just copy what other people submit.

I made it hide the "add model" button and the "edit mode" toggle for now. For now I figure this should be a mostly hidden feature that we don't advertise until we have a slightly better system. We can just link people the add-model page, and that page will take them to the dummy page, which has editMode overridden to be enabled. This means the only editable page when deployed is that dummy page.

Doesn't fully finish #254 as it does not generate a diff, but instead the full json (why do we need the diff when we can just overwrite the json? is that really necessary?)

RunDevelopment commented 6 months ago

Sorry to say it in such a harsh way, but in its current form, this PR is unusable.

Let's start by talking about why it is a terrible idea to enable edit mode for all users. Obviously, a random visitor just looking to download a model has no interest in editing that model. It must also be noted that the editing UI is pretty ugly, so we shouldn't just shove it into everyone's faces. This PR currently makes the UI worse for everyone. The irony here is that the local edit mode does have a way to turn off edit mode for UI elements.

We also don't want random people to spam our repo. Putting a "Submit model as GH issue" button under every model for every user to see is just inviting trouble.

Remember, the overarching goal of #254 isn't to have and edit mode on the live website by any mean necessary. The goal is to improve the UX for users that want to add/edit models. Right now, this PR barely improves the editing UX and makes the UX for everyone else worse. That's not acceptable.

So how do we fix this? Since the main problem is that the live website is always in edit mode, we need to add a way for users to opt in and out of it. By default, users should be in "view" mode (=what the website is right now). They can then perform some action to opt into edit mode (=what this PR or npm run dev is right now), and perform another action to opt out of edit mode.

As for what those action should be: I'm not sure. We could just have a big button "Let me edit" somewhere (maybe in the docs page for adding models). We could also have something like a fake login/sign up process, so all edits must be performed by a user. The former is easier to do, so we should probably do something like it for now.

I realized after working on this for a while that it's kinda impossible to make a new page on an entirely statically generated site.

The solution is to have a single dummy page and then use a URL parameter to switch between model. E.g. /models/dummy?id=2x-my-new-model. The tricky thing here isn't static pages, but getting the links right. Every link to a model now has to know whether it should point to /models/1x-foo or /models/dummy?id=1x-foo. This means that the entire website has to be actively aware of which models the user added. Not too difficult to do, but it requires some new abstractions.

joeyballentine commented 6 months ago

Whoops. Sorry, the intention was not to enable edit mode all the time but rather only for the dummy page. I seem to have unintentionally enabled it all the time due to the webapi now not being undefined. This is my bad and I should have regression tested other pages before submitting this. Apologies for wasting your time.

we need to add a way for users to opt in and out of it

Not really. My intention was for nobody to accidentally run into this, but rather for us to be able to link people to the add model page when they ask how to add a model. I don't want to have random users of the site seeing an "add model" button or an "edit" button and getting confused.

The other thing is that right now i dont think we need to do your last point about being able to edit models. i think limiting it to adding new ones is fine for now as it enables the most important kind of contributions for people who previously could not.

RunDevelopment commented 6 months ago

Doesn't work. image

What I did:

  1. Hard-code IS_DEPLOYED to true.
  2. npm run build
  3. Serve files from out/

Basically, I did what GH pages would do.

RunDevelopment commented 6 months ago

Since we won't have an easy PR review workflow for people that create GH issues, we should prevent them for creating issues for invalid models. Basically, we need to do a npm run validate-db before creating an issue. This should be relatively easy to add, I think.

joeyballentine commented 6 months ago

I just made it alert() all the errors for now. Was much easier than trying to make actual UI for it and I personally think it's fine for now

RunDevelopment commented 6 months ago

Doesn't work again. image

Steps to reproduce:

  1. Hard-code IS_DEPLOYED to true.
  2. Go to /add-model and add any model.
  3. See the above error.
joeyballentine commented 6 months ago

Fixed the issue. I made a mistake when making the new function for the session storage map.

RunDevelopment commented 6 months ago

Alright, one last remark: I think the new buttons you added (1) should only be present in deployed mode and (2) should be in the same row as the Delete Model button.