Skip to content

feat(ragflow): support request-scoped retrieval filters #2633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ycymac wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat(ragflow): support request-scoped retrieval filters #2633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7959d6d
feat(ragflow): support request-scoped filters
Ycymac Aug 9, 2026
8c51af6
Merge branch 'main' into codex/feat-ragflow-dynamic-filters-686
Ycymac Aug 10, 2026
1786e07
Merge branch 'main' into codex/feat-ragflow-dynamic-filters-686
Ycymac Aug 11, 2026
643f39b
Merge branch 'main' into codex/feat-ragflow-dynamic-filters-686
Ycymac Aug 11, 2026
df0ced4
Merge branch 'main' into codex/feat-ragflow-dynamic-filters-686
Ycymac Aug 17, 2026
fe4b314
docs(ragflow): clarify request filter semantics
Ycymac Aug 18, 2026
5f62215
Merge branch 'main' into codex/feat-ragflow-dynamic-filters-686
Ycymac Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 53 additions & 7 deletions ...s-rag-ragflow/src/main/java/io/agentscope/core/rag/integration/ragflow/RAGFlowClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
import io.agentscope.core.rag.integration.ragflow.model.RAGFlowResponse;
import io.agentscope.core.util.JsonUtils;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import okhttp3.Interceptor;
Expand Down Expand Up @@ -102,6 +104,43 @@ public Mono<RAGFlowResponse> retrieve(
Double similarityThreshold,
Map<String, Object> metadataCondition) {

return retrieve(question, topK, similarityThreshold, null, metadataCondition);
}

