Skip to content

fix(kb): improve retrieval resilience on per-KB failure and rerank selection#9122

Merged
Soulter merged 2 commits into
AstrBotDevs:masterfrom
lxfight:fix/kb-retrieval-resilience
Jul 3, 2026
Merged

fix(kb): improve retrieval resilience on per-KB failure and rerank selection#9122
Soulter merged 2 commits into
AstrBotDevs:masterfrom
lxfight:fix/kb-retrieval-resilience

Conversation

@lxfight

@lxfight lxfight commented Jul 3, 2026

Copy link
Copy Markdown
Member

Modifications / 改动点

  • astrbot/core/knowledge_base/retrieval/manager.py:

    • 单知识库稠密检索失败时不再抛出 RuntimeError,改为记录日志并跳过错的知识库,与其他知识库的行为保持一致,避免单点故障导致整体检索崩溃
    • 简化 rerank provider 选择逻辑:移除多余的 rerank_pi == rerank_provider.meta().id 检查(provider 本身就是从该 ID 加载的),以及不必要的嵌套条件
  • This is NOT a breaking change. / 这不是一个破坏性变更。

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

tests/ -k "kb" 19 passed

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.txtpyproject.toml 文件相应位置。

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

Summary by Sourcery

Improve knowledge base retrieval resilience and simplify rerank provider selection.

Bug Fixes:

  • Prevent single knowledge base dense retrieval failures from raising runtime errors by logging and skipping faulty knowledge bases instead.

Enhancements:

  • Simplify rerank provider detection by selecting the first available provider directly and making vec_db access more robust when retrieving rerank providers.

…lection

- Remove RuntimeError on single-KB dense retrieval failure: always skip
  the faulty KB and return partial results instead of crashing.
- Simplify rerank provider selection: remove redundant rerank_pi check
  since the provider is already loaded from that ID.
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jul 3, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In _dense_retrieve, changing single-KB failures from raising to just logging and skipping alters error visibility; consider at least surfacing a clear signal to the caller (e.g., a special result or aggregated error info) so that a completely failed retrieval is distinguishable from an empty successful one.
  • When logging dense retrieval failures, including the exception type and relevant KB configuration (e.g., provider IDs or vec_db meta info) in the log message would make operational debugging of per-KB failures easier.
  • In the rerank selection loop, you can short-circuit earlier by iterating directly over kb_options.values() instead of using kb_id indexing, which would simplify the code further and avoid repeated dictionary lookups.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `_dense_retrieve`, changing single-KB failures from raising to just logging and skipping alters error visibility; consider at least surfacing a clear signal to the caller (e.g., a special result or aggregated error info) so that a completely failed retrieval is distinguishable from an empty successful one.
- When logging dense retrieval failures, including the exception type and relevant KB configuration (e.g., provider IDs or vec_db meta info) in the log message would make operational debugging of per-KB failures easier.
- In the rerank selection loop, you can short-circuit earlier by iterating directly over `kb_options.values()` instead of using `kb_id` indexing, which would simplify the code further and avoid repeated dictionary lookups.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request simplifies the reranking provider selection logic and modifies error handling in dense retrieval to skip faulty knowledge bases instead of raising a RuntimeError when only one knowledge base is queried. Feedback points out that simplifying the reranking logic by removing the ID consistency check could lead to executing reranking when disabled or using an expired reranker, and suggests retaining the validation checks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread astrbot/core/knowledge_base/retrieval/manager.py Outdated
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jul 3, 2026
@Soulter Soulter merged commit 468eea9 into AstrBotDevs:master Jul 3, 2026
21 checks passed
BegoniaHe pushed a commit to BegoniaHe/AstrBot that referenced this pull request Jul 4, 2026
…lection (AstrBotDevs#9122)

* fix(kb): improve retrieval resilience on per-KB failure and rerank selection

- Remove RuntimeError on single-KB dense retrieval failure: always skip
  the faulty KB and return partial results instead of crashing.
- Simplify rerank provider selection: remove redundant rerank_pi check
  since the provider is already loaded from that ID.

* fix(kb): simplify rerank selection loop, improve dense retrieval error log
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants