feat(feedback): frontend to display summary #93567

vishnupsatish · 2025-06-13T22:41:04Z

Linear ticket: https://linear.app/getsentry/issue/REPLAY-294/frontend-to-display-the-summary

Loading (placeholder) state:

Summary:

Error state:

To be honest I don't know how to implement the "Read more" logic that's on the Figma, image below:

I think I might be able to add the "Read more" on the next line, but I'm wondering that if the response is meant to be only a few sentences, do we need it?

--

Revert endpoint changes and feature flag default=True

…d uses create_feedback

…format

…sh/cache-ai-summary

…perly??

…sh/cache-ai-summary merge the branch

aliu39 · 2025-06-16T20:51:23Z

I'm wondering that if the response is meant to be only a few sentences, do we need it?

great point! I think in most cases we won't, but maybe the read more is meant as a "fallback" so the summary component stays within a max height - which you can set either as a raw number of pixels, or % of screen height.

static/app/components/feedback/list/useFeedbackSummary.tsx

static/app/views/feedback/feedbackListPage.tsx

static/app/components/feedback/list/useFeedbackSummary.tsx

billyvg · 2025-06-16T22:46:56Z

static/app/components/feedback/list/useFeedbackSummary.tsx

+      feedbackSummaryData.num_feedbacks_used === 0 &&
+      feedbackSummaryData.success === false,


Can it be a valid case that num_feedbacks_used === 0 and success === true?

No, that can't be true

…mmary-frontend merge

michellewzhang · 2025-06-17T16:40:57Z

static/app/components/feedback/feedbackSummary.tsx

+  if (error) {
+    return <LoadingError message={t('There was an error loading the summary')} />;
+  }


should we hide the box completely in this case? unless we provide an option to try again

Yes, we can hide the box for now

michellewzhang · 2025-06-17T16:41:19Z

static/app/components/feedback/feedbackSummary.tsx

+        <div>
+          <IconSeer size="xs" />
+        </div>


do we need the div?

I think so, for some reason without the div it looks like this:

and with the div, this is what it looks like:

hmm, maybe that can be fixed with some alignment css?

i'm applying align-items: baseline here to the SummaryIconContainer

this is what it looks like with the div (pretty much the same i think)

I'll update it to use align-items: baseline, thanks for experimenting!

static/app/components/feedback/feedbackSummary.tsx

static/app/components/feedback/list/useFeedbackSummary.tsx

static/app/views/feedback/feedbackListPage.tsx

michellewzhang · 2025-06-17T16:53:46Z

static/app/views/feedback/feedbackListPage.tsx

