refactor: modularize runtime for library embedding and PyPI reuse#61
Merged
Conversation
Revert the parallel-tool prompt regression, clarify that end must run in its own round after reading tool results, and defer end execution after other tools succeed instead of skipping it and forcing another LLM turn. Co-authored-by: Cursor <cursoragent@cursor.com>
When end is bundled with other tools in one turn, run the other tools normally but return an explicit rejection for end; clarify this in prompts and each.md so models do not expect end to succeed in the same round. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep assistant plain-text in messages and use a generic retry hint instead of hardcoding send_message/end, avoiding misleading follow-up tool calls. Co-authored-by: Cursor <cursoragent@cursor.com>
Wire config_class as the canonical Config, slim loader to a compat shim, and restore root lazy re-exports so library embed tests can inject config without config.toml. Co-authored-by: Cursor <cursoragent@cursor.com>
Extract ai, handlers, attachments, cognitive, memes, coordinator, onebot, and agent runner into subpackages with compatibility shims; add python-api docs and py.typed; trim noisy inline comments across runtime code. Co-authored-by: Cursor <cursoragent@cursor.com>
Strip agent-generated label comments and duplicate section headers without changing runtime behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
Extract shared meme image utilities, consolidate ingest lock model, and simplify memes API and bilibili WBI nav parsing. Co-authored-by: Cursor <cursoragent@cursor.com>
The ai/client/setup.py module is one level deeper than the old client.py, so Path(__file__).parents[1] pointed at Undefined/ai/ and loaded zero builtin tools. Centralize package root resolution and add regression tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Export set_config from the root package, update python-api.md, and expand README with a simpler core-feature bullet plus a Skills-focused embed example. Co-authored-by: Cursor <cursoragent@cursor.com>
Delete unreachable monolith files shadowed by subpackages, sync set_config with ConfigManager, declare py.typed in the wheel, and add layout regressions. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
📝 WalkthroughWalkthroughLarge reorganization: AI client split into setup/queue/ask_loop; LLM logic moved to llm/*; new Config/ConfigBuilder, build_config, sectioned load_sections and env registry; attachments split into models/segments/render; Naga API routes added; prompt/tool end-call rules tightened; docs and packaging updated. ChangesCore modularization and API/docs alignment
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Configure base_branches to .* so CodeRabbit reviews PRs targeting any branch.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
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 (1)
src/Undefined/api/routes/naga/send.py (1)
47-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject non-object JSON payloads early.
A non-dict JSON payload will crash on Line 52 (
body.get(...)) and return 500 instead of a client error.💡 Suggested fix
try: body = await request.json() except Exception: return _json_error("Invalid JSON", status=400) + if not isinstance(body, dict): + return _json_error("JSON body must be an object", status=400) bind_uuid = str(body.get("bind_uuid", "") or "").strip()🤖 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/naga/send.py` around lines 47 - 57, After parsing request.json() in the route handler, validate that the returned body is a dict/object before accessing body.get(...) and return a 400 via _json_error if it is not; specifically, after "body = await request.json()" add a check for "not isinstance(body, dict)" (or equivalent) and return _json_error("JSON payload must be an object", status=400) to avoid the subsequent body.get(...) calls (bind_uuid, naga_id, delivery_signature, uuid, target, message) from raising an exception.
🟡 Minor comments (5)
docs/configuration.md-1087-1087 (1)
1087-1087:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix heading level jump in env registry section.
#### accessjumps one level too deep from surrounding structure; this trips markdownlint and weakens TOC hierarchy.🤖 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/configuration.md` at line 1087, The "access" heading currently uses one too many #'s (shown as '#### access') which breaks the TOC and markdownlint; change that heading to the correct level (e.g., '### access') in the env registry section so it aligns with surrounding headings and restores proper hierarchy—locate the heading text 'access' and replace the four-hash form with the three-hash form to fix the level jump.docs/python-api.md-103-105 (1)
103-105:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSpecify fence language for the precedence block.
Add a language tag (e.g.,
text) to satisfy markdown linting.Suggested patch
-``` +```text Python 显式 mapping / override > config.toml > 环境变量 > 代码默认值</details> <details> <summary>🤖 Prompt for AI Agents</summary>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/python-api.mdaround lines 103 - 105, The fenced precedence line in
docs/python-api.md lacks a language tag; update the triple-backtick fence
surrounding the text "Python 显式 mapping / override > config.toml > 环境变量 >
代码默认值" to include a language specifier (e.g., changetotext) so the code
block is properly tagged for markdown linting and rendering.</details> </blockquote></details> <details> <summary>docs/deployment.md-5-7 (1)</summary><blockquote> `5-7`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Remove blank line inside the opening blockquote.** The current quote block is split by an empty line, which triggers `MD028`. <details> <summary>Suggested patch</summary> ```diff > **作为 Python 库嵌入**：若你不需要启动 QQ Bot CLI，而是要在自己的应用或测试中复用 Undefined 组件（配置、`AIClient`、Skills、认知记忆等），请参阅 [Python 库 API 参考](python-api.md) 与 [配置详解 — 库嵌入配置](configuration.md#2-库嵌入配置)。CLI 入口（`Undefined` / `Undefined-webui`）行为不受库嵌入 API 影响。 - > Python 版本要求：`3.11`~`3.13`（包含）。 ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` 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/deployment.md` around lines 5 - 7, The blockquote in docs/deployment.md is split by an empty line causing MD028; remove the blank line between the two blockquote lines so the quoted content ("作为 Python 库嵌入…CLI 入口（`Undefined` / `Undefined-webui`）行为不受库嵌入 API 影响." and "Python 版本要求：`3.11`~`3.13`（包含）。") forms a single continuous blockquote; simply join the lines so every quoted paragraph starts with '>' with no intervening blank line. ``` </details> </blockquote></details> <details> <summary>docs/configuration.md-59-61 (1)</summary><blockquote> `59-61`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add language tag to fenced code block.** This fence should specify a language (e.g., `text`) to satisfy markdown linting and keep docs tooling consistent. <details> <summary>Suggested patch</summary> ```diff -``` +```text Python 显式 mapping / builder.override > config.toml > 环境变量 > 代码默认值 ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>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/configuration.mdaround lines 59 - 61, The fenced code block lacks a
language tag; update the markdown fence containing "Python 显式 mapping /
builder.override > config.toml > 环境变量 > 代码默认值" to include a language
(e.g., add "text" after the opening ```), so the block becomes a proper fenced
code block with a language tag for markdown linting and tooling consistency.</details> </blockquote></details> <details> <summary>docs/message-batching.md-87-90 (1)</summary><blockquote> `87-90`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix relative links to source files from `docs/`.** These links currently point to `docs/src/...` when rendered. They should be prefixed with `../` to resolve correctly. <details> <summary>Suggested patch</summary> ```diff -- 实现：[src/Undefined/services/message_batcher/](src/Undefined/services/message_batcher/) +- 实现：[src/Undefined/services/message_batcher/](../src/Undefined/services/message_batcher/) @@ -- 创建/注入：[src/Undefined/handlers/message_flow.py](src/Undefined/handlers/message_flow.py) +- 创建/注入：[src/Undefined/handlers/message_flow.py](../src/Undefined/handlers/message_flow.py) ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` 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/message-batching.md` around lines 87 - 90, Update the broken relative links in docs/message-batching.md so they point to the repository source (prefix with ../); replace "src/Undefined/services/message_batcher/" with "../src/Undefined/services/message_batcher/", "src/Undefined/services/ai_coordinator.py" with "../src/Undefined/services/ai_coordinator.py" (which references functions handle_auto_reply / handle_private_reply / _dispatch_grouped_request), "src/Undefined/handlers/message_flow.py" with "../src/Undefined/handlers/message_flow.py", and "src/Undefined/main.py" with "../src/Undefined/main.py" so the links resolve correctly when rendered from docs/. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (3)</summary><blockquote> <details> <summary>docs/configuration.md (1)</summary><blockquote> `185-185`: _⚡ Quick win_ **Section numbering is out of sync after inserting new section 2.** Several headers still use old numbering (e.g., `### 4.1` under `## 5`), which makes anchors and cross-references confusing. <details> <summary>Suggested patch</summary> ```diff -### 4.1 `[core]` 机器人核心行为 +### 5.1 `[core]` 机器人核心行为 ``` ```diff -### 5.1 热更新监听对象 +### 6.1 热更新监听对象 ``` ```diff -### 5.2 明确“需重启”的字段 +### 6.2 明确“需重启”的字段 ``` ```diff -### 5.3 明确“会执行热应用”的字段 +### 6.3 明确“会执行热应用”的字段 ``` ```diff -### 5.4 其他字段 +### 6.4 其他字段 ``` (And continue this renumbering pattern for subsequent subsections.) </details> Also applies to: 1012-1012, 1065-1065 <details> <summary>🤖 Prompt for AI Agents</summary> ``` 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/configuration.md` at line 185, Section numbering is out of sync: update the misplaced header "### 4.1 `[core]` 机器人核心行为" (currently under "## 5") and any other headers like those at the noted locations (e.g., lines referenced 1012 and 1065) so that subsection numbers follow the parent section number (renumber "### 4.1" to "### 5.1" and continue the same renumbering pattern for all subsequent subsections and related anchors/cross-references); ensure any internal links or table of contents entries that reference the old numbers are updated to the new numbers to keep anchors consistent. ``` </details> </blockquote></details> <details> <summary>src/Undefined/ai/multimodal/analyzer.py (1)</summary><blockquote> `19-20`: _⚡ Quick win_ **No runtime break: `Undefined.ai.multimodal.__init__` aliases `_MEDIA_URL_*` constants before loading `analyzer`.** The analyzer’s `_multimodal_pkg._MEDIA_URL_*` lookups won’t raise `AttributeError` because `__init__.py` explicitly re-exports `_MEDIA_URL_CACHE_DIR`, `_MEDIA_URL_CACHE_TTL_SECONDS`, `_MEDIA_URL_CACHE_MAX_FILES`, `_MEDIA_URL_CACHE_CLEANUP_INTERVAL_SECONDS`, `_MEDIA_URL_DOWNLOAD_TIMEOUT_SECONDS`, and `_MEDIA_URL_DOWNLOAD_TMP_SUFFIX` from `Undefined.ai.multimodal.constants` (and includes them in `__all__`) before importing `MultimodalAnalyzer`. Optional: direct-importing these from `constants.py` could reduce indirection. <details> <summary>🤖 Prompt for AI Agents</summary> ``` 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/multimodal/analyzer.py` around lines 19 - 20, The analyzer currently relies on Undefined.ai.multimodal (referenced as _multimodal_pkg) to re-export internal constants like _MEDIA_URL_CACHE_DIR, _MEDIA_URL_CACHE_TTL_SECONDS, _MEDIA_URL_CACHE_MAX_FILES, _MEDIA_URL_CACHE_CLEANUP_INTERVAL_SECONDS, _MEDIA_URL_DOWNLOAD_TIMEOUT_SECONDS and _MEDIA_URL_DOWNLOAD_TMP_SUFFIX before loading MultimodalAnalyzer; to remove this indirection and avoid fragile aliasing, import those constants directly from Undefined.ai.multimodal.constants (or replace uses of _multimodal_pkg._MEDIA_URL_* with direct references to the constants module) in analyzer.py so MultimodalAnalyzer reads the values straight from the source (keep references to _multimodal_pkg and MultimodalAnalyzer only where needed). ``` </details> </blockquote></details> <details> <summary>src/Undefined/config/parsers/helpers.py (1)</summary><blockquote> `24-31`: _⚡ Quick win_ **Preserve admin order when deduplicating IDs** Line 27 uses `set`, which can reorder admin IDs and make downstream behavior/log output unstable. Prefer order-preserving dedupe. <details> <summary>Proposed change</summary> ```diff def _merge_admins(superadmin_qq: int, admin_qqs: list[int]) -> tuple[int, list[int]]: # admins.json 与 config.toml 的 admin_qq 合并，去重后超管必在列表中 local_admins = load_local_admins() - all_admins = list(set(admin_qqs + local_admins)) + all_admins = list(dict.fromkeys([*admin_qqs, *local_admins])) if superadmin_qq and superadmin_qq not in all_admins: all_admins.append(superadmin_qq) # 校验必填字段 return superadmin_qq, all_admins ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` 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/config/parsers/helpers.py` around lines 24 - 31, The _merge_admins function currently uses set() to deduplicate admin_qqs + local_admins which can reorder IDs; replace the set-based dedupe with an order-preserving approach (e.g., iterate admin_qqs then local_admins and build a new list using a seen set or dict.fromkeys on the concatenated sequence) so that all_admins preserves original order while removing duplicates, and still ensure superadmin_qq is appended if present and missing. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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@src/Undefined/ai/client/queue.py:
- Line 109: The code registers a pending entry in _pending_llm_calls keyed by
request_id before calling add_queued_llm_request, which can fail and leave a
stale entry; wrap the call to add_queued_llm_request in a try/except (or move
the registration until after a successful enqueue) and ensure any exception
removes the entry (pop request_id from _pending_llm_calls) and re-raises the
error; apply the same cleanup pattern for the block that currently relies on the
later pop (i.e., the code paths around add_queued_llm_request and the later
removal) so no pending entries leak on enqueue failure.In
@src/Undefined/ai/client/setup.py:
- Around line 418-425: apply_intro_config currently launches
_refresh_intro_generator as a detached asyncio task which can outlive shutdown;
change it to store and track the task (e.g., self._intro_refresh_task or add to
a background tasks set like self._bg_tasks) instead of dropping the handle,
cancel any existing tracked task before creating a new one, attach a done
callback that removes the task from tracking and logs exceptions, and ensure
close() awaits or cancels and awaits this tracked task so
_refresh_intro_generator cannot recreate resources after teardown (update
apply_intro_config, _refresh_intro_generator tracking usage, and close()
accordingly).- Around line 744-756: In request_model() the merged tool list from
tool_manager.maybe_merge_agent_tools(...) is never passed through
_filter_tools_for_runtime_config(), so runtime-hidden tools (e.g.,
naga_code_analysis_agent) can still be sent upstream; after merging (assign to
tools) and before any upstream use or the _maybe_prefetch_tools call, run the
tools through self._filter_tools_for_runtime_config(tools, call_type) (await it
if it's async) and assign the result back to tools so the filtered list is used
thereafter (affecting subsequent _maybe_prefetch_tools and any transmission).In
@src/Undefined/ai/llm/requester.py:
- Around line 158-182: The prompt cache scope builder currently places raw
stable identifiers into the cache key (see _build_scope_prompt_cache_part used
by _build_default_prompt_cache_key), which leaks user/group/sender IDs to
upstream APIs; change _build_scope_prompt_cache_part to emit an opaque token
instead of plaintext ids by hashing/encoding the id (e.g., run the numeric id
through the existing _hash8 or an HMAC with a server-side secret) and return
e.g. "group:{hash}", "private:{hash}", "sender:{hash}" (leave the global and
request_type branches unchanged) so the key still partitions correctly but no
raw identifiers are exposed.In
@src/Undefined/ai/llm/streaming.py:
- Around line 357-383: aggregate_responses_stream currently sets final_response
for any event that carries a dict "response", which causes non-terminal events
to override accumulated output and skip synthesis from
output_text_parts/output_items; to fix, remove the early unconditional
assignment "if isinstance(response, dict): final_response = response" and only
assign final_response inside the "if event_type == 'response.completed'" branch
(i.e., set final_response = response there), then keep the existing usage-merge
logic (checking usage and final_response.get("usage")) and allow the fallback
synthesis from output_text_parts/output_items when final_response remains None;
reference symbols: aggregate_responses_stream, final_response, response,
event_type, output_text_parts, output_items, usage.In
@src/Undefined/ai/multimodal/detection.py:
- Around line 31-42: The _get_media_type_by_extension function is too permissive
because it checks "if ext in url_lower" across the whole URL; instead extract
the URL path (e.g., via urllib.parse.urlparse) or derive the filename
(os.path.basename) and then match the file extension exactly (use
os.path.splitext or check path.lower().endswith(extension) with a leading dot)
against IMAGE_EXTENSIONS, AUDIO_EXTENSIONS, and VIDEO_EXTENSIONS; update the
function to operate on the parsed path/filename and perform exact extension
comparisons so filenames like "file.png" map to "image" and substrings elsewhere
in the URL no longer cause misclassification (leave the default return "image"
if no extension matches).In
@src/Undefined/ai/multimodal/parsing.py:
- Around line 11-14: _pars e_line_value currently splits generically on "：" and
":" which truncates values containing colons; change it to locate the exact
prefix in the line (use line.find(prefix)), compute the slice start as the index
after the matched prefix, then if the next character is a full‑width or ASCII
colon skip that single character and return the rest stripped (return "" if the
stripped result == "无"); apply the same fix to the other occurrence noted (the
identical parsing use at line ~37) so both places use prefix-based slicing
instead of generic split.In
@src/Undefined/ai/prompts/system_context.py:
- Around line 23-27: The code currently returns a hardcoded
"res/prompts/undefined.xml" when NagaAgent mode is off, ignoring the function's
default_path parameter; update the branch that handles enabled == False to
return the provided default_path argument (or fall back to default_path if the
runtime_config lookup fails) instead of the hardcoded string so callers of the
function (who pass default_path) get the intended path; locate the logic around
the nagaagent_mode_enabled check (the enabled variable and the if enabled: ...
return "res/prompts/undefined_nagaagent.xml") and change the final return to use
default_path.In
@src/Undefined/api/routes/naga/bind.py:
- Around line 40-47: After parsing request.json() in the route handler, validate
that the result is a mapping/object before extracting fields: check
isinstance(body, dict) (or collections.abc.Mapping) and if not return
_json_error("Invalid JSON", status=400); then safely pull bind_uuid, naga_id,
status as before. Also ensure you handle None values (treat as empty strings) so
AttributeError won't occur when accessing .get on a non-dict.In
@src/Undefined/api/routes/naga/unbind.py:
- Around line 37-44: The handler currently assumes request.json() returns a dict
and calling body.get(...) can raise AttributeError when the JSON payload is not
an object; after awaiting request.json() (in the same block where body is
assigned) add a type check (e.g., isinstance(body, dict)) and if it is not a
dict return _json_error("Invalid JSON", status=400); then proceed to extract
bind_uuid, naga_id, and delivery_signature as currently done. Ensure this
validation is applied in the same scope where body is used so _json_error is
returned for non-object JSON instead of letting body.get raise.In
@src/Undefined/attachments/render.py:
- Around line 151-160: The current logic unconditionally converts
record.local_path to a file:// URI even if the file is missing, which can
override a valid record.source_ref; update the branch that sets image_source to
Path(record.local_path).resolve().as_uri() to first check that record.local_path
points to an existing file (e.g., Path(record.local_path).exists() or is_file())
and only then set image_source from the local path; if the local file is
missing, fall back to using record.source_ref (and only emit the missing-file
replacement + raise AttachmentRenderError / append to
delivery_parts/history_parts when neither local file nor source_ref is
available), keeping existing behavior for strict, uid, delivery_parts,
history_parts, and AttachmentRenderError.- Around line 261-272: The current code treats any non-"group" target_type as a
private send (calling sender.send_private_file), which can deliver files to the
wrong channel; change the conditional in the attachments/render.py block that
uses target_type/target_id/send_record so it explicitly handles "group" and
"private" (calling sender.send_group_file or sender.send_private_file
respectively) and otherwise skips sending and logs or records an unsupported
target_type error (include target_type and target_id in the log) so unknown
types are not silently sent as private.In
@src/Undefined/config/load_sections/finalize.py:
- Around line 26-34: Guard the debug logging call so missing keys don't raise
KeyError when strict=False: before calling _log_debug_info in finalize.py, check
the strict flag or verify each context key exists (e.g.,
"chat_model","vision_model","security_model","naga_model","agent_model","summary_model","grok_model")
and only pass present values (or skip the call entirely) so _log_debug_info is
invoked only when those ctx keys exist; update the call site that currently does
_log_debug_info(ctx["chat_model"], ...) to use conditional presence checks or
ctx.get(...) and short-circuit when strict is False.In
@src/Undefined/config/load_sections/history_skills.py:
- Around line 192-203: The hot-reload timing values returned by
skills_hot_reload_interval and skills_hot_reload_debounce are unbounded and may
be non-positive; after obtaining and coercing each value via _get_value and
_coerce_float, clamp or normalize them to sensible positive minimums (e.g.,
enforce interval >= 0.1s and debounce >= 0.01s) before returning or storing so
watcher scheduling cannot be destabilized; update the handling around the
skills_hot_reload_interval and skills_hot_reload_debounce assignments to apply
this min-value check.In
@src/Undefined/config/load_sections/integrations.py:
- Around line 152-159: The code reads integer config values via
_get_value/_coerce_int (e.g., code_delivery_command_timeout and
code_delivery_max_command_output) but doesn't enforce lower bounds, allowing
zero/negative values; update the parsing to validate and clamp values to
sensible minima (for example, command timeout >= 1 second, max output >= 1 char)
and fall back to the existing defaults when values are missing or invalid; apply
the same lower-bound guards to the other code delivery fields in the same block
(the variables parsed around lines 166-178) so archive/size/retry counts cannot
be zero/negative and use defaults when out of range.In
@src/Undefined/config/load_sections/knowledge.py:
- Around line 60-64: The code currently only enforces knowledge_chunk_overlap >=
0; change the validation after computing knowledge_chunk_overlap (the block
using _coerce_int and _get_value) to ensure overlap is strictly less than
knowledge_chunk_size by capping it to at most max(knowledge_chunk_size - 1, 0)
(use the existing knowledge_chunk_size value from config) so that downstream
chunk stepping remains positive; update the block that sets
knowledge_chunk_overlap to also check knowledge_chunk_size and reduce overlap
when knowledge_chunk_overlap >= knowledge_chunk_size.In
@src/Undefined/config/load_sections/logging_tools.py:
- Around line 37-42: log_max_size_mb and log_backup_count are currently taken
directly from _get_value/_coerce_int and can be non-positive or negative; clamp
them to safe bounds after coercion: for log_max_size_mb call
_coerce_int/_get_value as today but then replace with something like
log_max_size_mb = max(1, log_max_size_mb) to ensure rotation stays enabled, and
for log_backup_count use log_backup_count = max(0, log_backup_count) to prevent
negative backups; update the assignments around the existing symbols
log_max_size_mb and log_backup_count (which call _coerce_int/_get_value) to
perform these min/max clamps immediately after coercion.
Outside diff comments:
In@src/Undefined/api/routes/naga/send.py:
- Around line 47-57: After parsing request.json() in the route handler, validate
that the returned body is a dict/object before accessing body.get(...) and
return a 400 via _json_error if it is not; specifically, after "body = await
request.json()" add a check for "not isinstance(body, dict)" (or equivalent) and
return _json_error("JSON payload must be an object", status=400) to avoid the
subsequent body.get(...) calls (bind_uuid, naga_id, delivery_signature, uuid,
target, message) from raising an exception.
Minor comments:
In@docs/configuration.md:
- Line 1087: The "access" heading currently uses one too many #'s (shown as
'#### access') which breaks the TOC and markdownlint; change that heading to the
correct level (e.g., '### access') in the env registry section so it aligns with
surrounding headings and restores proper hierarchy—locate the heading text
'access' and replace the four-hash form with the three-hash form to fix the
level jump.- Around line 59-61: The fenced code block lacks a language tag; update the
markdown fence containing "Python 显式 mapping / builder.override > config.toml环境变量 > 代码默认值" to include a language (e.g., add "text" after the opening
markdown linting and tooling consistency. In `@docs/deployment.md`: - Around line 5-7: The blockquote in docs/deployment.md is split by an empty line causing MD028; remove the blank line between the two blockquote lines so the quoted content ("作为 Python 库嵌入…CLI 入口（`Undefined` / `Undefined-webui`）行为不受库嵌入 API 影响." and "Python 版本要求：`3.11`~`3.13`（包含）。") forms a single continuous blockquote; simply join the lines so every quoted paragraph starts with '>' with no intervening blank line. In `@docs/message-batching.md`: - Around line 87-90: Update the broken relative links in docs/message-batching.md so they point to the repository source (prefix with ../); replace "src/Undefined/services/message_batcher/" with "../src/Undefined/services/message_batcher/", "src/Undefined/services/ai_coordinator.py" with "../src/Undefined/services/ai_coordinator.py" (which references functions handle_auto_reply / handle_private_reply / _dispatch_grouped_request), "src/Undefined/handlers/message_flow.py" with "../src/Undefined/handlers/message_flow.py", and "src/Undefined/main.py" with "../src/Undefined/main.py" so the links resolve correctly when rendered from docs/. In `@docs/python-api.md`: - Around line 103-105: The fenced precedence line in docs/python-api.md lacks a language tag; update the triple-backtick fence surrounding the text "Python 显式 mapping / override > config.toml > 环境变量 > 代码默认值" to include a language specifier (e.g., change ``` to ```text) so the code block is properly tagged for markdown linting and rendering. --- Nitpick comments: In `@docs/configuration.md`: - Line 185: Section numbering is out of sync: update the misplaced header "### 4.1 `[core]` 机器人核心行为" (currently under "## 5") and any other headers like those at the noted locations (e.g., lines referenced 1012 and 1065) so that subsection numbers follow the parent section number (renumber "### 4.1" to "### 5.1" and continue the same renumbering pattern for all subsequent subsections and related anchors/cross-references); ensure any internal links or table of contents entries that reference the old numbers are updated to the new numbers to keep anchors consistent. In `@src/Undefined/ai/multimodal/analyzer.py`: - Around line 19-20: The analyzer currently relies on Undefined.ai.multimodal (referenced as _multimodal_pkg) to re-export internal constants like _MEDIA_URL_CACHE_DIR, _MEDIA_URL_CACHE_TTL_SECONDS, _MEDIA_URL_CACHE_MAX_FILES, _MEDIA_URL_CACHE_CLEANUP_INTERVAL_SECONDS, _MEDIA_URL_DOWNLOAD_TIMEOUT_SECONDS and _MEDIA_URL_DOWNLOAD_TMP_SUFFIX before loading MultimodalAnalyzer; to remove this indirection and avoid fragile aliasing, import those constants directly from Undefined.ai.multimodal.constants (or replace uses of _multimodal_pkg._MEDIA_URL_* with direct references to the constants module) in analyzer.py so MultimodalAnalyzer reads the values straight from the source (keep references to _multimodal_pkg and MultimodalAnalyzer only where needed). In `@src/Undefined/config/parsers/helpers.py`: - Around line 24-31: The _merge_admins function currently uses set() to deduplicate admin_qqs + local_admins which can reorder IDs; replace the set-based dedupe with an order-preserving approach (e.g., iterate admin_qqs then local_admins and build a new list using a seen set or dict.fromkeys on the concatenated sequence) so that all_admins preserves original order while removing duplicates, and still ensure superadmin_qq is appended if present and missing.🪄 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:
ef3d8fbd-9e4c-4554-89f9-1941aee9f747📒 Files selected for processing (150)
.coderabbit.yamlARCHITECTURE.mdREADME.mddocs/configuration.mddocs/deployment.mddocs/development.mddocs/message-batching.mddocs/python-api.mdpyproject.tomlres/IMPORTANT/each.mdres/prompts/undefined.xmlres/prompts/undefined_nagaagent.xmlsrc/Undefined/__init__.pysrc/Undefined/ai/client.pysrc/Undefined/ai/client/__init__.pysrc/Undefined/ai/client/ask_loop.pysrc/Undefined/ai/client/queue.pysrc/Undefined/ai/client/setup.pysrc/Undefined/ai/llm.pysrc/Undefined/ai/llm/__init__.pysrc/Undefined/ai/llm/requester.pysrc/Undefined/ai/llm/sanitize.pysrc/Undefined/ai/llm/streaming.pysrc/Undefined/ai/llm/thinking.pysrc/Undefined/ai/llm/types.pysrc/Undefined/ai/model_selector.pysrc/Undefined/ai/multimodal/__init__.pysrc/Undefined/ai/multimodal/analyzer.pysrc/Undefined/ai/multimodal/constants.pysrc/Undefined/ai/multimodal/detection.pysrc/Undefined/ai/multimodal/parsing.pysrc/Undefined/ai/parsing.pysrc/Undefined/ai/prompts/__init__.pysrc/Undefined/ai/prompts/builder.pysrc/Undefined/ai/prompts/cognitive.pysrc/Undefined/ai/prompts/constants.pysrc/Undefined/ai/prompts/system_context.pysrc/Undefined/ai/tooling.pysrc/Undefined/ai/transports/openai_transport.pysrc/Undefined/api/routes/memes.pysrc/Undefined/api/routes/naga/__init__.pysrc/Undefined/api/routes/naga/auth.pysrc/Undefined/api/routes/naga/bind.pysrc/Undefined/api/routes/naga/send.pysrc/Undefined/api/routes/naga/unbind.pysrc/Undefined/attachments/__init__.pysrc/Undefined/attachments/models.pysrc/Undefined/attachments/registry.pysrc/Undefined/attachments/render.pysrc/Undefined/attachments/segments.pysrc/Undefined/bilibili/wbi.pysrc/Undefined/cognitive/historian/__init__.pysrc/Undefined/cognitive/historian/helpers.pysrc/Undefined/cognitive/historian/tools.pysrc/Undefined/cognitive/historian/worker.pysrc/Undefined/cognitive/job_queue.pysrc/Undefined/cognitive/profile_storage.pysrc/Undefined/cognitive/service/__init__.pysrc/Undefined/cognitive/service/helpers.pysrc/Undefined/cognitive/service/service.pysrc/Undefined/cognitive/vector_store.pysrc/Undefined/config/__init__.pysrc/Undefined/config/build_config.pysrc/Undefined/config/config_class.pysrc/Undefined/config/domain_parsers.pysrc/Undefined/config/env_registry.pysrc/Undefined/config/load_sections/__init__.pysrc/Undefined/config/load_sections/access.pysrc/Undefined/config/load_sections/core.pysrc/Undefined/config/load_sections/domains.pysrc/Undefined/config/load_sections/finalize.pysrc/Undefined/config/load_sections/history_skills.pysrc/Undefined/config/load_sections/integrations.pysrc/Undefined/config/load_sections/knowledge.pysrc/Undefined/config/load_sections/logging_tools.pysrc/Undefined/config/load_sections/models.pysrc/Undefined/config/load_sections/network.pysrc/Undefined/config/loader.pysrc/Undefined/config/manager.pysrc/Undefined/config/parsers/__init__.pysrc/Undefined/config/parsers/agent.pysrc/Undefined/config/parsers/chat.pysrc/Undefined/config/parsers/embedding.pysrc/Undefined/config/parsers/grok.pysrc/Undefined/config/parsers/helpers.pysrc/Undefined/config/parsers/historian.pysrc/Undefined/config/parsers/image.pysrc/Undefined/config/parsers/naga.pysrc/Undefined/config/parsers/pool.pysrc/Undefined/config/parsers/security.pysrc/Undefined/config/parsers/summary.pysrc/Undefined/config/parsers/vision.pysrc/Undefined/config/toml_io.pysrc/Undefined/handlers.pysrc/Undefined/handlers/__init__.pysrc/Undefined/handlers/auto_extract.pysrc/Undefined/handlers/message_flow.pysrc/Undefined/handlers/poke.pysrc/Undefined/handlers/repeat.pysrc/Undefined/memes/_image_utils.pysrc/Undefined/memes/ingest.pysrc/Undefined/memes/models.pysrc/Undefined/memes/search.pysrc/Undefined/memes/service.pysrc/Undefined/onebot/__init__.pysrc/Undefined/onebot/client.pysrc/Undefined/onebot/message.pysrc/Undefined/py.typedsrc/Undefined/services/ai_coordinator.pysrc/Undefined/services/commands/bugfix.pysrc/Undefined/services/commands/stats.pysrc/Undefined/services/coordinator/__init__.pysrc/Undefined/services/coordinator/background.pysrc/Undefined/services/coordinator/batching.pysrc/Undefined/services/coordinator/group.pysrc/Undefined/services/coordinator/private.pysrc/Undefined/services/message_batcher/__init__.pysrc/Undefined/services/message_batcher/scheduler.pysrc/Undefined/services/message_batcher/state.pysrc/Undefined/skills/agents/code_delivery_agent/tools/read/handler.pysrc/Undefined/skills/agents/runner.pysrc/Undefined/skills/agents/runner/__init__.pysrc/Undefined/skills/agents/runner/context.pysrc/Undefined/skills/agents/runner/loop.pysrc/Undefined/skills/agents/runner/tools.pysrc/Undefined/skills/tools/__init__.pysrc/Undefined/skills/tools/bilibili_video/handler.pysrc/Undefined/skills/tools/fetch_image_uid/handler.pysrc/Undefined/skills/tools/get_current_time/handler.pysrc/Undefined/skills/tools/get_picture/handler.pysrc/Undefined/skills/tools/get_user_info/handler.pysrc/Undefined/skills/tools/python_interpreter/handler.pysrc/Undefined/skills/tools/qq_like/handler.pysrc/Undefined/skills/tools/task_progress/handler.pysrc/Undefined/utils/paths.pysrc/Undefined/utils/render_cache.pysrc/Undefined/utils/resources.pysrc/Undefined/utils/sender_helpers.pysrc/Undefined/webui/routes/_runtime.pytests/test_ai_client_setup_paths.pytests/test_ai_coordinator_queue_routing.pytests/test_cli_startup_compat.pytests/test_config_env_only.pytests/test_config_env_registry.pytests/test_config_from_mapping.pytests/test_end_defer_co_call.pytests/test_handlers_meme_annotation.pytests/test_llm_retry_suppression.pytests/test_package_layout.pytests/test_public_api_imports.py💤 Files with no reviewable changes (9)
- src/Undefined/ai/parsing.py
- src/Undefined/config/domain_parsers.py
- src/Undefined/ai/model_selector.py
- src/Undefined/cognitive/job_queue.py
- src/Undefined/ai/llm.py
- src/Undefined/ai/transports/openai_transport.py
- src/Undefined/cognitive/vector_store.py
- src/Undefined/ai/client.py
- src/Undefined/cognitive/profile_storage.py
Harden queued LLM retries, pending-call cleanup, tool filtering, config bounds, attachment rendering, and privacy-safe prompt cache keys. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Warning Docstrings generation - SKIPPED Too many files! This PR contains 151 files, which is 1 over the limit of 150. To get a review, narrow the scope: |
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Contributor
|
✅ Created PR with unit tests: #62 |
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Undefined/__init__.py (1)
52-59: ⚡ Quick winKeep the lazy root exports statically typed.
Lines 52-59 route every package-level export through
__getattr__(...) -> Any, so externalfrom Undefined import Config/AIClientusers can lose concrete types even though the wheel now shipspy.typed. Please verify this surface with mypy/pyright and addTYPE_CHECKINGimports or a small__init__.pyiif needed.Example pattern
-from typing import Any +from typing import Any, TYPE_CHECKING + +if TYPE_CHECKING: + from Undefined.config import Config, get_config, set_config + from Undefined.ai import AIClient + # ...repeat for the remaining public re-exports🤖 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/__init__.py` around lines 52 - 59, The current __getattr__ fallback returns Any for lazy exports (defined in _LAZY_IMPORTS) which loses static types for consumers (e.g., Config, AIClient); fix by adding explicit TYPE_CHECKING-only imports in this module that import and re-export the real types (use "if TYPE_CHECKING: from .config import Config; from .client import AIClient") so mypy/pyright see concrete types, or alternatively add a small __init__.pyi stub that declares the exported names and their types; keep the runtime lazy behavior via the existing __getattr__ and _LAZY_IMPORTS while ensuring the type checker sees the real symbols.
🤖 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.
Nitpick comments:
In `@src/Undefined/__init__.py`:
- Around line 52-59: The current __getattr__ fallback returns Any for lazy
exports (defined in _LAZY_IMPORTS) which loses static types for consumers (e.g.,
Config, AIClient); fix by adding explicit TYPE_CHECKING-only imports in this
module that import and re-export the real types (use "if TYPE_CHECKING: from
.config import Config; from .client import AIClient") so mypy/pyright see
concrete types, or alternatively add a small __init__.pyi stub that declares the
exported names and their types; keep the runtime lazy behavior via the existing
__getattr__ and _LAZY_IMPORTS while ensuring the type checker sees the real
symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f0507d0-7d57-4561-931b-3a7080ddee82
⛔ Files ignored due to path filters (3)
apps/undefined-console/package-lock.jsonis excluded by!**/package-lock.jsonapps/undefined-console/src-tauri/Cargo.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
CHANGELOG.mdapps/undefined-console/package.jsonapps/undefined-console/src-tauri/Cargo.tomlapps/undefined-console/src-tauri/tauri.conf.jsonpyproject.tomlsrc/Undefined/__init__.py
✅ Files skipped from review due to trivial changes (4)
- apps/undefined-console/src-tauri/tauri.conf.json
- apps/undefined-console/src-tauri/Cargo.toml
- apps/undefined-console/package.json
- CHANGELOG.md
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Undefined/__init__.py (1)
25-64: ⚡ Quick winDerive
__all__from_LAZY_IMPORTSto avoid export drift.
__all__and_LAZY_IMPORTScurrently duplicate the same symbol list, which can diverge on future edits.♻️ Proposed refactor
-__all__ = [ - "__version__", - "Config", - "get_config", - "set_config", - "AIClient", - "ToolRegistry", - "AgentRegistry", - "PipelineRegistry", - "BaseRegistry", - "AnthropicSkillRegistry", - "CognitiveService", - "KnowledgeManager", - "MemeService", - "AttachmentRegistry", - "RuntimeAPIServer", - "RuntimeAPIContext", -] - -# symbol -> (module_path, attribute_name)；首次访问时才 importlib 加载 _LAZY_IMPORTS: dict[str, tuple[str, str]] = { "Config": ("Undefined.config", "Config"), "get_config": ("Undefined.config", "get_config"), @@ "RuntimeAPIServer": ("Undefined.api.app", "RuntimeAPIServer"), "RuntimeAPIContext": ("Undefined.api._context", "RuntimeAPIContext"), } + +__all__ = ["__version__", *_LAZY_IMPORTS.keys()]🤖 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/__init__.py` around lines 25 - 64, Replace the hard-coded __all__ with a derived list built from the keys of _LAZY_IMPORTS (keeping "__version__" as an explicit export) to avoid drift; update the module so "__all__" is assigned from ["__version__"] plus list(_LAZY_IMPORTS.keys()) (or equivalent that preserves insertion order) and ensure the resulting object remains a list of strings; touch the __all__ and _LAZY_IMPORTS symbols in src/Undefined/__init__.py.
🤖 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.
Nitpick comments:
In `@src/Undefined/__init__.py`:
- Around line 25-64: Replace the hard-coded __all__ with a derived list built
from the keys of _LAZY_IMPORTS (keeping "__version__" as an explicit export) to
avoid drift; update the module so "__all__" is assigned from ["__version__"]
plus list(_LAZY_IMPORTS.keys()) (or equivalent that preserves insertion order)
and ensure the resulting object remains a list of strings; touch the __all__ and
_LAZY_IMPORTS symbols in src/Undefined/__init__.py.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5035676b-2b07-480c-a486-532bda280b49
📒 Files selected for processing (1)
src/Undefined/__init__.py
Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ai/client,config,handlers,cognitive,memes, etc.) into subpackages with compatibility shims and lazy root re-exports for library useConfig.from_mapping/set_config,docs/python-api.md, and README library embed section; fixPACKAGE_ROOTskills path regression after client splitTest plan
uv run ruff format . && uv run ruff check . && uv run mypy .uv run pytest tests/test_public_api_imports.py tests/test_ai_client_setup_paths.py tests/test_config_from_mapping.py -vMade with Cursor
Summary by CodeRabbit
New Features
Documentation
Behavior Changes
Chores