Hide the assistant entry when there isn't data2summary agent #417

Qxisylolo · 2025-01-27T06:24:27Z

Description

This pr hides the assistant entry when there isn't data2summary agent
!! This pr is a hard dependency that should be merged after opensearch-project/OpenSearch-Dashboards#9277 has been merged.

has agent:

no agent:

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test.
New functionality has user manual doc added.
Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Qxisylolo <[email protected]>

public/components/ui_action_context_menu.tsx

SuZhou-Joe · 2025-01-27T06:40:23Z

public/components/ui_action_context_menu.tsx

-  // The action button should be not displayed when there is no action and result summary disabled.
-  if (actionsRef.current.length === 0 && !resultSummaryEnabled) {
+  // The action button should be not displayed when there is no action and result summary disabled or there is no data2Summary agent
+  if (!isSummaryAgentAvailable || (actionsRef.current.length === 0 && !resultSummaryEnabled)) {


Nit: I'd recommend moving isSummaryAgentAvailable behind !resultSummaryEnabled because we need to make a network request to get the status.

public/plugin.tsx

SuZhou-Joe · 2025-01-27T06:42:57Z

public/components/ui_action_context_menu.tsx

+  async function checkAgentsExist(
+    http: HttpSetup,
+    agentConfigName: string | string[],
+    dataSourceId?: string
+  ) {
+    const response = await http.get(AGENT_API.CONFIG_EXISTS, {
+      query: { agentConfigName, dataSourceId },
+    });
+    return response;
+  }


We should move this function out of render function.

SuZhou-Joe · 2025-01-27T06:50:16Z

public/components/ui_action_context_menu.tsx

    return null;
  }

-  // The action button should be disabled when context menu has no item and result summary disabled.
+  // The action button should be disabled when context menu has no item and result summary disabled
  const actionDisabled = (panels.value?.[0]?.items ?? []).length === 0 && !resultSummaryEnabled;


Suggested change

const actionDisabled = (panels.value?.[0]?.items ?? []).length === 0 && !resultSummaryEnabled;

const actionDisabled = (panels.value?.[0]?.items ?? []).length === 0 && !resultSummaryEnabled && !isSummaryAgentAvailable;

Do we need to add check on isSummaryAgentAvailable?

I add this check at first, but if there isn't agent, should we hide the button instead of disabling it?

I think it is OK that we check on both places, but sure it is good enough if we will hide the button if no summary agent is available on target data source.

update~ and thanks for all the comments

SuZhou-Joe · 2025-01-27T07:13:42Z

public/components/ui_action_context_menu.tsx

+  useEffect(() => {
+    const fetchSummaryAgent = async () => {
+      try {
+        const summaryAgentStatus = await checkAgentsExist(


Do we need to call props.isSummaryAgentAvailable$.next(false) before making the call to check if agent exists? if we switch from data source A(agent configured) to data source B (agent not configured) and the call somehow failed, the status will be incorrect.

yes, it's import, sorry I forget it., updated~

Signed-off-by: Qxisylolo <[email protected]>

wanglam · 2025-02-07T09:47:46Z

public/components/ui_action_context_menu.tsx

-  // The action button should be not displayed when there is no action and result summary disabled.
-  if (actionsRef.current.length === 0 && !resultSummaryEnabled) {
+  // The action button should be not displayed when there is no action and result summary disabled or there is no data2Summary agent
+  if ((!resultSummaryEnabled && actionsRef.current.length === 0) || !isSummaryAgentAvailable) {


Shall we need to allow other actions to be displayed if the summary agent is not available? The other actions won't be displayed if the summary agent is not available after the current change. Is it possible to not register the summary action if the summary agent is not available?

The other action includes anomaly detection suggestion and generate visualization, right? If yes, They should be displayed even if their agents are configured, should not be impacted by summary agent.

Thanks for the comments, already updated, now the UI context bar will be disabled only when there isn't actions and result summary disabled or or no data2Summary agent is available

Signed-off-by: Qxisylolo <[email protected]>

codecov · 2025-02-08T03:28:51Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.03%. Comparing base (581a7ca) to head (b2b3469).
Report is 20 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
- Coverage   87.46%   85.03%   -2.44%     
==========================================
  Files          65       72       +7     
  Lines        1891     2105     +214     
  Branches      473      518      +45     
==========================================
+ Hits         1654     1790     +136     
- Misses        236      313      +77     
- Partials        1        2       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

wanglam · 2025-02-08T04:15:13Z

public/components/ui_action_context_menu.tsx

@@ -39,6 +43,26 @@ export const ActionContextMenu = (props: Props) => {
  });
  const resultSummaryEnabled = useObservable(props.resultSummaryEnabled$, false);
  const isQuerySummaryCollapsed = useObservable(props.isQuerySummaryCollapsed$, false);
+  const isSummaryAgentAvailable = useObservable(props.isSummaryAgentAvailable$, false);


I prefer to add a shouldShowSummarizationAction flag here. Something like

const shouldShowSummarizationAction = resultSummaryEnabled && isSummaryAgentAvailable;

We always need to check resultSummaryEnabled and isSummaryAgentAvailable when we need to determine if this action can be displayed. I'm suggest to combine them in same flag. It can improve the code readability.

yes, it's a good advice, update~

public/components/ui_action_context_menu.tsx

Signed-off-by: Qxisylolo <[email protected]>

fix summary entry when no agent

48a7218

Signed-off-by: Qxisylolo <[email protected]>

Qxisylolo requested review from lezzago, joshuali925, anirudha, ruanyl, SuZhou-Joe, xluo-aws, gaobinlong, Hailong-am, wanglam and raintygao as code owners January 27, 2025 06:24

add changelog

b4c0dbc

Signed-off-by: Qxisylolo <[email protected]>

SuZhou-Joe reviewed Jan 27, 2025

View reviewed changes

public/components/ui_action_context_menu.tsx Outdated Show resolved Hide resolved

SuZhou-Joe reviewed Jan 27, 2025

View reviewed changes

public/plugin.tsx Outdated Show resolved Hide resolved

SuZhou-Joe reviewed Jan 27, 2025

View reviewed changes

Qxisylolo added 2 commits January 27, 2025 15:43

resolve comments

8676537

Signed-off-by: Qxisylolo <[email protected]>

add dataAssistant check first

d9266cf

Signed-off-by: Qxisylolo <[email protected]>

wanglam reviewed Feb 7, 2025

View reviewed changes

the other actions are independent

b2b3469

Signed-off-by: Qxisylolo <[email protected]>

Merge branch 'main' into fix/hide_summary_entry

96d8589

wanglam reviewed Feb 8, 2025

View reviewed changes

optimize code

48d3b47

Signed-off-by: Qxisylolo <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hide the assistant entry when there isn't data2summary agent #417

Hide the assistant entry when there isn't data2summary agent #417

Qxisylolo commented Jan 27, 2025 •

edited

Loading

SuZhou-Joe Jan 27, 2025 •

edited

Loading

SuZhou-Joe Jan 27, 2025

SuZhou-Joe Jan 27, 2025

Qxisylolo Jan 27, 2025 •

edited

Loading

SuZhou-Joe Jan 27, 2025

Qxisylolo Jan 27, 2025

SuZhou-Joe Jan 27, 2025 •

edited

Loading

Qxisylolo Jan 27, 2025

wanglam Feb 7, 2025

xluo-aws Feb 7, 2025

Qxisylolo Feb 8, 2025

codecov bot commented Feb 8, 2025

wanglam Feb 8, 2025

Qxisylolo Feb 8, 2025

	const actionDisabled = (panels.value?.[0]?.items ?? []).length === 0 && !resultSummaryEnabled;
	const actionDisabled = (panels.value?.[0]?.items ?? []).length === 0 && !resultSummaryEnabled && !isSummaryAgentAvailable;

Hide the assistant entry when there isn't data2summary agent #417

Are you sure you want to change the base?

Hide the assistant entry when there isn't data2summary agent #417

Conversation

Qxisylolo commented Jan 27, 2025 • edited Loading

Description

Check List

SuZhou-Joe Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Qxisylolo Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuZhou-Joe Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 8, 2025

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Qxisylolo commented Jan 27, 2025 •

edited

Loading

SuZhou-Joe Jan 27, 2025 •

edited

Loading

Qxisylolo Jan 27, 2025 •

edited

Loading

SuZhou-Joe Jan 27, 2025 •

edited

Loading