src/main/presenter/knowledgePresenter/knowledgeTaskPresenter.ts (2)

12-20: Concurrent tracking via runningTasks looks good; consider small robustness tweaks

The switch from a single “current task” to runningTasks and maxConcurrency is consistent across status helpers and should give correct aggregate stats and activity checks.

Two minor follow-ups:

• maxConcurrency is accepted as-is (including 0/negative), which would stall processing. Consider clamping to >= 1 in the constructor or throwing on invalid values.
• The header comment still says “sequential execution”; it might be worth updating to reflect bounded parallelism to avoid confusion for future readers.

Also, CI is flagging Prettier; please run the formatter once you settle the implementation.

Also applies to: 61-64, 77-83, 89-91, 94-101, 103-109, 111-117

---

29-47: Termination semantics rely on cooperative tasks; document the contract

removeTasks now correctly:

• Drops matching tasks from the queue.
• Aborts and cleans up any matching entries in runningTasks via terminateTask.

destroy then calls removeTasks(() => true), clears the queue, aborts remaining controllers, and clears runningTasks, which is consistent.

One behavioral nuance: terminateTask removes a running task from runningTasks immediately after aborting its controller, so the concurrency limiter may start new work even if the aborted task’s run implementation doesn’t actually respect AbortSignal and keeps running. This is fine if KnowledgeChunkTask.run is expected to be cooperative w.r.t. signal, but it’s worth documenting that assumption so future callers don’t accidentally violate the contract.

Also applies to: 119-130, 132-139

src/main/presenter/knowledgePresenter/knowledgeStorePresenter.ts (2)

89-93: Be aware of changed failure semantics around vectorP.* operations

All these call sites now go through enqueueFileTask, so until its error-handling is adjusted:

• Failures in updateFile/insertFile during addFile will not reject addFile; callers may think a file was registered while it wasn’t.
• Failures in insertChunks, updateChunkStatus, insertVector, deleteFile, deleteChunksByFile, deleteVectorsByFile, and file status updates in onFileFinish/handleFileProcessingError will be logged but treated as success by their callers.

Once enqueueFileTask is updated to propagate errors (as suggested earlier), these sites will again behave like the previous direct await-based implementation, while still benefiting from per-file serialization.

Also applies to: 127-128, 140-143, 156-158, 213-221, 271-273, 299-300, 317-318, 334-335, 370-371

---

325-342: Deletion/re-add/destroy flows mostly align; consider a couple of lifecycle refinements

• deleteFile now cancels queued/running chunk tasks and deletes the file via the per-file queue, which is consistent with the new model.
• reAddFile cleans out chunks and vectors via the per-file queue before calling addFile, which is sensible.

Two optional improvements:

1. reAddFile task cancellation.
   If reAddFile can be called while there are still active tasks for that file, you may want to invoke this.taskP.cancelTasksByFile(fileId) here as well, mirroring deleteFile.

2. destroy/close and fileQueueMap.
   Both destroy and close cancel tasks and clear fileProgressMap but don’t touch fileQueueMap. Once you adjust enqueueFileTask to clean up entries, that may be sufficient, but if there’s a chance of in-flight queued operations at teardown time, explicitly clearing fileQueueMap (after ensuring you’re OK with abandoning pending DB work) would make lifecycle semantics clearer.

These are not correctness blockers but worth considering for long-running processes.

Also applies to: 365-373, 442-465

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/main/presenter/knowledgePresenter/knowledgeTaskPresenter.ts (3)

1-20: Align class documentation with new concurrent execution model

The class-level comment still says “sequential execution”, but with runningTasks and maxConcurrency you now allow up to maxConcurrency tasks to run in parallel. It would be clearer to update the doc to something like “bounded concurrent execution (default 16 tasks)” so callers understand the behavior and the meaning of maxConcurrency.

You might also consider mentioning the concurrency behavior in any higher-level presenter docs that reference this class, so the global scheduler’s semantics are explicit.

---

29-47: Cancellation now covers running tasks; consider semantics for queued task termination callbacks

Extending removeTasks to iterate runningTasks and call terminateTask for matching entries is a good improvement and ensures running tasks are actually aborted and removed.

One open design point: for tasks that are still in the queue (never started), you currently remove them and abort their controller, but you never invoke onTerminate. If callers expect onTerminate to fire whenever a task is canceled (regardless of whether it had started), you may want to invoke it when filtering queued tasks:

this.queue = this.queue.filter((task) => {
  if (filter(task)) {
    console.log(`[RAG TASK] Removing queued task: ${task.id}`)
    this.terminateTask(task.id)
+   task.onTerminate?.()
    return false
  }
  return true
})

If onTerminate is intended only for tasks that had actually begun execution, the current behavior is fine; just worth double‑checking the contract.

---

119-130: Destroy behavior thoroughly cleans up queue, controllers, and runningTasks

destroy() now:

• Cancels all queued and running tasks via removeTasks(() => true), which drives terminateTask and aborts controllers.
• Resets this.queue to [].
• Aborts any remaining controllers and clears both controllers and runningTasks.

This provides a robust cleanup point for the presenter. The extra controllers.forEach((c) => c.abort()) after removeTasks is slightly redundant in the current implementation (since terminateTask already aborts), but it’s harmless and defensive if new code ever adds controllers without going through removeTasks.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.297Z
Learning: Applies to src/main/presenter/**/*.ts : Organize core business logic into dedicated Presenter classes, with one presenter per functional domain

Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T05:28:20.500Z
Learning: Applies to src/main/**/*.ts : Use the Presenter pattern in the main process for UI coordination

Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.201Z
Learning: Implement separation of concerns where `src/main/presenter/llmProviderPresenter/index.ts` manages the Agent loop and conversation history, while Provider files handle LLM API interactions, Provider-specific request/response formatting, tool definition conversion, and native vs non-native tool call mechanisms

Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.201Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : In `src/main/presenter/llmProviderPresenter/index.ts` (`startStreamCompletion`), implement the Agent loop that manages the overall conversation flow, including multiple rounds of LLM calls and tool usage, maintaining `conversationMessages` history, calling `provider.coreStream()` on each iteration, and controlling the loop using `needContinueConversation` and `toolCallCount` (compared against `MAX_TOOL_CALLS`)

Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.201Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : In `src/main/presenter/llmProviderPresenter/index.ts`, listen for standardized events yielded by `coreStream` and handle them accordingly: buffer text content (`currentContent`), handle `tool_call_start/chunk/end` events by collecting tool details and calling `presenter.mcpPresenter.callTool`, send frontend events via `eventBus` with tool call status, format tool results for the next LLM call, and set `needContinueConversation = true`