Skip to content

Commit

Permalink
[workspace]Hide the assistant entry when there isn't data2summary age…
Browse files Browse the repository at this point in the history
…nt (#9277)

* fix summary entry when no agent in workspace

Signed-off-by: Qxisylolo <qianxisy@amazon.com>

* Changeset file for PR #9277 created/updated

* resolve comments

Signed-off-by: Qxisylolo <qianxisy@amazon.com>

* fix test

Signed-off-by: Qxisylolo <qianxisy@amazon.com>

* separate test

Signed-off-by: Qxisylolo <qianxisy@amazon.com>

---------

Signed-off-by: Qxisylolo <qianxisy@amazon.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Co-authored-by: SuZhou-Joe <suzhou@amazon.com>
  • Loading branch information
3 people authored Feb 25, 2025
1 parent eba051a commit 46ee60c
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 56 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/9277.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Hide the assistant entry when there isn't data2summary agent ([#9277](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/9277))
3 changes: 3 additions & 0 deletions src/plugins/query_enhancements/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export class QueryEnhancementsPlugin
private readonly config: ConfigSchema;
private isQuerySummaryCollapsed$ = new BehaviorSubject<boolean>(false);
private resultSummaryEnabled$ = new BehaviorSubject<boolean>(false);
private isSummaryAgentAvailable$ = new BehaviorSubject<boolean>(false);

constructor(initializerContext: PluginInitializerContext) {
this.config = initializerContext.config.get<ConfigSchema>();
Expand Down Expand Up @@ -206,6 +207,7 @@ export class QueryEnhancementsPlugin
data,
this.config.queryAssist,
this.isQuerySummaryCollapsed$,
this.isSummaryAgentAvailable$,
this.resultSummaryEnabled$,
usageCollection
),
Expand All @@ -217,6 +219,7 @@ export class QueryEnhancementsPlugin
return {
isQuerySummaryCollapsed$: this.isQuerySummaryCollapsed$,
resultSummaryEnabled$: this.resultSummaryEnabled$,
isSummaryAgentAvailable$: this.isSummaryAgentAvailable$,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ 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 @@ -22,8 +21,6 @@ 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 @@ -45,7 +42,6 @@ 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 @@ -62,10 +58,6 @@ describe('query assist summary', () => {
},
};

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

afterEach(() => {
data$.next(undefined);
question$.next('');
Expand Down Expand Up @@ -99,6 +91,11 @@ describe('query assist summary', () => {
NO: false,
};

const ISSUMMARYAGENT = {
YES: true,
NO: false,
};

const renderQueryAssistSummary = (isCollapsed: boolean) => {
const component = render(
<div>
Expand All @@ -123,9 +120,9 @@ describe('query assist summary', () => {
summary,
loading,
feedback,
isSummaryAgentAvailable = false,
isAssistantEnabledByCapability = true,
isQuerySummaryCollapsed = COLLAPSED.NO
isQuerySummaryCollapsed = COLLAPSED.NO,
isSummaryAgentAvailable = ISSUMMARYAGENT.YES
) => {
React.useState.mockImplementationOnce(() => [summary, setSummary]);
React.useState.mockImplementationOnce(() => [loading, setLoading]);
Expand All @@ -134,14 +131,12 @@ describe('query assist summary', () => {
isAssistantEnabledByCapability,
setIsAssistantEnabledByCapability,
]);
React.useState.mockImplementationOnce(() => [
isSummaryAgentAvailable,
setIsSummaryAgentAvailable,
]);

useQueryAssist.mockImplementationOnce(() => ({
question: 'question',
question$,
isQuerySummaryCollapsed,
isSummaryAgentAvailable,
}));
};

Expand All @@ -164,8 +159,7 @@ describe('query assist summary', () => {
});

it('should not show if is not summary agent', () => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: false });
mockUseState(null, LOADING.NO, FEEDBACK.NONE, false);
mockUseState(null, LOADING.NO, FEEDBACK.NONE, false, ISSUMMARYAGENT.NO);
renderQueryAssistSummary(COLLAPSED.NO);
const summaryPanels = screen.queryAllByTestId('queryAssist__summary');
expect(summaryPanels).toHaveLength(0);
Expand All @@ -179,31 +173,27 @@ describe('query assist summary', () => {
});

it('should show if collapsed is false', () => {
(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', () => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState(null, LOADING.YES, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
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', () => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState('summary', LOADING.YES, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
expect(screen.getByTestId('queryAssist_summary_loading')).toBeInTheDocument();
Expand All @@ -212,7 +202,6 @@ describe('query assist summary', () => {
});

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 @@ -221,7 +210,6 @@ describe('query assist summary', () => {
});

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

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

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

it('should hide thumbdown button if thumbup button is clicked', async () => {
(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();
Expand All @@ -270,8 +255,6 @@ describe('query assist summary', () => {
});

it('should hide thumbup button if thumbdown button is clicked', async () => {
(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();
Expand All @@ -280,7 +263,6 @@ describe('query assist summary', () => {
});

it('should not fetch summary if data is empty', async () => {
(checkAgentsExist as jest.Mock).mockResolvedValue({ exist: true });
mockUseState(null, LOADING.NO, FEEDBACK.NONE, true);
renderQueryAssistSummary(COLLAPSED.NO);
question$.next(question);
Expand Down Expand Up @@ -330,7 +312,6 @@ 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 @@ -343,7 +324,6 @@ 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 @@ -362,7 +342,6 @@ 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,8 +30,6 @@ import { QueryAssistContextType } from '../../../common/query_assist';
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 @@ -72,9 +70,8 @@ export const QueryAssistSummary: React.FC<QueryAssistSummaryProps> = (props) =>
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$, isQuerySummaryCollapsed } = useQueryAssist();
const { question$, isQuerySummaryCollapsed, isSummaryAgentAvailable } = useQueryAssist();
const METRIC_APP = `query-assist`;
const afterFeedbackTip = i18n.translate('queryEnhancements.queryAssist.summary.afterFeedback', {
defaultMessage:
Expand Down Expand Up @@ -209,26 +206,6 @@ export const QueryAssistSummary: React.FC<QueryAssistSummaryProps> = (props) =>
});
}, [props.core]);

useEffect(() => {
setIsSummaryAgentAvailable(false);
const fetchSummaryAgent = async () => {
try {
const summaryAgentStatus = await checkAgentsExist(
props.http,
DATA2SUMMARY_AGENT_CONFIG_ID,
selectedDataset.current?.dataSource?.id
);
setIsSummaryAgentAvailable(summaryAgentStatus.exists);
} catch (error) {
// eslint-disable-next-line no-console
console.error(error);
}
};
if (isEnabledByCapability) {
fetchSummaryAgent();
}
}, [selectedDataset.current?.dataSource?.id, props.http, isEnabledByCapability]);

const onFeedback = useCallback(
(satisfied: boolean) => {
if (feedback !== FeedbackStatus.NONE) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export interface QueryAssistContextValue {
question$: BehaviorSubject<string>;
updateQuestion: (question: string) => void;
isQuerySummaryCollapsed: boolean;
isSummaryAgentAvailable: boolean;
}
export const QueryAssistContext = React.createContext<QueryAssistContextValue>(
{} as QueryAssistContextValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
import { i18n } from '@osd/i18n';
import { HttpSetup } from 'opensearch-dashboards/public';
import React, { useEffect, useState } from 'react';
import { BehaviorSubject } from 'rxjs';
import { BehaviorSubject, of } from 'rxjs';
import { useObservable } from 'react-use';
import { distinctUntilChanged, map, startWith, switchMap } from 'rxjs/operators';
import { DATA_STRUCTURE_META_TYPES, DEFAULT_DATA } from '../../../../data/common';
import {
Expand Down Expand Up @@ -93,6 +94,7 @@ export const createQueryAssistExtension = (
data: DataPublicPluginSetup,
config: ConfigSchema['queryAssist'],
isQuerySummaryCollapsed$: BehaviorSubject<boolean>,
isSummaryAgentAvailable$: BehaviorSubject<boolean>,
resultSummaryEnabled$: BehaviorSubject<boolean>,
usageCollection?: UsageCollectionSetup
): QueryEditorExtensionConfig => {
Expand Down Expand Up @@ -125,6 +127,7 @@ export const createQueryAssistExtension = (
http={http}
data={data}
isQuerySummaryCollapsed$={isQuerySummaryCollapsed$}
isSummaryAgentAvailable$={isSummaryAgentAvailable$}
{...(config.summary.enabled && { resultSummaryEnabled$ })}
question$={question$}
>
Expand Down Expand Up @@ -162,13 +165,15 @@ interface QueryAssistWrapperProps {
invert?: boolean;
isQuerySummaryCollapsed$?: BehaviorSubject<boolean>;
resultSummaryEnabled$?: BehaviorSubject<boolean>;
isSummaryAgentAvailable$?: BehaviorSubject<boolean>;
question$?: BehaviorSubject<string>;
}

const QueryAssistWrapper: React.FC<QueryAssistWrapperProps> = (props) => {
const [visible, setVisible] = useState(false);
const [question, setQuestion] = useState('');
const [isQuerySummaryCollapsed, setIsQuerySummaryCollapsed] = useState(true);
const isSummaryAgentAvailable = useObservable(props.isSummaryAgentAvailable$ ?? of(false), false);
const updateQuestion = (newQuestion: string) => {
props.question$?.next(newQuestion);
};
Expand All @@ -178,6 +183,7 @@ const QueryAssistWrapper: React.FC<QueryAssistWrapperProps> = (props) => {
const subscription = props.isQuerySummaryCollapsed$?.subscribe((isCollapsed) => {
setIsQuerySummaryCollapsed(isCollapsed);
});

const questionSubscription = props.question$?.subscribe((newQuestion) => {
setQuestion(newQuestion);
});
Expand Down Expand Up @@ -216,6 +222,7 @@ const QueryAssistWrapper: React.FC<QueryAssistWrapperProps> = (props) => {
question$,
updateQuestion,
isQuerySummaryCollapsed,
isSummaryAgentAvailable,
}}
>
{props.children}
Expand Down
1 change: 1 addition & 0 deletions src/plugins/query_enhancements/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { UsageCollectionSetup } from '../../usage_collection/public';
export interface QueryEnhancementsPluginSetup {
isQuerySummaryCollapsed$: BehaviorSubject<boolean>;
resultSummaryEnabled$: BehaviorSubject<boolean>;
isSummaryAgentAvailable$: BehaviorSubject<boolean>;
}

// eslint-disable-next-line @typescript-eslint/no-empty-interface
Expand Down

0 comments on commit 46ee60c

Please sign in to comment.