feat(knowledge): add read-only RAGFlow retrieval - #4955
Conversation
d06a331 to
5341371
Compare
willem-bd
left a comment
There was a problem hiding this comment.
Reviewed at head 5341371. The overall shape is solid and follows the established community-tool conventions closely (get_tool_config + model_extra with a warned-once set mirrors brave; env refs like $RAGFLOW_API_KEY are resolved by the config loader's resolve_env_variables before validation, so the SecretStr flow works; hatchling ships the package; bounded output at chunk and response level; api-key and UUID redaction on every error path with tests). Four findings below - two suggestions on model-visible output, two convention/robustness nits. One design observation, not blocking: knowledge_search lists all datasets (paginated) before every retrieval to resolve names - fine for the no-state design, just be aware each search pays at least one extra HTTP roundtrip, and list_knowledge_bases depends on the knowledge_search entry being present even when only the listing tool is configured (the error message explains this, and the docs call it out).
There was a problem hiding this comment.
@zhangwei139623 感谢贡献。这个 PR 对应 RFC #4900 的 Phase 2(retrieval-only 切片),方向是社区呼声很高的功能(#3268 / #3302 / #2609)。测试覆盖和文档同步做得比较完整,API key 的双层 redaction(连 base_url 内嵌凭据的场景都覆盖了)考虑得很细;provider 连接配置放在 tool entry 上也正确地沿用了 web_search 的 provider 模式,这部分保持现状即可。不过存在一个正确性问题和一个设计问题,需要一轮较大的调整,逐条说明如下。
1. 全库兜底检索在 RAGFlow 的 /api/v1/retrieval 上不成立(blocking)
PR 将「省略 knowledge_bases 时不传 dataset_ids,由 RAGFlow 全库检索」作为受支持的路径,并固化在 docstring、README 和 test_retrieve_omits_dataset_ids_when_unspecified 中。但 POST /api/v1/retrieval 的第一个校验就是 dataset_ids 必填,且自 SDK HTTP API 引入以来的所有版本均如此(已核查 v0.13.0、v0.15.0、v0.17.2、v0.19.1、v0.21.1、v0.23.0、v0.25.0、v0.26.4):
- v0.26.4(RFC 的基线版本):https://github.com/infiniflow/ragflow/blob/v0.26.4/api/apps/restful_apis/chunk_api.py#L311-L320
- v0.19.1:https://github.com/infiniflow/ragflow/blob/v0.19.1/api/apps/sdk/doc.py(相同校验)
- v0.13.0(SDK API 首个版本):同样存在该校验
if not req.get("dataset_ids"):
return get_error_data_result("`dataset_ids` is required.")需要说明一个容易混淆的点:RAGFlow 官方 API 文档对 dataset_ids 的描述是与 document_ids 二选一("If you do not set this argument, ensure that you set document_ids"),但源码实现比文档更严格——该检查位于 handler 第一行,此时尚未读取 document_ids,因此只传 document_ids 同样会被拒绝;document_ids 的实际语义是在给定 datasets 范围内的进一步过滤(chunk_api.py#L335-L345)。无论按文档口径还是实现口径,「两者都不传 = 全库检索」均不成立。
如果你手上有省略 dataset_ids 可以工作的部署,请给出具体的 RAGFlow 版本号和所测端点。无论如何,实现不应依赖这里的版本或文档口径差异:始终显式传 dataset_ids(见第 2 条,它将这一点结构化)。
另外说明:这个错误断言源自 RFC 本身。RFC §5.1 写了「省略 knowledge_bases 即全库检索」并声称已对 v0.26.4 验证——这条验证是失实的,属于 RFC 的缺陷,已在 #4900 中修订。但这也正说明了 mock 测试的局限:mock 验证的是我们对 API 的假设,只有真实调用才能验证假设本身。请在下一版对真实 RAGFlow 实例完整跑通工具链,并在 PR 描述中注明版本、验证命令与输出摘要。
同源的一个小问题:formatting.py 对 chunk 字段做了 kb_id/dataset_id 双备选兼容,但已核查的所有版本都会在返回前做显式改名(kb_id → dataset_id、doc_id → document_id、docnm_kwd → document_keyword),真实响应中不会出现 kb_id:https://github.com/infiniflow/ragflow/blob/v0.26.4/api/apps/restful_apis/chunk_api.py#L424-L433 。除非你能指出确实返回 kb_id 的版本,否则请写死字段并在注释中注明验证版本。
2. 在 tool 配置上显式绑定 dataset,取消动态目录(blocking)
结合第 1 条,这个切片的正确形态是:operator 在 knowledge_search 的 tool entry 上(与现有连接配置并列)按名字绑定一个或多个 dataset,工具总是解析这些名字并显式传 dataset_ids。理由:
- 租户级 API key 可见租户下所有 dataset,当前动态
list_datasets等于把整个租户目录无差别暴露给每个 Agent;显式绑定是 allowlist,由 operator 决定 Agent 的可读范围; - operator 可在配置时一次性保证所绑 dataset 的 embedding model 兼容,消除跨库检索的运行时报错(chunk_api.py#L325-L327);
- 每次搜索减少一次串行的全量分页
list_datasets调用(当前实现即使模型未指定库名也会先全量拉取目录用于格式化),且第 1 条从此结构性成立——dataset_ids永远存在; - 为后续 per-agent / per-user 的 dataset 隔离铺路。
相应地,list_knowledge_bases 工具不再必要,应移除;绑定的库名直接写入 knowledge_search 的工具描述。这同时消除了当前的耦合——list_knowledge_bases 的 entry 是空壳、运行时借读 knowledge_search 的配置;调整后整个集成收敛为一个自足的 tool entry,与 web_search provider 完全同构,将来换成其他引擎(Dify 等)仍只是改一行 use:。
说明:这与 RFC §5.1 的原方案相反——原方案否决配置绑定、选择动态目录,依据的正是第 1 条中的错误假设。该裁决已作为 RFC 修订记录在 #4900,本 PR 按修订后的方向实现即可。
dataset 存在性:配置加载时不做网络校验(config 校验不应有网络 IO);在调用时按名解析(GET /datasets?name= 支持按名过滤),解析失败则返回明确错误,提示绑定的 dataset 可能已被删除或改名、需检查 config.yaml。
3. 模型可见文本应使用英文(blocking)
client.py 抛出的异常消息与 tools.py 返回的错误串目前为中文(如「请求超时」「未检索到相关内容」)。community/ 下现有工具的模型可见错误均为英文("Error: ..." 形式),请保持一致。
4. Non-blocking
_RAGFLOW_UUID_PATTERN会把模型可见文本中所有 32 位 hex / 标准 UUID 格式的内容替换为[DATASET_ID],且该替换同样作用于knowledge_search的正常检索输出——检索正文中合法的 MD5 校验和、trace id、UUID 字面量会被破坏,引用保真度受损,替换标记还会误导模型将其理解为知识库 ID。正常路径上formatting.py已将 dataset UUID 映射为库名、并无泄漏面,该正则应收缩到仅错误路径(RAGFlow 的报错确实可能回显 dataset UUID,那里值得保留);- 每次请求新建
httpx.AsyncClient,无连接复用;v1 可接受,后续 Gateway 侧接入时再统一考虑; - 按第 2 条返工时,config_version 的改动请合并到同一版中,避免连续两次 bump。
|
@rayhpeng 已按这轮 review 和 RFC #4900 的 v2 修订完成返工,提交为
PR 描述已更新真实版本、验证命令和输出摘要。定向测试 32 passed,harness 边界/打包 3 passed,完整 Ruff lint/format 通过。烦请复核。 |
|
@rayhpeng Follow-up: the two remaining review-evidence gaps are now closed. |
|
@rayhpeng 按最新反馈已将 |
|
@rayhpeng 已按后续意见补充默认兜底行为(commit
PR 描述也已更新为新的可选配置和真实验证结果。 |
|
@zhangwei139623 感谢持续跟进。经维护者侧讨论,当前版本的两个方向性设计予以确认: E2E 验证(基于
|
| # | 场景 | 结果 |
|---|---|---|
| T1 | 绑定 HR Policies 正向检索 |
✅ [1] HR Policies / leave-policy.txt (score 0.44) + 正确内容 |
| T2 | 检索正文中 UUID/MD5 保真 | ✅ 均原样保留,error-only 脱敏有效 |
| T3 | 绑定不存在的 dataset | ❌ 见下文问题 2 |
| T4 | dataset UUID / API key 泄漏 | ✅ 均未出现 |
| T6 | 空 query 拦截 | ✅ |
| C1 | 裸 API:省略 dataset_ids |
✅ code=102 "dataset_ids is required." |
| C2 | 裸 API:GET /datasets?name=<不存在> |
code=102 "User '...' lacks permission for dataset '...'",不是空列表,见问题 2 |
问题 1(blocking):默认全量检索必须说明并处理混合 embedding 失败
RAGFlow 拒绝跨 embedding 模型的检索——服务端对本次检索涉及的所有 dataset 取 embedding 模型集合,不唯一即直接报错(chunk_api.py#L325-L327):
embd_nms = list(set([split_model_name(kb.embd_id)[0] for kb in kbs]))
if len(embd_nms) != 1:
return get_result(message="Datasets use different embedding models.", code=RetCode.DATA_ERROR)RAGFlow 每个 dataset 建库时可独立选择 embedding 模型,租户运行一段时间后异构是常态。也就是说默认全量行为会随部署状态从"能用"退化为"每次检索必失败":单库正常,第二个异构库一建,所有未配置 datasets 的部署立即开始报 Datasets use different embedding models.。默认行为保留,但这个风险必须显式处理,要求:
-
文档 note:
config.example.yaml的配置注释、README、backend/docs/CONFIGURATION.md三处明确说明——默认全量要求所有可见 dataset 使用相同 embedding 模型,异构租户必须配置datasets缩小范围; -
运行时指引:默认全量路径下捕获该错误(按 RAGFlow 错误码,不要匹配消息文本),将模型可见错误包装为可操作指引,例如
Error: The accessible datasets use different embedding models; configure knowledge_search.datasets to a subset that shares one embedding model.——否则 operator 只能看到 RAGFlow 原文,不知道解法在配置里; -
为该场景补一条测试(mock 按真实响应形状:
code=102+ 该 message),并在真实实例上验证一次(建两个不同 embedding 的库即可复现)。
问题 2(blocking):missing-dataset 指引是死代码,已三个版本未修
实现假设"绑定的 dataset 不存在时,GET /datasets?id= 返回空列表",并据此设计了 "...check knowledge_search.datasets in config.yaml" 的指引。但真实 API 在 miss 时返回的是 code != 0 错误而非空列表——C2 是我在 v0.27.0 真实实例上的实测,id 分支与 name 分支是相邻的同款代码(v0.27.0 dataset_api_service.py#L433-L439):
if kb_id:
kbs = KnowledgebaseService.get_kb_by_id(kb_id, tenant_id)
if not kbs:
return False, f"User '{tenant_id}' lacks permission for dataset '{kb_id}'"因此 client 抛出的是 RAGFlowAPIError,走 _tool_error 通用分支,精心设计的配置指引从未触发。T3 实测的模型可见输出为:
Error: User '[DATASET_ID]' lacks permission for dataset 'Nonexistent Dataset E2E'
三个叠加的问题:指引死代码;RAGFlow 回显的 tenant id 被 UUID 正则误标为 [DATASET_ID](模型会误读为"某个 dataset 无权限");错误是权限口径,operator 得不到任何可操作信息。test_missing_bound_dataset_returns_operator_guidance 的 mock(返回空列表)固化的是真实 API 不会产生的行为——这一假设已以相同姿势在 v1(全库兜底)、v2(按名 miss)、v3(按 ID miss)三个版本漏网。
修复建议:
-
在解析阶段捕获
RAGFlowAPIError并统一包装为 missing-dataset 指引(按阶段捕获,不要匹配错误消息文本——那是 RAGFlow 内部实现); -
[DATASET_ID]标记建议改为语义中性的[REDACTED_ID]——该正则实际命中的是任意 32 位 hex id(本例中是 tenant id); -
测试 mock 改为真实响应形状(
code=102错误),并在真实实例上补一条"绑定已删除/不存在的 dataset ID"的验证,验收标准:工具返回指向knowledge_search.datasets的配置指引。今后凡涉及 RAGFlow 响应形状的断言,请一律以真实调用为准——mock 只能验证我们对 API 的假设,不能验证假设本身。
Non-blocking
-
绑定 ID 后模型可见的工具描述不再包含可检索范围,模型只能盲调;建议描述中至少静态说明作用域规则(已配置=受限范围,未配置=全部可见库),当前文案已接近,保持即可;
-
解析阶段
asyncio.gather未设异常策略,首个异常传播时其余任务结果被丢弃且可能产生 "Task exception was never retrieved" 噪音;绑定条目通常很少,顺序解析即可; -
每次请求新建
AsyncClient(N 个绑定 + 1 次检索 = N+1 条连接),v1 可接受,Gateway 侧接入时再统一。
|
@zhangwei139623 看了 1. 上一条 review 的问题 2(missing-dataset)仍未修复,这是第四个版本原样保留(blocking,请最优先处理)
修复要求与上一条 review 相同,这里重申验收标准:
请在下一轮提交中优先完成这一项,再做其他改动。 2. 新问题:
|
|
@rayhpeng 已在
真实 RAGFlow v0.27.0 已验证:
测试:RAGFlow targeted |
willem-bd
left a comment
There was a problem hiding this comment.
Requesting changes for three scope/error-handling issues. The targeted RAGFlow tests and Ruff checks pass, but the empty-allowlist behavior is fail-open and should be addressed before merge.
|
@zhangwei139623 1ad7f59 已核查:问题 1(missing-dataset 解析阶段捕获 + 序号定位 + 测试改真实形状)、问题 2(空 embedding 死库跳过、有 chunk 缺元数据仍 fail loud)、Minor 的 score 处理均验收通过,PR 描述的真实验证记录也符合要求。 另外,处理一下上述 @willem-bd 的修改建议 |
1ad7f59 to
1f177a5
Compare
|
@zhangwei139623
文档一条(README/AGENTS.md)经维护者侧讨论撤回:agent guides 保持精简、feature 细节由 CONFIGURATION.md 承载的现状可以接受,不再要求恢复。 我这边的 review 范围没有遗留问题,准备 approve。剩余事项在 @willem-bd 的 changes-requested 里,我补充三点意见: 1. 2. 解析阶段错误一刀切——同意。 当前 3. 分页 total 字段——现有实现是正确的,这条建议不成立,但可做防御性兼容。 @willem-bd 引用的 if total is not None:
response["total_datasets"] = total我在真实 v0.27.0 实例上做分页验证时也确认过该字段名。误导源是 第 1、2 条修复落地后,本 PR 从我的角度即可合入。感谢几轮快速迭代——从首版到现在,工具链在真实 RAGFlow 上的行为已经完整验证过一遍。 |
Summary
httpxclient for read-only RAGFlow retrievalknowledge_search(query); there is no Agent-visible tenant catalog toolbase_url,api_key, retrieval limits, and an optional stable dataset-ID allowlist on the normalknowledge_searchtool entrydataset_idslist toPOST /api/v1/retrievaldatasetsis omitted, list every tenant-visible dataset, skip empty datasets, and group searchable datasets by exact embedding-model identifierpage_size, and omit score labels on multi-group output because scores are not comparable across embedding spacesConfiguration
datasetsis an optional operator-controlled stable-ID allowlist. IDs remain valid when a dataset is renamed. If it is omitted, DeerFlow lists all pages at invocation time and explicitly forwards every searchable ID, grouped by embedding model. Configuration loading remains network-free. Dataset IDs and catalog listing are never exposed to the Agent.API contract and live validation
The implementation follows RAGFlow's official List datasets and Retrieval contracts. RAGFlow v0.26.4 and v0.27.0 expose the normalized response fields consumed here and support the
idfilter used by the configured allowlist path.Validated on 2026-08-25 against a real local
infiniflow/ragflow:v0.27.0stack, not a mock.Missing configured dataset
A deliberately nonexistent configured ID produced the real RAGFlow response shape (
code=102, provider text containinglacks permission). The tool converted it during dataset resolution to:The provider permission text and opaque ID were absent from model-visible output; the full configured ID and
code=102remained in the server warning log.Empty embedding dataset and heterogeneous groups
The live tenant contained an empty dataset whose API representation was
embedding_model: ""andchunk_count: 0. A temporary v2 dataset was then created, one document was uploaded and parsed to one chunk, and it was searched together with the existing v3 dataset through the default all-dataset path.Both temporary datasets were deleted after validation. A separate real negative control that omitted
dataset_idsreturned RAGFlowcode=102, confirming that the field must always be explicit.Tests
uv run pytest tests/test_ragflow_client.py tests/test_ragflow_tools.py -q— 43 passedmake test— 12,348 passed, 78 skipped; two pre-existing environment-only failures remained outside this change: public GitHub DNS resolution and the developer checkout's local SQLite checkpointer/config isolation