Refactor and clean up code across multiple files

Author: Kingshuk-Microsoft

infra/scripts/index_scripts/02_process_data.py

@@ -177,6 +177,6 @@ def prepare_search_doc(content, document_id):
         docs = []
 
 if docs != []:
-    results = search_client.upload_documents(documents=docs)
+    search_client.upload_documents(documents=docs)
 
 print(f'{str(counter)} files processed.')

scripts/data_utils.py

@@ -157,7 +157,13 @@ def extract_caption(self, text):
 
     def mask_urls_and_imgs(self, text) -> Tuple[Dict[str, str], str]:
         def find_urls(string):
-            regex = r"(?i)\b((?:https?://|www\d{0,3}[.]|[a-z0-9.\-]+[.][a-z]{2,4}/)(?:[^()\s<>]+|\(([^()\s<>]+|(\([^()\s<>]+\)))*\))+(?:\(([^()\s<>]+|(\([^()\s<>]+\)))*\)|[^()\s`!()\[\]{};:'\".,<>?«»“”‘’]))"
+            regex = (
+                r"(?i)\b("
+                r"(?:https?://|www\d{0,3}[.]|[a-z0-9.\-]+[.][a-z]{2,4}/)"
+                r"(?:[^()\s<>]+|\(([^()\s<>]+|(\([^()\s<>]+\)))*\))+"
+                r"(?:\(([^()\s<>]+|(\([^()\s<>]+\)))*\)|[^()\s`!()\[\]{};:'\".,<>?«»“”‘’])"
+                r")"
+            )
             urls = re.findall(regex, string)
             return [x[0] for x in urls]
 
@@ -693,7 +699,9 @@ def extract_pdf_content(file_path, form_recognizer_client, use_layout=False):
     page_map = []
     model = "prebuilt-layout" if use_layout else "prebuilt-read"
 
-    base64file = base64.b64encode(open(file_path, "rb").read()).decode()
+    with open(file_path, "rb") as f:
+        file_bytes = f.read()
+    base64file = base64.b64encode(file_bytes).decode()
     poller = form_recognizer_client.begin_analyze_document(
         model, AnalyzeDocumentRequest(bytes_source=base64file)
     )
@@ -1048,7 +1056,8 @@ def image_content_to_tag(image_content: str) -> str:
 
 
 def get_caption(image_path, captioning_model_endpoint, captioning_model_key):
-    encoded_image = base64.b64encode(open(image_path, "rb").read()).decode("ascii")
+    with open(image_path, "rb") as image_file:
+        encoded_image = base64.b64encode(image_file.read()).decode("ascii")
     file_ext = image_path.split(".")[-1]
     headers = {
         "Content-Type": "application/json",

src/.env.sample

@@ -167,7 +167,6 @@ async def init_ai_foundry_client():
         return ai_foundry_client
     except Exception as e:
         logging.exception("Exception in AI Foundry initialization", e)
-        ai_foundry_client = None
         raise e
 
 
@@ -197,7 +196,6 @@ def init_cosmosdb_client():
             if span is not None:
                 span.record_exception(e)
                 span.set_status(Status(StatusCode.ERROR, str(e)))
-            cosmos_conversation_client = None
             raise e
     else:
         logging.debug("CosmosDB not configured")

src/app.py

@@ -110,7 +110,7 @@ async def delete_messages(self, conversation_id, user_id):
                     item=message["id"], partition_key=user_id
                 )
                 response_list.append(resp)
-            return response_list
+        return response_list
 
     async def get_conversations(self, user_id, limit, sort_order="DESC", offset=0):
         parameters = [{"name": "@userId", "value": user_id}]

src/backend/history/cosmosdbservice.py

@@ -249,8 +249,9 @@ def split_contexts(
 class DatasourcePayloadConstructor(BaseModel, ABC):
     _settings: "_AppSettings" = PrivateAttr()
 
-    def __init__(self, settings: "_AppSettings", **data):
-        super().__init__(**data)
+    def __init__(self, *args, settings: "_AppSettings", **data):
+        # Call next __init__ in MRO to allow cooperative multiple inheritance
+        super().__init__(*args, **data)
         self._settings = settings
 
     @abstractmethod
@@ -274,6 +275,9 @@ class _AzureSearchSettings(BaseSettings, DatasourcePayloadConstructor):
     endpoint_suffix: str = Field(default="search.windows.net", exclude=True)
     connection_name: Optional[str] = None
     index: str = Field(serialization_alias="index_name")
+    def __init__(self, settings: "_AppSettings", **data):
+        # Ensure both BaseSettings and DatasourcePayloadConstructor are initialized
+        super().__init__(settings=settings, **data)
     key: Optional[str] = Field(default=None, exclude=True)
     use_semantic_search: bool = Field(default=False, exclude=True)
     semantic_search_config: str = Field(
@@ -439,6 +443,7 @@ def set_datasource_settings(self) -> Self:
             logging.warning(
                 "No datasource configuration found in the environment -- calls will be made to Azure OpenAI without grounding data."
             )
+            return self
 
 
 app_settings = _AppSettings()

src/backend/settings.py

@@ -2,15 +2,9 @@
 
 import React from 'react';
 
-// Mock implementation of react-markdown
-const mockNode = {
-  children: [{ value: 'console.log("Test Code");' }]
-};
-const mockProps = { className: 'language-javascript' };
-
 const ReactMarkdown: React.FC<{ children: React.ReactNode , components: any }> = ({ children,components }) => {
   return <div data-testid="reactMockDown">
-    {/* {components && components.code({ node: mockNode, ...mockProps })} */}
+    {/* {components && components.code({ node: { children: [{ value: 'console.log("Test Code");' }] }, ...mockProps })} */}
     {children}</div>; // Simply render the children
 };
 

src/frontend/__mocks__/react-markdown.tsx

@@ -1,5 +1,3 @@
-import Plotly from 'react-plotly.js'
-
 export type AskResponse = {
   answer: string
   citations: Citation[]

src/frontend/src/api/models.ts

@@ -1,11 +1,9 @@
-import { renderWithContext, screen, waitFor, fireEvent, act, logRoles } from '../../test/test.utils';
+import { renderWithContext, screen, waitFor, fireEvent, act } from '../../test/test.utils';
 import { Answer } from './Answer'
-import { AppStateContext } from '../../state/AppProvider'
 import {AskResponse, Citation, Feedback, historyMessageFeedback } from '../../api';
 //import { Feedback, AskResponse, Citation } from '../../api/models'
 import { cloneDeep } from 'lodash'
 import userEvent from '@testing-library/user-event';
-import { CitationPanel } from '../../pages/chat/Components/CitationPanel';
 
 // Mock required modules and functions
 jest.mock('../../api/api', () => ({
@@ -27,9 +25,6 @@ jest.mock('remark-gfm', () => jest.fn());
 jest.mock('rehype-raw', () => jest.fn());
 jest.mock('remark-supersub', () => jest.fn());
 
-const mockDispatch = jest.fn();
-const mockOnCitationClicked = jest.fn();
-
 // Mock context provider values
 let mockAppState = {
     frontendSettings: { feedback_enabled: true, sanitize_answer: true },
@@ -360,15 +355,14 @@ describe('Answer Component', () => {
     it('should open and submit negative feedback dialog', async () => {
         userEvent.setup();
         renderComponent();
-        const handleChange = jest.fn();
         const dislikeButton = screen.getByLabelText('Dislike this response');
 
         // Click dislike to open dialog
         await fireEvent.click(dislikeButton);
         expect(screen.getByText("Why wasn't this response helpful?")).toBeInTheDocument();
 
         // Select feedback and submit
-        const checkboxEle = await screen.findByLabelText(/Citations are wrong/i)
+        const checkboxEle = await screen.findByLabelText(/Citations are wrong/i);
         //logRoles(checkboxEle)
         await waitFor(() => {
             userEvent.click(checkboxEle);
@@ -382,12 +376,8 @@ describe('Answer Component', () => {
 
     it('calls resetFeedbackDialog and setFeedbackState with Feedback.Neutral on dialog dismiss', async () => {
 
-        const resetFeedbackDialogMock = jest.fn();
-        const setFeedbackStateMock = jest.fn();
-
         userEvent.setup();
         renderComponent();
-        const handleChange = jest.fn();
         const dislikeButton = screen.getByLabelText('Dislike this response');
 
         // Click dislike to open dialog
@@ -410,7 +400,6 @@ describe('Answer Component', () => {
     it('Dialog Options should be able to select and unSelect', async () => {
         userEvent.setup();
         renderComponent();
-        const handleChange = jest.fn();
         const dislikeButton = screen.getByLabelText('Dislike this response');
 
         // Click dislike to open dialog
@@ -419,15 +408,15 @@ describe('Answer Component', () => {
         expect(screen.getByText("Why wasn't this response helpful?")).toBeInTheDocument();
 
         // Select feedback and submit
-        const checkboxEle = await screen.findByLabelText(/Citations are wrong/i)
+        const checkboxEle = await screen.findByLabelText(/Citations are wrong/i);
         expect(checkboxEle).not.toBeChecked();
 
         await userEvent.click(checkboxEle);
         await waitFor(() => {
             expect(checkboxEle).toBeChecked();
         });
 
-        const checkboxEle1 = await screen.findByLabelText(/Citations are wrong/i)
+        const checkboxEle1 = await screen.findByLabelText(/Citations are wrong/i);
 
         await userEvent.click(checkboxEle1);
         await waitFor(() => {
@@ -439,7 +428,6 @@ describe('Answer Component', () => {
     it('Should able to show ReportInappropriateFeedbackContent form while click on "InappropriateFeedback" button ', async () => {
         userEvent.setup();
         renderComponent();
-        const handleChange = jest.fn();
         const dislikeButton = screen.getByLabelText('Dislike this response');
 
         // Click dislike to open dialog
@@ -514,7 +502,6 @@ describe('Answer Component', () => {
             feedbackState: { '123': Feedback.OtherHarmful },
         }
         renderComponent(answerWithMissingFeedback, extraMockState);
-        const handleChange = jest.fn();
         const dislikeButton = screen.getByLabelText('Dislike this response');
 
         // Click dislike to open dialog
@@ -529,10 +516,6 @@ describe('Answer Component', () => {
 
         tempMockCitation[0].filepath = '';
         tempMockCitation[0].reindex_id = '';
-        const answerWithMissingFeedback = {
-            ...mockAnswerProps,
-            CitationPanel: [...tempMockCitation]
-        }
 
         renderComponent();
 

src/frontend/src/components/Answer/Answer.test.tsx

@@ -39,7 +39,6 @@ export const Answer = ({ answer, onCitationClicked }: Props) => {
   }
 
   const [isRefAccordionOpen, { toggle: toggleIsRefAccordionOpen }] = useBoolean(false)
-  const filePathTruncationLimit = 50
 
   const parsedAnswer = useMemo(() => parseAnswer(answer), [answer])
   const [chevronIsExpanded, setChevronIsExpanded] = useState(isRefAccordionOpen)
@@ -156,7 +155,7 @@ export const Answer = ({ answer, onCitationClicked }: Props) => {
   const onLikeResponseClicked = async () => {
     if (answer.message_id == undefined) return
 
-    let newFeedbackState = feedbackState
+    let newFeedbackState: Feedback.Neutral | Feedback.Positive | Feedback.Negative
     // Set or unset the thumbs up state
     if (feedbackState == Feedback.Positive) {
       newFeedbackState = Feedback.Neutral
@@ -176,7 +175,7 @@ export const Answer = ({ answer, onCitationClicked }: Props) => {
   const onDislikeResponseClicked = async () => {
     if (answer.message_id == undefined) return
 
-    let newFeedbackState = feedbackState
+    let newFeedbackState: Feedback.Neutral | Feedback.Negative
     if (feedbackState === undefined || feedbackState === Feedback.Neutral || feedbackState === Feedback.Positive) {
       newFeedbackState = Feedback.Negative
       setFeedbackState(newFeedbackState)