/**
* Retrieve documents with request-specific dataset IDs and metadata conditions.
*
* <p>Non-empty request parameters take precedence over values from {@link RAGFlowConfig}.
* When a request parameter is {@code null} or empty, the corresponding config value is used.
* The dataset ID list and top-level metadata condition map are copied when this method is
* called. Later changes to the list or top-level map entries therefore cannot affect the
* asynchronous request, but nested mutable metadata values are not deep-copied.
*
* @param question the query text (required)
* @param topK the number of documents to retrieve (optional, defaults to config value)
* @param similarityThreshold the minimum similarity threshold (optional, defaults to config
* value)
* @param datasetIds dataset IDs for this request (optional, defaults to config value)
* @param metadataCondition metadata filtering conditions for this request (optional, defaults
* to config value)
* @return a Mono emitting the retrieval response
*/
public Mono<RAGFlowResponse> retrieve(
String question,
Integer topK,
Double similarityThreshold,
List<String> datasetIds,
Map<String, Object> metadataCondition) {

List<String> effectiveDatasetIds =
datasetIds != null && !datasetIds.isEmpty()

@AgentScopeJavaBot AgentScopeJavaBot Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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 AgentScopeJavaBot Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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.

? new ArrayList<>(datasetIds)
: copyList(config.getDatasetIds());
Map<String, Object> effectiveMetadataCondition =
metadataCondition != null && !metadataCondition.isEmpty()
? new HashMap<>(metadataCondition)
: copyMap(config.getMetadataCondition());

@AgentScopeJavaBot AgentScopeJavaBot Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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 AgentScopeJavaBot Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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 Ycymac Aug 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.


return Mono.fromCallable(
() -> {
if (question == null || question.trim().isEmpty()) {
Expand All @@ -113,8 +152,10 @@ public Mono<RAGFlowResponse> retrieve(
// Required: question text
requestBody.put("question", question);

// Required: dataset_ids (array)
requestBody.put("dataset_ids", config.getDatasetIds());
// Required unless document_ids is provided: dataset_ids (array)
if (!effectiveDatasetIds.isEmpty()) {

@AgentScopeJavaBot AgentScopeJavaBot Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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 AgentScopeJavaBot Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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 Ycymac Aug 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

requestBody.put("dataset_ids", effectiveDatasetIds);
}

// Optional: document_ids (filter to specific documents)
if (config.getDocumentIds() != null && !config.getDocumentIds().isEmpty()) {
Expand Down Expand Up @@ -185,11 +226,8 @@ public Mono<RAGFlowResponse> retrieve(
}

// Optional: metadata_condition for filtering
if (metadataCondition != null && !metadataCondition.isEmpty()) {
requestBody.put("metadata_condition", metadataCondition);
} else if (config.getMetadataCondition() != null
&& !config.getMetadataCondition().isEmpty()) {
requestBody.put("metadata_condition", config.getMetadataCondition());
if (!effectiveMetadataCondition.isEmpty()) {
requestBody.put("metadata_condition", effectiveMetadataCondition);
}

String jsonBody = JsonUtils.getJsonCodec().toJson(requestBody);
Expand Down Expand Up @@ -262,6 +300,14 @@ public Mono<RAGFlowResponse> retrieve(
});
}

private static <T> List<T> copyList(List<T> values) {
return values == null ? new ArrayList<>() : new ArrayList<>(values);
}

private static <K, V> Map<K, V> copyMap(Map<K, V> values) {
return values == null ? new HashMap<>() : new HashMap<>(values);
}

private void handleErrorResponse(int statusCode, String responseBody) {
logger.error("RAGFlow API error: status={}, body={}", statusCode, responseBody);

Expand Down
28 changes: 26 additions & 2 deletions ...ag-ragflow/src/main/java/io/agentscope/core/rag/integration/ragflow/RAGFlowKnowledge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.agentscope.core.rag.model.RetrieveConfig;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import reactor.core.publisher.Mono;
Expand Down Expand Up @@ -123,6 +124,30 @@ public static Builder builder() {
*/
@Override
public Mono<List<Document>> retrieve(String query, RetrieveConfig config) {
return retrieve(query, config, null, null);
}

/**
* Retrieve documents with request-specific RAGFlow filters.
*
* <p>Non-empty {@code datasetIds} and {@code metadataCondition} values override the defaults in
* {@link RAGFlowConfig} for this request only. If either parameter is {@code null} or empty, the
* corresponding config value is used. This allows one {@code RAGFlowKnowledge} instance to
* serve requests with different filters without mutating shared configuration or rebuilding an
* agent.
*
* @param query the query text (required)
* @param config the retrieval configuration (limit, score threshold)
* @param datasetIds dataset IDs for this request, or {@code null} to use the configured IDs
* @param metadataCondition metadata filtering conditions for this request, or {@code null} to
* use the configured condition
* @return a Mono emitting the list of retrieved documents, sorted by relevance
*/
public Mono<List<Document>> retrieve(
String query,
RetrieveConfig config,
List<String> datasetIds,
Map<String, Object> metadataCondition) {
if (query == null || query.trim().isEmpty()) {
logger.warn("Empty query provided, returning empty result");
return Mono.just(new ArrayList<>());
Expand All @@ -134,8 +159,7 @@ public Mono<List<Document>> retrieve(String query, RetrieveConfig config) {
Integer topK = config != null ? config.getLimit() : null;
Double similarityThreshold = config != null ? config.getScoreThreshold() : null;

// Call RAGFlow API (metadata condition from config)
return client.retrieve(query, topK, similarityThreshold, null)
return client.retrieve(query, topK, similarityThreshold, datasetIds, metadataCondition)
.map(
response -> {
if (response.getData() == null
Expand Down
77 changes: 77 additions & 0 deletions ...agflow/src/test/java/io/agentscope/core/rag/integration/ragflow/RAGFlowKnowledgeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,21 @@
package io.agentscope.core.rag.integration.ragflow;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import com.fasterxml.jackson.core.type.TypeReference;
import io.agentscope.core.rag.model.Document;
import io.agentscope.core.rag.model.RetrieveConfig;
import io.agentscope.core.util.JsonUtils;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -239,6 +244,78 @@ void testRetrieveWithNullChunksResponse() throws Exception {
assertTrue(documents.isEmpty());
}

@Test
void testRetrieveWithDynamicFiltersAndConfigFallback() throws Exception {
mockWebServer.enqueue(createSuccessResponse());
mockWebServer.enqueue(createSuccessResponse());
mockWebServer.enqueue(createSuccessResponse());

@AgentScopeJavaBot AgentScopeJavaBot Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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 AgentScopeJavaBot Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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.


RAGFlowConfig config =
RAGFlowConfig.builder()
.apiKey("test-api-key")
.baseUrl(mockWebServer.url("").toString().replaceAll("/$", ""))
.addDatasetId("default-dataset")
.metadataCondition(Map.of("source", "default"))
.maxRetries(0)
.build();
RAGFlowKnowledge knowledge = RAGFlowKnowledge.builder().config(config).build();
RetrieveConfig retrieveConfig =
RetrieveConfig.builder().limit(10).scoreThreshold(0.5).build();

knowledge
.retrieve(
"first query",
retrieveConfig,
List.of("dataset-a"),
Map.of("source", "first"))
.block();
knowledge
.retrieve(
"second query",
retrieveConfig,
List.of("dataset-b"),
Map.of("source", "second"))
.block();
knowledge.retrieve("fallback query", retrieveConfig, List.of(), Map.of()).block();

Map<String, Object> firstBody = readRequestBody(mockWebServer.takeRequest());
Map<String, Object> secondBody = readRequestBody(mockWebServer.takeRequest());
Map<String, Object> fallbackBody = readRequestBody(mockWebServer.takeRequest());

assertEquals(List.of("dataset-a"), firstBody.get("dataset_ids"));
assertEquals(Map.of("source", "first"), firstBody.get("metadata_condition"));
assertEquals(List.of("dataset-b"), secondBody.get("dataset_ids"));
assertEquals(Map.of("source", "second"), secondBody.get("metadata_condition"));
assertEquals(List.of("default-dataset"), fallbackBody.get("dataset_ids"));
assertEquals(Map.of("source", "default"), fallbackBody.get("metadata_condition"));
}

@Test
void testRetrieveWithDocumentOnlyConfigOmitsDatasetIds() throws Exception {
mockWebServer.enqueue(createSuccessResponse());

RAGFlowConfig config =
RAGFlowConfig.builder()
.apiKey("test-api-key")
.baseUrl(mockWebServer.url("").toString().replaceAll("/$", ""))
.addDocumentId("document-only")
.maxRetries(0)
.build();
RAGFlowKnowledge knowledge = RAGFlowKnowledge.builder().config(config).build();

knowledge.retrieve("document query", null).block();

Map<String, Object> requestBody = readRequestBody(mockWebServer.takeRequest());
assertFalse(requestBody.containsKey("dataset_ids"));
assertEquals(List.of("document-only"), requestBody.get("document_ids"));
}

private Map<String, Object> readRequestBody(RecordedRequest request) {
return JsonUtils.getJsonCodec()
.fromJson(
request.getBody().readUtf8(), new TypeReference<Map<String, Object>>() {});
}

// === AddDocuments Tests ===

@Test
Expand Down
Loading