Conversation
将 20 个平铺文件按领域分组到子目录: - core/: Agent 引擎 (brain, engine, state, instructions, react_agent, tools) - rag/: 检索增强 (retrieval, ingestion) - memory/: 记忆系统 (extraction, retrieval, notebook, file_storage) - research/: 深度研究 (deep_research, evaluation, reflection) - writing/: 写作辅助 (composer, ghost_text, scene_detector) - kg/: 知识图谱 (knowledge_graph) 每个子目录 __init__.py 导出公共 API，全部外部 import 已更新 Made-with: Cursor
- providers/llm.py 新增 get_client() 和 get_model() 统一入口 - ai/router.py 5 处 AsyncOpenAI(...) 替换为 get_client() - workers/tasks.py 3 处 AsyncOpenAI(...) 替换为 get_client() - 保留 config/setup test-llm 端点的直接使用（测试自定义凭证） Made-with: Cursor
- 新增 services/conversation_service.py (class-based, DI db + user_id) - conversation/router.py 从 473 行瘦身至 110 行（薄路由） - 所有 DB 操作、Agent 编排、后台任务调度移入 Service - 参考 LobeHub AiChatService 模式 Made-with: Cursor
- 新增 services/source_service.py，提取所有上传/导入/下载/删除业务逻辑 - source/router.py 从 ~440 行瘦身至 147 行 - 参考 LobeHub class-based service DI 模式 Made-with: Cursor
原 733 行 God File 拆分为: - routers/suggestions.py: AI 建议、上下文问候、来源建议 - routers/research.py: 深度研究 SSE 流 - routers/writing.py: 文本润色、写作上下文 - routers/knowledge.py: 跨笔记本知识发现 - routers/insights.py: 主动洞察 CRUD ai/router.py 仅保留 20 行聚合路由 Made-with: Cursor
将 ai/memory/config/knowledge_graph/knowledge/feedback/auth/setup/skill 9 个 domain 的 inline Pydantic schemas 提取到独立 schemas.py router 仅保留 HTTP 逻辑，schema 定义集中管理 Made-with: Cursor
- start.sh 新增 _check_port() 函数，dev/docker 模式启动前检测 8000/3000 端口占用 - _tcp_ready 从 /dev/tcp 改为 nc -z，修复 macOS 兼容性 - venv 创建移除 pyenv 硬依赖，fallback 到 python3 - lyra init 生产模式日志修正（不再误报 api/.env 已生成） - handleUnauthorized() 增加 cookie 清除逻辑 Made-with: Cursor
- 输入框：圆角胶囊、箭头发送按钮、浅蓝 hover 工具栏 - 工具栏：+ 号弹出菜单（添加文件、深度研究、思考模式），激活项以 pill 展示 - 深度研究 pill：默认图标、hover 显示取消 X，模式下拉改为「简要/全面」，取消按钮 hover 背景缩小 - 思考模式：仅在选择 thinking 模型时显示，文案「思考模式」/THINKING，灯泡图标，hover 同位置变 X、浅蓝背景 - 消息气泡：Mermaid 流程图渲染与放大、表格/代码块/标题层级、Gemini 风格思考过程折叠 - 后端：深度研究任务关联会话、流式错误处理、日志与 Celery 配置、mind_map enum 修正 Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces a deep-research background task system with streaming TaskBuffer and ResearchTask persistence; reorganizes agent packages into core/rag/research/memory/writing with many shimmed re-exports; centralizes schemas/services, adds new AI sub-routers, logging, migrations, frontend deep-research UI/store, and related tests and UX updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Web as Web Client
participant API as FastAPI API
participant DB as PostgreSQL
participant LLM as LLM Provider
User->>Web: POST /ai/deep-research (query, mode, notebook_id)
Web->>API: createDeepResearch -> {task_id, conversation_id}
API->>DB: Persist ResearchTask (status=running) and Conversation/Message
API->>API: spawn run_research_task(task_id)
API->>LLM: plan / research / synthesize steps
LLM-->>API: plan/learning/token/deliverable events
API->>API: TaskBuffer.push(event)
loop streaming events
Web->>API: GET /ai/deep-research/{task_id}/events (SSE)
API->>Web: stream buffered events to client
end
API->>DB: Update ResearchTask (report, deliverable, timeline, status, completed_at)
sequenceDiagram
participant User
participant Web as Web Client
participant API as FastAPI API
participant DB as PostgreSQL
participant RAG as RAG Retrieval
participant LLM as LLM Provider
User->>Web: POST /conversations/{id}/messages (content, attachments)
Web->>API: ConversationService.stream_agent(...)
API->>DB: save user Message
API->>DB: load conversation history and notebook summary
API->>API: build_memory_context (memory service)
API->>RAG: retrieve_chunks(query, notebook_id)
RAG-->>API: chunks
API->>LLM: run_agent (ReAct with tools + retrieval)
loop streaming
LLM-->>API: tokens / tool calls / thoughts / reasoning
API->>Web: SSE events (token, citations, reasoning, done)
alt tool call
API->>API: execute_tool -> result
API->>LLM: tool result fed back
end
end
API->>DB: persist assistant Message (content, citations, reasoning)
API->>API: dispatch background tasks (memory extraction, reflection...)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
web/src/components/chat-input/chat-input.tsx (1)
142-149:⚠️ Potential issue | 🟡 MinorDisable the action button when streaming without
onCancel.Line 199 keeps the button enabled whenever
streamingis true, but Line 144 can no-op ifonCancelis missing. That leaves an active control that does nothing.💡 Suggested fix
- const canSend = !disabled && !streaming && !!value.trim(); + const canSend = !disabled && !streaming && !!value.trim(); + const canCancel = streaming && !!onCancel; + const isActionDisabled = streaming ? !canCancel : !canSend; ... - disabled={!streaming && !canSend} + disabled={isActionDisabled} className={cn( "flex items-center justify-center rounded-full transition-all", isCompact ? "h-8 w-8" : "h-9 w-9", streaming - ? "bg-foreground text-background hover:bg-foreground/80" + ? canCancel + ? "bg-foreground text-background hover:bg-foreground/80" + : "cursor-not-allowed bg-muted/60 text-muted-foreground/30" : canSend ? "bg-foreground text-background hover:bg-foreground/80 active:scale-95" : "cursor-not-allowed bg-muted/60 text-muted-foreground/30", )}Also applies to: 199-207
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/chat-input/chat-input.tsx` around lines 142 - 149, The action button remains enabled while streaming even if no onCancel is provided, causing clicks to no-op; update the button's disabled logic to prevent interaction when streaming and onCancel is undefined (e.g., set disabled={streaming && !onCancel} or compute a canInteract = !streaming || !!onCancel and use that) and ensure handleSendClick remains unchanged (it already calls onCancel?.() and onSubmit(q)). Target the component around handleSendClick and the button render where streaming/value/onCancel/onSubmit are used.api/app/domains/knowledge_graph/router.py (2)
116-130:⚠️ Potential issue | 🔴 CriticalGate notebook rebuilds by ownership before enqueueing work.
_current_useris unused here, so any authenticated caller who knows a notebook UUID can trigger a rebuild against another user's notebook. Please verify ownership before the task is queued or the inline fallback runs.🔐 Suggested guard
async def rebuild_graph( notebook_id: UUID, _current_user: CurrentUser, db: DbDep, ): """Trigger a full rebuild of the knowledge graph for a notebook.""" + from app.models import Notebook + + owned_notebook = await db.execute( + select(Notebook.id).where( + Notebook.id == notebook_id, + Notebook.user_id == _current_user.id, + Notebook.status == "active", + ) + ) + if owned_notebook.scalar_one_or_none() is None: + raise NotFoundError("笔记本不存在") + try: from app.workers.tasks import rebuild_knowledge_graph_task🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/domains/knowledge_graph/router.py` around lines 116 - 130, The rebuild_graph endpoint currently ignores _current_user and allows any authenticated caller to enqueue or run a rebuild; add an ownership check before queuing or running the fallback: load the notebook record (by notebook_id) from the DB using the existing db dependency, compare its owner/user id to _current_user.id (or appropriate identity field on CurrentUser), and if they don't match return an unauthorized/forbidden response; perform this check before calling rebuild_knowledge_graph_task.delay(...) and again before calling the inline rebuild_notebook_graph fallback so both paths are gated by ownership (refer to the rebuild_graph function, the _current_user param, notebook_id, db, and rebuild_knowledge_graph_task/rebuild_notebook_graph symbols).
122-130:⚠️ Potential issue | 🟠 MajorDon't run the rebuild synchronously in the task-dispatch fallback.
Because this catches every exception around import/dispatch, a broker outage or wiring bug silently turns these background endpoints into long-running request handlers.
rebuild-allalso still returnsstatus="started"after doing the work inline, which makes the API contract misleading.Also applies to: 176-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/domains/knowledge_graph/router.py` around lines 122 - 130, Replace the broad except block that falls back to running rebuild synchronously: do not call rebuild_notebook_graph inline. Instead catch the dispatch/import error as e (avoid bare except Exception), log the error, and return a failure response (e.g., RebuildStatusOut(status="failed", detail=str(e)) or an appropriate error message) so the API doesn't claim "started" when work ran inline; apply the same change for the other fallback at the second occurrence (references: rebuild_knowledge_graph_task.delay, rebuild_notebook_graph, RebuildStatusOut).api/app/agents/memory/file_storage.py (1)
185-226:⚠️ Potential issue | 🟠 MajorThe default Chinese
MEMORY.mdsections all upsert under the same key.
_parse_memory_doc_sections()only preserves[a-z0-9_], so headings like关于我/技术背景collapse tofile_. Insync_memory_doc_to_db(), each_upsert_memory()then overwrites the previous section and only the last block survives in memory.🛠️ One simple fallback
slug = re.sub(r"[^a-z0-9_]", "_", heading.lower()).strip("_") if not slug: slug = f"section_{i // 2}" key = f"file_{slug}"Also applies to: 229-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/agents/memory/file_storage.py` around lines 185 - 226, _pars e_memory_doc_sections currently strips all non a-z0-9_ characters which makes headings like "关于我" collapse to an empty slug so every item becomes the same key and later _upsert_memory overwrites previous entries; fix by producing a stable fallback slug when the sanitized heading is empty (e.g., compute slug = re.sub(...).strip("_"); if not slug: slug = f"section_{i//2}" or include the loop index or a short hash) and then build the key as f"file_{slug}"; apply the same fallback logic in the other parsing block referenced around lines 229-272 and ensure sync_memory_doc_to_db/_upsert_memory receive unique keys per section.api/app/workers/tasks.py (1)
210-227:⚠️ Potential issue | 🟠 Major
generate_notebook_summarystill hard-codes the model.This call site now uses the centralized client, but it still pins
"gpt-4o-mini". That bypasses runtime config and can break as soon as the selected provider/model changes.Suggested change
- from app.providers.llm import get_client + from app.providers.llm import get_client, get_model client = get_client() @@ - model="gpt-4o-mini", + model=get_model() or "gpt-4o-mini",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/workers/tasks.py` around lines 210 - 227, The call in generate_notebook_summary hardcodes "gpt-4o-mini"; update it to use the configured runtime model instead—locate the client returned by get_client() and replace the literal model string in the client.chat.completions.create call with the provider/model value from your runtime config or the client object (e.g., use client.default_model or settings.get("OPENAI_MODEL") / app config used elsewhere), so the selected provider/model is respected at runtime; ensure the symbol names referenced are generate_notebook_summary, get_client, and client.chat.completions.create.
🟡 Minor comments (15)
web/src/components/settings/settings-primitives.tsx-82-84 (1)
82-84:⚠️ Potential issue | 🟡 MinorHardcoded UI strings should use translation system.
The component uses
useTranslationsfor other text but has hardcoded strings:
- Lines 83, 107:
"Thinking"(English)- Line 132:
"搜索模型..."(Chinese)- Line 151:
"无匹配模型"(Chinese)These should use the translation system for consistency and proper i18n support.
🌐 Proposed fix to use translations
Add translation keys and update the code:
- <span className="flex-shrink-0 rounded-md bg-violet-500/15 px-1.5 py-0.5 text-[10px] font-medium text-violet-400"> - Thinking - </span> + <span className="flex-shrink-0 rounded-md bg-violet-500/15 px-1.5 py-0.5 text-[10px] font-medium text-violet-400"> + {tc("thinking")} + </span>- placeholder="搜索模型..." + placeholder={tc("searchModel")}- <div className="px-3 py-2 text-xs text-muted-foreground">无匹配模型</div> + <div className="px-3 py-2 text-xs text-muted-foreground">{tc("noMatchingModel")}</div>Also applies to: 106-108, 132-132, 151-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/settings/settings-primitives.tsx` around lines 82 - 84, The three hardcoded UI strings in settings-primitives.tsx ("Thinking" in the status badge, "搜索模型..." placeholder, and "无匹配模型" empty-state text) must use the existing translation hook instead of literal text: import/use the useTranslations instance already used in this file (t) and replace those literals with t('settings.thinking'), t('settings.searchPlaceholder') and t('settings.noMatchingModels') (or similar keys), then add those keys and translations to your i18n resource files; update the span, input placeholder, and empty-state render in the component to call t(...) so all UI text is localized.web/src/components/settings/settings-primitives.tsx-46-51 (1)
46-51:⚠️ Potential issue | 🟡 MinorMissing cleanup for
setTimeoutcan cause stale focus.If the dropdown closes before the 50ms timeout fires, the focus call will execute on a closed dropdown, potentially causing unexpected focus behavior.
🛡️ Proposed fix to add timeout cleanup
useEffect(() => { if (open) { setSearch(""); - setTimeout(() => inputRef.current?.focus(), 50); + const timerId = setTimeout(() => inputRef.current?.focus(), 50); + return () => clearTimeout(timerId); } }, [open]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/settings/settings-primitives.tsx` around lines 46 - 51, The effect that focuses the input after a 50ms setTimeout (inside useEffect) can run after the dropdown closes; capture the timer id when calling setTimeout and return a cleanup function from useEffect that calls clearTimeout for that id so the pending focus call is cancelled when open changes or the component unmounts; update the effect that uses setSearch, setTimeout and inputRef.current?.focus() to store the timeout handle and clear it in the cleanup.api/app/services/source_service.py-298-301 (1)
298-301:⚠️ Potential issue | 🟡 MinorPreserve exception chain when re-raising.
When catching
FileNotFoundErrorand raisingNotFoundError, use exception chaining to preserve the original traceback for debugging.🔧 Proposed fix
except FileNotFoundError: - raise NotFoundError("文件未找到") + raise NotFoundError("文件未找到") from NoneUsing
from Noneexplicitly suppresses the chain if you don't want the original exception shown, or usefrom errto preserve it:except FileNotFoundError as err: raise NotFoundError("文件未找到") from err🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/services/source_service.py` around lines 298 - 301, The except block that catches FileNotFoundError around the call to get_storage().download should preserve the original exception chain when re-raising NotFoundError; update the handler in the source_service.py code that wraps get_storage().download so it catches FileNotFoundError as a variable (e.g., err) and re-raises NotFoundError using exception chaining (raise NotFoundError("文件未找到") from err) to keep the original traceback available for debugging while still returning the higher-level NotFoundError.scripts/start.sh-44-55 (1)
44-55:⚠️ Potential issue | 🟡 Minor
_check_portonly handles one PID and may leave the port occupied.Line 44 takes only the first listener (
head -1). If multiple listeners exist, startup can still fail later with the same port conflict.Suggested patch
_check_port() { local port=$1 name=$2 - if lsof -i ":$port" -sTCP:LISTEN -t &>/dev/null; then - local pid - pid=$(lsof -i ":$port" -sTCP:LISTEN -t | head -1) + if lsof -i ":$port" -sTCP:LISTEN -t &>/dev/null; then + local pids pid + pids=$(lsof -i ":$port" -sTCP:LISTEN -t | sort -u) + pid=$(echo "$pids" | head -1) local pname pname=$(ps -p "$pid" -o comm= 2>/dev/null || echo "unknown") warn "$name 端口 $port 被占用 (PID $pid - $pname)" read -rp " 是否终止该进程？[y/N] " ans if [[ "$ans" =~ ^[Yy]$ ]]; then - kill -9 "$pid" && log "已终止 PID $pid" + while read -r pid; do + kill -9 "$pid" && log "已终止 PID $pid" + done <<< "$pids" sleep 1 + lsof -i ":$port" -sTCP:LISTEN -t &>/dev/null && error "端口 $port 仍被占用，无法启动 $name" else error "端口 $port 被占用，无法启动 $name" fi fi }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/start.sh` around lines 44 - 55, The _check_port logic only inspects/kills the first PID from lsof (head -1), leaving other listeners and causing lingering port conflicts; change the implementation in _check_port to collect all listener PIDs for the port (use lsof -t or equivalent) and iterate over each PID, resolve its process name via ps -p, prompt once or per-PID as desired, and attempt to kill each returned PID (with logging on success/failure) so the port is fully cleared before proceeding; update the warn/log/error calls in the same function to reflect per-PID results.web/src/app/(workspace)/app/notebooks/[id]/page.tsx-19-19 (1)
19-19:⚠️ Potential issue | 🟡 MinorAvoid duplicate border layers in dark mode.
NotebookWorkspacealready applies the same border styling, so this wrapper can create a doubled border visual.♻️ Proposed fix
- <div className="flex h-full flex-col dark:border border-border/40"> + <div className="flex h-full flex-col">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/`(workspace)/app/notebooks/[id]/page.tsx at line 19, The wrapper div with className "flex h-full flex-col dark:border border-border/40" is adding the same border that NotebookWorkspace already applies, causing a doubled border in dark mode; remove the border classes ("dark:border" and "border-border/40") from that div (keep "flex h-full flex-col") so only NotebookWorkspace controls the border styling and visual duplication is resolved.docs-site/en/getting-started.md-99-99 (1)
99-99:⚠️ Potential issue | 🟡 MinorDo not list
DEBUGas a minimum required variable.This variable is optional and development-only; current wording in the “Minimum Required Environment Variables” section is misleading.
📝 Proposed doc fix
-| `DEBUG` | Set `true` to skip authentication | +| `DEBUG` | Optional (local development only): set `true` to skip authentication |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs-site/en/getting-started.md` at line 99, Remove `DEBUG` from the "Minimum Required Environment Variables" list and update the docs so that `DEBUG` is documented only in the optional/development settings section (e.g., an "Optional / Development Environment Variables" or "Development-only" note). Locate the occurrence of the `DEBUG` entry in the "Minimum Required Environment Variables" section and delete or move it, then add a brief sentence in the development section clarifying that `DEBUG` is optional and intended for local/dev use only (set to true to skip authentication).api/app/skills/builtin/mind_map.py-45-47 (1)
45-47:⚠️ Potential issue | 🟡 MinorInconsistent default type in
depthargument handling.The schema defines
depthas a string enum["2", "3"], but line 59 uses an integer default:args.get("depth", 2). If the argument is missing, it defaults to2(int), not"2"(string). Whileint()handles both, the inconsistency may cause confusion.🔧 Proposed fix for consistency
- depth = int(args.get("depth", 2)) + depth = int(args.get("depth", "2"))Also applies to: 59-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/skills/builtin/mind_map.py` around lines 45 - 47, The depth argument's schema uses string enum ["2","3"] but the code calls args.get("depth", 2) returning an int default; update the handler so the default matches the schema by using args.get("depth", "2") and then coerce with int(args.get("depth", "2")) where depth is used (or alternatively change the schema to use integers); specifically adjust the code around the depth retrieval (the args.get("depth", ...) usage) to return a string default "2" and then convert to int consistently.web/src/app/setup/page.tsx-449-450 (1)
449-450:⚠️ Potential issue | 🟡 MinorLocalize the “Thinking” badge label.
Line 450hardcodes English text in an otherwise localized flow. Please move this to i18n keys for language consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/setup/page.tsx` around lines 449 - 450, Hardcoded "Thinking" badge text needs to be replaced with a localized string: update the span that currently renders "Thinking" to use the app's i18n lookup (e.g., call the translation function/hook used across the codebase such as t('status.thinking') or equivalent) and add the new key "status.thinking" to your locale files for all languages; ensure the component (page.tsx) imports/uses the translation hook (useTranslation or the project's helper) if not already present and keep the existing className and markup intact.web/src/store/use-deep-research-store.ts-225-231 (1)
225-231:⚠️ Potential issue | 🟡 MinorMissing error handling for
createDeepResearchfailure.If
createDeepResearchthrows, the store is left in an inconsistent state withisActive: truebut notaskId. Consider wrapping the API call and subscription in a try-catch to reset state on failure.Proposed fix
async start(query, notebookId, mode) { _abortController?.abort(); set({ query, notebookId, mode, progress: makeInitialProgress(mode), reportTokens: "", isActive: true, eventIndex: 0, taskId: null, conversationId: null, }); + try { const { taskId, conversationId } = await createDeepResearch(query, { notebookId, mode, }); set({ taskId, conversationId }); subscribeTo(taskId, get, set); + } catch (err) { + set({ + isActive: false, + progress: null, + taskId: null, + conversationId: null, + }); + throw err; // Re-throw so caller can handle + } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/store/use-deep-research-store.ts` around lines 225 - 231, Wrap the createDeepResearch call and subsequent subscribeTo invocation in a try-catch so failures don’t leave the store inconsistent; call createDeepResearch(...) and only on success call set({ taskId, conversationId }) and subscribeTo(taskId, get, set), and in the catch reset the store (e.g. set isActive:false and clear taskId/conversationId) and surface/log the error. Ensure subscribeTo is never invoked if createDeepResearch throws and that isActive is only set true after a successful createDeepResearch/subscribeTo sequence.web/src/features/notebook/notebook-card.tsx-139-150 (1)
139-150:⚠️ Potential issue | 🟡 MinorPotential double
router.refresh()call.In
handleSave(line 146),router.refresh()is called afteronSaved(). However, theonSavedcallback passed fromNotebookCard(line 338) also callsrouter.refresh(). This results in the router being refreshed twice, which is unnecessary and could cause performance issues.🔧 Proposed fix: Remove duplicate refresh
Either remove the refresh from
handleSave:async function handleSave() { const title = name.trim(); if (!title || loading) return; setLoading(true); try { await updateNotebook(notebook.id, { title, cover_emoji: iconId }); onSaved(); - router.refresh(); } finally { setLoading(false); } }Or simplify the
onSavedcallback:<EditNotebookDialog open={editOpen} notebook={notebook} onClose={() => setEditOpen(false)} - onSaved={() => { setEditOpen(false); router.refresh(); }} + onSaved={() => setEditOpen(false)} />Also applies to: 334-339
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/features/notebook/notebook-card.tsx` around lines 139 - 150, handleSave currently calls router.refresh() and the onSaved callback passed into NotebookCard also calls router.refresh(), causing a duplicate refresh; remove one of them — either delete the router.refresh() call inside handleSave (function handleSave) after onSaved(), or change the onSaved callback provided by NotebookCard to not call router.refresh() so only one refresh remains; update the code where onSaved is defined/used to ensure a single refresh happens and keep setLoading/reset logic intact.web/src/features/chat/chat-view.tsx-598-627 (1)
598-627:⚠️ Potential issue | 🟡 MinorPotential duplicate user message on focus return.
When
drFocusRequestedtriggers anddrState.queryexists, a new synthetic user message is created. If the backend already saved the user message during DR initiation (as mentioned in the completion watcher), this could result in a duplicate when the messages list is later refreshed from the server.Consider checking if the conversation already has messages before creating a synthetic one, or relying solely on the server-persisted messages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/features/chat/chat-view.tsx` around lines 598 - 627, When drFocusRequested triggers in the useEffect handling deep research return, avoid always inserting a synthetic user message from drState.query; instead check the current conversation messages before injecting to prevent duplicates. In the effect that uses useDeepResearchStore.getState(), use the existing identifiers setActiveConvId, saveActiveConversation, setMessages and drState.query to first inspect the current messages for the target conversation (or the local messages array) and only create/set the synthetic LocalMessage if the conversation has no messages or the last message content/role doesn't match drState.query; otherwise rely on server-persisted messages and skip creating the synthetic user message.api/app/domains/knowledge/schemas.py-28-34 (1)
28-34:⚠️ Potential issue | 🟡 MinorAdd default value for optional
errorfield.In Pydantic v2,
str | Nonewithout a default makes the field required. Sinceerroris semantically optional (jobs may not have errors), it should default toNone.🐛 Proposed fix
class JobStatusOut(BaseModel): id: UUID type: str status: str - error: str | None + error: str | None = None model_config = {"from_attributes": True}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/domains/knowledge/schemas.py` around lines 28 - 34, The JobStatusOut Pydantic model marks error as optional with the type union `str | None` but does not provide a default, making it required in Pydantic v2; update the JobStatusOut model by giving the error field a default of None (e.g., set error to have a default None) so it is truly optional while keeping model_config = {"from_attributes": True}.web/src/features/chat/dr-floating-indicator.tsx-62-84 (1)
62-84:⚠️ Potential issue | 🟡 MinorAdd keyboard handler for accessibility.
The
m.divhasrole="button"andtabIndex={0}but lacks anonKeyDownhandler. Keyboard users won't be able to activate it with Enter or Space.♿ Proposed fix
onClick={() => { if (isChatPage) { requestFocus(); } else { router.push("/app/chat"); requestFocus(); } }} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + if (isChatPage) { + requestFocus(); + } else { + router.push("/app/chat"); + requestFocus(); + } + } + }} role="button" tabIndex={0}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/features/chat/dr-floating-indicator.tsx` around lines 62 - 84, The floating indicator (the m.div with role="button" and tabIndex={0}) is missing keyboard support; add an onKeyDown handler that listens for Enter and Space and invokes the same activation logic as the onClick (call requestFocus() and router.push("/app/chat") when needed), ensuring you preventDefault() for Space to avoid page scrolling and keep the same isChatPage branching; implement the handler as a named function (e.g., handleActivateKey) near the component so it can be reused by onClick and onKeyDown.web/src/features/chat/use-chat-stream.ts-133-140 (1)
133-140:⚠️ Potential issue | 🟡 MinorClear partial reasoning if the stream later fails.
After one or more
"reasoning"events, the catch path only replacescontent, so the assistant bubble can end up showing stale reasoning next to the error message.🧹 Suggested fix
} catch (error) { if (isAbortError(error)) return; const message = getErrorMessage(error, t("streamFailed")); streamLifecycle.fail(message); notifyError(message); setStreaming(false); + const curId = assistantIdRef.current; setMessages((prev) => - prev.map((m) => m.id === assistantId ? { ...m, content: message } : m) + prev.map((m) => + m.id === curId ? { ...m, content: message, reasoning: undefined } : m + ) );Also applies to: 171-179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/features/chat/use-chat-stream.ts` around lines 133 - 140, The assistant's partial "reasoning" buffer (reasoningContentRef) can remain visible when the stream later fails because the error-handling path only replaces message.content; update the catch/error branches that call setMessages to also clear the assistant message's reasoning field (set reasoning: "" or undefined) for the current assistant id (use assistantIdRef.current) so stale reasoning is removed; apply the same change to both places handling stream failure where setMessages is used (refer to reasoningContentRef, assistantIdRef, and the setMessages mapping logic).api/app/services/conversation_service.py-325-344 (1)
325-344:⚠️ Potential issue | 🟡 MinorNested task also needs reference storage.
The
asyncio.create_task(self._sync_diary_safe(today))on line 341 has the same issue as flagged above—store the task reference to prevent potential garbage collection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/services/conversation_service.py` around lines 325 - 344, In _maybe_flush_diary, the asyncio.create_task(self._sync_diary_safe(today)) call can be garbage-collected because its reference isn't retained; instead assign the created task to a variable and store it on the instance (e.g., self._pending_tasks or self._background_tasks) so the task reference is kept alive, and optionally add a done callback to remove the task from the collection when complete; update the code around asyncio.create_task in the _maybe_flush_diary method to use this pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 495b423f-37d3-4cc5-99a1-7845523b92b6
⛔ Files ignored due to path filters (26)
api/app/__pycache__/config.cpython-312.pycis excluded by!**/*.pycapi/app/__pycache__/main.cpython-312.pycis excluded by!**/*.pycapi/app/__pycache__/models.cpython-312.pycis excluded by!**/*.pycapi/app/domains/ai/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/auth/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/config/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/conversation/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/feedback/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/knowledge/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/knowledge_graph/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/memory/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/setup/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/skill/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/source/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/providers/__pycache__/llm.cpython-312.pycis excluded by!**/*.pycapi/app/providers/__pycache__/storage.cpython-312.pycis excluded by!**/*.pycapi/app/skills/__pycache__/base.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/compare_sources.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/deep_read.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/mind_map.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/scheduled_task.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/search_knowledge.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/summarize.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/update_memory_doc.cpython-312.pycis excluded by!**/*.pycapi/app/workers/__pycache__/tasks.cpython-312.pycis excluded by!**/*.pycweb/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (125)
README.mdREADME.zh-CN.mdapi/alembic/versions/025_drop_clerk_id.pyapi/alembic/versions/026_add_research_tasks.pyapi/alembic/versions/027_research_task_conversation.pyapi/app/agents/core/__init__.pyapi/app/agents/core/brain.pyapi/app/agents/core/engine.pyapi/app/agents/core/instructions.pyapi/app/agents/core/react_agent.pyapi/app/agents/core/state.pyapi/app/agents/core/tools.pyapi/app/agents/kg/__init__.pyapi/app/agents/kg/knowledge_graph.pyapi/app/agents/memory.pyapi/app/agents/memory/__init__.pyapi/app/agents/memory/extraction.pyapi/app/agents/memory/file_storage.pyapi/app/agents/memory/notebook.pyapi/app/agents/memory/retrieval.pyapi/app/agents/rag/__init__.pyapi/app/agents/rag/ingestion.pyapi/app/agents/rag/retrieval.pyapi/app/agents/research/__init__.pyapi/app/agents/research/deep_research.pyapi/app/agents/research/evaluation.pyapi/app/agents/research/reflection.pyapi/app/agents/research/task_manager.pyapi/app/agents/writing/__init__.pyapi/app/agents/writing/composer.pyapi/app/agents/writing/ghost_text.pyapi/app/agents/writing/scene_detector.pyapi/app/auth.pyapi/app/config.pyapi/app/database.pyapi/app/domains/ai/router.pyapi/app/domains/ai/routers/__init__.pyapi/app/domains/ai/routers/insights.pyapi/app/domains/ai/routers/knowledge.pyapi/app/domains/ai/routers/research.pyapi/app/domains/ai/routers/suggestions.pyapi/app/domains/ai/routers/writing.pyapi/app/domains/ai/schemas.pyapi/app/domains/auth/router.pyapi/app/domains/auth/schemas.pyapi/app/domains/config/router.pyapi/app/domains/config/schemas.pyapi/app/domains/conversation/router.pyapi/app/domains/feedback/router.pyapi/app/domains/feedback/schemas.pyapi/app/domains/knowledge/router.pyapi/app/domains/knowledge/schemas.pyapi/app/domains/knowledge_graph/router.pyapi/app/domains/knowledge_graph/schemas.pyapi/app/domains/memory/router.pyapi/app/domains/memory/schemas.pyapi/app/domains/notebook/schemas.pyapi/app/domains/setup/router.pyapi/app/domains/setup/schemas.pyapi/app/domains/skill/router.pyapi/app/domains/skill/schemas.pyapi/app/domains/source/router.pyapi/app/logging_config.pyapi/app/main.pyapi/app/models.pyapi/app/providers/llm.pyapi/app/providers/openai_provider.pyapi/app/schemas/response.pyapi/app/services/__init__.pyapi/app/services/conversation_service.pyapi/app/services/source_service.pyapi/app/skills/base.pyapi/app/skills/builtin/compare_sources.pyapi/app/skills/builtin/deep_read.pyapi/app/skills/builtin/mind_map.pyapi/app/skills/builtin/scheduled_task.pyapi/app/skills/builtin/search_knowledge.pyapi/app/skills/builtin/summarize.pyapi/app/skills/builtin/update_memory_doc.pyapi/app/workers/tasks.pydocs-site/en/getting-started.mddocs-site/zh/getting-started.mdlyrascripts/start.shweb/README.mdweb/messages/en.jsonweb/messages/zh.jsonweb/package.jsonweb/src/app/(auth)/login/page.tsxweb/src/app/(marketing)/notebooks/[id]/page.tsxweb/src/app/(marketing)/page.tsxweb/src/app/(workspace)/app/notebooks/[id]/page.tsxweb/src/app/(workspace)/app/page.tsxweb/src/app/(workspace)/app/settings/page.tsxweb/src/app/(workspace)/layout.tsxweb/src/app/setup/page.tsxweb/src/components/chat-input/chat-input.tsxweb/src/components/layout/app-shell.tsxweb/src/components/settings/settings-primitives.tsxweb/src/features/auth/auth-provider.tsxweb/src/features/chat/chat-message-bubble.tsxweb/src/features/chat/chat-view.tsxweb/src/features/chat/choice-cards.tsxweb/src/features/chat/dr-floating-indicator.tsxweb/src/features/chat/dr-learning-card.tsxweb/src/features/chat/dr-mode-toggle.tsxweb/src/features/chat/dr-types.tsweb/src/features/chat/use-chat-stream.tsweb/src/features/chat/use-deep-research.tsweb/src/features/knowledge/knowledge-view.tsxweb/src/features/notebook/notebook-card.tsxweb/src/features/notebook/notebook-icons.tsxweb/src/features/notebook/notebook-workspace.tsxweb/src/features/notebook/notebooks-view.tsxweb/src/features/tasks/tasks-view.tsxweb/src/lib/api-routes.tsweb/src/lib/constants.tsweb/src/lib/http-client.tsweb/src/lib/request-error.tsweb/src/middleware.tsweb/src/schemas/setup-schemas.tsweb/src/services/ai-service.tsweb/src/services/auth-service.tsweb/src/services/notebook-service.tsweb/src/store/use-deep-research-store.ts
💤 Files with no reviewable changes (5)
- README.zh-CN.md
- web/package.json
- README.md
- api/app/auth.py
- api/app/agents/memory.py
| sa.Column("mode", sa.String(20), server_default="quick"), | ||
| sa.Column("status", sa.String(20), server_default="running"), |
There was a problem hiding this comment.
Make core task-state columns non-nullable.
Line 25, Line 26, and Line 31 currently permit explicit NULL despite having defaults. That can create invalid task rows (status/mode/created_at missing) and break queue/state filters.
🧱 Proposed migration hardening
- sa.Column("mode", sa.String(20), server_default="quick"),
- sa.Column("status", sa.String(20), server_default="running"),
+ sa.Column("mode", sa.String(20), server_default="quick", nullable=False),
+ sa.Column("status", sa.String(20), server_default="running", nullable=False),
@@
- sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now()),
+ sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False),Also applies to: 31-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/alembic/versions/026_add_research_tasks.py` around lines 25 - 26, The
migration currently defines sa.Column("mode", sa.String(20),
server_default="quick"), sa.Column("status", sa.String(20),
server_default="running"), and sa.Column("created_at", sa.DateTime(),
server_default=sa.func.now()) but leaves them nullable; update these column
definitions in 026_add_research_tasks.py to enforce non-nullability by setting
nullable=False while retaining the server_default values so existing rows get
defaults; ensure any Alembic operations that create or alter these columns (the
sa.Column("mode"...), sa.Column("status"...), and sa.Column("created_at"...))
are updated accordingly and that the migration will run without trying to set
NULL defaults.
| def get_client() -> AsyncOpenAI: | ||
| """Return the underlying AsyncOpenAI client from the active provider. | ||
|
|
||
| Escape hatch for scenarios that need the raw client (e.g. LangGraph, | ||
| streaming with custom parameters). Prefer ``chat`` / ``chat_stream`` | ||
| for normal usage. | ||
| """ | ||
| from app.config import settings | ||
|
|
||
| return AsyncOpenAI( | ||
| api_key=settings.openai_api_key, | ||
| base_url=settings.openai_base_url or None, | ||
| ) |
There was a problem hiding this comment.
get_client() bypasses the selected provider.
This helper always constructs AsyncOpenAI from the openai_* settings, so callers using it ignore llm_provider and will break as soon as Anthropic is the active provider. Either route this through provider_factory, or fail fast for non-OpenAI-compatible providers instead of exposing it as the active provider’s escape hatch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/providers/llm.py` around lines 17 - 29, get_client() currently
instantiates AsyncOpenAI directly which ignores the configured llm_provider;
update get_client to obtain the active provider via provider_factory (using
settings.llm_provider) and return its underlying async client, or if the active
provider does not expose an OpenAI-compatible raw client, raise a clear error
(e.g., NotImplementedError) so callers fail fast; reference get_client,
provider_factory, llm_provider, and AsyncOpenAI when making the change.
| def _dispatch_post_chat_tasks( | ||
| self, | ||
| conversation_id: UUID, | ||
| scene: str, | ||
| user_memories: list[dict], | ||
| ) -> None: | ||
| asyncio.create_task(self._extract_memories_safe(conversation_id)) | ||
| asyncio.create_task(self._reflect_safe(conversation_id, scene, user_memories)) | ||
| asyncio.create_task(self._compress_safe(conversation_id)) | ||
| asyncio.create_task(self._maybe_flush_diary(conversation_id)) | ||
|
|
||
| from app.config import settings | ||
| if settings.memory_evaluation_sample_rate > 0 and random.random() < settings.memory_evaluation_sample_rate: | ||
| asyncio.create_task(self._evaluate_safe(conversation_id)) |
There was a problem hiding this comment.
Store references to background tasks to prevent premature garbage collection.
asyncio.create_task() returns a task object that should be stored. Without a reference, tasks may be garbage collected before completion, especially under memory pressure. This can cause silent failures in memory extraction, reflection, compression, and diary flushing.
🛠️ Proposed fix using a task set
+ _background_tasks: set[asyncio.Task] = set()
+
def _dispatch_post_chat_tasks(
self,
conversation_id: UUID,
scene: str,
user_memories: list[dict],
) -> None:
- asyncio.create_task(self._extract_memories_safe(conversation_id))
- asyncio.create_task(self._reflect_safe(conversation_id, scene, user_memories))
- asyncio.create_task(self._compress_safe(conversation_id))
- asyncio.create_task(self._maybe_flush_diary(conversation_id))
+ for coro in [
+ self._extract_memories_safe(conversation_id),
+ self._reflect_safe(conversation_id, scene, user_memories),
+ self._compress_safe(conversation_id),
+ self._maybe_flush_diary(conversation_id),
+ ]:
+ task = asyncio.create_task(coro)
+ self._background_tasks.add(task)
+ task.add_done_callback(self._background_tasks.discard)
from app.config import settings
if settings.memory_evaluation_sample_rate > 0 and random.random() < settings.memory_evaluation_sample_rate:
- asyncio.create_task(self._evaluate_safe(conversation_id))
+ task = asyncio.create_task(self._evaluate_safe(conversation_id))
+ self._background_tasks.add(task)
+ task.add_done_callback(self._background_tasks.discard)Note: The random.random() usage on line 287 is appropriate for probabilistic sampling—it's not being used for cryptographic purposes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _dispatch_post_chat_tasks( | |
| self, | |
| conversation_id: UUID, | |
| scene: str, | |
| user_memories: list[dict], | |
| ) -> None: | |
| asyncio.create_task(self._extract_memories_safe(conversation_id)) | |
| asyncio.create_task(self._reflect_safe(conversation_id, scene, user_memories)) | |
| asyncio.create_task(self._compress_safe(conversation_id)) | |
| asyncio.create_task(self._maybe_flush_diary(conversation_id)) | |
| from app.config import settings | |
| if settings.memory_evaluation_sample_rate > 0 and random.random() < settings.memory_evaluation_sample_rate: | |
| asyncio.create_task(self._evaluate_safe(conversation_id)) | |
| _background_tasks: set[asyncio.Task] = set() | |
| def _dispatch_post_chat_tasks( | |
| self, | |
| conversation_id: UUID, | |
| scene: str, | |
| user_memories: list[dict], | |
| ) -> None: | |
| for coro in [ | |
| self._extract_memories_safe(conversation_id), | |
| self._reflect_safe(conversation_id, scene, user_memories), | |
| self._compress_safe(conversation_id), | |
| self._maybe_flush_diary(conversation_id), | |
| ]: | |
| task = asyncio.create_task(coro) | |
| self._background_tasks.add(task) | |
| task.add_done_callback(self._background_tasks.discard) | |
| from app.config import settings | |
| if settings.memory_evaluation_sample_rate > 0 and random.random() < settings.memory_evaluation_sample_rate: | |
| task = asyncio.create_task(self._evaluate_safe(conversation_id)) | |
| self._background_tasks.add(task) | |
| task.add_done_callback(self._background_tasks.discard) |
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 281-281: Store a reference to the return value of asyncio.create_task
(RUF006)
[warning] 282-282: Store a reference to the return value of asyncio.create_task
(RUF006)
[warning] 283-283: Store a reference to the return value of asyncio.create_task
(RUF006)
[warning] 284-284: Store a reference to the return value of asyncio.create_task
(RUF006)
[error] 287-287: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
[warning] 288-288: Store a reference to the return value of asyncio.create_task
(RUF006)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/services/conversation_service.py` around lines 275 - 288,
_dispatch_post_chat_tasks currently fires background work with
asyncio.create_task(...) but drops the Task objects, risking premature GC;
capture and retain them on the instance (e.g., add to a self._background_tasks
set) when creating tasks for _extract_memories_safe, _reflect_safe,
_compress_safe, _maybe_flush_diary and _evaluate_safe, and register an
add_done_callback to remove each task from the set when finished; ensure
self._background_tasks is initialized (e.g., in __init__) so these tasks are
referenced until completion.
| def _dispatch_refresh_summary(self, notebook_id: UUID) -> None: | ||
| asyncio.create_task(self._refresh_summary_safe(notebook_id)) | ||
|
|
||
| async def _refresh_summary_safe(self, notebook_id: UUID) -> None: | ||
| from app.agents.memory import refresh_notebook_summary | ||
| from app.database import AsyncSessionLocal | ||
| try: | ||
| async with AsyncSessionLocal() as session: | ||
| await refresh_notebook_summary(notebook_id, session) | ||
| await session.commit() | ||
| except Exception as exc: | ||
| logger.warning("Failed to refresh notebook summary after source deletion: %s", exc) |
There was a problem hiding this comment.
Fire-and-forget task may be garbage collected before completion.
The task created by asyncio.create_task without storing a reference can be garbage collected before it completes. Python's asyncio documentation explicitly warns about this behavior.
Consider storing task references in a set to prevent premature GC:
🔧 Proposed fix to prevent task garbage collection
+_background_tasks: set = set()
+
def _dispatch_refresh_summary(self, notebook_id: UUID) -> None:
- asyncio.create_task(self._refresh_summary_safe(notebook_id))
+ task = asyncio.create_task(self._refresh_summary_safe(notebook_id))
+ _background_tasks.add(task)
+ task.add_done_callback(_background_tasks.discard)🧰 Tools
🪛 Ruff (0.15.6)
[warning] 106-106: Store a reference to the return value of asyncio.create_task
(RUF006)
[warning] 115-115: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/services/source_service.py` around lines 105 - 116, The
fire-and-forget task created in _dispatch_refresh_summary via
asyncio.create_task(self._refresh_summary_safe(...)) can be garbage-collected
before completion; update the service to keep a strong reference (e.g.,
self._refresh_tasks = set()) and add each created task to that set in
_dispatch_refresh_summary, attach a done-callback that removes the task from
self._refresh_tasks (and logs exceptions if task.exception() is set), and leave
_refresh_summary_safe unchanged; this ensures tasks started by
_dispatch_refresh_summary are retained until finished and cleaned up when done.
| @worker_process_init.connect | ||
| def _on_worker_init(**_kwargs): | ||
| """Runs in each forked worker process: load DB config (API keys etc.).""" | ||
| from app.logging_config import setup_logging | ||
| setup_logging(debug=settings.debug) | ||
| _load_db_settings_sync() |
There was a problem hiding this comment.
Make DB-backed settings preload best-effort.
_load_db_settings_sync() now runs unguarded during worker process init. A transient DB outage here can stop the worker from booting at all, which stalls queued retries and beat-dispatched jobs. Catch/log and continue with env settings, or move this refresh out of the startup path.
Suggested change
`@worker_process_init.connect`
def _on_worker_init(**_kwargs):
"""Runs in each forked worker process: load DB config (API keys etc.)."""
+ import logging
+
from app.logging_config import setup_logging
setup_logging(debug=settings.debug)
- _load_db_settings_sync()
+ try:
+ _load_db_settings_sync()
+ except Exception:
+ logging.getLogger(__name__).exception(
+ "Failed to preload DB-backed settings; continuing with environment defaults"
+ )Also applies to: 61-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/workers/tasks.py` around lines 53 - 58, The worker init handler
_on_worker_init calls _load_db_settings_sync() unguarded so a transient DB
outage can crash worker startup; make the DB-backed settings preload best-effort
by wrapping the _load_db_settings_sync() call in a try/except that logs the
exception (using process/logger from app.logging_config) and continues without
raising, falling back to environment settings; apply the same pattern to the
other startup path(s) mentioned (the similar handler around lines 61-80) so
refreshes do not block worker boot or retries.
| // ── MermaidBlock ────────────────────────────────────────────────────────────── | ||
|
|
||
| const MERMAID_CDN = "https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.esm.min.mjs"; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| let mermaidReady: Promise<any> | null = null; | ||
|
|
||
| function loadMermaid() { | ||
| if (!mermaidReady) { | ||
| mermaidReady = import(/* webpackIgnore: true */ MERMAID_CDN).then((m) => { | ||
| const mermaid = m.default; | ||
| mermaid.initialize({ | ||
| startOnLoad: false, | ||
| theme: "dark", | ||
| themeVariables: { | ||
| primaryColor: "#6366f1", | ||
| primaryTextColor: "#e2e8f0", | ||
| primaryBorderColor: "#4f46e5", | ||
| lineColor: "#64748b", | ||
| secondaryColor: "#1e293b", | ||
| tertiaryColor: "#0f172a", | ||
| fontFamily: "inherit", | ||
| fontSize: "13px", | ||
| }, | ||
| }); | ||
| return mermaid; | ||
| }); | ||
| } | ||
| return mermaidReady; | ||
| } | ||
|
|
||
| function MermaidBlock({ code }: { code: string }) { | ||
| const containerRef = useRef<HTMLDivElement>(null); | ||
| const uniqueId = useId().replace(/:/g, "_"); | ||
| const [error, setError] = useState<string | null>(null); | ||
| const [svgHtml, setSvgHtml] = useState<string | null>(null); | ||
| const [zoomed, setZoomed] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| let cancelled = false; | ||
| loadMermaid() | ||
| .then(async (mermaid) => { | ||
| if (cancelled) return; | ||
| try { | ||
| const { svg } = await mermaid.render(`mermaid${uniqueId}`, code.trim()); | ||
| if (!cancelled) { | ||
| setSvgHtml(svg); | ||
| if (containerRef.current) containerRef.current.innerHTML = svg; | ||
| } | ||
| } catch { | ||
| if (!cancelled) setError("Diagram render failed"); | ||
| } | ||
| }) | ||
| .catch(() => { | ||
| if (!cancelled) setError("Failed to load Mermaid"); | ||
| }); | ||
| return () => { cancelled = true; }; | ||
| }, [code, uniqueId]); | ||
|
|
||
| if (error) { | ||
| return ( | ||
| <pre className="my-2 overflow-x-auto rounded-xl bg-accent/60 p-3 font-mono text-xs leading-5"> | ||
| <code>{code}</code> | ||
| </pre> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
| <div | ||
| ref={containerRef} | ||
| onClick={() => svgHtml && setZoomed(true)} | ||
| className="group my-2 flex cursor-zoom-in justify-center overflow-x-auto rounded-xl bg-accent/30 p-4 transition-colors hover:bg-accent/50 [&_svg]:max-w-full" | ||
| /> | ||
| <AnimatePresence> | ||
| {zoomed && svgHtml && ( | ||
| <m.div | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| transition={{ duration: 0.15 }} | ||
| className="fixed inset-0 z-[999] flex cursor-zoom-out items-center justify-center bg-black/70 backdrop-blur-sm" | ||
| onClick={() => setZoomed(false)} | ||
| > | ||
| <m.div | ||
| initial={{ scale: 0.9, opacity: 0 }} | ||
| animate={{ scale: 1, opacity: 1 }} | ||
| exit={{ scale: 0.9, opacity: 0 }} | ||
| transition={{ duration: 0.2, ease: "easeOut" }} | ||
| className="max-h-[90vh] max-w-[90vw] overflow-auto rounded-2xl bg-card/95 p-6 shadow-2xl ring-1 ring-white/10 [&_svg]:max-h-[80vh] [&_svg]:max-w-full" | ||
| onClick={(e) => e.stopPropagation()} | ||
| dangerouslySetInnerHTML={{ __html: svgHtml }} | ||
| /> | ||
| </m.div> | ||
| )} | ||
| </AnimatePresence> | ||
| </> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Mermaid.js sanitize SVG output to prevent XSS attacks?
💡 Result:
No, Mermaid.js does not fully sanitize its SVG output to prevent all XSS attacks, though it implements measures like DOMPurify and security levels to mitigate risks. Recent CVEs (e.g., CVE-2025-54881, CVE-2025-54880) demonstrate that vulnerabilities allowing XSS via unsanitized user input in labels and iconText existed in versions before 11.10.0, where malicious HTML was injected into SVG elements and executed via innerHTML sinks during rendering. Official docs confirm default "strict" securityLevel encodes HTML tags in text and disables click handlers, with DOMPurify used by default (configurable). However, apps using mermaid.render and inserting raw SVG via dangerouslySetInnerHTML remain vulnerable if user-controlled diagrams bypass protections (e.g., via %%{init: {"securityLevel": "loose"}}%%). Always use latest versions, strict config, and sanitize/escape SVG before DOM insertion.
Citations:
- 1: https://security.snyk.io/vuln/SNYK-JS-MERMAID-12027649
- 2: GHSA-7rqq-prvp-x9jh
- 3: GHSA-8gwm-58g9-j8pw
- 4: GHSA-8gwm-58g9-j8pw
- 5: https://mermaid.js.org/community/security.html
- 6: https://mermaid.js.org/config/schema-docs/config-properties-securitylevel.html
- 7: https://mermaid.js.org/config/configuration.html
- 8: https://mermaid.js.org/config/schema-docs/config.html
- 9: GHSA-r4hj-mc62-jmwj
- 10: https://advisories.gitlab.com/pkg/npm/oneuptime/CVE-2026-32308/
Sanitize SVG output before rendering with dangerouslySetInnerHTML.
The dangerouslySetInnerHTML at line 246 poses a real XSS risk. While Mermaid implements security measures, recent CVEs (CVE-2025-54881, CVE-2025-54880) demonstrate that versions before 11.10.0 are vulnerable to XSS via unsanitized user input in labels and diagram syntax (e.g., iconText fields). Since the input code originates from message content, attackers could inject malicious diagram syntax to execute arbitrary scripts.
Mermaid's default "strict" securityLevel provides some protection but is insufficient when paired with dangerouslySetInnerHTML, which bypasses React's built-in XSS defenses. Additional measures required:
- Upgrade Mermaid to ≥11.10.0 and explicitly configure
securityLevel: "strict" - Sanitize the SVG output using DOMPurify before passing to
dangerouslySetInnerHTML, or alternatively, useinnerHTMLwith proper escaping on a detached container
🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 245-245: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/features/chat/chat-message-bubble.tsx` around lines 156 - 253, The
MermaidBlock is rendering untrusted SVG via dangerouslySetInnerHTML and must be
hardened: upgrade the imported MERMAID_CDN to a version >=11.10.0, set
mermaid.initialize({ ..., securityLevel: "strict" }) inside loadMermaid, and
sanitize any SVG before it’s injected (where MermaidBlock uses
dangerouslySetInnerHTML and containerRef.innerHTML) by passing the generated svg
through a sanitizer like DOMPurify (or sanitizing on a detached DOM) and only
then calling setSvgHtml / innerHTML; keep the rest of the flow (loadMermaid,
mermaid.render, svgHtml, containerRef) intact.
| // OpenAI | ||
| { value: "o3", label: "o3", group: "OpenAI", thinking: true }, | ||
| { value: "o3-mini", label: "o3 Mini", group: "OpenAI", thinking: true }, | ||
| { value: "o4-mini", label: "o4 Mini", group: "OpenAI", thinking: true }, | ||
| { value: "gpt-5.4", label: "GPT-5.4", group: "OpenAI" }, | ||
| { value: "gpt-5.4-pro", label: "GPT-5.4 Pro", group: "OpenAI" }, | ||
| { value: "gpt-5.2-instant", label: "GPT-5.2 Instant", group: "OpenAI" }, | ||
| { value: "gpt-4o", label: "GPT-4o", group: "OpenAI" }, | ||
| { value: "gpt-4o-mini", label: "GPT-4o Mini", group: "OpenAI" }, | ||
| // Anthropic | ||
| { value: "claude-opus-4-6", label: "Claude Opus 4.6", group: "Anthropic" }, | ||
| { value: "claude-sonnet-4-6", label: "Claude Sonnet 4.6", group: "Anthropic" }, | ||
| { value: "claude-sonnet-4-5-20250514", label: "Claude Sonnet 4.5", group: "Anthropic", thinking: true }, | ||
| { value: "claude-haiku-4-5", label: "Claude Haiku 4.5", group: "Anthropic" }, | ||
| // DeepSeek | ||
| { value: "deepseek-chat", label: "DeepSeek V3", group: "DeepSeek" }, | ||
| { value: "deepseek-reasoner", label: "DeepSeek R1", group: "DeepSeek", thinking: true }, | ||
| { value: "gemini-2.5-pro-preview-05-06", label: "Gemini 2.5 Pro", group: "Google", thinking: true }, | ||
| { value: "gemini-2.5-flash-preview-05-20",label: "Gemini 2.5 Flash", group: "Google", thinking: true }, | ||
| { value: "gemini-3.1-pro-preview", label: "Gemini 3.1 Pro", group: "Google" }, | ||
| { value: "gemini-3.1-flash-lite-preview", label: "Gemini 3.1 Flash Lite", group: "Google" }, | ||
| // Qwen | ||
| { value: "qwen-max", label: "Qwen Max", group: "Qwen" }, | ||
| { value: "qwen-plus", label: "Qwen Plus", group: "Qwen" }, | ||
| { value: "qwen-turbo", label: "Qwen Turbo", group: "Qwen" }, | ||
| { value: "qwq-plus", label: "QwQ Plus", group: "Qwen", thinking: true }, | ||
| // Meta | ||
| { value: "llama-4-maverick", label: "Llama 4 Maverick", group: "Meta" }, | ||
| { value: "llama-4-scout", label: "Llama 4 Scout", group: "Meta" }, | ||
| // Mistral | ||
| { value: "mistral-large-latest", label: "Mistral Large", group: "Mistral" }, | ||
| { value: "mistral-small-latest", label: "Mistral Small", group: "Mistral" }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the current OpenAI model names available via API in 2026?
💡 Result:
The current OpenAI models available via API as of March 2026, based on official documentation from OpenAI's model catalog pages, include the following key models across categories (note: the full list is dynamic and can be retrieved via the /v1/models endpoint; these are the prominently featured ones): Frontier models: - gpt-5.4 - gpt-5.4 pro - gpt-5.4 mini - gpt-5.4 nano - gpt-5 mini Other GPT models: - GPT-4.1 - GPT-5.2 - GPT-5.1 - GPT-5.2 pro - GPT-5 pro Image models: - GPT Image 1.5 (likely gpt-image-1.5) - chatgpt-image-latest - gpt-image-1-mini Video models: - Sora 2 - Sora 2 Pro Realtime & audio models: - gpt-realtime-1.5 - gpt-realtime - gpt-realtime-mini - gpt-audio-1.5 Open-weight models: - gpt-oss-120b - gpt-oss-20b Specialized mentions: - o3 (possibly o3-mini or similar reasoning model) - o4-mini-deep-research To get the complete, real-time list, call GET https://api.openai.com/v1/models with your API key.
Citations:
- 1: https://developers.openai.com/api/docs/models
- 2: https://platform.openai.com/docs/models
- 3: https://developers.openai.com/api/docs/models/all
- 4: https://platform.openai.com/docs/models/
- 5: https://developers.openai.com/api/docs/
🌐 Web query:
What are the current Anthropic Claude API model names available in 2026?
💡 Result:
The current Anthropic Claude API models available as of March 18, 2026, include both the latest recommended models and older still-available ones. Note that claude-3-haiku-20240307 is deprecated and scheduled for retirement on April 20, 2026. Latest models (recommended): - claude-opus-4-6 - claude-sonnet-4-6 - claude-haiku-4-5-20251001 (alias: claude-haiku-4-5) Still available (consider migrating): - claude-sonnet-4-5-20250929 (alias: claude-sonnet-4-5) - claude-opus-4-5-20251101 (alias: claude-opus-4-5) - claude-opus-4-1-20250805 (alias: claude-opus-4-1) - claude-sonnet-4-20250514 (alias: claude-sonnet-4-0) - claude-opus-4-20250514 (alias: claude-opus-4-0) - claude-3-haiku-20240307 (deprecated) These can be queried programmatically via the /v1/models endpoint.
Citations:
- 1: https://docs.anthropic.com/en/docs/about-claude/models
- 2: https://docs.anthropic.com/en/api/models
- 3: https://platform.claude.com/docs/en/about-claude/model-deprecations
- 4: https://platform.claude.com/docs/en/about-claude/models/overview
- 5: https://docs.anthropic.com/en/docs/about-claude/models/all-models
🌐 Web query:
What are the current Google Gemini API model names available in 2026?
💡 Result:
The current Google Gemini API model names available as of March 2026, based on official documentation, include both stable and preview models. Stable models are recommended for production, while previews offer cutting-edge features but may change. Stable models: - gemini-2.5-pro - gemini-2.5-flash - gemini-2.5-flash-lite Preview models (actively promoted and available via API): - gemini-3.1-pro-preview - gemini-3.1-pro-preview-customtools - gemini-3-flash (or gemini-3.0-flash-preview) - gemini-2.5-flash-live (preview) - gemini-3.1-flash-lite (preview) Deprecated models to migrate from: - gemini-2.0-flash - gemini-2.0-flash-lite - gemini-3-pro (shut down) To get the exact list programmatically, use the Gemini API's models.list endpoint: GET https://generativelanguage.googleapis.com/v1beta/models?key=YOUR_API_KEY. Model names follow patterns like gemini-X.Y-flash, gemini-X.Y-pro, with suffixes -preview-MM-YYYY for previews or -latest aliases.
Citations:
- 1: https://ai.google.dev/gemini-api/docs/models
- 2: https://ai.google.dev/api/models
- 3: https://docs.cloud.google.com/vertex-ai/generative-ai/docs/models
- 4: https://ai.google.dev/gemini-api/docs/models/gemini-3.1-pro-preview
- 5: https://ai.google.dev/gemini-api/docs/models/gemini-2.5-pro
- 6: https://docs.cloud.google.com/vertex-ai/generative-ai/docs/models/gemini/2-5-flash
- 7: https://cloud.google.com/vertex-ai/generative-ai/docs/learn/model-versions
🌐 Web query:
What are the current DeepSeek API model names available in 2026?
💡 Result:
The current DeepSeek API models available as of March 2026 are deepseek-chat and deepseek-reasoner, both based on DeepSeek-V3.2. deepseek-chat is the non-thinking mode for general tasks (128K context, supports JSON output, tool calls, FIM beta), while deepseek-reasoner is the thinking mode for advanced reasoning (128K context, supports JSON output and tool calls, higher max output). These are the primary models listed in official documentation and used in API examples. To get the real-time list, call GET /models on https://api.deepseek.com.
Citations:
- 1: https://api-docs.deepseek.com/api/list-models
- 2: https://api-docs.deepseek.com/quick_start/pricing
- 3: https://www.datastudios.org/post/deepseek-available-models-supported-api-models-version-differences-capabilities-comparison-and-a
- 4: https://api-docs.deepseek.com/
- 5: https://platform.deepseek.com/api-docs/api/list-models/
- 6: https://api-docs.deepseek.com/api/list-models/
🌐 Web query:
What are the current Qwen API model names available in 2026?
💡 Result:
The current Qwen API models available through Alibaba Cloud Model Studio (DashScope API) in 2026 include the following primary commercial and specialized models: Commercial general-purpose LLMs: - qwen-max - qwen-plus (upgraded to Qwen3.5, also qwen3.5-plus) - qwen-flash Specialized models: - qwen-vl-plus, qwen-vl-max (vision-language) - qwen3-coder-plus, qwen3-coder-flash (coding) - qwen-math-plus, qwen-math-turbo (math) - qwen-mt-plus, qwen-mt-turbo, qwen-mt-flash (translation) - qwen-doc-turbo (document processing) - qwen-deep-research (research) Open-source variants like qwen3.5-397b-a17b, qwen3, qwen2.5 are also accessible via API. Variants with -latest or dated snapshots (e.g., qwen-plus-latest, qwen-turbo-2024-09-19) point to the most recent versions. Access requires an Alibaba Cloud API key via DashScope endpoints, with OpenAI-compatible interfaces available.
Citations:
- 1: https://www.alibabacloud.com/help/en/model-studio/models
- 2: https://www.alibabacloud.com/help/en/model-studio/qwen-api-via-dashscope
- 3: https://alibabacloud.com/help/en/model-studio/models
- 4: https://modelstudio.alibabacloud.com/
- 5: https://www.alibabacloud.com/help/en/model-studio/text-generation
- 6: https://docs.litellm.ai/docs/providers/dashscope
🌐 Web query:
What are the current Meta Llama API model names available in 2026?
💡 Result:
The current Meta Llama API (at llama.meta.com or llama.developer.meta.com, currently in preview/waitlist as of 2026) supports the latest Llama models, primarily from the Llama 4 and Llama 3 families. Official documentation highlights availability of: - Llama 4 Maverick (natively multimodal, e.g., meta-llama/Llama-4-Maverick-17B-128E-Instruct-FP8 in partner APIs) - Llama 4 Scout (natively multimodal, single H100 efficient, 10M context) Additional models mentioned as available via Llama API include previously unreleased Llama 3.3 8B, alongside Llama 3.1 (8B, 70B, 405B), Llama 3.2 (1B, 3B, 11B Vision, 90B Vision), and Llama 3.3 70B. Model IDs follow patterns like "llama-4-maverick", "llama-4-scout", or Hugging Face-style "meta-llama/Llama-4-Maverick-17B-128E-Instruct". Exact list can be queried via the API's /v1/models endpoint. Llama API is OpenAI-compatible and supports chat completions, image understanding, tool calling, etc. Partners like Together AI and AWS Bedrock host these with specific IDs.
Citations:
- 1: https://ai.meta.com/llama/
- 2: https://llama.meta.com/docs/
- 3: https://www.llama.com/
- 4: https://llama.developer.meta.com/docs/models/
- 5: https://llama.developer.meta.com/docs/overview/
- 6: https://www.llama.com/products/llama-api/
- 7: https://llama.developer.meta.com/docs/api/models
- 8: http://llama.meta.com/products/llama-api/
- 9: https://llama.com/models/llama-4/
- 10: http://llama.meta.com/docs/model-cards-and-prompt-formats/llama4/
- 11: http://www.together.ai/models/llama-4-maverick
🌐 Web query:
What are the current Mistral API model names available in 2026?
💡 Result:
The current Mistral API models available as of March 2026, based on official Mistral AI documentation, include the following key models accessible via their La Plateforme API (use /v1/models endpoint for user-specific list). Featured and frontier models: - mistral-large-3 (or mistral-large-latest): State-of-the-art open-weight general-purpose multimodal model (v25.12). - devstral-2 (or devstral-latest): Frontier code agents model for software engineering tasks. - mistral-medium-3.1 (or mistral-medium-latest): Frontier-class multimodal model (v25.08, released Aug 2025). - mistral-small-4 (or mistral-small-latest): Hybrid instruct, reasoning, coding model (v26.03). - mistral-small-3.2: Small model update (v25.06, June 2025). - ministral-3-14b (or ministral-latest): Text and vision capabilities (v25.12). - magistral-medium-1.2 (or magistral-medium-latest): Multimodal reasoning model (v25.09). - magistral-small-1.2 (or magistral-small-latest): Small reasoning model with vision (v25.09). Specialized models: - mistral-moderation-latest: Moderation with 128k context. - ocr-3 (or similar): OCR for Document AI. - voxtral-mini-transcribe-realtime: Audio transcription. - mistral-embed: Embeddings. - Others: devstral-small-2, leanstral, mistral-small-creative, pixtral-large, mistral-nemo-12b, mistral-large-2.1. Legacy/deprecated models (some retired by early 2026, e.g., older magistral 1.1 versions retired Nov 2025): Use alternatives like *-latest aliases. "Latest" aliases point to current versions (e.g., mistral-small-latest -> mistral-small-4 v26.03). All premier models are API-available unless specified open-only; confirm via API key's /v1/models list.
Citations:
- 1: https://docs.mistral.ai/getting-started/models
- 2: https://docs.mistral.ai/models/
- 3: https://docs.mistral.ai/platform/endpoints
- 4: https://docs.mistral.ai/platform/endpoints/
- 5: https://docs.mistral.ai/models
- 6: https://docs.mistral.ai/guides/model-selection/#mistral-medium-intermediate-tasks-that-require-language-transformation
Update model identifiers to match current provider APIs.
Several model identifiers do not match current provider API documentation as of March 2026:
- Anthropic:
claude-sonnet-4-5-20250514should beclaude-sonnet-4-5-20250929(latest snapshot per Anthropic docs) - Google:
gemini-2.5-pro-preview-05-06andgemini-2.5-flash-preview-05-20should use stable versions (gemini-2.5-proandgemini-2.5-flash); the preview-dated IDs appear outdated - Qwen:
qwen-turboandqwq-plusare not listed in current Qwen API documentation; verify these exist or use confirmed models (qwen-max,qwen-plus,qwen-flash) - OpenAI:
gpt-5.2-instant,gpt-4o, andgpt-4o-minido not appear in current OpenAI API documentation; confirm availability or remove
Confirmed correct:
- OpenAI:
o3,o3-mini,gpt-5.4,gpt-5.4-pro - Anthropic:
claude-opus-4-6,claude-sonnet-4-6,claude-haiku-4-5 - Google:
gemini-3.1-pro-preview,gemini-3.1-flash-lite-preview - DeepSeek, Meta, Mistral: all listed models confirmed
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/lib/constants.ts` around lines 37 - 69, The model identifier list
contains outdated or invalid provider model IDs; update the constants array
entries to match current provider APIs by: replace Anthropic
"claude-sonnet-4-5-20250514" with "claude-sonnet-4-5-20250929"; replace Google
"gemini-2.5-pro-preview-05-06" and "gemini-2.5-flash-preview-05-20" with
"gemini-2.5-pro" and "gemini-2.5-flash"; verify and remove or replace OpenAI
entries "gpt-5.2-instant", "gpt-4o", and "gpt-4o-mini" if not supported (keep
only confirmed OpenAI ones: "o3", "o3-mini", "gpt-5.4", "gpt-5.4-pro"); for
Qwen, ensure only supported IDs ("qwen-max", "qwen-plus", "qwen-flash") remain
and remove "qwen-turbo" and "qwq-plus" if they are invalid; update the array
entries by editing the constant that contains these objects (look for the
objects with value keys matching the listed model strings) so labels and group
fields remain consistent with the new values.
| } catch { | ||
| // aborted or network error; non-fatal for background tasks | ||
| } finally { | ||
| if (_abortController === ac) _abortController = null; | ||
|
|
||
| // If the abort signal fired (e.g. page refresh or manual cancel), | ||
| // do NOT mark progress as "done" — the backend task is still running. | ||
| // Just set isActive to false so the persist layer keeps taskId for reconnect. | ||
| if (ac.signal.aborted) { | ||
| set({ isActive: false }); | ||
| return; | ||
| } | ||
|
|
||
| const current = get(); | ||
| if (current.progress && current.progress.status !== "done") { | ||
| set({ | ||
| progress: { ...current.progress, status: "done" }, | ||
| isActive: false, | ||
| }); | ||
| } else { | ||
| set({ isActive: false }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Unsafe return inside finally block."It is generally a bad idea to use control flow statements (return, throw, break, continue) in the finally block because they can override the effect of previously executed control flow statements." The return on line 181 inside the finally block can suppress exceptions or override other return values, making the control flow confusing.
In this case, if subscribeDeepResearch throws an error that isn't caught (the catch block is empty), the return in finally would silently swallow that exception. Restructure to check the abort condition before the cleanup logic instead.
Proposed fix
try {
await subscribeDeepResearch(
taskId,
(event) => {
eventIdx++;
const current = get();
if (!current.progress) return;
const next = processEvent(event, current.progress, reportTokens);
set({ progress: next, reportTokens: reportTokens.value, eventIndex: eventIdx });
},
ac.signal,
fromIndex,
);
- } catch {
- // aborted or network error; non-fatal for background tasks
+ } catch (err) {
+ // If aborted, we'll handle state in the finally block
+ // Only log unexpected errors
+ if (!ac.signal.aborted && err instanceof Error) {
+ console.warn("Deep research subscription error:", err.message);
+ }
} finally {
if (_abortController === ac) _abortController = null;
-
- // If the abort signal fired (e.g. page refresh or manual cancel),
- // do NOT mark progress as "done" — the backend task is still running.
- // Just set isActive to false so the persist layer keeps taskId for reconnect.
- if (ac.signal.aborted) {
- set({ isActive: false });
- return;
- }
-
- const current = get();
- if (current.progress && current.progress.status !== "done") {
- set({
- progress: { ...current.progress, status: "done" },
- isActive: false,
- });
- } else {
- set({ isActive: false });
- }
}
+
+ // Handle state updates after try/catch/finally completes
+ // If the abort signal fired, keep taskId for reconnect
+ if (ac.signal.aborted) {
+ set({ isActive: false });
+ return;
+ }
+
+ const current = get();
+ if (current.progress && current.progress.status !== "done") {
+ set({
+ progress: { ...current.progress, status: "done" },
+ isActive: false,
+ });
+ } else {
+ set({ isActive: false });
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch { | |
| // aborted or network error; non-fatal for background tasks | |
| } finally { | |
| if (_abortController === ac) _abortController = null; | |
| // If the abort signal fired (e.g. page refresh or manual cancel), | |
| // do NOT mark progress as "done" — the backend task is still running. | |
| // Just set isActive to false so the persist layer keeps taskId for reconnect. | |
| if (ac.signal.aborted) { | |
| set({ isActive: false }); | |
| return; | |
| } | |
| const current = get(); | |
| if (current.progress && current.progress.status !== "done") { | |
| set({ | |
| progress: { ...current.progress, status: "done" }, | |
| isActive: false, | |
| }); | |
| } else { | |
| set({ isActive: false }); | |
| } | |
| } | |
| try { | |
| await subscribeDeepResearch( | |
| taskId, | |
| (event) => { | |
| eventIdx++; | |
| const current = get(); | |
| if (!current.progress) return; | |
| const next = processEvent(event, current.progress, reportTokens); | |
| set({ progress: next, reportTokens: reportTokens.value, eventIndex: eventIdx }); | |
| }, | |
| ac.signal, | |
| fromIndex, | |
| ); | |
| } catch (err) { | |
| // If aborted, we'll handle state in the finally block | |
| // Only log unexpected errors | |
| if (!ac.signal.aborted && err instanceof Error) { | |
| console.warn("Deep research subscription error:", err.message); | |
| } | |
| } finally { | |
| if (_abortController === ac) _abortController = null; | |
| } | |
| // Handle state updates after try/catch/finally completes | |
| // If the abort signal fired, keep taskId for reconnect | |
| if (ac.signal.aborted) { | |
| set({ isActive: false }); | |
| return; | |
| } | |
| const current = get(); | |
| if (current.progress && current.progress.status !== "done") { | |
| set({ | |
| progress: { ...current.progress, status: "done" }, | |
| isActive: false, | |
| }); | |
| } else { | |
| set({ isActive: false }); | |
| } | |
| } |
🧰 Tools
🪛 Biome (2.4.7)
[error] 181-181: Unsafe usage of 'return'.
(lint/correctness/noUnsafeFinally)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/store/use-deep-research-store.ts` around lines 171 - 193, The finally
block contains a return when ac.signal.aborted which can swallow errors;
refactor so no control-flow (return) happens inside finally. Capture the abort
state into a local variable (e.g. const wasAborted = ac.signal.aborted) before
entering the finally/cleanup, or set a local flag inside the try/catch, then in
the finally use that flag to decide behavior without returning from inside
finally. Update logic around _abortController, ac, get(), and set() so you only
early-return from the outer function (not inside finally) when wasAborted is
true, and otherwise perform the progress/status updates and set isActive
consistently.
- 后端：Message 增加 reasoning 字段，migration 028；conversation 路由与 conversation_service 调整 - 前端：ChatToolbar 抽离为 chat-toolbar.tsx，chat-tools 工具函数，chat-view/use-deep-research/store 与 home-qa 联调 - i18n 与 chat-api schema 更新 Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
api/app/services/conversation_service.py (1)
32-35:⚠️ Potential issue | 🟠 MajorRetain spawned background tasks until they complete.
These
asyncio.create_task(...)calls still drop theirTaskobjects, including the diary sync path. Keep them in an instance set and discard them in a done callback so they stay referenced for the full run.Suggested fix
class ConversationService: """Conversation domain business logic.""" def __init__(self, db: AsyncSession, user_id: UUID): self.db = db self.user_id = user_id + self._background_tasks: set[asyncio.Task[None]] = set() @@ - asyncio.create_task(self._extract_memories_safe(conversation_id)) - asyncio.create_task(self._reflect_safe(conversation_id, scene, user_memories)) - asyncio.create_task(self._compress_safe(conversation_id)) - asyncio.create_task(self._maybe_flush_diary(conversation_id)) + for coro in ( + self._extract_memories_safe(conversation_id), + self._reflect_safe(conversation_id, scene, user_memories), + self._compress_safe(conversation_id), + self._maybe_flush_diary(conversation_id), + ): + task = asyncio.create_task(coro) + self._background_tasks.add(task) + task.add_done_callback(self._background_tasks.discard) @@ from app.config import settings if settings.memory_evaluation_sample_rate > 0 and random.random() < settings.memory_evaluation_sample_rate: - asyncio.create_task(self._evaluate_safe(conversation_id)) + task = asyncio.create_task(self._evaluate_safe(conversation_id)) + self._background_tasks.add(task) + task.add_done_callback(self._background_tasks.discard) @@ - asyncio.create_task(self._sync_diary_safe(today)) + task = asyncio.create_task(self._sync_diary_safe(today)) + self._background_tasks.add(task) + task.add_done_callback(self._background_tasks.discard)Also applies to: 292-299, 349-352
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/services/conversation_service.py` around lines 32 - 35, The created background tasks are being dropped immediately; initialize an instance set (e.g., self._tasks = set()) in ConversationService.__init__ and, at every asyncio.create_task(...) call (including the diary sync path), add the returned Task to that set and attach a done callback to remove it (task.add_done_callback(self._tasks.discard)) so the Task stays referenced until completion; ensure all create_task sites use this pattern so spawned tasks are retained and cleaned up when finished.web/src/store/use-deep-research-store.ts (1)
175-193:⚠️ Potential issue | 🟠 MajorRemove control flow from
finally.The
returns here are redundant and still triplint/correctness/noUnsafeFinally. Keepfinallycleanup-only so it can't mask future throws/returns, and this should also clear the current Biome error.🧹 Minimal cleanup
} finally { if (_abortController === ac) _abortController = null; - - // If the abort signal fired (e.g. page refresh or manual cancel), - // do NOT mark progress as "done" — the backend task is still running. - // Just set isActive to false so the persist layer keeps taskId for reconnect. - if (ac.signal.aborted) { - set({ isActive: false }); - return; - } - - // Non-abort exits can happen on transient SSE/network interruptions. - // Only treat as completed when we actually received a "done" event. - if (sawDoneEvent) { - set({ isActive: false }); - return; - } set({ isActive: false }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/store/use-deep-research-store.ts` around lines 175 - 193, The finally block currently performs control flow via multiple returns (checking _abortController, ac.signal.aborted and sawDoneEvent) which triggers lint/correctness/noUnsafeFinally; change the finally to be cleanup-only: remove all return statements and only perform state cleanup (clear _abortController when _abortController === ac and call set({ isActive: false }) as needed) so no early returns occur inside the finally. Keep the logic that differentiates abort vs non-abort and sawDoneEvent by recording any flags (e.g., examine ac.signal.aborted and sawDoneEvent into local variables before the try/finally or set them earlier) and let the surrounding function handle any post-finally branching/returns instead of returning inside finally; ensure references to _abortController, ac, ac.signal.aborted, sawDoneEvent and set remain consistent.
🧹 Nitpick comments (8)
api/app/agents/ingestion.py (1)
1-3: LGTM — clean backward-compatibility shim.The wildcard re-export with appropriate
noqasuppression is a valid pattern for maintaining legacy import paths during a refactor. The docstring clearly communicates the module's purpose.Optional: If the goal is to eventually retire this legacy path, consider emitting a
DeprecationWarningso consumers are encouraged to migrate:💡 Optional deprecation warning
"""Backward-compatibility shim for legacy import path `app.agents.ingestion`.""" + +import warnings + +warnings.warn( + "Importing from 'app.agents.ingestion' is deprecated. " + "Use 'app.agents.rag.ingestion' instead.", + DeprecationWarning, + stacklevel=2, +) from app.agents.rag.ingestion import * # noqa: F401,F403,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/agents/ingestion.py` around lines 1 - 3, Add a runtime deprecation notice to the shim module app.agents.ingestion by emitting a warnings.warn call at module import time (right after the module docstring and before or after the existing from app.agents.rag.ingestion import *), using the DeprecationWarning category and a short message that the legacy import path is deprecated and points consumers to app.agents.rag.ingestion; ensure the call is at module level so it runs on import and use the warnings module (warnings.warn, DeprecationWarning) to avoid altering existing behavior.web/src/components/chat-input/chat-toolbar.tsx (3)
253-267: Unnecessary fragment wrapper around single element.The fragment
<>...</>at lines 253-267 wraps only a single<button>. Fragments are only needed when returning multiple siblings. This pattern also appears at lines 269-336 and 338-425.♻️ Remove unnecessary fragments
{isThinkingModel && ( - <> <button type="button" ... </button> - </> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/chat-input/chat-toolbar.tsx` around lines 253 - 267, Remove the unnecessary React fragment wrapping the single <button> in the ChatToolbar component: when rendering the thinking-mode block (guarded by isThinkingModel) delete the surrounding <>...</> so the button is returned directly; update the same pattern in the other similar blocks in this file (the other places that wrap single elements with fragments) — locate the JSX guarded by isThinkingModel (and the blocks that use primaryItemClass, onToggleThinking, closeMenu, thinkingEnabled, and Lightbulb) and remove the extraneous fragments so only the single element is rendered.
136-138: Potential infinite re-render ifonMenuOpenChangeis not memoized.If the parent passes an inline arrow function for
onMenuOpenChange, this effect will fire on every render, potentially causing a render loop or excessive calls. Consider either:
- Documenting that the callback should be stable (memoized with
useCallback)- Using a ref to store the callback to avoid the dependency
♻️ Ref-based approach to stabilize callback
+ const onMenuOpenChangeRef = useRef(onMenuOpenChange); + useEffect(() => { + onMenuOpenChangeRef.current = onMenuOpenChange; + }); + useEffect(() => { - onMenuOpenChange?.(menuOpen); - }, [menuOpen, onMenuOpenChange]); + onMenuOpenChangeRef.current?.(menuOpen); + }, [menuOpen]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/chat-input/chat-toolbar.tsx` around lines 136 - 138, The effect in chat-toolbar.tsx using useEffect(() => onMenuOpenChange?.(menuOpen), [menuOpen, onMenuOpenChange]) can trigger repeated renders if onMenuOpenChange is passed inline; change to a ref-stabilized callback: store the latest onMenuOpenChange in a ref (e.g., callbackRef.current = onMenuOpenChange in an effect) and then have the effect that watches only menuOpen call callbackRef.current?.(menuOpen); alternatively document that onMenuOpenChange must be memoized with useCallback — update references around useEffect, onMenuOpenChange, and menuOpen accordingly.
140-197: Consider extracting repeated outside-click logic into a custom hook.There are four nearly identical
useEffectblocks handling outside-click dismissal formenuRef,drRef,toolsRef, andnotebooksRef. A small custom hook likeuseClickOutside(ref, isOpen, onClose)would reduce duplication and improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/chat-input/chat-toolbar.tsx` around lines 140 - 197, Duplicate outside-click useEffect logic across the menu, dr, tools, and notebooks blocks should be extracted into a small reusable hook; create a hook like useClickOutside(ref, isOpen, onClose) that attaches/removes the "mousedown" listener only when isOpen is true and calls onClose when clicks occur outside (use the same MouseEvent -> Node check used in the current handlers), then replace the four useEffect blocks by calling useClickOutside(menuRef, menuOpen, () => setMenuOpen(false)), useClickOutside(drRef, drDropdownOpen, () => setDrDropdownOpen(false)), useClickOutside(toolsRef, toolsMenuOpen, () => setToolsMenuOpen(false)), and useClickOutside(notebooksRef, notebooksMenuOpen, () => setNotebooksMenuOpen(false)); keep the existing resize logic and panel layout updates (getSecondaryPanelLayout, setToolsPanelLayout, setNotebooksPanelLayout) in their respective effects or combine them with the new hook only if you also support an optional resize handler parameter to preserve current behavior and proper cleanup.api/app/agents/instructions.py (1)
1-3: Stabilize the shim's public export surface by defining__all__.The core module
api/app/agents/core/instructions.pydoes not define__all__, and this shim re-exports its contents with a bare wildcard import. This creates an implicit, unversioned API surface that could drift if new symbols are added to the core module. For a backward-compatibility layer, pinning an explicit__all__ensures the legacy API contract remains intentional.The core module currently exposes 7 public symbols:
CallLLMInstruction,CallToolsInstruction,CallRAGInstruction,StreamAnswerInstruction,CompressContextInstruction,RequestHumanApprovalInstruction, andFinishInstruction. Add an explicit__all__to this shim to mirror that surface:♻️ Suggested change
"""Backward-compatibility shim for legacy import path `app.agents.instructions`.""" +import app.agents.core.instructions as _instructions + from app.agents.core.instructions import * # noqa: F401,F403 +__all__ = getattr(_instructions, "__all__", [name for name in globals() if not name.startswith("_")])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/agents/instructions.py` around lines 1 - 3, The shim currently re-exports everything from app.agents.core.instructions via a bare wildcard which creates an unstable public surface; fix it by defining an explicit __all__ in this file that pins the legacy API to the seven public symbols: CallLLMInstruction, CallToolsInstruction, CallRAGInstruction, StreamAnswerInstruction, CompressContextInstruction, RequestHumanApprovalInstruction, and FinishInstruction (keep the existing from app.agents.core.instructions import * but immediately add __all__ = [...the seven names as strings...] to stabilize the exported surface).api/app/domains/conversation/router.py (1)
27-28: Add ownership regression coverage at the new service boundary.The router no longer scopes notebooks/conversations itself; every read/write/stream/delete path now trusts
ConversationServiceto enforcecurrent_user.id. A small set of cross-user tests here would catch a serious authorization regression early.Also applies to: 39-40, 54-55, 66-67, 81-84, 94-103, 116-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/domains/conversation/router.py` around lines 27 - 28, The router now delegates ownership enforcement to ConversationService which creates a regression risk; add unit/integration tests that exercise each router endpoint (list_by_notebook, read, create/append/stream, delete) to assert cross-user access is denied when the current_user.id does not match the notebook/conversation owner and allowed when it does, targeting the ConversationService boundary (instantiate ConversationService(db, current_user.id) or mock it) and calling the router handlers used on lines around the listed ranges to verify authorization is enforced and appropriate error/status responses are returned.web/src/features/chat/use-deep-research.ts (1)
213-216: Don't keep a silent no-op setter in the hook contract.
setDrProgressis still typed as a realDispatch, but it never updates anything. That makes stale callers fail quietly. Either remove it once callers are migrated, or make the deprecated path noisy in development so misuse is visible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/features/chat/use-deep-research.ts` around lines 213 - 216, setDrProgress is currently a typed Dispatch but implemented as a silent no-op, which can hide stale-caller bugs; update the useDeepResearch hook so this deprecated setter is either removed from the public contract or replaced with a noisy dev-only setter that warns (or throws in strict mode) when called. Locate the symbol setDrProgress inside the useDeepResearch hook and either (a) remove it and update callers, or (b) implement it to call the real state setter or to console.warn with contextual info (including function name and call-site hint) when process.env.NODE_ENV !== 'production', so misuse becomes visible during development.web/src/store/use-deep-research-store.ts (1)
260-270: Either resume fromeventIndexor delete the cursor.The store tracks
eventIndexas a reconnect cursor, butreconnect()always resets state and resubscribes from0. That means every reconnect still replays the whole stream. Either keep enough partial state to resume from the stored index, or remove the cursor/comment so the API matches the behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/store/use-deep-research-store.ts` around lines 260 - 270, The code resets eventIndex to 0 and always calls subscribeTo(taskId, get, set, 0), which prevents resuming; change reconnect() so it preserves and uses the stored cursor: set eventIndex to status.eventIndex (or read the existing store eventIndex if present) instead of always 0 and pass that value into subscribeTo (i.e., subscribeTo(taskId, get, set, eventIndex)). Update the set call that creates initial state (where eventIndex is currently hardcoded) to use the preserved cursor, and ensure makeInitialProgress(status.mode) remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/app/agents/research/task_manager.py`:
- Around line 65-70: The in-memory _buffers dict (and its use in
get_buffer/TaskBuffer) is single-worker only; replace it with a shared store and
optional pub/sub so buffers are visible across workers: remove the module-level
_buffers and implement a Redis (or other shared datastore)-backed buffer
registry used by get_buffer(TaskBuffer) and any create/delete/update_buffer
functions, persist TaskBuffer state (serialize/deserialize) and publish changes
on a channel so other workers can subscribe and update local caches;
alternatively, if you cannot introduce shared storage, document and enforce
sticky routing for endpoints that create/use TaskBuffer (e.g., POST
/ai/deep-research and GET /events) so both requests always route to the same
worker. Ensure you update function signatures and any callers of get_buffer and
buffer-mutating functions to use the shared-store accessors.
In `@api/app/domains/ai/routers/research.py`:
- Around line 134-165: The completed-task replay branch (inside the buf is None
and task.status == "done" block) ignores the client's resume offset, causing
duplicate events on reconnect; update the replay_done() generator to accept and
honor the ?from=N offset (read the request's `from` query param) and skip
yielding events until the stream position reaches that offset when emitting
plan, learning, writing, report_complete, done, and deliverable events; locate
the replay logic in replay_done, use task.timeline_json and any existing event
ordering logic to compute each event's sequence index, advance/seek past events
lower than `from`, then yield only remaining events before sending the final
"data: [DONE]\n\n" via StreamingResponse.
- Around line 24-25: The ResearchTask DB row is left as status="running" while
execution is launched with an untracked asyncio.create_task(), so on restart the
row is orphaned; fix by keeping a reference to the started Task in an in-memory
registry (e.g., a module-level dict like running_tasks keyed by ResearchTask.id)
instead of fire-and-forget, attach a done callback to the Task that updates the
ResearchTask DB row to a terminal status and removes the Task from
running_tasks, and ensure creation flow (where ResearchTask is committed as
status="running") stores the returned Task into running_tasks; for durability
beyond restarts migrate execution to a queue, but for the minimal fix modify
places that call asyncio.create_task(...) to save the Task and register a
callback that calls the existing DB update logic for ResearchTask.
In `@api/app/services/conversation_service.py`:
- Around line 276-282: The current fallback selects messages ordered ascending
and limits to 20, returning the oldest 20; instead, change the query in
conversation history retrieval to order by Message.created_at.desc() (newest
first) with .limit(20), then take result.scalars().all(), reverse that list to
restore chronological order, and finally return [{"role": m.role, "content":
m.content} for m in reversed_list]; update the code around the select(Message)
... .order_by(...) and the result.scalars().all() handling so the agent sees the
most recent window of messages in chronological order.
- Around line 228-241: The flushed assistant Message is not yet visible to new
AsyncSessionLocal sessions because the outer transaction isn't committed before
_dispatch_post_chat_tasks spawns background tasks; change the flow to await
self.db.commit() (or otherwise commit the outer transaction/context that
includes self.db.add/await self.db.flush) before calling
_dispatch_post_chat_tasks so background workers see the new message, and update
the asyncio.create_task usages referenced in _dispatch_post_chat_tasks (and the
calls at the noted lines) to capture and manage the returned Task objects (store
them in variables, append to a list, or return them) instead of dropping the
handles so task lifetimes are explicit.
In `@web/src/components/home/home-qa.tsx`:
- Line 108: Replace the hardcoded Chinese label clearNotebookLabel="清除笔记本限制" in
the HomeQA component with a translated string (e.g.,
clearNotebookLabel={t('clearNotebookFilter')}) using the existing i18n helper
(t) or useTranslation hook used elsewhere in this component; then add the
"clearNotebookFilter" key to the locale message files used by the app with
values "Clear notebook filter" (English) and "清除笔记本限制" (Chinese) so the label is
served from translations rather than hardcoded text.
In `@web/src/store/use-deep-research-store.ts`:
- Around line 210-232: start() sets optimistic active state before awaiting
createDeepResearch(), so if createDeepResearch() rejects the store remains
stale; wrap the call to createDeepResearch(query, { notebookId, mode }) in a
try/catch and on error rollback the optimistic fields (reset isActive to false,
clear taskId, conversationId, revert progress to null or previous state from
get(), and ensure eventIndex/reportTokens are cleared) and rethrow or handle the
error; keep subscribeTo(taskId, get, set) only after successful creation and
setting of taskId/conversationId so you never subscribe when creation fails.
---
Duplicate comments:
In `@api/app/services/conversation_service.py`:
- Around line 32-35: The created background tasks are being dropped immediately;
initialize an instance set (e.g., self._tasks = set()) in
ConversationService.__init__ and, at every asyncio.create_task(...) call
(including the diary sync path), add the returned Task to that set and attach a
done callback to remove it (task.add_done_callback(self._tasks.discard)) so the
Task stays referenced until completion; ensure all create_task sites use this
pattern so spawned tasks are retained and cleaned up when finished.
In `@web/src/store/use-deep-research-store.ts`:
- Around line 175-193: The finally block currently performs control flow via
multiple returns (checking _abortController, ac.signal.aborted and sawDoneEvent)
which triggers lint/correctness/noUnsafeFinally; change the finally to be
cleanup-only: remove all return statements and only perform state cleanup (clear
_abortController when _abortController === ac and call set({ isActive: false })
as needed) so no early returns occur inside the finally. Keep the logic that
differentiates abort vs non-abort and sawDoneEvent by recording any flags (e.g.,
examine ac.signal.aborted and sawDoneEvent into local variables before the
try/finally or set them earlier) and let the surrounding function handle any
post-finally branching/returns instead of returning inside finally; ensure
references to _abortController, ac, ac.signal.aborted, sawDoneEvent and set
remain consistent.
---
Nitpick comments:
In `@api/app/agents/ingestion.py`:
- Around line 1-3: Add a runtime deprecation notice to the shim module
app.agents.ingestion by emitting a warnings.warn call at module import time
(right after the module docstring and before or after the existing from
app.agents.rag.ingestion import *), using the DeprecationWarning category and a
short message that the legacy import path is deprecated and points consumers to
app.agents.rag.ingestion; ensure the call is at module level so it runs on
import and use the warnings module (warnings.warn, DeprecationWarning) to avoid
altering existing behavior.
In `@api/app/agents/instructions.py`:
- Around line 1-3: The shim currently re-exports everything from
app.agents.core.instructions via a bare wildcard which creates an unstable
public surface; fix it by defining an explicit __all__ in this file that pins
the legacy API to the seven public symbols: CallLLMInstruction,
CallToolsInstruction, CallRAGInstruction, StreamAnswerInstruction,
CompressContextInstruction, RequestHumanApprovalInstruction, and
FinishInstruction (keep the existing from app.agents.core.instructions import *
but immediately add __all__ = [...the seven names as strings...] to stabilize
the exported surface).
In `@api/app/domains/conversation/router.py`:
- Around line 27-28: The router now delegates ownership enforcement to
ConversationService which creates a regression risk; add unit/integration tests
that exercise each router endpoint (list_by_notebook, read,
create/append/stream, delete) to assert cross-user access is denied when the
current_user.id does not match the notebook/conversation owner and allowed when
it does, targeting the ConversationService boundary (instantiate
ConversationService(db, current_user.id) or mock it) and calling the router
handlers used on lines around the listed ranges to verify authorization is
enforced and appropriate error/status responses are returned.
In `@web/src/components/chat-input/chat-toolbar.tsx`:
- Around line 253-267: Remove the unnecessary React fragment wrapping the single
<button> in the ChatToolbar component: when rendering the thinking-mode block
(guarded by isThinkingModel) delete the surrounding <>...</> so the button is
returned directly; update the same pattern in the other similar blocks in this
file (the other places that wrap single elements with fragments) — locate the
JSX guarded by isThinkingModel (and the blocks that use primaryItemClass,
onToggleThinking, closeMenu, thinkingEnabled, and Lightbulb) and remove the
extraneous fragments so only the single element is rendered.
- Around line 136-138: The effect in chat-toolbar.tsx using useEffect(() =>
onMenuOpenChange?.(menuOpen), [menuOpen, onMenuOpenChange]) can trigger repeated
renders if onMenuOpenChange is passed inline; change to a ref-stabilized
callback: store the latest onMenuOpenChange in a ref (e.g., callbackRef.current
= onMenuOpenChange in an effect) and then have the effect that watches only
menuOpen call callbackRef.current?.(menuOpen); alternatively document that
onMenuOpenChange must be memoized with useCallback — update references around
useEffect, onMenuOpenChange, and menuOpen accordingly.
- Around line 140-197: Duplicate outside-click useEffect logic across the menu,
dr, tools, and notebooks blocks should be extracted into a small reusable hook;
create a hook like useClickOutside(ref, isOpen, onClose) that attaches/removes
the "mousedown" listener only when isOpen is true and calls onClose when clicks
occur outside (use the same MouseEvent -> Node check used in the current
handlers), then replace the four useEffect blocks by calling
useClickOutside(menuRef, menuOpen, () => setMenuOpen(false)),
useClickOutside(drRef, drDropdownOpen, () => setDrDropdownOpen(false)),
useClickOutside(toolsRef, toolsMenuOpen, () => setToolsMenuOpen(false)), and
useClickOutside(notebooksRef, notebooksMenuOpen, () =>
setNotebooksMenuOpen(false)); keep the existing resize logic and panel layout
updates (getSecondaryPanelLayout, setToolsPanelLayout, setNotebooksPanelLayout)
in their respective effects or combine them with the new hook only if you also
support an optional resize handler parameter to preserve current behavior and
proper cleanup.
In `@web/src/features/chat/use-deep-research.ts`:
- Around line 213-216: setDrProgress is currently a typed Dispatch but
implemented as a silent no-op, which can hide stale-caller bugs; update the
useDeepResearch hook so this deprecated setter is either removed from the public
contract or replaced with a noisy dev-only setter that warns (or throws in
strict mode) when called. Locate the symbol setDrProgress inside the
useDeepResearch hook and either (a) remove it and update callers, or (b)
implement it to call the real state setter or to console.warn with contextual
info (including function name and call-site hint) when process.env.NODE_ENV !==
'production', so misuse becomes visible during development.
In `@web/src/store/use-deep-research-store.ts`:
- Around line 260-270: The code resets eventIndex to 0 and always calls
subscribeTo(taskId, get, set, 0), which prevents resuming; change reconnect() so
it preserves and uses the stored cursor: set eventIndex to status.eventIndex (or
read the existing store eventIndex if present) instead of always 0 and pass that
value into subscribeTo (i.e., subscribeTo(taskId, get, set, eventIndex)). Update
the set call that creates initial state (where eventIndex is currently
hardcoded) to use the preserved cursor, and ensure
makeInitialProgress(status.mode) remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8913edfe-2fc4-453d-b3de-cffcb50a544c
📒 Files selected for processing (36)
api/alembic/versions/028_message_reasoning.pyapi/app/agents/brain.pyapi/app/agents/composer.pyapi/app/agents/deep_research.pyapi/app/agents/engine.pyapi/app/agents/evaluation.pyapi/app/agents/file_memory.pyapi/app/agents/ingestion.pyapi/app/agents/instructions.pyapi/app/agents/knowledge_graph.pyapi/app/agents/memory_extraction.pyapi/app/agents/memory_notebook.pyapi/app/agents/memory_retrieval.pyapi/app/agents/react_agent.pyapi/app/agents/reflection.pyapi/app/agents/research/task_manager.pyapi/app/agents/retrieval.pyapi/app/agents/scene_detector.pyapi/app/agents/state.pyapi/app/agents/tools.pyapi/app/domains/ai/routers/research.pyapi/app/domains/conversation/router.pyapi/app/domains/conversation/schemas.pyapi/app/models.pyapi/app/services/conversation_service.pyweb/messages/en.jsonweb/messages/zh.jsonweb/src/components/chat-input/chat-toolbar.tsxweb/src/components/chat-input/index.tsweb/src/components/home/home-qa.tsxweb/src/features/chat/chat-helpers.tsxweb/src/features/chat/chat-view.tsxweb/src/features/chat/use-deep-research.tsweb/src/lib/chat-tools.tsweb/src/schemas/chat-api.tsweb/src/store/use-deep-research-store.ts
✅ Files skipped from review due to trivial changes (6)
- web/src/components/chat-input/index.ts
- web/src/features/chat/chat-helpers.tsx
- api/app/domains/conversation/schemas.py
- api/alembic/versions/028_message_reasoning.py
- web/src/features/chat/chat-view.tsx
- web/messages/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- api/app/models.py
| # Module-level registry of active task buffers | ||
| _buffers: dict[str, TaskBuffer] = {} | ||
|
|
||
|
|
||
| def get_buffer(task_id: str) -> TaskBuffer | None: | ||
| return _buffers.get(task_id) |
There was a problem hiding this comment.
Move the task buffer out of process memory.
_buffers only exists inside one API worker. If POST /ai/deep-research lands on worker A and GET /events lands on worker B, the second request will see no buffer and treat an active task as expired. Back this with shared storage/pub-sub, or explicitly require sticky single-worker routing.
Also applies to: 85-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/agents/research/task_manager.py` around lines 65 - 70, The in-memory
_buffers dict (and its use in get_buffer/TaskBuffer) is single-worker only;
replace it with a shared store and optional pub/sub so buffers are visible
across workers: remove the module-level _buffers and implement a Redis (or other
shared datastore)-backed buffer registry used by get_buffer(TaskBuffer) and any
create/delete/update_buffer functions, persist TaskBuffer state
(serialize/deserialize) and publish changes on a channel so other workers can
subscribe and update local caches; alternatively, if you cannot introduce shared
storage, document and enforce sticky routing for endpoints that create/use
TaskBuffer (e.g., POST /ai/deep-research and GET /events) so both requests
always route to the same worker. Ensure you update function signatures and any
callers of get_buffer and buffer-mutating functions to use the shared-store
accessors.
| router = APIRouter() | ||
| logger = logging.getLogger(__name__) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file structure
find . -name "research.py" -type f | head -5Repository: LinMoQC/LyraNote
Length of output: 100
🏁 Script executed:
# Get an overview of the repository structure
git ls-files | grep -E "(routers|research)" | head -20Repository: LinMoQC/LyraNote
Length of output: 898
🏁 Script executed:
# Check if the file exists and its size
wc -l api/app/domains/ai/routers/research.py 2>/dev/null || echo "File not found at expected path"Repository: LinMoQC/LyraNote
Length of output: 102
🏁 Script executed:
# Read the entire file to understand the structure
cat -n api/app/domains/ai/routers/research.pyRepository: LinMoQC/LyraNote
Length of output: 8652
🏁 Script executed:
# Check the task_manager module to understand task lifecycle
cat -n api/app/agents/research/task_manager.pyRepository: LinMoQC/LyraNote
Length of output: 7862
🏁 Script executed:
# Search for ResearchTask status updates to understand lifecycle
rg "status\s*=" api/app/agents/research/ -A 2 -B 2Repository: LinMoQC/LyraNote
Length of output: 929
🏁 Script executed:
# Verify the exact lines mentioned "Also applies to: 70-80, 89-101"
sed -n '70,80p; 89,101p' api/app/domains/ai/routers/research.py | cat -nRepository: LinMoQC/LyraNote
Length of output: 922
Retain asyncio.Task references to prevent orphaned "running" rows on restart.
The ResearchTask row is committed as status="running" (line 70), then execution is launched with an untracked fire-and-forget asyncio.create_task() (line 89). On process restart/reload, this background coroutine is abandoned, leaving the row permanently running with no worker to finish it.
The 10-minute buffer window allows client reconnects, but the database row remains orphaned indefinitely. At minimum, retain the Task object; if this feature must survive full process restarts, migrate execution to a durable queue.
Minimal fix for task retention
router = APIRouter()
logger = logging.getLogger(__name__)
+_research_tasks: set[asyncio.Task[None]] = set()
@@
- asyncio.create_task(
+ research_task = asyncio.create_task(
run_research_task(
task_id=str(task_id),
query=body.query,
notebook_id=str(notebook.id),
conversation_id=str(conv.id),
user_id=str(current_user.id),
mode=body.mode,
model=settings.llm_model,
tavily_api_key=settings.tavily_api_key or None,
user_memories=user_memories,
)
)
+ _research_tasks.add(research_task)
+ research_task.add_done_callback(_research_tasks.discard)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/domains/ai/routers/research.py` around lines 24 - 25, The
ResearchTask DB row is left as status="running" while execution is launched with
an untracked asyncio.create_task(), so on restart the row is orphaned; fix by
keeping a reference to the started Task in an in-memory registry (e.g., a
module-level dict like running_tasks keyed by ResearchTask.id) instead of
fire-and-forget, attach a done callback to the Task that updates the
ResearchTask DB row to a terminal status and removes the Task from
running_tasks, and ensure creation flow (where ResearchTask is committed as
status="running") stores the returned Task into running_tasks; for durability
beyond restarts migrate execution to a queue, but for the minimal fix modify
places that call asyncio.create_task(...) to save the Task and register a
callback that calls the existing DB update logic for ResearchTask.
| if buf is None and task.status == "done": | ||
| async def replay_done(): | ||
| if task.timeline_json: | ||
| tl = task.timeline_json | ||
| plan_event = { | ||
| "type": "plan", | ||
| "data": { | ||
| "sub_questions": tl.get("subQuestions", []), | ||
| "research_goal": tl.get("researchGoal"), | ||
| "evaluation_criteria": tl.get("evaluationCriteria"), | ||
| "report_title": tl.get("reportTitle"), | ||
| }, | ||
| } | ||
| yield f"data: {json.dumps(plan_event, ensure_ascii=False)}\n\n" | ||
|
|
||
| for learning in tl.get("learnings", []): | ||
| yield f"data: {json.dumps({'type': 'learning', 'data': learning}, ensure_ascii=False)}\n\n" | ||
|
|
||
| yield f"data: {json.dumps({'type': 'writing', 'data': {}}, ensure_ascii=False)}\n\n" | ||
|
|
||
| if task.report: | ||
| yield f"data: {json.dumps({'type': 'report_complete', 'data': {'report': task.report}}, ensure_ascii=False)}\n\n" | ||
|
|
||
| done_cites = tl.get("doneCitations", []) | ||
| yield f"data: {json.dumps({'type': 'done', 'data': {'citations': done_cites}}, ensure_ascii=False)}\n\n" | ||
|
|
||
| if tl.get("deliverable"): | ||
| yield f"data: {json.dumps({'type': 'deliverable', 'data': tl['deliverable']}, ensure_ascii=False)}\n\n" | ||
|
|
||
| yield "data: [DONE]\n\n" | ||
|
|
||
| return StreamingResponse(replay_done(), media_type="text/event-stream") |
There was a problem hiding this comment.
Apply from= to the completed-task replay branch.
When the live buffer has already been cleaned up, this branch always replays from the start. A reconnect with ?from=N will duplicate the earlier plan/learning/report events and can rebuild the UI twice.
One way to honor the resume offset
if buf is None and task.status == "done":
async def replay_done():
+ events: list[dict] = []
if task.timeline_json:
tl = task.timeline_json
plan_event = {
"type": "plan",
"data": {
@@
"evaluation_criteria": tl.get("evaluationCriteria"),
"report_title": tl.get("reportTitle"),
},
}
- yield f"data: {json.dumps(plan_event, ensure_ascii=False)}\n\n"
+ events.append(plan_event)
for learning in tl.get("learnings", []):
- yield f"data: {json.dumps({'type': 'learning', 'data': learning}, ensure_ascii=False)}\n\n"
+ events.append({"type": "learning", "data": learning})
- yield f"data: {json.dumps({'type': 'writing', 'data': {}}, ensure_ascii=False)}\n\n"
+ events.append({"type": "writing", "data": {}})
if task.report:
- yield f"data: {json.dumps({'type': 'report_complete', 'data': {'report': task.report}}, ensure_ascii=False)}\n\n"
+ events.append({"type": "report_complete", "data": {"report": task.report}})
done_cites = tl.get("doneCitations", [])
- yield f"data: {json.dumps({'type': 'done', 'data': {'citations': done_cites}}, ensure_ascii=False)}\n\n"
+ events.append({"type": "done", "data": {"citations": done_cites}})
if tl.get("deliverable"):
- yield f"data: {json.dumps({'type': 'deliverable', 'data': tl['deliverable']}, ensure_ascii=False)}\n\n"
+ events.append({"type": "deliverable", "data": tl["deliverable"]})
+
+ for event in events[from_index:]:
+ yield f"data: {json.dumps(event, ensure_ascii=False)}\n\n"
yield "data: [DONE]\n\n"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/domains/ai/routers/research.py` around lines 134 - 165, The
completed-task replay branch (inside the buf is None and task.status == "done"
block) ignores the client's resume offset, causing duplicate events on
reconnect; update the replay_done() generator to accept and honor the ?from=N
offset (read the request's `from` query param) and skip yielding events until
the stream position reaches that offset when emitting plan, learning, writing,
report_complete, done, and deliverable events; locate the replay logic in
replay_done, use task.timeline_json and any existing event ordering logic to
compute each event's sequence index, advance/seek past events lower than `from`,
then yield only remaining events before sending the final "data: [DONE]\n\n" via
StreamingResponse.
| async def send_message( | ||
| conversation_id: UUID, body: MessageCreate, db: DbDep, current_user: CurrentUser | ||
| ): | ||
| """Non-streaming: save user message and return full assistant reply.""" | ||
| conv = await _get_owned_conv(db, conversation_id, current_user.id) | ||
|
|
||
| user_msg = Message( | ||
| conversation_id=conversation_id, | ||
| role="user", | ||
| content=body.content, | ||
| ) | ||
| db.add(user_msg) | ||
| await db.flush() | ||
|
|
||
| history = await _load_history(db, conversation_id) | ||
|
|
||
| from app.agents.retrieval import retrieve_chunks | ||
| from app.agents.composer import compose_answer | ||
| from app.agents.memory import get_user_memories, get_notebook_summary | ||
|
|
||
| user_memories = await get_user_memories(current_user.id, db) | ||
| notebook_summary = await get_notebook_summary(conv.notebook_id, db) | ||
| chunks = await retrieve_chunks(body.content, str(conv.notebook_id), db) | ||
| answer, citations = await compose_answer( | ||
| body.content, chunks, history, | ||
| user_memories=user_memories, | ||
| notebook_summary=notebook_summary, | ||
| ) | ||
|
|
||
| assistant_msg = Message( | ||
| conversation_id=conversation_id, | ||
| role="assistant", | ||
| content=answer, | ||
| citations=citations, | ||
| ) | ||
| db.add(assistant_msg) | ||
| await db.flush() | ||
| await db.refresh(assistant_msg) | ||
| return success(assistant_msg) | ||
| svc = ConversationService(db, current_user.id) | ||
| return success(await svc.send_message(conversation_id, body.content)) |
There was a problem hiding this comment.
Don't accept fields this handler drops.
Line 64 reuses MessageCreate, but this handler only forwards body.content. Since the same schema exposes global_search, tool_hint, attachment_ids, and attachments_meta for /messages/stream, the plain /messages endpoint currently accepts fields it silently ignores. Either persist the supported extras here or use a narrower request model.
| assistant_msg = Message( | ||
| conversation_id=conversation_id, | ||
| role="assistant", | ||
| content=content_text, | ||
| reasoning=reasoning_text, | ||
| citations=citations, | ||
| agent_steps=agent_steps or None, | ||
| ) | ||
| self.db.add(assistant_msg) | ||
| await self.db.flush() | ||
|
|
||
| self._dispatch_post_chat_tasks( | ||
| conversation_id, scene, user_memories | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "conversation_service.py" | head -20Repository: LinMoQC/LyraNote
Length of output: 102
🏁 Script executed:
wc -l ./api/app/services/conversation_service.pyRepository: LinMoQC/LyraNote
Length of output: 106
🏁 Script executed:
cat -n ./api/app/services/conversation_service.py | sed -n '220,250p'Repository: LinMoQC/LyraNote
Length of output: 1510
🏁 Script executed:
cat -n ./api/app/services/conversation_service.py | sed -n '1,50p'Repository: LinMoQC/LyraNote
Length of output: 1789
🏁 Script executed:
rg -n "_dispatch_post_chat_tasks" ./api/app/services/conversation_service.py -A 20Repository: LinMoQC/LyraNote
Length of output: 2062
🏁 Script executed:
cat -n ./api/app/services/conversation_service.py | sed -n '286,330p'Repository: LinMoQC/LyraNote
Length of output: 2432
🏁 Script executed:
cat -n ./api/app/services/conversation_service.py | sed -n '130,170p'Repository: LinMoQC/LyraNote
Length of output: 1685
🏁 Script executed:
cat -n ./api/app/services/conversation_service.py | sed -n '220,250p'Repository: LinMoQC/LyraNote
Length of output: 1510
🏁 Script executed:
cat -n ./api/app/services/conversation_service.py | sed -n '136,250p'Repository: LinMoQC/LyraNote
Length of output: 5070
🏁 Script executed:
rg -n "stream_agent" . --type py -B 3 -A 3 | head -60Repository: LinMoQC/LyraNote
Length of output: 1080
🏁 Script executed:
cat -n ./api/app/domains/conversation/router.py | sed -n '80,110p'Repository: LinMoQC/LyraNote
Length of output: 1252
🏁 Script executed:
cat -n ./api/app/database.py | head -80Repository: LinMoQC/LyraNote
Length of output: 980
🏁 Script executed:
cat -n ./api/app/services/conversation_service.py | sed -n '301,340p'Repository: LinMoQC/LyraNote
Length of output: 2125
🏁 Script executed:
cat -n ./api/app/services/conversation_service.py | sed -n '336,360p'Repository: LinMoQC/LyraNote
Length of output: 1478
🏁 Script executed:
cat -n ./api/app/services/conversation_service.py | sed -n '251,285p'Repository: LinMoQC/LyraNote
Length of output: 1711
Commit the outer transaction before dispatching background tasks.
The assistant message is flushed but not yet committed when _dispatch_post_chat_tasks() spawns background tasks. Since background tasks open fresh AsyncSessionLocal() sessions, they cannot see the newly flushed message until the outer transaction commits—which doesn't happen until after the response stream finishes. This causes background jobs to operate on stale conversation state.
Additionally, all asyncio.create_task() calls at lines 292–295, 299, and 352 drop their returned task handles, leaving task lifetimes implicitly managed.
Either commit before dispatch, or defer task spawning to code that runs after the outer transaction commits.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/services/conversation_service.py` around lines 228 - 241, The flushed
assistant Message is not yet visible to new AsyncSessionLocal sessions because
the outer transaction isn't committed before _dispatch_post_chat_tasks spawns
background tasks; change the flow to await self.db.commit() (or otherwise commit
the outer transaction/context that includes self.db.add/await self.db.flush)
before calling _dispatch_post_chat_tasks so background workers see the new
message, and update the asyncio.create_task usages referenced in
_dispatch_post_chat_tasks (and the calls at the noted lines) to capture and
manage the returned Task objects (store them in variables, append to a list, or
return them) instead of dropping the handles so task lifetimes are explicit.
| result = await self.db.execute( | ||
| select(Message) | ||
| .where(Message.conversation_id == conversation_id) | ||
| .order_by(Message.created_at.asc(), Message.role.desc()) | ||
| .limit(20) | ||
| ) | ||
| return [{"role": m.role, "content": m.content} for m in result.scalars().all()] |
There was a problem hiding this comment.
Load the most recent history window here.
This fallback orders ascending and then limits to 20, so once a conversation exceeds 20 messages the agent sees the oldest turns instead of the latest context. Query the newest slice and reverse it before returning.
Suggested fix
result = await self.db.execute(
select(Message)
.where(Message.conversation_id == conversation_id)
- .order_by(Message.created_at.asc(), Message.role.desc())
- .limit(20)
+ .order_by(Message.created_at.desc(), Message.role.asc())
+ .limit(RAW_HISTORY_WINDOW)
)
- return [{"role": m.role, "content": m.content} for m in result.scalars().all()]
+ recent = list(reversed(result.scalars().all()))
+ return [{"role": m.role, "content": m.content} for m in recent]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/services/conversation_service.py` around lines 276 - 282, The current
fallback selects messages ordered ascending and limits to 20, returning the
oldest 20; instead, change the query in conversation history retrieval to order
by Message.created_at.desc() (newest first) with .limit(20), then take
result.scalars().all(), reverse that list to restore chronological order, and
finally return [{"role": m.role, "content": m.content} for m in reversed_list];
update the code around the select(Message) ... .order_by(...) and the
result.scalars().all() handling so the agent sees the most recent window of
messages in chronological order.
| onNotebookSelect={setSelectedNotebook} | ||
| notebookLabel={th("notebook")} | ||
| notebookEmptyLabel={tn("empty")} | ||
| clearNotebookLabel="清除笔记本限制" |
There was a problem hiding this comment.
Hardcoded Chinese string breaks i18n.
"清除笔记本限制" should use a translation key for consistency with the rest of the internationalized UI. The chat.json files should have this string defined.
♻️ Suggested fix
- clearNotebookLabel="清除笔记本限制"
+ clearNotebookLabel={t("clearNotebookFilter")}Then add the corresponding keys to web/messages/en.json and web/messages/zh.json:
"clearNotebookFilter": "Clear notebook filter" // en.json
"clearNotebookFilter": "清除笔记本限制" // zh.json📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| clearNotebookLabel="清除笔记本限制" | |
| clearNotebookLabel={t("clearNotebookFilter")} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/home/home-qa.tsx` at line 108, Replace the hardcoded
Chinese label clearNotebookLabel="清除笔记本限制" in the HomeQA component with a
translated string (e.g., clearNotebookLabel={t('clearNotebookFilter')}) using
the existing i18n helper (t) or useTranslation hook used elsewhere in this
component; then add the "clearNotebookFilter" key to the locale message files
used by the app with values "Clear notebook filter" (English) and "清除笔记本限制"
(Chinese) so the label is served from translations rather than hardcoded text.
| async start(query, notebookId, mode) { | ||
| _abortController?.abort(); | ||
|
|
||
| set({ | ||
| query, | ||
| notebookId, | ||
| mode, | ||
| progress: makeInitialProgress(mode), | ||
| reportTokens: "", | ||
| isActive: true, | ||
| eventIndex: 0, | ||
| taskId: null, | ||
| conversationId: null, | ||
| }); | ||
|
|
||
| const { taskId, conversationId } = await createDeepResearch(query, { | ||
| notebookId, | ||
| mode, | ||
| }); | ||
| set({ taskId, conversationId }); | ||
|
|
||
| subscribeTo(taskId, get, set); | ||
| }, |
There was a problem hiding this comment.
Rollback optimistic state if task creation fails.
start() marks the store active before createDeepResearch() runs, but a rejected create leaves isActive: true and an initial progress with no taskId. The hook catches the error, so this stale store state is what keeps the UI looking like a live task that can never reconnect.
🛠️ Suggested rollback
async start(query, notebookId, mode) {
_abortController?.abort();
set({
query,
notebookId,
mode,
progress: makeInitialProgress(mode),
reportTokens: "",
isActive: true,
eventIndex: 0,
taskId: null,
conversationId: null,
});
- const { taskId, conversationId } = await createDeepResearch(query, {
- notebookId,
- mode,
- });
- set({ taskId, conversationId });
-
- subscribeTo(taskId, get, set);
+ try {
+ const { taskId, conversationId } = await createDeepResearch(query, {
+ notebookId,
+ mode,
+ });
+ set({ taskId, conversationId });
+ void subscribeTo(taskId, get, set);
+ } catch (error) {
+ set({
+ taskId: null,
+ conversationId: null,
+ progress: null,
+ reportTokens: "",
+ isActive: false,
+ eventIndex: 0,
+ });
+ throw error;
+ }
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/store/use-deep-research-store.ts` around lines 210 - 232, start()
sets optimistic active state before awaiting createDeepResearch(), so if
createDeepResearch() rejects the store remains stale; wrap the call to
createDeepResearch(query, { notebookId, mode }) in a try/catch and on error
rollback the optimistic fields (reset isActive to false, clear taskId,
conversationId, revert progress to null or previous state from get(), and ensure
eventIndex/reportTokens are cleared) and rethrow or handle the error; keep
subscribeTo(taskId, get, set) only after successful creation and setting of
taskId/conversationId so you never subscribe when creation fails.
- 空字符串返回 ("", "") 而非 ("（无内容）", "")
- valid JSON 有 counterpoint 但无 finding 时，finding 回退到 raw 文本
- valid JSON finding 字段值不再截断到 max_chars
- 绝对 fallback 返回 "" 而非 "（提取失败）"
Made-with: Cursor
这些文件虽在 .gitignore 中，但历史上已被追踪。 统一 git rm --cached 清理，后续不再入库。 Made-with: Cursor
覆盖 5 条提取路径： - Path 1: 完整 JSON parse（有/无 counterpoint、缺 finding、超长值不截断） - Path 2: 嵌入式 JSON（prose 中、markdown fence 包裹） - Path 3: 严格正则（转义引号） - Path 5: 原始文本回退（空串、纯文本、截断边界） - EdgeCases: 返回类型、数字转字符串、多行、max_chars=0 Made-with: Cursor
当输入是残缺 JSON（以 {"finding" 开头但无法解析）时，
fallback 路径应剥去 JSON 脚手架后返回剩余文本，
而不是原样返回带 {"finding" 前缀的字符串。
Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
api/app/agents/research/deep_research.py (6)
34-34: Unused constantMAX_QUERIES_PER_DIM.This constant is defined but never referenced—
cfg.queries_per_dimfromModeConfigis used instead. Consider removing it to avoid confusion.Proposed fix
RAG_THRESHOLD = 0.50 -MAX_QUERIES_PER_DIM = 2 # cap queries per dimension to avoid runaway cost LEARNING_MAX_CHARS = 200🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/agents/research/deep_research.py` at line 34, Remove the unused constant MAX_QUERIES_PER_DIM: locate its definition and delete it, and ensure all code paths use cfg.queries_per_dim from ModeConfig (already in use) so no other changes are needed; if any future references are intended, replace them with cfg.queries_per_dim to avoid duplication and confusion.
227-236: Rename ambiguous variableltolearning.The single-letter
lis easily confused with1orI. Static analysis (E741) flags this for readability.Proposed fix
def compute_evidence_strength(learnings: list[dict]) -> str: """Aggregate evidence strength across all learnings.""" - total = sum(len(l.get("citations", [])) for l in learnings) - has_web = any(c.get("type") == "web" for l in learnings for c in l.get("citations", [])) - has_internal = any(c.get("type") == "internal" for l in learnings for c in l.get("citations", [])) + total = sum(len(learning.get("citations", [])) for learning in learnings) + has_web = any(c.get("type") == "web" for learning in learnings for c in learning.get("citations", [])) + has_internal = any(c.get("type") == "internal" for learning in learnings for c in learning.get("citations", [])) if total >= 6 and has_web and has_internal: return "high"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/agents/research/deep_research.py` around lines 227 - 236, The variable name `l` in compute_evidence_strength is ambiguous and flagged by static analysis; rename it to a clearer identifier like `learning` wherever used (in the generator expressions for total, has_web, and has_internal) to improve readability and satisfy E741, ensuring all references to the old `l` within the function body are updated consistently.
555-555: Same ambiguouslvariable issue.Consistent with the earlier recommendation, rename
ltolearningfor readability.Proposed fix
- all_citations_count = sum(len(l.get("citations", [])) for l in learnings) + all_citations_count = sum(len(learning.get("citations", [])) for learning in learnings)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/agents/research/deep_research.py` at line 555, The generator expression computing all_citations_count uses an ambiguous loop variable `l`; change it to a descriptive name (e.g., `learning`) so the expression becomes sum(len(learning.get("citations", [])) for learning in learnings) — update the variable in the expression that assigns to all_citations_count and any identical short-variable uses nearby (referencing the all_citations_count assignment and the learnings iterable).
487-494: Additionallvariable usage to rename.This comprehension in
_synthesize_reportalso usesl.Proposed fix
learnings_text = "\n\n".join( - f"[{DIMENSION_LABELS.get(l.get('dimension', 'concept'), '研究')}] " - f"查询：{l['sub_question']}\n" - f"发现 [{l.get('evidence_grade', 'weak')}]：{l['content']}" - + (f"\n反例/风险：{l['counterpoint']}" if l.get("counterpoint") else "") - for l in learnings - if l.get("content") and l.get("content") != "未找到相关信息" + f"[{DIMENSION_LABELS.get(learning.get('dimension', 'concept'), '研究')}] " + f"查询：{learning['sub_question']}\n" + f"发现 [{learning.get('evidence_grade', 'weak')}]：{learning['content']}" + + (f"\n反例/风险：{learning['counterpoint']}" if learning.get("counterpoint") else "") + for learning in learnings + if learning.get("content") and learning.get("content") != "未找到相关信息" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/agents/research/deep_research.py` around lines 487 - 494, The list comprehension in _synthesize_report uses the single-letter variable name "l" which duplicates other uses; rename the loop variable to something descriptive (e.g., "learning" or "entry") and update all references inside the comprehension (learning.get(...), learning['sub_question'], learning['content'], learning.get('counterpoint'), etc.) so the comprehension building learnings_text uses the new variable name consistently and avoids shadowing/duplicate usage.
241-242: Misleading function name:web_search_syncis actually async.The
_syncsuffix suggests synchronous execution, but this is anasync deffunction usinghttpx.AsyncClient. Consider renaming toweb_searchorweb_search_asyncfor clarity.Proposed fix
-async def web_search_sync(query: str, tavily_api_key: str, max_results: int = 4) -> list[dict]: - """Call Tavily synchronously. Does NOT persist sources to DB.""" +async def web_search(query: str, tavily_api_key: str, max_results: int = 4) -> list[dict]: + """Call Tavily asynchronously. Does NOT persist sources to DB."""Also update the call site at line 421:
- web_results = await web_search_sync(query, tavily_api_key, max_results=max_web_results) + web_results = await web_search(query, tavily_api_key, max_results=max_web_results)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/agents/research/deep_research.py` around lines 241 - 242, The function web_search_sync is misleading because it's declared async and uses httpx.AsyncClient; rename the function (e.g., to web_search_async) and update its docstring accordingly, then update every call site and any imports/references that use web_search_sync to the new name (search for web_search_sync and references to httpx.AsyncClient in this module to locate usages), keeping the same signature and behavior to avoid breaking callers.
686-686: Renameltolearninghere as well.Proposed fix
- all_citations = [c for l in state["learnings"] for c in l.get("citations", [])] + all_citations = [c for learning in state["learnings"] for c in learning.get("citations", [])]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/agents/research/deep_research.py` at line 686, The list comprehension that builds all_citations uses a terse loop variable name `l`; rename it to `learning` for clarity by changing the comprehension `all_citations = [c for l in state["learnings"] for c in l.get("citations", [])]` to use `learning` (i.e., iterate `for learning in state["learnings"]` and call `learning.get("citations", [])`) — update any nearby uses in the same expression to match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/app/agents/research/deep_research.py`:
- Around line 437-438: Replace the silent "except Exception: pass" in
api/app/agents/research/deep_research.py with an exception handler that logs the
failure and exception details (e.g., change to "except Exception as e:" and call
the appropriate logger such as logger.warning(..., exc_info=True) or
self.logger.exception(...)) so web search errors (rate limits, auth, config) are
recorded for diagnosis while keeping execution flow intact.
---
Nitpick comments:
In `@api/app/agents/research/deep_research.py`:
- Line 34: Remove the unused constant MAX_QUERIES_PER_DIM: locate its definition
and delete it, and ensure all code paths use cfg.queries_per_dim from ModeConfig
(already in use) so no other changes are needed; if any future references are
intended, replace them with cfg.queries_per_dim to avoid duplication and
confusion.
- Around line 227-236: The variable name `l` in compute_evidence_strength is
ambiguous and flagged by static analysis; rename it to a clearer identifier like
`learning` wherever used (in the generator expressions for total, has_web, and
has_internal) to improve readability and satisfy E741, ensuring all references
to the old `l` within the function body are updated consistently.
- Line 555: The generator expression computing all_citations_count uses an
ambiguous loop variable `l`; change it to a descriptive name (e.g., `learning`)
so the expression becomes sum(len(learning.get("citations", [])) for learning in
learnings) — update the variable in the expression that assigns to
all_citations_count and any identical short-variable uses nearby (referencing
the all_citations_count assignment and the learnings iterable).
- Around line 487-494: The list comprehension in _synthesize_report uses the
single-letter variable name "l" which duplicates other uses; rename the loop
variable to something descriptive (e.g., "learning" or "entry") and update all
references inside the comprehension (learning.get(...),
learning['sub_question'], learning['content'], learning.get('counterpoint'),
etc.) so the comprehension building learnings_text uses the new variable name
consistently and avoids shadowing/duplicate usage.
- Around line 241-242: The function web_search_sync is misleading because it's
declared async and uses httpx.AsyncClient; rename the function (e.g., to
web_search_async) and update its docstring accordingly, then update every call
site and any imports/references that use web_search_sync to the new name (search
for web_search_sync and references to httpx.AsyncClient in this module to locate
usages), keeping the same signature and behavior to avoid breaking callers.
- Line 686: The list comprehension that builds all_citations uses a terse loop
variable name `l`; rename it to `learning` for clarity by changing the
comprehension `all_citations = [c for l in state["learnings"] for c in
l.get("citations", [])]` to use `learning` (i.e., iterate `for learning in
state["learnings"]` and call `learning.get("citations", [])`) — update any
nearby uses in the same expression to match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: de4955d2-cdfb-45ba-be60-045f76be1767
⛔ Files ignored due to path filters (121)
api/alembic/__pycache__/env.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/001_initial_schema.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/002_memory_tables.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/003_notebook_is_global.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/004_memory_v2.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/005_restore_user_memory_unique_index.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/006_skills.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/007_conversation_summary.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/008_source_storage_key.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/009_message_agent_steps.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/010_single_user_auth.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/011_user_avatar.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/012_clerk_id_nullable.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/013_memory_doc.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/014_chunks_fts_index.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/015_oauth_ids.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/016_oauth_unbound.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/017_memory_embedding_and_eval.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/018_message_feedbacks.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/019_message_attachments.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/020_note_as_source.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/021_scheduled_tasks.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/022_notebook_public.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/023_knowledge_graph.cpython-312.pycis excluded by!**/*.pycapi/alembic/versions/__pycache__/bcd2ff1941ab_add_note_word_count.cpython-312.pycis excluded by!**/*.pycapi/app/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/__pycache__/auth.cpython-312.pycis excluded by!**/*.pycapi/app/__pycache__/config.cpython-312.pycis excluded by!**/*.pycapi/app/__pycache__/database.cpython-312.pycis excluded by!**/*.pycapi/app/__pycache__/dependencies.cpython-312.pycis excluded by!**/*.pycapi/app/__pycache__/exceptions.cpython-312.pycis excluded by!**/*.pycapi/app/__pycache__/main.cpython-312.pycis excluded by!**/*.pycapi/app/__pycache__/models.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/composer.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/deep_research.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/evaluation.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/file_memory.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/ingestion.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/knowledge_graph.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/memory.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/memory_extraction.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/memory_notebook.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/memory_retrieval.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/react_agent.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/reflection.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/retrieval.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/scene_detector.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/tools.cpython-312.pycis excluded by!**/*.pycapi/app/agents/__pycache__/writing.cpython-312.pycis excluded by!**/*.pycapi/app/domains/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/ai/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/ai/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/artifact/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/artifact/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/artifact/__pycache__/schemas.cpython-312.pycis excluded by!**/*.pycapi/app/domains/auth/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/config/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/config/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/conversation/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/conversation/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/conversation/__pycache__/schemas.cpython-312.pycis excluded by!**/*.pycapi/app/domains/feedback/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/knowledge/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/knowledge/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/knowledge_graph/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/knowledge_graph/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/memory/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/memory/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/note/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/note/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/note/__pycache__/schemas.cpython-312.pycis excluded by!**/*.pycapi/app/domains/notebook/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/notebook/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/notebook/__pycache__/schemas.cpython-312.pycis excluded by!**/*.pycapi/app/domains/public/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/public/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/public/__pycache__/schemas.cpython-312.pycis excluded by!**/*.pycapi/app/domains/setup/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/skill/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/skill/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/source/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/source/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/source/__pycache__/schemas.cpython-312.pycis excluded by!**/*.pycapi/app/domains/task/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/task/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/domains/task/__pycache__/schemas.cpython-312.pycis excluded by!**/*.pycapi/app/domains/upload/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/domains/upload/__pycache__/router.cpython-312.pycis excluded by!**/*.pycapi/app/providers/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/providers/__pycache__/email.cpython-312.pycis excluded by!**/*.pycapi/app/providers/__pycache__/embedding.cpython-312.pycis excluded by!**/*.pycapi/app/providers/__pycache__/llm.cpython-312.pycis excluded by!**/*.pycapi/app/providers/__pycache__/perplexity.cpython-312.pycis excluded by!**/*.pycapi/app/providers/__pycache__/storage.cpython-312.pycis excluded by!**/*.pycapi/app/providers/__pycache__/tavily.cpython-312.pycis excluded by!**/*.pycapi/app/schemas/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/schemas/__pycache__/response.cpython-312.pycis excluded by!**/*.pycapi/app/skills/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/skills/__pycache__/base.cpython-312.pycis excluded by!**/*.pycapi/app/skills/__pycache__/registry.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/compare_sources.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/create_note.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/deep_read.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/mind_map.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/scheduled_task.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/search_knowledge.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/summarize.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/update_memory_doc.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/update_preference.cpython-312.pycis excluded by!**/*.pycapi/app/skills/builtin/__pycache__/web_search.cpython-312.pycis excluded by!**/*.pycapi/app/utils/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/utils/__pycache__/cron.cpython-312.pycis excluded by!**/*.pycapi/app/utils/__pycache__/markdown_email.cpython-312.pycis excluded by!**/*.pycapi/app/workers/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/app/workers/__pycache__/tasks.cpython-312.pycis excluded by!**/*.pycapi/scripts/__pycache__/__init__.cpython-312.pycis excluded by!**/*.pycapi/scripts/__pycache__/backfill_memory_embeddings.cpython-312.pycis excluded by!**/*.pycapi/scripts/__pycache__/delete_duplicate_user.cpython-312.pycis excluded by!**/*.pycapi/scripts/__pycache__/reset_all_data.cpython-312.pycis excluded by!**/*.pyc
📒 Files selected for processing (1)
api/app/agents/research/deep_research.py
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Silent exception swallowing hides web search failures.
The except Exception: pass block silently discards all errors from web search, including API rate limits, authentication failures, or configuration issues. At minimum, log a warning so operators can diagnose issues.
Proposed fix
- except Exception:
- pass
+ except Exception as exc:
+ _extract_log.warning("Web search failed for %r: %s", query, exc)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except Exception: | |
| pass | |
| except Exception as exc: | |
| _extract_log.warning("Web search failed for %r: %s", query, exc) |
🧰 Tools
🪛 Ruff (0.15.6)
[error] 437-438: try-except-pass detected, consider logging the exception
(S110)
[warning] 437-437: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/agents/research/deep_research.py` around lines 437 - 438, Replace the
silent "except Exception: pass" in api/app/agents/research/deep_research.py with
an exception handler that logs the failure and exception details (e.g., change
to "except Exception as e:" and call the appropriate logger such as
logger.warning(..., exc_info=True) or self.logger.exception(...)) so web search
errors (rate limits, auth, config) are recorded for diagnosis while keeping
execution flow intact.
目录结构： - tests/unit/ 纯函数，无 IO（毫秒级） - tests/integration/ 服务层，直接调 DB，无 HTTP - tests/e2e/ 完整 ASGI client 迁移： - test_auth / test_health / test_notebook / test_upload → e2e/ - test_deep_research_extract_finding → unit/ 新增： - unit/test_deep_research_utils.py _strip_fences / _try_json_dict / grade_evidence / compute_evidence_strength - unit/test_response_utils.py success() / fail() / not_configured() / ApiResponse - integration/test_conversation_service.py ConversationService CRUD + 消息持久化 - e2e/test_conversation.py 对话端点 + MessageSave + 消息列表 Made-with: Cursor
- unit: grade_evidence 单条 web → "weak"（非 "medium"），修正测试名和断言 - e2e: conversation 创建端点返回 201 非 200，helper 和测试改为 in (200, 201) - integration: list_messages 顺序在同毫秒内不稳定，改为验证内容包含而非 ID 顺序 Made-with: Cursor
Made-with: Cursor
1 participant
Summary
Type of change
Related issue
Closes #
Changes
How to test
Screenshots (if applicable)
Checklist
./lyra lintand there are no type errorsfeat:,fix:, etc.)Summary by CodeRabbit
New Features
Improvements
Removals
Tests