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

[DERCBOT-1314] Add documentsRequired Setting to BotRAGConfiguration #1814

Conversation

scezen
Copy link
Contributor

@scezen scezen commented Dec 23, 2024

Related Tickets

JIRA: [DERCBOT-1314]

Description

This PR introduces a new boolean attribute documentsRequired to the BotRAGConfiguration and integrates it across the system. The documentsRequired attribute determines whether documents are mandatory for processing the RAG chain.

Key Changes:

  1. Model Updates:

  2. Orchestrator Enhancements:

    • Updated RAGQuery to include documentsRequired as an optional parameter.
    • Modified __rag_guard in rag_chain.py to respect the documentsRequired setting.
  3. Bot API Modifications:

    • Updated RAGAnswerHandler to fetch the documentsRequired setting from the bot configuration and pass it to the RAG query.

Testing :

    def to_dict(self):
        return {
            'k': self.k,
            'filter': {'tag1': 'vector stores'},
        }
  • Then we are free to test if an answer is still provided while no retrieval of document is made.

@scezen scezen changed the title [DERCBOT-1314] Add documentsRequired Setting to BotRAGConfiguration for RAG Logic Enhancement [DERCBOT-1314] Add documentsRequired Setting to BotRAGConfiguration Dec 23, 2024
Copy link
Contributor

@assouktim assouktim left a comment

Choose a reason for hiding this comment

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

LGTM, just update the unit tests (python) and everything should be ready.

@scezen
Copy link
Contributor Author

scezen commented Jan 10, 2025

Tests added @assouktim

Copy link
Contributor

@assouktim assouktim left a comment

Choose a reason for hiding this comment

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

Thanks a lot.

Copy link
Member

@Benvii Benvii left a comment

Choose a reason for hiding this comment

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

Thanks good PR, I just found an edge case maybe we can argue about it @assouktim @scezen but it's also easy to handle it if we want.

@scezen
Copy link
Contributor Author

scezen commented Jan 16, 2025

Fixed the edge case, now handles missing documents without 'no_answer' configuration @Benvii

Copy link
Member

@Benvii Benvii left a comment

Choose a reason for hiding this comment

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

  • Remove the last condition see my comment.
  • Add a test without the 'no_answer' input to check that it works

Copy link
Member

@Benvii Benvii left a comment

Choose a reason for hiding this comment

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

Thanks for the last updates ok for me.

@Benvii Benvii merged commit f91bff6 into theopenconversationkit:master Jan 22, 2025
@Benvii Benvii deleted the feature/optional-rag-guard/DERCBOT-1314 branch January 22, 2025 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants