Skip to content

fix(rag): fix document ID Python compatibility and respect defaultConfig limit#1362

Open
Fruank4 wants to merge 1 commit into
agentscope-ai:mainfrom
Fruank4:worktree-fix+rag-document-id-and-limit
Open

fix(rag): fix document ID Python compatibility and respect defaultConfig limit#1362
Fruank4 wants to merge 1 commit into
agentscope-ai:mainfrom
Fruank4:worktree-fix+rag-document-id-and-limit

Conversation

@Fruank4

@Fruank4 Fruank4 commented May 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Document.generateDocumentId: 将 getContent()(返回 ContentBlock 对象)替换为 getContentText()(返回纯文本字符串)。原来 Jackson 序列化后的 JSON key 是 {"content":{"text":"...","type":"text"}},而 Python 端的 _map_text_to_uuid 用的是 {"content":"..."},导致同一份文档 Java/Python 生成的 UUID 不同,跨语言共用向量库时文档无法正确匹配。
  • KnowledgeRetrievalTools.retrieveKnowledge: LLM 未传 limit 参数时,从硬编码的 5 改为使用 defaultConfig.getLimit(),使构建时配置的 limit 生效。

Test plan

  • DocumentTest#testDocumentIdUsesTextNotContentBlockObject:验证 ID 基于文本字符串而非 ContentBlock 对象结构生成
  • KnowledgeRetrievalToolsTest#testNullLimitFallsBackToDefaultConfig:验证 null limit 时使用 defaultConfig.getLimit()
  • KnowledgeRetrievalToolsTest#testExplicitLimitOverridesDefault:验证显式传入的 limit 优先于 defaultConfig
  • 原有 DocumentTestKnowledgeTestReActAgentRAGConfigTest 全部通过

🤖 Generated with Claude Code

@Fruank4 Fruank4 requested a review from a team May 9, 2026 07:34
…ltConfig limit

- Document.generateDocumentId: use getContentText() instead of getContent()
  so the JSON key contains a plain string matching the Python implementation's
  _map_text_to_uuid, ensuring Java and Python generate identical UUIDs for the
  same document content when sharing a vector store.

- KnowledgeRetrievalTools.retrieveKnowledge: fall back to
  defaultConfig.getLimit() instead of hardcoded 5 when the LLM omits the
  limit parameter, so the limit configured at construction time is honoured.

Add tests: DocumentTest.testDocumentIdUsesTextNotContentBlockObject and
KnowledgeRetrievalToolsTest covering both limit fallback and explicit override.
@Fruank4 Fruank4 force-pushed the worktree-fix+rag-document-id-and-limit branch from 24df01e to e14d687 Compare May 9, 2026 07:40
@codecov

codecov Bot commented May 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@LearningGp LearningGp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Currently, DocumentMetadata.getContentText() calls content.toString() for non-TextBlock elements. Because these subclasses do not override toString(), this falls back to Object.toString(), which appends the identity hash code (e.g., io.agentscope.core.message.ImageBlock@1f23a4b).

The Impact:
This means that instantiating the same image twice will result in different IDs. This directly violates the Document class Javadoc's promise of a "deterministic UUID for the same content," rendering cross-process and cross-instance deduplication impossible for multimodal documents.

Suggestion:
Restrict the plain-text path exclusively to TextBlock to maintain compatibility with Python. For all other ContentBlock types, we should use Jackson structured serialization. Since these subclasses already have @JsonProperty annotations, they are inherently stable and safe to serialize.

@AgentScopeJavaBot AgentScopeJavaBot added bug Something isn't working area/core/tool Tool, skill, RAG abstractions labels May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/core/tool Tool, skill, RAG abstractions bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants