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

Feature/add agent content tool #1854

Merged
merged 14 commits into from
Jan 22, 2025
Merged

Conversation

emrgnt-cmplxty
Copy link
Contributor

@emrgnt-cmplxty emrgnt-cmplxty commented Jan 22, 2025

Important

This pull request adds agent content tools, refactors retrieval and RAG agents, updates configurations, and enhances database models, while also cleaning up deprecated code and updating tests.

  • Retrieval Client:
    • Added maxToolContextLength and tools parameters to RetrievalClient in retrieval.ts.
    • Updated RetrievalClient to handle new parameters in requests.
  • Configuration:
    • Replaced tool_names with tools in multiple TOML configuration files.
    • Updated r2r.toml to include content in tools.
  • Agent and RAG:
    • Introduced RAGAgentMixin for adding local search, web search, and content tools.
    • Refactored R2RRAGAgent and R2RStreamingRAGAgent to use RAGAgentMixin.
    • Added new methods for handling content and search in rag.py.
  • Database and Models:
    • Added total_tokens column to documents table in documents.py.
    • Updated migration scripts to handle total_tokens.
  • Testing:
    • Updated test cases in test_chunks.py and test_limits.py to reflect new functionality.
    • Added tests for collection ID filters in test_collection_id_filter.py.
  • Miscellaneous:
    • Removed deprecated pipes and pipelines.
    • Refactored services to remove unused dependencies and streamline logic.

This description was created by Ellipsis for 4f0b50c. It will automatically update as commits are pushed.

@emrgnt-cmplxty emrgnt-cmplxty marked this pull request as ready for review January 22, 2025 18:00
@emrgnt-cmplxty emrgnt-cmplxty merged commit 9d7fb9e into main Jan 22, 2025
3 of 4 checks passed
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 4f0b50c in 4 minutes and 44 seconds

More details
  • Looked at 12362 lines of code in 158 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. py/core/providers/database/documents.py:84
  • Draft comment:
    The logic for checking and adding the total_tokens column is overly complex. Consider simplifying the check for existing columns and streamline the logic for adding the column if it doesn't exist.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code is checking for the presence of a column in a database table, but the logic is overly complex and could be simplified. The check for existing columns and the subsequent logic could be streamlined.
2. py/core/providers/database/documents.py:92
  • Draft comment:
    Consider providing a more user-friendly error message when the 'total_tokens' column is missing but documents are present. This will help users understand the next steps more clearly.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code is checking for the presence of a 'total_tokens' column in the 'documents' table. If the column is missing, it checks if the table already has data. If data is present, it raises an error asking the user to run a migration. This is a good practice to ensure schema consistency, but the error message could be more user-friendly.
3. py/core/providers/database/documents.py:95
  • Draft comment:
    Consider providing a more user-friendly error message when the 'total_tokens' column is missing but documents exist. This will help users understand the next steps more clearly.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code is checking for the presence of the 'total_tokens' column in the documents table. If the column is missing, it checks if there are existing documents. If there are, it raises an error asking the user to run a migration. This is a good practice to ensure data integrity, but the error message could be more user-friendly.

Workflow ID: wflow_3k5aicvPmGjKCkp7


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

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.

1 participant