-
Notifications
You must be signed in to change notification settings - Fork 14
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
migrate openai-asst-webpage-js to vite/esm #271
base: main
Are you sure you want to change the base?
migrate openai-asst-webpage-js to vite/esm #271
Conversation
manekinekko
commented
Apr 16, 2024
•
edited
Loading
edited
- remove webpack and add vite config
- migrate to ESM imports
- move onclick events from HTML to script.js
- rename ai.png to favicon.png
7a63686
to
541f7ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
541f7ea
to
00435cd
Compare
|
||
static AZURE_OPENAI_SYSTEM_PROMPT = process.env.AZURE_OPENAI_SYSTEM_PROMPT ?? "<#= AZURE_OPENAI_SYSTEM_PROMPT #>"; | ||
static AZURE_OPENAI_SYSTEM_PROMPT = import.meta.env.AZURE_OPENAI_SYSTEM_PROMPT ?? "You are a helpful AI assistant."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all the other variables fail if not set, just double checking that this is what we want as a default here. Not sure what a "failure" system prompt would be unless we set it to something like "no matter what I say respond with 'you have not set your system prompt'"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of propagating the error message to the user's UI so they can get instant feedback (vs having to switch back to terminal console or open browser console).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I just noticed that this is currently the case. Missing env vars are error message is shown to users in the UI. So ignore my previous comment :)