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.

Sign up for GitHub

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

Jump to bottom

streamlit chat ui #16

Merged
merged 21 commits into from
Mar 4, 2025
Merged

streamlit chat ui #16

merged 21 commits into from
Mar 4, 2025

Conversation

anunay999
Copy link
Owner

Issue #12

@anunay999 anunay999 mentioned this pull request Mar 1, 2025
Closed
@anunay999
Copy link
Owner Author

UI
Screenshot 2025-03-02 at 2 58 45 PM

@anunay999
Copy link
Owner Author

@nitinsurya draft pr, still few bugs to fix but working overall. Playaround with it and let me know

@anunay999
Copy link
Owner Author

anunay999 commented Mar 2, 2025 •
edited
Loading

This PR addresses the following:

  • UI integration
  • Generalizing LLM providers
  • Chromadb integration to store and query embeddings

Bugs that still need to be addressed with the changes:

  • Currently, it works with one file upload at a time but fails to upload and process all files simultaneously.
  • It has connectivity issues with Ollama for subsequent requests. I’ve added a retry mechanism for now, but I need to find a better solution.
  • I managed to eliminate Docker Compose and integrate everything into a Docker image itself.
  • I need to figure out how to access Ollama hosted on the local machine within the app without having to include Ollama in the Docker image. However, it works as expected when using the API.

@nitinsurya take a look and let me know

@anunay999 anunay999 requested a review from nitinsurya March 2, 2025 13:25
nitinsurya
nitinsurya reviewed Mar 4, 2025
View reviewed changes
src/enums/models.py
@property
def get_api_key_val(self) -> str:
"""Fetch API key dynamically from the correct environment variable."""
api_key = os.environ.get(self.provider.api_key_env_var)
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should move to pydantic settings. once you merge, can take it up.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yep, I'll create a issue and assign it to you

nitinsurya
nitinsurya reviewed Mar 4, 2025
View reviewed changes
src/enums/models.py
"""Enum mapping embedding models to their providers."""

# OpenAI Embeddings
TEXT_EMBEDDING_ADA_002 = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel we should separate out enums and EmbeddingModel, and mark EmbeddingModel as a class.

@nitinsurya
Copy link
Collaborator

Added some thoughts. Those can be followup tasks we can take up as well, but overall, this is a big bump 🎉 .
We should maybe enable some automated changelog setup.

@anunay999 anunay999 mentioned this pull request Mar 4, 2025
Closed
@anunay999 anunay999 linked an issue Mar 4, 2025 that may be closed by this pull request
UI integration #12
Closed
@anunay999 anunay999 merged commit 5fa7ed9 into main Mar 4, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nitinsurya nitinsurya nitinsurya left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UI integration
2 participants
@anunay999 @nitinsurya