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

Make DocQA a one-clickable app implementation #151

Merged
merged 27 commits into from
Feb 27, 2025
Merged

Make DocQA a one-clickable app implementation #151

merged 27 commits into from
Feb 27, 2025

Conversation

wukaixingxp
Copy link
Contributor

@wukaixingxp wukaixingxp commented Jan 10, 2025

What does this PR do?

This PR adds a one-clickable app implementation of DocQA example, where user can choose local provider like Ollama or cloud provider like together or fireworks to do RAG. It allows users to install the DocQA app into the Application folder from the DocQA.dmg file. This MacQA app leverages llama-stack new LlamaStackAsLibraryClient feature and make every agentic component in-line. This PR also explains the steps for building this app.

This DocQA app will replace the existing docker solution as the docker solution is not easy to user and maintain.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

Thanks for contributing 🎉!

@wukaixingxp wukaixingxp self-assigned this Jan 10, 2025
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jan 10, 2025
@HamidShojanazeri
Copy link
Contributor

@wukaixingxp I wonder if there is a chance to consolidate app.py and MacA.py?

Co-authored-by: Hamid Shojanazeri <[email protected]>
@wukaixingxp
Copy link
Contributor Author

wukaixingxp commented Jan 13, 2025

@wukaixingxp I wonder if there is a chance to consolidate app.py and MacA.py?

Sure, there is a different behavior regarding how to launch Ollama between MacQA.py and app.py, maybe I can have a system env "USE_DOCKER" to control that.

Update on Jan 13: The merge has been completed in f610415. Please take a look

@wukaixingxp wukaixingxp changed the title Added MacQA, a one-clickable app implementation to DocQA Added MacQA, a one-clickable app implementation to DocQA(DO_NOT_REVIEW, waiting for 0.1) Jan 15, 2025
@wukaixingxp wukaixingxp changed the title Added MacQA, a one-clickable app implementation to DocQA(DO_NOT_REVIEW, waiting for 0.1) Added MacQA, a one-clickable app implementation to DocQA Jan 24, 2025
@wukaixingxp
Copy link
Contributor Author

@ashwinb I updated the MacQA app to work with v0.1 and this PR is ready for review. Can you take a look at this PR and any feedback is greatly appreciated! CC: @HamidShojanazeri @raghotham

@ashwinb ashwinb requested a review from ehhuang January 29, 2025 19:19
@ashwinb
Copy link
Contributor

ashwinb commented Jan 29, 2025

@hardikjshah @ehhuang It would be good to have your feedback here. I will take a look at this in a bit too (today.)

@wukaixingxp
Copy link
Contributor Author

@ashwinb @hardikjshah Gentle reminder on this.. given that this PR has been here for more than a month now.. Thanks! Any feedback is welcomed.

@wukaixingxp
Copy link
Contributor Author

@HamidShojanazeri I talked to @raghotham last week for this PR review. The idea is to make things simpler so that user can follow, thus we will remove the DockerQA as it is not very easy to use and maintain. Also the UI has been changed from using 'localhost in browser to a python GUI. Here is the modified one-clickable app, please take a look and let me know what do you think.

@wukaixingxp wukaixingxp changed the title Added MacQA, a one-clickable app implementation to DocQA Make DocQA a one-clickable app implementation Feb 19, 2025
@wukaixingxp
Copy link
Contributor Author

Besides the comment from @hardikjshah, there are things need to address: 1. Add Ollama pull then ollama run
2. Missed first word on output display
3. Waiting for this PR to be merged so that we do not do retrieval everytime.
4. Spec file is missing

@wukaixingxp
Copy link
Contributor Author

wukaixingxp commented Feb 25, 2025

Besides the comment from @hardikjshah, there are things need to address: 1. Add Ollama pull then ollama run 2. Missed first word on output display 3. Waiting for this PR to be merged so that we do not do retrieval everytime. 4. Spec file is missing

Now (1) (2) (4) has been fixed, but this PR was not merged in v0.1.4. also v0.1.4 has some small problems: meta-llama/llama-stack#1255, so we need to switch back to v0.1.3. @hardikjshah Please take another look at this PR, thanks!

@wukaixingxp
Copy link
Contributor Author

@hardikjshah I noticed the this PR has been merged, but given the comment here, I do not think Ollama 1B & 3B can work with builtin::rag::knowledge_search. I would like to just get my PR work with v0.1.3 for now and be able to merged, then we can upgrade this app later to support builtin::rag::knowledge_search

@wukaixingxp
Copy link
Contributor Author

From my test: 3.3 70B can handle the tool_call well
Screenshot 2025-02-26 at 3 44 31 PM

but 3.1B ollama failed:

Screenshot 2025-02-26 at 3 57 28 PM

Copy link
Contributor

@hardikjshah hardikjshah left a comment

Choose a reason for hiding this comment

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

leaving a few comments but looks good to go. Going to approve to get it out

@wukaixingxp wukaixingxp merged commit 994d8b4 into main Feb 27, 2025
1 check passed
@wukaixingxp wukaixingxp deleted the docqav2 branch February 27, 2025 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants