Fs 80/use https for webagent search #21

gaganahluwalia · 2024-11-11T09:08:41Z

Description

This change is to make sure we are only using 'https' websites for our web search and scraping.
Also made a small change the way Web Search was being used after the usual LLM based on the LLM's answer.

Changelog

Mainly the Web_Agent.py and Web_utils.py

evpearce · 2024-11-11T11:29:54Z

backend/tests/websockets/user_confirmer_test.py

@@ -1,73 +1,75 @@
-import logging
-from unittest.mock import Mock, patch
+# import logging


If these tests aren't needed any more then they should be deleted and not just commented out.

Tests deleted

evpearce · 2024-11-11T11:33:06Z

backend/src/prompts/templates/answer-user-ques.j2

-2. **Validity Check**: The LLM checks if its generated answer is complete and correct. This could be based on factual accuracy, coverage of the query, or relevance to the user's question.
-3. **Validation Reason**: The LLM explains why the answer is valid or invalid.
+2. **Search More**: If a more general web search using a web search engine is required then indicate that, it should set this field to true.
+4. **Validation Reason**: The LLM explains why the answer is valid or invalid.


Should this now be named something like "search more reason" and should the text be changed to address the changes on line 28

Changed it to search_more_reason

evpearce · 2024-11-11T11:33:25Z

backend/src/prompts/templates/answer-user-ques.j2

-2. **Validity Check**: The LLM checks if its generated answer is complete and correct. This could be based on factual accuracy, coverage of the query, or relevance to the user's question.
-3. **Validation Reason**: The LLM explains why the answer is valid or invalid.
+2. **Search More**: If a more general web search using a web search engine is required then indicate that, it should set this field to true.
+4. **Validation Reason**: The LLM explains why the answer is valid or invalid.


why has this point changed to 4. ?

changed to 3

IMladjenovic · 2024-11-11T16:51:41Z

backend/src/utils/web_utils.py

    try:
-        for url in search(search_query, num_results=num_results):
-            urls.append(url)
+        https_urls = [str(url) for url in search(search_query, num_results=num_results) if str(url).startswith("https")]


Is it possible to test this by mocking out search library?

IMladjenovic · 2024-11-11T16:57:42Z

backend/src/prompts/templates/answer-user-ques.j2

 ### **Explanation:**

 1. **Answer**: The LLM generates an answer based on the user’s question and the provided content.
-2. **Validity Check**: The LLM checks if its generated answer is complete and correct. This could be based on factual accuracy, coverage of the query, or relevance to the user's question.
-3. **Validation Reason**: The LLM explains why the answer is valid or invalid.
+2. **Search More**: If a more general web search using a web search engine is required then indicate that, it should set this field to true.


Can this be renamed "should perform web search"?

Am I correct in understanding that when we trigger a web_general_search from the WebAgent we're getting to this prompt first and attempting to answer the question using the underlying models training data, and if it doesn't have info we then perform a web search?

IMladjenovic · 2024-11-11T16:58:34Z

backend/src/prompts/templates/answer-user-ques.j2

@@ -9,23 +9,24 @@ User's question is:
 Once you generate an answer:


Can this file (and all references) be renamed to "answer-user-question"

(Sorry to add some housekeeping!)

IMladjenovic · 2024-11-11T17:02:07Z

backend/src/agents/web_agent.py

-        valid_answer = json.loads(answer_result["response"]).get("is_valid", "")
-        if valid_answer:
+        search_more = json.loads(answer_result["response"]).get("search_more", "")
+        if not search_more:


For a bit of extra safety, can we also update perform_scrape method in this class to verify that the urls it has been given are https? This method would be a good one to add tests for as well

IMladjenovic · 2024-11-13T09:43:18Z

backend/tests/agents/web_agent_test.py

@@ -81,3 +82,35 @@ async def test_web_general_search_core_invalid_summary(
    }
    assert json.loads(result) == expected_response

+@pytest.mark.asyncio
+@patch("src.utils.web_utils.search")
+async def test_https_urls(mock_search):


mic-smith · 2024-11-13T12:59:35Z

backend/tests/BDD/step_defs/test_prompts.py

-
-            assert result["value"] == "Y", (
+    # Allow `expected_response` to be a list of possible valid responses
+    possible_responses = [resp.strip() for resp in expected_response.split(",")]


It's good that this now support multiple possible answers.
I guess the implication of splitting on , and then doing a contains check for each substring though is that the resulting check is less strict. E.g. for a question like
Check the database and tell me the fund with the lowest Governance ESG score with an expected response of
Dynamics Industries, Silvermans Global ETF, WhiteRocks ETF, which has a score of 60.

This would pass for any answer that contains one of those fund names or which contains which has a score of 60.

@mic-smith - I get your point, so what do you suggest we should do?

I guess we could repeat the whole string in each option e.g. Dynamics Industries which has a score of 60, Silvermans Global ETF which has a score of 60, WhiteRocks ETF which has a score of 60 maybe with a different separator than , so it's more obvious? Though not sure how well that would work with the case we need to use an LLM because the response isn't a substring.

We're also commenting out these datasource based tests in #23 . So, I wonder if it's worth pulling these test changes into in a separate branch so it doesn't block this PR and then addressing once we have the new flow to upload data and a different data set available?

backend/src/prompts/templates/answer-user-question.j2

evpearce · 2024-11-14T16:07:49Z

backend/src/agents/web_agent.py

@@ -38,8 +38,8 @@ async def web_general_search_core(search_query, llm, model) -> str:
                }
            return json.dumps(response, indent=4)
        logger.info(f'Answer found successfully {answer_result}')
-        valid_answer = json.loads(answer_result["response"]).get("is_valid", "")
-        if valid_answer:
+        perform_web_search = json.loads(answer_result["response"]).get("perform_web_search", "")


this should be should_perform_web_search too I think

variable name not changed

@evpearce - Done..

backend/src/prompts/templates/answer-user-question.j2

And Changes for Emma's feedback.

Gagan Singh added 9 commits November 7, 2024 09:02

Push the FS 77 Changes related to the complext queries

236aa5e

Removing trailing white spaces

08d6c37

Sorting the failed test

e91b72f

Moving redis cache configuration up in .env.example file

d0981c1

Pushing the changes of filtering out http

d0c532a

Merge branch 'main' into FS-80/Use-HTTPS-For-Webagent-Search

1bfb87a

Backend type check change

af12225

Type check solved

d402397

Commenting User confirmation tests as it's not being used right now.

dc319ac

evpearce reviewed Nov 11, 2024

View reviewed changes

Changes based on Emma's Comments

ab09932

gaganahluwalia requested a review from evpearce November 11, 2024 14:04

IMladjenovic requested changes Nov 11, 2024

View reviewed changes

Gagan Singh added 3 commits November 13, 2024 08:59

Fixes based on the Ivan's feedback

f1c9382

BDD changes to make it work when the answer is one of the many options.

319290c

Merge branch 'main' into FS-80/Use-HTTPS-For-Webagent-Search

c4016df

IMladjenovic reviewed Nov 13, 2024

View reviewed changes

IMladjenovic approved these changes Nov 13, 2024

View reviewed changes

mic-smith reviewed Nov 13, 2024

View reviewed changes

evpearce reviewed Nov 14, 2024

View reviewed changes

backend/src/prompts/templates/answer-user-question.j2 Outdated Show resolved Hide resolved

evpearce reviewed Nov 14, 2024

View reviewed changes

backend/src/prompts/templates/answer-user-question.j2 Outdated Show resolved Hide resolved

Merge branch 'main' into FS-80/Use-HTTPS-For-Webagent-Search

b41e90a

And Changes for Emma's feedback.

gaganahluwalia requested a review from evpearce November 18, 2024 10:28

Changed the variable name

e0d306c

gaganahluwalia requested a review from mic-smith November 20, 2024 11:26

evpearce approved these changes Nov 20, 2024

View reviewed changes

gaganahluwalia merged commit c9566e0 into main Nov 22, 2024
4 checks passed

gaganahluwalia deleted the FS-80/Use-HTTPS-For-Webagent-Search branch November 22, 2024 09:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fs 80/use https for webagent search #21

Fs 80/use https for webagent search #21

gaganahluwalia commented Nov 11, 2024

evpearce Nov 11, 2024

gaganahluwalia Nov 11, 2024

evpearce Nov 11, 2024

gaganahluwalia Nov 11, 2024

evpearce Nov 11, 2024

gaganahluwalia Nov 11, 2024

IMladjenovic Nov 11, 2024

IMladjenovic Nov 11, 2024

IMladjenovic Nov 11, 2024

IMladjenovic Nov 11, 2024

IMladjenovic Nov 11, 2024 •

edited

Loading

IMladjenovic Nov 13, 2024

mic-smith Nov 13, 2024

gaganahluwalia Nov 19, 2024

mic-smith Nov 20, 2024 •

edited

Loading

evpearce Nov 14, 2024

evpearce Nov 19, 2024

gaganahluwalia Nov 19, 2024

		@@ -9,23 +9,24 @@ User's question is:
		Once you generate an answer:

Fs 80/use https for webagent search #21

Fs 80/use https for webagent search #21

Conversation

gaganahluwalia commented Nov 11, 2024

Description

Changelog

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IMladjenovic Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mic-smith Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IMladjenovic Nov 11, 2024 •

edited

Loading

mic-smith Nov 20, 2024 •

edited

Loading