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

Add data2summary check in data summary panel in discover #8716

Merged
merged 13 commits into from
Jan 17, 2025
2 changes: 2 additions & 0 deletions changelogs/fragments/8716.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- Adds data2summary agent check in data summary panel in discover. ([#8716](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8716))
4 changes: 4 additions & 0 deletions src/plugins/query_enhancements/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const PLUGIN_ID = 'queryEnhancements';
export const PLUGIN_NAME = 'queryEnhancements';

export const BASE_API = '/api/enhancements';
export const BASE_API_ASSISTANT = '/api/assistant';

export const DATASET = {
S3: 'S3',
Expand Down Expand Up @@ -34,6 +35,9 @@ export const API = {
ASYNC_JOBS: `${BASE_API}/jobs`,
CONNECTIONS: `${BASE_API}/connections`,
},
AGENT_API: {
CONFIG_EXISTS: `${BASE_API_ASSISTANT}/agent_config/_exists`,
},
};

export const URI = {
Expand Down
5 changes: 2 additions & 3 deletions src/plugins/query_enhancements/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ export class QueryEnhancementsPlugin
],
};
queryString.getLanguageService().registerLanguage(sqlLanguageConfig);

data.__enhance({
editor: {
queryEditorExtension: createQueryAssistExtension(
Expand All @@ -201,10 +200,10 @@ export class QueryEnhancementsPlugin

public start(
core: CoreStart,
deps: QueryEnhancementsPluginStartDependencies
{ data }: QueryEnhancementsPluginStartDependencies
): QueryEnhancementsPluginStart {
setStorage(this.storage);
setData(deps.data);
setData(data);
return {};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { QueryAssistSummary, convertResult } from './query_assist_summary';
import { useQueryAssist } from '../hooks';
import { IDataFrame, Query } from '../../../../data/common';
import { FeedbackStatus as FEEDBACK } from '../../../common/query_assist';
import { checkAgentsExist } from '../utils/get_is_summary_agent';

jest.mock('react', () => ({
...jest.requireActual('react'),
Expand All @@ -21,6 +22,8 @@ jest.mock('../hooks', () => ({
useQueryAssist: jest.fn(),
}));

jest.mock('../utils/get_is_summary_agent', () => ({ checkAgentsExist: jest.fn() }));

describe('query assist summary', () => {
const PPL = 'ppl';
const question = 'Are there any errors in my logs?';
Expand All @@ -42,6 +45,7 @@ describe('query assist summary', () => {
const setSummary = jest.fn();
const setLoading = jest.fn();
const setFeedback = jest.fn();
const setIsSummaryAgentAvailable = jest.fn();
const setIsAssistantEnabledByCapability = jest.fn();
const getQuery = jest.fn();
const dataMock = {
Expand All @@ -58,6 +62,10 @@ describe('query assist summary', () => {
},
};

beforeEach(() => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: false });
});

afterEach(() => {
data$.next(undefined);
question$.next('');
Expand All @@ -71,13 +79,13 @@ describe('query assist summary', () => {
CLICK: 'click',
},
};

const props: ComponentProps<typeof QueryAssistSummary> = {
data: dataMock,
http: httpMock,
usageCollection: usageCollectionMock,
dependencies: {
isCollapsed: false,
isSummaryCollapsed: false,
},
core: coreSetupMock,
};
Expand Down Expand Up @@ -115,6 +123,7 @@ describe('query assist summary', () => {
summary,
loading,
feedback,
isSummaryAgentAvailable = false,
isAssistantEnabledByCapability = true,
isQueryAssistCollapsed = COLLAPSED.NO
) => {
Expand All @@ -125,6 +134,10 @@ describe('query assist summary', () => {
isAssistantEnabledByCapability,
setIsAssistantEnabledByCapability,
]);
React.useState.mockImplementationOnce(() => [
isSummaryAgentAvailable,
setIsSummaryAgentAvailable,
]);
useQueryAssist.mockImplementationOnce(() => ({
question: 'question',
question$,
Expand All @@ -133,7 +146,7 @@ describe('query assist summary', () => {
};

const defaultUseStateMock = () => {
mockUseState(null, LOADING.NO, FEEDBACK.NONE);
mockUseState(null, LOADING.NO, FEEDBACK.NONE, true);
};

it('should not show if collapsed is true', () => {
Expand All @@ -150,37 +163,56 @@ describe('query assist summary', () => {
expect(summaryPanels).toHaveLength(0);
});

it('should not show if query assistant is collapsed', () => {
mockUseState(null, LOADING.NO, FEEDBACK.NONE, true, COLLAPSED.YES);
it('should not show if is not summary agent', () => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: false });
mockUseState(null, LOADING.NO, FEEDBACK.NONE, false);
renderQueryAssistSummary(COLLAPSED.NO);
const summaryPanels = screen.queryAllByTestId('queryAssist__summary');
expect(summaryPanels).toHaveLength(0);
});

it('should not show if query assistant is collapsed', () => {
mockUseState(null, LOADING.NO, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.YES);
const summaryPanels = screen.queryAllByTestId('queryAssist__summary');
expect(summaryPanels).toHaveLength(0);
});

it('should show if collapsed is false', () => {
defaultUseStateMock();
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState(null, LOADING.NO, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
const summaryPanels = screen.queryAllByTestId('queryAssist__summary');
expect(summaryPanels).toHaveLength(1);
});

it('should show if summary agent', () => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState(null, LOADING.NO, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
const summaryPanels = screen.queryAllByTestId('queryAssist__summary');
expect(summaryPanels).toHaveLength(1);
});

it('should display loading view if loading state is true', () => {
mockUseState(null, LOADING.YES, FEEDBACK.NONE);
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState(null, LOADING.YES, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
expect(screen.getByTestId('queryAssist_summary_loading')).toBeInTheDocument();
expect(screen.queryAllByTestId('queryAssist_summary_result')).toHaveLength(0);
expect(screen.queryAllByTestId('queryAssist_summary_empty_text')).toHaveLength(0);
});

it('should display loading view if loading state is true even with summary', () => {
mockUseState('summary', LOADING.YES, FEEDBACK.NONE);
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState('summary', LOADING.YES, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
expect(screen.getByTestId('queryAssist_summary_loading')).toBeInTheDocument();
expect(screen.queryAllByTestId('queryAssist_summary_result')).toHaveLength(0);
expect(screen.queryAllByTestId('queryAssist_summary_empty_text')).toHaveLength(0);
});

it('should display initial view if loading state is false and no summary', () => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
defaultUseStateMock();
renderQueryAssistSummary(COLLAPSED.NO);
expect(screen.getByTestId('queryAssist_summary_empty_text')).toBeInTheDocument();
Expand All @@ -189,7 +221,8 @@ describe('query assist summary', () => {
});

it('should display summary result', () => {
mockUseState('summary', LOADING.NO, FEEDBACK.NONE);
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState('summary', LOADING.NO, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
expect(screen.getByTestId('queryAssist_summary_result')).toBeInTheDocument();
expect(screen.getByTestId('queryAssist_summary_result')).toHaveTextContent('summary');
Expand All @@ -198,7 +231,8 @@ describe('query assist summary', () => {
});

it('should report metric for thumbup click', async () => {
mockUseState('summary', LOADING.NO, FEEDBACK.NONE);
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState('summary', LOADING.NO, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
expect(screen.getByTestId('queryAssist_summary_result')).toBeInTheDocument();
await screen.getByTestId('queryAssist_summary_buttons_thumbup');
Expand All @@ -212,7 +246,8 @@ describe('query assist summary', () => {
});

it('should report metric for thumbdown click', async () => {
mockUseState('summary', LOADING.NO, FEEDBACK.NONE);
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState('summary', LOADING.NO, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
expect(screen.getByTestId('queryAssist_summary_result')).toBeInTheDocument();
await screen.getByTestId('queryAssist_summary_buttons_thumbdown');
Expand All @@ -226,23 +261,27 @@ describe('query assist summary', () => {
});

it('should hide thumbdown button if thumbup button is clicked', async () => {
mockUseState('summary', LOADING.NO, FEEDBACK.THUMB_UP);
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState('summary', LOADING.NO, FEEDBACK.THUMB_UP, true);
renderQueryAssistSummary(COLLAPSED.NO);
expect(screen.getByTestId('queryAssist_summary_result')).toBeInTheDocument();
await screen.getByTestId('queryAssist_summary_buttons_thumbup');
expect(screen.queryByTestId('queryAssist_summary_buttons_thumbdown')).not.toBeInTheDocument();
});

it('should hide thumbup button if thumbdown button is clicked', async () => {
mockUseState('summary', LOADING.NO, FEEDBACK.THUMB_DOWN);
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });

mockUseState('summary', LOADING.NO, FEEDBACK.THUMB_DOWN, true);
renderQueryAssistSummary(COLLAPSED.NO);
expect(screen.getByTestId('queryAssist_summary_result')).toBeInTheDocument();
await screen.getByTestId('queryAssist_summary_buttons_thumbdown');
expect(screen.queryByTestId('queryAssist_summary_buttons_thumbup')).not.toBeInTheDocument();
});

it('should not fetch summary if data is empty', async () => {
mockUseState(null, LOADING.NO, FEEDBACK.NONE);
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState(null, LOADING.NO, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
question$.next(question);
query$.next({ query: PPL, language: 'PPL' });
Expand All @@ -251,7 +290,7 @@ describe('query assist summary', () => {
});

it('should fetch summary with expected payload and response', async () => {
mockUseState('summary', LOADING.NO, FEEDBACK.NONE);
mockUseState('summary', LOADING.NO, FEEDBACK.NONE, true);
const RESPONSE_TEXT = 'response';
httpMock.post.mockResolvedValue(RESPONSE_TEXT);
renderQueryAssistSummary(COLLAPSED.NO);
Expand Down Expand Up @@ -291,6 +330,7 @@ describe('query assist summary', () => {
});

it('should not update queryResults if subscription changed not in order', async () => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState('summary', LOADING.NO, FEEDBACK.NONE);
const RESPONSE_TEXT = 'response';
httpMock.post.mockResolvedValue(RESPONSE_TEXT);
Expand All @@ -303,6 +343,7 @@ describe('query assist summary', () => {
});

it('should update queryResults if subscriptions changed in order', async () => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState('summary', LOADING.NO, FEEDBACK.NONE);
const RESPONSE_TEXT = 'response';
httpMock.post.mockResolvedValue(RESPONSE_TEXT);
Expand All @@ -321,6 +362,7 @@ describe('query assist summary', () => {
});

it('should reset feedback state if re-fetch summary', async () => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState('summary', LOADING.NO, FEEDBACK.THUMB_UP);
const RESPONSE_TEXT = 'response';
httpMock.post.mockResolvedValue(RESPONSE_TEXT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import sparkleHollowSvg from '../../assets/sparkle_hollow.svg';
import sparkleSolidSvg from '../../assets/sparkle_solid.svg';
import { FeedbackStatus } from '../../../common/query_assist';
import { checkAgentsExist } from '../utils/get_is_summary_agent';
import { DATA2SUMMARY_AGENT_CONFIG_ID } from '../utils/constant';

export interface QueryContext {
question: string;
Expand Down Expand Up @@ -70,6 +72,7 @@
const [loading, setLoading] = useState(false); // track loading state
const [feedback, setFeedback] = useState(FeedbackStatus.NONE);
const [isEnabledByCapability, setIsEnabledByCapability] = useState(false);
const [isSummaryAgentAvailable, setIsSummaryAgentAvailable] = useState(false);
const selectedDataset = useRef(query.queryString.getQuery()?.dataset);
const { question$, isQueryAssistCollapsed } = useQueryAssist();
const METRIC_APP = `query-assist`;
Expand Down Expand Up @@ -206,6 +209,26 @@
});
}, [props.core]);

useEffect(() => {
setIsSummaryAgentAvailable(false);
const fetchSummaryAgent = async () => {
try {
const summaryAgentStatus = await checkAgentsExist(
Copy link
Contributor

Choose a reason for hiding this comment

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

What will happen when checkAgentsExist throw exception? Should we try/catch the exception?

props.http,
DATA2SUMMARY_AGENT_CONFIG_ID,
selectedDataset.current?.dataSource?.id
);
setIsSummaryAgentAvailable(summaryAgentStatus.exists);
} catch (error) {
// eslint-disable-next-line no-console
console.error(error);

Check warning on line 224 in src/plugins/query_enhancements/public/query_assist/components/query_assist_summary.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/query_enhancements/public/query_assist/components/query_assist_summary.tsx#L224

Added line #L224 was not covered by tests
}
};
if (isEnabledByCapability) {
fetchSummaryAgent();
}
}, [selectedDataset.current?.dataSource?.id, props.http, isEnabledByCapability]);

const onFeedback = useCallback(
(satisfied: boolean) => {
if (feedback !== FeedbackStatus.NONE) return;
Expand All @@ -216,8 +239,15 @@
[feedback, reportMetric]
);

if (props.dependencies.isCollapsed || isQueryAssistCollapsed || !isEnabledByCapability)
if (
props.dependencies.isCollapsed ||
isQueryAssistCollapsed ||
!isEnabledByCapability ||
!isSummaryAgentAvailable
) {
return null;
}

const isDarkMode = props.core.uiSettings.get('theme:darkMode');
return (
<EuiSplitPanel.Outer
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

export const DATA2SUMMARY_AGENT_CONFIG_ID = 'os_data2summary';
Loading
Loading