address CodeRabbit review findings in the AI agent

- recover from panics in the AI reply and FAQ-mining workers so one bad run can't crash the process
- guard image decode with a pixel-count cap to block image bombs
- hand off (not silently drop) when the confirmation reply fails to send
- give the model a generic tool-failure message instead of the raw error
- log Redis Expire and assistant-cache refresh failures instead of ignoring them
- drop chunk text from RAG debug logs
- refetch the assistant when the edit route's id changes
This commit is contained in:
Abhinav Raut
2026-07-21 01:05:26 +05:30
parent 2fced94268
commit c659e30eca
8 changed files with 68 additions and 12 deletions
@@ -141,7 +141,7 @@
</template>
<script setup>
import { computed, onMounted, ref } from 'vue'
import { computed, onMounted, ref, watch } from 'vue'
import { useRouter } from 'vue-router'
import api from '@/api'
import AssistantForm from '@/features/admin/ai/AssistantForm.vue'
@@ -288,7 +288,7 @@ const selectRange = (option) => {
fetchStats()
}
onMounted(async () => {
const loadAssistant = async () => {
if (!props.id) return
try {
isLoading.value = true
@@ -302,5 +302,18 @@ onMounted(async () => {
} finally {
isLoading.value = false
}
})
}
onMounted(loadAssistant)
// The router reuses this view across ai/assistants/:id/edit, so refetch when the id changes.
watch(
() => props.id,
() => {
previewMessage.value = ''
previewReply.value = ''
previewSources.value = []
loadAssistant()
}
)
</script>
+1 -1
View File
@@ -105,7 +105,7 @@ func (m *Manager) executeToolCall(ctx context.Context, registry map[string]Tool,
out, err := tool.Execute(ctx, tc.Function.Arguments)
if err != nil {
m.lo.Error("error executing tool", "tool", tc.Function.Name, "error", err)
return "error executing tool: " + err.Error()
return "the tool call failed"
}
m.lo.Debug("ai run tool result", "tool", tc.Function.Name, "result_len", len(out), "result", out)
return out
+1 -1
View File
@@ -107,7 +107,7 @@ func (m *Manager) Search(ctx context.Context, query string, k int) ([]models.Sea
}
m.lo.Debug("rag search", "query_len", len(query), "hits", len(results))
for i, r := range results {
m.lo.Debug("rag fetched chunk", "rank", i+1, "score", r.Score, "source_type", r.SourceType, "source_id", r.SourceID, "chunk_len", len(r.ChunkText), "chunk_text", r.ChunkText)
m.lo.Debug("rag fetched chunk", "rank", i+1, "score", r.Score, "source_type", r.SourceType, "source_id", r.SourceID, "chunk_len", len(r.ChunkText))
}
return results, nil
}
+9 -3
View File
@@ -285,7 +285,9 @@ func (m *Manager) CreateAssistant(a models.Assistant) (models.Assistant, error)
m.lo.Error("error committing assistant", "error", err)
return models.Assistant{}, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil)
}
m.refreshAssistantUserIDs()
if err := m.refreshAssistantUserIDs(); err != nil {
m.lo.Error("error refreshing assistant user ids cache", "error", err)
}
return m.GetAssistant(id)
}
@@ -327,7 +329,9 @@ func (m *Manager) UpdateAssistant(id int, a models.Assistant) (models.Assistant,
m.lo.Error("error committing assistant update", "error", err)
return models.Assistant{}, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil)
}
m.refreshAssistantUserIDs()
if err := m.refreshAssistantUserIDs(); err != nil {
m.lo.Error("error refreshing assistant user ids cache", "error", err)
}
return m.GetAssistant(id)
}
@@ -358,7 +362,9 @@ func (m *Manager) DeleteAssistant(id int) (int, error) {
m.lo.Error("error committing assistant delete", "error", err)
return 0, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil)
}
m.refreshAssistantUserIDs()
if err := m.refreshAssistantUserIDs(); err != nil {
m.lo.Error("error refreshing assistant user ids cache", "error", err)
}
return a.UserID, nil
}
+10 -1
View File
@@ -132,12 +132,21 @@ func (m *Manager) miningWorker(ctx context.Context) {
case <-ctx.Done():
return
case convID := <-m.miningQueue:
m.mine(ctx, convID)
m.mineWithRecover(ctx, convID)
m.markMiningDone(convID)
}
}
}
func (m *Manager) mineWithRecover(ctx context.Context, convID int) {
defer func() {
if r := recover(); r != nil {
m.lo.Error("recovered from panic in ai agent mining worker", "conversation_id", convID, "panic", r)
}
}()
m.mine(ctx, convID)
}
func (m *Manager) enqueueMining(convID int) {
m.miningMu.Lock()
if m.miningInflight[convID] {
+3 -1
View File
@@ -88,7 +88,9 @@ func (m *Manager) incrOTPSends(convUUID string) (bool, error) {
return false, err
}
if n == 1 {
m.redis.Expire(ctx, key, otpVerifiedTTL)
if err := m.redis.Expire(ctx, key, otpVerifiedTTL).Err(); err != nil {
m.lo.Error("error setting ttl on otp sends key", "conversation_uuid", convUUID, "error", err)
}
}
return n > otpMaxSends, nil
}
+16 -2
View File
@@ -83,12 +83,23 @@ func (m *Manager) worker(ctx context.Context) {
case <-ctx.Done():
return
case convID := <-m.queue:
m.handle(ctx, convID)
m.handleWithRecover(ctx, convID)
m.markDone(convID)
}
}
}
// handleWithRecover runs handle and recovers from panics so a single bad run (LLM chain or tool
// execution) can't crash the whole process and take down every channel.
func (m *Manager) handleWithRecover(ctx context.Context, convID int) {
defer func() {
if r := recover(); r != nil {
m.lo.Error("recovered from panic in ai agent worker", "conversation_id", convID, "panic", r)
}
}()
m.handle(ctx, convID)
}
// HandleConversationEvent enqueues a response when the assignee is an AI assistant.
func (m *Manager) HandleConversationEvent(conversationID, assigneeUserID int) {
if conversationID == 0 || assigneeUserID == 0 {
@@ -280,7 +291,10 @@ func (m *Manager) handle(ctx context.Context, convID int) {
}
}
if confirm != "" {
m.postReply(conv, assistant, confirm, map[string]any{"is_confirmation": true})
if err := m.postReply(conv, assistant, confirm, map[string]any{"is_confirmation": true}); err != nil {
m.handoff(conv, assistant, m.i18n.T("ai.agent.handoffError"))
return
}
}
if outcome.resolved && (answer != "" || turns > 0) {
m.resolve(conv, assistant)
+12
View File
@@ -5,6 +5,8 @@ package image
import (
"bytes"
"encoding/base64"
"fmt"
"image"
"io"
"github.com/disintegration/imaging"
@@ -15,6 +17,9 @@ const (
// llmMaxDim caps an image's longest edge before it is sent to a vision model.
llmMaxDim = 1568
llmJPEGQuality = 85
// maxDecodePixels bounds width*height read from the header before decoding, blocking image bombs
// that declare huge dimensions in a small file.
maxDecodePixels = 100_000_000
)
var (
@@ -78,6 +83,13 @@ func CreateThumb(thumbPxSize int, r io.Reader) (*bytes.Reader, error) {
// EncodeForLLM decodes an image, downscales its longest edge to at most llmMaxDim, re-encodes it as
// JPEG, and returns the base64 payload plus media type for a vision model request.
func EncodeForLLM(content []byte) (data string, mediaType string, err error) {
cfg, _, err := image.DecodeConfig(bytes.NewReader(content))
if err != nil {
return "", "", err
}
if int64(cfg.Width)*int64(cfg.Height) > maxDecodePixels {
return "", "", fmt.Errorf("image dimensions %dx%d exceed decode limit", cfg.Width, cfg.Height)
}
img, err := imaging.Decode(bytes.NewReader(content))
if err != nil {
return "", "", err