feat(ragflow): support request-scoped retrieval filters

[Ycymac]

AgentScope-Java Version

2.0.3-SNAPSHOT

Description

Closes #686.

RAGFlow retrieval filters were bound to RAGFlowConfig when a RAGFlowKnowledge instance was created, so application code could not change datasetIds or metadataCondition between retrieval calls without rebuilding the knowledge or agent setup.

This PR:

• adds request-scoped datasetIds and metadataCondition overloads to RAGFlowKnowledge and RAGFlowClient;
• keeps the existing APIs and configuration validation unchanged;
• uses non-empty request values when provided and falls back to RAGFlowConfig for null or empty values;
• copies dataset ID lists and top-level metadata-condition maps before deferred execution; nested mutable metadata values are not deep-copied;
• adds coverage for two consecutive dynamic filter requests on one knowledge instance, config fallback, and the document-only request shape.

Request semantics

For a document-only configuration, the previous client serialized "dataset_ids": []. The updated client omits dataset_ids when the effective list is empty, matching the documented RAGFlow retrieval contract that allows document_ids to be provided instead.

Validation

mvn -pl agentscope-extensions/agentscope-extensions-rag/agentscope-extensions-rag-ragflow -am '-Dtest=RAGFlow*Test' '-DfailIfNoTests=false' test

Result: 98 tests passed, 0 failures, 0 errors, 0 skipped. Spotless also passed as part of the Maven build.

Checklist

• Code formatting passes Spotless
• All tests are passing (mvn test)
• Javadoc comments are complete and follow project conventions
• Related documentation has been updated (e.g. links, examples, etc.)
• Code is ready for review

[codecov]

Codecov Report

❌ Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...pe/core/rag/integration/ragflow/RAGFlowClient.java	84.61%	0 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

[Ycymac]

CI rerun report for the unchanged commit 7959d6d1:

• The initial workflow run 31306550141 passed on both Ubuntu and Windows.
• The first reopen run 31307227664 reproduced the unrelated agentscope-harness temporary-directory cleanup race. Ubuntu failed in HarnessAgentDynamicHookBuilderTest.disableDynamicSkills_keepsWorkspaceLazyResourcesLoadable; Windows logged the same cleanup error in HarnessAgentSubagentStreamTest.call_localSubagent_returnsReplyWithoutStreaming before being cancelled by fail-fast.
• A second reopen run 31307826560 reproduced it again. Ubuntu failed in HarnessAgentSubagentStreamTest.call_localSubagent_returnsReplyWithoutStreaming—the same test that logged the Windows cleanup error in the previous run. Windows was cancelled by fail-fast before producing an independent result.

In each completed failure, the test body had no assertion failure. JUnit failed while closing the extension context because its temporary directory could not be deleted (DirectoryNotEmptyException). The latest Ubuntu run reported 812 tests, 0 failures, and 1 cleanup error. The RAGFlow module was not reached after the Harness failure and is marked skipped in the reactor summary.

The focused RAGFlow suite still passes locally: 97 tests, 0 failures, 0 errors. One all-green run followed by two unchanged-SHA runs whose cleanup error moved between Harness tests and operating systems is strong evidence of a repository-level flaky Harness cleanup race rather than a failure caused by this PR's three RAGFlow files. No code change has been made in response to these CI runs.

[AgentScopeJavaBot]

🤖 AI Review

This PR adds request-scoped datasetIds and metadataCondition overloads to RAGFlowKnowledge and RAGFlowClient, solving the problem of having to rebuild a knowledge instance to change retrieval filters between calls. The implementation is clean: old methods delegate to new ones, defensive copies are made before the deferred Mono.fromCallable execution, and fallback to config values works correctly. Test coverage is solid with both dynamic-filter and config-fallback scenarios. One subtle behavior change in how dataset_ids is serialized should be noted.

[AgentScopeJavaBot]

[minor] Subtle behavior change: the old code unconditionally put dataset_ids in the request body (even when config.getDatasetIds() was null or empty), while the new code omits the field entirely when the effective list is empty. This is actually more correct per the RAGFlow API contract ("Required unless document_ids is provided"), but the PR description states "keeps the existing APIs and configuration validation unchanged." Consider mentioning this in the PR description for reviewer awareness, and confirming that the RAGFlow server handles a missing dataset_ids field the same way it handled "dataset_ids": null.

[AgentScopeJavaBot]

[nit] The shallow copy via new HashMap<>(metadataCondition) protects against top-level mutations (put/remove), but nested mutable values inside the map are still shared between the caller and the deferred request. The Javadoc states "Parameter values are copied … so that later caller mutations cannot affect the asynchronous request," which is slightly overreaching. In practice this is unlikely to matter since metadata condition values are typically primitives/strings, but consider either softening the Javadoc claim (e.g. "top-level copies") or noting the limitation.

[AgentScopeJavaBot]

[praise] Good defensive-copying pattern. Copying datasetIds and metadataCondition eagerly (before Mono.fromCallable) ensures that mutations on the caller's collections after this method returns cannot affect the in-flight async request. Clean implementation.

[AgentScopeJavaBot]

[praise] Good test coverage: verifies two consecutive requests with different dynamic filters produce distinct request bodies, and that empty/null parameters correctly fall back to config values. This gives confidence that request isolation and config fallback both work as intended.

[AgentScopeJavaBot]

🤖 AI Review

This PR adds request-scoped datasetIds and metadataCondition overloads to RAGFlowKnowledge and RAGFlowClient, solving the problem of having to rebuild a knowledge instance to change retrieval filters between calls. The implementation is clean: old methods delegate to new ones, defensive copies are made before the deferred Mono.fromCallable execution, and fallback to config values works correctly. Test coverage is solid with both dynamic-filter and config-fallback scenarios. One subtle behavior change in how dataset_ids is serialized should be noted.

[AgentScopeJavaBot]

[minor] Subtle behavior change: the old code unconditionally put dataset_ids in the request body (even when config.getDatasetIds() was null or empty), while the new code omits the field entirely when the effective list is empty. This is actually more correct per the RAGFlow API contract ("Required unless document_ids is provided"), but the PR description states "keeps the existing APIs and configuration validation unchanged." Consider mentioning this in the PR description for reviewer awareness, and confirming that the RAGFlow server handles a missing dataset_ids field the same way it handled "dataset_ids": null.

[Ycymac]

Thanks for pointing this out. I rechecked the valid configuration path: datasetIds defaults to an empty list, and RAGFlowConfig requires either non-empty dataset IDs or document IDs, so the previous document-only payload was "dataset_ids": [], rather than null.

The updated client omits dataset_ids when the effective list is empty. This matches the documented retrieval contract, which allows document_ids to be provided instead. Current RAGFlow main checks if not req.get("dataset_ids"), so an omitted field and an empty list currently take the same rejection path before document_ids is processed; this wire-format normalization does not remove a previously working current-main path, but it does expose an upstream documentation/implementation mismatch.

I updated the PR description to disclose the serialization change and added testRetrieveWithDocumentOnlyConfigOmitsDatasetIds in fe4b3140. The focused knowledge test passes 16/16, and the full RAGFlow test selection passes 98/98.

[AgentScopeJavaBot]

[nit] The shallow copy via new HashMap<>(metadataCondition) protects against top-level mutations (put/remove), but nested mutable values inside the map are still shared between the caller and the deferred request. The Javadoc states "Parameter values are copied … so that later caller mutations cannot affect the asynchronous request," which is slightly overreaching. In practice this is unlikely to matter since metadata condition values are typically primitives/strings, but consider either softening the Javadoc claim (e.g. "top-level copies") or noting the limitation.

[Ycymac]

Good catch. new HashMap<>(metadataCondition) snapshots only the top-level map structure; nested mutable lists or maps remain shared, and RAGFlow metadata conditions can contain nested condition objects.

I addressed the overbroad wording in fe4b3140: the Javadoc and PR description now state that the dataset ID list and top-level metadata-condition map are copied, while nested mutable metadata values are not deep-copied. I kept a generic deep-copy mechanism out of this focused PR because Map<String, Object> can carry arbitrary JSON-serializable values; a true deep-snapshot guarantee would need an explicit implementation and regression contract.

The focused knowledge test passes 16/16, and the full RAGFlow test selection passes 98/98.

[AgentScopeJavaBot]

[praise] Good defensive-copying pattern. Copying datasetIds and metadataCondition eagerly (before Mono.fromCallable) ensures that mutations on the caller's collections after this method returns cannot affect the in-flight async request. Clean implementation.

[AgentScopeJavaBot]

[praise] Good test coverage: verifies two consecutive requests with different dynamic filters produce distinct request bodies, and that empty/null parameters correctly fall back to config values. This gives confidence that request isolation and config fallback both work as intended.