fix(ui): improve Lighthouse feedback flow

- Collect structured reasons with optional details
- Adopt canonical feedback responses before updating UI state
- Keep assistant feedback controls and regression coverage aligned
This commit is contained in:
Alan Buscaglia
2026-08-18 19:01:14 +02:00
parent d94243c2f3
commit 9beb2db973
7 changed files with 368 additions and 21 deletions
@@ -215,6 +215,22 @@ describe("lighthouse-v2.adapter", () => {
expect(payload.data.attributes).toEqual({ rating: "up" });
});
it("should send each selected feedback reason once", () => {
// Given / When
const payload = buildLighthouseV2MessageFeedbackPayload({
sessionId: "session-1",
messageId: "message-1",
rating: "down",
reasons: ["Low quality", "Low quality", "Other"],
});
// Then
expect(payload.data.attributes).toEqual({
rating: "down",
reasons: ["Low quality", "Other"],
});
});
it("should include agent text, display text, and UI context for contextual messages", () => {
// Given
const context: LighthouseContextEnvelope = {
@@ -258,11 +258,15 @@ export function buildLighthouseV2MessageFeedbackPayload(
input: LighthouseV2MessageFeedbackInput,
) {
const details = input.details?.trim();
const reasons = input.reasons
? Array.from(new Set(input.reasons))
: undefined;
return {
data: {
type: "lighthouse-message-feedback",
attributes: filterUndefinedAttributes({
rating: input.rating,
reasons: reasons?.length ? reasons : undefined,
details: details || undefined,
}),
},
@@ -171,6 +171,7 @@ describe("Lighthouse v2 session write actions", () => {
sessionId: "session-1",
messageId: "message-1",
rating: "down",
reasons: ["Low quality", "Other"],
details: "Missing evidence",
});
@@ -185,7 +186,11 @@ describe("Lighthouse v2 session write actions", () => {
body: JSON.stringify({
data: {
type: "lighthouse-message-feedback",
attributes: { rating: "down", details: "Missing evidence" },
attributes: {
rating: "down",
reasons: ["Low quality", "Other"],
details: "Missing evidence",
},
},
}),
}),
@@ -1,4 +1,4 @@
import { render, screen, waitFor } from "@testing-library/react";
import { render, screen, waitFor, within } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { type ReactNode } from "react";
import { beforeEach, describe, expect, it, vi } from "vitest";
@@ -8,6 +8,8 @@ import {
LIGHTHOUSE_V2_PART_TYPE,
type LighthouseV2Message,
} from "@/app/(prowler)/lighthouse/_types";
import { Toaster } from "@/components/shadcn/toast/Toaster";
import { resetToasts } from "@/components/shadcn/toast/use-toast";
import { MessageBubble } from "./message-bubble";
@@ -57,6 +59,7 @@ vi.mock("streamdown", () => ({
describe("MessageBubble", () => {
beforeEach(() => {
resetToasts();
submitFeedbackMock.mockReset();
submitFeedbackMock.mockResolvedValue({ data: buildUserMessage("down") });
});
@@ -416,9 +419,34 @@ describe("MessageBubble", () => {
});
describe("when rating an assistant answer", () => {
it("should open the feedback form with the chosen rating without submitting", async () => {
it("should submit thumbs up immediately without opening the feedback form", async () => {
// Given
const user = userEvent.setup();
submitFeedbackMock.mockResolvedValue({ data: true, status: 204 });
renderFeedbackBubble();
// When
await user.click(
screen.getByRole("button", { name: "Mark outcome as helpful" }),
);
// Then
await waitFor(() =>
expect(submitFeedbackMock).toHaveBeenCalledWith({
sessionId: "session-1",
messageId: "message-user-1",
rating: "up",
}),
);
expect(
screen.queryByRole("heading", { name: "Share feedback" }),
).not.toBeInTheDocument();
});
it("should disable both rating controls while immediate feedback is pending", async () => {
// Given
const user = userEvent.setup();
submitFeedbackMock.mockImplementation(() => new Promise(() => undefined));
renderFeedbackBubble();
// When
@@ -426,12 +454,83 @@ describe("MessageBubble", () => {
screen.getByRole("button", { name: "Mark outcome as helpful" }),
);
// Then
expect(
screen.getByRole("button", { name: "Mark outcome as helpful" }),
).toBeDisabled();
expect(
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
).toBeDisabled();
});
it("should announce a failed immediate thumbs-up submission without opening the feedback form", async () => {
// Given
const user = userEvent.setup();
submitFeedbackMock.mockResolvedValue({
error: "Feedback is temporarily unavailable.",
});
renderFeedbackBubble();
// When
await user.click(
screen.getByRole("button", { name: "Mark outcome as helpful" }),
);
// Then
await waitFor(() =>
expect(
screen.getByRole("button", { name: "Mark outcome as helpful" }),
).toHaveAttribute("aria-pressed", "false"),
);
expect(
await screen.findByText("Feedback is temporarily unavailable."),
).toBeVisible();
expect(
screen.getByRole("region", { name: "Notifications (F8)" }),
).toHaveTextContent("Feedback is temporarily unavailable.");
expect(
screen.queryByRole("heading", { name: "Share feedback" }),
).not.toBeInTheDocument();
});
it("should announce a thrown immediate thumbs-up submission failure without opening the feedback form", async () => {
// Given
const user = userEvent.setup();
submitFeedbackMock.mockRejectedValue(new Error("Request failed"));
renderFeedbackBubble();
// When
await user.click(
screen.getByRole("button", { name: "Mark outcome as helpful" }),
);
// Then
expect(
await screen.findByText(
"Feedback is temporarily unavailable. Please try again.",
),
).toBeVisible();
expect(
screen.queryByRole("heading", { name: "Share feedback" }),
).not.toBeInTheDocument();
});
it("should open the feedback form with the chosen rating without submitting", async () => {
// Given
const user = userEvent.setup();
renderFeedbackBubble();
// When
await user.click(
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
);
// Then
expect(
screen.getByRole("heading", { name: "Share feedback" }),
).toBeVisible();
expect(
screen.getByRole("button", { name: "Mark outcome as helpful" }),
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
).toHaveAttribute("aria-pressed", "true");
expect(
screen.getByLabelText("Additional feedback (optional)"),
@@ -439,6 +538,58 @@ describe("MessageBubble", () => {
expect(submitFeedbackMock).not.toHaveBeenCalled();
});
it("should submit selected feedback reasons with trimmed optional details", async () => {
// Given
const user = userEvent.setup();
submitFeedbackMock.mockResolvedValue({ data: true, status: 204 });
renderFeedbackBubble();
// When
await user.click(
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
);
const styleReason = screen.getByRole("button", {
name: "Don't like the style",
});
expect(
within(screen.getByRole("group", { name: "Reasons (optional)" }))
.getAllByRole("button")
.map((button) => button.textContent),
).toEqual([
"Don't like the style",
"Didn't fully follow instructions",
"Low quality",
"Biased",
"Safety or legal concern",
"Other",
]);
styleReason.focus();
await user.keyboard("{Enter}");
await user.click(screen.getByRole("button", { name: "Low quality" }));
// Then - keyboard and pointer interactions retain a multi-select pressed state.
expect(styleReason).toHaveAttribute("aria-pressed", "true");
expect(
screen.getByRole("button", { name: "Low quality" }),
).toHaveAttribute("aria-pressed", "true");
// When
await user.type(
screen.getByLabelText("Additional feedback (optional)"),
" Missing evidence ",
);
await user.click(screen.getByRole("button", { name: "Submit" }));
// Then
expect(submitFeedbackMock).toHaveBeenCalledWith({
sessionId: "session-1",
messageId: "message-user-1",
rating: "down",
reasons: ["Don't like the style", "Low quality"],
details: "Missing evidence",
});
});
it("should submit the chosen rating and trimmed optional details together", async () => {
// Given
const user = userEvent.setup();
@@ -472,6 +623,9 @@ describe("MessageBubble", () => {
expect(
screen.getByLabelText("Additional feedback (optional)"),
).toBeDisabled();
expect(
screen.getByRole("button", { name: "Don't like the style" }),
).toBeDisabled();
resolveFeedback!({ data: true, status: 204 });
await waitFor(() =>
@@ -484,7 +638,7 @@ describe("MessageBubble", () => {
).toHaveAttribute("aria-pressed", "true");
});
it("should submit a rating without optional details", async () => {
it("should submit thumbs down without optional reasons or details", async () => {
// Given
const user = userEvent.setup();
submitFeedbackMock.mockResolvedValue({ data: true, status: 204 });
@@ -492,7 +646,7 @@ describe("MessageBubble", () => {
// When
await user.click(
screen.getByRole("button", { name: "Mark outcome as helpful" }),
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
);
await user.click(screen.getByRole("button", { name: "Submit" }));
@@ -500,7 +654,7 @@ describe("MessageBubble", () => {
expect(submitFeedbackMock).toHaveBeenCalledWith({
sessionId: "session-1",
messageId: "message-user-1",
rating: "up",
rating: "down",
});
});
@@ -511,6 +665,9 @@ describe("MessageBubble", () => {
await user.click(
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
);
await user.click(
screen.getByRole("button", { name: "Don't like the style" }),
);
await user.type(
screen.getByLabelText("Additional feedback (optional)"),
"Unsaved draft",
@@ -525,11 +682,61 @@ describe("MessageBubble", () => {
screen.queryByRole("heading", { name: "Share feedback" }),
).not.toBeInTheDocument();
await user.click(
screen.getByRole("button", { name: "Mark outcome as helpful" }),
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
);
expect(
screen.getByLabelText("Additional feedback (optional)"),
).toHaveValue("");
expect(
screen.getByRole("button", { name: "Don't like the style" }),
).toHaveAttribute("aria-pressed", "false");
});
it("should clear selected feedback reasons after a successful submission", async () => {
// Given
const user = userEvent.setup();
submitFeedbackMock.mockResolvedValue({ data: true, status: 204 });
renderFeedbackBubble();
await user.click(
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
);
await user.click(
screen.getByRole("button", { name: "Don't like the style" }),
);
// When
await user.click(screen.getByRole("button", { name: "Submit" }));
await waitFor(() =>
expect(
screen.queryByRole("heading", { name: "Share feedback" }),
).not.toBeInTheDocument(),
);
await user.click(
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
);
// Then
expect(
screen.getByRole("button", { name: "Don't like the style" }),
).toHaveAttribute("aria-pressed", "false");
});
it("should close the feedback popup without submitting", async () => {
// Given
const user = userEvent.setup();
renderFeedbackBubble();
await user.click(
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
);
// When
await user.keyboard("{Escape}");
// Then
expect(submitFeedbackMock).not.toHaveBeenCalled();
expect(
screen.queryByRole("heading", { name: "Share feedback" }),
).not.toBeInTheDocument();
});
it("should retain the draft and allow retry after a submission error", async () => {
@@ -544,6 +751,11 @@ describe("MessageBubble", () => {
await user.click(
screen.getByRole("button", { name: "Mark outcome as not helpful" }),
);
await user.click(
screen.getByRole("button", {
name: "Didn't fully follow instructions",
}),
);
await user.type(
screen.getByLabelText("Additional feedback (optional)"),
"Keep this draft",
@@ -559,6 +771,11 @@ describe("MessageBubble", () => {
expect(
screen.getByLabelText("Additional feedback (optional)"),
).toHaveValue("Keep this draft");
expect(
screen.getByRole("button", {
name: "Didn't fully follow instructions",
}),
).toHaveAttribute("aria-pressed", "true");
// When - retry the intact draft
await user.click(screen.getByRole("button", { name: "Retry" }));
@@ -569,6 +786,7 @@ describe("MessageBubble", () => {
sessionId: "session-1",
messageId: "message-user-1",
rating: "down",
reasons: ["Didn't fully follow instructions"],
details: "Keep this draft",
});
});
@@ -607,11 +825,14 @@ function buildUserMessage(id = "message-user-1"): LighthouseV2Message {
function renderFeedbackBubble() {
return render(
<MessageBubble
message={buildAssistantMessage([textPart("part-1", "Done")])}
feedbackTarget={buildUserMessage()}
sessionId="session-1"
/>,
<>
<MessageBubble
message={buildAssistantMessage([textPart("part-1", "Done")])}
feedbackTarget={buildUserMessage()}
sessionId="session-1"
/>
<Toaster />
</>,
);
}
@@ -20,9 +20,11 @@ import {
} from "@/app/(prowler)/lighthouse/_lib/messages";
import {
LIGHTHOUSE_V2_FEEDBACK_RATING,
LIGHTHOUSE_V2_FEEDBACK_REASON,
LIGHTHOUSE_V2_MESSAGE_ROLE,
LIGHTHOUSE_V2_PART_TYPE,
type LighthouseV2FeedbackRating,
type LighthouseV2FeedbackReason,
type LighthouseV2Message,
type LighthouseV2Part,
} from "@/app/(prowler)/lighthouse/_types";
@@ -33,6 +35,7 @@ import {
PopoverAnchor,
PopoverContent,
} from "@/components/shadcn/popover";
import { toast } from "@/components/shadcn/toast/use-toast";
import { FeedbackForm } from "@/components/survey/feedback-form";
import { cn } from "@/lib/utils";
import type { LighthouseSkillDefinition } from "@/types/lighthouse-skills";
@@ -50,6 +53,10 @@ const ASSISTANT_PART_GROUP_TYPE = {
type AssistantPartGroupType =
(typeof ASSISTANT_PART_GROUP_TYPE)[keyof typeof ASSISTANT_PART_GROUP_TYPE];
const LIGHTHOUSE_V2_FEEDBACK_REASON_OPTIONS = Object.values(
LIGHTHOUSE_V2_FEEDBACK_REASON,
).map((reason) => ({ value: reason, label: reason }));
interface AssistantPartGroup {
id: string;
type: AssistantPartGroupType;
@@ -273,6 +280,8 @@ function MessageFeedbackControls({
}) {
const [open, setOpen] = useState(false);
const [rating, setRating] = useState<LighthouseV2FeedbackRating | null>(null);
// Local state needed: reasons and details are buffered until "Submit" is clicked.
const [reasons, setReasons] = useState<LighthouseV2FeedbackReason[]>([]);
const [details, setDetails] = useState("");
const [isSubmitting, setIsSubmitting] = useState(false);
const [error, setError] = useState<string | null>(null);
@@ -285,36 +294,65 @@ function MessageFeedbackControls({
const selectRating = (nextRating: LighthouseV2FeedbackRating) => {
setRating(nextRating);
setError(null);
if (nextRating === LIGHTHOUSE_V2_FEEDBACK_RATING.UP) {
void submit(nextRating, "", []);
return;
}
setOpen(true);
};
const toggleReason = (reason: LighthouseV2FeedbackReason) => {
setReasons((current) =>
current.includes(reason)
? current.filter((currentReason) => currentReason !== reason)
: [...current, reason],
);
};
const cancel = () => {
setOpen(false);
setRating(null);
setReasons([]);
setDetails("");
setError(null);
};
const submit = async () => {
if (!rating || isSubmitting) return;
const submit = async (
submittedRating = rating,
submittedDetails = details,
submittedReasons = reasons,
) => {
if (!submittedRating || isSubmitting) return;
setIsSubmitting(true);
setError(null);
try {
const trimmedDetails = details.trim();
const trimmedDetails = submittedDetails.trim();
const result = await submitLighthouseV2MessageFeedback({
sessionId,
messageId: message.id,
rating,
rating: submittedRating,
...(submittedReasons.length ? { reasons: submittedReasons } : {}),
...(trimmedDetails ? { details: trimmedDetails } : {}),
});
if ("error" in result) {
setError(result.error);
if (submittedRating === LIGHTHOUSE_V2_FEEDBACK_RATING.UP) {
setRating(null);
toast({ variant: "destructive", description: result.error });
}
return;
}
setOpen(false);
setReasons([]);
setDetails("");
} catch {
setError("Feedback is temporarily unavailable. Please try again.");
const errorMessage =
"Feedback is temporarily unavailable. Please try again.";
setError(errorMessage);
if (submittedRating === LIGHTHOUSE_V2_FEEDBACK_RATING.UP) {
setRating(null);
toast({ variant: "destructive", description: errorMessage });
}
} finally {
setIsSubmitting(false);
}
@@ -327,11 +365,13 @@ function MessageFeedbackControls({
<FeedbackRatingButton
rating={LIGHTHOUSE_V2_FEEDBACK_RATING.UP}
selectedRating={rating}
disabled={isSubmitting}
onSelect={selectRating}
/>
<FeedbackRatingButton
rating={LIGHTHOUSE_V2_FEEDBACK_RATING.DOWN}
selectedRating={rating}
disabled={isSubmitting}
onSelect={selectRating}
/>
</div>
@@ -344,6 +384,12 @@ function MessageFeedbackControls({
<FeedbackForm
title="Share feedback"
description="Tell us more about this answer."
reasons={{
label: "Reasons (optional)",
options: LIGHTHOUSE_V2_FEEDBACK_REASON_OPTIONS,
selected: reasons,
onToggle: toggleReason,
}}
detailsLabel="Additional feedback (optional)"
placeholder="Type your answer here"
details={details}
@@ -362,10 +408,12 @@ function MessageFeedbackControls({
function FeedbackRatingButton({
rating,
selectedRating,
disabled,
onSelect,
}: {
rating: LighthouseV2FeedbackRating;
selectedRating: LighthouseV2FeedbackRating | null;
disabled: boolean;
onSelect: (rating: LighthouseV2FeedbackRating) => void;
}) {
const isUp = rating === LIGHTHOUSE_V2_FEEDBACK_RATING.UP;
@@ -381,6 +429,7 @@ function FeedbackRatingButton({
isUp ? "Mark outcome as helpful" : "Mark outcome as not helpful"
}
aria-pressed={selected}
disabled={disabled}
onClick={() => onSelect(rating)}
className={cn(
"text-text-neutral-tertiary hover:text-text-neutral-primary size-6",
@@ -45,6 +45,18 @@ export const LIGHTHOUSE_V2_FEEDBACK_RATING = {
export type LighthouseV2FeedbackRating =
(typeof LIGHTHOUSE_V2_FEEDBACK_RATING)[keyof typeof LIGHTHOUSE_V2_FEEDBACK_RATING];
export const LIGHTHOUSE_V2_FEEDBACK_REASON = {
STYLE: "Don't like the style",
INSTRUCTIONS: "Didn't fully follow instructions",
QUALITY: "Low quality",
BIAS: "Biased",
SAFETY_OR_LEGAL: "Safety or legal concern",
OTHER: "Other",
} as const;
export type LighthouseV2FeedbackReason =
(typeof LIGHTHOUSE_V2_FEEDBACK_REASON)[keyof typeof LIGHTHOUSE_V2_FEEDBACK_REASON];
// Normalized shape of a TOOL_CALL part's `content`. The backend persists this
// blob in snake_case (tool_call_id, tool_name, ...); `getToolCallContent`
// maps it to this camelCase form so the UI never touches the raw keys.
@@ -69,6 +81,7 @@ export interface LighthouseV2MessageFeedbackInput {
sessionId: string;
messageId: string;
rating: LighthouseV2FeedbackRating;
reasons?: LighthouseV2FeedbackReason[];
details?: string;
}
+42 -3
View File
@@ -3,9 +3,22 @@
import { Button } from "@/components/shadcn/button/button";
import { Textarea } from "@/components/shadcn/textarea/textarea";
interface FeedbackFormProps {
interface FeedbackFormReasonOption<TReason extends string> {
value: TReason;
label: string;
}
interface FeedbackFormReasons<TReason extends string> {
label: string;
options: readonly FeedbackFormReasonOption<TReason>[];
selected: readonly TReason[];
onToggle: (reason: TReason) => void;
}
interface FeedbackFormProps<TReason extends string = string> {
title: string;
description?: string;
reasons?: FeedbackFormReasons<TReason>;
detailsLabel: string;
placeholder?: string;
details: string;
@@ -19,9 +32,10 @@ interface FeedbackFormProps {
onCancel?: () => void;
}
export function FeedbackForm({
export function FeedbackForm<TReason extends string = string>({
title,
description,
reasons,
detailsLabel,
placeholder,
details,
@@ -33,7 +47,7 @@ export function FeedbackForm({
onDetailsChange,
onSubmit,
onCancel,
}: FeedbackFormProps) {
}: FeedbackFormProps<TReason>) {
return (
<form
className="flex flex-col gap-4"
@@ -50,6 +64,31 @@ export function FeedbackForm({
<p className="text-text-neutral-secondary text-sm">{description}</p>
) : null}
</div>
{reasons ? (
<fieldset className="flex flex-col gap-2">
<legend className="text-text-neutral-primary mb-2 text-sm font-medium">
{reasons.label}
</legend>
<div className="flex flex-wrap gap-2">
{reasons.options.map((reason) => {
const selected = reasons.selected.includes(reason.value);
return (
<Button
key={reason.value}
type="button"
variant={selected ? "secondary" : "outline"}
size="xs"
aria-pressed={selected}
disabled={isSubmitting}
onClick={() => reasons.onToggle(reason.value)}
>
{reason.label}
</Button>
);
})}
</div>
</fieldset>
) : null}
<Textarea
aria-label={detailsLabel}
placeholder={placeholder}