From bf8ba851f280cbb49f882ebdcb6512dabf07d34a Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Tue, 25 Aug 2026 14:41:17 -0400 Subject: [PATCH 1/4] Add distribution-owned response feedback UI Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-019fbabe-6c0e-70ab-8559-31ba601d4897 --- src-tauri/src/commands/feedback.rs | 1 + src-tauri/src/commands/runtime_config.rs | 3 + .../ResponseFeedbackControls.test.tsx | 58 +++++ .../ResponseFeedbackControls.tsx | 100 +++++++++ .../feedbackSurveyEvents.test.ts | 41 ++++ .../response-feedback/feedbackSurveyEvents.ts | 38 ++++ .../response-feedback/feedbackSurveySink.ts | 15 ++ .../responseFeedbackRows.test.ts | 46 ++++ .../response-feedback/responseFeedbackRows.ts | 30 +++ .../responseFeedbackState.test.ts | 120 ++++++++++ .../responseFeedbackState.ts | 208 ++++++++++++++++++ src/features/chat/ui/MessageBubble.tsx | 37 ++++ src/features/chat/ui/MessageBubbleActions.tsx | 14 ++ src/features/chat/ui/MessageTimeline.tsx | 12 + .../chat/ui/VirtualMessageTimeline.tsx | 8 + .../chat/ui/VirtualMessageTimelineGate.tsx | 2 +- src/features/chat/ui/VirtualTranscriptRow.tsx | 6 + src/shared/i18n/locales/en/chat.json | 2 + src/shared/i18n/locales/es/chat.json | 2 + src/shared/runtime-config/schema.test.ts | 9 + src/shared/runtime-config/schema.ts | 1 + 21 files changed, 752 insertions(+), 1 deletion(-) create mode 100644 src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx create mode 100644 src/features/chat/response-feedback/ResponseFeedbackControls.tsx create mode 100644 src/features/chat/response-feedback/feedbackSurveyEvents.test.ts create mode 100644 src/features/chat/response-feedback/feedbackSurveyEvents.ts create mode 100644 src/features/chat/response-feedback/feedbackSurveySink.ts create mode 100644 src/features/chat/response-feedback/responseFeedbackRows.test.ts create mode 100644 src/features/chat/response-feedback/responseFeedbackRows.ts create mode 100644 src/features/chat/response-feedback/responseFeedbackState.test.ts create mode 100644 src/features/chat/response-feedback/responseFeedbackState.ts diff --git a/src-tauri/src/commands/feedback.rs b/src-tauri/src/commands/feedback.rs index 830a01060..6149eca02 100644 --- a/src-tauri/src/commands/feedback.rs +++ b/src-tauri/src/commands/feedback.rs @@ -540,6 +540,7 @@ mod tests { let disabled = runtime_config_with_feedback(Some(RuntimeFeedbackConfig { enabled: Some(false), project_key: Some("CUSTOM".to_string()), + response_rating_enabled: None, })); assert!(!feedback_enabled(&disabled)); assert_eq!(feedback_project_key(&disabled), "CUSTOM"); diff --git a/src-tauri/src/commands/runtime_config.rs b/src-tauri/src/commands/runtime_config.rs index 6df36c249..4dfa28696 100644 --- a/src-tauri/src/commands/runtime_config.rs +++ b/src-tauri/src/commands/runtime_config.rs @@ -166,6 +166,8 @@ pub struct RuntimeFeedbackConfig { pub enabled: Option, #[serde(skip_serializing_if = "Option::is_none", default)] pub project_key: Option, + #[serde(skip_serializing_if = "Option::is_none", default)] + pub response_rating_enabled: Option, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -1194,6 +1196,7 @@ mod tests { feedback: Some(RuntimeFeedbackConfig { enabled: Some(true), project_key: Some("BOT".to_string()), + response_rating_enabled: Some(true), }), kgoose: Some(RuntimeKgooseConfig { base_url: Some("https://kgoose.example.test".to_string()), diff --git a/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx b/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx new file mode 100644 index 000000000..f4498d101 --- /dev/null +++ b/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx @@ -0,0 +1,58 @@ +import { act, render } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { feedbackSurveySink } from "./feedbackSurveySink"; +import { ResponseFeedbackControls } from "./ResponseFeedbackControls"; + +vi.mock("./feedbackSurveySink", () => ({ feedbackSurveySink: vi.fn() })); + +const sink = vi.mocked(feedbackSurveySink); +let intersectionCallback: IntersectionObserverCallback; + +class MockIntersectionObserver { + constructor(callback: IntersectionObserverCallback) { + intersectionCallback = callback; + } + observe() {} + disconnect() {} +} + +describe("ResponseFeedbackControls", () => { + beforeEach(() => { + localStorage.clear(); + sink.mockClear(); + vi.stubGlobal("IntersectionObserver", MockIntersectionObserver); + }); + + it("records an appearance when persistent controls enter the viewport", () => { + render( + , + ); + + expect(sink).not.toHaveBeenCalled(); + act(() => { + intersectionCallback( + [{ isIntersecting: true } as IntersectionObserverEntry], + {} as IntersectionObserver, + ); + }); + expect(sink).toHaveBeenCalledWith( + expect.objectContaining({ eventType: "appeared" }), + ); + }); + + it("does not record hidden controls merely because they are mounted", () => { + render( + , + ); + + expect(sink).not.toHaveBeenCalled(); + }); +}); diff --git a/src/features/chat/response-feedback/ResponseFeedbackControls.tsx b/src/features/chat/response-feedback/ResponseFeedbackControls.tsx new file mode 100644 index 000000000..83740a347 --- /dev/null +++ b/src/features/chat/response-feedback/ResponseFeedbackControls.tsx @@ -0,0 +1,100 @@ +import { useEffect, useRef, useState } from "react"; +import { ThumbsDown, ThumbsUp } from "lucide-react"; +import { useTranslation } from "react-i18next"; +import { cn } from "@/shared/lib/cn"; +import { MessageAction } from "@/shared/ui/ai-elements/message"; +import { + getResponseFeedbackSelection, + markResponseFeedbackAppeared, + setResponseFeedbackSelection, + type ResponseFeedbackSelection, +} from "./responseFeedbackState"; + +interface ResponseFeedbackControlsProps { + sessionId: string; + messageId: string; + persistentlyVisible: boolean; +} + +export function ResponseFeedbackControls({ + sessionId, + messageId, + persistentlyVisible, +}: ResponseFeedbackControlsProps) { + const { t } = useTranslation("chat"); + const controlsRef = useRef(null); + const [selection, setSelection] = useState( + () => getResponseFeedbackSelection(sessionId, messageId), + ); + + useEffect(() => { + setSelection(getResponseFeedbackSelection(sessionId, messageId)); + }, [messageId, sessionId]); + + useEffect(() => { + const target = controlsRef.current; + if ( + !persistentlyVisible || + !target || + typeof IntersectionObserver === "undefined" + ) { + return; + } + const observer = new IntersectionObserver((entries) => { + if (entries.some((entry) => entry.isIntersecting)) { + markResponseFeedbackAppeared(sessionId, messageId); + observer.disconnect(); + } + }); + observer.observe(target); + return () => observer.disconnect(); + }, [messageId, persistentlyVisible, sessionId]); + + const select = (requested: ResponseFeedbackSelection) => { + const current = getResponseFeedbackSelection(sessionId, messageId); + const next = current === requested ? null : requested; + setSelection(setResponseFeedbackSelection(sessionId, messageId, next)); + }; + const goodSelected = selection === "good"; + const badSelected = selection === "bad"; + const selectedClassName = + "bg-accent text-foreground hover:bg-accent active:bg-accent"; + + return ( + markResponseFeedbackAppeared(sessionId, messageId)} + onFocusCapture={() => markResponseFeedbackAppeared(sessionId, messageId)} + > + select("good")} + > + + + select("bad")} + > + + + + ); +} diff --git a/src/features/chat/response-feedback/feedbackSurveyEvents.test.ts b/src/features/chat/response-feedback/feedbackSurveyEvents.test.ts new file mode 100644 index 000000000..1d0731eb9 --- /dev/null +++ b/src/features/chat/response-feedback/feedbackSurveyEvents.test.ts @@ -0,0 +1,41 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { feedbackSurveySink } from "./feedbackSurveySink"; +import { sendFeedbackSurveyEvent } from "./feedbackSurveyEvents"; + +vi.mock("./feedbackSurveySink", () => ({ feedbackSurveySink: vi.fn() })); + +const sink = vi.mocked(feedbackSurveySink); + +describe("feedbackSurveyEvents", () => { + beforeEach(() => { + localStorage.clear(); + sink.mockClear(); + }); + + it("assigns a persistent session-wide event sequence", () => { + sendFeedbackSurveyEvent({ + sessionId: "session", + messageId: "message", + appearanceId: "appearance", + surveyType: "response", + eventType: "appeared", + }); + sendFeedbackSurveyEvent({ + sessionId: "session", + messageId: "message", + appearanceId: "appearance", + surveyType: "response", + eventType: "responded", + response: "good", + }); + + expect(sink).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ eventSequence: 1 }), + ); + expect(sink).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ eventSequence: 2 }), + ); + }); +}); diff --git a/src/features/chat/response-feedback/feedbackSurveyEvents.ts b/src/features/chat/response-feedback/feedbackSurveyEvents.ts new file mode 100644 index 000000000..7d2f85693 --- /dev/null +++ b/src/features/chat/response-feedback/feedbackSurveyEvents.ts @@ -0,0 +1,38 @@ +import { + type FeedbackSurveySinkEvent, + feedbackSurveySink, +} from "./feedbackSurveySink"; + +export type FeedbackSurveyEventInput = Omit< + FeedbackSurveySinkEvent, + "eventSequence" +>; + +const FEEDBACK_SEQUENCE_STORAGE_PREFIX = "berd:feedback-event-sequence:v1:"; +const volatileSequences = new Map(); + +function nextFeedbackEventSequence(sessionId: string): number { + const key = `${FEEDBACK_SEQUENCE_STORAGE_PREFIX}${sessionId}`; + let current = volatileSequences.get(key) ?? 0; + try { + const stored = Number(localStorage.getItem(key)); + if (Number.isSafeInteger(stored) && stored > current) current = stored; + } catch { + // The in-memory counter still preserves ordering for this app process. + } + const next = current + 1; + volatileSequences.set(key, next); + try { + localStorage.setItem(key, String(next)); + } catch { + // Persistence is best-effort; feedback delivery must not affect chat. + } + return next; +} + +export function sendFeedbackSurveyEvent(input: FeedbackSurveyEventInput): void { + feedbackSurveySink({ + ...input, + eventSequence: nextFeedbackEventSequence(input.sessionId), + }); +} diff --git a/src/features/chat/response-feedback/feedbackSurveySink.ts b/src/features/chat/response-feedback/feedbackSurveySink.ts new file mode 100644 index 000000000..87079a953 --- /dev/null +++ b/src/features/chat/response-feedback/feedbackSurveySink.ts @@ -0,0 +1,15 @@ +export type FeedbackSurveyEventType = "appeared" | "responded"; +export type FeedbackSurveyResponse = "good" | "bad" | "cleared"; + +export interface FeedbackSurveySinkEvent { + sessionId: string; + messageId: string; + appearanceId: string; + surveyType: "response"; + eventSequence: number; + eventType: FeedbackSurveyEventType; + response?: FeedbackSurveyResponse; +} + +/** Distribution-owned transport seam; stock Berd intentionally sends nothing. */ +export function feedbackSurveySink(_event: FeedbackSurveySinkEvent): void {} diff --git a/src/features/chat/response-feedback/responseFeedbackRows.test.ts b/src/features/chat/response-feedback/responseFeedbackRows.test.ts new file mode 100644 index 000000000..fdd002543 --- /dev/null +++ b/src/features/chat/response-feedback/responseFeedbackRows.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "vitest"; +import { selectResponseFeedbackRowIds } from "./responseFeedbackRows"; + +describe("selectResponseFeedbackRowIds", () => { + it("prefers the answer over companion rows", () => { + expect([ + ...selectResponseFeedbackRowIds([ + { + kind: "message", + rowId: "message:assistant:companion-image", + messageId: "assistant", + responseStartMessageId: "assistant", + }, + { + kind: "message", + rowId: "message:assistant:answer", + messageId: "assistant", + responseStartMessageId: "assistant", + }, + { + kind: "message", + rowId: "message:assistant:companion-mcp-app", + messageId: "assistant", + responseStartMessageId: "assistant", + }, + ]), + ]).toEqual(["message:assistant:answer"]); + }); + + it("uses one final host row when there is no answer row", () => { + expect([ + ...selectResponseFeedbackRowIds([ + { + kind: "assistant-content-fragment", + rowId: "message:assistant:fragment-0", + messageId: "assistant", + }, + { + kind: "assistant-content-fragment", + rowId: "message:assistant:fragment-1", + messageId: "assistant", + }, + ]), + ]).toEqual(["message:assistant:fragment-1"]); + }); +}); diff --git a/src/features/chat/response-feedback/responseFeedbackRows.ts b/src/features/chat/response-feedback/responseFeedbackRows.ts new file mode 100644 index 000000000..a0de0a1d7 --- /dev/null +++ b/src/features/chat/response-feedback/responseFeedbackRows.ts @@ -0,0 +1,30 @@ +import type { TranscriptRowDescriptor } from "@/features/chat/transcript/projection"; + +type FeedbackRow = Pick< + TranscriptRowDescriptor, + "kind" | "messageId" | "responseStartMessageId" | "rowId" +>; + +export function selectResponseFeedbackRowIds( + rows: readonly FeedbackRow[], +): ReadonlySet { + const selectedByResponse = new Map< + string, + { rowId: string; isAnswer: boolean } + >(); + for (const row of rows) { + if ( + (row.kind !== "message" && row.kind !== "assistant-content-fragment") || + !row.messageId + ) { + continue; + } + const responseId = row.responseStartMessageId ?? row.messageId; + const current = selectedByResponse.get(responseId); + const isAnswer = row.rowId.endsWith(":answer"); + if (!current || isAnswer || !current.isAnswer) { + selectedByResponse.set(responseId, { rowId: row.rowId, isAnswer }); + } + } + return new Set([...selectedByResponse.values()].map(({ rowId }) => rowId)); +} diff --git a/src/features/chat/response-feedback/responseFeedbackState.test.ts b/src/features/chat/response-feedback/responseFeedbackState.test.ts new file mode 100644 index 000000000..58c1397e1 --- /dev/null +++ b/src/features/chat/response-feedback/responseFeedbackState.test.ts @@ -0,0 +1,120 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { Message, MessageContent } from "@/shared/types/messages"; +import { feedbackSurveySink } from "./feedbackSurveySink"; +import { + getResponseFeedbackSelection, + isResponseFeedbackEligible, + markResponseFeedbackAppeared, + setResponseFeedbackSelection, +} from "./responseFeedbackState"; + +vi.mock("./feedbackSurveySink", () => ({ feedbackSurveySink: vi.fn() })); + +const sink = vi.mocked(feedbackSurveySink); + +function assistantMessage(overrides: Partial = {}): Message { + return { + id: "assistant-message", + role: "assistant", + created: Date.now(), + content: [{ type: "text", text: "Done" }], + ...overrides, + }; +} + +describe("responseFeedbackState", () => { + beforeEach(() => { + localStorage.clear(); + sink.mockClear(); + }); + + it("emits one appeared event and persists it", () => { + expect(markResponseFeedbackAppeared("appeared-session", "message")).toBe( + true, + ); + expect(markResponseFeedbackAppeared("appeared-session", "message")).toBe( + false, + ); + expect(sink).toHaveBeenCalledTimes(1); + expect(sink).toHaveBeenCalledWith( + expect.objectContaining({ + sessionId: "appeared-session", + eventType: "appeared", + }), + ); + }); + + it("emits selections, switches, and clears without duplicate transitions", () => { + expect( + setResponseFeedbackSelection("selection-session", "message", "good"), + ).toBe("good"); + expect( + setResponseFeedbackSelection("selection-session", "message", "good"), + ).toBe("good"); + expect( + setResponseFeedbackSelection("selection-session", "message", "bad"), + ).toBe("bad"); + expect( + setResponseFeedbackSelection("selection-session", "message", null), + ).toBeNull(); + expect( + getResponseFeedbackSelection("selection-session", "message"), + ).toBeNull(); + + expect(sink).toHaveBeenCalledTimes(4); + expect(sink.mock.calls.map(([event]) => event)).toEqual([ + expect.objectContaining({ eventType: "appeared" }), + expect.objectContaining({ eventType: "responded", response: "good" }), + expect.objectContaining({ eventType: "responded", response: "bad" }), + expect.objectContaining({ eventType: "responded", response: "cleared" }), + ]); + }); + + it("only allows completed, user-visible assistant responses", () => { + const visibleText: MessageContent[] = [{ type: "text", text: "Done" }]; + const eligible = ( + message: Message, + content = visibleText, + isStreaming = false, + ) => isResponseFeedbackEligible({ message, content, isStreaming }); + + expect(eligible(assistantMessage())).toBe(true); + expect( + eligible(assistantMessage(), [ + { + type: "mcpApp", + id: "mcp-app", + payload: { + sessionId: "session", + toolCallId: "tool-call", + toolCallTitle: "Interactive result", + source: "toolCallUpdateMeta", + tool: { + name: "show_result", + extensionName: "example", + resourceUri: "ui://example/result", + }, + resource: { result: null }, + }, + }, + ]), + ).toBe(true); + expect(eligible(assistantMessage(), visibleText, true)).toBe(false); + expect( + eligible(assistantMessage({ metadata: { completionStatus: "error" } })), + ).toBe(false); + expect(eligible({ ...assistantMessage(), role: "user" })).toBe(false); + expect(eligible(assistantMessage(), [{ type: "text", text: " " }])).toBe( + false, + ); + expect( + eligible(assistantMessage(), [ + { + type: "text", + text: "internal", + annotations: { audience: ["assistant"] }, + }, + ]), + ).toBe(false); + }); +}); diff --git a/src/features/chat/response-feedback/responseFeedbackState.ts b/src/features/chat/response-feedback/responseFeedbackState.ts new file mode 100644 index 000000000..a3489e9a7 --- /dev/null +++ b/src/features/chat/response-feedback/responseFeedbackState.ts @@ -0,0 +1,208 @@ +import type { Message, MessageContent } from "@/shared/types/messages"; +import { sendFeedbackSurveyEvent } from "./feedbackSurveyEvents"; + +export type ResponseFeedbackSelection = "good" | "bad"; + +interface StoredResponseFeedback { + version: 1; + appearanceId: string; + appeared: boolean; + response: ResponseFeedbackSelection | null; +} + +const RESPONSE_FEEDBACK_STORAGE_PREFIX = "berd:response-feedback:v1:"; +const volatileRecords = new Map(); +const volatileOnlyKeys = new Set(); + +function responseFeedbackStorageKey( + sessionId: string, + messageId: string, +): string { + return `${RESPONSE_FEEDBACK_STORAGE_PREFIX}${JSON.stringify([ + sessionId, + messageId, + ])}`; +} + +function createStoredResponseFeedback(): StoredResponseFeedback { + return { + version: 1, + appearanceId: crypto.randomUUID(), + appeared: false, + response: null, + }; +} + +function parseStoredResponseFeedback( + value: unknown, +): StoredResponseFeedback | null { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return null; + } + const record = value as Record; + if ( + record.version !== 1 || + typeof record.appearanceId !== "string" || + record.appearanceId.length === 0 || + typeof record.appeared !== "boolean" || + (record.response !== null && + record.response !== "good" && + record.response !== "bad") + ) { + return null; + } + return { + version: 1, + appearanceId: record.appearanceId, + appeared: record.appeared, + response: record.response, + }; +} + +function readResponseFeedback( + sessionId: string, + messageId: string, +): StoredResponseFeedback { + const key = responseFeedbackStorageKey(sessionId, messageId); + if (volatileOnlyKeys.has(key)) { + return volatileRecords.get(key) ?? createStoredResponseFeedback(); + } + + try { + const raw = window.localStorage.getItem(key); + if (raw) { + const parsed = parseStoredResponseFeedback(JSON.parse(raw)); + if (parsed) { + volatileRecords.set(key, parsed); + return parsed; + } + } + } catch { + return volatileRecords.get(key) ?? createStoredResponseFeedback(); + } + + return createStoredResponseFeedback(); +} + +function writeResponseFeedback( + sessionId: string, + messageId: string, + record: StoredResponseFeedback, +): void { + const key = responseFeedbackStorageKey(sessionId, messageId); + volatileRecords.set(key, record); + try { + window.localStorage.setItem(key, JSON.stringify(record)); + volatileOnlyKeys.delete(key); + } catch { + volatileOnlyKeys.add(key); + } +} + +function emitResponseFeedback( + sessionId: string, + messageId: string, + record: StoredResponseFeedback, + event: + | { eventType: "appeared" } + | { + eventType: "responded"; + response: ResponseFeedbackSelection | "cleared"; + }, +): void { + sendFeedbackSurveyEvent({ + sessionId, + messageId, + appearanceId: record.appearanceId, + surveyType: "response", + ...event, + }); +} + +export function getResponseFeedbackSelection( + sessionId: string, + messageId: string, +): ResponseFeedbackSelection | null { + return readResponseFeedback(sessionId, messageId).response; +} + +export function markResponseFeedbackAppeared( + sessionId: string, + messageId: string, +): boolean { + const current = readResponseFeedback(sessionId, messageId); + if (current.appeared) { + return false; + } + const next = { ...current, appeared: true }; + writeResponseFeedback(sessionId, messageId, next); + emitResponseFeedback(sessionId, messageId, next, { eventType: "appeared" }); + return true; +} + +export function setResponseFeedbackSelection( + sessionId: string, + messageId: string, + selection: ResponseFeedbackSelection | null, +): ResponseFeedbackSelection | null { + const current = readResponseFeedback(sessionId, messageId); + if (current.response === selection) { + return current.response; + } + + const next = { ...current, appeared: true, response: selection }; + writeResponseFeedback(sessionId, messageId, next); + if (!current.appeared) { + emitResponseFeedback(sessionId, messageId, next, { eventType: "appeared" }); + } + emitResponseFeedback(sessionId, messageId, next, { + eventType: "responded", + response: selection ?? "cleared", + }); + return next.response; +} + +function isUserVisibleContent(content: MessageContent): boolean { + const audience = + "annotations" in content ? content.annotations?.audience : undefined; + return !audience || audience.length === 0 || audience.includes("user"); +} + +function isResponseContent(content: MessageContent): boolean { + if (!isUserVisibleContent(content)) { + return false; + } + if (content.type === "text") { + return content.text.trim().length > 0; + } + return content.type === "image" || content.type === "mcpApp"; +} + +export function isResponseFeedbackEligible({ + message, + content, + isStreaming, +}: { + message: Message; + content: readonly MessageContent[]; + isStreaming: boolean; +}): boolean { + if ( + message.role !== "assistant" || + message.metadata?.userVisible === false || + isStreaming + ) { + return false; + } + + const completionStatus = message.metadata?.completionStatus; + if ( + completionStatus === "inProgress" || + completionStatus === "error" || + completionStatus === "stopped" + ) { + return false; + } + + return content.some(isResponseContent); +} diff --git a/src/features/chat/ui/MessageBubble.tsx b/src/features/chat/ui/MessageBubble.tsx index 3882ae4ec..187f347a9 100644 --- a/src/features/chat/ui/MessageBubble.tsx +++ b/src/features/chat/ui/MessageBubble.tsx @@ -56,8 +56,14 @@ import type { } from "@/shared/types/messages"; import { Button } from "@/shared/ui/button"; import { LinkifiedText } from "@/shared/ui/LinkifiedText"; +import { useProfileCapability } from "@/shared/profile/capabilities"; +import { useRuntimeConfigStore } from "@/shared/runtime-config/runtimeConfigStore"; import { MessageBubbleActions } from "./MessageBubbleActions"; import { MessageMetadataChip } from "./MessageMetadataChip"; +import { + isResponseFeedbackEligible, + markResponseFeedbackAppeared, +} from "../response-feedback/responseFeedbackState"; import { couldOverflowUserMessagePreview, UserMessageClamp, @@ -339,6 +345,7 @@ interface MessageBubbleProps { contentOverride?: readonly MessageContent[]; contentContext?: readonly MessageContent[]; actionMessageId?: string; + feedbackSessionId?: string; fragmentRole?: "single" | "start" | "middle" | "end"; onCopy?: () => void; onRetryMessage?: (messageId: string) => void; @@ -694,6 +701,7 @@ export const MessageBubble = memo(function MessageBubble({ contentOverride, contentContext, actionMessageId = message.id, + feedbackSessionId, fragmentRole, onRetryMessage, onEditMessage, @@ -728,6 +736,10 @@ export const MessageBubble = memo(function MessageBubble({ const { isCopied: isCopyConfirmed, copyToClipboard } = useCopyToClipboard(); const hasPersonaAvatar = Boolean(persona?.avatar); const catalogEntries = useProviderCatalogStore((state) => state.entries); + const feedbackEnabled = useProfileCapability("feedback"); + const responseRatingEnabled = useRuntimeConfigStore( + (state) => state.config.feedback?.responseRatingEnabled === true, + ); const runItCodeRenderers = useMemo( () => onRunShellCommand @@ -883,6 +895,22 @@ export const MessageBubble = memo(function MessageBubble({ showMessageActions || (!isUser && isStreaming && canHostMessageActions); const messageActionsArePersistentlyVisible = actionsAlwaysVisible || isCopyConfirmed; + const responseFeedback = + canHostMessageActions && + feedbackEnabled && + responseRatingEnabled && + feedbackSessionId && + isResponseFeedbackEligible({ + message, + content, + isStreaming: Boolean(isStreaming), + }) + ? { + sessionId: feedbackSessionId, + messageId: actionMessageId, + persistentlyVisible: messageActionsArePersistentlyVisible, + } + : undefined; const outerSpacingClassName = fragmentRole === "start" ? "pt-1 pb-0" @@ -964,6 +992,14 @@ export const MessageBubble = memo(function MessageBubble({ ? "max-w-[var(--chat-user-message-max-width)] items-end" : "w-full items-start", )} + onPointerEnter={() => { + if (responseFeedback) { + markResponseFeedbackAppeared( + responseFeedback.sessionId, + responseFeedback.messageId, + ); + } + }} > {showAssistantIdentity ? (
@@ -1124,6 +1160,7 @@ export const MessageBubble = memo(function MessageBubble({ !isUser && !isStreaming ? onJumpToResponseStart : undefined } onForkFromMessage={!isStreaming ? onForkFromMessage : undefined} + responseFeedback={responseFeedback} showJumpToResponseStartHint={ !isUser && !isStreaming ? showJumpToResponseStartHint : false } diff --git a/src/features/chat/ui/MessageBubbleActions.tsx b/src/features/chat/ui/MessageBubbleActions.tsx index 616b36498..14e6c33d9 100644 --- a/src/features/chat/ui/MessageBubbleActions.tsx +++ b/src/features/chat/ui/MessageBubbleActions.tsx @@ -6,6 +6,7 @@ import { cn } from "@/shared/lib/cn"; import { MessageAction, MessageActions } from "@/shared/ui/ai-elements/message"; import { Popover, PopoverContent, PopoverTrigger } from "@/shared/ui/popover"; import { Button } from "@/shared/ui/button"; +import { ResponseFeedbackControls } from "../response-feedback/ResponseFeedbackControls"; interface MessageBubbleActionsProps { isUser: boolean; @@ -19,6 +20,11 @@ interface MessageBubbleActionsProps { onJumpToResponseStart?: (messageId: string) => void; onForkFromMessage?: (messageId: string) => void; showJumpToResponseStartHint?: boolean; + responseFeedback?: { + sessionId: string; + messageId: string; + persistentlyVisible: boolean; + }; onJumpToResponseStartHintClose?: (messageId: string) => void; onJumpToResponseStartHintDismiss?: (messageId: string) => void; } @@ -35,6 +41,7 @@ export function MessageBubbleActions({ onJumpToResponseStart, onForkFromMessage, showJumpToResponseStartHint, + responseFeedback, onJumpToResponseStartHintClose, onJumpToResponseStartHintDismiss, }: MessageBubbleActionsProps) { @@ -163,6 +170,13 @@ export function MessageBubbleActions({ )} + {!isUser && responseFeedback ? ( + + ) : null} {!isUser && timestamp} ); diff --git a/src/features/chat/ui/MessageTimeline.tsx b/src/features/chat/ui/MessageTimeline.tsx index 36d7eea4e..4aac140fd 100644 --- a/src/features/chat/ui/MessageTimeline.tsx +++ b/src/features/chat/ui/MessageTimeline.tsx @@ -15,6 +15,7 @@ import { cn } from "@/shared/lib/cn"; import { useLocaleFormatting } from "@/shared/i18n"; import { TranscriptSearchSkip } from "./TranscriptSearchSkip"; import { MessageTimelineScrollContainer } from "./MessageTimelineScrollContainer"; +import { selectResponseFeedbackRowIds } from "../response-feedback/responseFeedbackRows"; import type { Message } from "@/shared/types/messages"; import { createTranscriptProjectionCache, @@ -68,6 +69,7 @@ const GUTTER_RESPONSE_START_THRESHOLD_PX = 16; interface MessageTimelineProps extends MessageTimelineBubbleCallbacks { messages: Message[]; + feedbackSessionId?: string; streamingMessageId?: string | null; scrollTargetMessageId?: string | null; scrollTargetQuery?: string | null; @@ -117,6 +119,7 @@ function formatRowDateSeparator( export function MessageTimeline({ messages, + feedbackSessionId, streamingMessageId, scrollTargetMessageId, scrollTargetQuery, @@ -205,6 +208,10 @@ export function MessageTimeline({ }), [messages, nowBucket, streamingMessageId], ); + const responseFeedbackRowIds = useMemo( + () => selectResponseFeedbackRowIds(snapshot.rows), + [snapshot.rows], + ); const visibleMessages = useMemo( () => messages.filter( @@ -1475,6 +1482,11 @@ export function MessageTimeline({ row.messageId === latestAssistantMessageId && (row.responseStartMessageId ?? row.messageId) !== streamingMessageId } + feedbackSessionId={ + responseFeedbackRowIds.has(row.rowId) + ? feedbackSessionId + : undefined + } showJumpToResponseStartHint={ row.messageId === responseStartHintMessageId && responseStartHintActive diff --git a/src/features/chat/ui/VirtualMessageTimeline.tsx b/src/features/chat/ui/VirtualMessageTimeline.tsx index 1e6a3592e..4263ef051 100644 --- a/src/features/chat/ui/VirtualMessageTimeline.tsx +++ b/src/features/chat/ui/VirtualMessageTimeline.tsx @@ -19,6 +19,7 @@ import { useTranslation } from "react-i18next"; import { cn } from "@/shared/lib/cn"; import { useLocaleFormatting } from "@/shared/i18n"; import type { Message } from "@/shared/types/messages"; +import { selectResponseFeedbackRowIds } from "../response-feedback/responseFeedbackRows"; import { ASSISTIVE_UX_RULES } from "@/shared/assistive-ux/registry"; import { hasAssistiveMomentBeenShown, @@ -1112,6 +1113,10 @@ function VirtualMessageTimelineSession({ ], ); const stableRows = useStableTranscriptRows(snapshot.rows); + const responseFeedbackRowIds = useMemo( + () => selectResponseFeedbackRowIds(stableRows), + [stableRows], + ); const [settlingAgentWorkMessageId, setSettlingAgentWorkMessageId] = useState< string | null >(null); @@ -3522,6 +3527,9 @@ function VirtualMessageTimelineSession({ row.messageId === latestAssistantMessageId && (row.responseStartMessageId ?? row.messageId) !== streamingMessageId } + feedbackSessionId={ + responseFeedbackRowIds.has(row.rowId) ? sessionId : undefined + } showJumpToResponseStartHint={ row.messageId === responseStartHintMessageId && responseStartHintIsActive diff --git a/src/features/chat/ui/VirtualMessageTimelineGate.tsx b/src/features/chat/ui/VirtualMessageTimelineGate.tsx index d87aa21c4..5f1fc43f2 100644 --- a/src/features/chat/ui/VirtualMessageTimelineGate.tsx +++ b/src/features/chat/ui/VirtualMessageTimelineGate.tsx @@ -38,7 +38,7 @@ export function VirtualMessageTimelineGate({ ); if (!loadedTranscript) { - return ; + return ; } return ( diff --git a/src/features/chat/ui/VirtualTranscriptRow.tsx b/src/features/chat/ui/VirtualTranscriptRow.tsx index 395aa5532..adaacccdc 100644 --- a/src/features/chat/ui/VirtualTranscriptRow.tsx +++ b/src/features/chat/ui/VirtualTranscriptRow.tsx @@ -45,6 +45,7 @@ interface VirtualTranscriptRowProps { settleAgentWorkOnMount?: boolean; actionsAlwaysVisible?: boolean; showJumpToResponseStartHint?: boolean; + feedbackSessionId?: string; isPulsing?: boolean; rowStateProvider?: TranscriptVirtualRowStateProviderConfig; bubbleCallbacks?: MessageBubbleCallbacks; @@ -70,6 +71,7 @@ export const VirtualTranscriptRow = memo(function VirtualTranscriptRow({ settleAgentWorkOnMount, actionsAlwaysVisible, showJumpToResponseStartHint, + feedbackSessionId, isPulsing, rowStateProvider, bubbleCallbacks, @@ -300,9 +302,11 @@ export const VirtualTranscriptRow = memo(function VirtualTranscriptRow({ message={message} animateEntry={false} contentOverride={row.fragment.content} + actionMessageId={row.responseStartMessageId ?? row.messageId} fragmentRole={row.fragment.role} isStreaming={row.fragment.isStreamingTail && isStreaming} actionsAlwaysVisible={actionsAlwaysVisible} + feedbackSessionId={feedbackSessionId} showJumpToResponseStartHint={showJumpToResponseStartHint} onRetryMessage={ row.fragment.role === "end" || row.fragment.role === "single" @@ -383,6 +387,7 @@ export const VirtualTranscriptRow = memo(function VirtualTranscriptRow({ actionMessageId={row.responseStartMessageId ?? row.messageId} isStreaming={isStreaming} actionsAlwaysVisible={actionsAlwaysVisible} + feedbackSessionId={feedbackSessionId} showJumpToResponseStartHint={showJumpToResponseStartHint} onRetryMessage={ message.role === "assistant" ? onRetryMessage : undefined @@ -447,6 +452,7 @@ function areVirtualTranscriptRowPropsEqual( previous.settleAgentWorkOnMount === next.settleAgentWorkOnMount && previous.actionsAlwaysVisible === next.actionsAlwaysVisible && previous.showJumpToResponseStartHint === next.showJumpToResponseStartHint && + previous.feedbackSessionId === next.feedbackSessionId && previous.isPulsing === next.isPulsing && previous.rowStateProvider === next.rowStateProvider && previous.bubbleCallbacks === next.bubbleCallbacks && diff --git a/src/shared/i18n/locales/en/chat.json b/src/shared/i18n/locales/en/chat.json index aa5775f70..9c2924cd2 100644 --- a/src/shared/i18n/locales/en/chat.json +++ b/src/shared/i18n/locales/en/chat.json @@ -402,6 +402,8 @@ "mcpAppLoading": "Loading MCP App…", "mcpAppRenderError": "Unable to render MCP App inline.", "redactedThinking": "(thinking redacted)", + "responseFeedbackGood": "Good response", + "responseFeedbackBad": "Bad response", "providerError": { "anthropicThinkingHistory": "This chat can't continue with a Claude model because its earlier reasoning history is no longer in a form Claude will accept. Start a new chat, or switch this chat to a non-Claude model to keep going." }, diff --git a/src/shared/i18n/locales/es/chat.json b/src/shared/i18n/locales/es/chat.json index da05f2ef1..3df509392 100644 --- a/src/shared/i18n/locales/es/chat.json +++ b/src/shared/i18n/locales/es/chat.json @@ -401,6 +401,8 @@ "mcpAppLoading": "Cargando MCP App…", "mcpAppRenderError": "No se pudo renderizar MCP App en línea.", "redactedThinking": "(pensamiento redactado)", + "responseFeedbackGood": "Buena respuesta", + "responseFeedbackBad": "Mala respuesta", "providerError": { "anthropicThinkingHistory": "Este chat no puede continuar con un modelo Claude porque su historial de razonamiento previo ya no tiene una forma que Claude acepte. Inicia un chat nuevo o cambia este chat a un modelo que no sea Claude para continuar." }, diff --git a/src/shared/runtime-config/schema.test.ts b/src/shared/runtime-config/schema.test.ts index 79bf2b957..60895c3c8 100644 --- a/src/shared/runtime-config/schema.test.ts +++ b/src/shared/runtime-config/schema.test.ts @@ -98,6 +98,15 @@ describe("runtimeConfigSchema", () => { ); }); + it("accepts distribution-owned response feedback policy", () => { + expect( + runtimeConfigSchema.parse({ + ...DEFAULT_RUNTIME_CONFIG, + feedback: { enabled: true, responseRatingEnabled: true }, + }).feedback, + ).toEqual({ enabled: true, responseRatingEnabled: true }); + }); + it("accepts an empty managed-provider list as unrestricted policy", () => { expect(runtimeConfigSchema.parse(DEFAULT_RUNTIME_CONFIG)).toEqual( DEFAULT_RUNTIME_CONFIG, diff --git a/src/shared/runtime-config/schema.ts b/src/shared/runtime-config/schema.ts index b047b6e78..4fe24d02e 100644 --- a/src/shared/runtime-config/schema.ts +++ b/src/shared/runtime-config/schema.ts @@ -282,6 +282,7 @@ export const runtimeFeedbackConfigSchema = z .object({ enabled: z.boolean().optional(), projectKey: nonEmptyString("feedback projectKey").optional(), + responseRatingEnabled: z.boolean().optional(), }) .strict(); From a2d2ebf99fe21b714971b11ba6b7c30ca35074e1 Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Wed, 26 Aug 2026 10:18:07 -0400 Subject: [PATCH 2/4] Remove response feedback appearance events Amp-Thread-ID: https://ampcode.com/threads/T-019fbabe-6c0e-70ab-8559-31ba601d4897 Co-authored-by: Amp --- .../ResponseFeedbackControls.test.tsx | 46 ++++--------------- .../ResponseFeedbackControls.tsx | 32 +------------ .../feedbackSurveyEvents.test.ts | 5 +- .../response-feedback/feedbackSurveySink.ts | 5 +- .../responseFeedbackState.test.ts | 20 +------- .../responseFeedbackState.ts | 38 ++------------- src/features/chat/ui/MessageBubble.tsx | 14 +----- src/features/chat/ui/MessageBubbleActions.tsx | 2 - 8 files changed, 24 insertions(+), 138 deletions(-) diff --git a/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx b/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx index f4498d101..e1a00da2a 100644 --- a/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx +++ b/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx @@ -1,4 +1,4 @@ -import { act, render } from "@testing-library/react"; +import { fireEvent, render, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { feedbackSurveySink } from "./feedbackSurveySink"; import { ResponseFeedbackControls } from "./ResponseFeedbackControls"; @@ -6,53 +6,27 @@ import { ResponseFeedbackControls } from "./ResponseFeedbackControls"; vi.mock("./feedbackSurveySink", () => ({ feedbackSurveySink: vi.fn() })); const sink = vi.mocked(feedbackSurveySink); -let intersectionCallback: IntersectionObserverCallback; - -class MockIntersectionObserver { - constructor(callback: IntersectionObserverCallback) { - intersectionCallback = callback; - } - observe() {} - disconnect() {} -} describe("ResponseFeedbackControls", () => { beforeEach(() => { localStorage.clear(); sink.mockClear(); - vi.stubGlobal("IntersectionObserver", MockIntersectionObserver); }); - it("records an appearance when persistent controls enter the viewport", () => { + it("records only user selections", () => { render( - , + , ); expect(sink).not.toHaveBeenCalled(); - act(() => { - intersectionCallback( - [{ isIntersecting: true } as IntersectionObserverEntry], - {} as IntersectionObserver, - ); - }); - expect(sink).toHaveBeenCalledWith( - expect.objectContaining({ eventType: "appeared" }), - ); - }); - it("does not record hidden controls merely because they are mounted", () => { - render( - , - ); + fireEvent.click(screen.getByRole("button", { name: /good/i })); - expect(sink).not.toHaveBeenCalled(); + expect(sink).toHaveBeenCalledWith( + expect.objectContaining({ + eventType: "responded", + response: "good", + }), + ); }); }); diff --git a/src/features/chat/response-feedback/ResponseFeedbackControls.tsx b/src/features/chat/response-feedback/ResponseFeedbackControls.tsx index 83740a347..32c19cf9e 100644 --- a/src/features/chat/response-feedback/ResponseFeedbackControls.tsx +++ b/src/features/chat/response-feedback/ResponseFeedbackControls.tsx @@ -1,11 +1,10 @@ -import { useEffect, useRef, useState } from "react"; +import { useEffect, useState } from "react"; import { ThumbsDown, ThumbsUp } from "lucide-react"; import { useTranslation } from "react-i18next"; import { cn } from "@/shared/lib/cn"; import { MessageAction } from "@/shared/ui/ai-elements/message"; import { getResponseFeedbackSelection, - markResponseFeedbackAppeared, setResponseFeedbackSelection, type ResponseFeedbackSelection, } from "./responseFeedbackState"; @@ -13,16 +12,13 @@ import { interface ResponseFeedbackControlsProps { sessionId: string; messageId: string; - persistentlyVisible: boolean; } export function ResponseFeedbackControls({ sessionId, messageId, - persistentlyVisible, }: ResponseFeedbackControlsProps) { const { t } = useTranslation("chat"); - const controlsRef = useRef(null); const [selection, setSelection] = useState( () => getResponseFeedbackSelection(sessionId, messageId), ); @@ -31,25 +27,6 @@ export function ResponseFeedbackControls({ setSelection(getResponseFeedbackSelection(sessionId, messageId)); }, [messageId, sessionId]); - useEffect(() => { - const target = controlsRef.current; - if ( - !persistentlyVisible || - !target || - typeof IntersectionObserver === "undefined" - ) { - return; - } - const observer = new IntersectionObserver((entries) => { - if (entries.some((entry) => entry.isIntersecting)) { - markResponseFeedbackAppeared(sessionId, messageId); - observer.disconnect(); - } - }); - observer.observe(target); - return () => observer.disconnect(); - }, [messageId, persistentlyVisible, sessionId]); - const select = (requested: ResponseFeedbackSelection) => { const current = getResponseFeedbackSelection(sessionId, messageId); const next = current === requested ? null : requested; @@ -61,12 +38,7 @@ export function ResponseFeedbackControls({ "bg-accent text-foreground hover:bg-accent active:bg-accent"; return ( - markResponseFeedbackAppeared(sessionId, messageId)} - onFocusCapture={() => markResponseFeedbackAppeared(sessionId, messageId)} - > + { messageId: "message", appearanceId: "appearance", surveyType: "response", - eventType: "appeared", + eventType: "responded", + response: "good", }); sendFeedbackSurveyEvent({ sessionId: "session", @@ -26,7 +27,7 @@ describe("feedbackSurveyEvents", () => { appearanceId: "appearance", surveyType: "response", eventType: "responded", - response: "good", + response: "bad", }); expect(sink).toHaveBeenNthCalledWith( diff --git a/src/features/chat/response-feedback/feedbackSurveySink.ts b/src/features/chat/response-feedback/feedbackSurveySink.ts index 87079a953..f165288ee 100644 --- a/src/features/chat/response-feedback/feedbackSurveySink.ts +++ b/src/features/chat/response-feedback/feedbackSurveySink.ts @@ -1,4 +1,3 @@ -export type FeedbackSurveyEventType = "appeared" | "responded"; export type FeedbackSurveyResponse = "good" | "bad" | "cleared"; export interface FeedbackSurveySinkEvent { @@ -7,8 +6,8 @@ export interface FeedbackSurveySinkEvent { appearanceId: string; surveyType: "response"; eventSequence: number; - eventType: FeedbackSurveyEventType; - response?: FeedbackSurveyResponse; + eventType: "responded"; + response: FeedbackSurveyResponse; } /** Distribution-owned transport seam; stock Berd intentionally sends nothing. */ diff --git a/src/features/chat/response-feedback/responseFeedbackState.test.ts b/src/features/chat/response-feedback/responseFeedbackState.test.ts index 58c1397e1..6a6dac38d 100644 --- a/src/features/chat/response-feedback/responseFeedbackState.test.ts +++ b/src/features/chat/response-feedback/responseFeedbackState.test.ts @@ -4,7 +4,6 @@ import { feedbackSurveySink } from "./feedbackSurveySink"; import { getResponseFeedbackSelection, isResponseFeedbackEligible, - markResponseFeedbackAppeared, setResponseFeedbackSelection, } from "./responseFeedbackState"; @@ -28,22 +27,6 @@ describe("responseFeedbackState", () => { sink.mockClear(); }); - it("emits one appeared event and persists it", () => { - expect(markResponseFeedbackAppeared("appeared-session", "message")).toBe( - true, - ); - expect(markResponseFeedbackAppeared("appeared-session", "message")).toBe( - false, - ); - expect(sink).toHaveBeenCalledTimes(1); - expect(sink).toHaveBeenCalledWith( - expect.objectContaining({ - sessionId: "appeared-session", - eventType: "appeared", - }), - ); - }); - it("emits selections, switches, and clears without duplicate transitions", () => { expect( setResponseFeedbackSelection("selection-session", "message", "good"), @@ -61,9 +44,8 @@ describe("responseFeedbackState", () => { getResponseFeedbackSelection("selection-session", "message"), ).toBeNull(); - expect(sink).toHaveBeenCalledTimes(4); + expect(sink).toHaveBeenCalledTimes(3); expect(sink.mock.calls.map(([event]) => event)).toEqual([ - expect.objectContaining({ eventType: "appeared" }), expect.objectContaining({ eventType: "responded", response: "good" }), expect.objectContaining({ eventType: "responded", response: "bad" }), expect.objectContaining({ eventType: "responded", response: "cleared" }), diff --git a/src/features/chat/response-feedback/responseFeedbackState.ts b/src/features/chat/response-feedback/responseFeedbackState.ts index a3489e9a7..3ee204c03 100644 --- a/src/features/chat/response-feedback/responseFeedbackState.ts +++ b/src/features/chat/response-feedback/responseFeedbackState.ts @@ -6,7 +6,6 @@ export type ResponseFeedbackSelection = "good" | "bad"; interface StoredResponseFeedback { version: 1; appearanceId: string; - appeared: boolean; response: ResponseFeedbackSelection | null; } @@ -28,7 +27,6 @@ function createStoredResponseFeedback(): StoredResponseFeedback { return { version: 1, appearanceId: crypto.randomUUID(), - appeared: false, response: null, }; } @@ -44,7 +42,6 @@ function parseStoredResponseFeedback( record.version !== 1 || typeof record.appearanceId !== "string" || record.appearanceId.length === 0 || - typeof record.appeared !== "boolean" || (record.response !== null && record.response !== "good" && record.response !== "bad") @@ -54,7 +51,6 @@ function parseStoredResponseFeedback( return { version: 1, appearanceId: record.appearanceId, - appeared: record.appeared, response: record.response, }; } @@ -103,19 +99,15 @@ function emitResponseFeedback( sessionId: string, messageId: string, record: StoredResponseFeedback, - event: - | { eventType: "appeared" } - | { - eventType: "responded"; - response: ResponseFeedbackSelection | "cleared"; - }, + response: ResponseFeedbackSelection | "cleared", ): void { sendFeedbackSurveyEvent({ sessionId, messageId, appearanceId: record.appearanceId, surveyType: "response", - ...event, + eventType: "responded", + response, }); } @@ -126,20 +118,6 @@ export function getResponseFeedbackSelection( return readResponseFeedback(sessionId, messageId).response; } -export function markResponseFeedbackAppeared( - sessionId: string, - messageId: string, -): boolean { - const current = readResponseFeedback(sessionId, messageId); - if (current.appeared) { - return false; - } - const next = { ...current, appeared: true }; - writeResponseFeedback(sessionId, messageId, next); - emitResponseFeedback(sessionId, messageId, next, { eventType: "appeared" }); - return true; -} - export function setResponseFeedbackSelection( sessionId: string, messageId: string, @@ -150,15 +128,9 @@ export function setResponseFeedbackSelection( return current.response; } - const next = { ...current, appeared: true, response: selection }; + const next = { ...current, response: selection }; writeResponseFeedback(sessionId, messageId, next); - if (!current.appeared) { - emitResponseFeedback(sessionId, messageId, next, { eventType: "appeared" }); - } - emitResponseFeedback(sessionId, messageId, next, { - eventType: "responded", - response: selection ?? "cleared", - }); + emitResponseFeedback(sessionId, messageId, next, selection ?? "cleared"); return next.response; } diff --git a/src/features/chat/ui/MessageBubble.tsx b/src/features/chat/ui/MessageBubble.tsx index 187f347a9..650951aef 100644 --- a/src/features/chat/ui/MessageBubble.tsx +++ b/src/features/chat/ui/MessageBubble.tsx @@ -60,10 +60,7 @@ import { useProfileCapability } from "@/shared/profile/capabilities"; import { useRuntimeConfigStore } from "@/shared/runtime-config/runtimeConfigStore"; import { MessageBubbleActions } from "./MessageBubbleActions"; import { MessageMetadataChip } from "./MessageMetadataChip"; -import { - isResponseFeedbackEligible, - markResponseFeedbackAppeared, -} from "../response-feedback/responseFeedbackState"; +import { isResponseFeedbackEligible } from "../response-feedback/responseFeedbackState"; import { couldOverflowUserMessagePreview, UserMessageClamp, @@ -908,7 +905,6 @@ export const MessageBubble = memo(function MessageBubble({ ? { sessionId: feedbackSessionId, messageId: actionMessageId, - persistentlyVisible: messageActionsArePersistentlyVisible, } : undefined; const outerSpacingClassName = @@ -992,14 +988,6 @@ export const MessageBubble = memo(function MessageBubble({ ? "max-w-[var(--chat-user-message-max-width)] items-end" : "w-full items-start", )} - onPointerEnter={() => { - if (responseFeedback) { - markResponseFeedbackAppeared( - responseFeedback.sessionId, - responseFeedback.messageId, - ); - } - }} > {showAssistantIdentity ? (
diff --git a/src/features/chat/ui/MessageBubbleActions.tsx b/src/features/chat/ui/MessageBubbleActions.tsx index 14e6c33d9..c39d5c9ca 100644 --- a/src/features/chat/ui/MessageBubbleActions.tsx +++ b/src/features/chat/ui/MessageBubbleActions.tsx @@ -23,7 +23,6 @@ interface MessageBubbleActionsProps { responseFeedback?: { sessionId: string; messageId: string; - persistentlyVisible: boolean; }; onJumpToResponseStartHintClose?: (messageId: string) => void; onJumpToResponseStartHintDismiss?: (messageId: string) => void; @@ -174,7 +173,6 @@ export function MessageBubbleActions({ ) : null} {!isUser && timestamp} From 0629b688a545dab153d845e89da1a306cc21cda8 Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Wed, 26 Aug 2026 10:49:59 -0400 Subject: [PATCH 3/4] Synchronize response feedback across renderers Amp-Thread-ID: https://ampcode.com/threads/T-019fbabe-6c0e-70ab-8559-31ba601d4897 Co-authored-by: Amp --- .../ResponseFeedbackControls.test.tsx | 24 +++++++++++++ .../ResponseFeedbackControls.tsx | 21 +++++++----- .../responseFeedbackState.ts | 34 +++++++++++++++++++ 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx b/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx index e1a00da2a..97bba576a 100644 --- a/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx +++ b/src/features/chat/response-feedback/ResponseFeedbackControls.test.tsx @@ -29,4 +29,28 @@ describe("ResponseFeedbackControls", () => { }), ); }); + + it("synchronizes the selected response across renderers", () => { + render( + <> + + + , + ); + + const goodButtons = screen.getAllByRole("button", { name: /good/i }); + fireEvent.click(goodButtons[0]); + expect(goodButtons.every((button) => button.ariaPressed === "true")).toBe( + true, + ); + + fireEvent.click(goodButtons[1]); + expect(goodButtons.every((button) => button.ariaPressed === "false")).toBe( + true, + ); + expect(sink.mock.calls.map(([event]) => event.response)).toEqual([ + "good", + "cleared", + ]); + }); }); diff --git a/src/features/chat/response-feedback/ResponseFeedbackControls.tsx b/src/features/chat/response-feedback/ResponseFeedbackControls.tsx index 32c19cf9e..1420e8139 100644 --- a/src/features/chat/response-feedback/ResponseFeedbackControls.tsx +++ b/src/features/chat/response-feedback/ResponseFeedbackControls.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useCallback, useSyncExternalStore } from "react"; import { ThumbsDown, ThumbsUp } from "lucide-react"; import { useTranslation } from "react-i18next"; import { cn } from "@/shared/lib/cn"; @@ -6,7 +6,7 @@ import { MessageAction } from "@/shared/ui/ai-elements/message"; import { getResponseFeedbackSelection, setResponseFeedbackSelection, - type ResponseFeedbackSelection, + subscribeResponseFeedbackSelection, } from "./responseFeedbackState"; interface ResponseFeedbackControlsProps { @@ -19,18 +19,21 @@ export function ResponseFeedbackControls({ messageId, }: ResponseFeedbackControlsProps) { const { t } = useTranslation("chat"); - const [selection, setSelection] = useState( + const subscribe = useCallback( + (onStoreChange: () => void) => + subscribeResponseFeedbackSelection(sessionId, messageId, onStoreChange), + [messageId, sessionId], + ); + const getSnapshot = useCallback( () => getResponseFeedbackSelection(sessionId, messageId), + [messageId, sessionId], ); + const selection = useSyncExternalStore(subscribe, getSnapshot, getSnapshot); - useEffect(() => { - setSelection(getResponseFeedbackSelection(sessionId, messageId)); - }, [messageId, sessionId]); - - const select = (requested: ResponseFeedbackSelection) => { + const select = (requested: "good" | "bad") => { const current = getResponseFeedbackSelection(sessionId, messageId); const next = current === requested ? null : requested; - setSelection(setResponseFeedbackSelection(sessionId, messageId, next)); + setResponseFeedbackSelection(sessionId, messageId, next); }; const goodSelected = selection === "good"; const badSelected = selection === "bad"; diff --git a/src/features/chat/response-feedback/responseFeedbackState.ts b/src/features/chat/response-feedback/responseFeedbackState.ts index 3ee204c03..aff93b200 100644 --- a/src/features/chat/response-feedback/responseFeedbackState.ts +++ b/src/features/chat/response-feedback/responseFeedbackState.ts @@ -10,6 +10,7 @@ interface StoredResponseFeedback { } const RESPONSE_FEEDBACK_STORAGE_PREFIX = "berd:response-feedback:v1:"; +const RESPONSE_FEEDBACK_CHANGE_EVENT = "berd:response-feedback-change"; const volatileRecords = new Map(); const volatileOnlyKeys = new Set(); @@ -93,6 +94,9 @@ function writeResponseFeedback( } catch { volatileOnlyKeys.add(key); } + window.dispatchEvent( + new CustomEvent(RESPONSE_FEEDBACK_CHANGE_EVENT, { detail: { key } }), + ); } function emitResponseFeedback( @@ -118,6 +122,36 @@ export function getResponseFeedbackSelection( return readResponseFeedback(sessionId, messageId).response; } +export function subscribeResponseFeedbackSelection( + sessionId: string, + messageId: string, + onStoreChange: () => void, +): () => void { + if (typeof window === "undefined") return () => {}; + + const key = responseFeedbackStorageKey(sessionId, messageId); + const handleLocalChange = (event: Event) => { + if ((event as CustomEvent<{ key?: string }>).detail?.key === key) { + onStoreChange(); + } + }; + const handleStorageChange = (event: StorageEvent) => { + if (event.key === key || event.key === null) { + onStoreChange(); + } + }; + + window.addEventListener(RESPONSE_FEEDBACK_CHANGE_EVENT, handleLocalChange); + window.addEventListener("storage", handleStorageChange); + return () => { + window.removeEventListener( + RESPONSE_FEEDBACK_CHANGE_EVENT, + handleLocalChange, + ); + window.removeEventListener("storage", handleStorageChange); + }; +} + export function setResponseFeedbackSelection( sessionId: string, messageId: string, From ccf75fd2b9b3da43266ac54f08c57491d12d8419 Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Wed, 26 Aug 2026 22:52:53 -0400 Subject: [PATCH 4/4] Separate human feedback capability Amp-Thread-ID: https://ampcode.com/threads/T-019fbabe-6c0e-70ab-8559-31ba601d4897 Co-authored-by: Amp --- .github/workflows/release.yml | 1 + scripts/build_linux_docker.sh | 1 + .../release/tests/release-scripts.test.mjs | 1 + src/env.d.ts | 1 + .../feedbackSurveyEvents.test.ts | 33 ++++++------------- .../response-feedback/feedbackSurveyEvents.ts | 32 ++---------------- .../response-feedback/feedbackSurveySink.ts | 3 +- src/features/chat/ui/MessageBubble.tsx | 4 +-- .../ui/__tests__/settingsSections.test.ts | 1 + src/shared/profile/buildProfile.test.ts | 17 ++++++++-- src/shared/profile/buildProfile.ts | 11 +++++-- src/shared/profile/capabilities.test.ts | 26 +++++++++++++++ src/shared/profile/capabilities.ts | 5 +++ 13 files changed, 75 insertions(+), 61 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 90a82bbea..0a6b9c466 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -32,6 +32,7 @@ env: VITE_AUTOMATIONS: "0" VITE_BUILDERBOT: "0" VITE_FEEDBACK: "0" + VITE_FEEDBACK_SURVEYS: "0" VITE_MANAGED_CONNECTIONS: "0" VITE_TELEMETRY_ENFORCED: "0" VITE_VOICE_DICTATION: "0" diff --git a/scripts/build_linux_docker.sh b/scripts/build_linux_docker.sh index f572d0bd5..0e17cf945 100755 --- a/scripts/build_linux_docker.sh +++ b/scripts/build_linux_docker.sh @@ -72,6 +72,7 @@ vite_env_names=( VITE_AUTOMATIONS VITE_BUILDERBOT VITE_FEEDBACK + VITE_FEEDBACK_SURVEYS VITE_MANAGED_CONNECTIONS VITE_TELEMETRY_ENFORCED VITE_VOICE_DICTATION diff --git a/scripts/release/tests/release-scripts.test.mjs b/scripts/release/tests/release-scripts.test.mjs index 2f1253c76..56bc5ba37 100644 --- a/scripts/release/tests/release-scripts.test.mjs +++ b/scripts/release/tests/release-scripts.test.mjs @@ -730,6 +730,7 @@ describe("desktop release workflow platform gate", () => { VITE_AUTOMATIONS: "0", VITE_BUILDERBOT: "0", VITE_FEEDBACK: "0", + VITE_FEEDBACK_SURVEYS: "0", VITE_MANAGED_CONNECTIONS: "0", VITE_VOICE_DICTATION: "0", VITE_BYO_KEY_PROVIDERS: "1", diff --git a/src/env.d.ts b/src/env.d.ts index cd0c7be90..60e73d7ca 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -11,6 +11,7 @@ declare global { readonly VITE_AUTOMATIONS?: string; readonly VITE_BUILDERBOT?: string; readonly VITE_FEEDBACK?: string; + readonly VITE_FEEDBACK_SURVEYS?: string; readonly VITE_BYO_KEY_PROVIDERS?: string; readonly VITE_TELEMETRY?: string; readonly VITE_VOICE_DICTATION?: string; diff --git a/src/features/chat/response-feedback/feedbackSurveyEvents.test.ts b/src/features/chat/response-feedback/feedbackSurveyEvents.test.ts index cc2a444f6..3a7e7d8cc 100644 --- a/src/features/chat/response-feedback/feedbackSurveyEvents.test.ts +++ b/src/features/chat/response-feedback/feedbackSurveyEvents.test.ts @@ -8,35 +8,22 @@ const sink = vi.mocked(feedbackSurveySink); describe("feedbackSurveyEvents", () => { beforeEach(() => { - localStorage.clear(); sink.mockClear(); }); - it("assigns a persistent session-wide event sequence", () => { - sendFeedbackSurveyEvent({ + it("forwards survey identity and state to the distribution-owned sink", () => { + const event = { sessionId: "session", messageId: "message", appearanceId: "appearance", - surveyType: "response", - eventType: "responded", - response: "good", - }); - sendFeedbackSurveyEvent({ - sessionId: "session", - messageId: "message", - appearanceId: "appearance", - surveyType: "response", - eventType: "responded", - response: "bad", - }); + surveyType: "response" as const, + eventType: "responded" as const, + response: "good" as const, + }; + + sendFeedbackSurveyEvent(event); - expect(sink).toHaveBeenNthCalledWith( - 1, - expect.objectContaining({ eventSequence: 1 }), - ); - expect(sink).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ eventSequence: 2 }), - ); + expect(sink).toHaveBeenCalledOnce(); + expect(sink).toHaveBeenCalledWith(event); }); }); diff --git a/src/features/chat/response-feedback/feedbackSurveyEvents.ts b/src/features/chat/response-feedback/feedbackSurveyEvents.ts index 7d2f85693..263fd2e56 100644 --- a/src/features/chat/response-feedback/feedbackSurveyEvents.ts +++ b/src/features/chat/response-feedback/feedbackSurveyEvents.ts @@ -3,36 +3,8 @@ import { feedbackSurveySink, } from "./feedbackSurveySink"; -export type FeedbackSurveyEventInput = Omit< - FeedbackSurveySinkEvent, - "eventSequence" ->; - -const FEEDBACK_SEQUENCE_STORAGE_PREFIX = "berd:feedback-event-sequence:v1:"; -const volatileSequences = new Map(); - -function nextFeedbackEventSequence(sessionId: string): number { - const key = `${FEEDBACK_SEQUENCE_STORAGE_PREFIX}${sessionId}`; - let current = volatileSequences.get(key) ?? 0; - try { - const stored = Number(localStorage.getItem(key)); - if (Number.isSafeInteger(stored) && stored > current) current = stored; - } catch { - // The in-memory counter still preserves ordering for this app process. - } - const next = current + 1; - volatileSequences.set(key, next); - try { - localStorage.setItem(key, String(next)); - } catch { - // Persistence is best-effort; feedback delivery must not affect chat. - } - return next; -} +export type FeedbackSurveyEventInput = FeedbackSurveySinkEvent; export function sendFeedbackSurveyEvent(input: FeedbackSurveyEventInput): void { - feedbackSurveySink({ - ...input, - eventSequence: nextFeedbackEventSequence(input.sessionId), - }); + feedbackSurveySink(input); } diff --git a/src/features/chat/response-feedback/feedbackSurveySink.ts b/src/features/chat/response-feedback/feedbackSurveySink.ts index f165288ee..1499fead1 100644 --- a/src/features/chat/response-feedback/feedbackSurveySink.ts +++ b/src/features/chat/response-feedback/feedbackSurveySink.ts @@ -5,10 +5,9 @@ export interface FeedbackSurveySinkEvent { messageId: string; appearanceId: string; surveyType: "response"; - eventSequence: number; eventType: "responded"; response: FeedbackSurveyResponse; } -/** Distribution-owned transport seam; stock Berd intentionally sends nothing. */ +/** Distribution-owned transport and ordering seam; stock Berd sends nothing. */ export function feedbackSurveySink(_event: FeedbackSurveySinkEvent): void {} diff --git a/src/features/chat/ui/MessageBubble.tsx b/src/features/chat/ui/MessageBubble.tsx index 650951aef..7d263a6b9 100644 --- a/src/features/chat/ui/MessageBubble.tsx +++ b/src/features/chat/ui/MessageBubble.tsx @@ -733,7 +733,7 @@ export const MessageBubble = memo(function MessageBubble({ const { isCopied: isCopyConfirmed, copyToClipboard } = useCopyToClipboard(); const hasPersonaAvatar = Boolean(persona?.avatar); const catalogEntries = useProviderCatalogStore((state) => state.entries); - const feedbackEnabled = useProfileCapability("feedback"); + const feedbackSurveysEnabled = useProfileCapability("feedbackSurveys"); const responseRatingEnabled = useRuntimeConfigStore( (state) => state.config.feedback?.responseRatingEnabled === true, ); @@ -894,7 +894,7 @@ export const MessageBubble = memo(function MessageBubble({ actionsAlwaysVisible || isCopyConfirmed; const responseFeedback = canHostMessageActions && - feedbackEnabled && + feedbackSurveysEnabled && responseRatingEnabled && feedbackSessionId && isResponseFeedbackEligible({ diff --git a/src/features/settings/ui/__tests__/settingsSections.test.ts b/src/features/settings/ui/__tests__/settingsSections.test.ts index 9709a8b27..4d0f0f0ea 100644 --- a/src/features/settings/ui/__tests__/settingsSections.test.ts +++ b/src/features/settings/ui/__tests__/settingsSections.test.ts @@ -15,6 +15,7 @@ const enabledCapabilities: ProfileCapabilityState = { builderbot: true, doctor: true, feedback: true, + feedbackSurveys: true, telemetry: true, voiceDictation: true, voiceConversation: true, diff --git a/src/shared/profile/buildProfile.test.ts b/src/shared/profile/buildProfile.test.ts index 7f2e4b925..e0ad63d25 100644 --- a/src/shared/profile/buildProfile.test.ts +++ b/src/shared/profile/buildProfile.test.ts @@ -14,7 +14,7 @@ describe("buildProfile", () => { vi.resetModules(); }); - it("defaults all six Block-service-dependent product families off", () => { + it("defaults distribution-specific product families off", () => { expect(getBuildFeatureState()).toEqual({ authGate: false, agentTools: false, @@ -22,6 +22,7 @@ describe("buildProfile", () => { builderbot: false, byoKeyProviders: true, feedback: false, + feedbackSurveys: false, managedConnections: false, telemetry: true, telemetryEnforced: false, @@ -36,7 +37,7 @@ describe("buildProfile", () => { ["VITE_AGENT_TOOLS", "agentTools"], ["VITE_AUTOMATIONS", "automations"], ["VITE_BUILDERBOT", "builderbot"], - ["VITE_FEEDBACK", "feedback"], + ["VITE_FEEDBACK_SURVEYS", "feedbackSurveys"], ["VITE_MANAGED_CONNECTIONS", "managedConnections"], ["VITE_VOICE_DICTATION", "voiceDictation"], ] as const)("enables %s independently", async (env, feature) => { @@ -50,6 +51,7 @@ describe("buildProfile", () => { "automations", "builderbot", "feedback", + "feedbackSurveys", "managedConnections", "voiceDictation", ] as const) { @@ -57,6 +59,17 @@ describe("buildProfile", () => { } }); + it("keeps VITE_FEEDBACK as the compatibility opt-in for issue feedback and surveys", async () => { + vi.resetModules(); + vi.stubEnv("VITE_FEEDBACK", "1"); + const { getBuildFeatureState: fresh } = await import("./buildProfile"); + + expect(fresh()).toMatchObject({ + feedback: true, + feedbackSurveys: true, + }); + }); + it("disables bring-your-own-key providers when VITE_BYO_KEY_PROVIDERS is 0 (inverse-positive default-on)", async () => { vi.resetModules(); vi.stubEnv("VITE_BYO_KEY_PROVIDERS", "0"); diff --git a/src/shared/profile/buildProfile.ts b/src/shared/profile/buildProfile.ts index a6319359a..37dc0d4bf 100644 --- a/src/shared/profile/buildProfile.ts +++ b/src/shared/profile/buildProfile.ts @@ -5,6 +5,7 @@ export type BuildFeature = | "builderbot" | "byoKeyProviders" | "feedback" + | "feedbackSurveys" | "managedConnections" | "telemetry" | "telemetryEnforced" @@ -16,8 +17,9 @@ export type BuildFeature = /** * Product families backed by Block-only services are positive opt-ins. A * normal public build has no value for these variables and therefore cannot - * expose a path that depends on KGoose, G2, or Builderbot. Distributions may - * restore each family independently by setting exactly its variable to `1`. + * expose a path that depends on KGoose, G2, or Builderbot. Human feedback + * surveys use the same opt-in posture because their transport is supplied by + * the distribution. */ function readBuildFeatures(): Record { return { @@ -29,6 +31,11 @@ function readBuildFeatures(): Record { builderbot: import.meta.env.VITE_BUILDERBOT === "1", byoKeyProviders: import.meta.env.VITE_BYO_KEY_PROVIDERS !== "0", feedback: import.meta.env.VITE_FEEDBACK === "1", + // Keep the existing broad opt-in working while allowing survey transports + // that do not include KGoose issue feedback. + feedbackSurveys: + import.meta.env.VITE_FEEDBACK_SURVEYS === "1" || + import.meta.env.VITE_FEEDBACK === "1", managedConnections: import.meta.env.VITE_MANAGED_CONNECTIONS === "1", telemetry: import.meta.env.VITE_TELEMETRY !== "0", // Managed internal distributions force telemetry consent ON: the user diff --git a/src/shared/profile/capabilities.test.ts b/src/shared/profile/capabilities.test.ts index 67b5ca50d..dab14dd8e 100644 --- a/src/shared/profile/capabilities.test.ts +++ b/src/shared/profile/capabilities.test.ts @@ -26,6 +26,7 @@ const enabledBuildFeatures: Record = { builderbot: true, byoKeyProviders: false, feedback: true, + feedbackSurveys: true, managedConnections: true, telemetry: true, telemetryEnforced: false, @@ -230,6 +231,7 @@ describe("profile capabilities", () => { voiceConversation: true, managedConnections: false, feedback: false, + feedbackSurveys: true, agentTools: true, telemetry: true, doctor: true, @@ -246,6 +248,30 @@ describe("profile capabilities", () => { ).toBe(true); }); + it("keeps feedback surveys independent of Kgoose issue feedback", () => { + expect( + resolve({ + kgooseConfigured: false, + runtimeConfig: { + ...DEFAULT_RUNTIME_CONFIG, + feedback: { enabled: false, responseRatingEnabled: true }, + }, + }), + ).toMatchObject({ + feedback: false, + feedbackSurveys: true, + }); + + expect( + resolve({ + buildFeatures: { + ...enabledBuildFeatures, + feedbackSurveys: false, + }, + }).feedbackSurveys, + ).toBe(false); + }); + it("enables Kgoose-backed capabilities for an explicit distro or environment endpoint", () => { expect( resolve({ diff --git a/src/shared/profile/capabilities.ts b/src/shared/profile/capabilities.ts index 55f5ec4b7..d66a0069b 100644 --- a/src/shared/profile/capabilities.ts +++ b/src/shared/profile/capabilities.ts @@ -23,6 +23,7 @@ export type ProfileCapabilityId = | "managedConnections" | "updates" | "feedback" + | "feedbackSurveys" | "doctor"; type CapabilitySource = @@ -102,6 +103,10 @@ export const PROFILE_CAPABILITY_REGISTRY: ProfileCapabilityRegistry = { requiresKgoose: true, runtimeConfigSection: "feedback", }, + feedbackSurveys: { + kind: "buildFeature", + feature: "feedbackSurveys", + }, doctor: { kind: "runtimeConfigSection", field: "doctor" }, };