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

migrate openai-asst-webpage-js to vite/esm #271

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

manekinekko
Copy link
Member

@manekinekko manekinekko commented Apr 16, 2024

  • remove webpack and add vite config
  • migrate to ESM imports
  • move onclick events from HTML to script.js
  • rename ai.png to favicon.png

@manekinekko manekinekko requested a review from robch as a code owner April 16, 2024 15:01
@manekinekko manekinekko force-pushed the openai-asst-webpage-js-vite-esm branch 2 times, most recently from 7a63686 to 541f7ea Compare April 16, 2024 15:04
Copy link
Contributor

@robch robch left a comment

Choose a reason for hiding this comment

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

Looks great!

@manekinekko manekinekko force-pushed the openai-asst-webpage-js-vite-esm branch from 541f7ea to 00435cd Compare April 22, 2024 15:56

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.";
Copy link
Contributor

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'"

Copy link
Member Author

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).

Copy link
Member Author

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 :)

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.

None yet

3 participants