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

Add webui App #30

Merged
merged 22 commits into from
Jul 8, 2024
Merged

Add webui App #30

merged 22 commits into from
Jul 8, 2024

Conversation

snekkenull
Copy link
Contributor

@snekkenull snekkenull commented Jun 29, 2024

Context

What is the purpose of this PR? Is it to

  • add a new feature
  • fix a bug
  • update tests and/or documentation
  • other (please add here)

Please link to any issues this PR addresses.
#23

Changelog

Add a gradio WebUI for translation-agent, run it locally or cloud.

  • Tokenized Text: Displays translated text with tokenization, highlighting differences between original and translated words.
  • Document Upload: Supports uploading various document formats (PDF, TXT, DOC, etc.) for translation.
  • Multiple API Support: Integrates with popular language models like:
    • Groq
    • OpenAI
    • Ollama
    • Together AI
      ...Any OpenAI compatible LLMs, just set the base url in UI.
  • Enable individual model for reflection: Now you can enable second Endpoint to use another LLM for reflection.
  • Export content: Export the final translation to .txt file.
  • Full document: A simple way to run it locally by README document.
  • Concise: Do not modify the source file, simply patch the completion function, not affect the source codes and prompts, convenient for maintaining updates.

Test plan

Please make sure to do each of the following if applicable to your PR. (If you're not sure about any one of these just ask and we will happily help.)

  • run pre-commit hooks and linters (make sure you've first installed via pre-commit install)
  • add unit tests for any new functionality
  • update docstrings for any new or updated methods or classes
  • run unit tests via pytest tests
  • include relevant commands and any other artifacts in this summary (eval results, etc.)

@j-dominguez9
Copy link
Collaborator

Making change suggestions as I go:

  • Can we stick to base api clients instead of llama-index wrappers, to keep the dependencies more stable and minimal? So, openai and cohere (ollama, groq, anyscale, and together.ai all have openai compatibility).
  • Have the requirements.txt dependencies be as an optional "group" in the root pyproject.toml file, so that it can be installed with poetry install --with app and remove the requirements.txt file.
  • remove the init.py files as it wouldn't act as a module; instead you could have the imports be "from patch import ____" on the app.py, which should work if it's in the same directory (let me know if there's issues with this)

@snekkenull
Copy link
Contributor Author

snekkenull commented Jul 6, 2024

@j-dominguez9
Thank you for your suggestion, it has been modified as you suggested.

  • Using base url, it is now possible to use the default openai, groq, together-ai, ollama templates, or customize OpenAI compatible endpoints in UI.
  • Use poetry install --with app for installation.
  • Simplified import.

Preview:
image

@j-dominguez9
Copy link
Collaborator

@snekkenull Thank you for your work on this and incorporating the changes. I went ahead and made minor formatting fixes to conform with python style. The only thing it's missing before merging and hoping you could help with this last detail is an error that comes out when you press the Clear button. Attached output below. Thanks!
Screenshot from 2024-07-08 10-38-17

@snekkenull
Copy link
Contributor Author

@j-dominguez9 Thank you for testing, I have benefited greatly from your code style and work.
I have fixed the issue (the problem was with a error on the ClearButton component's event logic).

@j-dominguez9
Copy link
Collaborator

@snekkenull of course! Thank you for your contribution. Note for self future reference: There is still an error if the user doesn't have an OPENAI_API_KEY in the .env file, even if they don't intend to use OpenAI. This however traces back to the utils file, which will be addressed soon, so I'll come back to this part afterwards to make sure it works without having an OpenAI key.

@j-dominguez9 j-dominguez9 merged commit 2f0c344 into andrewyng:main Jul 8, 2024
1 check passed
@methanet
Copy link
Collaborator

methanet commented Jul 12, 2024

@snekkenull many thanks for working on the app! One small issue I just noticed are several inconsistencies between the READMEs. Would you be able to take a stab at those?

  • Can you double check for consistency with the main README and edit as needed? For example, the app README says "This repository contains..." and has its own contributing guide - those can be removed/adjusted as this is now part of this repo.
  • Explicitly list all the APIs supported.
  • Also briefly describe the advanced options the UI supports.
  • I think the Linux and Window installation can be added to the main README. You can briefly describe what the '--with app' option is and link to the app directory.
    We appreciate your help!

@snekkenull
Copy link
Contributor Author

snekkenull commented Jul 12, 2024 via email

@snekkenull snekkenull mentioned this pull request Aug 4, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants