From b4759403ac62992b75c066e6443ae5b916935eb3 Mon Sep 17 00:00:00 2001 From: Chris Weaver <25087905+Weves@users.noreply.github.com> Date: Fri, 7 Jul 2023 09:56:01 -0700 Subject: [PATCH] Adjust slack bot (#144) * Add handling for cases where an answer is not found * Make danswer bot slightly more configurable * Don't respond to messages in thread + add better formatting for slack messages --- backend/danswer/configs/app_configs.py | 9 ++ backend/danswer/direct_qa/answer_question.py | 6 +- backend/danswer/direct_qa/llm.py | 5 +- backend/danswer/listeners/slack_listener.py | 148 +++++++++++-------- backend/danswer/search/semantic_search.py | 1 + backend/danswer/server/models.py | 1 + backend/requirements/default.txt | 1 + backend/requirements/dev.txt | 1 + 8 files changed, 110 insertions(+), 62 deletions(-) diff --git a/backend/danswer/configs/app_configs.py b/backend/danswer/configs/app_configs.py index 991d27ad2d2c..08d43d813137 100644 --- a/backend/danswer/configs/app_configs.py +++ b/backend/danswer/configs/app_configs.py @@ -136,3 +136,12 @@ DYNAMIC_CONFIG_STORE = os.environ.get( "DYNAMIC_CONFIG_STORE", "FileSystemBackedDynamicConfigStore" ) DYNAMIC_CONFIG_DIR_PATH = os.environ.get("DYNAMIC_CONFIG_DIR_PATH", "/home/storage") + + +##### +# Danswer Slack Bot Configs +##### +DANSWER_BOT_NUM_DOCS_TO_DISPLAY = int( + os.environ.get("DANSWER_BOT_NUM_DOCS_TO_DISPLAY", "5") +) +DANSWER_BOT_NUM_RETRIES = int(os.environ.get("DANSWER_BOT_NUM_RETRIES", "5")) diff --git a/backend/danswer/direct_qa/answer_question.py b/backend/danswer/direct_qa/answer_question.py index d4826f14a9ee..06788a27239e 100644 --- a/backend/danswer/direct_qa/answer_question.py +++ b/backend/danswer/direct_qa/answer_question.py @@ -19,7 +19,9 @@ from danswer.utils.logging import setup_logger logger = setup_logger() -def answer_question(question: QuestionRequest, user: User | None) -> QAResponse: +def answer_question( + question: QuestionRequest, user: User | None, qa_model_timeout: int = QA_TIMEOUT +) -> QAResponse: start_time = time.time() query = question.query @@ -53,7 +55,7 @@ def answer_question(question: QuestionRequest, user: User | None) -> QAResponse: predicted_search=predicted_search, ) - qa_model = get_default_backend_qa_model(timeout=QA_TIMEOUT) + qa_model = get_default_backend_qa_model(timeout=qa_model_timeout) chunk_offset = offset_count * NUM_GENERATIVE_AI_INPUT_DOCS if chunk_offset >= len(ranked_chunks): raise ValueError("Chunks offset too large, should not retry this many times") diff --git a/backend/danswer/direct_qa/llm.py b/backend/danswer/direct_qa/llm.py index f873b71bd6d2..1e7db8d3faf0 100644 --- a/backend/danswer/direct_qa/llm.py +++ b/backend/danswer/direct_qa/llm.py @@ -177,8 +177,11 @@ def process_answer( answer_raw: str, chunks: list[InferenceChunk] ) -> tuple[str | None, dict[str, dict[str, str | int | None]] | None]: answer, quote_strings = separate_answer_quotes(answer_raw) - if not answer or not quote_strings: + if answer == UNCERTAINTY_PAT or not answer: return None, None + + if not quote_strings: + return answer, None quotes_dict = match_quotes_to_docs(quote_strings, chunks) return answer, quotes_dict diff --git a/backend/danswer/listeners/slack_listener.py b/backend/danswer/listeners/slack_listener.py index c07744704c50..7a940f4d6203 100644 --- a/backend/danswer/listeners/slack_listener.py +++ b/backend/danswer/listeners/slack_listener.py @@ -1,12 +1,16 @@ import os +from danswer.configs.app_configs import DANSWER_BOT_NUM_DOCS_TO_DISPLAY +from danswer.configs.app_configs import DANSWER_BOT_NUM_RETRIES from danswer.configs.app_configs import QDRANT_DEFAULT_COLLECTION +from danswer.configs.constants import DocumentSource from danswer.connectors.slack.utils import make_slack_api_rate_limited from danswer.direct_qa.answer_question import answer_question from danswer.server.models import QAResponse from danswer.server.models import QuestionRequest from danswer.server.models import SearchDoc from danswer.utils.logging import setup_logger +from retry import retry from slack_sdk import WebClient from slack_sdk.socket_mode import SocketModeClient from slack_sdk.socket_mode.request import SocketModeRequest @@ -14,9 +18,6 @@ from slack_sdk.socket_mode.response import SocketModeResponse logger = setup_logger() -_NUM_RETRIES = 3 -_NUM_DOCS_TO_DISPLAY = 5 - def _get_socket_client() -> SocketModeClient: # For more info on how to set this up, checkout the docs: @@ -34,6 +35,28 @@ def _get_socket_client() -> SocketModeClient: ) +_MAX_BLURB_LEN = 25 + + +def _build_custom_semantic_identifier( + semantic_identifier: str, blurb: str, source: str +) -> str: + """ + On slack, since we just show the semantic identifier rather than semantic + blurb, we need + to do some custom formatting to make sure the semantic identifier is unique and meaningful. + """ + if source == DocumentSource.SLACK.value: + truncated_blurb = ( + f"{blurb[:_MAX_BLURB_LEN]}..." if len(blurb) > _MAX_BLURB_LEN else blurb + ) + if truncated_blurb: + return f"#{semantic_identifier}: {truncated_blurb}" + else: + return f"#{semantic_identifier}" + + return semantic_identifier + + def _process_quotes( quotes: dict[str, dict[str, str | int | None]] | None ) -> tuple[str | None, list[str]]: @@ -43,11 +66,17 @@ def _process_quotes( quote_lines: list[str] = [] doc_identifiers: list[str] = [] for quote_dict in quotes.values(): + doc_id = str(quote_dict.get("document_id", "")) doc_link = quote_dict.get("link") doc_name = quote_dict.get("semantic_identifier") - if doc_link and doc_name and doc_name not in doc_identifiers: - doc_identifiers.append(str(doc_name)) - quote_lines.append(f"- <{doc_link}|{doc_name}>") + if doc_link and doc_name and doc_id and doc_id not in doc_identifiers: + doc_identifiers.append(str(doc_id)) + custom_semantic_identifier = _build_custom_semantic_identifier( + semantic_identifier=doc_name, + blurb=quote_dict.get("blurb", ""), + source=quote_dict.get("source_type", ""), + ) + quote_lines.append(f"- <{doc_link}|{custom_semantic_identifier}>") if not quote_lines: return None, [] @@ -56,25 +85,30 @@ def _process_quotes( def _process_documents( - documents: list[SearchDoc] | None, already_displayed_doc_identifiers: list[str] + documents: list[SearchDoc] | None, + already_displayed_doc_identifiers: list[str], + num_docs_to_display: int = DANSWER_BOT_NUM_DOCS_TO_DISPLAY, ) -> str | None: if not documents: return None seen_docs_identifiers = set(already_displayed_doc_identifiers) - top_documents: list[SearchDoc] = [] + top_document_lines: list[str] = [] for d in documents: - if d.semantic_identifier in seen_docs_identifiers: + if d.document_id in seen_docs_identifiers: continue - seen_docs_identifiers.add(d.semantic_identifier) - top_documents.append(d) - if len(top_documents) >= _NUM_DOCS_TO_DISPLAY: + seen_docs_identifiers.add(d.document_id) + + custom_semantic_identifier = _build_custom_semantic_identifier( + semantic_identifier=d.semantic_identifier, + blurb=d.blurb, + source=d.source_type, + ) + top_document_lines.append(f"- <{d.link}|{custom_semantic_identifier}>") + if len(top_document_lines) >= num_docs_to_display: break - top_documents_str = "\n".join( - [f"- <{d.link}|{d.semantic_identifier}>" for d in top_documents] - ) - return "*Other potentially relevant documents:*\n" + top_documents_str + return "\n".join(top_document_lines) def process_slack_event(client: SocketModeClient, req: SocketModeRequest) -> None: @@ -100,8 +134,13 @@ def process_slack_event(client: SocketModeClient, req: SocketModeRequest) -> Non logger.info("Ignoring message from bot") return + message_ts = req.payload.get("event", {}).get("ts") + thread_ts = req.payload.get("event", {}).get("thread_ts") + if thread_ts and message_ts != thread_ts: + logger.info("Skipping message since it is not the root of a thread") + return + msg = req.payload.get("event", {}).get("text") - thread_ts = req.payload.get("event", {}).get("ts") if not msg: logger.error("Unable to process empty message") return @@ -109,54 +148,52 @@ def process_slack_event(client: SocketModeClient, req: SocketModeRequest) -> Non # TODO: message should be enqueued and processed elsewhere, # but doing it here for now for simplicity - def _get_answer(question: QuestionRequest) -> QAResponse | None: - try: - answer = answer_question(question=question, user=None) - if not answer.error_msg: - return answer - else: - raise RuntimeError(answer.error_msg) - except Exception as e: - logger.error(f"Unable to process message: {e}") - return None + @retry(tries=DANSWER_BOT_NUM_RETRIES, delay=0.25, backoff=2, logger=logger) + def _get_answer(question: QuestionRequest) -> QAResponse: + answer = answer_question(question=question, user=None) + if not answer.error_msg: + return answer + else: + raise RuntimeError(answer.error_msg) answer = None - for _ in range(_NUM_RETRIES): + try: answer = _get_answer( QuestionRequest( query=req.payload.get("event", {}).get("text"), collection=QDRANT_DEFAULT_COLLECTION, - use_keyword=False, + use_keyword=None, filters=None, offset=None, ) ) - if answer: - break - - if not answer: - logger.error( - f"Unable to process message - did not successfully answer in {_NUM_RETRIES} attempts" + except Exception: + logger.exception( + f"Unable to process message - did not successfully answer in {DANSWER_BOT_NUM_RETRIES} attempts" ) return - if not answer.answer: - logger.error(f"Unable to process message - no answer found") - return - # convert raw response into "nicely" formatted Slack message quote_str, doc_identifiers = _process_quotes(answer.quotes) top_documents_str = _process_documents(answer.top_ranked_docs, doc_identifiers) - if quote_str: - text = f"{answer.answer}\n\n*Sources:*\n{quote_str}\n\n{top_documents_str}" - else: - text = f"{answer.answer}\n\n*Warning*: no sources were quoted for this answer, so it may be unreliable 😔\n\n{top_documents_str}" + if not answer.answer: + text = f"Sorry, I was unable to find an answer, but I did find some potentially relevant docs 🤓\n\n{top_documents_str}" + else: + top_documents_str_with_header = ( + f"*Other potentially relevant docs:*\n{top_documents_str}" + ) + if quote_str: + text = f"{answer.answer}\n\n*Sources:*\n{quote_str}\n\n{top_documents_str_with_header}" + else: + text = f"{answer.answer}\n\n*Warning*: no sources were quoted for this answer, so it may be unreliable 😔\n\n{top_documents_str_with_header}" + + @retry(tries=DANSWER_BOT_NUM_RETRIES, delay=0.25, backoff=2, logger=logger) def _respond_in_thread( channel: str, text: str, thread_ts: str, - ) -> str | None: + ) -> None: slack_call = make_slack_api_rate_limited(client.web_client.chat_postMessage) response = slack_call( channel=channel, @@ -164,25 +201,18 @@ def process_slack_event(client: SocketModeClient, req: SocketModeRequest) -> Non thread_ts=thread_ts, ) if not response.get("ok"): - return f"Unable to post message: {response}" - return None + raise RuntimeError(f"Unable to post message: {response}") - successfully_answered = False - for _ in range(_NUM_RETRIES): - error_msg = _respond_in_thread( + try: + _respond_in_thread( channel=req.payload.get("event", {}).get("channel"), text=text, - thread_ts=thread_ts, + thread_ts=thread_ts + or message_ts, # pick the root of the thread (if a thread exists) ) - if error_msg: - logger.error(error_msg) - else: - successfully_answered = True - break - - if not successfully_answered: - logger.error( - f"Unable to process message - could not respond in slack in {_NUM_RETRIES} attempts" + except Exception: + logger.exception( + f"Unable to process message - could not respond in slack in {DANSWER_BOT_NUM_RETRIES} attempts" ) return diff --git a/backend/danswer/search/semantic_search.py b/backend/danswer/search/semantic_search.py index 8bc2486fc881..d598d20a55a0 100644 --- a/backend/danswer/search/semantic_search.py +++ b/backend/danswer/search/semantic_search.py @@ -27,6 +27,7 @@ def chunks_to_search_docs(chunks: list[InferenceChunk] | None) -> list[SearchDoc search_docs = ( [ SearchDoc( + document_id=chunk.document_id, semantic_identifier=chunk.semantic_identifier, link=chunk.source_links.get(0) if chunk.source_links else None, blurb=chunk.blurb, diff --git a/backend/danswer/server/models.py b/backend/danswer/server/models.py index 28ec285e52f4..2e0941f2258a 100644 --- a/backend/danswer/server/models.py +++ b/backend/danswer/server/models.py @@ -80,6 +80,7 @@ class UserRoleResponse(BaseModel): class SearchDoc(BaseModel): + document_id: str semantic_identifier: str link: str | None blurb: str diff --git a/backend/requirements/default.txt b/backend/requirements/default.txt index b32ebbedd694..34d19f7972db 100644 --- a/backend/requirements/default.txt +++ b/backend/requirements/default.txt @@ -27,6 +27,7 @@ pytest-playwright==0.3.2 python-multipart==0.0.6 qdrant-client==1.2.0 requests==2.31.0 +retry==0.9.2 rfc3986==1.5.0 sentence-transformers==2.2.2 slack-sdk==3.20.2 diff --git a/backend/requirements/dev.txt b/backend/requirements/dev.txt index 56f589205cf0..e1e67f1ebc2b 100644 --- a/backend/requirements/dev.txt +++ b/backend/requirements/dev.txt @@ -9,4 +9,5 @@ types-psycopg2==2.9.21.10 types-python-dateutil==2.8.19.13 types-regex==2023.3.23.1 types-requests==2.28.11.17 +types-retry==0.9.9.3 types-urllib3==1.26.25.11