From e79ffff1dc0734ebba362f4cfec7c514000e947a Mon Sep 17 00:00:00 2001
From: chihlasm <michael@chihlas.com>
Date: Fri, 27 Feb 2026 08:53:54 -0500
Subject: [PATCH] fix: address code review findings for AI chat builder
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- C1: Fix race condition in handleReset — await abandonSession before
  starting new session to prevent store state corruption
- I1: Extract error messages from Axios response.data.detail instead of
  generic error.message — users now see helpful backend messages (quota
  limits, message caps, etc.)
- I2: Add isGenerating guard in generateTree store action to prevent
  concurrent generation requests on double-click
- I3: Add isResponding guard in sendMessage to prevent concurrent sends
- M5: Remove redundant type casts on flowType
- M6: Add rate limiter to DELETE /sessions/{id} for consistency

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 backend/app/api/endpoints/ai_chat.py     |  2 ++
 frontend/src/pages/AIChatBuilderPage.tsx |  8 +++----
 frontend/src/store/aiChatStore.ts        | 30 ++++++++++++++----------
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/backend/app/api/endpoints/ai_chat.py b/backend/app/api/endpoints/ai_chat.py
index a1b8c648..7fb16748 100644
--- a/backend/app/api/endpoints/ai_chat.py
+++ b/backend/app/api/endpoints/ai_chat.py
@@ -417,7 +417,9 @@ async def import_tree(
 
 
 @router.delete("/sessions/{session_id}", status_code=204)
+@limiter.limit("10/minute")
 async def abandon_session(
+    request: Request,
     session_id: UUID,
     current_user: Annotated[User, Depends(get_current_active_user)],
     db: Annotated[AsyncSession, Depends(get_db)],
diff --git a/frontend/src/pages/AIChatBuilderPage.tsx b/frontend/src/pages/AIChatBuilderPage.tsx
index 913cce73..005fad1d 100644
--- a/frontend/src/pages/AIChatBuilderPage.tsx
+++ b/frontend/src/pages/AIChatBuilderPage.tsx
@@ -40,7 +40,7 @@ export function AIChatBuilderPage() {
     if (resumeId && !sessionId) {
       resumeSession(resumeId)
     } else if (!sessionId && status === 'idle') {
-      startSession(flowType as 'troubleshooting' | 'procedural')
+      startSession(flowType)
     }
   }, []) // eslint-disable-line react-hooks/exhaustive-deps
 
@@ -81,15 +81,15 @@ export function AIChatBuilderPage() {
     }
   }, [importToEditor, treeMetadata, flowType, navigate])
 
-  const handleReset = useCallback(() => {
-    abandonSession()
+  const handleReset = useCallback(async () => {
+    await abandonSession()
     // Clear session from URL
     setSearchParams((prev) => {
       const next = new URLSearchParams(prev)
       next.delete('session')
       return next
     }, { replace: true })
-    startSession(flowType as 'troubleshooting' | 'procedural')
+    startSession(flowType)
   }, [abandonSession, startSession, flowType, setSearchParams])
 
   // Show error toast
diff --git a/frontend/src/store/aiChatStore.ts b/frontend/src/store/aiChatStore.ts
index 60eaa5fa..987c8ff0 100644
--- a/frontend/src/store/aiChatStore.ts
+++ b/frontend/src/store/aiChatStore.ts
@@ -1,4 +1,5 @@
 import { create } from 'zustand'
+import { AxiosError } from 'axios'
 import { aiChatApi } from '@/api/aiChat'
 import type {
   ChatMessage,
@@ -61,6 +62,15 @@ const initialState = {
   error: null,
 }
 
+function extractErrorMessage(e: unknown, fallback: string): string {
+  if (e instanceof AxiosError && e.response?.data?.detail) {
+    const detail = e.response.data.detail
+    return typeof detail === 'string' ? detail : detail.message || fallback
+  }
+  if (e instanceof Error) return e.message
+  return fallback
+}
+
 export const useAIChatStore = create<AIChatState>((set, get) => ({
   ...initialState,
 
@@ -80,14 +90,13 @@ export const useAIChatStore = create<AIChatState>((set, get) => ({
         isResponding: false,
       })
     } catch (e: unknown) {
-      const message = e instanceof Error ? e.message : 'Failed to start session'
-      set({ error: message, isResponding: false, status: 'idle' })
+      set({ error: extractErrorMessage(e, 'Failed to start session'), isResponding: false, status: 'idle' })
     }
   },
 
   sendMessage: async (content) => {
-    const { sessionId, messages } = get()
-    if (!sessionId) return
+    const { sessionId, messages, isResponding } = get()
+    if (!sessionId || isResponding) return
 
     const userMessage: ChatMessage = {
       role: 'user',
@@ -117,14 +126,13 @@ export const useAIChatStore = create<AIChatState>((set, get) => ({
         isResponding: false,
       }))
     } catch (e: unknown) {
-      const message = e instanceof Error ? e.message : 'Failed to send message'
-      set({ error: message, isResponding: false })
+      set({ error: extractErrorMessage(e, 'Failed to send message'), isResponding: false })
     }
   },
 
   generateTree: async () => {
-    const { sessionId } = get()
-    if (!sessionId) return
+    const { sessionId, isGenerating } = get()
+    if (!sessionId || isGenerating) return
 
     set({ isGenerating: true, error: null })
 
@@ -138,8 +146,7 @@ export const useAIChatStore = create<AIChatState>((set, get) => ({
         isGenerating: false,
       })
     } catch (e: unknown) {
-      const message = e instanceof Error ? e.message : 'Failed to generate tree'
-      set({ error: message, isGenerating: false })
+      set({ error: extractErrorMessage(e, 'Failed to generate tree'), isGenerating: false })
     }
   },
 
@@ -181,8 +188,7 @@ export const useAIChatStore = create<AIChatState>((set, get) => ({
         isResponding: false,
       })
     } catch (e: unknown) {
-      const message = e instanceof Error ? e.message : 'Failed to resume session'
-      set({ error: message, isResponding: false })
+      set({ error: extractErrorMessage(e, 'Failed to resume session'), isResponding: false })
     }
   },