From fd02c7914a216079eaf52dd74d07952b3b2d423a Mon Sep 17 00:00:00 2001 From: bnodir Date: Sun, 12 Jan 2025 00:20:19 +0900 Subject: [PATCH 1/4] Add user feedback with Langfuse integration - Add thumbs up/down feedback for chat responses - Persist feedback in IndexedDB and server - Track responses with Langfuse analytics --- app/backend/app.py | 165 +++++++++--- .../approaches/chatreadretrieveread.py | 12 + app/frontend/src/api/models.ts | 28 ++- .../src/components/Answer/Answer.module.css | 47 ++++ app/frontend/src/components/Answer/Answer.tsx | 238 +++++++++++++++--- .../src/components/Answer/FeedbackContext.tsx | 135 ++++++++++ .../src/components/Answer/FeedbackStore.ts | 140 +++++++++++ app/frontend/src/components/Answer/index.ts | 1 + .../components/HistoryPanel/HistoryPanel.tsx | 38 +-- .../components/HistoryProviders/CosmosDB.ts | 15 ++ .../HistoryProviders/FeedbackStore.ts | 145 +++++++++++ .../HistoryProviders/HistoryManager.ts | 12 +- .../components/HistoryProviders/IProvider.ts | 20 +- .../components/HistoryProviders/IndexedDB.ts | 33 ++- .../src/components/HistoryProviders/None.ts | 3 + app/frontend/src/pages/chat/Chat.tsx | 4 +- 16 files changed, 943 insertions(+), 93 deletions(-) create mode 100644 app/frontend/src/components/Answer/FeedbackContext.tsx create mode 100644 app/frontend/src/components/Answer/FeedbackStore.ts create mode 100644 app/frontend/src/components/HistoryProviders/FeedbackStore.ts diff --git a/app/backend/app.py b/app/backend/app.py index b9d904b476..fe12df503f 100644 --- a/app/backend/app.py +++ b/app/backend/app.py @@ -26,14 +26,18 @@ from azure.search.documents.indexes.aio import SearchIndexClient from azure.storage.blob.aio import ContainerClient from azure.storage.blob.aio import StorageStreamDownloader as BlobDownloader -from azure.storage.filedatalake.aio import FileSystemClient -from azure.storage.filedatalake.aio import StorageStreamDownloader as DatalakeDownloader +from azure.storage.filedatalake.aio import ( + FileSystemClient, +) +from azure.storage.filedatalake.aio import ( + StorageStreamDownloader as DatalakeDownloader, +) +from langfuse import Langfuse +from langfuse.decorators import observe from openai import AsyncAzureOpenAI, AsyncOpenAI from opentelemetry.instrumentation.aiohttp_client import AioHttpClientInstrumentor from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware -from opentelemetry.instrumentation.httpx import ( - HTTPXClientInstrumentor, -) +from opentelemetry.instrumentation.httpx import HTTPXClientInstrumentor from opentelemetry.instrumentation.openai import OpenAIInstrumentor from quart import ( Blueprint, @@ -94,6 +98,13 @@ from prepdocslib.filestrategy import UploadUserFileStrategy from prepdocslib.listfilestrategy import File +# Initialize Langfuse client +langfuse = Langfuse( + secret_key=os.getenv("LANGFUSE_SECRET_KEY"), + public_key=os.getenv("LANGFUSE_PUBLIC_KEY"), + host=os.getenv("LANGFUSE_HOST", "https://cloud.langfuse.com"), +) + bp = Blueprint("routes", __name__, static_folder="static") # Fix Windows registry issue with mimetypes mimetypes.add_type("application/javascript", ".js") @@ -169,13 +180,26 @@ async def content_file(path: str, auth_claims: Dict[str, Any]): @bp.route("/ask", methods=["POST"]) @authenticated +@observe() async def ask(auth_claims: Dict[str, Any]): if not request.is_json: return jsonify({"error": "request must be json"}), 415 + request_json = await request.get_json() context = request_json.get("context", {}) context["auth_claims"] = auth_claims + try: + # Create a trace for this ask request + trace = langfuse.trace( + metadata={"endpoint": "/ask", "method": "POST", "entra_oid": auth_claims.get("oid", "anonymous")} + ) + + # Create a generation + generation = trace.generation( + name="ask_response", metadata={"messages": request_json.get("messages", []), "context": context} + ) + use_gpt4v = context.get("overrides", {}).get("use_gpt4v", False) approach: Approach if use_gpt4v and CONFIG_ASK_VISION_APPROACH in current_app.config: @@ -185,8 +209,14 @@ async def ask(auth_claims: Dict[str, Any]): r = await approach.run( request_json["messages"], context=context, session_state=request_json.get("session_state") ) + + # End the generation after getting response + generation.end(metadata={"model": approach.__class__.__name__, "use_gpt4v": use_gpt4v}) + return jsonify(r) except Exception as error: + if "generation" in locals(): + generation.end(error=str(error)) return error_response(error, "/ask") @@ -208,6 +238,7 @@ async def format_as_ndjson(r: AsyncGenerator[dict, None]) -> AsyncGenerator[str, @bp.route("/chat", methods=["POST"]) @authenticated +@observe() async def chat(auth_claims: Dict[str, Any]): if not request.is_json: return jsonify({"error": "request must be json"}), 415 @@ -242,39 +273,80 @@ async def chat(auth_claims: Dict[str, Any]): @bp.route("/chat/stream", methods=["POST"]) @authenticated +@observe() async def chat_stream(auth_claims: Dict[str, Any]): if not request.is_json: return jsonify({"error": "request must be json"}), 415 + request_json = await request.get_json() context = request_json.get("context", {}) context["auth_claims"] = auth_claims + try: - use_gpt4v = context.get("overrides", {}).get("use_gpt4v", False) - approach: Approach - if use_gpt4v and CONFIG_CHAT_VISION_APPROACH in current_app.config: - approach = cast(Approach, current_app.config[CONFIG_CHAT_VISION_APPROACH]) - else: - approach = cast(Approach, current_app.config[CONFIG_CHAT_APPROACH]) + # Create trace_id at the start + trace_id = create_session_id(True, True) - # If session state is provided, persists the session state, - # else creates a new session_id depending on the chat history options enabled. - session_state = request_json.get("session_state") - if session_state is None: - session_state = create_session_id( - current_app.config[CONFIG_CHAT_HISTORY_COSMOS_ENABLED], - current_app.config[CONFIG_CHAT_HISTORY_BROWSER_ENABLED], - ) - result = await approach.run_stream( - request_json["messages"], - context=context, - session_state=session_state, + # Make trace_id available at all levels + context["trace_id"] = trace_id + if "overrides" not in context: + context["overrides"] = {} + context["overrides"]["trace_id"] = trace_id + + current_app.logger.info(f"Generated trace_id: {trace_id}") + + # Create a new trace with unique ID and generation at the start + trace = langfuse.trace( + id=trace_id, + metadata={"endpoint": "/chat/stream", "method": "POST", "entra_oid": auth_claims.get("oid", "anonymous")}, + ) + + # Create generation early + generation = trace.generation( + name="chat_stream_response", metadata={"messages": request_json.get("messages", []), "context": context} ) - response = await make_response(format_as_ndjson(result)) - response.timeout = None # type: ignore - response.mimetype = "application/json-lines" - return response + + current_app.logger.info(f"Generated trace_id: {trace_id}") + + try: + use_gpt4v = context.get("overrides", {}).get("use_gpt4v", False) + approach: Approach + if use_gpt4v and CONFIG_CHAT_VISION_APPROACH in current_app.config: + approach = cast(Approach, current_app.config[CONFIG_CHAT_VISION_APPROACH]) + else: + approach = cast(Approach, current_app.config[CONFIG_CHAT_APPROACH]) + + session_state = request_json.get("session_state") + if session_state is None: + session_state = create_session_id( + current_app.config[CONFIG_CHAT_HISTORY_COSMOS_ENABLED], + current_app.config[CONFIG_CHAT_HISTORY_BROWSER_ENABLED], + ) + + # Get result first + result = await approach.run_stream( + request_json["messages"], + context=context, + session_state=session_state, + ) + + # Create response after getting result + response = await make_response(format_as_ndjson(result)) + response.timeout = None # type: ignore + response.mimetype = "application/json-lines" + + # End generation before returning response + generation.end(metadata={"model": approach.__class__.__name__, "use_gpt4v": use_gpt4v, "status": "success"}) + + return response + + except Exception as inner_error: + current_app.logger.error(f"Error in chat processing: {inner_error}") + generation.end(error=str(inner_error), metadata={"status": "error"}) + raise inner_error + except Exception as error: - return error_response(error, "/chat") + current_app.logger.error(f"Error in chat_stream: {error}") + return error_response(error, "/chat/stream") # Send MSAL.js settings to the client UI @@ -406,6 +478,43 @@ async def list_uploaded(auth_claims: dict[str, Any]): return jsonify(files), 200 +@bp.route("/feedback", methods=["POST"]) +@authenticated +async def feedback(auth_claims: Dict[str, Any]): + if not request.is_json: + return jsonify({"error": "request must be json"}), 415 + + try: + request_json = await request.get_json() + trace_id = request_json.get("trace_id") + value = request_json.get("value") + + current_app.logger.info(f"Received feedback request: {request_json}") + + if not trace_id or value is None: + return jsonify({"error": "trace_id and value are required"}), 400 + + # Add LLM provider info to the feedback + llm_provider = os.getenv("OPENAI_HOST", "openai") + score = langfuse.score( + trace_id=trace_id, + name="user_feedback_thumbs", + value=value, + comment=None, + metadata={ + "llm_provider": llm_provider, + "model": os.getenv("AZURE_OPENAI_CHATGPT_MODEL", "gpt-4") if llm_provider == "azure" else "gpt-4", + }, + ) + + current_app.logger.info(f"Successfully submitted feedback: {score}") + + return jsonify({"message": "Feedback received successfully", "trace_id": trace_id, "value": value}), 200 + except Exception as error: + current_app.logger.error(f"Error in feedback endpoint: {error}") + return jsonify({"error": str(error)}), 500 + + @bp.before_app_serving async def setup_clients(): # Replace these with your own values, either in environment variables or directly here diff --git a/app/backend/approaches/chatreadretrieveread.py b/app/backend/approaches/chatreadretrieveread.py index b752547e71..24705c2ab6 100644 --- a/app/backend/approaches/chatreadretrieveread.py +++ b/app/backend/approaches/chatreadretrieveread.py @@ -193,6 +193,11 @@ async def run_until_final_call( data_points = {"text": sources_content} + # Get trace_id from all possible locations + trace_id = ( + overrides.get("trace_id") or overrides.get("context", {}).get("trace_id") or auth_claims.get("trace_id") + ) + extra_info = { "data_points": data_points, "thoughts": [ @@ -231,8 +236,15 @@ async def run_until_final_call( ), ), ], + "trace_id": trace_id, # Add trace_id } + # Log for verification + print(f"trace_id in extra_info: {extra_info['trace_id']}") + + # Log for debugging + print(f"extra_info in run_until_final_call: {extra_info}") + chat_coroutine = self.openai_client.chat.completions.create( # Azure OpenAI takes the deployment name as the model name model=self.chatgpt_deployment if self.chatgpt_deployment else self.chatgpt_model, diff --git a/app/frontend/src/api/models.ts b/app/frontend/src/api/models.ts index ef1fa154b0..d8dbffefb5 100644 --- a/app/frontend/src/api/models.ts +++ b/app/frontend/src/api/models.ts @@ -1,3 +1,5 @@ +import { FeedbackProviderOptions } from "../components/HistoryProviders/IProvider"; + export const enum RetrievalMode { Hybrid = "hybrid", Vectors = "vectors", @@ -54,6 +56,7 @@ export type ResponseContext = { data_points: string[]; followup_questions: string[] | null; thoughts: Thoughts[]; + trace_id: string; // Make this required }; export type ChatAppResponseOrError = { @@ -67,7 +70,13 @@ export type ChatAppResponseOrError = { export type ChatAppResponse = { message: ResponseMessage; delta: ResponseMessage; - context: ResponseContext; + context: { + data_points: string[]; + followup_questions: string[] | null; + thoughts: Thoughts[]; + trace_id?: string; + feedback?: number; + }; session_state: any; }; @@ -123,3 +132,20 @@ export type HistroyApiResponse = { answers: any; timestamp: number; }; + +export interface AppConfig { + showUserFeedback: boolean; + feedbackProvider: FeedbackProviderOptions; +} + +export interface FeedbackRequest { + trace_id: string; + value: number; + type: "thumbs"; +} + +export type FeedbackResponse = { + trace_id: string; + status: "success" | "error"; + message?: string; +}; diff --git a/app/frontend/src/components/Answer/Answer.module.css b/app/frontend/src/components/Answer/Answer.module.css index 8722a67b02..329fc38ff8 100644 --- a/app/frontend/src/components/Answer/Answer.module.css +++ b/app/frontend/src/components/Answer/Answer.module.css @@ -119,6 +119,53 @@ sup { width: fit-content; } +.feedbackMessage { + position: fixed; + bottom: 1.25rem; + right: 1.25rem; + padding: 0.625rem 1.25rem; + background-color: #4caf50; + color: white; + border-radius: 0.25rem; + z-index: 1000; + animation: fadeInOut 3s ease; +} + +.feedbackButtonClicked i { + color: #323130 !important; /* Darker color for solid icons */ +} + +.feedbackButton:hover i { + color: #201f1e !important; /* Darker on hover */ +} + +.errorMessage { + position: fixed; + bottom: 1.25rem; + right: 1.25rem; + padding: 0.625rem 1.25rem; + background-color: #f44336; + color: white; + border-radius: 0.25rem; + z-index: 1000; + animation: fadeInOut 3s ease; +} + +@keyframes fadeInOut { + 0% { + opacity: 0; + } + 10% { + opacity: 1; + } + 90% { + opacity: 1; + } + 100% { + opacity: 0; + } +} + @keyframes loading { 0% { content: ""; diff --git a/app/frontend/src/components/Answer/Answer.tsx b/app/frontend/src/components/Answer/Answer.tsx index 75b0a03504..7776ae388d 100644 --- a/app/frontend/src/components/Answer/Answer.tsx +++ b/app/frontend/src/components/Answer/Answer.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from "react"; +import { useMemo, useState, useEffect, useRef, useContext } from "react"; import { Stack, IconButton } from "@fluentui/react"; import { useTranslation } from "react-i18next"; import DOMPurify from "dompurify"; @@ -7,11 +7,17 @@ import remarkGfm from "remark-gfm"; import rehypeRaw from "rehype-raw"; import styles from "./Answer.module.css"; -import { ChatAppResponse, getCitationFilePath, SpeechConfig } from "../../api"; +import { type ChatAppResponse, getCitationFilePath, type SpeechConfig } from "../../api"; import { parseAnswerToHtml } from "./AnswerParser"; import { AnswerIcon } from "./AnswerIcon"; import { SpeechOutputBrowser } from "./SpeechOutputBrowser"; import { SpeechOutputAzure } from "./SpeechOutputAzure"; +import { useFeedback } from "./FeedbackContext"; +import { useHistoryManager } from "../HistoryProviders/HistoryManager"; +import { HistoryProviderOptions } from "../HistoryProviders/IProvider"; +import { FeedbackStore } from "./FeedbackStore"; +import { useLogin } from "../../authConfig"; +import { LoginContext } from "../../loginContext"; interface Props { answer: ChatAppResponse; @@ -19,6 +25,7 @@ interface Props { speechConfig: SpeechConfig; isSelected?: boolean; isStreaming: boolean; + historyFeedback?: number; onCitationClicked: (filePath: string) => void; onThoughtProcessClicked: () => void; onSupportingContentClicked: () => void; @@ -28,28 +35,97 @@ interface Props { showSpeechOutputAzure?: boolean; } -export const Answer = ({ - answer, - index, - speechConfig, - isSelected, - isStreaming, - onCitationClicked, - onThoughtProcessClicked, - onSupportingContentClicked, - onFollowupQuestionClicked, - showFollowupQuestions, - showSpeechOutputAzure, - showSpeechOutputBrowser -}: Props) => { +interface FeedbackData { + store: FeedbackStore; + traceId: string | undefined; + historyValue: number | undefined; +} + +// Add this new hook to manage feedback state properly +const useFeedbackState = (traceId: string | undefined, historyFeedback: number | undefined) => { + const [feedbackValue, setFeedbackValue] = useState(null); + const initializeAttempted = useRef(false); + const feedbackStore = useMemo(() => FeedbackStore.getInstance(), []); + + // Reset state when traceId changes + useEffect(() => { + setFeedbackValue(null); + initializeAttempted.current = false; + }, [traceId]); + + // Load feedback data + useEffect(() => { + const loadFeedback = async () => { + if (!traceId || initializeAttempted.current) return; + + try { + initializeAttempted.current = true; + console.log(`Initializing feedback for trace ${traceId}`); + + // First try to get cached feedback + const cachedValue = await feedbackStore.getFeedback(traceId); + if (cachedValue !== null) { + console.log(`Found cached feedback for ${traceId}:`, cachedValue); + setFeedbackValue(cachedValue); + return; + } + + // Then try history feedback + if (historyFeedback !== undefined) { + console.log(`Using history feedback for ${traceId}:`, historyFeedback); + setFeedbackValue(historyFeedback); + await feedbackStore.setFeedback(traceId, historyFeedback); + } + } catch (error) { + console.error(`Failed to load feedback for ${traceId}:`, error); + initializeAttempted.current = false; + } + }; + + loadFeedback(); + }, [traceId, historyFeedback, feedbackStore]); + + return { feedbackValue, setFeedbackValue, feedbackStore }; +}; + +export const Answer = ({ answer, historyFeedback, ...props }: Props) => { const followupQuestions = answer.context?.followup_questions; - const parsedAnswer = useMemo(() => parseAnswerToHtml(answer, isStreaming, onCitationClicked), [answer]); + const parsedAnswer = useMemo( + () => parseAnswerToHtml(answer, props.isStreaming, props.onCitationClicked), + [answer, props.isStreaming, props.onCitationClicked] + ); const { t } = useTranslation(); const sanitizedAnswerHtml = DOMPurify.sanitize(parsedAnswer.answerHtml); const [copied, setCopied] = useState(false); + const { feedbackState } = useFeedback(); + const historyManager = useHistoryManager(HistoryProviderOptions.IndexedDB); + const traceId = answer.context?.trace_id; + const { loggedIn } = useContext(LoginContext); + const useAuth = useLogin && loggedIn; + + const answerContextValue = useMemo( + () => ({ + fullContext: answer.context, + traceId: answer.context?.trace_id, + fullAnswer: answer, + hasTrace: Boolean(answer.context?.trace_id), + contextKeys: Object.keys(answer.context || {}) + }), + [answer] + ); + + const feedback = useMemo( + () => ({ + store: FeedbackStore.getInstance(), + traceId: answer.context?.trace_id, + historyValue: historyFeedback + }), + [answer.context?.trace_id, historyFeedback] + ); + + const { feedbackValue, setFeedbackValue, feedbackStore } = useFeedbackState(answer.context?.trace_id, historyFeedback); const handleCopy = () => { - // Single replace to remove all HTML tags to remove the citations const textToCopy = sanitizedAnswerHtml.replace(/]*>\d+<\/sup><\/a>|<[^>]+>/g, ""); navigator.clipboard @@ -61,12 +137,100 @@ export const Answer = ({ .catch(err => console.error("Failed to copy text: ", err)); }; + const handleFeedback = async (value: number) => { + if (!feedback.traceId) return; + + try { + setFeedbackValue(value); + + await feedbackStore.setFeedback(feedback.traceId, value); + + if (useAuth) { + await Promise.all([ + historyManager.updateFeedback(feedback.traceId, value), + fetch("/feedback", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + trace_id: feedback.traceId, + value, + type: "thumbs" + }) + }).then(res => { + if (!res.ok) throw new Error("Failed to submit feedback to server"); + }) + ]); + } + + const storedValue = await feedbackStore.getFeedback(feedback.traceId); + if (storedValue !== value) { + console.warn(`Local feedback verification failed: stored=${storedValue}, expected=${value}`); + } + + // Show success message + const feedbackMessage = document.createElement("div"); + feedbackMessage.className = styles.feedbackMessage; + feedbackMessage.textContent = value ? "Thanks for the positive feedback!" : "Thanks for the feedback. We'll work to improve."; + document.body.appendChild(feedbackMessage); + setTimeout(() => feedbackMessage.remove(), 3000); + } catch (error) { + console.error(`Failed to store feedback for ${feedback.traceId}:`, error); + // Show error in UI + const errorMessage = document.createElement("div"); + errorMessage.className = styles.errorMessage; + errorMessage.textContent = error instanceof Error ? error.message : "Failed to submit feedback"; + document.body.appendChild(errorMessage); + setTimeout(() => errorMessage.remove(), 3000); + } + }; + + useEffect(() => { + const isDevelopment = import.meta.env?.DEV ?? false; + if (isDevelopment) { + console.log("Answer mounted with context:", answerContextValue); + } + }, [answerContextValue]); + + const renderFeedbackButton = () => { + if (feedbackValue !== null) { + return ( + + ); + } + + return ( + <> + handleFeedback(1)} + disabled={!traceId || props.isStreaming} + /> + handleFeedback(0)} + disabled={!traceId || props.isStreaming} + /> + + ); + }; + return ( - +
+ {renderFeedbackButton()} onThoughtProcessClicked()} + onClick={() => props.onThoughtProcessClicked()} disabled={!answer.context.thoughts?.length} /> onSupportingContentClicked()} + onClick={() => props.onSupportingContentClicked()} disabled={!answer.context.data_points} /> - {showSpeechOutputAzure && ( - + {props.showSpeechOutputAzure && ( + )} - {showSpeechOutputBrowser && } + {props.showSpeechOutputBrowser && }
@@ -111,7 +280,7 @@ export const Answer = ({ {parsedAnswer.citations.map((x, i) => { const path = getCitationFilePath(x); return ( - onCitationClicked(path)}> + props.onCitationClicked(path)}> {`${++i}. ${x}`} ); @@ -120,17 +289,20 @@ export const Answer = ({ )} - {!!followupQuestions?.length && showFollowupQuestions && onFollowupQuestionClicked && ( + {!!followupQuestions?.length && props.showFollowupQuestions && props.onFollowupQuestionClicked && ( {t("followupQuestions")} - {followupQuestions.map((x, i) => { - return ( - onFollowupQuestionClicked(x)}> - {`${x}`} - - ); - })} + {followupQuestions.map((x, i) => ( + props.onFollowupQuestionClicked && props.onFollowupQuestionClicked(x)} + > + {x} + + ))} )} diff --git a/app/frontend/src/components/Answer/FeedbackContext.tsx b/app/frontend/src/components/Answer/FeedbackContext.tsx new file mode 100644 index 0000000000..02d0c7c0e5 --- /dev/null +++ b/app/frontend/src/components/Answer/FeedbackContext.tsx @@ -0,0 +1,135 @@ +import { createContext, useContext, useState, useCallback, useEffect, useRef } from "react"; +import { openDB, IDBPDatabase } from "idb"; + +type FeedbackState = { + [traceId: string]: number; +}; + +interface FeedbackContextType { + feedbackState: FeedbackState; + setFeedback: (traceId: string, value: number) => Promise; + loadSavedFeedback: (traceId: string) => Promise; +} + +const FeedbackContext = createContext({ + feedbackState: {}, + setFeedback: async () => {}, + loadSavedFeedback: async () => null +}); + +export const FeedbackProvider: React.FC<{ children: React.ReactNode }> = ({ children }) => { + const [feedbackState, setFeedbackState] = useState({}); + const dbRef = useRef(); + const initialized = useRef(false); + + // Consolidated cleanup function + const cleanup = useCallback(() => { + if (dbRef.current) { + dbRef.current.close(); + } + initialized.current = false; + // Don't reset state on every cleanup - only when component unmounts + }, []); + + // Single useEffect for initialization and cleanup + useEffect(() => { + const init = async () => { + if (initialized.current) return; + + try { + const db = await openDB("feedback-store", 1, { + upgrade(db) { + if (!db.objectStoreNames.contains("feedback")) { + db.createObjectStore("feedback"); + } + } + }); + dbRef.current = db; + + // Load existing feedback + const tx = db.transaction("feedback", "readonly"); + const store = tx.objectStore("feedback"); + const keys = await store.getAllKeys(); + const values = await Promise.all( + keys.map(async key => ({ + key: key as string, + value: await store.get(key) + })) + ); + + // Update state with existing feedback + const newState: FeedbackState = {}; + values.forEach(({ key, value }) => { + if (value !== undefined) { + newState[key] = value; + } + }); + + setFeedbackState(newState); + initialized.current = true; + console.log("Feedback store initialized with:", newState); + } catch (error) { + console.error("Failed to initialize feedback store:", error); + } + }; + + init(); + + // Only reset state when component is unmounted + return () => { + cleanup(); + setFeedbackState({}); + }; + }, [cleanup]); + + const setFeedback = useCallback(async (traceId: string, value: number) => { + if (!dbRef.current) { + throw new Error("Feedback store not initialized"); + } + + try { + // Update DB first + await dbRef.current.put("feedback", value, traceId); + + // Only update state if DB operation succeeded + setFeedbackState(prev => { + const newState = { ...prev, [traceId]: value }; + console.log(`Updated feedback state for ${traceId}:`, newState); + return newState; + }); + } catch (error) { + console.error(`Failed to set feedback for ${traceId}:`, error); + throw error; + } + }, []); + + const loadSavedFeedback = useCallback( + async (traceId: string): Promise => { + // Check memory state first + if (feedbackState[traceId] !== undefined) { + return feedbackState[traceId]; + } + + if (!dbRef.current) { + console.warn("Feedback store not initialized"); + return null; + } + + try { + const value = await dbRef.current.get("feedback", traceId); + if (value !== undefined) { + setFeedbackState(prev => ({ ...prev, [traceId]: value })); + } + return value ?? null; + } catch (error) { + console.error(`Failed to load feedback for ${traceId}:`, error); + return null; + } + }, + [feedbackState] + ); + + return {children}; +}; + +export const useFeedback = () => useContext(FeedbackContext); diff --git a/app/frontend/src/components/Answer/FeedbackStore.ts b/app/frontend/src/components/Answer/FeedbackStore.ts new file mode 100644 index 0000000000..6060a9d820 --- /dev/null +++ b/app/frontend/src/components/Answer/FeedbackStore.ts @@ -0,0 +1,140 @@ +import { openDB, IDBPDatabase } from "idb"; + +export class FeedbackStore { + private static instance: FeedbackStore; + private readonly dbName = "ChatFeedbackDB"; + private readonly storeName = "feedback"; + private dbPromise: Promise; + private memoryCache: Map = new Map(); + private initPromise: Promise; + private initialized = false; + + private constructor() { + this.dbPromise = this.initializeDB(); + this.initPromise = this.loadAllFeedback(); + } + + public static getInstance(): FeedbackStore { + if (!FeedbackStore.instance) { + FeedbackStore.instance = new FeedbackStore(); + } + return FeedbackStore.instance; + } + + private initializeDB(): Promise { + return new Promise((resolve, reject) => { + const request = indexedDB.open(this.dbName, 1); + + request.onerror = () => reject(request.error); + + request.onupgradeneeded = event => { + const db = (event.target as IDBOpenDBRequest).result; + if (!db.objectStoreNames.contains(this.storeName)) { + db.createObjectStore(this.storeName); + } + }; + + request.onsuccess = () => resolve(request.result); + }); + } + + private async loadAllFeedback(): Promise { + if (this.initialized) return; + + try { + const db = await this.dbPromise; + const tx = db.transaction(this.storeName, "readonly"); + const store = tx.objectStore(this.storeName); + const keys = await new Promise((resolve, reject) => { + const request = store.getAllKeys(); + request.onsuccess = () => resolve(request.result as string[]); + request.onerror = () => reject(request.error); + }); + const values = await Promise.all( + keys.map(async key => ({ + key: key as string, + value: await new Promise((resolve, reject) => { + const request = store.get(key); + request.onsuccess = () => resolve(request.result); + request.onerror = () => reject(request.error); + }) + })) + ); + + values.forEach(({ key, value }) => { + if (value !== undefined) { + this.memoryCache.set(key, value); + } + }); + + console.log("Loaded feedback cache:", Object.fromEntries(this.memoryCache)); + this.initialized = true; + } catch (error) { + console.error("Failed to load feedback cache:", error); + throw error; + } + } + + private async ensureInitialized(): Promise { + if (this.initialized) return; + await this.initPromise; + } + + async getFeedback(traceId: string): Promise { + await this.ensureInitialized(); + + // First check memory cache + const cached = this.memoryCache.get(traceId); + if (cached !== undefined) { + return cached; + } + + try { + const db = await this.dbPromise; + const request = await db.transaction(this.storeName).objectStore(this.storeName).get(traceId); + + const value = request.result; + + if (value !== undefined) { + this.memoryCache.set(traceId, value); + } + return value ?? null; + } catch (error) { + console.error(`Error getting feedback for ${traceId}:`, error); + return null; + } + } + + async setFeedback(traceId: string, value: number): Promise { + await this.initPromise; + + try { + const db = await this.dbPromise; + const transaction = db.transaction(this.storeName, "readwrite"); + const store = transaction.objectStore(this.storeName); + + return new Promise((resolve, reject) => { + const request = store.put(value, traceId); + + request.onsuccess = () => { + this.memoryCache.set(traceId, value); + console.log(`FeedbackStore: Stored feedback=${value} for ${traceId}`); + resolve(); + }; + + request.onerror = () => { + console.error(`FeedbackStore: Failed to store feedback=${value} for ${traceId}`, request.error); + reject(request.error); + }; + }); + } catch (error) { + this.memoryCache.delete(traceId); // Remove from cache if storage fails + throw error; + } + } + + async getAllFeedback(): Promise> { + await this.initPromise; + return new Map(this.memoryCache); + } +} diff --git a/app/frontend/src/components/Answer/index.ts b/app/frontend/src/components/Answer/index.ts index 96291567d8..8a3e4c9fa6 100644 --- a/app/frontend/src/components/Answer/index.ts +++ b/app/frontend/src/components/Answer/index.ts @@ -3,3 +3,4 @@ export * from "./AnswerLoading"; export * from "./AnswerError"; export * from "./SpeechOutputBrowser"; export * from "./SpeechOutputAzure"; +export * from "./FeedbackContext"; diff --git a/app/frontend/src/components/HistoryPanel/HistoryPanel.tsx b/app/frontend/src/components/HistoryPanel/HistoryPanel.tsx index acaf3b7870..557c8033b4 100644 --- a/app/frontend/src/components/HistoryPanel/HistoryPanel.tsx +++ b/app/frontend/src/components/HistoryPanel/HistoryPanel.tsx @@ -1,28 +1,25 @@ import { useMsal } from "@azure/msal-react"; import { getToken, useLogin } from "../../authConfig"; import { Panel, PanelType, Spinner } from "@fluentui/react"; -import { useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useRef, useState, useCallback } from "react"; import { HistoryData, HistoryItem } from "../HistoryItem"; import { Answers, HistoryProviderOptions } from "../HistoryProviders/IProvider"; import { useHistoryManager, HistoryMetaData } from "../HistoryProviders"; import { useTranslation } from "react-i18next"; import styles from "./HistoryPanel.module.css"; +import { type ChatAppResponse } from "../../api"; const HISTORY_COUNT_PER_LOAD = 20; -export const HistoryPanel = ({ - provider, - isOpen, - notify, - onClose, - onChatSelected -}: { +type HistoryPanelProps = { provider: HistoryProviderOptions; isOpen: boolean; notify: boolean; onClose: () => void; - onChatSelected: (answers: Answers) => void; -}) => { + onChatSelected: (answers: [string, ChatAppResponse][]) => void; +}; + +export const HistoryPanel = ({ provider, isOpen, notify, onClose, onChatSelected }: HistoryPanelProps) => { const historyManager = useHistoryManager(provider); const [history, setHistory] = useState([]); const [isLoading, setIsLoading] = useState(false); @@ -50,13 +47,20 @@ export const HistoryPanel = ({ setIsLoading(() => false); }; - const handleSelect = async (id: string) => { - const token = client ? await getToken(client) : undefined; - const item = await historyManager.getItem(id, token); - if (item) { - onChatSelected(item); - } - }; + const handleSelect = useCallback( + async (id: string) => { + try { + const item = await historyManager.getItem(id); + if (item) { + onChatSelected(item); + onClose(); + } + } catch (error) { + console.error("Error loading chat history:", error); + } + }, + [historyManager, onChatSelected, onClose] + ); const handleDelete = async (id: string) => { const token = client ? await getToken(client) : undefined; diff --git a/app/frontend/src/components/HistoryProviders/CosmosDB.ts b/app/frontend/src/components/HistoryProviders/CosmosDB.ts index 4d613b28a8..cbcb33064f 100644 --- a/app/frontend/src/components/HistoryProviders/CosmosDB.ts +++ b/app/frontend/src/components/HistoryProviders/CosmosDB.ts @@ -48,4 +48,19 @@ export class CosmosDBProvider implements IHistoryProvider { await deleteChatHistoryApi(id, idToken || ""); return; } + + async updateFeedback(id: string, feedback: number): Promise { + // Assuming you have an API endpoint for updating feedback in CosmosDB + try { + await fetch(`/api/history/${id}/feedback`, { + method: "PUT", + headers: { + "Content-Type": "application/json" + }, + body: JSON.stringify({ feedback }) + }); + } catch (error) { + console.error("Error updating feedback in CosmosDB:", error); + } + } } diff --git a/app/frontend/src/components/HistoryProviders/FeedbackStore.ts b/app/frontend/src/components/HistoryProviders/FeedbackStore.ts new file mode 100644 index 0000000000..c1e06a176d --- /dev/null +++ b/app/frontend/src/components/HistoryProviders/FeedbackStore.ts @@ -0,0 +1,145 @@ +import { openDB, IDBPDatabase } from "idb"; + +export class FeedbackStore { + private static instance: FeedbackStore; + private readonly dbName = "ChatFeedbackDB"; + private readonly storeName = "feedback"; + private dbPromise: Promise; + private memoryCache: Map = new Map(); + private initPromise: Promise; + private initialized = false; + + private constructor() { + this.dbPromise = this.initializeDB(); + this.initPromise = this.loadAllFeedback(); + } + + public static getInstance(): FeedbackStore { + if (!FeedbackStore.instance) { + FeedbackStore.instance = new FeedbackStore(); + } + return FeedbackStore.instance; + } + + private initializeDB(): Promise { + return new Promise((resolve, reject) => { + const request = indexedDB.open(this.dbName, 1); + + request.onerror = () => reject(request.error); + + request.onupgradeneeded = event => { + const db = (event.target as IDBOpenDBRequest).result; + if (!db.objectStoreNames.contains(this.storeName)) { + db.createObjectStore(this.storeName); + } + }; + + request.onsuccess = () => resolve(request.result); + }); + } + + private async loadAllFeedback(): Promise { + if (this.initialized) return; + + try { + const db = await this.dbPromise; + const tx = db.transaction(this.storeName, "readonly"); + const store = tx.objectStore(this.storeName); + const keys = await new Promise((resolve, reject) => { + const request = store.getAllKeys(); + request.onsuccess = () => resolve(request.result as string[]); + request.onerror = () => reject(request.error); + }); + const values = await Promise.all( + keys.map(async key => ({ + key: key as string, + value: await new Promise((resolve, reject) => { + const request = store.get(key); + request.onsuccess = () => resolve(request.result); + request.onerror = () => reject(request.error); + }) + })) + ); + + values.forEach(({ key, value }) => { + if (value !== undefined) { + this.memoryCache.set(key, value); + } + }); + + console.log("Loaded feedback cache:", Object.fromEntries(this.memoryCache)); + this.initialized = true; + } catch (error) { + console.error("Failed to load feedback cache:", error); + throw error; + } + } + + async getFeedback(traceId: string): Promise { + await this.initPromise; + + // Check memory cache first + if (this.memoryCache.has(traceId)) { + return this.memoryCache.get(traceId) ?? null; + } + + try { + const db = await this.dbPromise; + const transaction = db.transaction(this.storeName, "readonly"); + const store = transaction.objectStore(this.storeName); + + return new Promise((resolve, reject) => { + const request = store.get(traceId); + request.onsuccess = () => { + const value = request.result; + if (value !== undefined) { + this.memoryCache.set(traceId, value); + } + resolve(value ?? null); + }; + request.onerror = () => reject(request.error); + }); + } catch (error) { + console.error(`FeedbackStore: Error getting feedback for ${traceId}:`, error); + return null; + } + } + + async setFeedback(traceId: string, value: number): Promise { + await this.initPromise; + + try { + const db = await this.dbPromise; + const transaction = db.transaction(this.storeName, "readwrite"); + const store = transaction.objectStore(this.storeName); + + // Return a promise that only resolves when transaction is complete + return new Promise((resolve, reject) => { + transaction.oncomplete = () => { + this.memoryCache.set(traceId, value); + console.log(`FeedbackStore: Transaction completed for ${traceId}, value=${value}`); + resolve(); + }; + + transaction.onerror = () => { + console.error(`FeedbackStore: Transaction failed for ${traceId}`, transaction.error); + reject(transaction.error); + }; + + const request = store.put(value, traceId); + request.onerror = () => { + console.error(`FeedbackStore: Failed to store feedback=${value} for ${traceId}`, request.error); + reject(request.error); + }; + }); + } catch (error) { + console.error(`FeedbackStore: Error setting feedback for ${traceId}:`, error); + throw error; + } + } + + async getAllFeedback(): Promise> { + await this.initPromise; + return new Map(this.memoryCache); + } +} diff --git a/app/frontend/src/components/HistoryProviders/HistoryManager.ts b/app/frontend/src/components/HistoryProviders/HistoryManager.ts index e796d04933..6819d9e324 100644 --- a/app/frontend/src/components/HistoryProviders/HistoryManager.ts +++ b/app/frontend/src/components/HistoryProviders/HistoryManager.ts @@ -1,11 +1,11 @@ import { useMemo } from "react"; -import { IHistoryProvider, HistoryProviderOptions } from "../HistoryProviders/IProvider"; -import { NoneProvider } from "../HistoryProviders/None"; -import { IndexedDBProvider } from "../HistoryProviders/IndexedDB"; -import { CosmosDBProvider } from "../HistoryProviders/CosmosDB"; +import { IHistoryProvider, HistoryProviderOptions } from "./IProvider"; +import { NoneProvider } from "./None"; +import { IndexedDBProvider } from "./IndexedDB"; +import { CosmosDBProvider } from "./CosmosDB"; export const useHistoryManager = (provider: HistoryProviderOptions): IHistoryProvider => { - const providerInstance = useMemo(() => { + return useMemo(() => { switch (provider) { case HistoryProviderOptions.IndexedDB: return new IndexedDBProvider("chat-database", "chat-history"); @@ -16,6 +16,4 @@ export const useHistoryManager = (provider: HistoryProviderOptions): IHistoryPro return new NoneProvider(); } }, [provider]); - - return providerInstance; }; diff --git a/app/frontend/src/components/HistoryProviders/IProvider.ts b/app/frontend/src/components/HistoryProviders/IProvider.ts index 026443d681..4ead05d34b 100644 --- a/app/frontend/src/components/HistoryProviders/IProvider.ts +++ b/app/frontend/src/components/HistoryProviders/IProvider.ts @@ -1,6 +1,11 @@ import { ChatAppResponse } from "../../api"; -export type HistoryMetaData = { id: string; title: string; timestamp: number }; +export type HistoryMetaData = { + id: string; + title: string; + timestamp: number; + feedback?: number; // Add feedback field +}; export type Answers = [user: string, response: ChatAppResponse][]; export const enum HistoryProviderOptions { @@ -16,4 +21,17 @@ export interface IHistoryProvider { addItem(id: string, answers: Answers, idToken?: string): Promise; getItem(id: string, idToken?: string): Promise; deleteItem(id: string, idToken?: string): Promise; + updateFeedback(id: string, feedback: number): Promise; // Add this method +} + +export interface IFeedbackProvider { + getFeedback(traceId: string): Promise; + setFeedback(traceId: string, value: number): Promise; + getAllFeedback(): Promise>; +} + +export enum FeedbackProviderOptions { + None = "none", + IndexedDB = "indexeddb", + CosmosDB = "cosmosdb" } diff --git a/app/frontend/src/components/HistoryProviders/IndexedDB.ts b/app/frontend/src/components/HistoryProviders/IndexedDB.ts index ca8fa19fe8..42ecb8dca7 100644 --- a/app/frontend/src/components/HistoryProviders/IndexedDB.ts +++ b/app/frontend/src/components/HistoryProviders/IndexedDB.ts @@ -76,17 +76,42 @@ export class IndexedDBProvider implements IHistoryProvider { async addItem(id: string, answers: Answers): Promise { const timestamp = new Date().getTime(); - const db = await this.init(); // 自動的に初期化 + const db = await this.init(); const tx = db.transaction(this.storeName, "readwrite"); const current = await tx.objectStore(this.storeName).get(id); + if (current) { - await tx.objectStore(this.storeName).put({ ...current, id, timestamp, answers }); + await tx.objectStore(this.storeName).put({ + ...current, + id, + timestamp, + answers, + feedback: current.feedback // Preserve existing feedback + }); } else { const title = answers[0][0].length > 50 ? answers[0][0].substring(0, 50) + "..." : answers[0][0]; - await tx.objectStore(this.storeName).add({ id, title, timestamp, answers }); + await tx.objectStore(this.storeName).add({ + id, + title, + timestamp, + answers, + feedback: undefined + }); + } + await tx.done; + } + + async updateFeedback(id: string, feedback: number): Promise { + const db = await this.init(); + const tx = db.transaction(this.storeName, "readwrite"); + const current = await tx.objectStore(this.storeName).get(id); + if (current) { + await tx.objectStore(this.storeName).put({ + ...current, + feedback + }); } await tx.done; - return; } async getItem(id: string): Promise { diff --git a/app/frontend/src/components/HistoryProviders/None.ts b/app/frontend/src/components/HistoryProviders/None.ts index a662d54f72..f445f52e2a 100644 --- a/app/frontend/src/components/HistoryProviders/None.ts +++ b/app/frontend/src/components/HistoryProviders/None.ts @@ -17,4 +17,7 @@ export class NoneProvider implements IHistoryProvider { async deleteItem(id: string): Promise { return; } + async updateFeedback(_id: string, _feedback: number): Promise { + return Promise.resolve(); + } } diff --git a/app/frontend/src/pages/chat/Chat.tsx b/app/frontend/src/pages/chat/Chat.tsx index 7dabff8c7f..f2437866b8 100644 --- a/app/frontend/src/pages/chat/Chat.tsx +++ b/app/frontend/src/pages/chat/Chat.tsx @@ -52,7 +52,7 @@ const Chat = () => { const [useSemanticCaptions, setUseSemanticCaptions] = useState(false); const [includeCategory, setIncludeCategory] = useState(""); const [excludeCategory, setExcludeCategory] = useState(""); - const [useSuggestFollowupQuestions, setUseSuggestFollowupQuestions] = useState(false); + const [useSuggestFollowupQuestions, setUseSuggestFollowupQuestions] = useState(true); const [vectorFieldList, setVectorFieldList] = useState([VectorFieldOptions.Embedding]); const [useOidSecurityFilter, setUseOidSecurityFilter] = useState(false); const [useGroupsSecurityFilter, setUseGroupsSecurityFilter] = useState(false); @@ -473,7 +473,7 @@ const Chat = () => { isOpen={isHistoryPanelOpen} notify={!isStreaming && !isLoading} onClose={() => setIsHistoryPanelOpen(false)} - onChatSelected={answers => { + onChatSelected={(answers: [string, ChatAppResponse][]) => { if (answers.length === 0) return; setAnswers(answers); lastQuestionRef.current = answers[answers.length - 1][0]; From a394e0490dc2a505e442f0d675dbc4fad7325f0c Mon Sep 17 00:00:00 2001 From: bnodir Date: Sun, 12 Jan 2025 10:56:46 +0900 Subject: [PATCH 2/4] feedback score --- app/frontend/src/components/Answer/Answer.tsx | 48 +++++++++++++------ 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/app/frontend/src/components/Answer/Answer.tsx b/app/frontend/src/components/Answer/Answer.tsx index 7776ae388d..ebf5832753 100644 --- a/app/frontend/src/components/Answer/Answer.tsx +++ b/app/frontend/src/components/Answer/Answer.tsx @@ -142,24 +142,42 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { try { setFeedbackValue(value); - await feedbackStore.setFeedback(feedback.traceId, value); - if (useAuth) { - await Promise.all([ - historyManager.updateFeedback(feedback.traceId, value), - fetch("/feedback", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - trace_id: feedback.traceId, - value, - type: "thumbs" - }) - }).then(res => { - if (!res.ok) throw new Error("Failed to submit feedback to server"); + // Send single feedback request that handles both history and Langfuse + try { + const res = await fetch("/feedback", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + trace_id: feedback.traceId, + value, // For history + score: value, // For Langfuse + type: "user_feedback", // For Langfuse + comment: value === 1 ? "👍 Positive feedback" : "👎 Negative feedback" // For Langfuse }) - ]); + }); + + const responseBody = await res.text(); + const isHtmlError = responseBody.trim().startsWith(" Date: Sun, 12 Jan 2025 13:23:40 +0900 Subject: [PATCH 3/4] with comment --- app/backend/app.py | 18 +++++++++------- app/frontend/src/components/Answer/Answer.tsx | 21 +++++++++---------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/app/backend/app.py b/app/backend/app.py index fe12df503f..faaf2aacd7 100644 --- a/app/backend/app.py +++ b/app/backend/app.py @@ -487,29 +487,33 @@ async def feedback(auth_claims: Dict[str, Any]): try: request_json = await request.get_json() trace_id = request_json.get("trace_id") - value = request_json.get("value") + value = request_json.get("value") # For thumbs history + score = request_json.get("score", value) # For Langfuse, fallback to value if not provided + feedback_type = request_json.get("type", "thumbs") + comment = request_json.get("comment") current_app.logger.info(f"Received feedback request: {request_json}") - if not trace_id or value is None: - return jsonify({"error": "trace_id and value are required"}), 400 + if not trace_id or (value is None and score is None): + return jsonify({"error": "trace_id and value/score are required"}), 400 # Add LLM provider info to the feedback llm_provider = os.getenv("OPENAI_HOST", "openai") score = langfuse.score( trace_id=trace_id, name="user_feedback_thumbs", - value=value, - comment=None, + value=score, # Use score for Langfuse + comment=comment, metadata={ "llm_provider": llm_provider, "model": os.getenv("AZURE_OPENAI_CHATGPT_MODEL", "gpt-4") if llm_provider == "azure" else "gpt-4", + "feedback_type": feedback_type, }, ) - current_app.logger.info(f"Successfully submitted feedback: {score}") - + current_app.logger.info(f"Successfully submitted feedback to Langfuse: {score}") return jsonify({"message": "Feedback received successfully", "trace_id": trace_id, "value": value}), 200 + except Exception as error: current_app.logger.error(f"Error in feedback endpoint: {error}") return jsonify({"error": str(error)}), 500 diff --git a/app/frontend/src/components/Answer/Answer.tsx b/app/frontend/src/components/Answer/Answer.tsx index ebf5832753..e73487174d 100644 --- a/app/frontend/src/components/Answer/Answer.tsx +++ b/app/frontend/src/components/Answer/Answer.tsx @@ -41,7 +41,7 @@ interface FeedbackData { historyValue: number | undefined; } -// Add this new hook to manage feedback state properly +// New custom hook for feedback state management const useFeedbackState = (traceId: string | undefined, historyFeedback: number | undefined) => { const [feedbackValue, setFeedbackValue] = useState(null); const initializeAttempted = useRef(false); @@ -53,7 +53,7 @@ const useFeedbackState = (traceId: string | undefined, historyFeedback: number | initializeAttempted.current = false; }, [traceId]); - // Load feedback data + // Load feedback data with improved error handling useEffect(() => { const loadFeedback = async () => { if (!traceId || initializeAttempted.current) return; @@ -62,7 +62,6 @@ const useFeedbackState = (traceId: string | undefined, historyFeedback: number | initializeAttempted.current = true; console.log(`Initializing feedback for trace ${traceId}`); - // First try to get cached feedback const cachedValue = await feedbackStore.getFeedback(traceId); if (cachedValue !== null) { console.log(`Found cached feedback for ${traceId}:`, cachedValue); @@ -70,7 +69,6 @@ const useFeedbackState = (traceId: string | undefined, historyFeedback: number | return; } - // Then try history feedback if (historyFeedback !== undefined) { console.log(`Using history feedback for ${traceId}:`, historyFeedback); setFeedbackValue(historyFeedback); @@ -137,6 +135,7 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { .catch(err => console.error("Failed to copy text: ", err)); }; + // Enhanced error handling in feedback submission const handleFeedback = async (value: number) => { if (!feedback.traceId) return; @@ -144,17 +143,16 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { setFeedbackValue(value); await feedbackStore.setFeedback(feedback.traceId, value); - // Send single feedback request that handles both history and Langfuse try { const res = await fetch("/feedback", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ trace_id: feedback.traceId, - value, // For history - score: value, // For Langfuse - type: "user_feedback", // For Langfuse - comment: value === 1 ? "👍 Positive feedback" : "👎 Negative feedback" // For Langfuse + value, + score: value, + type: "user_feedback", + comment: value === 1 ? "👍 Positive feedback" : "👎 Negative feedback" }) }); @@ -185,7 +183,7 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { console.warn(`Local feedback verification failed: stored=${storedValue}, expected=${value}`); } - // Show success message + // Show success message with proper cleanup const feedbackMessage = document.createElement("div"); feedbackMessage.className = styles.feedbackMessage; feedbackMessage.textContent = value ? "Thanks for the positive feedback!" : "Thanks for the feedback. We'll work to improve."; @@ -193,7 +191,8 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { setTimeout(() => feedbackMessage.remove(), 3000); } catch (error) { console.error(`Failed to store feedback for ${feedback.traceId}:`, error); - // Show error in UI + + // Show error message with proper cleanup const errorMessage = document.createElement("div"); errorMessage.className = styles.errorMessage; errorMessage.textContent = error instanceof Error ? error.message : "Failed to submit feedback"; From ca75a73a85249ea985d92d0a7308c52d40e3f632 Mon Sep 17 00:00:00 2001 From: bnodir Date: Sun, 12 Jan 2025 13:47:19 +0900 Subject: [PATCH 4/4] feedback dialog --- app/frontend/src/components/Answer/Answer.tsx | 202 ++++++++++-------- .../src/components/Answer/FeedbackDialog.tsx | 48 +++++ app/frontend/src/components/Answer/index.ts | 1 + 3 files changed, 161 insertions(+), 90 deletions(-) create mode 100644 app/frontend/src/components/Answer/FeedbackDialog.tsx diff --git a/app/frontend/src/components/Answer/Answer.tsx b/app/frontend/src/components/Answer/Answer.tsx index e73487174d..d960a22c3c 100644 --- a/app/frontend/src/components/Answer/Answer.tsx +++ b/app/frontend/src/components/Answer/Answer.tsx @@ -18,6 +18,7 @@ import { HistoryProviderOptions } from "../HistoryProviders/IProvider"; import { FeedbackStore } from "./FeedbackStore"; import { useLogin } from "../../authConfig"; import { LoginContext } from "../../loginContext"; +import { FeedbackDialog } from "./FeedbackDialog"; interface Props { answer: ChatAppResponse; @@ -123,6 +124,9 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { const { feedbackValue, setFeedbackValue, feedbackStore } = useFeedbackState(answer.context?.trace_id, historyFeedback); + const [showFeedbackDialog, setShowFeedbackDialog] = useState(false); + const [pendingFeedbackValue, setPendingFeedbackValue] = useState(null); + const handleCopy = () => { const textToCopy = sanitizedAnswerHtml.replace(/]*>\d+<\/sup><\/a>|<[^>]+>/g, ""); @@ -138,10 +142,16 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { // Enhanced error handling in feedback submission const handleFeedback = async (value: number) => { if (!feedback.traceId) return; + setPendingFeedbackValue(value); + setShowFeedbackDialog(true); + }; + + const handleFeedbackSubmit = async (comment: string) => { + if (!feedback.traceId || pendingFeedbackValue === null) return; try { - setFeedbackValue(value); - await feedbackStore.setFeedback(feedback.traceId, value); + setFeedbackValue(pendingFeedbackValue); + await feedbackStore.setFeedback(feedback.traceId, pendingFeedbackValue); try { const res = await fetch("/feedback", { @@ -149,10 +159,10 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ trace_id: feedback.traceId, - value, - score: value, + value: pendingFeedbackValue, + score: pendingFeedbackValue, type: "user_feedback", - comment: value === 1 ? "👍 Positive feedback" : "👎 Negative feedback" + comment: comment || (pendingFeedbackValue === 1 ? "👍 Positive feedback" : "👎 Negative feedback") }) }); @@ -160,33 +170,33 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { const isHtmlError = responseBody.trim().startsWith(" feedbackMessage.remove(), 3000); } catch (error) { @@ -198,6 +208,9 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { errorMessage.textContent = error instanceof Error ? error.message : "Failed to submit feedback"; document.body.appendChild(errorMessage); setTimeout(() => errorMessage.remove(), 3000); + } finally { + setShowFeedbackDialog(false); + setPendingFeedbackValue(null); } }; @@ -242,87 +255,96 @@ export const Answer = ({ answer, historyFeedback, ...props }: Props) => { }; return ( - - - - -
- {renderFeedbackButton()} - - props.onThoughtProcessClicked()} - disabled={!answer.context.thoughts?.length} - /> - props.onSupportingContentClicked()} - disabled={!answer.context.data_points} - /> - {props.showSpeechOutputAzure && ( - - )} - {props.showSpeechOutputBrowser && } -
- - - - -
- -
- - - {!!parsedAnswer.citations.length && ( + <> + - - {t("citationWithColon")} - {parsedAnswer.citations.map((x, i) => { - const path = getCitationFilePath(x); - return ( - props.onCitationClicked(path)}> - {`${++i}. ${x}`} - - ); - })} + + +
+ {renderFeedbackButton()} + + props.onThoughtProcessClicked()} + disabled={!answer.context.thoughts?.length} + /> + props.onSupportingContentClicked()} + disabled={!answer.context.data_points} + /> + {props.showSpeechOutputAzure && ( + + )} + {props.showSpeechOutputBrowser && } +
- )} - {!!followupQuestions?.length && props.showFollowupQuestions && props.onFollowupQuestionClicked && ( - - - {t("followupQuestions")} - {followupQuestions.map((x, i) => ( - props.onFollowupQuestionClicked && props.onFollowupQuestionClicked(x)} - > - {x} - - ))} - + +
+ +
- )} - + + {!!parsedAnswer.citations.length && ( + + + {t("citationWithColon")} + {parsedAnswer.citations.map((x, i) => { + const path = getCitationFilePath(x); + return ( + props.onCitationClicked(path)}> + {`${++i}. ${x}`} + + ); + })} + + + )} + + {!!followupQuestions?.length && props.showFollowupQuestions && props.onFollowupQuestionClicked && ( + + + {t("followupQuestions")} + {followupQuestions.map((x, i) => ( + props.onFollowupQuestionClicked && props.onFollowupQuestionClicked(x)} + > + {x} + + ))} + + + )} + + + setShowFeedbackDialog(false)} + onSubmit={handleFeedbackSubmit} + /> + ); }; diff --git a/app/frontend/src/components/Answer/FeedbackDialog.tsx b/app/frontend/src/components/Answer/FeedbackDialog.tsx new file mode 100644 index 0000000000..6a52f811ed --- /dev/null +++ b/app/frontend/src/components/Answer/FeedbackDialog.tsx @@ -0,0 +1,48 @@ +import React from "react"; +import { Dialog, DialogType, TextField, DefaultButton, PrimaryButton, Stack } from "@fluentui/react"; + +interface FeedbackDialogProps { + isOpen: boolean; + value: number; + onDismiss: () => void; + onSubmit: (comment: string) => void; +} + +export const FeedbackDialog: React.FC = ({ isOpen, value, onDismiss, onSubmit }) => { + const [comment, setComment] = React.useState(""); + + const handleSubmit = () => { + onSubmit(comment); + setComment(""); // Reset comment after submission + }; + + return ( + + ); +}; diff --git a/app/frontend/src/components/Answer/index.ts b/app/frontend/src/components/Answer/index.ts index 8a3e4c9fa6..2c5db718de 100644 --- a/app/frontend/src/components/Answer/index.ts +++ b/app/frontend/src/components/Answer/index.ts @@ -4,3 +4,4 @@ export * from "./AnswerError"; export * from "./SpeechOutputBrowser"; export * from "./SpeechOutputAzure"; export * from "./FeedbackContext"; +export * from "./FeedbackDialog";