fix: address code review findings for AI chat builder
- 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>
This commit is contained in:
@@ -417,7 +417,9 @@ async def import_tree(
|
|||||||
|
|
||||||
|
|
||||||
@router.delete("/sessions/{session_id}", status_code=204)
|
@router.delete("/sessions/{session_id}", status_code=204)
|
||||||
|
@limiter.limit("10/minute")
|
||||||
async def abandon_session(
|
async def abandon_session(
|
||||||
|
request: Request,
|
||||||
session_id: UUID,
|
session_id: UUID,
|
||||||
current_user: Annotated[User, Depends(get_current_active_user)],
|
current_user: Annotated[User, Depends(get_current_active_user)],
|
||||||
db: Annotated[AsyncSession, Depends(get_db)],
|
db: Annotated[AsyncSession, Depends(get_db)],
|
||||||
|
|||||||
@@ -40,7 +40,7 @@ export function AIChatBuilderPage() {
|
|||||||
if (resumeId && !sessionId) {
|
if (resumeId && !sessionId) {
|
||||||
resumeSession(resumeId)
|
resumeSession(resumeId)
|
||||||
} else if (!sessionId && status === 'idle') {
|
} else if (!sessionId && status === 'idle') {
|
||||||
startSession(flowType as 'troubleshooting' | 'procedural')
|
startSession(flowType)
|
||||||
}
|
}
|
||||||
}, []) // eslint-disable-line react-hooks/exhaustive-deps
|
}, []) // eslint-disable-line react-hooks/exhaustive-deps
|
||||||
|
|
||||||
@@ -81,15 +81,15 @@ export function AIChatBuilderPage() {
|
|||||||
}
|
}
|
||||||
}, [importToEditor, treeMetadata, flowType, navigate])
|
}, [importToEditor, treeMetadata, flowType, navigate])
|
||||||
|
|
||||||
const handleReset = useCallback(() => {
|
const handleReset = useCallback(async () => {
|
||||||
abandonSession()
|
await abandonSession()
|
||||||
// Clear session from URL
|
// Clear session from URL
|
||||||
setSearchParams((prev) => {
|
setSearchParams((prev) => {
|
||||||
const next = new URLSearchParams(prev)
|
const next = new URLSearchParams(prev)
|
||||||
next.delete('session')
|
next.delete('session')
|
||||||
return next
|
return next
|
||||||
}, { replace: true })
|
}, { replace: true })
|
||||||
startSession(flowType as 'troubleshooting' | 'procedural')
|
startSession(flowType)
|
||||||
}, [abandonSession, startSession, flowType, setSearchParams])
|
}, [abandonSession, startSession, flowType, setSearchParams])
|
||||||
|
|
||||||
// Show error toast
|
// Show error toast
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { create } from 'zustand'
|
import { create } from 'zustand'
|
||||||
|
import { AxiosError } from 'axios'
|
||||||
import { aiChatApi } from '@/api/aiChat'
|
import { aiChatApi } from '@/api/aiChat'
|
||||||
import type {
|
import type {
|
||||||
ChatMessage,
|
ChatMessage,
|
||||||
@@ -61,6 +62,15 @@ const initialState = {
|
|||||||
error: null,
|
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) => ({
|
export const useAIChatStore = create<AIChatState>((set, get) => ({
|
||||||
...initialState,
|
...initialState,
|
||||||
|
|
||||||
@@ -80,14 +90,13 @@ export const useAIChatStore = create<AIChatState>((set, get) => ({
|
|||||||
isResponding: false,
|
isResponding: false,
|
||||||
})
|
})
|
||||||
} catch (e: unknown) {
|
} catch (e: unknown) {
|
||||||
const message = e instanceof Error ? e.message : 'Failed to start session'
|
set({ error: extractErrorMessage(e, 'Failed to start session'), isResponding: false, status: 'idle' })
|
||||||
set({ error: message, isResponding: false, status: 'idle' })
|
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
sendMessage: async (content) => {
|
sendMessage: async (content) => {
|
||||||
const { sessionId, messages } = get()
|
const { sessionId, messages, isResponding } = get()
|
||||||
if (!sessionId) return
|
if (!sessionId || isResponding) return
|
||||||
|
|
||||||
const userMessage: ChatMessage = {
|
const userMessage: ChatMessage = {
|
||||||
role: 'user',
|
role: 'user',
|
||||||
@@ -117,14 +126,13 @@ export const useAIChatStore = create<AIChatState>((set, get) => ({
|
|||||||
isResponding: false,
|
isResponding: false,
|
||||||
}))
|
}))
|
||||||
} catch (e: unknown) {
|
} catch (e: unknown) {
|
||||||
const message = e instanceof Error ? e.message : 'Failed to send message'
|
set({ error: extractErrorMessage(e, 'Failed to send message'), isResponding: false })
|
||||||
set({ error: message, isResponding: false })
|
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
generateTree: async () => {
|
generateTree: async () => {
|
||||||
const { sessionId } = get()
|
const { sessionId, isGenerating } = get()
|
||||||
if (!sessionId) return
|
if (!sessionId || isGenerating) return
|
||||||
|
|
||||||
set({ isGenerating: true, error: null })
|
set({ isGenerating: true, error: null })
|
||||||
|
|
||||||
@@ -138,8 +146,7 @@ export const useAIChatStore = create<AIChatState>((set, get) => ({
|
|||||||
isGenerating: false,
|
isGenerating: false,
|
||||||
})
|
})
|
||||||
} catch (e: unknown) {
|
} catch (e: unknown) {
|
||||||
const message = e instanceof Error ? e.message : 'Failed to generate tree'
|
set({ error: extractErrorMessage(e, 'Failed to generate tree'), isGenerating: false })
|
||||||
set({ error: message, isGenerating: false })
|
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
@@ -181,8 +188,7 @@ export const useAIChatStore = create<AIChatState>((set, get) => ({
|
|||||||
isResponding: false,
|
isResponding: false,
|
||||||
})
|
})
|
||||||
} catch (e: unknown) {
|
} catch (e: unknown) {
|
||||||
const message = e instanceof Error ? e.message : 'Failed to resume session'
|
set({ error: extractErrorMessage(e, 'Failed to resume session'), isResponding: false })
|
||||||
set({ error: message, isResponding: false })
|
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user