Skip to content

Commit b721615

Browse files
edwinjosechittilappillyautofix-ci[bot]phact
authored andcommitted
fix: cherrypick v1 updates from main to fix mcp and sdk (#1940)
* fix: Add filter_id support and v1 filter resolution (#1666) * Add filter_id support and v1 filter resolution Add server-side resolution for filter_id and extend document deletion to accept a filter_id. Introduces api.v1._filter_resolution to normalize a filter_id into concrete filters/limits/score_thresholds and to strip wildcards. v1 endpoints (chat, search, documents) now accept filter_id and merge resolved values with inline overrides; documents DELETE can delete all filenames referenced by a filter_id (with wildcard/empty data_sources rejected to avoid mass deletion). SDK updates (Python and TypeScript) allow DocumentsClient.delete to accept either filename or filter_id (mutually exclusive), include additional response fields (filenames, filter_id, per_file), and preserve idempotent semantics for filename deletes while surfacing filter-not-found errors. Tests updated/added to cover filter_id behavior, inline overrides, streaming, and validation. Additional minor logging/validation and type updates included. * style: ruff autofix (auto) * Clarify /v1/documents DELETE description Expand the DELETE /v1/documents component description to indicate it can delete single or multiple documents and requires exactly one of `filename` or `filter_id`. Also notes that wildcards are rejected for safety, improving API documentation and removing ambiguity about deletion semantics. * style: ruff autofix (auto) * fix lint * style: ruff autofix (auto) * Set auth context for API-key search requests Ensure API-key authenticated searches set the auth context and pass the user's JWT to the search service so downstream tools can resolve the user. Adds import for set_auth_context, calls it when handling API-key requests (mirroring v1 chat), and passes user.jwt_token to search_service.search instead of None. Also cleans up an unused typing import. * Exclude ingest route; expose /v1 GETs as tools Clarify search component docs to document `filter_id` and inline `filters` (inline filters override per-field). Document that /v1/documents/ingest is intentionally not customized/exposed via MCP because multipart/form-data uploads are not supported by FastMCP's from_fastapi conversion; clients should use the HTTP API or SDK to ingest. Add a RouteMap to explicitly exclude POST /v1/documents/ingest and consolidate route maps to expose all /v1/ routes (including GET) as MCP tools, with a note explaining that GETs are exposed as tools to better support LLM agent workflows. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> (cherry picked from commit eda6aaf) * fix: more filter_id issues (#1730) * missed filter_id fixes * unit test * clarify filter comments * Use bracket notation for document body keys (#1734) Change assignments to the request body to use bracket notation (body['filename'], body['filter_id']) instead of dot notation. This ensures the string keys are set explicitly on the Record<string, string> and avoids relying on declared property names. * fix: Update integration.test.ts (#1737) --------- Co-authored-by: Edwin Jose <edwin.jose@datastax.com> (cherry picked from commit db9ae7f) * Update documents.ts * test: raise TS SDK filter-scope test timeouts and parallelize ingests The "filterId in chat actually scopes retrieval to data_sources" integration test was timing out at 60s and failing CI on every run. The chat itself works (backend logs show the response generated in ~7.6s); the 60s budget was consumed by two sequential Docling ingests (~25s each in CI) plus the filter create before the chat even started, so the test crossed the deadline. Bump the three ingest-heavy filter-scope tests (chat, search, delete-by-filter) to a 180s per-test timeout and ingest the two docs concurrently via Promise.all to shorten the critical path. Raise the global vitest testTimeout 60s -> 120s so lighter tests have headroom too. The Python equivalent already passes because pytest imposes no per-test cap; no backend/SDK behavior changes. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Sebastián Estévez <estevezsebastian@gmail.com>
1 parent bba1269 commit b721615

14 files changed

Lines changed: 800 additions & 113 deletions

File tree

sdks/python/openrag_sdk/documents.py

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,27 +125,46 @@ async def wait_for_task(
125125
await asyncio.sleep(poll_interval)
126126
elapsed += poll_interval
127127

128-
raise TimeoutError(f"Ingestion task {task_id} did not complete within {timeout}s")
128+
raise TimeoutError(
129+
f"Ingestion task {task_id} did not complete within {timeout}s"
130+
)
129131

130-
async def delete(self, filename: str) -> DeleteDocumentResponse:
132+
async def delete(
133+
self,
134+
filename: str | None = None,
135+
*,
136+
filter_id: str | None = None,
137+
) -> DeleteDocumentResponse:
131138
"""
132-
Delete a document from the knowledge base.
139+
Delete document(s) from the knowledge base.
133140
134-
Args:
135-
filename: Name of the file to delete.
141+
Provide exactly one of:
142+
filename: delete all chunks for that filename.
143+
filter_id: delete chunks for each filename in the filter's data_sources.
136144
137145
Returns:
138146
DeleteDocumentResponse with deleted chunk count.
139147
"""
148+
if bool(filename) == bool(filter_id):
149+
raise ValueError("Provide exactly one of `filename` or `filter_id`")
150+
151+
body: dict[str, str] = {}
152+
if filename is not None:
153+
body["filename"] = filename
154+
if filter_id is not None:
155+
body["filter_id"] = filter_id
156+
140157
try:
141158
response = await self._client._request(
142159
"DELETE",
143160
"/api/v1/documents",
144-
json={"filename": filename},
161+
json=body,
145162
)
146163
except NotFoundError as e:
147-
# Keep delete idempotent for SDK callers: a missing document is not an exception.
148-
if getattr(e, "status_code", None) == 404:
164+
# Keep delete idempotent for SDK callers: a missing document is not
165+
# an exception.
166+
# Filter-not-found 404s still raise because the filter_id is caller input.
167+
if filename is not None and getattr(e, "status_code", None) == 404:
149168
return DeleteDocumentResponse(
150169
success=False,
151170
deleted_chunks=0,

sdks/python/openrag_sdk/models.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
"""OpenRAG SDK data models."""
22

3-
from datetime import datetime
4-
from typing import Any, Literal
3+
from typing import Literal
54

65
from pydantic import BaseModel, Field
76

@@ -98,6 +97,10 @@ class DeleteDocumentResponse(BaseModel):
9897
filename: str | None = None
9998
message: str | None = None
10099
error: str | None = None
100+
# Populated when deleting by filter_id — one entry per resolved data_source.
101+
filenames: list[str] | None = None
102+
filter_id: str | None = None
103+
per_file: list[dict] | None = None
101104

102105

103106
# Chat history models

sdks/typescript/src/documents.ts

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import type { OpenRAGClient } from "./client";
66
import type {
7+
DeleteDocumentOptions,
78
DeleteDocumentResponse,
89
IngestResponse,
910
IngestTaskStatus,
@@ -144,33 +145,57 @@ export class DocumentsClient {
144145
}
145146

146147
/**
147-
* Delete a document from the knowledge base.
148+
* Delete document(s) from the knowledge base.
149+
*
150+
* Provide exactly one of:
151+
* - filename: a single filename, or
152+
* - { filename } / { filterId }: an options object.
148153
*
149-
* @param filename - Name of the file to delete.
150154
* @returns DeleteDocumentResponse with deleted chunk count.
151155
*/
152-
async delete(filename: string): Promise<DeleteDocumentResponse> {
156+
async delete(
157+
arg: string | DeleteDocumentOptions
158+
): Promise<DeleteDocumentResponse> {
159+
const opts: DeleteDocumentOptions =
160+
typeof arg === "string" ? { filename: arg } : arg;
161+
162+
if (!opts.filename === !opts.filterId) {
163+
throw new Error(
164+
"Provide exactly one of `filename` or `filterId`"
165+
);
166+
}
167+
168+
const body: Record<string, string> = {};
169+
if (opts.filename) body["filename"] = opts.filename;
170+
if (opts.filterId) body["filter_id"] = opts.filterId;
171+
153172
try {
154173
const response = await this.client._request("DELETE", "/api/v1/documents", {
155-
body: JSON.stringify({ filename }),
174+
body: JSON.stringify(body),
156175
});
157176

158177
const data = await response.json();
159178
return {
160179
success: data.success ?? false,
161180
deleted_chunks: data.deleted_chunks ?? 0,
162-
filename: data.filename ?? filename,
181+
filename: data.filename ?? opts.filename ?? null,
163182
message: data.message ?? null,
164183
error: data.error ?? null,
184+
filenames: data.filenames ?? null,
185+
filter_id: data.filter_id ?? null,
186+
per_file: data.per_file ?? null,
165187
};
166188
} catch (error) {
167-
// Delete is idempotent: if no chunks match, backend may return 404.
168-
// Surface this as a non-throwing "nothing deleted" response.
169-
if ((error as NotFoundError)?.statusCode === 404) {
189+
// Filename delete stays idempotent (404 -> success:false). Filter-id 404s
190+
// are caller errors (bad filter id) and should propagate.
191+
if (
192+
opts.filename &&
193+
(error as NotFoundError)?.statusCode === 404
194+
) {
170195
return {
171196
success: false,
172197
deleted_chunks: 0,
173-
filename,
198+
filename: opts.filename,
174199
message: null,
175200
error: (error as Error)?.message ?? "Resource not found",
176201
};

sdks/typescript/src/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,15 @@ export interface DeleteDocumentResponse {
7777
filename?: string | null;
7878
message?: string | null;
7979
error?: string | null;
80+
// Populated when deleting by filter_id — one entry per resolved data_source.
81+
filenames?: string[] | null;
82+
filter_id?: string | null;
83+
per_file?: Array<Record<string, unknown>> | null;
84+
}
85+
86+
export interface DeleteDocumentOptions {
87+
filename?: string;
88+
filterId?: string;
8089
}
8190

8291
// Chat history types

sdks/typescript/tests/integration.test.ts

Lines changed: 119 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -190,55 +190,153 @@ describe.skipIf(SKIP_TESTS)("OpenRAG TypeScript SDK Integration", () => {
190190
expect(filter).toBeNull();
191191
});
192192

193-
it("should use filterId in chat", async () => {
194-
// Create a filter first
193+
it("filterId in chat actually scopes retrieval to data_sources", async () => {
194+
// Ingest two distinguishable docs.
195+
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sdk-filter-"));
196+
const alphaName = `alpha_${Date.now()}.md`;
197+
const betaName = `beta_${Date.now()}.md`;
198+
const alphaPath = path.join(tmpDir, alphaName);
199+
const betaPath = path.join(tmpDir, betaName);
200+
fs.writeFileSync(alphaPath, "# Alpha\n\nPurple elephants live here.\n");
201+
fs.writeFileSync(betaPath, "# Beta\n\nYellow tigers live here.\n");
202+
await Promise.all([
203+
client.documents.ingest({ filePath: alphaPath }),
204+
client.documents.ingest({ filePath: betaPath }),
205+
]);
206+
195207
const createResult = await client.knowledgeFilters.create({
196-
name: "Chat Test Filter",
197-
description: "Filter for testing chat with filterId",
208+
name: `TS chat filter scope ${Date.now()}`,
209+
description: "Filter scoped to alpha only",
198210
queryData: {
199-
query: "test",
200-
limit: 5,
211+
query: "",
212+
filters: {
213+
data_sources: [alphaName],
214+
document_types: ["*"],
215+
owners: ["*"],
216+
connector_types: ["*"],
217+
},
218+
limit: 10,
219+
scoreThreshold: 0,
201220
},
202221
});
203222
expect(createResult.success).toBe(true);
204223
const filterId = createResult.id!;
205224

206225
try {
207-
// Use filter in chat
208226
const response = await client.chat.create({
209-
message: "Hello with filter",
227+
message: "What animals appear in these documents?",
210228
filterId,
211229
});
230+
expect(response.sources).toBeDefined();
231+
const names = (response.sources ?? []).map((s) => s.filename);
232+
// Beta must NOT leak through the filter.
233+
expect(names).not.toContain(betaName);
234+
} finally {
235+
await client.knowledgeFilters.delete(filterId);
236+
await client.documents.delete(alphaName);
237+
await client.documents.delete(betaName);
238+
}
239+
}, 180_000);
240+
241+
it("filterId in search actually scopes results to data_sources", async () => {
242+
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sdk-filter-"));
243+
const alphaName = `alpha_${Date.now()}.md`;
244+
const betaName = `beta_${Date.now()}.md`;
245+
const alphaPath = path.join(tmpDir, alphaName);
246+
const betaPath = path.join(tmpDir, betaName);
247+
fs.writeFileSync(alphaPath, "# Alpha\n\nPurple elephants live here.\n");
248+
fs.writeFileSync(betaPath, "# Beta\n\nYellow tigers live here.\n");
249+
await Promise.all([
250+
client.documents.ingest({ filePath: alphaPath }),
251+
client.documents.ingest({ filePath: betaPath }),
252+
]);
212253

213-
expect(response.response).toBeDefined();
254+
const createResult = await client.knowledgeFilters.create({
255+
name: `TS search filter scope ${Date.now()}`,
256+
description: "Filter scoped to alpha only",
257+
queryData: {
258+
query: "",
259+
filters: {
260+
data_sources: [alphaName],
261+
document_types: ["*"],
262+
owners: ["*"],
263+
connector_types: ["*"],
264+
},
265+
limit: 10,
266+
scoreThreshold: 0,
267+
},
268+
});
269+
expect(createResult.success).toBe(true);
270+
const filterId = createResult.id!;
271+
272+
try {
273+
const results = await client.search.query("animals", { filterId });
274+
for (const r of results.results) {
275+
expect(r.filename).not.toBe(betaName);
276+
}
214277
} finally {
215-
// Cleanup
216278
await client.knowledgeFilters.delete(filterId);
279+
await client.documents.delete(alphaName);
280+
await client.documents.delete(betaName);
217281
}
218-
});
282+
}, 180_000);
283+
284+
it("documents.delete(filterId) only removes filenames in the filter", async () => {
285+
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sdk-filter-"));
286+
const alphaName = `alpha_${Date.now()}.md`;
287+
const betaName = `beta_${Date.now()}.md`;
288+
const alphaPath = path.join(tmpDir, alphaName);
289+
const betaPath = path.join(tmpDir, betaName);
290+
fs.writeFileSync(alphaPath, "# Alpha\n\nPurple elephants.\n");
291+
fs.writeFileSync(betaPath, "# Beta\n\nYellow tigers.\n");
292+
await Promise.all([
293+
client.documents.ingest({ filePath: alphaPath }),
294+
client.documents.ingest({ filePath: betaPath }),
295+
]);
219296

220-
it("should use filterId in search", async () => {
221-
// Create a filter first
222297
const createResult = await client.knowledgeFilters.create({
223-
name: "Search Test Filter",
224-
description: "Filter for testing search with filterId",
298+
name: `TS delete-by-filter ${Date.now()}`,
299+
description: "Filter scoped to alpha only",
225300
queryData: {
226-
query: "test",
227-
limit: 5,
301+
query: "",
302+
filters: {
303+
data_sources: [alphaName],
304+
document_types: ["*"],
305+
owners: ["*"],
306+
connector_types: ["*"],
307+
},
308+
limit: 10,
309+
scoreThreshold: 0,
228310
},
229311
});
230312
expect(createResult.success).toBe(true);
231313
const filterId = createResult.id!;
232314

233315
try {
234-
// Use filter in search
235-
const results = await client.search.query("test query", { filterId });
316+
const result = await client.documents.delete({ filterId });
317+
expect(result.success).toBe(true);
318+
expect(result.filenames).toContain(alphaName);
319+
expect(result.filenames ?? []).not.toContain(betaName);
236320

237-
expect(results.results).toBeDefined();
321+
// Beta still searchable
322+
const remaining = await client.search.query("tigers");
323+
const remainingNames = remaining.results.map((r) => r.filename);
324+
expect(remainingNames).toContain(betaName);
238325
} finally {
239-
// Cleanup
240326
await client.knowledgeFilters.delete(filterId);
327+
await client.documents.delete(alphaName);
328+
await client.documents.delete(betaName);
241329
}
330+
}, 180_000);
331+
332+
it("documents.delete rejects both filename and filterId together", async () => {
333+
await expect(
334+
client.documents.delete({ filename: "x.pdf", filterId: "y" })
335+
).rejects.toThrow();
336+
});
337+
338+
it("documents.delete rejects when neither arg is set", async () => {
339+
await expect(client.documents.delete({})).rejects.toThrow();
242340
});
243341
});
244342

sdks/typescript/vitest.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { defineConfig } from "vitest/config";
33
export default defineConfig({
44
test: {
55
include: ["tests/**/*.test.ts"],
6-
testTimeout: 60000, // 60 second timeout for integration tests
6+
testTimeout: 120000, // 120 second timeout for integration tests
77
hookTimeout: 60000,
88
},
99
});

0 commit comments

Comments
 (0)