Refactor for message action bar and add metrics for alerting summary #304

000FLMS · 2024-09-12T06:11:53Z

Description

Refactor popover to add message action bar and add metrics to thumb-up and thumb-down

Issues Resolved

[List any issues this PR will resolve]

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.

Screenshots

Action buttons in alerting summary and insight

Same component is used in chat bubbles

Signed-off-by: Sihan He <[email protected]>

Signed-off-by: 000FLMS <[email protected]>

Signed-off-by: Sihan He <[email protected]>

Signed-off-by: Heng Qian <[email protected]>

Signed-off-by: Sihan He <[email protected]>

Signed-off-by: Heng Qian <[email protected]>

Signed-off-by: 000FLMS <[email protected]>

Signed-off-by: Sihan He <[email protected]>

…ithRAG # Conflicts: # CHANGELOG.md

Signed-off-by: 000FLMS <[email protected]>

…ithRAG # Conflicts: # CHANGELOG.md # common/constants/llm.ts # server/plugin.ts

Signed-off-by: Heng Qian <[email protected]>

Signed-off-by: 000FLMS <[email protected]>

Hailong-am · 2024-09-12T08:17:12Z

can you put a screenshot of this change? That would be much helpful to understand the change.

Signed-off-by: 000FLMS <[email protected]>

000FLMS · 2024-09-13T02:30:30Z

can you put a screenshot of this change? That would be much helpful to understand the change.

Sure, I've added some screenshots, feel free to check them

CHANGELOG.md

public/components/incontext_insight/generate_popover_body.test.tsx

opensearch_dashboards.json

public/plugin.tsx

Hailong-am · 2024-09-13T03:43:24Z

public/tabs/chat/messages/message_action.tsx

+  );
+
+  const feedbackTip =
+    'Thank you for the feedback. Try again by adjusting your prompt so that I have the opportunity to better assist you.';


the tips may confuse user as they can't specify prompt. Also please add i18n support

It just follows the UI design
So I do not know if we could change it.

I think that tip is probably for text2viz summary, which is not delivered by decision.

@000FLMS there has a updated one, please update here accordingly.

Have changed the tip

public/components/incontext_insight/index.tsx

public/components/incontext_insight/generate_popover_body.tsx

Hailong-am · 2024-09-13T04:01:18Z

public/components/incontext_insight/generate_popover_body.tsx

@@ -98,6 +114,7 @@ export const GeneratePopoverBody: React.FC<{
              `Please provide your insight on this ${summaryType}.`
            );
          }
+          reportGeneratedMetric('generated');


nit: should we have seprate metric for alert summary and insight?

Should we report this metric if error happened? And what if summary succeed while insight fails?

If we need to report metrics no matter it succeed or not, we should move this logic to finally block.

I think we should only report when the summary is generated successfully.

public/components/incontext_insight/generate_popover_body.tsx

Signed-off-by: Sihan He <[email protected]>

…ds-assistant into alertingInsight

Signed-off-by: Sihan He <[email protected]>

…ds-assistant into alertingInsight

qianheng-aws · 2024-09-19T06:40:23Z

public/tabs/chat/messages/message_action.tsx

+
+type ButtonKey = 'copy' | 'regenerate' | 'thumbUp' | 'thumbDown' | 'trace';
+
+export const MessageActions: React.FC<MessageActionsProps> = ({


This component seems very general. Not only for chat but also insight, or maybe other cases. So I think it should be moved out of chat to a more proper directory.

How about put it into a new "action_button" under the "component" directory? Any suggestion?

qianheng-aws · 2024-09-19T06:47:51Z

public/components/incontext_insight/generate_popover_body.tsx

+            <MessageActions
+              contentToCopy={summary}
+              showFeedback
+              showTraceIcon={insightAvailable}


It seems weird to pass insight related status to Trace Icon and its related action function.

How about add a new parameter for the purpose of extending the MessageActions with customize buttons configuration? For this case, insight button should be passed in as a extending actions. You can think about which actions should be internal ones, and which should be extending ones.

It seems like that the only difference between insight and trace is just the click function, so I think adding a new button is a little redundent.

Not exactly. insight button needs to show info "Insight With RAG" when moving your cursor over it. And what if you wants to have both trace button and insight button, or need other customized buttons like insight? Overall, they should be 2 different buttons.

It could be an enhancement since this PR has been merged.

opensearch_dashboards.json

songkant-aws · 2024-09-19T06:33:36Z

public/components/incontext_insight/generate_popover_body.tsx

  const [summary, setSummary] = useState('');
  const [insight, setInsight] = useState('');
  const [insightAvailable, setInsightAvailable] = useState(false);
  const [showInsight, setShowInsight] = useState(false);
+  const metricAppName = 'alertSumm';


Does metric name have length restriction? If this is the case, you might need to add some protection check in util function.

Actually I do not really know any specific restriction

should we use full name alertSummary?

public/utils/report_metric.ts

Signed-off-by: Sihan He <[email protected]>

Hailong-am · 2024-09-20T03:00:31Z

public/components/incontext_insight/generate_popover_body.tsx

@@ -98,6 +103,7 @@ export const GeneratePopoverBody: React.FC<{
              `Please provide your insight on this ${summaryType}.`
            );
          }
+          reportMetric(usageCollection, metricAppName, 'generated');


As we record generated metric, i think the metric type could be METRIC_TYPE.COUNT.

if we want to record click metric, it should not care the result of agent execution.

ok, I'll change it to METRIC_TYPE.COUNT.

Hailong-am · 2024-09-20T03:08:58Z

public/plugin.tsx

+        const usageCollection = setupDeps.usageCollection || null;
+        return (
+          <IncontextInsightComponent
+            {...props}
+            httpSetup={httpSetup}
+            usageCollection={usageCollection}
+          />
+        );
      },
    };


Suggested change

const usageCollection = setupDeps.usageCollection || null;

return (

<IncontextInsightComponent

{...props}

httpSetup={httpSetup}

usageCollection={usageCollection}

/>

);

},

};

return (

<IncontextInsightComponent

{...props}

httpSetup={httpSetup}

usageCollection={setupDeps.usageCollection}

/>

);

},

};

Signed-off-by: Sihan He <[email protected]>

…ds-assistant into alertingInsight

…304) * Add message action module Signed-off-by: Sihan He <[email protected]> * Finish basic function of feedback buttons Signed-off-by: Sihan He <[email protected]> * Add test cases and fix bugs Signed-off-by: Sihan He <[email protected]> * Refactor the code to deal with core loss and fix the regenerate bug Signed-off-by: Sihan He <[email protected]> * Update CHANGELOG.md Signed-off-by: Sihan He <[email protected]> * Update for smaller buttons Signed-off-by: Sihan He <[email protected]> * Update for ui-metric Signed-off-by: Sihan He <[email protected]> * Support insight with RAG Signed-off-by: Heng Qian <[email protected]> * Add change in CHANGELOG.md Signed-off-by: Heng Qian <[email protected]> * Put footer in the same panel with content to match UX design Signed-off-by: Heng Qian <[email protected]> * Refine alert prompt Signed-off-by: Heng Qian <[email protected]> * set CSS scrollbar-width to thin Signed-off-by: Heng Qian <[email protected]> * Hide insight agent id from front-end Signed-off-by: Heng Qian <[email protected]> * Change summary agent config id Signed-off-by: Heng Qian <[email protected]> * Address comments Signed-off-by: Heng Qian <[email protected]> * Incorporate message action buttons into alerting summary Signed-off-by: Sihan He <[email protected]> * Add hover tips and rearrange buttons Signed-off-by: Sihan He <[email protected]> * Fix UT Signed-off-by: Heng Qian <[email protected]> * Add ui-metric for alerting summary Signed-off-by: Sihan He <[email protected]> * fix testing bugs Signed-off-by: Sihan He <[email protected]> * Change agent execute API Signed-off-by: Heng Qian <[email protected]> * Remove prompt from node JS server Signed-off-by: Heng Qian <[email protected]> * Replace CSS with component property Signed-off-by: Heng Qian <[email protected]> * Save the status when switch between summary and insight Signed-off-by: Sihan He <[email protected]> * Add tests for metrics Signed-off-by: Sihan He <[email protected]> * Use usageCollectionPluginMock in tests Signed-off-by: Sihan He <[email protected]> * Refactor reportMetric as util funciton Signed-off-by: Sihan He <[email protected]> * Remove telementry plugin Signed-off-by: Sihan He <[email protected]> * Fix metric type Signed-off-by: Sihan He <[email protected]> --------- Signed-off-by: Sihan He <[email protected]> Signed-off-by: 000FLMS <[email protected]> Signed-off-by: Heng Qian <[email protected]> Co-authored-by: Heng Qian <[email protected]> (cherry picked from commit db5155b) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md

