From 37f11d5b3daa3a88b37f795afad9cc338cf5c17b Mon Sep 17 00:00:00 2001 From: Jayme Klein Date: Fri, 17 Jul 2026 15:00:28 -0300 Subject: [PATCH] feat(indexing): replace the no-op re-index with a confirmed retry of failed ingestions The per-collection re-index button called a no-op task: BM25 is the auto-maintained chunks.text_tsv generated column, so there is nothing to rebuild. A coverage gap is an unfinished ingestion, so the useful action is re-ingesting the failed documents (which re-embeds them) behind a confirmation. The button now shows only when a collection has failed documents and, on confirm, runs the existing bulk retry scoped to that collection. Scoping needs an exact id, not the queue filter's name substring (which would sweep a sibling whose name merely contains the same text), so add JobListQuery.collection_id and a CollectionIdSpec, with a test that pins the overlapping-name case. Also invalidate the index overview after a retry, correct the page header, and remove the now-orphaned useIndexCollection hook and client method. --- api/models/document.py | 4 ++ api/services/job_filters.py | 13 ++++ docs/openapi.yaml | 16 +++++ tests/integration/test_jobs.py | 29 ++++++++ tests/unit/test_job_filters.py | 2 +- ui/src/api/client.ts | 5 -- ui/src/api/hooks.ts | 24 +++---- ui/src/api/types.ts | 2 + ui/src/pages/Indexing.tsx | 128 +++++++++++++++++++++------------ 9 files changed, 159 insertions(+), 64 deletions(-) diff --git a/api/models/document.py b/api/models/document.py index 7359c41..afacc65 100644 --- a/api/models/document.py +++ b/api/models/document.py @@ -76,5 +76,9 @@ class JobListQuery(BaseModel): filename: str | None = None # case-insensitive substring file_type: str | None = None collection: str | None = None # case-insensitive substring on the collection name + # Exact collection id. Distinct from ``collection`` on purpose: the name substring is the + # queue's human filter, but an action scoped to *one* collection (the indexing page's retry) + # must not sweep a sibling whose name merely contains the same text. + collection_id: str | None = None created_after: str | None = None created_before: str | None = None diff --git a/api/services/job_filters.py b/api/services/job_filters.py index a7d7b94..a378ad4 100644 --- a/api/services/job_filters.py +++ b/api/services/job_filters.py @@ -41,6 +41,18 @@ async def to_condition(self, db: AsyncSession) -> Any | None: return ilike_contains(col_t.c.name, self.value) if self.value else None +class CollectionIdSpec(FilterSpec): + """Exact collection id, straight off the job row (no join, no fuzziness). + + The sibling of :class:`CollectionSpec` for callers that must scope to exactly one + collection — a bulk retry driven from the indexing page, say. The name substring + cannot do that: "leis" also matches "leis-antigas". + """ + + async def to_condition(self, db: AsyncSession) -> Any | None: + return job_t.c.collection_id == self.value if self.value else None + + class CreatedAfterSpec(FilterSpec): async def to_condition(self, db: AsyncSession) -> Any | None: return job_t.c.created_at >= self.value if self.value else None @@ -59,6 +71,7 @@ def build_specs(query: JobListQuery) -> list[FilterSpec]: FilenameSpec(query.filename), FileTypeSpec(query.file_type), CollectionSpec(query.collection), + CollectionIdSpec(query.collection_id), CreatedAfterSpec(query.created_after), CreatedBeforeSpec(query.created_before), ] diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 1cc22a3..63f7767 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2084,6 +2084,14 @@ paths: - type: string - type: 'null' title: Collection + - name: collection_id + in: query + required: false + schema: + anyOf: + - type: string + - type: 'null' + title: Collection Id - name: created_after in: query required: false @@ -2243,6 +2251,14 @@ paths: - type: string - type: 'null' title: Collection + - name: collection_id + in: query + required: false + schema: + anyOf: + - type: string + - type: 'null' + title: Collection Id - name: created_after in: query required: false diff --git a/tests/integration/test_jobs.py b/tests/integration/test_jobs.py index 00bcb0a..9e4d796 100644 --- a/tests/integration/test_jobs.py +++ b/tests/integration/test_jobs.py @@ -145,6 +145,35 @@ async def test_retry_failed_respects_listing_filters(client): assert r.json() == {"retried": 1} +async def test_retry_failed_scopes_to_one_collection_by_id(client): + """collection_id must retry exactly one collection — the indexing page's per-row retry. + + Both collections here are named so the ``collection`` *substring* filter would match both + ("Leis" is a prefix of "Leis Antigas"); only the exact id keeps the sibling out of it. + """ + ws_id = (await client.post("/workspaces", json={"name": "WS"}, headers=AUTH)).json()["id"] + mk = lambda name: client.post( # noqa: E731 + f"/workspaces/{ws_id}/collections", json={"name": name}, headers=AUTH + ) + target = (await mk("Leis")).json()["id"] + sibling = (await mk("Leis Antigas")).json()["id"] + here = await _upload(client, ws_id, target, name="a.txt") + there = await _upload(client, ws_id, sibling, name="b.txt") + await _mark_failed(client, here["document_id"]) + await _mark_failed(client, there["document_id"]) + + r = await client.post(f"/ingestion/jobs/retry-failed?collection_id={target}", headers=AUTH) + assert r.status_code == 202 + assert r.json() == {"retried": 1} # the sibling's failure is untouched + + rows = (await client.get("/ingestion/jobs", headers=AUTH)).json()["items"] + assert sorted(j["status"] for j in rows if j["document_id"] == here["document_id"]) == [ + "failed", + "pending", + ] + assert [j["status"] for j in rows if j["document_id"] == there["document_id"]] == ["failed"] + + async def test_retry_failed_is_zero_when_nothing_failed(client): ws_id, col_id = await _setup(client) await _upload(client, ws_id, col_id) # pending, not failed diff --git a/tests/unit/test_job_filters.py b/tests/unit/test_job_filters.py index dd030b1..e809678 100644 --- a/tests/unit/test_job_filters.py +++ b/tests/unit/test_job_filters.py @@ -21,7 +21,7 @@ async def test_unset_filters_return_none_and_are_skipped() -> None: async def test_set_filters_return_a_condition() -> None: q = JobListQuery( - status="failed", filename="a", file_type=".pdf", collection="c", + status="failed", filename="a", file_type=".pdf", collection="c", collection_id="col_1", created_after="2024-01-01", created_before="2024-02-01", ) for spec in build_specs(q): diff --git a/ui/src/api/client.ts b/ui/src/api/client.ts index 80aad76..d3b1087 100644 --- a/ui/src/api/client.ts +++ b/ui/src/api/client.ts @@ -188,11 +188,6 @@ export const api = { `/workspaces/${enc(wsId)}/collections/${enc(colId)}/documents/${enc(docId)}/index`, { method: 'POST' }, ), - indexCollection: (wsId: string, colId: string) => - request( - `/workspaces/${enc(wsId)}/collections/${enc(colId)}/index`, - { method: 'POST' }, - ), // ── API keys ────────────────────────────────────────────────────────────── listApiKeys: (wsId: string, colId: string) => diff --git a/ui/src/api/hooks.ts b/ui/src/api/hooks.ts index c0cfdb5..75e7518 100644 --- a/ui/src/api/hooks.ts +++ b/ui/src/api/hooks.ts @@ -406,15 +406,21 @@ export function useReprocessDocument(wsId?: string, colId?: string) { } /** - * Bulk "retry all errors" for the ingestion queue: re-enqueue every currently-failed document - * matching the given listing filters. Refreshes the queue (`qk.jobs` prefix → list + stats) so - * the fresh pending rows surface. + * Bulk "retry all errors": re-enqueue every currently-failed document matching the given listing + * filters — the queue's page-wide retry, and (via `collection_id`) the indexing page's per-row one. + * Refreshes the queue (`qk.jobs` prefix → list + stats) so the fresh pending rows surface, and the + * index overview, which counts those documents as unindexed until their chunks land. */ export function useRetryFailedJobs() { const queryClient = useQueryClient() return useMutation({ mutationFn: (query: JobQuery = {}) => api.retryFailedJobs(query), - onSuccess: () => queryClient.invalidateQueries({ queryKey: qk.jobs }), + onSuccess: async () => { + await Promise.all([ + queryClient.invalidateQueries({ queryKey: qk.jobs }), + queryClient.invalidateQueries({ queryKey: qk.indexStatus }), + ]) + }, }) } @@ -460,16 +466,6 @@ export function useIndexDocument(wsId: string, colId: string) { }) } -/** Enqueue a BM25 (re)index of an entire collection. */ -export function useIndexCollection() { - const invalidate = useInvalidateIndex() - return useMutation({ - mutationFn: ({ wsId, colId }: { wsId: string; colId: string }) => - api.indexCollection(wsId, colId), - onSuccess: invalidate, - }) -} - // ── Tags ────────────────────────────────────────────────────────────────── export function useTags(wsId: string) { diff --git a/ui/src/api/types.ts b/ui/src/api/types.ts index fda579c..bb5de84 100644 --- a/ui/src/api/types.ts +++ b/ui/src/api/types.ts @@ -275,6 +275,8 @@ export interface JobQuery { filename?: string file_type?: string collection?: string + /** Exact collection id — scopes an action to one collection, unlike the `collection` substring. */ + collection_id?: string created_after?: string created_before?: string } diff --git a/ui/src/pages/Indexing.tsx b/ui/src/pages/Indexing.tsx index a29bb75..2ab8d5d 100644 --- a/ui/src/pages/Indexing.tsx +++ b/ui/src/pages/Indexing.tsx @@ -1,9 +1,18 @@ +import { useState } from 'react' import { AlertTriangle, DatabaseZap, RefreshCw } from 'lucide-react' -import { useIndexCollection, useIndexStatus } from '../api/hooks' +import { useIndexStatus, useRetryFailedJobs } from '../api/hooks' import type { CollectionIndexStatus } from '../api/types' -import { Button, Card, EmptyState, QueryError, Skeleton, useToast } from '../components/ui' +import { + Button, + Card, + ConfirmDialog, + EmptyState, + QueryError, + Skeleton, + useToast, +} from '../components/ui' -/** BM25 index control: coverage per workspace → collection, with re-index triggers. */ +/** Keyword-index coverage per workspace → collection, with a retry for failed ingestions. */ export default function Indexing() { const { data, isLoading, isError, error, refetch } = useIndexStatus() @@ -50,7 +59,7 @@ export default function Indexing() { {ws.collections.map((col) => ( - + ))} @@ -66,64 +75,95 @@ function Header() {

Indexing

- BM25 keyword-index coverage per collection. Re-index to rebuild from the vector - store (no re-embedding). + Keyword-index coverage per collection. A document is searchable once its chunks are + stored — the keyword index is maintained automatically, so there is nothing to rebuild. + Anything short of full coverage is an ingestion that has not finished; retry the failed + ones here.

) } -/** One collection: coverage bar, counts, and a re-index trigger. */ -function CollectionRow({ wsId, col }: { wsId: string; col: CollectionIndexStatus }) { +/** + * One collection: coverage bar, counts, and — when something actually failed — a confirmed retry. + * + * There is deliberately no "re-index" action: the keyword index is a generated column that + * Postgres maintains on write, so a rebuild is a no-op (see api/services/indexing.py). A gap in + * coverage is always an unfinished ingestion, so the only useful action is re-ingesting the failed + * documents — which re-embeds them, hence the confirmation. In-flight ones are left to finish. + */ +function CollectionRow({ col }: { col: CollectionIndexStatus }) { const toast = useToast() - const indexMut = useIndexCollection() + const retryMut = useRetryFailedJobs() + const [confirming, setConfirming] = useState(false) const pct = col.total > 0 ? Math.round((col.indexed / col.total) * 100) : 100 const fullyIndexed = col.unindexed === 0 + const failedDocs = `${col.failed} failed document${col.failed === 1 ? '' : 's'}` - const reindex = () => - indexMut.mutate( - { wsId, colId: col.collection_id }, + const retryFailed = () => + retryMut.mutate( + { collection_id: col.collection_id }, { - onSuccess: () => toast.success('Re-index started.'), + onSuccess: ({ retried }) => { + toast.success(`Re-queued ${retried} document${retried === 1 ? '' : 's'}.`) + setConfirming(false) + }, + // Leave the dialog open on failure so it can be retried from there. onError: (e) => toast.error(e.message), }, ) return ( -
-
-
-

{col.collection_name}

- {!fullyIndexed && ( - - - {col.unindexed} unindexed + <> +
+
+
+

{col.collection_name}

+ {!fullyIndexed && ( + + + {col.unindexed} unindexed + + )} +
+
+
+
+
+ + {col.indexed}/{col.total} indexed + {col.pending > 0 && ` · ${col.pending} ingesting`} + {col.failed > 0 && ` · ${col.failed} failed`} - )} -
-
-
-
- - {col.indexed}/{col.total} indexed - {col.pending > 0 && ` · ${col.pending} ingesting`} - {col.failed > 0 && ` · ${col.failed} failed`} -
+ {/* Only offer the retry when there is something stuck to retry: a document still + ingesting will finish on its own, and a fully-indexed collection has nothing to do. */} + {col.failed > 0 && ( + + )}
- -
+ + setConfirming(false)} + /> + ) }