-                        <FeedbackList />
-                      </Container>
+                      <SummaryListContainer style={{gridArea: 'list'}}>
+                        {hasFeedbackSummaryEnabled && (


if the feedback summary child controls when to render, maybe the flag check should be moved into the child as well. feels a bit disjoint to have separate checks here and in the child

Hmm, looking at now I think the flag check is better in the parent, because we're unnecessarily invoking the useFeedbackSummary hook otherwise. And the flag deciding whether to render FeedbackSummary is a different situation compared to an error/pending/tooFewFeedbacks state imo

if we include the flag check in the enabled part of the hook, then it won't be called unnecessarily right?

Also leads to a dependency in the useFeedbackSummary hook where we have to check the flag there as well

i think we would only need to check it once in the hook file - what do you mean by dependency?

yes, sorry my previous comment sent before I saw yours. If we include it in the enabled part, it won't be called. What I meant is, if we check the flag it in the parent, the hook won't even be invoked, so checking it in FeedbackSummary would mean we have to actually check it twice (once in useFeedbackSummary as well, as you mentioned). But I don't feel too strongly on this.

ryan953 · 2025-06-17T16:58:34Z

static/app/views/feedback/feedbackListPage.tsx

+                          /* is this bad design? FeedbackSummary conditionally renders itself if there is a summary, but should that decision be made in its parent instead? causes weird issues like we can't wrap FeedbackSummary with a Container here, it must be wrapped inside the component itself */
+                          /* the reason i don't generate the summary here instead of in FeedbackSummary is that useFeedbackSummary requires the FeedbackQueryKeys context to be present to be able to parse the start/end date */
+                          /* another option is to create a new component that wraps the summary and the list, and generate the summary in that new component so the parent would conditionally render the child instead of the child conditionally rendering itself */


lets answer this in this PR! Michelle suggested a path forward here: https://github.com/getsentry/sentry/pull/93567/files#r2152739050

Suggested change

/* is this bad design? FeedbackSummary conditionally renders itself if there is a summary, but should that decision be made in its parent instead? causes weird issues like we can't wrap FeedbackSummary with a Container here, it must be wrapped inside the component itself */

/* the reason i don't generate the summary here instead of in FeedbackSummary is that useFeedbackSummary requires the FeedbackQueryKeys context to be present to be able to parse the start/end date */

/* another option is to create a new component that wraps the summary and the list, and generate the summary in that new component so the parent would conditionally render the child instead of the child conditionally rendering itself */

michellewzhang · 2025-06-17T17:20:44Z

static/app/views/feedback/feedbackListPage.tsx

+// grid-template-columns: minmax(195px, 1fr) 1.5fr; hmm, need to come back to this, i kinda get what it does but it seems a bit sus, but it works better than what was there previously since before there was invalid CSS here
+


what was the invalid CSS?

It was grid-template-columns: minmax(1fr, 195px) 1fr;. devtools and ChatGPT told me that is invalid

ahh okay i see. is this what was causing the bug you mentioned? does the current css you have fix that?

Yes this was causing the bug I mentioned, and I think the current CSS I have fixes that (just by playing around with it a bit)

let's clean up this comment before we merge

vishnupsatish · 2025-06-17T17:25:49Z

Jesse mentioned he'll get a design out for this with updated states for loading and if there aren't enough feedbacks. Should we merge and experiment with this for now, and make those changes in a follow-up PR? Or wait to incorporate those changes here.

michellewzhang · 2025-06-17T17:30:46Z

Jesse mentioned he'll get a design out for this with updated states for loading and if there aren't enough feedbacks. Should we merge and experiment with this for now, and make those changes in a follow-up PR? Or wait to incorporate those changes here.

i think it's fine to merge what we're happy with here & make changes in a followup PR

static/app/components/feedback/list/useFeedbackSummary.tsx

vishnupsatish · 2025-06-17T20:44:07Z

static/app/components/feedback/list/useFeedbackSummary.tsx

+  const {selection} = usePageFilters();
+
+  const normalizedDateRange = normalizeDateTimeParams(selection.datetime);


I've updated the way the date range filtering is done and from my testing (and looking at the code) this seems to work. Can you please have a look @ryan953 @michellewzhang?

woo, fewer weird things going on!

michellewzhang

hmm, any idea why i'm getting an error when i select any statsPeriod or some start/end times?

june 10-15 works though.

michellewzhang · 2025-06-17T21:06:48Z

static/app/components/feedback/feedbackSummary.tsx

+
+const SummaryIconContainer = styled('div')`
+  display: flex;
+  flex-direction: row;


nit: the default value for flex-direction is row!

michellewzhang · 2025-06-17T21:07:35Z

static/app/components/feedback/feedbackSummary.tsx

+
+  const organization = useOrganization();
+
+  if (!organization.features.includes('user-feedback-ai-summaries')) {


can combine this case with the tooFewFeedbacks case by bringing that one up here

vishnupsatish · 2025-06-17T21:42:17Z

hmm, maybe the error you're seeing is similar to the one Ryan was seeing? like the LLM not generating in the right format. I'm investigating now

Edit: looking at Sentry, it does seem that you were facing the same parsing error that Ryan encountered (I am encountering this too)

vishnupsatish added 30 commits June 5, 2025 09:10

feedback usecase to generate summary and parse it, and testing it

1af771f

update tests and some config

a15f6f7

update based on review - rename, strip consecutive whitespaces, etc.

9ba5ee5

add max tokens and add comment back

3b80ac2

create endpoint, very rough and no tests

937f1bf

create test file using cursor, need to change so it actually works an…

860cdb1

…d uses create_feedback

trying out tests and creating user feedbacks using kafka

376de16

write endpoint test that actually works

019c174

more tests for the API endpoint

0a3360f

format

aba030a

fix docstring for endpoint

1956230

update comments

b16637f

remove custom exception for parse, fix failing test

b34742f

format tests

7368e0f

remove default=True for user-feedback-ui

c20e77a

Merge branch 'master' into vishnupsatish/uf-summary-api-endpoint

4a1b805

update based on review

5da9100

log more stuff

a22d4c6

cache summary and write tests for the caching aspect

7493363

update based on review

55bfad7

update url

6a9430d

gate behind feature flag

faf5701

test for nonexistent feature flag, test for different project qparam …

40dd6cc

…format

Merge branch 'vishnupsatish/uf-summary-api-endpoint' into vishnupsati…

d497108

…sh/cache-ai-summary

merge master, why is a test_create_feedback test failing?

c6e3051

use monkeypatch instead of patch, but it is still not cleaning up pro…

d630eef

…perly??

revert to how it was before as monkeypatch isnt fixing the issue

5cda5e8

update based on review, mock generate_summary instead of openai

43ce9a4

Merge branch 'vishnupsatish/uf-summary-api-endpoint' into vishnupsati…

a451cef

…sh/cache-ai-summary merge the branch

update cache tests

72cb051

revert feature flags

59c98db

vercel bot deployed to Preview June 16, 2025 20:42 View deployment

not enough feedbacks, ensure success is false too

bd44743

vercel bot deployed to Preview June 16, 2025 21:00 View deployment

billyvg reviewed Jun 16, 2025

View reviewed changes

update based on review

5062dfe

vercel bot deployed to Preview June 16, 2025 23:28 View deployment

num_feedbacks_used -> numFeedbacksUsed in the frontend too

ece7000

vercel bot deployed to Preview June 16, 2025 23:51 View deployment

Merge remote-tracking branch 'origin/master' into vishnupsatish/uf-su…

5827c79

…mmary-frontend merge

vercel bot deployed to Preview June 17, 2025 16:37 View deployment

michellewzhang reviewed Jun 17, 2025

View reviewed changes

michellewzhang requested a review from ryan953 June 17, 2025 16:54

ryan953 reviewed Jun 17, 2025

View reviewed changes

michellewzhang reviewed Jun 17, 2025

View reviewed changes

static/app/components/feedback/list/useFeedbackSummary.tsx Outdated Show resolved Hide resolved

change the way date and time are sent to the backend, still draft-ish

e1e7f99

vishnupsatish requested a review from a team as a code owner June 17, 2025 20:36

vercel bot deployed to Preview June 17, 2025 20:38 View deployment

update based on review

4a81d56

vishnupsatish removed the request for review from a team June 17, 2025 20:39

vercel bot deployed to Preview June 17, 2025 20:40 View deployment

vishnupsatish commented Jun 17, 2025

View reviewed changes

reset feature flags

d6fdea2

vercel bot deployed to Preview June 17, 2025 20:58 View deployment

michellewzhang reviewed Jun 17, 2025

View reviewed changes

		feedbackSummaryData.num_feedbacks_used === 0 &&
		feedbackSummaryData.success === false,

	/* is this bad design? FeedbackSummary conditionally renders itself if there is a summary, but should that decision be made in its parent instead? causes weird issues like we can't wrap FeedbackSummary with a Container here, it must be wrapped inside the component itself */
	/* the reason i don't generate the summary here instead of in FeedbackSummary is that useFeedbackSummary requires the FeedbackQueryKeys context to be present to be able to parse the start/end date */
	/* another option is to create a new component that wraps the summary and the list, and generate the summary in that new component so the parent would conditionally render the child instead of the child conditionally rendering itself */

		// grid-template-columns: minmax(195px, 1fr) 1.5fr; hmm, need to come back to this, i kinda get what it does but it seems a bit sus, but it works better than what was there previously since before there was invalid CSS here

		const {selection} = usePageFilters();

		const normalizedDateRange = normalizeDateTimeParams(selection.datetime);


		const organization = useOrganization();

		if (!organization.features.includes('user-feedback-ai-summaries')) {

Uh oh!

feat(feedback): frontend to display summary #93567

Are you sure you want to change the base?

feat(feedback): frontend to display summary #93567

Conversation

vishnupsatish commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aliu39 commented Jun 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michellewzhang Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michellewzhang Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vishnupsatish Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vishnupsatish commented Jun 17, 2025

Uh oh!

michellewzhang commented Jun 17, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michellewzhang left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

vishnupsatish commented Jun 13, 2025 •

edited

Loading

michellewzhang Jun 17, 2025 •

edited

Loading

michellewzhang Jun 17, 2025 •

edited

Loading

vishnupsatish Jun 17, 2025 •

edited

Loading

michellewzhang left a comment •

edited

Loading

vishnupsatish commented Jun 17, 2025 •

edited

Loading