fix: Ask Agent indexes and attaches files before sending to chat

[itomek]

Summary

• Root cause: handleAskAgent in FileBrowser dispatched a gaia:send-prompt CustomEvent that nothing listened for (dead code), performing zero indexing or session attachment
• Fix: Rewrote handleAskAgent to index files, attach them to the active session, and deliver the prompt via the existing pendingPrompt Zustand mechanism
• Reactive consumption: Replaced imperative useEffect([sessionId]) in ChatView with reactive useEffect([pendingPrompt]) + getState() StrictMode guard so prompts are consumed whether ChatView is already mounted or not

Changes

• FileBrowser.tsx: Extract shared indexAndAttachFiles() helper, rewrite handleAskAgent, add try/catch/finally to handleIndexSelected, surface partial-failure warning
• ChatView.tsx: Reactive pendingPrompt useEffect with StrictMode guard and rAF cancellation on unmount
• tests/unit/chat/ui/test_ask_agent_workflow.py: 3 backend tests covering index→attach→chat pipeline (519 passing)

Test plan

• python -m pytest tests/unit/chat/ui/ --ignore=tests/unit/chat/ui/test_sse_confirmation.py — 519 passed
• npm run build — clean TypeScript compile
• python util/lint.py --all — all checks passed
• End-to-end MCP test with Qwen3.5-35B-A3B-GGUF: indexed employee_records.csv, attached to session, agent correctly returned CFO salary ($200,000) from document content

Closes #583

[itomek]

@claude review this PR

[github-actions]

Claude finished @itomek's task —— View job

---

Reviewing PR #725

• Read pr-diff.txt and changed files
• Review ChatView.tsx changes
• Review FileBrowser.tsx changes
• Review test file
• Provide feedback

---

Code Review Summary

This PR correctly fixes the "Ask Agent" functionality which was dispatching events that nothing listened to (dead code). The fix is well-structured and the approach is sound.

Positive Observations

1. Good code reuse - Extracted indexAndAttachFiles() helper to share logic between "Index Selected" and "Ask Agent" operations
2. Proper StrictMode handling - The useEffect in ChatView correctly uses getState() to prevent double-fire in React StrictMode
3. rAF cancellation guard - Added proper cleanup with cancelled flag to prevent unmount race conditions
4. Comprehensive error handling - Both paths handle errors gracefully with user-friendly messages
5. Good test coverage - Backend tests cover the core index→attach→chat pipeline

Issues Found

🟡 Important

1. FileBrowser.tsx:355-357 - The filename extraction logic has a subtle bug:

   const fileNames = result.docIds.length > 0
       ? result.supported.filter((_, i) => i < result.docIds.length).map(f => f.split(/[\\/]/).pop() || f)
       : result.supported.map(f => f.split(/[\\/]/).pop() || f);

   This assumes 1:1 correspondence between result.supported indices and result.docIds, but if indexing fails for a middle file, the indices won't align. For example, if files [A, B, C] are supported but B fails, docIds would be [A_id, C_id] but you'd take the first 2 entries from supported ([A, B]).

   Suggestion: Track which files succeeded during indexing, or simply use the supported file list (since partial failures are already surfaced via the warning message).

🟢 Minor

2. FileBrowser.tsx:336 - Early return in handleAskAgent without setIndexingFiles(new Set()) in finally block - but this is fine since finally always runs.

3. ChatView.tsx:282 - The pendingPrompt dependency is correct for reactive consumption, but consider adding a comment explaining why both the subscription and getState() are needed (subscription for reactivity, getState for StrictMode deduplication).

Suggestions for Future Improvements

1. Consider adding a frontend test for the Ask Agent → pendingPrompt → ChatView consumption flow
2. The indexAndAttachFiles helper could potentially be moved to a shared utility file if reused elsewhere

Verdict

The PR successfully fixes the Ask Agent workflow and follows good React patterns. The filename extraction bug (item 1) is worth addressing but doesn't block the fix - the worst case is showing the wrong filename in the prompt, which the LLM can still process correctly since the documents are properly attached.

Approved for merge with optional fix for the filename alignment issue.

---

[itomek]

@claude review this PR

[github-actions]

Claude finished @itomek's task —— View job

---

Reviewing PR #725

• Read pr-diff.txt and changed files
• Review ChatView.tsx changes
• Review FileBrowser.tsx changes
• Review test file
• Provide feedback

---

Code Review Summary

This PR correctly fixes the "Ask Agent" functionality which was dispatching events that nothing listened to (dead code). The fix is well-structured and the approach is sound.

Positive Observations

1. Good code reuse - Extracted indexAndAttachFiles() helper to share logic between "Index Selected" and "Ask Agent" operations
2. Proper StrictMode handling - The useEffect in ChatView correctly uses getState() to prevent double-fire in React StrictMode
3. rAF cancellation guard - Added proper cleanup with cancelled flag to prevent unmount race conditions (ChatView.tsx:277-281)
4. succeededFiles tracking - The previous review noted a filename alignment issue; this has been fixed by tracking succeededFiles explicitly rather than correlating indices
5. Comprehensive error handling - Both paths handle errors gracefully with user-friendly messages
6. Good test coverage - Backend tests cover the core index→attach→chat pipeline

Issues Found

🟢 Minor

1. FileBrowser.tsx:125-128 - The folder indexing logic could silently produce no docIds if the folder contains only unsupported files:

   if (newIds.length > 0) {
       newIds.forEach(id => docIds.push(id));
       succeededFiles.push(filepath);
   }

   Consider incrementing failed or logging when a folder is selected but yields no indexed documents, to give users clearer feedback about why a folder selection didn't work.

2. FileBrowser.tsx:323 - setIndexStatus(null) in the finally block clears the partial-failure warning message set at line 314. If partial failures occur, the user only sees the warning briefly before it's cleared. Consider using a timeout like handleIndexSelected does.

3. ChatView.tsx:282 - A clarifying comment would help explain the dual mechanism (subscription for reactivity, getState() for StrictMode deduplication), but this is optional.

Suggestions for Future Improvements

1. Consider adding a frontend test (Jest/Vitest) for the Ask Agent → pendingPrompt → ChatView consumption flow
2. The indexAndAttachFiles helper could potentially be moved to a shared utility file if reused elsewhere

Verdict

The PR successfully fixes the Ask Agent workflow and follows good React patterns. The minor issues noted above are non-blocking improvements.

Approved for merge ✅

---