fix: show insights after long-running generation (#1346)

* fix: keep insight polling active until completion

* fix: harden insight polling lifecycle

* fix: handle terminal insight edge cases

---------

Co-authored-by: Luis Novo <lfnovo@gmail.com>

frontend/src/components/sources/SourceDetailContent.test.tsx

@@ -1,10 +1,13 @@
-import { render, screen, waitFor } from '@testing-library/react'
+import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
 import { describe, it, expect, vi, beforeEach } from 'vitest'
 import { SourceDetailContent } from './SourceDetailContent'
 import { sourcesApi } from '@/lib/api/sources'
+import { insightsApi } from '@/lib/api/insights'
+import { transformationsApi } from '@/lib/api/transformations'
 import { QUERY_KEYS } from '@/lib/api/query-client'
 import { SourceDetailResponse } from '@/lib/types/api'
+import { toast } from 'sonner'
 
 // useTranslation is mocked globally in setup.ts (t returns the key string)
 
@@ -17,6 +20,8 @@ vi.mock('@/lib/api/sources', () => ({
 vi.mock('@/lib/api/insights', () => ({
   insightsApi: {
     listForSource: vi.fn().mockResolvedValue([]),
+    create: vi.fn(),
+    waitForCommand: vi.fn(),
   },
 }))
 
@@ -40,7 +45,41 @@ vi.mock('@/components/sources/NotebookAssociations', () => ({
   NotebookAssociations: () => null,
 }))
 
+vi.mock('@/components/ui/select', () => ({
+  Select: ({ children, onValueChange, ...props }: React.PropsWithChildren<{
+    onValueChange: (value: string) => void
+  }>) => (
+    <select {...props} aria-label="transformation" onChange={event => onValueChange(event.target.value)}>
+      {children}
+    </select>
+  ),
+  SelectTrigger: ({ children }: React.PropsWithChildren) => <>{children}</>,
+  SelectValue: () => null,
+  SelectContent: ({ children }: React.PropsWithChildren) => <>{children}</>,
+  SelectItem: ({ children, value }: React.PropsWithChildren<{ value: string }>) => (
+    <option value={value}>{children}</option>
+  ),
+}))
+
+vi.mock('@/components/ui/tabs', () => ({
+  Tabs: ({ children }: React.PropsWithChildren) => <div>{children}</div>,
+  TabsList: ({ children }: React.PropsWithChildren) => <div>{children}</div>,
+  TabsTrigger: ({ children }: React.PropsWithChildren) => <button>{children}</button>,
+  TabsContent: ({ children }: React.PropsWithChildren) => <div>{children}</div>,
+}))
+
+vi.mock('sonner', () => ({
+  toast: {
+    success: vi.fn(),
+    error: vi.fn(),
+  },
+}))
+
 const mockSourcesGet = vi.mocked(sourcesApi.get)
+const mockListInsights = vi.mocked(insightsApi.listForSource)
+const mockCreateInsight = vi.mocked(insightsApi.create)
+const mockWaitForCommand = vi.mocked(insightsApi.waitForCommand)
+const mockListTransformations = vi.mocked(transformationsApi.list)
 
 const notFoundError = Object.assign(new Error('Request failed with status code 404'), {
   isAxiosError: true,
@@ -52,18 +91,50 @@ const networkError = Object.assign(new Error('Network Error'), {
   response: undefined,
 })
 
-function renderContent(onClose?: () => void) {
+function renderContent(onClose?: () => void, sourceId = 'source:missing') {
   const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } })
   return render(
     <QueryClientProvider client={queryClient}>
-      <SourceDetailContent sourceId="source:missing" onClose={onClose} />
+      <SourceDetailContent sourceId={sourceId} onClose={onClose} />
     </QueryClientProvider>
   )
 }
 
+const loadedSource: SourceDetailResponse = {
+  id: 'source:loaded',
+  title: 'Loaded source',
+  asset: null,
+  embedded: false,
+  embedded_chunks: 0,
+  insights_count: 0,
+  created: '2026-01-01T00:00:00Z',
+  updated: '2026-01-01T00:00:00Z',
+  full_text: 'Source content',
+}
+
+async function startInsightGeneration() {
+  await screen.findByText('Loaded source')
+  fireEvent.change(screen.getByRole('combobox', { name: 'transformation' }), {
+    target: { value: 'transformation:summary' },
+  })
+  fireEvent.click(screen.getByRole('button', { name: 'common.create' }))
+}
+
 describe('SourceDetailContent', () => {
   beforeEach(() => {
     vi.clearAllMocks()
+    mockListInsights.mockResolvedValue([])
+    mockListTransformations.mockResolvedValue([{
+      id: 'transformation:summary',
+      title: 'Summary',
+      name: 'summary',
+      description: '',
+      prompt: 'Summarize',
+      apply_default: false,
+      model_id: null,
+      created: '2026-01-01T00:00:00Z',
+      updated: '2026-01-01T00:00:00Z',
+    }])
   })
 
   it('shows the shared not-found state when the source returns 404', async () => {
@@ -142,4 +213,83 @@ describe('SourceDetailContent', () => {
     screen.getByText('common.close').click()
     expect(onClose).toHaveBeenCalled()
   })
+
+  it('does not start polling when creation finishes after unmount', async () => {
+    mockSourcesGet.mockResolvedValue(loadedSource)
+    let resolveCreate!: (value: Awaited<ReturnType<typeof insightsApi.create>>) => void
+    mockCreateInsight.mockReturnValue(new Promise(resolve => {
+      resolveCreate = resolve
+    }))
+
+    const view = renderContent(undefined, loadedSource.id)
+    await startInsightGeneration()
+    await waitFor(() => expect(mockCreateInsight).toHaveBeenCalled())
+
+    view.unmount()
+    await act(async () => {
+      resolveCreate({
+        status: 'pending',
+        message: 'started',
+        source_id: loadedSource.id,
+        transformation_id: 'transformation:summary',
+        command_id: 'job-1',
+      })
+    })
+
+    expect(mockWaitForCommand).not.toHaveBeenCalled()
+  })
+
+  it('shows the backend error when insight generation fails', async () => {
+    mockSourcesGet.mockResolvedValue(loadedSource)
+    mockCreateInsight.mockResolvedValue({
+      status: 'pending',
+      message: 'started',
+      source_id: loadedSource.id,
+      transformation_id: 'transformation:summary',
+      command_id: 'job-1',
+    })
+    mockWaitForCommand.mockResolvedValue({
+      job_id: 'job-1',
+      status: 'failed',
+      error_message: 'Model ran out of memory',
+    })
+
+    renderContent(undefined, loadedSource.id)
+    await startInsightGeneration()
+
+    await waitFor(() => {
+      expect(toast.error).toHaveBeenCalledWith('Model ran out of memory')
+    })
+  })
+
+  it('does not apply terminal effects when unmounted during the final refresh', async () => {
+    mockSourcesGet.mockResolvedValue(loadedSource)
+    let resolveRefresh!: (value: Awaited<ReturnType<typeof insightsApi.listForSource>>) => void
+    mockListInsights
+      .mockResolvedValueOnce([])
+      .mockReturnValueOnce(new Promise(resolve => {
+        resolveRefresh = resolve
+      }))
+    mockCreateInsight.mockResolvedValue({
+      status: 'pending',
+      message: 'started',
+      source_id: loadedSource.id,
+      transformation_id: 'transformation:summary',
+      command_id: 'job-1',
+    })
+    mockWaitForCommand.mockResolvedValue({
+      job_id: 'job-1',
+      status: 'failed',
+      error_message: 'Stale failure',
+    })
+
+    const view = renderContent(undefined, loadedSource.id)
+    await startInsightGeneration()
+    await waitFor(() => expect(mockListInsights).toHaveBeenCalledTimes(2))
+
+    view.unmount()
+    await act(async () => resolveRefresh([]))
+
+    expect(toast.error).not.toHaveBeenCalledWith('Stale failure')
+  })
 })

frontend/src/components/sources/SourceDetailContent.tsx

@@ -1,6 +1,6 @@
 'use client'
 
-import { useState, useEffect, useCallback, useMemo } from 'react'
+import { useState, useEffect, useCallback, useMemo, useRef } from 'react'
 import { useQueryClient } from '@tanstack/react-query'
 import { MarkdownRenderer } from '@/components/ui/markdown-renderer'
 import { sourcesApi } from '@/lib/api/sources'
@@ -114,6 +114,7 @@ function SourceDetailContentInner({
   const [selectedInsight, setSelectedInsight] = useState<SourceInsightResponse | null>(null)
   const [insightToDelete, setInsightToDelete] = useState<string | null>(null)
   const [deletingInsight, setDeletingInsight] = useState(false)
+  const insightPollingRef = useRef<AbortController | null>(null)
 
   // A 404 means the source was deleted (e.g. a dangling chat/ask reference) —
   // handled by the shared "content no longer exists" state. The global query
@@ -157,17 +158,25 @@ function SourceDetailContentInner({
     }
   }, [fetchInsights, fetchTransformations, sourceId])
 
+  useEffect(() => () => insightPollingRef.current?.abort(), [])
+
   const createInsight = async () => {
     if (!selectedTransformation) {
       toast.error(t('sources.selectTransformation'))
       return
     }
 
+    insightPollingRef.current?.abort()
+    const controller = new AbortController()
+    insightPollingRef.current = controller
+
     try {
       setCreatingInsight(true)
       const response = await insightsApi.create(sourceId, {
         transformation_id: selectedTransformation
       })
+      if (controller.signal.aborted) return
+
       // Show toast for async operation
       toast.success(t('sources.insightGenerationStarted'))
       setSelectedTransformation('')
@@ -176,30 +185,50 @@ function SourceDetailContentInner({
       if (response.command_id) {
         // Poll in background (don't block UI)
         insightsApi.waitForCommand(response.command_id, {
-          maxAttempts: 120, // Up to 4 minutes (120 * 2s)
-          intervalMs: 2000
-        }).then(success => {
-          if (success) {
-            void fetchInsights()
-            // Invalidate sources queries so notebook page refreshes with updated insights_count
-            queryClient.invalidateQueries({ queryKey: ['sources'] })
+          intervalMs: 2000,
+          signal: controller.signal,
+        }).then(async status => {
+          if (controller.signal.aborted) return
+
+          await fetchInsights()
+          if (controller.signal.aborted) return
+          // Invalidate sources queries so notebook page refreshes with updated insights_count
+          queryClient.invalidateQueries({ queryKey: ['sources'] })
+          if (status?.status !== 'completed') {
+            toast.error(status?.error_message || t('common.error'))
           }
         }).catch(err => {
+          if (controller.signal.aborted) return
           console.error('Error waiting for insight command:', err)
+          toast.error(t('common.error'))
+        }).finally(() => {
+          if (insightPollingRef.current === controller) {
+            insightPollingRef.current = null
+          }
         })
       } else {
         // Fallback: refresh after delay if no command_id
         setTimeout(() => {
+          if (controller.signal.aborted) return
           void fetchInsights()
           // Also invalidate sources queries
           queryClient.invalidateQueries({ queryKey: ['sources'] })
+          if (insightPollingRef.current === controller) {
+            insightPollingRef.current = null
+          }
         }, 5000)
       }
     } catch (err) {
+      if (controller.signal.aborted) return
       console.error('Failed to create insight:', err)
       toast.error(t('common.error'))
+      if (insightPollingRef.current === controller) {
+        insightPollingRef.current = null
+      }
     } finally {
-      setCreatingInsight(false)
+      if (!controller.signal.aborted) {
+        setCreatingInsight(false)
+      }
     }
   }
 

frontend/src/lib/api/insights.test.ts

@@ -0,0 +1,91 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import apiClient from './client'
+import { insightsApi } from './insights'
+
+vi.mock('./client', () => ({
+  default: {
+    get: vi.fn(),
+  },
+}))
+
+const mockGet = vi.mocked(apiClient.get)
+
+describe('insightsApi.waitForCommand', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  afterEach(() => {
+    vi.restoreAllMocks()
+  })
+
+  it('keeps polling while a command remains pending', async () => {
+    mockGet.mockImplementation(async () => ({
+      data: {
+        job_id: 'job-1',
+        status: mockGet.mock.calls.length > 120 ? 'completed' : 'running',
+      },
+    }))
+
+    await expect(
+      insightsApi.waitForCommand('job-1', { intervalMs: 0 })
+    ).resolves.toMatchObject({ status: 'completed' })
+    expect(mockGet).toHaveBeenCalledTimes(121)
+  })
+
+  it('stops polling when aborted', async () => {
+    const controller = new AbortController()
+    mockGet.mockResolvedValue({ data: { job_id: 'job-1', status: 'running' } })
+
+    const polling = insightsApi.waitForCommand('job-1', {
+      intervalMs: 60_000,
+      signal: controller.signal,
+    })
+    await vi.waitFor(() => expect(mockGet).toHaveBeenCalledTimes(1))
+
+    controller.abort()
+
+    await expect(polling).resolves.toBeNull()
+    expect(mockGet).toHaveBeenCalledTimes(1)
+  })
+
+  it('stops polling when the job is unknown', async () => {
+    mockGet.mockResolvedValue({ data: { job_id: 'job-1', status: 'unknown' } })
+
+    await expect(
+      insightsApi.waitForCommand('job-1', { intervalMs: 0 })
+    ).resolves.toMatchObject({ status: 'unknown' })
+    expect(mockGet).toHaveBeenCalledTimes(1)
+  })
+
+  it('stops polling when the job reports an error', async () => {
+    const controller = new AbortController()
+    mockGet.mockResolvedValue({ data: { job_id: 'job-1', status: 'error' } })
+    const abort = setTimeout(() => controller.abort(), 20)
+
+    const status = await insightsApi.waitForCommand('job-1', {
+      intervalMs: 1,
+      signal: controller.signal,
+    })
+    clearTimeout(abort)
+
+    expect(status).toMatchObject({ status: 'error' })
+    expect(mockGet).toHaveBeenCalledTimes(1)
+  })
+
+  it('stops after three consecutive status errors', async () => {
+    const statusError = Object.assign(new Error('status unavailable'), {
+      config: { headers: { Authorization: 'Bearer secret' } },
+    })
+    const errorLog = vi.spyOn(console, 'error').mockImplementation(() => undefined)
+    mockGet.mockRejectedValue(statusError)
+
+    await expect(
+      insightsApi.waitForCommand('job-1', { intervalMs: 0 })
+    ).resolves.toBeNull()
+    expect(mockGet).toHaveBeenCalledTimes(3)
+    expect(errorLog).toHaveBeenNthCalledWith(1, 'Error checking command status')
+    expect(errorLog).toHaveBeenNthCalledWith(2, 'Error checking command status')
+    expect(errorLog).toHaveBeenNthCalledWith(3, 'Error checking command status')
+  })
+})

frontend/src/lib/api/insights.ts

@@ -52,44 +52,63 @@ export const insightsApi = {
     await apiClient.delete(`/insights/${insightId}`)
   },
 
-  getCommandStatus: async (commandId: string) => {
+  getCommandStatus: async (commandId: string, signal?: AbortSignal) => {
     const response = await apiClient.get<CommandJobStatusResponse>(
-      `/commands/jobs/${commandId}`
+      `/commands/jobs/${commandId}`,
+      { signal }
     )
     return response.data
   },
 
   /**
-   * Poll command status until completed or failed.
-   * Returns true if completed successfully, false if failed.
+   * Poll command status until it reaches a terminal state.
+   * Returns the terminal status, or null if polling is aborted or repeatedly errors.
    */
   waitForCommand: async (
     commandId: string,
-    options?: { maxAttempts?: number; intervalMs?: number }
-  ): Promise<boolean> => {
-    const maxAttempts = options?.maxAttempts ?? 60 // Default 60 attempts
+    options?: { intervalMs?: number; signal?: AbortSignal }
+  ): Promise<CommandJobStatusResponse | null> => {
     const intervalMs = options?.intervalMs ?? 2000 // Default 2 seconds
+    const signal = options?.signal
+    let consecutiveErrors = 0
 
-    for (let i = 0; i < maxAttempts; i++) {
+    while (!signal?.aborted) {
+      let status: CommandJobStatusResponse | undefined
       try {
-        const status = await insightsApi.getCommandStatus(commandId)
-        if (status.status === 'completed') {
-          return true
-        }
+        status = await insightsApi.getCommandStatus(commandId, signal)
+        consecutiveErrors = 0
+      } catch {
+        if (signal?.aborted) return null
+        console.error('Error checking command status')
+        consecutiveErrors += 1
+        if (consecutiveErrors >= 3) return null
+      }
+
+      if (status && ['completed', 'failed', 'canceled', 'error', 'unknown'].includes(status.status)) {
         if (status.status === 'failed' || status.status === 'canceled') {
           console.error('Command failed:', status.error_message)
-          return false
         }
-        // Still running, wait and retry
-        await new Promise(resolve => setTimeout(resolve, intervalMs))
-      } catch (error) {
-        console.error('Error checking command status:', error)
-        // Continue polling on error
-        await new Promise(resolve => setTimeout(resolve, intervalMs))
+        return status
       }
+
+      await new Promise<void>(resolve => {
+        if (signal?.aborted) {
+          resolve()
+          return
+        }
+
+        const onAbort = () => {
+          clearTimeout(timeout)
+          resolve()
+        }
+        const timeout = setTimeout(() => {
+          signal?.removeEventListener('abort', onAbort)
+          resolve()
+        }, intervalMs)
+        signal?.addEventListener('abort', onAbort, { once: true })
+      })
     }
-    // Timeout
-    console.warn('Command polling timed out')
-    return false
+
+    return null
   }
-}
+}