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

[workspace]Hide the assistant entry when there isn't data2summary agent #9277

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -38,6 +38,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 @@ -192,6 +193,7 @@ export class QueryEnhancementsPlugin
data,
this.config.queryAssist,
this.isQuerySummaryCollapsed$,
this.isSummaryAgentAvailable$,
this.resultSummaryEnabled$,
usageCollection
),
Expand All @@ -203,6 +205,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]);
Comment on lines -212 to -230
Copy link
Member

@SuZhou-Joe SuZhou-Joe Jan 27, 2025

Choose a reason for hiding this comment

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

Is there case that assistant is not installed but we turn on result summary?


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 @@ -93,6 +93,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 +126,7 @@ export const createQueryAssistExtension = (
http={http}
data={data}
isQuerySummaryCollapsed$={isQuerySummaryCollapsed$}
isSummaryAgentAvailable$={isSummaryAgentAvailable$}
{...(config.summary.enabled && { resultSummaryEnabled$ })}
question$={question$}
>
Expand Down Expand Up @@ -162,13 +164,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, setIsSummaryAgentAvailable] = useState(false);
Copy link
Member

Choose a reason for hiding this comment

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

What about we using useObservable(props.isSummaryAgentAvailable$)?

const updateQuestion = (newQuestion: string) => {
props.question$?.next(newQuestion);
};
Expand All @@ -178,13 +182,19 @@ const QueryAssistWrapper: React.FC<QueryAssistWrapperProps> = (props) => {
const subscription = props.isQuerySummaryCollapsed$?.subscribe((isCollapsed) => {
setIsQuerySummaryCollapsed(isCollapsed);
});
const isSummaryAgentAvailableSubscription = props.isSummaryAgentAvailable$?.subscribe(
(isAvailable) => {
setIsSummaryAgentAvailable(isAvailable);
}
);
const questionSubscription = props.question$?.subscribe((newQuestion) => {
setQuestion(newQuestion);
});

return () => {
questionSubscription?.unsubscribe();
subscription?.unsubscribe();
isSummaryAgentAvailableSubscription?.unsubscribe();
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand Down Expand Up @@ -216,6 +226,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
Loading