fix(agent): separate citations from final answers - #3182
Conversation
…ctor-scoped confirmation, and scheduled task auto-confirm
Emit structured citation metadata, keep legacy reference payloads out of visible content, and harden stream cleanup. Preserve the summary-to-preview flow with bounded animation-frame presentation and add regression coverage.
82324ee to
c6f43ac
Compare
📝 WalkthroughPurpose and implementationThis change separates user-visible final answers from citation metadata in ReAct agent responses.
Affected APIs and data
Risks
VerificationExisting coverage includes Python lifecycle, citation, and final-answer tests, plus TypeScript tests for final-answer decoding and summary presentation. Run the targeted checks: cd web
npm test
npm run test:react-final
npm run test:final-presentationRun the Python API tests with the repository’s configured Python test runner: pytest packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_agentic_data_api_lifecycle.py packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_knowledge_retrieve_citations.py packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_react_final.pyWalkthroughThe change introduces structured protocol-versioned ReAct answers with citations, lifecycle-safe streaming cleanup, canonical frontend decoding, citation-aware rendering, bounded summary presentation, and expanded backend and frontend tests. ChangesReAct final-answer flow
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (8)
web/utils/react-agent-final.test.ts (1)
176-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a protocol v2 history envelope case.
isHistoryEnvelopeacceptsversion === 1orprotocol_version === 2. The suite covers onlyversion: 1. The backend persists terminal payloads withprotocol_version: 2, so a regression in that branch would not fail any test. Add one case with{ type: 'react-agent', protocol_version: 2, final_content, citations }passed as a JSON string, which also exercises the string-parsing path indecodeHistoryAnswer.Source: Path instructions
web/hooks/use-react-agent.ts (1)
298-315: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
parseReActTextnow decodes history, and callers decode again.
parseReActTextcallsdecodeHistoryAnswer(text)at Line 306. Several callers already decode before calling it and pass the decoded content, for exampleparseContextToMessagePartsinweb/components/chat/opencode-agent-content.tsx(Line 67) andrenderHistoryTurninweb/components/chat/opencode-agent-chat-container.tsx. The second decode is idempotent, so behavior is unchanged, but the decoding responsibility is now duplicated across the seam.Pick one owner: either keep decoding inside
parseReActTextand let callers pass the raw context, or keep it in the callers and haveparseReActTextaccept already-clean text.web/components/chat/opencode-agent-content.tsx (1)
85-104: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid decoding the same context twice.
Line 87 calls
parseContextToMessageParts(context), which already runsdecodeHistoryAnswer(context)internally. Line 88 decodes the same string again only to obtain the fallback content at Line 101. Decoding runs the legacy<references>regex scan over the full message, so this doubles that cost per view message.Return the decoded content from
parseContextToMessagePartsand reuse it.♻️ Suggested shape
function parseContextToMessageParts(context: string): { parts: MessagePart[]; finalContent: string; citations: AgentCitation[]; + decodedContent: string; } { if (!context || typeof context !== 'string') { - return { parts: [], finalContent: '', citations: [] }; + return { parts: [], finalContent: '', citations: [], decodedContent: '' }; }web/new-components/chat/content/ManusRightPanel.tsx (1)
2369-2391: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNested interactive elements in the citation card.
The card wrapper uses
role='button'withtabIndex={0}, and Line 2418 renders an<a>inside it. An interactive element inside another interactive element is invalid and confuses screen readers, which announce the whole card as a single button. Keyboard users also reach the link only after the card.Move the click and key handlers to a dedicated header button, or make the card a plain container and expose selection through an explicit control.
web/components/chat/opencode-agent-chat-container.tsx (2)
141-150: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDerive
orderfrom the same source used for the append.Line 145 reads
history.lengthfrom state, but line 148 appends tohistoryRef.current. ThehandleChatcallback depends onhistory.length, so the two normally agree. They can diverge if the ref is updated byonCompletebefore React re-renders. Use the ref for both values.♻️ Proposed change
const humanMessage: IChatDialogueMessageSchema = { role: 'human', context: content, model_name: model, - order: history.length, + order: historyRef.current.length, time_stamp: Date.now(), };
233-239: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant intersection cast.
IChatDialogueMessageSchemanow declarescitations, soturn.view?.citationstypes correctly without a cast.OpenCodeChatCompletion.tsxalready reads it directly. Also note thatparseReActTextcallsdecodeHistoryAnswerinternally, so the decode at line 233 is applied twice; the second pass is a no-op but the duplication is easy to remove.♻️ Proposed change
- citations = - (turn.view as (IChatDialogueMessageSchema & { citations?: AgentCitation[] }) | undefined)?.citations ?? - historyAnswer.citations; + citations = turn.view?.citations ?? historyAnswer.citations;packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/react_final.py (1)
142-151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the allowlist from the adapter table.
_TRUSTED_CITATION_TOOLSandadapterslist the same four tool names. If one list is edited without the other,adapters[normalized_name]raisesKeyError, andKeyErroris not in the caught exception tuple at Line 150. Define one module-level mapping and derive the allowlist from its keys.♻️ Proposed refactor
-_TRUSTED_CITATION_TOOLS = { - "knowledge_retrieve", - "kb_cat", - "kb_grep", - "semantic_search", -} +# Populated after the adapters are defined; see _CITATION_ADAPTERS below.+_CITATION_ADAPTERS = { + "knowledge_retrieve": _adapt_knowledge_retrieve, + "kb_cat": _adapt_kb_cat, + "kb_grep": _adapt_kb_grep, + "semantic_search": _adapt_semantic_search, +} +_TRUSTED_CITATION_TOOLS = frozenset(_CITATION_ADAPTERS)- adapters = { - "knowledge_retrieve": _adapt_knowledge_retrieve, - "kb_cat": _adapt_kb_cat, - "kb_grep": _adapt_kb_grep, - "semantic_search": _adapt_semantic_search, - } try: - drafts = adapters[normalized_name](parsed_input, observation) + drafts = _CITATION_ADAPTERS[normalized_name](parsed_input, observation)packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/agentic_data_api.py (1)
975-987: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid a full JSON parse for every streamed event.
_react_agent_streamcalls_sse_event_typeon every event only to detect the two terminal types. Each call runsjson.loadson the whole payload, after_sse_eventalready ranjson.dumpson it. A single ReAct round emits manystep.chunkevents, andhtmlchunks can be large.Add a cheap prefilter before parsing.
♻️ Proposed refactor
def _sse_event_type(event: Any) -> Optional[str]: """Read an SSE event type without trusting arbitrary streamed text.""" if not isinstance(event, str): return None + # Terminal events are the only ones the caller acts on; skip the parse + # for the high-volume step/chunk events. + if '"final"' not in event and '"done"' not in event: + return None first_line = event.splitlines()[0] if event else ""Also applies to: 1037-1039
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2508357e-a689-4b20-b945-2cb209420bee
📒 Files selected for processing (25)
packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/agentic_data_api.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/react_final.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_agentic_data_api_lifecycle.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_knowledge_retrieve_citations.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_react_final.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tools/knowledge_retrieve.pyweb/components/chat/opencode-agent-chat-container.tsxweb/components/chat/opencode-agent-content.tsxweb/components/knowledge/embedded-chat.tsxweb/hooks/use-react-agent-chat.tsweb/hooks/use-react-agent.tsweb/new-components/chat/content/ManusLeftPanel.tsxweb/new-components/chat/content/ManusRightPanel.tsxweb/new-components/chat/content/OpenCodeChatCompletion.tsxweb/new-components/chat/content/OpenCodeSessionTurn.tsxweb/package.jsonweb/pages/construct/knowledge/index.tsxweb/pages/index.tsxweb/pages/share/[token].tsxweb/types/chat.tsweb/utils/final-presentation.test.tsweb/utils/final-presentation.tsweb/utils/react-agent-final.test.tsweb/utils/react-agent-final.tsweb/utils/react-sse-parser.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
web/**
⚙️ CodeRabbit configuration file
web/**: 这是一个使用 Next.js 13、React 18、TypeScript 和 Ant Design 的前端。
- 检查 SSR 阶段对浏览器 API 的使用、useEffect 依赖，以及 reader、timer、listener、
request 和 AbortController 的清理。- 对于 chat、Agent、SubAgent 和 SSE 代码，检查跨 chunk 解析、重复或乱序事件、
终止和错误事件、过期请求回写、函数式状态更新以及会话隔离。- 将模型或后端返回的 Markdown、HTML、SVG、URL、artifact 和文件视为不可信内容。
检查 sanitization、允许的 URL scheme、dangerouslySetInnerHTML、rehypeRaw、
window.open 和下载路径是否存在 XSS 风险。- API 路径和类型必须与后端字段、optional 和 required 字段、状态枚举以及历史记录
反序列化保持一致。正确编码动态参数，绝不能静默吞掉请求错误。- 保持英文和中文 locale key 及其插值参数同步。不要引入仅支持单一语言的硬编码
用户可见文本。- 当前构建会忽略 TypeScript 错误，并且不存在前端 test script。不要建议不存在的
npm test。对于类型或协议变更，应根据需要建议运行 yarn tsc --noEmit、只读
ESLint 和 yarn build。- CI 使用 Yarn。当 web/package.json 变更时，必须同步更新 web/yarn.lock。
若同时修改 web/package-lock.json，应说明原因，因为 CI 不使用该 lockfile。
OSV Scanner 负责检查受支持 manifest 和 lockfile 中的已知依赖漏洞。
Files:
web/pages/construct/knowledge/index.tsxweb/utils/react-agent-final.tsweb/types/chat.tsweb/package.jsonweb/new-components/chat/content/OpenCodeChatCompletion.tsxweb/utils/final-presentation.test.tsweb/new-components/chat/content/ManusLeftPanel.tsxweb/pages/share/[token].tsxweb/components/chat/opencode-agent-chat-container.tsxweb/utils/react-agent-final.test.tsweb/hooks/use-react-agent-chat.tsweb/components/chat/opencode-agent-content.tsxweb/hooks/use-react-agent.tsweb/utils/final-presentation.tsweb/pages/index.tsxweb/new-components/chat/content/ManusRightPanel.tsxweb/new-components/chat/content/OpenCodeSessionTurn.tsxweb/components/knowledge/embedded-chat.tsxweb/utils/react-sse-parser.ts
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use Python 3.10 or newer for project development.
Files:
packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_knowledge_retrieve_citations.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tools/knowledge_retrieve.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_react_final.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_agentic_data_api_lifecycle.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/react_final.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/agentic_data_api.py
packages/dbgpt-app/src/dbgpt_app/**/*.py
⚙️ CodeRabbit configuration file
packages/dbgpt-app/src/dbgpt_app/**/*.py: - dbgpt-app 是组合和启动层。共享 interface、storage abstraction 和
connector implementation 应位于更底层的 package 中。
- 审查 SystemApp 注册、初始化与关闭顺序、全局状态、配置默认值以及启动失败后的清理。
- OpenAPI endpoint、文件上传、GitHub import、skill extraction 和 artifact
download 必须防止绝对路径、父目录遍历、symlink escape、Zip Slip 和 Tar Slip。
必须限制文件数量、单文件大小和总大小。- 不可信的 Python、shell 和任意代码执行必须使用受限 sandbox。不得回退到宿主机
exec、eval、shell=True 或不受限的文件系统访问。SQL 执行应使用 datasource
connector 层，并强制实施参数化、最小权限、适用情况下的只读访问、行数限制和 timeout。- SSE 和 asynchronous generator 必须处理断开连接、取消、timeout、错误、
terminal event、task 清理和用户数据隔离。- 保持 conversation、message、streaming event 和前端消费协议的兼容性，并为
用户可见行为提供回归测试。
Files:
packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_knowledge_retrieve_citations.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tools/knowledge_retrieve.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_react_final.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_agentic_data_api_lifecycle.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/react_final.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/agentic_data_api.py
**/{tests/**/*.py,test_*.py,*_test.py}
⚙️ CodeRabbit configuration file
**/{tests/**/*.py,test_*.py,*_test.py}: 检查 Python 测试、fixture 和测试辅助代码是否提供了有意义的回归覆盖。
- 应断言可观察行为和对外相关契约，而不是只断言 mock 调用次数或实现细节。将 mock
保持在真实 I/O 或进程边界，使被测行为本身仍会执行。- 对于 bug 修复，聚焦的回归测试应在旧行为上失败。对于安全修复，在能够安全测试时，
应包含一个具体的绕过方式或恶意输入场景。- 当这些场景与变更的生产路径相关时，覆盖错误、空输入、边界、清理，以及异步取消
或 timeout 行为；不要要求与变更无关的穷尽式覆盖。- 保持单元测试确定且隔离，不依赖真实网络、外部模型、数据库、GPU、Docker、
wall-clock time 或开发者机器状态。确实需要这些资源的测试应位于 integration test
中或明确标记为 integration test，说明其前置条件，并清理创建的资源。- 如果可以使用 event、mock clock、同步原语或有界不变量验证行为，应避免使用 sleep
和脆弱的性能阈值。- 对于聚焦验证，建议运行 uv run pytest 并指定相关测试路径。不要假设 make test 会运行
dbgpt_app、dbgpt_serve、dbgpt_ext、dbgpt_client 或 dbgpt_sandbox 的测试；其当前
target 运行的是 dbgpt package 测试。不要将 make fmt-check 描述为只读命令，因为
当前 target 会调用 ruff check --fix。
Files:
packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_knowledge_retrieve_citations.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_react_final.pypackages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_agentic_data_api_lifecycle.py
🪛 OpenGrep (1.26.0)
web/utils/react-agent-final.ts
[ERROR] 223-223: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
web/pages/index.tsx
[ERROR] 1452-1452: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🪛 Ruff (0.16.1)
packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_agentic_data_api_lifecycle.py
[warning] 111-111: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
[warning] 111-111: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/agentic_data_api.py
[warning] 1046-1046: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
[warning] 1046-1046: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
🔇 Additional comments (65)
web/package.json (1)
21-24: LGTM!web/pages/construct/knowledge/index.tsx (1)
171-171: LGTM!web/utils/final-presentation.test.ts (1)
1-201: LGTM!web/utils/react-agent-final.ts (8)
9-60: LGTM!
77-110: LGTM!
112-129: LGTM!
131-200: LGTM!
202-263: LGTM!
202-209: LGTM!
265-276: LGTM!
278-305: LGTM!web/utils/react-agent-final.test.ts (1)
6-174: LGTM!Also applies to: 206-243
web/utils/react-sse-parser.ts (1)
13-18: LGTM!Also applies to: 53-54, 144-144, 255-255, 372-373, 414-427
web/hooks/use-react-agent.ts (1)
9-9: LGTM!Also applies to: 26-27, 38-39, 58-58, 67-74, 98-104, 126-155, 170-170, 202-202, 393-399
web/components/chat/opencode-agent-content.tsx (1)
14-14: LGTM!Also applies to: 24-25, 42-73, 105-113, 125-125, 136-143, 169-169, 196-196, 208-208, 223-223, 232-239
web/pages/index.tsx (3)
1675-1725: Presentation lifecycle guards look correct.I traced the loop risk: when the guards fail, Line 1684 calls
cancelSummaryPresentation, which clearspendingSummaryPresentation, so the re-run exits at Line 1676.cancelSummaryPresentationalso early-returns when the refs are already null. The effect terminates in both paths.
31-32: LGTM!Also applies to: 184-184, 629-667, 839-860, 1605-1673, 1738-1739, 1853-1856, 2251-2295, 2396-2429, 2513-2514, 2553-2553, 2563-2568, 2588-2588, 2773-2777, 2832-2843, 3590-3605
1348-1603: 🚀 Performance & ScalabilityNo change needed.
shell_interpretertruncates forwarded output at 50,000 characters before returning chunks, so the artifact scan is bounded by the backend limit.RegExp.prototype.execis not a child-process call.web/pages/share/[token].tsx (1)
20-20: LGTM!Also applies to: 57-57, 74-74, 250-258, 493-493, 786-794
web/new-components/chat/content/ManusLeftPanel.tsx (1)
109-110: LGTM!Also applies to: 147-150, 1062-1062, 1082-1083, 1270-1275
web/hooks/use-react-agent-chat.ts (3)
97-103: LGTM!
260-276: LGTM!
114-115: 🎯 Functional CorrectnessNo change needed.
web/types/chat.ts (1)
1-1: LGTM!Also applies to: 107-108
web/components/chat/opencode-agent-chat-container.tsx (1)
66-103: LGTM!web/new-components/chat/content/OpenCodeChatCompletion.tsx (1)
140-156: LGTM!Also applies to: 171-171, 195-195
web/new-components/chat/content/OpenCodeSessionTurn.tsx (1)
496-507: LGTM!web/components/knowledge/embedded-chat.tsx (2)
158-168: LGTM!Also applies to: 278-307
336-336: 🩺 Stability & AvailabilityNo issue. The referenced
/icons/knowledge.pngasset exists underweb/public.web/utils/final-presentation.ts (2)
50-56: LGTM!Also applies to: 124-169
104-108: 🩺 Stability & AvailabilityNo change needed.
summaryPresentationRef.currentalready references the presentation beforepresentation.start()returns, and the empty-summary path has no remaining caller to reorder.> Likely an incorrect or invalid review comment.packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/react_final.py (12)
10-38: LGTM!
41-96: LGTM!
99-106: LGTM!
155-207: LGTM!
210-230: LGTM!
233-291: LGTM!
294-337: LGTM!
340-398: LGTM!
401-436: LGTM!
439-474: LGTM!
477-504: LGTM!
16-21: 🩺 Stability & AvailabilityNo change needed for trusted citation tools.
make_kb_semantic_search()registers the wrapper with the namesemantic_search, so the allowlist and adapter lookup match the registered tool name.> Likely an incorrect or invalid review comment.packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tools/knowledge_retrieve.py (4)
4-4: LGTM!Also applies to: 27-28, 117-118, 130-131
43-56: LGTM!
57-91: LGTM!
103-107: LGTM!packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_react_final.py (5)
14-26: LGTM!Also applies to: 81-117
66-78: LGTM!Also applies to: 120-184
187-270: LGTM!
273-295: LGTM!
324-355: LGTM!packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/agentic_data_api.py (7)
1-1: LGTM!Also applies to: 34-34
990-1006: LGTM!
1023-1053: LGTM!
2796-2808: LGTM!
3181-3225: LGTM!
3254-3270: LGTM!Also applies to: 3322-3344, 3569-3569, 3610-3610
1056-1070: 🩺 Stability & AvailabilityNo change needed for
body_iterator.
body_iteratoris a coreStreamingResponseattribute across supported Starlette versions, so this does not needgetattr(self, "body_iterator", None).> Likely an incorrect or invalid review comment.packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_agentic_data_api_lifecycle.py (3)
18-44: LGTM!
86-142: LGTM!
145-200: LGTM!packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_knowledge_retrieve_citations.py (3)
13-56: LGTM!
73-93: LGTM!
96-131: LGTM!
| async def _cancel_and_await_agent_task(task: "asyncio.Task[Any]") -> None: | ||
| """Cancel a running agent task and always consume its terminal result.""" | ||
| was_done = task.done() | ||
| if not was_done: | ||
| task.cancel() | ||
| try: | ||
| await task | ||
| except asyncio.CancelledError: | ||
| pass | ||
| except Exception: | ||
| if not was_done: | ||
| logger.exception("ReAct agent task failed during stream cleanup") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the declared Python target; asyncio.Task.cancelling() needs 3.11+.
fd -t f -g 'pyproject.toml' -x rg -n 'requires-python|target-version' {}
fd -t f -g '.python-version' -x cat {}
# Look for an existing convention for cancellation-safe cleanup in this repo.
rg -nP --type=py -C3 '\bexcept asyncio\.CancelledError\b' -g '!**/tests/**' | head -60
rg -nP --type=py -C2 '\bcancelling\(\)|asyncio\.shield\b'Repository: eosphoros-ai/DB-GPT
Length of output: 669
🏁 Script executed:
#!/bin/bash
set -eu
FILE="packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/agentic_data_api.py"
echo "FILE=$FILE"
wc -l "$FILE"
echo "--- outline around helper ---"
ast-grep outline "$FILE" --match _cancel_and_await_agent_task --view expanded || true
echo "--- helper and caller area ---A---"
sed -n '960,1050p' "$FILE"
echo "--- caller calls to helper ---"
rg -n '_cancel_and_await_agent_task|def _react_agent_stream|final|aclose|aclose'\''|CancelledError|AgentStreamingResponse' "$FILE" -C 4
echo "--- test files mentioning cancellation/helper ---"
rg -n '_cancel_and_await_agent_task|stream cleanup|aclose|agent_task|CancelledError|disconnect|AgentStreamingResponse' packages/dbgpt-app -g '*.py' -C 3 | head -220Repository: eosphoros-ai/DB-GPT
Length of output: 37730
🏁 Script executed:
#!/bin/bash
set -eu
echo "--- lifecycle disconnect test relevant section ---"
sed -n '1,235p' packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_agentic_data_api_lifecycle.py
echo "--- asyncio CancelledError stacking behavior probe ---"
python3 - <<'PY'
import asyncio
async def _cancel_and_await_like(task: asyncio.Task) -> None:
# Mirrors the current helper except it does not cancel here so the
# exception source is unambiguous for this probe.
try:
await task
except asyncio.CancelledError:
print("helper caught CancelledError")
async def stream():
# The outer task has cancellation pending.
await asyncio.sleep(0)
await asyncio.current_task().cancel()
await asyncio.sleep(0)
try:
task = asyncio.create_task(_cancel_and_await_like(asyncio.create_task(asyncio.sleep(0.1))))
await task
except asyncio.CancelledError:
raise
else:
print("outer task completed; cancellation did not propagate")
try:
asyncio.run(stream())
except asyncio.CancelledError:
print("outer CancelledError propagated")
PYRepository: eosphoros-ai/DB-GPT
Length of output: 6692
🏁 Script executed:
#!/bin/bash
set -eu
echo "--- run cancellation stacking probe with runtime available ---"
python3 - <<'PY'
import asyncio
import sys
print("python", sys.version)
async def current():
return asyncio.current_task()
outer_cause = None
inner_cause = None
async def consume_task(task: asyncio.Task):
global inner_cause
try:
await task
except asyncio.CancelledError as exc:
inner_cause = exc
async def outer_cancel_path():
global outer_cause
t = await current()
t.cancel()
await task
async def shutdown_before_cancel_task():
global outer_cause, task, task2
task = asyncio.create_task(asyncio.sleep(1000))
task2 = asyncio.create_task(outer_cancel_path());
try:
await task2
except asyncio.CancelledError as exc:
outer_cause = exc
print("outer cancelled by", type(exc), exc.__class__)
print("outer cancelled message", getattr(exc, 'message', None))
# current task should be current task because in cancel task, await cancels
await asyncio.sleep(0)
print("current is same?", asyncio.current_task() is t)
else:
print("outer did not cancel")
# This is the exact helper path as if _cancel_and_await_agent_task(task2).
async def helper():
await consume_task(task2) # task2 has already gotten CancelledError
print("helper completed")
async def drive():
await helper()
print("drive completed")
asyncio.run(shutdown_before_cancel_task())
PY
echo "--- behavioral probe for helper swallowing an already-cancelled inner task ---"
python3 - <<'PY'
import asyncio
helper_could_continue = False
async def inner():
await asyncio.wait_for(asyncio.Event().wait(), timeout=10)
async def helper(task: asyncio.Task):
if task.done():
pass
else:
task.cancel()
try:
await task
except asyncio.CancelledError:
print("helper caught CancelledError")
if task.cancelling() == 0:
print("helper source: inner task")
else:
print("helper source: current task has pending cancelling", task.cancelling())
await asyncio.sleep(0) # prove helper execution can continue after swallow
print("helper continued via extra await")
raise asyncio.CancelledError("outer")
async def drive():
task = asyncio.create_task(inner())
print("task has pending", task.cancelling())
outer_task = asyncio.create_task(helper(task))
await asyncio.sleep(0)
print("helper before outer cancel", outer_task.done())
outer_task.cancel()
await asyncio.sleep(0)
print("outer_cancelled_result", outer_task.cancelled())
try:
await outer_task
except asyncio.CancelledError as e:
print("outer CancelledError propagated from helper", e)
asyncio.run(drive())
PY
echo "--- async context manager cancellation behavior probe ---"
python3 - []
PY
import asyncio
outer_continued = False
async def run():
with asyncio.Runner() as runner:
task = runner.create_task(
asyncio.create_task(asyncio.sleep(1000))
)
task.cancel()
try:
await asyncio.wait_for(asyncio.Event().wait(), timeout=10)
except asyncio.CancelledError:
print(f"runner task CancelledError: message={getattr(asyncio.current_task().get_cancelled_exc(), 'message', None)}")
await asyncio.sleep(0)
global outer_continued
outer_continued = True
print("run outer continued")
asyncio.run(run())
PYRepository: eosphoros-ai/DB-GPT
Length of output: 1443
🏁 Script executed:
#!/bin/bash
set -eu
echo "--- current environment Python ---"
python3 --version
echo "--- async cancellation propagation probe ---"
python3 - <<'PY'
import asyncio
result = {}
async def inner_task():
await asyncio.Event().wait()
async def helper(task, outer_cause):
try:
await task
except asyncio.CancelledError as exc:
result["inner_cause"] = exc
result["inner_cause_message"] = getattr(exc, "message", None)
print("helper caught CancelledError")
async def driver():
task = asyncio.create_task(inner_task())
outer_task = asyncio.create_task(helper(task, None))
task.cancel()
await asyncio.sleep(0)
print("outer_task done before outer cancellation?", outer_task.done())
print("outer_task cancelled before outer cancellation?", outer_task.cancelled())
outer_task.cancel()
print("outer_task done after outer cancellation?", outer_task.done())
print("outer_task cancelled after outer cancellation?", outer_task.cancelled())
try:
await outer_task
except asyncio.CancelledError as exc:
result["outer_cause"] = exc
result["outer_cause_message"] = getattr(exc, "message", None)
asyncio.run(driver())
print(result)
PY
echo "--- cancellation-only helper behavior ---"
python3 - <<'PY'
import asyncio
async def inner_task():
await asyncio.Event().wait()
async def helper(task):
if not task.done():
task.cancel()
try:
await task
except asyncio.CancelledError:
pass
async def driver():
task = asyncio.create_task(inner_task())
try:
await helper(task)
print("outer task completed after helper swallowed CancelledError")
except asyncio.CancelledError:
print("outer CancelledError propagated")
asyncio.run(driver())
PYRepository: eosphoros-ai/DB-GPT
Length of output: 646
Re-raise uncancelled CancelledError during cleanup
Line 1016 swallows CancelledError without distinguishing whether it came from task.cancel() or from the enclosing task. If this awaits during an outer ASGI task cancellation, it will complete the finally instead of propagating the cancellation. Only swallow CancelledError when the completed task itself was already cancelled.
| import asyncio | ||
| import json | ||
| from types import SimpleNamespace | ||
|
|
||
| import pytest | ||
|
|
||
| from dbgpt_app.openapi.api_v1 import agentic_data_api | ||
| from dbgpt_app.openapi.api_v1.react_final import AgentFinalAnswer | ||
|
|
||
|
|
||
| def _decode_sse_event(event: str): | ||
| assert event.startswith("data: ") | ||
| return json.loads(event.removeprefix("data: ").strip()) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# pytest-asyncio dependency and mode configuration.
fd -t f -g 'pyproject.toml' -x rg -n 'pytest-asyncio|asyncio_mode|\[tool\.pytest' -H {}
fd -t f -g 'pytest.ini' -o -g 'setup.cfg' -o -g 'tox.ini' -x rg -n 'asyncio' -H {}
# Which target runs the dbgpt_app tests.
rg -nP -C3 '(^|\s)test:' MakefileRepository: eosphoros-ai/DB-GPT
Length of output: 616
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== root pyproject tool.pytest section =="
sed -n '55,70p' pyproject.toml
echo
echo "== dbgpt-app pyproject references =="
fd -t f -g 'pyproject.toml' packages/dbgpt-app -x sh -c 'echo "--- $1"; rg -n "pytest|pytest-asyncio|asyncio_mode|tool.pytest|uv|dependencies" "$1"' sh {}
echo
echo "== all pytest.ini option markers =="
fd -t f -g 'pytest.ini' .
echo
echo "== test file metadata and async mark =="
wc -l packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_agentic_data_api_lifecycle.py
sed -n '1,110p' packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_agentic_data_api_lifecycle.py
echo
echo "== pytest-asyncio declarations across repo =="
rg -n '"pytest-asyncio|pytest-asyncio>=|pytest-asyncio|asyncio_mode|pytest\.addopts|marker="|pytestmark' pyproject.toml packages -g 'pyproject.toml' -g 'pytest.ini' || trueRepository: eosphoros-ai/DB-GPT
Length of output: 4178
Add pytest-asyncio coverage for the new async tests.
@pytest.mark.asyncio requires pytest-asyncio, but packages/dbgpt-app has no test dependency on it and the tracked make test target runs pytest --pyargs dbgpt, not the async tests under dbgpt-app. Add pytest-asyncio to the app/test dependencies with a supported configuration, or expose a target that runs these dbgpt-app tests.
Source: Path instructions
| assert await anext(stream) == "data: first\n\n" | ||
|
|
||
| await stream.aclose() | ||
|
|
||
| assert len(created_tasks) == 1 | ||
| assert created_tasks[0].cancelled() | ||
| assert created_tasks[0].done() | ||
| assert task_finished.is_set() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Bound the aclose() await so a regression fails instead of hanging.
_agent_work blocks on an asyncio.Event that is never set. If _cancel_and_await_agent_task regresses and stops cancelling the task, Line 78 waits forever and CI hangs rather than reporting a failure. test_response_disconnect_closes_stream_and_agent_task already wraps its await in asyncio.wait_for with a one-second timeout. Apply the same bound here.
💚 Proposed fix
- await stream.aclose()
+ await asyncio.wait_for(stream.aclose(), timeout=1)As per path instructions: "保持单元测试确定且隔离".
📝 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.
| assert await anext(stream) == "data: first\n\n" | |
| await stream.aclose() | |
| assert len(created_tasks) == 1 | |
| assert created_tasks[0].cancelled() | |
| assert created_tasks[0].done() | |
| assert task_finished.is_set() | |
| assert await anext(stream) == "data: first\n\n" | |
| await asyncio.wait_for(stream.aclose(), timeout=1) | |
| assert len(created_tasks) == 1 | |
| assert created_tasks[0].cancelled() | |
| assert created_tasks[0].done() | |
| assert task_finished.is_set() |
Source: Path instructions
| assert result["chunks"][0]["content"] == "No relevant information found" | ||
| assert result.get("citations", []) == [] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Assert the citations key is present, not merely absent-or-empty.
Line 70 uses result.get("citations", []), so the assertion passes even when the key is missing. The change under test adds "citations": [] to this branch, so this test does not fail on the previous behavior.
Key presence is also contractual: _adapt_knowledge_retrieve in react_final.py selects the explicit-citations branch with if "citations" in parsed. Index the key directly.
💚 Proposed fix
assert result["chunks"][0]["content"] == "No relevant information found"
- assert result.get("citations", []) == []
+ assert result["citations"] == []As per path instructions: "对于 bug 修复，聚焦的回归测试应在旧行为上失败".
📝 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.
| assert result["chunks"][0]["content"] == "No relevant information found" | |
| assert result.get("citations", []) == [] | |
| assert result["chunks"][0]["content"] == "No relevant information found" | |
| assert result["citations"] == [] |
Source: Path instructions
| assert payload == { | ||
| "type": "final", | ||
| "protocol_version": 2, | ||
| "content": "The final answer.", | ||
| "citations": [ | ||
| { | ||
| "index": 1, | ||
| "id": payload["citations"][0]["id"], | ||
| "sourceName": "docs/design.md", | ||
| "excerpt": ( | ||
| "docs/design.md (markdown, 20 lines)\n" | ||
| " 4 | The answer is supported by this design.\n" | ||
| " 5 | More supporting detail." | ||
| ), | ||
| "score": None, | ||
| "path": "docs/design.md", | ||
| "url": None, | ||
| } | ||
| ], | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert the citation id, do not copy it from the payload.
Line 51 reads payload["citations"][0]["id"] into the expected dict. The id field is therefore never checked. _stable_citation_id is deterministic, and the frontend keys citations by id. A change to the id scheme, or an empty id, would still pass this test.
Assert the id format explicitly.
💚 Proposed fix
payload = answer.to_sse_payload()
+ citation_id = payload["citations"][0]["id"]
+ assert re.fullmatch(r"citation-[0-9a-f]{16}", citation_id)
assert payload == {
"type": "final",
"protocol_version": 2,
"content": "The final answer.",
"citations": [
{
"index": 1,
- "id": payload["citations"][0]["id"],
+ "id": citation_id,As per path instructions: "应断言可观察行为和对外相关契约".
Source: Path instructions
| function toFileReferences(citations: AgentCitation[]): FileReference[] { | ||
| return citations.map(citation => ({ | ||
| id: getReferenceId(citation), | ||
| path: citation.path || citation.url || citation.sourceName, | ||
| name: citation.sourceName, | ||
| content: citation.excerpt, | ||
| status: 'completed', | ||
| })); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The path fallback makes every reference look like a file.
toFileReferences sets path to citation.sourceName when no path or url exists. Line 500 then computes isFile = Boolean(ref.path), which is now always true. The panel repeats the source name as a monospace path and shows a language tag derived from it. Keep path empty when the citation carries no location.
🐛 Proposed fix
function toFileReferences(citations: AgentCitation[]): FileReference[] {
return citations.map(citation => ({
id: getReferenceId(citation),
- path: citation.path || citation.url || citation.sourceName,
+ path: citation.path || citation.url || '',
name: citation.sourceName,
content: citation.excerpt,
status: 'completed',
}));
}The expanded-content header at line 544 also renders ref.path; consider falling back to ref.name there.
📝 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.
| function toFileReferences(citations: AgentCitation[]): FileReference[] { | |
| return citations.map(citation => ({ | |
| id: getReferenceId(citation), | |
| path: citation.path || citation.url || citation.sourceName, | |
| name: citation.sourceName, | |
| content: citation.excerpt, | |
| status: 'completed', | |
| })); | |
| function toFileReferences(citations: AgentCitation[]): FileReference[] { | |
| return citations.map(citation => ({ | |
| id: getReferenceId(citation), | |
| path: citation.path || citation.url || '', | |
| name: citation.sourceName, | |
| content: citation.excerpt, | |
| status: 'completed', | |
| })); |
|
|
||
| return ( | ||
| <div className='space-y-3' data-testid='references-panel'> | ||
| <div className='rounded-lg border border-blue-100 bg-blue-50/60 px-3 py-2 text-xs leading-5 text-blue-700 dark:border-blue-500/20 dark:bg-blue-500/10 dark:text-blue-300'> | ||
| 以下内容是回答过程中检索或读取的知识来源，不包含脚本执行回显、SQL 执行结果或其他工具输出。 | ||
| </div> | ||
| {citations.map(citation => { | ||
| const isSelected = citation.index === selectedCitationIndex; | ||
| const score = formatReferenceScore(citation.score); | ||
| const safeUrl = getSafeReferenceUrl(citation.url); | ||
| return ( | ||
| <div | ||
| key={`${citation.id}-${citation.index}`} | ||
| ref={node => { | ||
| if (node) itemRefs.current.set(citation.index, node); | ||
| else itemRefs.current.delete(citation.index); | ||
| }} | ||
| role='button' | ||
| tabIndex={0} | ||
| onClick={() => onCitationSelect?.(citation.index)} | ||
| onKeyDown={event => { | ||
| if (event.key === 'Enter' || event.key === ' ') { | ||
| event.preventDefault(); | ||
| onCitationSelect?.(citation.index); | ||
| } | ||
| }} | ||
| className={classNames( | ||
| 'w-full rounded-xl border bg-white p-4 text-left transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-blue-500/40 dark:bg-[#1a1b1e]', | ||
| isSelected | ||
| ? 'border-blue-400 ring-1 ring-blue-200 dark:border-blue-500 dark:ring-blue-500/20' | ||
| : 'border-gray-200 hover:border-blue-300 dark:border-gray-800 dark:hover:border-blue-600', | ||
| )} | ||
| data-reference-index={citation.index} | ||
| > | ||
| <div className='flex items-start gap-3'> | ||
| <span className='inline-flex h-6 min-w-6 flex-shrink-0 items-center justify-center rounded-md bg-blue-500 px-1.5 text-xs font-semibold text-white'> | ||
| {citation.index} | ||
| </span> | ||
| <div className='min-w-0 flex-1'> | ||
| <div className='flex flex-wrap items-center justify-between gap-2'> | ||
| <span className='min-w-0 break-words text-sm font-semibold text-gray-900 dark:text-gray-100'> | ||
| {citation.sourceName || citation.path || `来源 ${citation.index}`} | ||
| </span> | ||
| {score && ( | ||
| <span className='flex-shrink-0 rounded-full bg-emerald-50 px-2 py-0.5 text-[10px] font-medium text-emerald-700 dark:bg-emerald-500/10 dark:text-emerald-300'> | ||
| 相关度 {score} | ||
| </span> | ||
| )} | ||
| </div> | ||
| {citation.path && ( | ||
| <div className='mt-1 break-all font-mono text-[11px] text-gray-400 dark:text-gray-500'> | ||
| {citation.path} | ||
| </div> | ||
| )} | ||
| {citation.excerpt && ( | ||
| <div className='mt-3 whitespace-pre-wrap break-words text-xs leading-5 text-gray-600 dark:text-gray-300'> | ||
| {citation.excerpt} | ||
| </div> | ||
| )} | ||
| {safeUrl && ( | ||
| <a | ||
| href={safeUrl} | ||
| target='_blank' | ||
| rel='noreferrer noopener' | ||
| onClick={event => event.stopPropagation()} | ||
| className='mt-3 inline-flex items-center gap-1 text-xs font-medium text-blue-600 hover:text-blue-700 dark:text-blue-400 dark:hover:text-blue-300' | ||
| > | ||
| <LinkOutlined aria-hidden /> | ||
| 打开来源 | ||
| </a> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| })} | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The new citation UI ships hardcoded Chinese strings. All three sites add user-visible text directly in JSX instead of locale keys, so the English locale renders Chinese. Both components already call useTranslation.
web/new-components/chat/content/ManusRightPanel.tsx#L2358-L2436: replace the disclaimer banner (Line 2362), the来源 {index}fallback (Line 2399), the相关度score label (Line 2403), and the打开来源link text (Line 2426) witht()keys.web/new-components/chat/content/ManusRightPanel.tsx#L2758-L2777: replace the参考来源tab label with at()key.web/new-components/chat/content/ManusLeftPanel.tsx#L1276-L1285: replace查看 {n} 条参考来源with at()key that takes acountparameter.
Add the matching keys and interpolation parameters to both the English and Chinese locale files.
📍 Affects 2 files
web/new-components/chat/content/ManusRightPanel.tsx#L2358-L2436(this comment)web/new-components/chat/content/ManusRightPanel.tsx#L2758-L2777web/new-components/chat/content/ManusLeftPanel.tsx#L1276-L1285
Source: Path instructions
| {displayCitations.map(citation => ( | ||
| <Tooltip | ||
| key={`${citation.index}-${citation.id}`} | ||
| title={ | ||
| <div className='max-w-sm'> | ||
| <div className='font-medium mb-1'>{citation.sourceName}</div> | ||
| <div className='whitespace-pre-wrap break-words'>{citation.excerpt.slice(0, 800)}</div> | ||
| </div> | ||
| } | ||
| > | ||
| <button | ||
| type='button' | ||
| onClick={() => onCitationClick?.(citation)} | ||
| className={classNames( | ||
| 'inline-flex items-center gap-1 rounded-md px-1.5 py-0.5 text-[11px]', | ||
| 'border border-blue-200 dark:border-blue-800', | ||
| 'bg-blue-50 dark:bg-blue-900/20 text-blue-600 dark:text-blue-300', | ||
| onCitationClick && 'hover:bg-blue-100 dark:hover:bg-blue-900/40 cursor-pointer', | ||
| )} | ||
| > | ||
| <span className='font-semibold'>[{citation.index}]</span> | ||
| <span className='max-w-40 truncate'>{citation.sourceName}</span> | ||
| </button> | ||
| </Tooltip> | ||
| ))} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
antd 5 Tooltip trigger focus prop accessibility
💡 Result:
In Ant Design 5, the Tooltip component does not trigger on focus by default, which can limit accessibility for keyboard-only users [1][2]. To improve accessibility, you must explicitly configure the trigger property to include 'focus' [1][3]. You can enable focus triggering in two ways: 1. For a single component: Set the trigger property to an array that includes 'focus' [1][2]. <Tooltip trigger={['hover', 'focus']} title="Title"> Button 2. Globally: Use ConfigProvider to set the trigger behavior across your entire application [1][2]. <ConfigProvider tooltip={{ trigger: ['hover', 'focus'] }} popover={{ trigger: ['hover', 'focus'] }} popconfirm={{ trigger: ['hover', 'focus'] }} > When implementing this, ensure that your child components are capable of accepting and passing through the necessary events (such as onFocus) to the underlying DOM node [2][4]. Ant Design 5 manages the required WAI-ARIA relationships (such as aria-describedby) internally [5], helping to ensure that the tooltip content is associated with the trigger element for screen readers when configured correctly [5][6].
Citations:
- 1: https://ant.design/components/tooltip/
- 2: https://ant-design.antgroup.com/components/tooltip
- 3: https://github.com/ant-design/ant-design/blob/master/components/tooltip/shared/sharedFAQ.en-US.md
- 4: https://ant.design/components/tooltip.md
- 5: Tooltip component accessibility is lacking ant-design/ant-design#49891
- 6: https://www.w3.org/WAI/ARIA/apg/patterns/tooltip/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate file and nearby code =="
wc -l web/new-components/chat/content/OpenCodeSessionTurn.tsx
sed -n '660,730p' web/new-components/chat/content/OpenCodeSessionTurn.tsx
echo
echo "== Search citation handler usages/props =="
rg -n "onCitationClick|displayCitations|sourceName|excerpt" web/new-components/chat/content/OpenCodeSessionTurn.tsx web -g '*.tsx' -g '*.ts' | head -200
echo
echo "== Package Ant Design version =="
if [ -f web/package.json ]; then jq '.dependencies["antd"] // .devDependencies["antd"] // empty' web/package.json; fi
echo
echo "== Antd Tooltip source from node_modules if available =="
if [ -d web/node_modules/antd ]; then
rg -n "trigger =|defaultTrigger|focus.*tooltip|TooltipImpl|Trigger" web/node_modules/antd/lib/tooltip web/node_modules/antd/es/tooltip | head -80
fiRepository: eosphoros-ai/DB-GPT
Length of output: 12210
Make citation tooltips focusable and avoid clicking without a handler.
Tooltip defaults to hover, so keyboard users can focus the citation key without seeing sourceName or excerpt. Add trigger={['hover', 'focus']} and omit the button when onCitationClick is unset, so the citation key does not look clickable. If the rendered key must remain, disable it and use a non-interactive button when there is no handler.
Source: Path instructions
| // Final is already a complete server event. Build artifacts now; | ||
| // the bounded summary presentation controls only visible navigation. | ||
| setPendingFinalization({ responseId, summaryText, uploadedFilePath }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
uploadedFilePath is stale here; use currentUploadedFilePath.
processEvent is defined inside handleStart and closes over the uploadedFilePath state value from the render that started the request. handleStart computes the real path into the local currentUploadedFilePath (Lines 1748, 1764, 1769) and only schedules setUploadedFilePath(...), so the state value visible in this closure is the previous one.
For the first turn with an uploaded file, the closure value is null, so buildArtifactsFromExecution receives no filePath and the uploaded-file artifact is never created. On later turns it can carry the previous turn's path instead.
currentUploadedFilePath is in scope and holds the correct value.
🐛 Proposed fix
- setPendingFinalization({ responseId, summaryText, uploadedFilePath });
+ setPendingFinalization({ responseId, summaryText, uploadedFilePath: currentUploadedFilePath });📝 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.
| // Final is already a complete server event. Build artifacts now; | |
| // the bounded summary presentation controls only visible navigation. | |
| setPendingFinalization({ responseId, summaryText, uploadedFilePath }); | |
| // Final is already a complete server event. Build artifacts now; | |
| // the bounded summary presentation controls only visible navigation. | |
| setPendingFinalization({ responseId, summaryText, uploadedFilePath: currentUploadedFilePath }); |
| citations={activeRoundData?.showFinal ? activeRoundData.round.citations : []} | ||
| selectedCitationIndex={selectedCitationIndex} | ||
| onCitationSelect={setSelectedCitationIndex} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset or validate selectedCitationIndex when the round changes.
selectedCitationIndex is page-level state, but citation indexes restart at 1 in every round. When the replay moves to the next round, the old selection stays and silently highlights an unrelated source with the same index. ReferencesPanel also scrolls to it on mount.
web/pages/index.tsx (Lines 3600-3604) guards this by checking that the selected index exists in the active answer's citations. Apply the same guard here.
🐛 Proposed fix
- citations={activeRoundData?.showFinal ? activeRoundData.round.citations : []}
- selectedCitationIndex={selectedCitationIndex}
+ citations={activeRoundData?.showFinal ? activeRoundData.round.citations : []}
+ selectedCitationIndex={
+ activeRoundData?.showFinal &&
+ activeRoundData.round.citations.some(citation => citation.index === selectedCitationIndex)
+ ? selectedCitationIndex
+ : null
+ }
onCitationSelect={setSelectedCitationIndex}📝 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.
| citations={activeRoundData?.showFinal ? activeRoundData.round.citations : []} | |
| selectedCitationIndex={selectedCitationIndex} | |
| onCitationSelect={setSelectedCitationIndex} | |
| citations={activeRoundData?.showFinal ? activeRoundData.round.citations : []} | |
| selectedCitationIndex={ | |
| activeRoundData?.showFinal && | |
| activeRoundData.round.citations.some(citation => citation.index === selectedCitationIndex) | |
| ? selectedCitationIndex | |
| : null | |
| } | |
| onCitationSelect={setSelectedCitationIndex} |
anujbolewar
left a comment
anujbolewar
left a comment
There was a problem hiding this comment.
Separating final answers from citations makes the SSE contract nicer to consume. Parsing only the first line for the event type and swallowing bad JSON is the right defensive posture so malformed streamed text cannot crash the generator. Worth a test that a payload with a valid type but a non-JSON rest of the line still yields the expected event, and confirming event ordering is preserved when the final answer and citation blocks interleave.
2 participants
Description
Close #3181
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Snapshots:
Include snapshots for easier review.
Checklist: