fix(kb): resolve kb_id path parameter in dashboard retrieve endpoint

[JosephTian876]

The knowledge base retrieval panel in the WebUI is completely unusable: every
search fails with 缺少参数 kb_names 或格式错误.

POST /api/v1/knowledge-bases/{kb_id}/retrieve identifies the knowledge base
with the kb_id path parameter, and the route already forwards it:

# astrbot/dashboard/api/knowledge_bases.py:295
@router.post("/knowledge-bases/{kb_id}/retrieve")
async def retrieve_knowledge_base(kb_id: str, payload: KnowledgeBaseRetrieveRequest, ...):
    body = payload.model_dump(exclude_none=True)
    return await _run(lambda: service.retrieve({"kb_id": kb_id, **body}), prefix="检索失败")

But KnowledgeBaseService.retrieve() only ever read kb_names, which this
route never supplies (KnowledgeBaseRetrieveRequest has no such field, and the
dashboard client sends only query / top_k / score_threshold). So kb_names
was structurally always None and the request was rejected before any lookup
happened — reproducible 100% of the time, for any knowledge base and any query.

The service's retrieve() was written for the legacy POST /api/kb/retrieve
endpoint, which does pass kb_names in the body. When the v1 REST route was
introduced in #8688, the kb_id → kb_names translation was never added.

Modifications / 改动点

astrbot/dashboard/services/knowledge_base_service.py — when kb_names is
absent, resolve the kb_id the route already passes into the kb_names the
manager expects:

if not kb_names:
    # The REST route identifies the knowledge base with the kb_id path
    # parameter, so resolve it into the kb_names the manager expects.
    kb_id = payload.get("kb_id")
    if kb_id:
        kb_helper = await kb_manager.get_kb(kb_id)
        if not kb_helper:
            raise KnowledgeBaseServiceError("知识库不存在")
        kb_names = [kb_helper.kb.kb_name]

Resolving through the name is safe because KnowledgeBaseManager.create_kb()
already enforces unique kb_name (知识库名称 '{kb_name}' 已存在).

An explicit kb_names still takes precedence, so the legacy
POST /api/kb/retrieve endpoint keeps working unchanged.

tests/unit/test_knowledge_base_service_contract.py — 4 regression tests:
resolution from kb_id, the route→service binding, the legacy kb_names path,
and the two error cases.

• This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

The new tests fail on master and pass with the fix (same test code both runs):

# master, with the new tests applied but WITHOUT the source fix
$ uv run pytest tests/unit/test_knowledge_base_service_contract.py -q
FAILED tests/unit/test_knowledge_base_service_contract.py::test_retrieve_resolves_kb_id_from_path_parameter
FAILED tests/unit/test_knowledge_base_service_contract.py::test_retrieve_route_passes_kb_id_to_service
FAILED tests/unit/test_knowledge_base_service_contract.py::test_retrieve_raises_when_kb_id_is_unknown
3 failed, 15 passed in 4.70s

# with the fix
$ uv run pytest tests/unit/test_knowledge_base_service_contract.py -q
18 passed, 1 warning in 3.91s

Surrounding suites, no new failures:

$ uv run pytest tests/unit/test_kb_manager_resilience.py tests/unit/test_kb_document_cleanup.py \
    tests/unit/test_kb_upload_atomicity.py tests/unit/test_dashboard_util.py \
    tests/test_fastapi_v1_dashboard.py tests/test_dashboard.py tests/test_kb_import.py -q
3 failed, 186 passed in 62.87s

The 3 failures are the pre-existing tests/test_dashboard.py::test_t2i_* cases;
they fail identically on a clean master checkout and are unrelated to this change.

$ uv run ruff format --check .
483 files already formatted
$ uv run ruff check .
All checks passed!

No API schema or OpenAPI definition changed, so no frontend client regeneration
is needed.

---

Checklist / 检查清单

• 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  / 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。

• 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  / 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。

• 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
  / 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到 requirements.txt 和 pyproject.toml 文件相应位置。

• 😮 My changes do not introduce malicious code.
  / 我的更改没有引入恶意代码。

Summary by Sourcery

Fix knowledge base retrieval to work with the v1 dashboard endpoint by resolving kb_id path parameters into kb_names and enforcing proper error handling when no knowledge base is specified.

Bug Fixes:

• Resolve kb_id from the dashboard retrieve endpoint into kb_names so knowledge base searches succeed instead of failing with missing-parameter errors.
• Return a clear error when the provided kb_id does not exist or when no knowledge base identifier is supplied.

Tests:

• Add regression tests covering kb_id-to-kb_names resolution, route-to-service integration, explicit kb_names behavior, and error cases for unknown or missing knowledge base identifiers.

[dosubot]

📄 Knowledge review

Dosu skipped reviewing this PR because your organization has used its 200 included credits for the month. Your usage will reset on 2026-08-01. To have Dosu review this PR before then, ask your organization admin to upgrade to a pro account.

---

[sourcery-ai]

Hey - I've left some high level feedback:

• In KnowledgeBaseService.retrieve, when both kb_names and kb_id are provided, kb_names silently wins; consider either validating that they are consistent (same KB) or explicitly documenting and enforcing the precedence to avoid confusing, hard-to-debug situations.
• The retrieval logic now depends on raw dict access to kb_id (payload.get("kb_id")); if this pattern grows, consider introducing a small typed request object or helper to centralize the kb_id → kb_names resolution and reduce the implicit contract on the data shape.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `KnowledgeBaseService.retrieve`, when both `kb_names` and `kb_id` are provided, `kb_names` silently wins; consider either validating that they are consistent (same KB) or explicitly documenting and enforcing the precedence to avoid confusing, hard-to-debug situations.
- The retrieval logic now depends on raw `dict` access to `kb_id` (`payload.get("kb_id")`); if this pattern grows, consider introducing a small typed request object or helper to centralize the `kb_id` → `kb_names` resolution and reduce the implicit contract on the `data` shape.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[JosephTian876]

Thanks for the review. Addressed the first point, and I'd like to push back on the second.

> when both kb_names and kb_id are provided, kb_names silently wins

Fair — the precedence was only stated in the PR description, not in the code. KnowledgeBaseRetrieveRequest declares no kb_names field, but it extends OpenModel (extra="allow"), so a client can smuggle one into the REST route's body and override the path parameter.

I've documented it in 6b9cb8a rather than rejecting the combination, because the two are not interchangeable: kb_names is a list and is how the legacy POST /api/kb/retrieve endpoint searches several bases at once, while kb_id addresses exactly one. Consistency validation would have to define what "consistent" means for {"kb_id": "a", "kb_names": ["a", "b"]}, and no client sends both today. retrieve() also had no docstring at all, which AGENTS.md asks for, so this fills that gap too.

> consider introducing a small typed request object or helper to centralize the kb_id → kb_names resolution

I'd rather not, as it would work against the repo's own guidelines:

• AGENTS.md → No Unnecessary Helpers: "Inline-First Rule: If a logic block can be implemented directly within the main function without breaking overall readability, do not extract it into a new helper function", and a helper needs "High Reuse: The exact same logic is repeated across 3 or more different locations". This resolution happens in exactly one place.
• Every sibling method in KnowledgeBaseService (create_kb, update_kb, delete_chunk, list_chunks, …) reads its input the same way via payload.get(...). A typed object here only would make this one method inconsistent with the rest of the service.

The suggestion is prefixed with "if this pattern grows" — agreed, and at that point the right fix is converting the whole service at once, not just this method. Happy to revisit if a maintainer prefers otherwise.