From fa0078cb5be4a83d1ad3c507de92ddaeb98851c6 Mon Sep 17 00:00:00 2001 From: Nate Kelley Date: Fri, 2 May 2025 14:00:14 -0600 Subject: [PATCH] fix bogus question redirect bug --- web/playwright-tests/0_Question_2.test.ts | 19 +++++++++++++ web/playwright-tests/0_Question_3.test.ts | 17 ++++++++++++ web/playwright-tests/0_Question_4.test.ts | 18 +++++++++++++ web/playwright-tests/0_Question_5.test.ts | 17 ++++++++++++ .../question-helpers/ask-question.ts | 11 +++----- .../BusterChartLegend/useBusterChartLegend.ts | 4 +-- .../ChatContext/useAutoChangeLayout.ts | 27 +++++++++++++++---- .../ChatLayoutContext/useGetChatParams.tsx | 15 ----------- 8 files changed, 98 insertions(+), 30 deletions(-) create mode 100644 web/playwright-tests/0_Question_2.test.ts create mode 100644 web/playwright-tests/0_Question_3.test.ts create mode 100644 web/playwright-tests/0_Question_4.test.ts create mode 100644 web/playwright-tests/0_Question_5.test.ts diff --git a/web/playwright-tests/0_Question_2.test.ts b/web/playwright-tests/0_Question_2.test.ts new file mode 100644 index 000000000..dddef5626 --- /dev/null +++ b/web/playwright-tests/0_Question_2.test.ts @@ -0,0 +1,19 @@ +import { test, expect } from '@playwright/test'; +import { askQuestion, checkThatPageWasRedirected } from './question-helpers/ask-question'; + +test('Question: Can you make me a line chart that showcases my sales over time? The time frame should be 18 months. You should just create 1 file. The sales should be deliminated in USD. The line should also be kind of smooth so I do not see a sharp turn at every month. Also the time should be broken down by month.', async ({ + page +}) => { + await askQuestion( + page, + 'Can you make me a line chart that showcases my sales over time? The time frame should be 18 months. You should just create 1 file. The sales should be deliminated in USD. The line should also be kind of smooth so I do not see a sharp turn at every month. Also the time should be broken down by month.' + ); + + await checkThatPageWasRedirected(page, ['reasoning', 'chart']); + + //expect to for the text "Reasoned" on the page + await expect(page.getByText('Reasoned')).toBeVisible(); + + //expect to for the text "Chart" on the page + await expect(page.getByText('Chart')).toBeVisible(); +}); diff --git a/web/playwright-tests/0_Question_3.test.ts b/web/playwright-tests/0_Question_3.test.ts new file mode 100644 index 000000000..2115b0bc3 --- /dev/null +++ b/web/playwright-tests/0_Question_3.test.ts @@ -0,0 +1,17 @@ +import { test, expect } from '@playwright/test'; +import { askQuestion, checkThatPageWasRedirected } from './question-helpers/ask-question'; + +const question = + 'I need you to make a chart for me that shows my top 8 customers in the month of May. Can you make this chart sorted by the customers name? I want to see only 1 file created.'; + +test(`Question: ${question}`, async ({ page }) => { + await askQuestion(page, question); + + await checkThatPageWasRedirected(page, ['reasoning', 'chart']); + + //expect to for the text "Reasoned" on the page + await expect(page.getByText('Reasoned')).toBeVisible(); + + //expect to for the text "Chart" on the page + await expect(page.getByText('Chart')).toBeVisible(); +}); diff --git a/web/playwright-tests/0_Question_4.test.ts b/web/playwright-tests/0_Question_4.test.ts new file mode 100644 index 000000000..f0ee9f22c --- /dev/null +++ b/web/playwright-tests/0_Question_4.test.ts @@ -0,0 +1,18 @@ +import { test, expect } from '@playwright/test'; +import { askQuestion } from './question-helpers/ask-question'; + +const question = 'Who would win in a fight, 100 humans or a gorilla?'; + +test(`Question: ${question}`, async ({ page }) => { + await askQuestion(page, question); + + //wait for 2 seconds + await page.waitForTimeout(2000); + const currentURL = page.url(); + + //expect to for the text "Reasoned" on the page + await expect(page.getByText('Reasoned')).toBeVisible({ timeout: 30000 }); + + expect(page.url()).not.toContain('reasoning'); + expect(currentURL).toBe(currentURL); +}); diff --git a/web/playwright-tests/0_Question_5.test.ts b/web/playwright-tests/0_Question_5.test.ts new file mode 100644 index 000000000..51f3b5a0c --- /dev/null +++ b/web/playwright-tests/0_Question_5.test.ts @@ -0,0 +1,17 @@ +import { test, expect } from '@playwright/test'; +import { askQuestion } from './question-helpers/ask-question'; + +const question = 'What can I ask about?'; + +test(`Question: ${question}`, async ({ page }) => { + await askQuestion(page, question); + + //wait for 2 seconds + await page.waitForTimeout(2000); + + //expect to for the text "Reasoned" on the page + await expect(page.getByText('Reasoned')).toBeVisible({ timeout: 30000 }); + + expect(page.url()).not.toContain('metric'); + expect(page.url()).not.toContain('dashboard'); +}); diff --git a/web/playwright-tests/question-helpers/ask-question.ts b/web/playwright-tests/question-helpers/ask-question.ts index 47943f689..2c3b613b9 100644 --- a/web/playwright-tests/question-helpers/ask-question.ts +++ b/web/playwright-tests/question-helpers/ask-question.ts @@ -4,19 +4,14 @@ import { expect } from '@playwright/test'; export const askQuestion = async (page: Page, question: string) => { await page.goto('http://localhost:3000/app/home'); await page.getByRole('textbox', { name: 'Ask Buster a question...' }).click(); - await page - .getByRole('textbox', { name: 'Ask Buster a question...' }) - .fill('Who is my top customer?'); - + await page.getByRole('textbox', { name: 'Ask Buster a question...' }).fill(question); await expect(page.getByRole('main').getByRole('button')).toBeVisible(); await page.getByRole('textbox', { name: 'Ask Buster a question...' }).dblclick(); await page.getByRole('textbox', { name: 'Ask Buster a question...' }).press('ControlOrMeta+a'); await page.getByRole('textbox', { name: 'Ask Buster a question...' }).fill(''); await expect(page.getByRole('main').getByRole('button')).toBeDisabled(); await page.getByRole('textbox', { name: 'Ask Buster a question...' }).click(); - await page - .getByRole('textbox', { name: 'Ask Buster a question...' }) - .fill('Who is my top customer?'); + await page.getByRole('textbox', { name: 'Ask Buster a question...' }).fill(question); // Submit the question await page.getByRole('main').getByRole('button').click(); @@ -24,7 +19,7 @@ export const askQuestion = async (page: Page, question: string) => { export const checkThatPageWasRedirected = async ( page: Page, - redirectPath: ('metrics' | 'reasoning' | 'dashboard')[] + redirectPath: ('metrics' | 'reasoning' | 'dashboard' | 'chart')[] ) => { for (const path of redirectPath) { await page.waitForURL((url) => url.toString().includes(path), { diff --git a/web/src/components/ui/charts/BusterChartLegend/useBusterChartLegend.ts b/web/src/components/ui/charts/BusterChartLegend/useBusterChartLegend.ts index f7156680c..9182fa770 100644 --- a/web/src/components/ui/charts/BusterChartLegend/useBusterChartLegend.ts +++ b/web/src/components/ui/charts/BusterChartLegend/useBusterChartLegend.ts @@ -1,6 +1,6 @@ 'use client'; -import React, { useEffect, useMemo, useRef, useState } from 'react'; +import React, { useMemo, useState } from 'react'; import { useLegendAutoShow } from './useLegendAutoShow'; import { BusterChartLegendItem } from './interfaces'; import { @@ -15,7 +15,7 @@ import { DEFAULT_X_AXIS_COLUMN_NAMES, DEFAULT_Y_AXIS_COLUMN_NAMES } from './config'; -import { useUpdateEffect, useWhyDidYouUpdate } from '@/hooks'; +import { useUpdateEffect } from '@/hooks'; interface UseBusterChartLegendProps { selectedChartType: ChartType; diff --git a/web/src/layouts/ChatLayout/ChatContext/useAutoChangeLayout.ts b/web/src/layouts/ChatLayout/ChatContext/useAutoChangeLayout.ts index 36286b0cb..425bb85c5 100644 --- a/web/src/layouts/ChatLayout/ChatContext/useAutoChangeLayout.ts +++ b/web/src/layouts/ChatLayout/ChatContext/useAutoChangeLayout.ts @@ -3,7 +3,10 @@ import { useGetChatMessageMemoized, useGetChatMessage } from '@/api/buster_rest/chats'; import { useEffect, useRef } from 'react'; import findLast from 'lodash/findLast'; -import { BusterChatResponseMessage_file } from '@/api/asset_interfaces/chat'; +import type { + BusterChatMessageReasoning_text, + BusterChatResponseMessage_file +} from '@/api/asset_interfaces/chat'; import { useAppLayoutContextSelector } from '@/context/BusterAppLayout'; import { useGetFileLink } from '@/context/Assets/useGetFileLink'; import { useChatLayoutContextSelector } from '../ChatLayoutContext'; @@ -33,20 +36,34 @@ export const useAutoChangeLayout = ({ const onChangePage = useAppLayoutContextSelector((x) => x.onChangePage); const previousLastMessageId = useRef(null); - const { data: reasoningMessagesLength } = useGetChatMessage(lastMessageId, { - select: (x) => x?.reasoning_message_ids?.length || 0 + const { data: lastReasoningMessageId } = useGetChatMessage(lastMessageId, { + select: (x) => x?.reasoning_message_ids?.[x?.reasoning_message_ids?.length - 1] + }); + const { data: isFinishedReasoning } = useGetChatMessage(lastMessageId, { + select: (x) => + !!lastReasoningMessageId && + !!(x?.reasoning_messages[lastReasoningMessageId] as BusterChatMessageReasoning_text) + ?.finished_reasoning }); const { getFileLinkMeta } = useGetFileLink(); const previousIsCompletedStream = usePrevious(isCompletedStream); - const hasReasoning = !!reasoningMessagesLength; + const hasReasoning = !!lastReasoningMessageId; useEffect(() => { - console.log('REASONING: useEffect', isCompletedStream, hasReasoning, chatId, lastMessageId); + console.log( + 'REASONING: useEffect', + isCompletedStream, + hasReasoning, + chatId, + lastMessageId, + isFinishedReasoning + ); //this will trigger when the chat is streaming and is has not completed yet (new chat) if ( !isCompletedStream && + !isFinishedReasoning && hasReasoning && previousLastMessageId.current !== lastMessageId && chatId diff --git a/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.tsx b/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.tsx index e7da09d90..fb610b4d7 100644 --- a/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.tsx +++ b/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.tsx @@ -4,7 +4,6 @@ import { useAppLayoutContextSelector } from '@/context/BusterAppLayout'; import { useParams, usePathname, useSearchParams } from 'next/navigation'; import { useMemo } from 'react'; import { FileViewSecondary } from './useLayoutConfig'; -import { useWhyDidYouUpdate } from '@/hooks'; import { pathNameToRoute } from '@/routes/helpers'; export const useGetChatParams = () => { @@ -53,20 +52,6 @@ export const useGetChatParams = () => { return false; }, [secondaryView]); - useWhyDidYouUpdate('useGetChatParams', { - currentRoute, - isVersionHistoryMode, - chatId, - metricId, - dashboardId, - collectionId, - datasetId, - messageId, - metricVersionNumber, - dashboardVersionNumber, - secondaryView - }); - return useMemo( () => ({ currentRoute,