feat(webchat): 增强管理端 WebChat 会话与工具调用体验#69
Conversation
Add WebUI chat job endpoints with event replay, cancellation, and active job lookup so streamed chats can continue after refresh or disconnect. Bridge the legacy stream=true chat endpoint to the new job event stream while keeping stream=false synchronous behavior unchanged. Add paged WebUI chat history loading and protected clear-history behavior for virtual private chat system#42 without touching long-term memory, cognitive memory, or profiles. Update the management proxy, WebUI chat workspace, token/tool/agent progress rendering, docs, OpenAPI metadata, and focused runtime/WebUI tests. Verification: - uv run pytest tests/test_runtime_api_chat_stream.py tests/test_runtime_api_chat_history.py tests/test_runtime_api_chat_jobs.py tests/test_webui_management_api.py -q - uv run ruff check . - uv run mypy . - npm run check (apps/undefined-console) Co-authored-by: GPT-5 Codex <noreply@openai.com>
Keep streamed tokens, final messages, and tool progress for a WebChat job in the same AI bubble by binding the active message to the job id. Add WebChat UI hints so private send_message/send_private_message calls render as compact send status instead of repeating message arguments and sent results. Compact successful end tool display while preserving error details. Update docs and add regression coverage for WebChat stream payload hints and frontend event handling invariants. Tests: uv run pytest tests/test_runtime_api_chat_stream.py tests/test_runtime_api_chat_history.py tests/test_runtime_api_chat_jobs.py tests/test_webui_management_api.py tests/test_webui_runtime_chat_frontend.py Tests: uv run ruff check . Tests: uv run mypy . Tests: npm run check Co-authored-by: GPT-5 Codex <noreply@openai.com>
Stop exposing token and tool delta events through WebChat jobs. Render final messages and tool lifecycle events only, while preserving job resume semantics and compact send/end tool hints. Clean up the obsolete LLM stream event callback path and update frontend handling, docs, and tests for the narrower WebChat SSE contract. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Persist WebChat tool and agent lifecycle events as display-only history metadata so refreshed chats can restore tool blocks. Return webchat metadata from paged history, keep metadata-only entries visible, and filter display-only tool records out of AI context. Guard history clearing while chat jobs are running or finalizing to avoid stale job writebacks. Update WebUI history rendering, tests, and docs for restored tool blocks. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Render WebChat tool blocks before message content so send_message lifecycle state appears in call order. Prevent chat log grid rows from stretching message cards to fill the viewport; cards now size to their content. Add frontend regression coverage for tool block placement. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Render WebChat tool and message events as an ordered timeline inside the same AI bubble instead of grouping all tools around the final text. Persist message events in webchat history metadata so refresh restores tool/text/tool ordering. Update frontend regression tests and docs for timeline replay. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Remove the duplicate AI Chat title from the WebChat card and move the superadmin/private-command hint into the chat subtitle. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Add Runtime-owned WebChat stage events, backend-computed reply/tool durations, and persisted WebChat timing metadata for history restoration. Render the current stage after the AI label and display tool durations from event payloads without client-side timing inference. Update WebChat API docs and frontend/backend tests for stage events, duration metadata, and history persistence. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Move the WebChat clear-history button to the chat page header and remove the now-empty workspace header styles. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Persist backend webchat call metadata, timeline ordering, durations, and nested agent tool calls so the frontend can render structured history faithfully. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Persist nested WebChat message ordering, add auto-scroll controls, and sync user-visible sends through RequestContext so end no longer misreads copied tool contexts. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Use a single-line WebChat tool summary ordered by tool name, status, and kind so tool and agent blocks scan cleanly in chat history. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Keep WebChat tool details open for at least two seconds from invocation, collapse immediately after longer calls finish, and emphasize tool names in compact summaries. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Add search_request to grok_search so web_agent supplies a complete natural-language search brief, while keeping query as a legacy fallback. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Keep WebChat send/end tool result previews so expanding compact tool rows shows the real backend feedback instead of an empty body. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Add a single-worker Chroma operation scheduler with foreground burst fairness for cognitive vector reads and writes. Route Web/API foreground reads ahead of historian maintenance and background writes while keeping legacy vector store call signatures compatible. Document the new scheduler setting and cover priority, cancellation, serial execution, and config defaults in tests. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Keep tool blocks expanded for at least two seconds after the result is rendered instead of subtracting the tool execution duration. Parse Grok chat-completion style responses and return choices[0].message.content, falling back to the original payload when parsing fails. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Persist nested agent stage metadata, expose JSON job-event polling for refresh-safe WebChat recovery, and render backend-computed reply durations in the chat UI. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Redact WebChat tool preview metadata before event emission and history restore, serialize job stage snapshots under the job condition lock, and harden markdown rendering in the WebUI chat surface. Improve active job resume retry behavior, reduced-motion scrolling, keyboard/live-region accessibility, and narrow-screen tool preview layout. Update Runtime/WebUI API docs and add regression coverage for proxy auth, preview redaction, recovery polling, and mobile overflow guards. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Move WebChat session context into the chat title, compact the header controls for desktop and mobile, and keep more vertical room for messages. Show tool and agent durations next to their names, color the left status rail by running/done/error state, and stop rendering agent stage snapshots as noisy timeline chips. Keep backend-provided AI reply timing in the AI label, suppress zero-duration tool badges, and avoid redundant agent-stage redraws that caused visible flicker. Update WebChat frontend tests and API/WebUI docs for the compact timing and status behavior. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Expose running WebChat tool and agent snapshots from the runtime job so refresh and reconnect can recover active call state with backend-computed durations. Poll WebChat jobs every 0.5 seconds, locally advance displayed AI/tool/agent timers between backend snapshots, and lock the display to final backend durations when calls finish. Update tool and agent duration rendering without redrawing expanded blocks on every timer tick, and keep Agent stages in the same compact summary row. Document current_tool_calls and update WebChat frontend/runtime tests for live timing snapshots. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Add local syntax highlighting for WebChat Markdown code blocks while preserving HTML escaping. Inject Grok search instructions to use server time, search first, cross-check sources, and cite references. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Ask agents to describe search intent and requirements in natural language without inventing hard scope limits. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Scroll after the user message and again after the AI placeholder is mounted so layout updates cannot leave WebChat above the bottom. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Require search_request throughout grok_search and drop the legacy query parameter from schema, handler, and tests. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds job-based WebChat APIs with conversation persistence and SSE/JSON events, a prioritized Chroma scheduler integrated into cognitive vector-store calls, current-input batch and end-tool contract changes, WebUI runtime proxy/frontend updates, agent/tool webchat lifecycle events, secure download plumbing, and extensive docs/tests. ChangesWebChat runtime + cognitive scheduler + prompting updates
Sequence Diagram(s)sequenceDiagram
participant WebUI
participant WebUI_Proxy
participant RuntimeAPI
participant ChatJobManager
participant ConversationStore
WebUI->>WebUI_Proxy: POST /management/runtime/chat/jobs
WebUI_Proxy->>RuntimeAPI: POST /api/v1/chat/jobs (message, conversation_id)
RuntimeAPI->>ChatJobManager: create_job()
ChatJobManager->>ConversationStore: append user message
WebUI->>WebUI_Proxy: GET /management/runtime/chat/jobs/{id}/events (SSE/JSON)
WebUI_Proxy->>RuntimeAPI: GET /api/v1/chat/jobs/{id}/events
RuntimeAPI-->>WebUI_Proxy: stage/tool/message/done
WebUI_Proxy-->>WebUI: stage/tool/message/done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09bb143466
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 16
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)
src/Undefined/utils/paths.py (1)
8-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd type annotations to module-level constants.
The new
Pathconstants are missing explicit type annotations. As per coding guidelines, all Python code must have complete type annotations for strict mypy.📝 Proposed fix
DATA_DIR = Path("data") -HISTORY_DIR = DATA_DIR / "history" +HISTORY_DIR: Path = DATA_DIR / "history" CACHE_DIR = DATA_DIR / "cache" RENDER_CACHE_DIR = CACHE_DIR / "render" IMAGE_CACHE_DIR = CACHE_DIR / "images" ATTACHMENT_CACHE_DIR = CACHE_DIR / "attachments" DOWNLOAD_CACHE_DIR = CACHE_DIR / "downloads" TEXT_FILE_CACHE_DIR = CACHE_DIR / "text_files" URL_FILE_CACHE_DIR = CACHE_DIR / "url_files" WEBUI_FILE_CACHE_DIR = CACHE_DIR / "webui_files" ATTACHMENT_REGISTRY_FILE = DATA_DIR / "attachment_registry.json" -WEBCHAT_DIR = DATA_DIR / "webchat" -WEBCHAT_CONVERSATIONS_DIR = WEBCHAT_DIR / "conversations" -WEBCHAT_MIGRATION_MARKER_FILE = WEBCHAT_DIR / "legacy_private_42_migrated.json" +WEBCHAT_DIR: Path = DATA_DIR / "webchat" +WEBCHAT_CONVERSATIONS_DIR: Path = WEBCHAT_DIR / "conversations" +WEBCHAT_MIGRATION_MARKER_FILE: Path = WEBCHAT_DIR / "legacy_private_42_migrated.json"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/utils/paths.py` around lines 8 - 20, These module-level constants (HISTORY_DIR, CACHE_DIR, RENDER_CACHE_DIR, IMAGE_CACHE_DIR, ATTACHMENT_CACHE_DIR, DOWNLOAD_CACHE_DIR, TEXT_FILE_CACHE_DIR, URL_FILE_CACHE_DIR, WEBUI_FILE_CACHE_DIR, ATTACHMENT_REGISTRY_FILE, WEBCHAT_DIR, WEBCHAT_CONVERSATIONS_DIR, WEBCHAT_MIGRATION_MARKER_FILE) lack explicit type annotations; add type hints by importing Path from pathlib and annotating each constant as Path (e.g., HISTORY_DIR: Path = DATA_DIR / "history") and ensure DATA_DIR itself is typed as Path so mypy sees consistent types across the module.Source: Coding guidelines
tests/test_webui_runtime_chat_frontend.py (1)
19-1421:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftReplace direct
Path.read_textusage with the project I/O utility.The tests repeatedly perform direct filesystem reads, but repository rules require disk I/O to go through
src/Undefined/utils/io.py.As per coding guidelines: "
**/*.py: Disk I/O should go throughsrc/Undefined/utils/io.pyto ensure writes stay async-safe and atomic" and "Disk read/write operations must useutils/io.pywithasyncio.to_thread...".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_webui_runtime_chat_frontend.py` around lines 19 - 1421, Tests use direct Path.read_text calls (e.g. RUNTIME_JS.read_text(...), WEBUI_TEMPLATE.read_text(...), I18N_JS.read_text(...), API_JS.read_text(...), APP_CSS.read_text(...), RESPONSIVE_CSS.read_text(...), MAIN_JS.read_text(...), and Path("src/Undefined/webui/templates/index.html").read_text(...)) which violates the project's I/O policy; replace these direct filesystem reads with the project's I/O helper from src/Undefined/utils/io.py (import the appropriate read function and call it with the same path and encoding) throughout this test file (references: test_webchat_html_preview_csp_allows_inline_scripts_without_eval, test_webchat_frontend_has_conversation_sidebar, test_webchat_frontend_has_slash_command_palette, test_webchat_frontend_pastes_files_as_pending_attachments, test_webchat_layout_keeps_input_at_bottom_and_log_scrollable, etc.) so all disk reads go through the utils/io API and follow the async-safe/atomic conventions.Source: Coding guidelines
tests/test_cognitive_vector_store_metadata.py (1)
93-106:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStop the scheduler before the test exits.
ChromaOperationSchedulerleaves a worker task alive after the query completes, so this test now leaks a pending task into loop teardown. Wrap the query intry/finallyandawait scheduler.stop()there.Proposed fix
- store._chroma_scheduler = ChromaOperationScheduler() + scheduler = ChromaOperationScheduler() + store._chroma_scheduler = scheduler fake_collection = _FakeCollection() store._events = cast(Any, fake_collection) store._profiles = cast(Any, object()) - results = await store._query( - fake_collection, - "测试查询", - 1, - None, - None, - 1, - query_embedding=[0.11, 0.22, 0.33], - ) + try: + results = await store._query( + fake_collection, + "测试查询", + 1, + None, + None, + 1, + query_embedding=[0.11, 0.22, 0.33], + ) + finally: + await scheduler.stop()As per coding guidelines,
tests/**/*.py: Use pytest with pytest-asyncio for testing; configureasyncio_mode = 'auto'.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cognitive_vector_store_metadata.py` around lines 93 - 106, The test leaves a ChromaOperationScheduler worker running; to fix, assign and use the scheduler on the store (store._chroma_scheduler = ChromaOperationScheduler()), wrap the call to store._query(...) in a try/finally, and in the finally await scheduler.stop() (i.e., await store._chroma_scheduler.stop()) so the worker is stopped before test teardown; also ensure the test uses pytest-asyncio (async def test_... with asyncio_mode='auto') per test guidelines.Source: Coding guidelines
src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py (2)
8-8:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDependency injection violation: skills handler imports from outside
skills/.This handler imports
scope_from_contextfromUndefined.attachments, violating the architecture rule that skills handlers must not import repo-local modules outsideskills/. Dependencies should be injected via the execution context instead.As per coding guidelines, skills handlers must receive dependencies through the
contextparameter to maintain modularity and testability.♻️ Recommended approach
Pass the scope-from-context logic through the context:
# In the calling code (coordinator/runner), inject the helper: context["get_scope_from_context"] = scope_from_context # In handler.py, use it: get_scope_fn = context.get("get_scope_from_context") if get_scope_fn: scope_key = get_scope_fn(context)Or inject the resolved
scope_keydirectly if it's already known by the caller.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py` at line 8, The handler currently imports scope_from_context directly (scope_from_context) which breaks the rule against skills importing repo-local modules; remove that import and instead read a dependency injected through the handler's context (e.g. context["get_scope_from_context"] or context["scope_key"]). Update the handler to retrieve get_scope_from_context = context.get("get_scope_from_context") and call it with the context when present, or simply read a precomputed scope_key from context if the caller injects it; ensure any callers (coordinator/runner) inject either the helper function or the resolved scope_key into the context before invoking the handler.Source: Coding guidelines
115-115:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDependency injection violation: lazy import from outside
skills/in handler.The function-level import of
DOWNLOAD_CACHE_DIRandensure_dirfromUndefined.utils.pathsviolates the same architecture rule. Skills handlers must not import repo-local modules outsideskills/, even with lazy imports.Inject these paths/helpers through the execution context.
♻️ Recommended approach
# In the calling code, inject paths: context["download_cache_dir"] = DOWNLOAD_CACHE_DIR context["ensure_dir_fn"] = ensure_dir # In handler.py: cache_dir = context.get("download_cache_dir") ensure_dir_fn = context.get("ensure_dir_fn") if cache_dir and ensure_dir_fn: temp_dir = ensure_dir_fn(cache_dir / task_uuid)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py` at line 115, The handler currently does a lazy import of DOWNLOAD_CACHE_DIR and ensure_dir from Undefined.utils.paths which violates the skills/ boundary; remove that import and instead read these injected values from the execution context (expect keys like "download_cache_dir" and "ensure_dir_fn"), then use the injected ensure_dir_fn to create the per-task temp directory (e.g., using the task UUID) and use download_cache_dir as the base path; if either context value is missing, raise or return a clear error so callers know to inject them.Source: Coding guidelines
🟡 Minor comments (6)
src/Undefined/api/routes/commands.py-368-368 (1)
368-368:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace assertion with proper error handling.
Using
assertfor runtime logic validation is unsafe because assertions can be disabled with Python's-Ooptimization flag. While_build_commands_payloadreturns a non-None dict for list operations, defensive validation should use explicit error handling rather than assertions.🛡️ Proposed fix
- assert payload is not None + if payload is None: + return _json_error("Failed to build commands payload", status=500)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/api/routes/commands.py` at line 368, Replace the unsafe assertion "assert payload is not None" with explicit error handling: after calling _build_commands_payload, check if payload is None and raise an appropriate exception (e.g., raise HTTPException(status_code=400, detail="Missing or invalid commands payload") in this route handler) or raise ValueError if this is internal logic; use the variable name payload and the function _build_commands_payload to locate the call, and ensure the error path returns a clear, descriptive message instead of relying on assert.src/Undefined/api/webchat_store.py-26-37 (1)
26-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd type annotations to module-level constants.
The module constants are missing explicit type annotations. As per coding guidelines, all Python code must have complete type annotations for strict mypy.
📝 Proposed fix
-WEBCHAT_VIRTUAL_USER_ID = 42 -WEBCHAT_VIRTUAL_USER_NAME = "system" -DEFAULT_WEBCHAT_CONVERSATION_ID = "legacy-system-42" -_DEFAULT_TITLE = "新对话" -_TEMP_TITLE_CHARS = 18 -_MIGRATION_VERSION = 1 -_TITLE_STATUS_GENERATED = "generated" -_TITLE_STATUS_MANUAL = "manual" -_TITLE_STATUS_PENDING = "pending" -_TITLE_STATUS_TEMPORARY = "temporary" -_TITLE_STATUS_FAILED = "failed" +WEBCHAT_VIRTUAL_USER_ID: int = 42 +WEBCHAT_VIRTUAL_USER_NAME: str = "system" +DEFAULT_WEBCHAT_CONVERSATION_ID: str = "legacy-system-42" +_DEFAULT_TITLE: str = "新对话" +_TEMP_TITLE_CHARS: int = 18 +_MIGRATION_VERSION: int = 1 +_TITLE_STATUS_GENERATED: str = "generated" +_TITLE_STATUS_MANUAL: str = "manual" +_TITLE_STATUS_PENDING: str = "pending" +_TITLE_STATUS_TEMPORARY: str = "temporary" +_TITLE_STATUS_FAILED: str = "failed" _JsonT = TypeVar("_JsonT")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/api/webchat_store.py` around lines 26 - 37, Add explicit type annotations to the module-level constants: change assignments to annotated forms such as WEBCHAT_VIRTUAL_USER_ID: int = 42, WEBCHAT_VIRTUAL_USER_NAME: str = "system", DEFAULT_WEBCHAT_CONVERSATION_ID: str = "legacy-system-42", _DEFAULT_TITLE: str = "新对话", _TEMP_TITLE_CHARS: int = 18, _MIGRATION_VERSION: int = 1, _TITLE_STATUS_GENERATED: str = "generated", _TITLE_STATUS_MANUAL: str = "manual", _TITLE_STATUS_PENDING: str = "pending", _TITLE_STATUS_TEMPORARY: str = "temporary", _TITLE_STATUS_FAILED: str = "failed"; for the TypeVar keep an explicit annotation like _JsonT: TypeVar = TypeVar("_JsonT") so mypy recognizes its type while preserving the existing value.Source: Coding guidelines
tests/test_ai_coordinator_queue_routing.py-228-237 (1)
228-237:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd type annotations to test variables and function.
The test variables
captured_extra_context,captured_resources, and the nested_fake_askfunction are missing type annotations. As per coding guidelines, all Python code (including tests) must have complete type annotations for strict mypy.📝 Proposed fix
coordinator: Any = object.__new__(AICoordinator) sender = SimpleNamespace(send_group_message=AsyncMock()) - captured_extra_context: dict[str, Any] = {} - captured_resources: dict[str, Any] = {} + captured_extra_context: dict[str, Any] = {} + captured_resources: dict[str, Any] = {} - async def _fake_ask(*_args: Any, **kwargs: Any): + async def _fake_ask(*_args: Any, **kwargs: Any) -> str: captured_extra_context.update(kwargs.get("extra_context", {})) current_context = RequestContext.current() assert current_context is not None captured_resources.update(current_context.get_resources()) await kwargs["send_message_callback"]("hello group") return ""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_ai_coordinator_queue_routing.py` around lines 228 - 237, Annotate the test locals and the fake async function: add explicit types for captured_extra_context and captured_resources (e.g., dict[str, Any]), and update the signature of _fake_ask to include parameter and return types (e.g., _args: tuple[Any, ...], kwargs: dict[str, Any] or more specific keys like extra_context: dict[str, Any] and send_message_callback: Callable[[str], Awaitable[None]]) and an explicit return type of str; inside the function, keep using RequestContext.current() and captured_* updates but ensure types match the annotations so mypy passes.Source: Coding guidelines
src/Undefined/webui/templates/index.html-779-789 (1)
779-789:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWire the drawer toggle to its controlled panel.
The button advertises an expandable region via
aria-expanded, but it never declares which region it controls. Addingaria-controls="runtimeChatConversationDrawerPanel"makes the drawer state much clearer to assistive tech.Proposed fix
<button id="runtimeChatConversationDrawerToggle" class="runtime-chat-sidebar-tab" type="button" aria-expanded="false" + aria-controls="runtimeChatConversationDrawerPanel" data-i18n-aria-label="runtime.chat_conversations" aria-label="对话" >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/webui/templates/index.html` around lines 779 - 789, The toggle button with id runtimeChatConversationDrawerToggle currently uses aria-expanded but does not declare the controlled region; update the element referenced by id runtimeChatConversationDrawerToggle to include aria-controls="runtimeChatConversationDrawerPanel" so it explicitly associates with the panel element id runtimeChatConversationDrawerPanel and improves accessibility for assistive tech.src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py-293-293 (1)
293-293:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBlocking I/O in event loop:
stat()should useasyncio.to_thread.Line 293 calls
local_path.stat().st_sizesynchronously, which is blocking disk I/O. According to coding guidelines, disk read/write operations must useasyncio.to_threadto avoid blocking the event loop.🔧 Proposed fix
+ import asyncio + + size = await asyncio.to_thread(lambda: local_path.stat().st_size) - size = local_path.stat().st_size🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py` at line 293, The synchronous call size = local_path.stat().st_size performs blocking disk I/O; change it to run in a thread using asyncio.to_thread (e.g., await asyncio.to_thread(local_path.stat) and then read .st_size, or await asyncio.to_thread(lambda: local_path.stat().st_size)) so that local_path.stat() is executed off the event loop and assigned to size without blocking; update the surrounding async function to await this call where local_path and size are used.Source: Coding guidelines
src/Undefined/ai/prompts/current_input.py-22-22 (1)
22-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd return type annotation.
The function
_parse_attrsis missing a return type annotation. As per coding guidelines, all Python code must have complete type annotations and pass strict mypy checks.🔧 Proposed fix
-def _parse_attrs(attrs_text: str) -> dict[str, str]: +def _parse_attrs(attrs_text: str) -> dict[str, str]:Wait, that's the same. Let me check the actual code again - line 22 shows
def _parse_attrs(attrs_text: str) -> dict[str, str]:but the annotation says it's missing. Looking at the annotated code, line 22 showsdef _parse_attrs(attrs_text: str) -> dict[str, str]:. Actually, it DOES have the return type annotation. Let me reconsider.Actually, looking at the code again, line 22 is:
def _parse_attrs(attrs_text: str) -> dict[str, str]:This HAS a return type annotation. I was mistaken. Let me skip this.
[scratchpad_end] -->🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/ai/prompts/current_input.py` at line 22, The review comment is incorrect: _parse_attrs already has a return type annotation (def _parse_attrs(attrs_text: str) -> dict[str, str]:); remove or correct the PR review note and, if mypy still flags this, ensure the file uses a compatible Python typing style (replace dict[str, str] with typing.Dict[str, str] if targeting older typing or adjust project pyproject/python-version), then re-run type checks.Source: Coding guidelines
🧹 Nitpick comments (4)
src/Undefined/skills/agents/runner/tools.py (1)
14-26: ⚡ Quick winExtract duplicated webchat helpers to a shared module.
The functions
_webchat_agent_pathand_webchat_depthare duplicated betweenloop.py(lines 16-27) andtools.py(lines 14-24). Sinceloop.pyalready imports fromtools.py, introducing a new shared module (e.g.,runner/webchat_utils.py) would avoid circular dependencies and eliminate duplication.♻️ Suggested refactor
Create
src/Undefined/skills/agents/runner/webchat_utils.py:from __future__ import annotations from typing import Any def webchat_agent_path(value: Any) -> list[str]: if not isinstance(value, list): return [] return [str(item) for item in value if str(item).strip()] def webchat_depth(value: Any) -> int: try: return max(0, int(value)) except (TypeError, ValueError): return 0Then update both
loop.pyandtools.pyto import:from Undefined.skills.agents.runner.webchat_utils import ( webchat_agent_path, webchat_depth, )And replace:
_webchat_agent_path→webchat_agent_path_webchat_depth→webchat_depth🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/skills/agents/runner/tools.py` around lines 14 - 26, Extract the duplicated helpers into a new module and update imports: create a new file runner/webchat_utils.py and move the implementations of _webchat_agent_path and _webchat_depth into it as public functions webchat_agent_path and webchat_depth; then update src/Undefined/skills/agents/runner/tools.py and loop.py to import webchat_agent_path and webchat_depth from Undefined.skills.agents.runner.webchat_utils (replacing usages of _webchat_agent_path and _webchat_depth), ensuring no other references to the old underscored names remain.src/Undefined/api/webchat_store.py (1)
587-590: ⚡ Quick winMove
import copyto module level.Standard Python convention is to place imports at the module level unless there's a specific reason (e.g., circular dependency). Moving this import to the top of the file improves readability and follows best practices.
♻️ Proposed refactor
At the top of the file, add:
import copyThen simplify the function:
def _copy_json(value: _JsonT) -> _JsonT: - import copy - return copy.deepcopy(value)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/api/webchat_store.py` around lines 587 - 590, The local import inside _copy_json should be moved to the module level; add "import copy" at the top of the file and remove the inline "import copy" inside _copy_json, then simplify _copy_json to just return copy.deepcopy(value) (keep the function name _copy_json and its signature unchanged).tests/test_webchat_conversations.py (1)
62-78: ⚡ Quick winPrefer behavior-level log assertions over source-text scanning.
This test hard-codes log message literals by reading source files, which is brittle to harmless wording/refactor changes and does not verify logs are actually emitted at runtime. Consider asserting emitted records with
caplogduring an exercised flow instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_webchat_conversations.py` around lines 62 - 78, The test test_webchat_runtime_has_detailed_flow_logs currently reads source text from src/Undefined/api/routes/chat.py and src/Undefined/api/webchat_store.py and asserts on hard-coded log string literals; instead, change the test to exercise the actual runtime flow (call the WebChat route handler functions / job creation path and relevant WebChatStore methods) and use pytest's caplog to capture emitted log records, then assert behavior-level messages are present in caplog.records (or caplog.text) such as messages emitted by the route handlers in chat.py (e.g., the job creation/开始/调用 AI/logging points) and the store methods in webchat_store.py (e.g., 会话存储加载完成, 生成会话标题, 追加消息), so tests verify logs are actually emitted rather than scanning source files.docs/openapi.md (1)
237-402: 🏗️ Heavy liftConsider improving documentation structure for complex sections.
The WebUI AI Chat section contains extensive technical details spanning endpoint behavior, event formats, payload structures, and multiple JSON examples. While comprehensive, the density makes it challenging to quickly find specific information.
Consider:
- Using collapsible sections (
<details>) for lengthy JSON examples- Creating a separate subsection or table of contents for event types
- Breaking the behavior conventions into a bulleted list
- Moving large JSON schemas to appendices with cross-references
This would maintain the comprehensive coverage while improving navigability for users who need to reference specific event formats or behaviors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi.md` around lines 237 - 402, The WebUI AI Chat section is dense and hard to scan; split it into clearer parts by adding a short table of contents linking to "WebUI AI Chat(特殊私聊)", "WebUI AI Chat Conversations", "WebUI AI Chat 历史记录" and a new "Event types" subsection; collapse long JSON examples (e.g., the large webchat example under webchat.timeline/webchat.events) using <details> or move them to an appendix and replace inline examples with brief summaries and cross-references; extract the lifecycle/behavior conventions (mentions of stage/agent_stage/tool_start/tool_end/message/done/error, webchat.calls, webchat.timeline) into a bulleted list for quick scanning; and create a dedicated "Schemas / Appendix" section that contains full JSON schemas for webchat.events, webchat.calls, and history responses with links from the main text.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/undefined-console/src-tauri/tauri.conf.json`:
- Line 26: The CSP in the "csp" property currently includes "script-src 'self'
'unsafe-inline'", which weakens protection; remove "'unsafe-inline'" from the
"csp" value and instead adopt a safer pattern (e.g., use nonces or script hashes
for allowed inline scripts or limit inline scripts to a sandboxed renderer),
update the configuration entry for "csp" to only allow 'self' plus nonces/hashes
(or 'strict-dynamic' with nonces) and ensure your renderers inject and verify
the nonce value for any required inline script execution; locate the "csp"
string in tauri.conf.json and replace the unsafe inline allowance with the
nonce/hash-based approach.
In `@docs/management-api.md`:
- Line 190: Split the single dense paragraph into clear subsections per endpoint
group (runtime/commands; runtime/chat/conversations; runtime/chat,
runtime/chat/history, runtime/chat/jobs, runtime/chat/jobs/active;
runtime/chat/files; runtime/chat/jobs/{job_id}/events), and for each subsection
replace the long run-on text with: a short purpose sentence, a bullet list of
query/body parameters (e.g., conversation_id, scope=webui, after, format),
validation rules (e.g., session/token check, X-Undefined-API-Key injection,
conversation_id consistency) and typical responses/edge-status codes (include
409 behavior), plus small code blocks showing example request and response
shapes (JSON/SSE) for complex cases like events and files; keep notes about
metadata fields (webchat.duration_ms, webchat.events, current_tool_calls,
stage/agent_stage) in the relevant subsections and mark them as “display-only”
vs “AI-context” to preserve the original semantics.
In `@docs/openapi.md`:
- Around line 702-704: The long dense paragraph describing WebUI proxy behavior
should be split into clear subsections: create a short paragraph for auth/header
injection from `[api].auth_key`; a subsection for command proxy
`/api/runtime/commands` (note default `scope=webui`); a subsection for
conversation handling (mention `/api/runtime/chat/conversations`, and that
`/api/runtime/chat`, `/api/runtime/chat/history`, `/api/runtime/chat/jobs`,
`/api/runtime/chat/jobs/active` forward `conversation_id`); a subsection for
file uploads that explains `/api/runtime/chat/files` accepts multipart/form-data
field `file`, caches to WebUI file cache and returns `{id,name,size}` and how
the frontend merges it into a CQ message
`CQ:file,id=<id>,name=<name>,size=<size>`; and finally a subsection for
event/query behavior for `/api/runtime/chat/jobs/{job_id}/events` (default JSON
incremental, but forward SSE when `Accept: text/event-stream` and mention chat
timeout behavior tied to model queue budget); convert parameter rules into
bullet points and add a short code snippet example for the file upload → CQ:file
conversion flow to improve readability.
In `@res/prompts/undefined.xml`:
- Around line 247-258: The new rule mandating "QQ号..." for every
end.observations entry breaks group/WebUI cases; change the prompt text around
the observations / end.observations rules to split by entity type: require
"QQ号{sender_id}(昵称...)..." only for user-centric observations (sender_id == QQ
user), and introduce separate canonical formats for non-user entities such as
"group:群号{group_id}(群名...)..." for group observations and
"webui:system#{session_id}(session_name)..." for WebUI/system sessions; update
any examples and the sentence that currently enforces QQ-only so it
conditionally applies based on the entity type and reference the existing
symbols observations, end.observations and memo when editing.
In `@src/Undefined/ai/client/ask_loop.py`:
- Around line 522-533: The direct await of webchat_event_callback (the call
handling "tool_start" with call_id, internal_function_name, api_function_name,
function_args, is_agent_call and webchat_event_base) can raise and abort the
ask/tool flow; wrap the await in a try/except that catches Exception, logs the
failure (using the module/logger/processLogger available in this module) and
continues without re-raising so emission is best-effort. Apply the same
defensive wrapper to the other occurrence that emits webchat events (the block
around lines 576-589) so any callback errors are isolated and do not mask or
abort tool execution.
In `@src/Undefined/api/routes/chat.py`:
- Around line 1805-1810: The route currently performs blocking disk I/O via
Path(local_path).is_file(); replace that sync check with the async-safe IO
utility (use the helpers in utils/io.py or run the Path checks inside
asyncio.to_thread) so the event loop isn't blocked. Specifically, change the
logic around local_path and the setting of fields["render_source"] to call an
async function (e.g., await utils.io.path_is_file(local_path) or await
asyncio.to_thread(lambda: Path(local_path).is_file())) and only then set
fields["render_source"] = Path(local_path).resolve().as_uri(); preserve the
existing OSError handling around the async call.
In `@src/Undefined/services/ai_coordinator.py`:
- Around line 926-939: The _collect_message_ids static method is duplicated;
extract its logic into a single shared utility function (e.g.,
collect_message_ids) in a new utils module and replace both copies with
imports/calls to that utility. Create a new function with the same
signature/behavior (accepts list[BufferedMessage], returns list[str]), move the
loop/seen logic into it, update the static method in this file and the duplicate
in the coordinator batching module to call the new utility, adjust imports,
preserve type hints and behavior, and remove the duplicated implementation.
In `@src/Undefined/skills/toolsets/messages/react_message_emoji/handler.py`:
- Line 9: The handler imports a repo-local symbol mark_message_sent_this_turn
from Undefined.utils, breaching the skills boundary; remove that import and
instead obtain the dependency from the execution context passed into the handler
(or a skills-local abstraction). Update the handler code in
react_message_emoji/handler.py to accept a context param (or use the existing
context object) and call context.mark_message_sent_this_turn (or
context.utils.mark_message_sent_this_turn) or inject a small wrapper service
into context during skill initialization; replace all direct references to
mark_message_sent_this_turn with the context-provided function to avoid
importing Undefined.utils.
In `@src/Undefined/skills/toolsets/messages/send_message/handler.py`:
- Line 9: The handler currently imports the repo-local utility
mark_message_sent_this_turn; change it to use an injected dependency from the
handler context instead: remove the from Undefined.utils.message_turn import
mark_message_sent_this_turn import and update the send_message handler to call
the injected function (e.g., context.mark_message_sent_this_turn or
context["mark_message_sent_this_turn"]) after a successful send. Ensure the
handler signature accepts the context object and document/validate that the
context provides mark_message_sent_this_turn so tests and callers inject the
repo-local implementation.
In `@src/Undefined/skills/toolsets/messages/send_poke/handler.py`:
- Line 8: The handler currently imports mark_message_sent_this_turn from
Undefined.utils, which violates the skills-only import rule; remove that direct
import and obtain the helper from the execution context (or implement a small
skills-local helper) instead. Update the handler code that uses
mark_message_sent_this_turn (reference symbol: mark_message_sent_this_turn) to
call the context-provided function (e.g.,
execution_context["mark_message_sent_this_turn"] or a named parameter) or use
the local implementation, and ensure any tests or call sites pass the helper via
the existing execution context object used by this handler.
In `@src/Undefined/skills/toolsets/messages/send_private_message/handler.py`:
- Line 9: The handler currently imports mark_message_sent_this_turn from
Undefined.utils, violating the skills boundary; remove that repo-local import
and instead call an injected function from the execution context (e.g.,
context.mark_message_sent_this_turn or
context.adapters.utils.mark_message_sent_this_turn) inside the
send_private_message handler. Update the handler to expect and call the injected
symbol name mark_message_sent_this_turn on the provided context, and ensure the
skill/bootstrap code that wires up this handler injects the actual
implementation into context under that same symbol name.
In `@src/Undefined/webui/app.py`:
- Line 36: The CSP currently includes the literal "script-src 'self'
'unsafe-inline';" which weakens XSS protection; remove 'unsafe-inline' and
either (A) generate per-request nonces and inject them into the CSP header and
any legitimate inline scripts, or (B) use script-hashes for trusted inline code,
and ensure every piece of rendered inline script in the new HTML preview feature
passes through the allowlist sanitizer (e.g., your sanitize_html /
render_preview path) that strips/rewrites scripts; update the code that builds
the CSP header string (the occurrence of "script-src 'self' 'unsafe-inline'") to
produce a nonce-based or hash-based policy and add nonce generation/injection
logic where preview HTML is rendered, plus unit/integration checks that inline
scripts without valid nonces/hashes are blocked.
In `@src/Undefined/webui/routes/_runtime.py`:
- Around line 695-743: The handler runtime_chat_file_upload_handler is
performing blocking disk I/O (dest.open(...).write, shutil.rmtree) directly on
the event loop and must be switched to the repository's async-safe I/O
utilities; replace the direct open/write and rmtree calls with calls into
utils/io.py executed via asyncio.to_thread (or the library's provided async
wrappers) so writes use atomic temp-file semantics and cross-platform locking,
and use the utils/io.py atomic delete/cleanup helpers instead of shutil.rmtree;
ensure the write loop that consumes field_any.read_chunk() still runs in the
async function but hands each chunk to the thread-safe writer (or buffers and
flushes via the async-safe writer) and that the error paths call the async-safe
remove/cleanup routines rather than shutil.rmtree.
In `@src/Undefined/webui/templates/index.html`:
- Around line 824-837: The iframe used for chat-driven HTML preview (element id
runtimeHtmlRunnerFrame in the index.html template) currently uses
sandbox="allow-scripts allow-forms allow-modals"; reduce its attack surface by
removing unnecessary flags—change the sandbox attribute to only allow-scripts
(e.g., sandbox="allow-scripts") and remove allow-forms and allow-modals; if
later functionality demands specific capabilities, add only the minimal
individual sandbox flags back with a clear comment near the
runtimeHtmlRunnerFrame element.
In `@tests/test_runtime_api_chat_history.py`:
- Around line 432-435: The post-clear assertions are inconsistent with
_DummyHistoryManager.clear_private_history: after a successful clear the history
should be emptied and the cleared count should equal the number of removed
records (2), not 0; update the assertions that check payload["cleared"] and
history.records so they expect payload["success"] is True, payload["cleared"] ==
2, and history.records is empty (and make the identical change in the other test
that mirrors this check).
In `@tests/test_webui_runtime_chat_frontend.py`:
- Around line 6-15: Add explicit type annotations to the module-level path
constants by annotating each symbol (RUNTIME_JS, RUNTIME_CSS, WEBUI_TEMPLATE,
MAIN_JS, API_JS, APP_CSS, RESPONSIVE_CSS, I18N_JS, WEBUI_APP_PY, TAURI_CONF) as
Path (or Final[Path] if they should be immutable) and ensure pathlib.Path (and
typing.Final if used) are imported at the top of the file so mypy strict mode is
satisfied; keep the same assigned Path(...) expressions but add the type
annotations to the declarations.
---
Outside diff comments:
In
`@src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py`:
- Line 8: The handler currently imports scope_from_context directly
(scope_from_context) which breaks the rule against skills importing repo-local
modules; remove that import and instead read a dependency injected through the
handler's context (e.g. context["get_scope_from_context"] or
context["scope_key"]). Update the handler to retrieve get_scope_from_context =
context.get("get_scope_from_context") and call it with the context when present,
or simply read a precomputed scope_key from context if the caller injects it;
ensure any callers (coordinator/runner) inject either the helper function or the
resolved scope_key into the context before invoking the handler.
- Line 115: The handler currently does a lazy import of DOWNLOAD_CACHE_DIR and
ensure_dir from Undefined.utils.paths which violates the skills/ boundary;
remove that import and instead read these injected values from the execution
context (expect keys like "download_cache_dir" and "ensure_dir_fn"), then use
the injected ensure_dir_fn to create the per-task temp directory (e.g., using
the task UUID) and use download_cache_dir as the base path; if either context
value is missing, raise or return a clear error so callers know to inject them.
In `@src/Undefined/utils/paths.py`:
- Around line 8-20: These module-level constants (HISTORY_DIR, CACHE_DIR,
RENDER_CACHE_DIR, IMAGE_CACHE_DIR, ATTACHMENT_CACHE_DIR, DOWNLOAD_CACHE_DIR,
TEXT_FILE_CACHE_DIR, URL_FILE_CACHE_DIR, WEBUI_FILE_CACHE_DIR,
ATTACHMENT_REGISTRY_FILE, WEBCHAT_DIR, WEBCHAT_CONVERSATIONS_DIR,
WEBCHAT_MIGRATION_MARKER_FILE) lack explicit type annotations; add type hints by
importing Path from pathlib and annotating each constant as Path (e.g.,
HISTORY_DIR: Path = DATA_DIR / "history") and ensure DATA_DIR itself is typed as
Path so mypy sees consistent types across the module.
In `@tests/test_cognitive_vector_store_metadata.py`:
- Around line 93-106: The test leaves a ChromaOperationScheduler worker running;
to fix, assign and use the scheduler on the store (store._chroma_scheduler =
ChromaOperationScheduler()), wrap the call to store._query(...) in a
try/finally, and in the finally await scheduler.stop() (i.e., await
store._chroma_scheduler.stop()) so the worker is stopped before test teardown;
also ensure the test uses pytest-asyncio (async def test_... with
asyncio_mode='auto') per test guidelines.
In `@tests/test_webui_runtime_chat_frontend.py`:
- Around line 19-1421: Tests use direct Path.read_text calls (e.g.
RUNTIME_JS.read_text(...), WEBUI_TEMPLATE.read_text(...),
I18N_JS.read_text(...), API_JS.read_text(...), APP_CSS.read_text(...),
RESPONSIVE_CSS.read_text(...), MAIN_JS.read_text(...), and
Path("src/Undefined/webui/templates/index.html").read_text(...)) which violates
the project's I/O policy; replace these direct filesystem reads with the
project's I/O helper from src/Undefined/utils/io.py (import the appropriate read
function and call it with the same path and encoding) throughout this test file
(references: test_webchat_html_preview_csp_allows_inline_scripts_without_eval,
test_webchat_frontend_has_conversation_sidebar,
test_webchat_frontend_has_slash_command_palette,
test_webchat_frontend_pastes_files_as_pending_attachments,
test_webchat_layout_keeps_input_at_bottom_and_log_scrollable, etc.) so all disk
reads go through the utils/io API and follow the async-safe/atomic conventions.
---
Minor comments:
In `@src/Undefined/ai/prompts/current_input.py`:
- Line 22: The review comment is incorrect: _parse_attrs already has a return
type annotation (def _parse_attrs(attrs_text: str) -> dict[str, str]:); remove
or correct the PR review note and, if mypy still flags this, ensure the file
uses a compatible Python typing style (replace dict[str, str] with
typing.Dict[str, str] if targeting older typing or adjust project
pyproject/python-version), then re-run type checks.
In `@src/Undefined/api/routes/commands.py`:
- Line 368: Replace the unsafe assertion "assert payload is not None" with
explicit error handling: after calling _build_commands_payload, check if payload
is None and raise an appropriate exception (e.g., raise
HTTPException(status_code=400, detail="Missing or invalid commands payload") in
this route handler) or raise ValueError if this is internal logic; use the
variable name payload and the function _build_commands_payload to locate the
call, and ensure the error path returns a clear, descriptive message instead of
relying on assert.
In `@src/Undefined/api/webchat_store.py`:
- Around line 26-37: Add explicit type annotations to the module-level
constants: change assignments to annotated forms such as
WEBCHAT_VIRTUAL_USER_ID: int = 42, WEBCHAT_VIRTUAL_USER_NAME: str = "system",
DEFAULT_WEBCHAT_CONVERSATION_ID: str = "legacy-system-42", _DEFAULT_TITLE: str =
"新对话", _TEMP_TITLE_CHARS: int = 18, _MIGRATION_VERSION: int = 1,
_TITLE_STATUS_GENERATED: str = "generated", _TITLE_STATUS_MANUAL: str =
"manual", _TITLE_STATUS_PENDING: str = "pending", _TITLE_STATUS_TEMPORARY: str =
"temporary", _TITLE_STATUS_FAILED: str = "failed"; for the TypeVar keep an
explicit annotation like _JsonT: TypeVar = TypeVar("_JsonT") so mypy recognizes
its type while preserving the existing value.
In
`@src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py`:
- Line 293: The synchronous call size = local_path.stat().st_size performs
blocking disk I/O; change it to run in a thread using asyncio.to_thread (e.g.,
await asyncio.to_thread(local_path.stat) and then read .st_size, or await
asyncio.to_thread(lambda: local_path.stat().st_size)) so that local_path.stat()
is executed off the event loop and assigned to size without blocking; update the
surrounding async function to await this call where local_path and size are
used.
In `@src/Undefined/webui/templates/index.html`:
- Around line 779-789: The toggle button with id
runtimeChatConversationDrawerToggle currently uses aria-expanded but does not
declare the controlled region; update the element referenced by id
runtimeChatConversationDrawerToggle to include
aria-controls="runtimeChatConversationDrawerPanel" so it explicitly associates
with the panel element id runtimeChatConversationDrawerPanel and improves
accessibility for assistive tech.
In `@tests/test_ai_coordinator_queue_routing.py`:
- Around line 228-237: Annotate the test locals and the fake async function: add
explicit types for captured_extra_context and captured_resources (e.g.,
dict[str, Any]), and update the signature of _fake_ask to include parameter and
return types (e.g., _args: tuple[Any, ...], kwargs: dict[str, Any] or more
specific keys like extra_context: dict[str, Any] and send_message_callback:
Callable[[str], Awaitable[None]]) and an explicit return type of str; inside the
function, keep using RequestContext.current() and captured_* updates but ensure
types match the annotations so mypy passes.
---
Nitpick comments:
In `@docs/openapi.md`:
- Around line 237-402: The WebUI AI Chat section is dense and hard to scan;
split it into clearer parts by adding a short table of contents linking to
"WebUI AI Chat(特殊私聊)", "WebUI AI Chat Conversations", "WebUI AI Chat 历史记录" and a
new "Event types" subsection; collapse long JSON examples (e.g., the large
webchat example under webchat.timeline/webchat.events) using <details> or move
them to an appendix and replace inline examples with brief summaries and
cross-references; extract the lifecycle/behavior conventions (mentions of
stage/agent_stage/tool_start/tool_end/message/done/error, webchat.calls,
webchat.timeline) into a bulleted list for quick scanning; and create a
dedicated "Schemas / Appendix" section that contains full JSON schemas for
webchat.events, webchat.calls, and history responses with links from the main
text.
In `@src/Undefined/api/webchat_store.py`:
- Around line 587-590: The local import inside _copy_json should be moved to the
module level; add "import copy" at the top of the file and remove the inline
"import copy" inside _copy_json, then simplify _copy_json to just return
copy.deepcopy(value) (keep the function name _copy_json and its signature
unchanged).
In `@src/Undefined/skills/agents/runner/tools.py`:
- Around line 14-26: Extract the duplicated helpers into a new module and update
imports: create a new file runner/webchat_utils.py and move the implementations
of _webchat_agent_path and _webchat_depth into it as public functions
webchat_agent_path and webchat_depth; then update
src/Undefined/skills/agents/runner/tools.py and loop.py to import
webchat_agent_path and webchat_depth from
Undefined.skills.agents.runner.webchat_utils (replacing usages of
_webchat_agent_path and _webchat_depth), ensuring no other references to the old
underscored names remain.
In `@tests/test_webchat_conversations.py`:
- Around line 62-78: The test test_webchat_runtime_has_detailed_flow_logs
currently reads source text from src/Undefined/api/routes/chat.py and
src/Undefined/api/webchat_store.py and asserts on hard-coded log string
literals; instead, change the test to exercise the actual runtime flow (call the
WebChat route handler functions / job creation path and relevant WebChatStore
methods) and use pytest's caplog to capture emitted log records, then assert
behavior-level messages are present in caplog.records (or caplog.text) such as
messages emitted by the route handlers in chat.py (e.g., the job creation/开始/调用
AI/logging points) and the store methods in webchat_store.py (e.g., 会话存储加载完成,
生成会话标题, 追加消息), so tests verify logs are actually emitted rather than scanning
source files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 246ce639-b107-410e-a226-1a261c586ee6
⛔ Files ignored due to path filters (1)
src/Undefined/webui/static/js/vendor/highlight.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (111)
apps/undefined-console/src-tauri/tauri.conf.jsonconfig.toml.exampledocs/callable.mddocs/cognitive-memory.mddocs/configuration.mddocs/management-api.mddocs/message-batching.mddocs/openapi.mddocs/usage.mddocs/webui-guide.mdres/IMPORTANT/each.mdres/prompts/historian_profile_merge.mdres/prompts/historian_rewrite.mdres/prompts/undefined.xmlres/prompts/undefined_nagaagent.xmlsrc/Undefined/ai/client/ask_loop.pysrc/Undefined/ai/llm/requester.pysrc/Undefined/ai/prompts/builder.pysrc/Undefined/ai/prompts/cognitive.pysrc/Undefined/ai/prompts/current_input.pysrc/Undefined/api/_helpers.pysrc/Undefined/api/_openapi.pysrc/Undefined/api/app.pysrc/Undefined/api/routes/chat.pysrc/Undefined/api/routes/commands.pysrc/Undefined/api/webchat_store.pysrc/Undefined/attachments/segments.pysrc/Undefined/cognitive/chroma_scheduler.pysrc/Undefined/cognitive/historian/worker.pysrc/Undefined/cognitive/service/service.pysrc/Undefined/cognitive/vector_store.pysrc/Undefined/cognitive/vector_store_compat.pysrc/Undefined/config/domain_parsers.pysrc/Undefined/config/models.pysrc/Undefined/main.pysrc/Undefined/services/ai_coordinator.pysrc/Undefined/services/coordinator/batching.pysrc/Undefined/services/coordinator/group.pysrc/Undefined/services/coordinator/private.pysrc/Undefined/skills/agents/README.mdsrc/Undefined/skills/agents/agent_tool_registry.pysrc/Undefined/skills/agents/arxiv_analysis_agent/handler.pysrc/Undefined/skills/agents/code_delivery_agent/handler.pysrc/Undefined/skills/agents/entertainment_agent/handler.pysrc/Undefined/skills/agents/file_analysis_agent/handler.pysrc/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.pysrc/Undefined/skills/agents/info_agent/handler.pysrc/Undefined/skills/agents/naga_code_analysis_agent/handler.pysrc/Undefined/skills/agents/runner/__init__.pysrc/Undefined/skills/agents/runner/loop.pysrc/Undefined/skills/agents/runner/tools.pysrc/Undefined/skills/agents/summary_agent/handler.pysrc/Undefined/skills/agents/web_agent/README.mdsrc/Undefined/skills/agents/web_agent/handler.pysrc/Undefined/skills/agents/web_agent/prompt.mdsrc/Undefined/skills/agents/web_agent/tools/grok_search/config.jsonsrc/Undefined/skills/agents/web_agent/tools/grok_search/handler.pysrc/Undefined/skills/commands/changelog/config.jsonsrc/Undefined/skills/tools/end/README.mdsrc/Undefined/skills/tools/end/config.jsonsrc/Undefined/skills/tools/end/handler.pysrc/Undefined/skills/toolsets/messages/react_message_emoji/handler.pysrc/Undefined/skills/toolsets/messages/send_message/handler.pysrc/Undefined/skills/toolsets/messages/send_poke/handler.pysrc/Undefined/skills/toolsets/messages/send_private_message/handler.pysrc/Undefined/skills/toolsets/messages/send_text_file/handler.pysrc/Undefined/skills/toolsets/messages/send_url_file/handler.pysrc/Undefined/utils/history.pysrc/Undefined/utils/message_turn.pysrc/Undefined/utils/paths.pysrc/Undefined/webui/app.pysrc/Undefined/webui/routes/_runtime.pysrc/Undefined/webui/static/css/app.csssrc/Undefined/webui/static/css/components.csssrc/Undefined/webui/static/css/highlight-github.min.csssrc/Undefined/webui/static/css/responsive.csssrc/Undefined/webui/static/js/api.jssrc/Undefined/webui/static/js/i18n.jssrc/Undefined/webui/static/js/main.jssrc/Undefined/webui/static/js/runtime.jssrc/Undefined/webui/static/js/ui.jssrc/Undefined/webui/static/js/vendor/highlightjs.LICENSEsrc/Undefined/webui/templates/index.htmltests/test_ai_coordinator_queue_routing.pytests/test_attachments.pytests/test_cognitive_chroma_scheduler.pytests/test_cognitive_historian.pytests/test_cognitive_service.pytests/test_cognitive_vector_store_compat.pytests/test_cognitive_vector_store_metadata.pytests/test_config_cognitive_historian_limits.pytests/test_context_recent_messages_limit.pytests/test_end_tool.pytests/test_file_analysis_attachment_uid.pytests/test_grok_search_tool.pytests/test_history_level.pytests/test_llm_retry_suppression.pytests/test_message_batcher_integration.pytests/test_prompt_builder_cognitive_query.pytests/test_prompt_builder_message_order.pytests/test_runtime_api_chat_history.pytests/test_runtime_api_chat_jobs.pytests/test_runtime_api_chat_stream.pytests/test_runtime_api_commands.pytests/test_send_message_tool.pytests/test_send_private_message_tool.pytests/test_summary_agent.pytests/test_system_prompt_constraints.pytests/test_webchat_conversations.pytests/test_webui_management_api.pytests/test_webui_runtime_chat_frontend.py
Co-authored-by: GPT-5 Codex <noreply@openai.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py (1)
187-194:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDirect aiofiles usage bypasses atomic write pattern.
Lines 193-194 use
aiofiles.open(file_path, "wb")directly instead ofutils/io.write_bytes(). This bypasses the project's atomic temp-file-then-replace pattern. While the download target is in a unique temp directory, using the standardwrite_bytes(file_path, content, use_lock=False)would ensure consistent atomic write behavior across the codebase.🔧 Recommended fix
logger.info("正在下载文件...") response = await client.get(url, timeout=120.0) response.raise_for_status() filename = _safe_download_filename( preferred_name=_extract_filename_from_url(url), fallback_prefix="file", task_uuid=task_uuid, ) file_path = temp_dir / filename - async with aiofiles.open(file_path, "wb") as f: - await f.write(response.content) + from Undefined.utils import io as async_io + await async_io.write_bytes(file_path, response.content, use_lock=False) logger.info(f"文件已保存到: {file_path}") return str(file_path)Also applies to: 237-240, 256-263
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py` around lines 187 - 194, The direct use of aiofiles.open to write response.content bypasses the project's atomic write pattern; replace those writes (in the download handling flow where filename is produced by _safe_download_filename and file_path = temp_dir / filename) with the project utility utils/io.write_bytes(file_path, response.content, use_lock=False) so the file is written via the standard temp-file-then-replace approach; apply the same replacement for the other occurrences mentioned (the blocks around the code handling lines 237-240 and 256-263) to ensure consistent atomic writes across the download_file handler.Source: Coding guidelines
🧹 Nitpick comments (1)
src/Undefined/services/coordinator/batching.py (1)
90-91: 💤 Low valueConsider removing the wrapper method.
The static method
_collect_message_idssimply delegates tocollect_message_ids(items)without adding any logic. You can callcollect_message_ids(items)directly on line 103 instead of introducing this extra indirection.♻️ Proposed simplification
- `@staticmethod` - def _collect_message_ids(items: list[BufferedMessage]) -> list[str]: - return collect_message_ids(items) - async def _dispatch_grouped_request(self, items: list[BufferedMessage]) -> None: """根据一组 BufferedMessage 决定优先级、构造 prompt 并入队。 @@ -100,7 +97,7 @@ first = items[0] last = items[-1] full_question = self._build_grouped_prompt(items) - message_ids = self._collect_message_ids(items) + message_ids = collect_message_ids(items) any_poke = any(it.is_poke for it in items) any_at_bot = any(it.is_at_bot for it in items)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/services/coordinator/batching.py` around lines 90 - 91, The method _collect_message_ids is an unnecessary delegating wrapper around the free function collect_message_ids and should be removed; delete the _collect_message_ids function and update callers (e.g., the call at line where _collect_message_ids(items) is used) to call collect_message_ids(items) directly (keeping the same argument type BufferedMessage list) and run tests to ensure no references remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py`:
- Around line 187-194: The direct use of aiofiles.open to write response.content
bypasses the project's atomic write pattern; replace those writes (in the
download handling flow where filename is produced by _safe_download_filename and
file_path = temp_dir / filename) with the project utility
utils/io.write_bytes(file_path, response.content, use_lock=False) so the file is
written via the standard temp-file-then-replace approach; apply the same
replacement for the other occurrences mentioned (the blocks around the code
handling lines 237-240 and 256-263) to ensure consistent atomic writes across
the download_file handler.
---
Nitpick comments:
In `@src/Undefined/services/coordinator/batching.py`:
- Around line 90-91: The method _collect_message_ids is an unnecessary
delegating wrapper around the free function collect_message_ids and should be
removed; delete the _collect_message_ids function and update callers (e.g., the
call at line where _collect_message_ids(items) is used) to call
collect_message_ids(items) directly (keeping the same argument type
BufferedMessage list) and run tests to ensure no references remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 649985c5-1447-4d2c-84f6-8a5c87eaf51c
📒 Files selected for processing (37)
docs/management-api.mddocs/openapi.mdres/prompts/undefined.xmlsrc/Undefined/ai/client/ask_loop.pysrc/Undefined/ai/tooling.pysrc/Undefined/api/routes/chat.pysrc/Undefined/api/routes/commands.pysrc/Undefined/api/webchat_store.pysrc/Undefined/services/ai_coordinator.pysrc/Undefined/services/coordinator/batching.pysrc/Undefined/services/coordinator/message_ids.pysrc/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.pysrc/Undefined/skills/agents/runner/loop.pysrc/Undefined/skills/agents/runner/tools.pysrc/Undefined/skills/agents/runner/webchat_utils.pysrc/Undefined/skills/toolsets/messages/context_utils.pysrc/Undefined/skills/toolsets/messages/react_message_emoji/handler.pysrc/Undefined/skills/toolsets/messages/send_message/handler.pysrc/Undefined/skills/toolsets/messages/send_poke/handler.pysrc/Undefined/skills/toolsets/messages/send_private_message/handler.pysrc/Undefined/utils/io.pysrc/Undefined/utils/paths.pysrc/Undefined/webui/app.pysrc/Undefined/webui/routes/_index.pysrc/Undefined/webui/routes/_runtime.pysrc/Undefined/webui/static/js/runtime.jssrc/Undefined/webui/templates/index.htmltests/test_ai_coordinator_queue_routing.pytests/test_cognitive_vector_store_metadata.pytests/test_file_analysis_attachment_uid.pytests/test_react_message_emoji_tools.pytests/test_runtime_api_chat_history.pytests/test_send_message_tool.pytests/test_send_poke_tool.pytests/test_send_private_message_tool.pytests/test_webchat_conversations.pytests/test_webui_runtime_chat_frontend.py
✅ Files skipped from review due to trivial changes (2)
- src/Undefined/skills/toolsets/messages/context_utils.py
- src/Undefined/skills/agents/runner/webchat_utils.py
🚧 Files skipped from review as they are similar to previous changes (19)
- src/Undefined/skills/toolsets/messages/react_message_emoji/handler.py
- src/Undefined/skills/toolsets/messages/send_private_message/handler.py
- src/Undefined/skills/toolsets/messages/send_poke/handler.py
- tests/test_cognitive_vector_store_metadata.py
- src/Undefined/skills/toolsets/messages/send_message/handler.py
- tests/test_send_message_tool.py
- tests/test_ai_coordinator_queue_routing.py
- src/Undefined/skills/agents/runner/loop.py
- src/Undefined/api/routes/commands.py
- src/Undefined/services/ai_coordinator.py
- src/Undefined/webui/templates/index.html
- tests/test_webchat_conversations.py
- tests/test_send_private_message_tool.py
- src/Undefined/skills/agents/runner/tools.py
- tests/test_runtime_api_chat_history.py
- src/Undefined/webui/routes/_runtime.py
- src/Undefined/api/routes/chat.py
- src/Undefined/ai/client/ask_loop.py
- tests/test_webui_runtime_chat_frontend.py
Co-authored-by: GPT-5 Codex <noreply@openai.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py (1)
378-378:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUnreachable code: this return statement can never execute.
The
tryblock (lines 316–369) returns on every branch, and theexcept OSErrorblock (lines 370–376) also returns. Line 378 is dead code.🧹 Proposed fix
return "错误:附件文件读取失败" - - return f"错误:无法从附件 UID {getattr(record, 'uid', '')} 解析到可下载文件"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py` at line 378, The final return returning the Chinese error message with getattr(record, 'uid', '') is dead code because every branch in the try and the OSError except already returns; remove this unreachable return (or consolidate error returns into a single fallback return inside the function) so there is no leftover unreachable statement; locate the return that produces "错误:无法从附件 UID {getattr(record, 'uid', '')} 解析到可下载文件" in handler.py and delete it or move it into the appropriate except/final fallback branch so control flow has only reachable returns.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@src/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.py`:
- Line 378: The final return returning the Chinese error message with
getattr(record, 'uid', '') is dead code because every branch in the try and the
OSError except already returns; remove this unreachable return (or consolidate
error returns into a single fallback return inside the function) so there is no
leftover unreachable statement; locate the return that produces "错误:无法从附件 UID
{getattr(record, 'uid', '')} 解析到可下载文件" in handler.py and delete it or move it
into the appropriate except/final fallback branch so control flow has only
reachable returns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2acb8463-6d79-4c19-b74f-cf93553354ef
📒 Files selected for processing (5)
src/Undefined/ai/client/ask_loop.pysrc/Undefined/ai/tooling.pysrc/Undefined/services/coordinator/batching.pysrc/Undefined/skills/agents/file_analysis_agent/tools/download_file/handler.pytests/test_file_analysis_attachment_uid.py
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Undefined/ai/tooling.py
- tests/test_file_analysis_attachment_uid.py
- src/Undefined/ai/client/ask_loop.py
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
背景
这个分支围绕管理端 WebChat 做了一轮完整增强,目标是让浏览器里的聊天入口更接近日常可用:支持多会话、可恢复任务、工具调用过程可视化、附件/代码/HTML 预览,以及更稳定的历史恢复和滚动体验。
主要改动
测试覆盖
uv run pytest tests/、uv run ruff check .、uv run mypy .,以及管理端相关检查。关注点
runtime.js和聊天路由改动较多,建议优先验证真实管理端交互路径,包括多会话切换、附件预览、工具调用折叠/展开和移动端布局。Summary by CodeRabbit
New Features
Improvements