…304) (#320) * Add message action module Signed-off-by: Sihan He <[email protected]> * Finish basic function of feedback buttons Signed-off-by: Sihan He <[email protected]> * Add test cases and fix bugs Signed-off-by: Sihan He <[email protected]> * Refactor the code to deal with core loss and fix the regenerate bug Signed-off-by: Sihan He <[email protected]> * Update CHANGELOG.md Signed-off-by: Sihan He <[email protected]> * Update for smaller buttons Signed-off-by: Sihan He <[email protected]> * Update for ui-metric Signed-off-by: Sihan He <[email protected]> * Support insight with RAG Signed-off-by: Heng Qian <[email protected]> * Add change in CHANGELOG.md Signed-off-by: Heng Qian <[email protected]> * Put footer in the same panel with content to match UX design Signed-off-by: Heng Qian <[email protected]> * Refine alert prompt Signed-off-by: Heng Qian <[email protected]> * set CSS scrollbar-width to thin Signed-off-by: Heng Qian <[email protected]> * Hide insight agent id from front-end Signed-off-by: Heng Qian <[email protected]> * Change summary agent config id Signed-off-by: Heng Qian <[email protected]> * Address comments Signed-off-by: Heng Qian <[email protected]> * Incorporate message action buttons into alerting summary Signed-off-by: Sihan He <[email protected]> * Add hover tips and rearrange buttons Signed-off-by: Sihan He <[email protected]> * Fix UT Signed-off-by: Heng Qian <[email protected]> * Add ui-metric for alerting summary Signed-off-by: Sihan He <[email protected]> * fix testing bugs Signed-off-by: Sihan He <[email protected]> * Change agent execute API Signed-off-by: Heng Qian <[email protected]> * Remove prompt from node JS server Signed-off-by: Heng Qian <[email protected]> * Replace CSS with component property Signed-off-by: Heng Qian <[email protected]> * Save the status when switch between summary and insight Signed-off-by: Sihan He <[email protected]> * Add tests for metrics Signed-off-by: Sihan He <[email protected]> * Use usageCollectionPluginMock in tests Signed-off-by: Sihan He <[email protected]> * Refactor reportMetric as util funciton Signed-off-by: Sihan He <[email protected]> * Remove telementry plugin Signed-off-by: Sihan He <[email protected]> * Fix metric type Signed-off-by: Sihan He <[email protected]> --------- Signed-off-by: Sihan He <[email protected]> Signed-off-by: 000FLMS <[email protected]> Signed-off-by: Heng Qian <[email protected]> Co-authored-by: Heng Qian <[email protected]> (cherry picked from commit db5155b) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Hailong Cui <[email protected]>

000FLMS and others added 30 commits August 13, 2024 09:58

Add message action module

337dfd6

Signed-off-by: Sihan He <[email protected]>

Finish basic function of feedback buttons

7938e5f

Signed-off-by: Sihan He <[email protected]>

Add test cases and fix bugs

82c0fd4

Signed-off-by: Sihan He <[email protected]>

Refactor the code to deal with core loss and fix the regenerate bug

19a5749

Signed-off-by: Sihan He <[email protected]>

Update CHANGELOG.md

15f3d2a

Signed-off-by: Sihan He <[email protected]>

Update for smaller buttons

578d8b8

Signed-off-by: Sihan He <[email protected]>

Merge branch 'main' into main

876b68a

Signed-off-by: 000FLMS <[email protected]>

Update for ui-metric

931ceaf

Signed-off-by: Sihan He <[email protected]>

Support insight with RAG

2e25985

Signed-off-by: Heng Qian <[email protected]>

Add change in CHANGELOG.md

21e2278

Signed-off-by: Heng Qian <[email protected]>

Put footer in the same panel with content to match UX design

662923b

Signed-off-by: Heng Qian <[email protected]>

Refine alert prompt

3ba3482

Signed-off-by: Heng Qian <[email protected]>

set CSS scrollbar-width to thin

1956d49

Signed-off-by: Heng Qian <[email protected]>

Hide insight agent id from front-end

14f9030

Signed-off-by: Heng Qian <[email protected]>

Merge branch 'insightWithRAG' into alertingInsight

d247983

Change summary agent config id

f799e15

Signed-off-by: Heng Qian <[email protected]>

Address comments

d2278f1

Signed-off-by: Heng Qian <[email protected]>

Incorporate message action buttons into alerting summary

f2516bb

Signed-off-by: Sihan He <[email protected]>

Add hover tips and rearrange buttons

d0f227d

Signed-off-by: Sihan He <[email protected]>

Fix UT

a0e1a89

Signed-off-by: Heng Qian <[email protected]>

Merge branch 'main' into main

ae41d29

Signed-off-by: 000FLMS <[email protected]>

Add ui-metric for alerting summary

600f9f0

Signed-off-by: Sihan He <[email protected]>

fix testing bugs

5f68dfa

Signed-off-by: Sihan He <[email protected]>

Merge remote-tracking branch 'refs/remotes/origin/main' into insightW…

07a5cff

…ithRAG # Conflicts: # CHANGELOG.md

Merge branch 'main' into main

5f27ef3

Signed-off-by: 000FLMS <[email protected]>

Merge branch 'main' into main

77ac476

Signed-off-by: 000FLMS <[email protected]>

Merge remote-tracking branch 'refs/remotes/origin/main' into insightW…

4002c9b

…ithRAG # Conflicts: # CHANGELOG.md # common/constants/llm.ts # server/plugin.ts

Change agent execute API

15d44c3

Signed-off-by: Heng Qian <[email protected]>

Remove prompt from node JS server

14d6d0b

Signed-off-by: Heng Qian <[email protected]>

Merge branch 'main' into insightWithRAG

d94e244

000FLMS requested review from xluo-aws, gaobinlong, Hailong-am, wanglam and raintygao as code owners September 12, 2024 06:11

Merge branch 'main' into alertingInsight

53f7c4f

Signed-off-by: 000FLMS <[email protected]>

Merge branch 'main' into alertingInsight

1d37a61

Signed-off-by: 000FLMS <[email protected]>

Hailong-am reviewed Sep 13, 2024

View reviewed changes

000FLMS added 6 commits September 13, 2024 15:05

Merge branch 'main' into alertingInsight

61da316

Use usageCollectionPluginMock in tests

e7c2f7f

Signed-off-by: Sihan He <[email protected]>

Merge branch 'alertingInsight' of https://github.com/000FLMS/dashboar…

a56973a

…ds-assistant into alertingInsight

Merge branch 'main' into alertingInsight

9fafeae

Refactor reportMetric as util funciton

2cd22d1

Signed-off-by: Sihan He <[email protected]>

Merge branch 'alertingInsight' of https://github.com/000FLMS/dashboar…

b665a42

…ds-assistant into alertingInsight

qianheng-aws reviewed Sep 19, 2024

View reviewed changes

songkant-aws reviewed Sep 19, 2024

View reviewed changes

Remove telementry plugin

b8bbda1

Signed-off-by: Sihan He <[email protected]>

Hailong-am reviewed Sep 20, 2024

View reviewed changes

Merge branch 'main' into alertingInsight

e99cfd9

Hailong-am reviewed Sep 20, 2024

View reviewed changes

000FLMS added 2 commits September 20, 2024 11:47

Fix metric type

c209f93

Signed-off-by: Sihan He <[email protected]>

Merge branch 'alertingInsight' of https://github.com/000FLMS/dashboar…

0bb77d8

…ds-assistant into alertingInsight

Hailong-am approved these changes Sep 20, 2024

View reviewed changes

Hailong-am added the backport 2.x Trigger the backport flow to 2.x label Sep 20, 2024

Hailong-am merged commit db5155b into opensearch-project:main Sep 20, 2024
13 checks passed

opensearch-trigger-bot bot mentioned this pull request Sep 20, 2024

[Backport 2.x] Refactor for message action bar and add metrics for alerting summary #320

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor for message action bar and add metrics for alerting summary #304

Refactor for message action bar and add metrics for alerting summary #304

000FLMS commented Sep 12, 2024 •

edited

Loading

Hailong-am commented Sep 12, 2024

000FLMS commented Sep 13, 2024

Hailong-am Sep 13, 2024

000FLMS Sep 13, 2024 •

edited

Loading

songkant-aws Sep 19, 2024

Hailong-am Sep 20, 2024

000FLMS Sep 20, 2024

Hailong-am Sep 13, 2024

qianheng-aws Sep 19, 2024 •

edited

Loading

000FLMS Sep 19, 2024

qianheng-aws Sep 19, 2024

000FLMS Sep 19, 2024

qianheng-aws Sep 19, 2024

000FLMS Sep 19, 2024

qianheng-aws Sep 20, 2024 •

edited

Loading

songkant-aws Sep 19, 2024

000FLMS Sep 19, 2024

Hailong-am Sep 20, 2024

Hailong-am Sep 20, 2024 •

edited

Loading

000FLMS Sep 20, 2024 •

edited

Loading

Hailong-am Sep 20, 2024


		type ButtonKey = 'copy' \| 'regenerate' \| 'thumbUp' \| 'thumbDown' \| 'trace';

		export const MessageActions: React.FC<MessageActionsProps> = ({

Refactor for message action bar and add metrics for alerting summary #304

Refactor for message action bar and add metrics for alerting summary #304

Conversation

000FLMS commented Sep 12, 2024 • edited Loading

Description

Issues Resolved

Check List

Screenshots

Action buttons in alerting summary and insight

Same component is used in chat bubbles

Hailong-am commented Sep 12, 2024

000FLMS commented Sep 13, 2024

Choose a reason for hiding this comment

000FLMS Sep 13, 2024 • 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

qianheng-aws Sep 19, 2024 • 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

Choose a reason for hiding this comment

qianheng-aws Sep 20, 2024 • 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

Hailong-am Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

000FLMS Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

000FLMS commented Sep 12, 2024 •

edited

Loading

000FLMS Sep 13, 2024 •

edited

Loading

qianheng-aws Sep 19, 2024 •

edited

Loading

qianheng-aws Sep 20, 2024 •

edited

Loading

Hailong-am Sep 20, 2024 •

edited

Loading

000FLMS Sep 20, 2024 •

edited

Loading