From a5032143139346b9216aa9fd047b1d84d5fd8e4a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 14 Sep 2026 18:54:10 +0000 Subject: [PATCH 1/3] docs: draft document-centric list and history RFC Record maintainer ownership rules from #232/#231 so a later API change does not revive namespace or job-listing shortcuts. Co-authored-by: Frankie-Xu --- docs/design/document-centric-list-history.md | 137 +++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 docs/design/document-centric-list-history.md diff --git a/docs/design/document-centric-list-history.md b/docs/design/document-centric-list-history.md new file mode 100644 index 000000000..71f1ba92f --- /dev/null +++ b/docs/design/document-centric-list-history.md @@ -0,0 +1,137 @@ +# Document-centric list and history + +**Status:** Draft — RFC only, no implementation in this change +**Related PRs:** [#232](https://github.com/Ontos-AI/knowhere/pull/232) (@gdccyuen), [#231](https://github.com/Ontos-AI/knowhere/pull/231) (@SusannaShu) +**Maintainer review:** [suguanYang on #232](https://github.com/Ontos-AI/knowhere/pull/232#issuecomment-5265960465), [suguanYang on #231](https://github.com/Ontos-AI/knowhere/pull/231#issuecomment-5265855328) +**Observed against:** `main` @ `9489fa2c` (2026-09-14) + +This is a unifying API-ownership draft. It does not replace or rewrite +#232 / #231, and it does not include a code patch. Those PRs stay with +their authors. + +## Why this is an RFC + +Both PRs were blocked as **public API / resource-ownership** questions, not +as local bugs: + +- #232 `GET /documents/namespaces` does not list a Knowhere resource. + Namespace is a client-owned isolation label with no identity, create + operation, metadata, authz, or lifecycle. +- #231 `GET /jobs?namespace=` and job deletion treat a processing attempt as + a file registry. A job is the operational ledger and should remain after + the document is archived. + +Those are domain-contract changes. They should not land as drive-by +route additions. + +## Resource ownership (from maintainer, restated) + +| Concept | Owns | Does not own | +| --- | --- | --- | +| Namespace | Client (tenant / folder / workspace id) | Knowhere registry, empty folders, archived-only labels | +| Document | Persisted corpus: list, get, archive, retrieval scope | Processing attempts | +| Job | One parse attempt: status, billing, errors, provenance | Tenant file listing, delete-from-folder | + +## Existing surface (do not duplicate) + +On current `main`: + +- `GET /v1/documents?namespace=` already lists the credential's documents + in that namespace (`apps/api/app/api/v1/routes/documents.py`). +- `GET /v1/documents/{document_id}` and chunk listing already exist. +- Removal from the active corpus is `POST /v1/documents/{document_id}/archive` + (plus a legacy `:archive` alias). There is no `DELETE /documents/{id}`. +- `GET /v1/jobs` lists processing attempts with status/type/time filters. + It has **no** `namespace` query. +- `GET /v1/jobs/{job_id}` inspects one attempt. + +The keyboard-backend / on-prem folder workflow that #231 and #232 were +solving is therefore **mostly already on the document resource**: + +1. List a tenant folder: `GET /documents?namespace={label}` +2. Remove a file from that folder: `POST /documents/{document_id}/archive` +3. Poll a known upload: `GET /jobs/{job_id}` (client keeps the id while + pending) + +## Remaining gaps (only these need new API) + +### 1. Processing history under a document + +Suggested, not implemented: + +```http +GET /v1/documents/{document_id}/jobs +``` + +or, if the product name is "revisions": + +```http +GET /v1/documents/{document_id}/revisions +``` + +Returns the job ledger rows that produced this document (including failed +retries), newest first. Jobs stay immutable. This is not job deletion and +not a namespace filter on `GET /jobs`. + +### 2. Pending uploads with no document yet + +Clients should keep the job id returned at create time. If a self-hosted +app must reconcile abandoned uploads without that id, design that around +the existing external `data_id`, as a separate RFC. Do not add +`GET /jobs?namespace=` as a file browser. + +### 3. "What folders exist?" for an uploader ≠ consumer + +#232's on-prem case (gdccyuen, 2026-08-13): one credential, many domain +folders, the reader did not choose the namespace strings. + +Do **not** answer that with a Namespace resource. + +Options, in order of preference: + +1. **Derive from documents.** `GET /documents` (paged) already returns + each document's namespace. The client unique-sorts labels. Empty + folders do not exist in Knowhere; that matches "namespace appears only + after a document is created." +2. If pagination makes (1) too expensive, an **explicitly administrative + document-aggregation** query (for example distinct `documents.namespace` + for non-archived rows under this credential) may be considered later. It + must not be named or documented as `GET /namespaces`, must not imply + empty namespaces, and must not be mounted as a first-class resource. + +Archived-only labels stay out of that aggregation unless a later admin +tool asks for them separately. + +## What not to merge from #232 / #231 + +- `GET /documents/namespaces` (or `/api/v2/documents/namespaces` via the + v1-mounted-under-v2 router) as a Namespace resource +- `GET /jobs?namespace=` as a document-listing interface +- `DELETE /jobs/{job_id}` / soft-delete of the processing ledger +- Counting "active" documents as `status != archived` while calling them + "active document counts" without defining `status` + +#231's discussion already includes agreement to drop the job-listing / +job-deletion approach. This RFC records that direction so a future +document-history PR does not revive those routes. + +## Testing expectations for a later implementation PR + +- Contract tests for `GET /documents?namespace=` remain the listing + interface (already present in `test_documents_contract.py`). +- Any new `/documents/{id}/jobs` (or `/revisions`) test must show: archived + document still returns job history; deleting/archiving the document does + not hide jobs. +- No test should encode `GET /jobs?namespace=` as the supported client + workflow. + +## Ask + +@suguanYang: confirm (1) listing stays on `GET /documents?namespace=`, +(2) history is document-scoped jobs/revisions, (3) no Namespace resource +without a separate domain-contract change. + +@gdccyuen @SusannaShu: this is not a competing implementation of #232/#231. +If you want to continue, the document-history route is the remaining +gap; namespace discovery should be derived from documents or an explicit +admin aggregation, not a namespace resource. From 1e3cc2d72933e6dda76319a8c9c3476e85df90df Mon Sep 17 00:00:00 2001 From: Frankie-Xu <92643488+Frankie-Xu@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:39:34 +0800 Subject: [PATCH 2/3] feat(documents): add scoped processing history and namespace traversal --- apps/api/app/api/v1/routes/documents.py | 33 +- .../app/repositories/document_repository.py | 62 +++- .../services/documents/lifecycle_service.py | 56 +++- .../test_document_history_contract.py | 311 ++++++++++++++++++ docs/design/document-centric-list-history.md | 232 +++++++------ 5 files changed, 567 insertions(+), 127 deletions(-) create mode 100644 apps/api/tests/contract/test_document_history_contract.py diff --git a/apps/api/app/api/v1/routes/documents.py b/apps/api/app/api/v1/routes/documents.py index c9c4f2444..8fbaaebe7 100644 --- a/apps/api/app/api/v1/routes/documents.py +++ b/apps/api/app/api/v1/routes/documents.py @@ -12,7 +12,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from shared.core.database import get_db -from shared.core.exceptions.domain_exceptions import NotFoundException +from shared.core.exceptions.domain_exceptions import NotFoundException, ValidationException from shared.models.schemas.retrieval_namespace import normalize_retrieval_namespace router = APIRouter(tags=["Documents"]) @@ -44,12 +44,20 @@ async def _archive_document_response( @router.get("") async def list_documents( namespace: str | None = Query(None, max_length=255), + all_namespaces: bool = Query( + False, description="List user-owned documents across all namespaces", + ), page: int = Query(1, ge=1, description="Page number"), page_size: int = Query(50, ge=1, le=200, description="Items per page"), current_user: CurrentUser = Depends(with_current_user), db: AsyncSession = Depends(get_db), ): - effective_namespace = normalize_retrieval_namespace(namespace) + if all_namespaces and namespace is not None: + raise ValidationException( + user_message="namespace cannot be combined with all_namespaces", + violations=[{"field": "namespace", "description": "Omit when all_namespaces is true"}], + ) + effective_namespace = None if all_namespaces else normalize_retrieval_namespace(namespace) response = await _document_service.list_documents( db, user_id=current_user.user_id, @@ -60,6 +68,27 @@ async def list_documents( return response +@router.get("/{document_id}/jobs") +async def list_document_jobs( + document_id: str, + page: int = Query(1, ge=1, description="Page number"), + page_size: int = Query(50, ge=1, le=200, description="Items per page"), + current_user: CurrentUser = Depends(with_current_user), + db: AsyncSession = Depends(get_db), +): + """Read processing history, including failed attempts and archived documents.""" + response = await _document_service.list_document_jobs( + db, user_id=current_user.user_id, document_id=document_id, + page=page, page_size=page_size, + ) + if response is None: + raise NotFoundException( + resource="Document", resource_id=document_id, + internal_message="Document not found", + ) + return response + + @router.get("/{document_id}") async def get_document( document_id: str, diff --git a/apps/api/app/repositories/document_repository.py b/apps/api/app/repositories/document_repository.py index 110a96fad..43baf45a2 100644 --- a/apps/api/app/repositories/document_repository.py +++ b/apps/api/app/repositories/document_repository.py @@ -10,7 +10,7 @@ from datetime import datetime, timezone from typing import Sequence, cast -from sqlalchemy import func, select, true +from sqlalchemy import and_, func, or_, select, true from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import noload @@ -20,6 +20,27 @@ DocumentChunkRow = tuple[DocumentChunk, DocumentSection | None, JobResult] DocumentJobRevisionRow = tuple[Document, JobResult, Job] +DocumentJobHistoryRow = tuple[Job, JobResult | None] + + +def _document_job_history_query(*, user_id: str, document_id: str): + # A published result owns the canonical link. Metadata preserves attempts + # that failed or are still processing, and legacy results without a link. + return ( + select(Job, JobResult) + .outerjoin(JobResult, JobResult.job_id == Job.job_id) + .where(Job.user_id == user_id) + .where( + or_( + JobResult.document_id == document_id, + and_( + JobResult.document_id.is_(None), + JobResult.demo_document_id.is_(None), + Job.job_metadata["document_id"].as_string() == document_id, + ), + ) + ) + ) class DocumentRepository: @@ -28,7 +49,7 @@ async def list_by_user_namespace( db: AsyncSession, *, user_id: str, - namespace: str, + namespace: str | None, limit: int, offset: int, ) -> Sequence[Document]: @@ -36,7 +57,7 @@ async def list_by_user_namespace( result = await db.execute( select(corpusStorage.Document) .where(corpusStorage.Document.user_id == corpusStorage.resolve_owner(user_id)) - .where(corpusStorage.Document.namespace == namespace) + .where(corpusStorage.Document.namespace == namespace if namespace is not None else true()) .where(corpusStorage.Document.status != "archived") .order_by(corpusStorage.Document.updated_at.desc(), corpusStorage.Document.document_id.asc()) .limit(limit) @@ -49,13 +70,13 @@ async def count_by_user_namespace( db: AsyncSession, *, user_id: str, - namespace: str, + namespace: str | None, ) -> int: corpusStorage: CorpusStorage = CorpusStorage.resolve_namespace(namespace) result = await db.execute( select(func.count(corpusStorage.Document.document_id)) .where(corpusStorage.Document.user_id == corpusStorage.resolve_owner(user_id)) - .where(corpusStorage.Document.namespace == namespace) + .where(corpusStorage.Document.namespace == namespace if namespace is not None else true()) .where(corpusStorage.Document.status != "archived") ) return int(result.scalar_one()) @@ -76,6 +97,37 @@ async def get_document( ) return result.scalar_one_or_none() + async def count_document_jobs( + self, + db: AsyncSession, + *, + user_id: str, + document_id: str, + ) -> int: + result = await db.execute( + _document_job_history_query(user_id=user_id, document_id=document_id) + .with_only_columns(func.count(Job.job_id), maintain_column_froms=True) + ) + return int(result.scalar_one()) + + async def list_document_jobs( + self, + db: AsyncSession, + *, + user_id: str, + document_id: str, + limit: int, + offset: int, + ) -> Sequence[DocumentJobHistoryRow]: + result = await db.execute( + _document_job_history_query(user_id=user_id, document_id=document_id) + .options(noload(JobResult.chunks), noload(JobResult.job)) + .order_by(Job.created_at.desc(), Job.job_id.asc()) + .limit(limit) + .offset(offset) + ) + return cast(Sequence[DocumentJobHistoryRow], result.all()) + async def get_current_document_job_revision( self, db: AsyncSession, diff --git a/apps/api/app/services/documents/lifecycle_service.py b/apps/api/app/services/documents/lifecycle_service.py index a5b44a4c4..b67260725 100644 --- a/apps/api/app/services/documents/lifecycle_service.py +++ b/apps/api/app/services/documents/lifecycle_service.py @@ -265,7 +265,7 @@ async def list_documents( db: AsyncSession, *, user_id: str, - namespace: str, + namespace: str | None, page: int, page_size: int, ) -> dict[str, Any]: @@ -292,6 +292,60 @@ async def list_documents( }, } + async def list_document_jobs( + self, + db: AsyncSession, + *, + user_id: str, + document_id: str, + page: int, + page_size: int, + ) -> dict[str, Any] | None: + document = await self._repository.get_document( + db, user_id=user_id, document_id=document_id, + ) + if document is None: + return None + + # Shared demo documents do not expose their server-owned job ledger. + is_demo = CorpusStorage.resolve_document(document_id).is_demo + total = 0 if is_demo else await self._repository.count_document_jobs( + db, user_id=user_id, document_id=document_id, + ) + rows = [] if is_demo else await self._repository.list_document_jobs( + db, user_id=user_id, document_id=document_id, + limit=page_size, offset=(page - 1) * page_size, + ) + return { + "document_id": document.document_id, + "namespace": document.namespace, + "jobs": [ + { + "job_id": job.job_id, + "job_type": job.job_type, + "status": job.status, + "source_type": job.source_type, + "error_code": job.error_code, + "page_count": job.page_count, + "credits_charged": job.credits_charged, + "billing_status": job.billing_status, + "created_at": _datetime_payload(job.created_at), + "updated_at": _datetime_payload(job.updated_at), + "job_result_id": job_result.id if job_result else None, + "is_current_revision": bool( + job_result and job_result.id == document.current_job_result_id + ), + } + for job, job_result in rows + ], + "pagination": { + "page": page, + "page_size": page_size, + "total": total, + "total_pages": math.ceil(total / page_size) if total else 0, + }, + } + async def list_document_chunks( self, db: AsyncSession, diff --git a/apps/api/tests/contract/test_document_history_contract.py b/apps/api/tests/contract/test_document_history_contract.py new file mode 100644 index 000000000..123dc721c --- /dev/null +++ b/apps/api/tests/contract/test_document_history_contract.py @@ -0,0 +1,311 @@ +from collections.abc import Callable +from contextlib import AbstractAsyncContextManager +from datetime import datetime, timedelta + +import pytest +from httpx import AsyncClient + +from tests.support.contract_database import ContractDatabase + + +async def _document( + document_id: str, + *, + namespace: str = "history", + user_id: str = "local-dev-user", + status: str = "active", +) -> None: + await ContractDatabase.execute( + """INSERT INTO documents ( + document_id, user_id, namespace, status, parse_track, created_at, updated_at + ) VALUES (:id, :user, :namespace, :status, 'chunk', :timestamp, :timestamp)""", + { + "id": document_id, + "user": user_id, + "namespace": namespace, + "status": status, + "timestamp": datetime(2026, 1, 1), + }, + ) + + +@pytest.mark.parametrize("version", ["v1", "v2"]) +async def test_document_history_keeps_attempts_and_revisions_after_archive( + developer_api_client_factory: Callable[ + [], AbstractAsyncContextManager[AsyncClient] + ], + version: str, +) -> None: + async with developer_api_client_factory() as client: + await _document("doc_history") + await _document("doc_other") + await ContractDatabase.insert_user(user_id="other-history-user") + timestamp = datetime(2026, 1, 1) + for job_id, status, metadata, minute, owner in [ + ("job_old", "done", {}, 0, "local-dev-user"), + ( + "job_failed", + "failed", + {"document_id": "doc_history"}, + 1, + "local-dev-user", + ), + ("job_current", "done", {"document_id": "doc_other"}, 2, "local-dev-user"), + ( + "job_retry_a", + "running", + {"document_id": "doc_history"}, + 3, + "local-dev-user", + ), + ( + "job_retry_b", + "failed", + {"document_id": "doc_history"}, + 3, + "local-dev-user", + ), + ( + "job_conflict", + "done", + {"document_id": "doc_history"}, + 4, + "local-dev-user", + ), + ( + "job_foreign", + "failed", + {"document_id": "doc_history"}, + 5, + "other-history-user", + ), + ("job_unrelated", "done", {"namespace": "history"}, 6, "local-dev-user"), + ]: + await ContractDatabase.insert_job( + job_id=job_id, + user_id=owner, + status=status, + job_metadata=metadata, + created_at=timestamp + timedelta(minutes=minute), + error_code="INVALID_ARGUMENT" if status == "failed" else None, + ) + for job_id, result_id, document_id in [ + ("job_old", "result_old", "doc_history"), + ("job_current", "result_current", "doc_history"), + ("job_retry_b", "result_legacy", None), + ("job_conflict", "result_conflict", "doc_other"), + ("job_foreign", "result_foreign", "doc_history"), + ]: + await ContractDatabase.insert_job_result( + job_result_id=result_id, + job_id=job_id, + document_id=document_id, + ) + await ContractDatabase.execute( + "UPDATE documents SET current_job_result_id = 'result_current' WHERE document_id = 'doc_history'" + ) + url = f"/api/{version}/documents/doc_history/jobs" + before = await client.get(url) + assert before.status_code == 200 + body = before.json() + assert body["document_id"] == "doc_history" + assert body["namespace"] == "history" + assert body["pagination"]["total"] == 5 + assert [job["job_id"] for job in body["jobs"]] == [ + "job_retry_a", + "job_retry_b", + "job_current", + "job_failed", + "job_old", + ] + assert [ + job["job_id"] for job in body["jobs"] if job["is_current_revision"] + ] == ["job_current"] + assert body["jobs"][0]["job_result_id"] is None + assert body["jobs"][1]["job_result_id"] == "result_legacy" + assert body["jobs"][1]["error_code"] == "INVALID_ARGUMENT" + assert all("job_metadata" not in job for job in body["jobs"]) + pages = [ + await client.get(url, params={"page": page, "page_size": 2}) + for page in (1, 2, 3, 4) + ] + assert [ + job["job_id"] for response in pages for job in response.json()["jobs"] + ] == [job["job_id"] for job in body["jobs"]] + assert pages[-1].json()["pagination"] == { + "page": 4, + "page_size": 2, + "total": 5, + "total_pages": 3, + } + assert ( + await client.post(f"/api/{version}/documents/doc_history/archive") + ).status_code == 200 + after = await client.get(url) + assert after.status_code == 200 + assert after.json() == body + listing = await client.get( + f"/api/{version}/documents", params={"namespace": "history"} + ) + assert "doc_history" not in [ + document["document_id"] for document in listing.json()["documents"] + ] + assert (await client.get(f"/api/{version}/jobs/job_failed")).status_code == 200 + retained_job = await ContractDatabase.fetch_job("job_failed") + assert retained_job is not None and retained_job["status"] == "failed" + + +@pytest.mark.parametrize("version", ["v1", "v2"]) +async def test_document_history_is_empty_or_not_found_without_leaking_other_owners( + developer_api_client_factory: Callable[ + [], AbstractAsyncContextManager[AsyncClient] + ], + version: str, +) -> None: + async with developer_api_client_factory() as client: + await _document("doc_empty", status="archived") + await ContractDatabase.insert_user(user_id="history-foreign-owner") + await _document("doc_foreign", user_id="history-foreign-owner") + await ContractDatabase.insert_job( + job_id="job_pending_upload", + user_id="local-dev-user", + job_metadata={"document_id": "doc_unmaterialized"}, + ) + response = await client.get(f"/api/{version}/documents/doc_empty/jobs") + assert response.status_code == 200 + assert response.json()["jobs"] == [] + assert response.json()["pagination"]["total"] == 0 + for document_id in ( + "doc_missing", + "doc_foreign", + "doc_unmaterialized", + "ddoc_missing", + ): + response = await client.get(f"/api/{version}/documents/{document_id}/jobs") + assert response.status_code == 404 + + +@pytest.mark.parametrize("version", ["v1", "v2"]) +async def test_all_namespace_document_listing_preserves_defaults_and_owner_scope( + developer_api_client_factory: Callable[ + [], AbstractAsyncContextManager[AsyncClient] + ], + version: str, +) -> None: + async with developer_api_client_factory() as client: + from shared.core.config import settings + from shared.core.database import get_db_context + from shared.models.database.demo_corpus import DemoDocument + from shared.services.retrieval.demo_authorization import ( + authorize_demo_transaction, + ) + + settings.DEMO_MAINTAINER_USER_IDS = "local-dev-user" + async with get_db_context() as db: + await db.run_sync( + lambda session: authorize_demo_transaction( + session, user_id="local-dev-user" + ) + ) + db.add( + DemoDocument( + document_id="ddoc_shared_history", + demo_source_id="history-demo", + title="Synthetic shared document", + user_id="__knowhere_demo__", + namespace="__knowhere_demo__", + status="active", + ) + ) + await db.commit() + settings.DEMO_MAINTAINER_USER_IDS = "" + await ContractDatabase.insert_job( + job_id="job_shared_publisher", + user_id="local-dev-user", + status="done", + job_metadata={"document_id": "ddoc_shared_history"}, + ) + await _document("doc_a", namespace="alpha") + await ContractDatabase.insert_job( + job_id="job_demo_conflict", + user_id="local-dev-user", + status="done", + job_metadata={"document_id": "doc_a"}, + ) + await ContractDatabase.insert_job_result( + job_result_id="result_demo_conflict", + job_id="job_demo_conflict", + ) + await ContractDatabase.execute( + "UPDATE job_results SET demo_document_id = 'ddoc_shared_history' WHERE id = 'result_demo_conflict'" + ) + await _document("doc_b", namespace="beta") + await _document("doc_default", namespace="default") + await _document("doc_archived", namespace="archived-only", status="archived") + await ContractDatabase.insert_user(user_id="list-foreign-owner") + await _document( + "doc_foreign", namespace="foreign", user_id="list-foreign-owner" + ) + url = f"/api/{version}/documents" + for params in ({}, {"namespace": ""}, {"namespace": " "}): + response = await client.get(url, params=params) + assert response.status_code == 200 + assert response.json()["namespace"] == "default" + assert [ + document["document_id"] for document in response.json()["documents"] + ] == ["doc_default"] + pages = [ + await client.get( + url, params={"all_namespaces": "true", "page": page, "page_size": 2} + ) + for page in (1, 2) + ] + assert all(response.status_code == 200 for response in pages) + assert pages[0].json()["namespace"] is None + assert pages[0].json()["pagination"]["total"] == 3 + documents = [ + document for response in pages for document in response.json()["documents"] + ] + assert [document["document_id"] for document in documents] == [ + "doc_a", + "doc_b", + "doc_default", + ] + assert {document["namespace"] for document in documents} == { + "alpha", + "beta", + "default", + } + demo_listing = await client.get(url, params={"namespace": "__knowhere_demo__"}) + assert demo_listing.status_code == 200 + assert "ddoc_shared_history" in [ + document["document_id"] for document in demo_listing.json()["documents"] + ] + demo_history = await client.get(f"{url}/ddoc_shared_history/jobs") + assert demo_history.status_code == 200 + assert demo_history.json()["jobs"] == [] + assert demo_history.json()["pagination"]["total"] == 0 + private_history = await client.get(f"{url}/doc_a/jobs") + assert private_history.status_code == 200 + assert private_history.json()["jobs"] == [] + assert private_history.json()["pagination"]["total"] == 0 + for namespace in ("alpha", ""): + assert ( + await client.get( + url, params={"all_namespaces": "true", "namespace": namespace} + ) + ).status_code == 400 + + +async def test_document_history_validates_pagination_and_requires_authentication( + developer_api_client_factory: Callable[ + [], AbstractAsyncContextManager[AsyncClient] + ], +) -> None: + async with developer_api_client_factory() as client: + for version in ("v1", "v2"): + url = f"/api/{version}/documents/doc_any/jobs" + for params in ({"page": 0}, {"page_size": 0}, {"page_size": 201}): + assert (await client.get(url, params=params)).status_code == 400 + response = await client.get(url, headers={"Authorization": ""}) + assert response.status_code == 401 diff --git a/docs/design/document-centric-list-history.md b/docs/design/document-centric-list-history.md index 71f1ba92f..4686cef49 100644 --- a/docs/design/document-centric-list-history.md +++ b/docs/design/document-centric-list-history.md @@ -1,137 +1,131 @@ # Document-centric list and history -**Status:** Draft — RFC only, no implementation in this change -**Related PRs:** [#232](https://github.com/Ontos-AI/knowhere/pull/232) (@gdccyuen), [#231](https://github.com/Ontos-AI/knowhere/pull/231) (@SusannaShu) -**Maintainer review:** [suguanYang on #232](https://github.com/Ontos-AI/knowhere/pull/232#issuecomment-5265960465), [suguanYang on #231](https://github.com/Ontos-AI/knowhere/pull/231#issuecomment-5265855328) -**Observed against:** `main` @ `9489fa2c` (2026-09-14) +**Status:** Draft implementation — API decisions below require maintainer review +**Related PRs:** [#232](https://github.com/Ontos-AI/knowhere/pull/232), [#231](https://github.com/Ontos-AI/knowhere/pull/231) +**Maintainer direction:** [namespace ownership](https://github.com/Ontos-AI/knowhere/pull/232#issuecomment-5265960465), [processing ledger](https://github.com/Ontos-AI/knowhere/pull/231#issuecomment-5265855328) +**Implementation base:** `main` @ `f9d61119` (2026-10-07) -This is a unifying API-ownership draft. It does not replace or rewrite -#232 / #231, and it does not include a code patch. Those PRs stay with -their authors. +Namespaces remain free-form client-owned isolation labels. Documents own the +persisted corpus; jobs preserve processing attempts, billing and provenance. +This implementation adds document-scoped history and explicit cross-namespace +document listing. It does not change #231/#232 or their authors' branches. -## Why this is an RFC +## List documents -Both PRs were blocked as **public API / resource-ownership** questions, not -as local bugs: - -- #232 `GET /documents/namespaces` does not list a Knowhere resource. - Namespace is a client-owned isolation label with no identity, create - operation, metadata, authz, or lifecycle. -- #231 `GET /jobs?namespace=` and job deletion treat a processing attempt as - a file registry. A job is the operational ledger and should remain after - the document is archived. - -Those are domain-contract changes. They should not land as drive-by -route additions. - -## Resource ownership (from maintainer, restated) - -| Concept | Owns | Does not own | -| --- | --- | --- | -| Namespace | Client (tenant / folder / workspace id) | Knowhere registry, empty folders, archived-only labels | -| Document | Persisted corpus: list, get, archive, retrieval scope | Processing attempts | -| Job | One parse attempt: status, billing, errors, provenance | Tenant file listing, delete-from-folder | - -## Existing surface (do not duplicate) - -On current `main`: - -- `GET /v1/documents?namespace=` already lists the credential's documents - in that namespace (`apps/api/app/api/v1/routes/documents.py`). -- `GET /v1/documents/{document_id}` and chunk listing already exist. -- Removal from the active corpus is `POST /v1/documents/{document_id}/archive` - (plus a legacy `:archive` alias). There is no `DELETE /documents/{id}`. -- `GET /v1/jobs` lists processing attempts with status/type/time filters. - It has **no** `namespace` query. -- `GET /v1/jobs/{job_id}` inspects one attempt. - -The keyboard-backend / on-prem folder workflow that #231 and #232 were -solving is therefore **mostly already on the document resource**: - -1. List a tenant folder: `GET /documents?namespace={label}` -2. Remove a file from that folder: `POST /documents/{document_id}/archive` -3. Poll a known upload: `GET /jobs/{job_id}` (client keeps the id while - pending) - -## Remaining gaps (only these need new API) - -### 1. Processing history under a document - -Suggested, not implemented: +Existing calls keep their behavior on both `/api/v1` and `/api/v2`: ```http -GET /v1/documents/{document_id}/jobs +GET /api/v1/documents?namespace=tenant-42&page=1&page_size=50 ``` -or, if the product name is "revisions": +Omitting `namespace`, passing an empty value, or passing whitespace selects +`default`. Listing returns non-archived documents for that namespace. It does +not enumerate namespaces or represent empty folders. + +A reader whose client does not retain labels may now explicitly page through +all of the credential owner's private documents: ```http -GET /v1/documents/{document_id}/revisions +GET /api/v1/documents?all_namespaces=true&page=1&page_size=50 ``` -Returns the job ledger rows that produced this document (including failed -retries), newest first. Jobs stay immutable. This is not job deletion and -not a namespace filter on `GET /jobs`. - -### 2. Pending uploads with no document yet +The existing response envelope remains `namespace`, `documents`, `pagination`. +For `all_namespaces=true`, the envelope's `namespace` is `null`; each document +retains its actual namespace. Documents are ordered by `updated_at` descending, +then `document_id` ascending. `namespace` and `all_namespaces=true` are mutually +exclusive (400 `INVALID_ARGUMENT`), including an explicitly empty namespace value. Page size is +1–200, default 50; page numbers start at 1. -Clients should keep the job id returned at create time. If a self-hosted -app must reconcile abandoned uploads without that id, design that around -the existing external `data_id`, as a separate RFC. Do not add -`GET /jobs?namespace=` as a file browser. +The client may derive labels from the returned documents. Archived-only and +empty labels have no entries. This reads private user-owned corpus rows; +shared demo documents remain available through the existing explicit +`namespace=__knowhere_demo__` query and are not mixed into this traversal. -### 3. "What folders exist?" for an uploader ≠ consumer +## Read document processing history -#232's on-prem case (gdccyuen, 2026-08-13): one credential, many domain -folders, the reader did not choose the namespace strings. - -Do **not** answer that with a Namespace resource. - -Options, in order of preference: - -1. **Derive from documents.** `GET /documents` (paged) already returns - each document's namespace. The client unique-sorts labels. Empty - folders do not exist in Knowhere; that matches "namespace appears only - after a document is created." -2. If pagination makes (1) too expensive, an **explicitly administrative - document-aggregation** query (for example distinct `documents.namespace` - for non-archived rows under this credential) may be considered later. It - must not be named or documented as `GET /namespaces`, must not imply - empty namespaces, and must not be mounted as a first-class resource. - -Archived-only labels stay out of that aggregation unless a later admin -tool asks for them separately. - -## What not to merge from #232 / #231 - -- `GET /documents/namespaces` (or `/api/v2/documents/namespaces` via the - v1-mounted-under-v2 router) as a Namespace resource -- `GET /jobs?namespace=` as a document-listing interface -- `DELETE /jobs/{job_id}` / soft-delete of the processing ledger -- Counting "active" documents as `status != archived` while calling them - "active document counts" without defining `status` - -#231's discussion already includes agreement to drop the job-listing / -job-deletion approach. This RFC records that direction so a future -document-history PR does not revive those routes. - -## Testing expectations for a later implementation PR - -- Contract tests for `GET /documents?namespace=` remain the listing - interface (already present in `test_documents_contract.py`). -- Any new `/documents/{id}/jobs` (or `/revisions`) test must show: archived - document still returns job history; deleting/archiving the document does - not hide jobs. -- No test should encode `GET /jobs?namespace=` as the supported client - workflow. - -## Ask +```http +GET /api/v1/documents/{document_id}/jobs?page=1&page_size=50 +GET /api/v2/documents/{document_id}/jobs?page=1&page_size=50 +``` -@suguanYang: confirm (1) listing stays on `GET /documents?namespace=`, -(2) history is document-scoped jobs/revisions, (3) no Namespace resource -without a separate domain-contract change. +The document must exist and belong to the credential owner. An archived +private document remains readable. Missing documents, another user's +documents, and initial uploads that have not materialized a document return +404. An existing document with no associated jobs returns an empty page. +Shared demo documents return an empty history; their internal publisher's +processing and billing ledger is not exposed to corpus readers. + +Each item summarizes one attempt; a failed or running attempt need not have a +published revision: + +```json +{ + "document_id": "doc_example", + "namespace": "tenant-42", + "jobs": [ + { + "job_id": "job_retry", + "job_type": "document_ingestion", + "status": "failed", + "source_type": "file", + "error_code": "INVALID_ARGUMENT", + "page_count": null, + "credits_charged": 0, + "billing_status": "pending", + "created_at": "2026-10-07T10:00:00", + "updated_at": "2026-10-07T10:01:00", + "job_result_id": null, + "is_current_revision": false + } + ], + "pagination": {"page": 1, "page_size": 50, "total": 1, "total_pages": 1} +} +``` -@gdccyuen @SusannaShu: this is not a competing implementation of #232/#231. -If you want to continue, the document-history route is the remaining -gap; namespace discovery should be derived from documents or an explicit -admin aggregation, not a namespace resource. +Jobs are ordered by `created_at` descending, then `job_id` ascending so equal +timestamps paginate consistently. Pagination has the same bounds as document +listing. Counts and pages are separate reads, so concurrent submissions may +change totals between requests; this is offset pagination, not a frozen +snapshot. + +Association rules: + +1. `job_results.document_id` is the canonical persisted link and takes priority. +2. When both the private and shared-demo canonical result links are absent + (including no result), + `jobs.job_metadata.document_id` associates failed, pending and legacy attempts. + A result published to a demo document cannot enter private history through + conflicting metadata. +3. The job must independently belong to the requesting user. A namespace match, + filename or external `data_id` alone does not associate a job. + +Every associated result is represented once. `job_result_id` identifies the +attempt's result when present; `is_current_revision` compares it with the +current document pointer. Full job metadata, source URLs, result payloads, +asset URLs and raw error messages are excluded from the summary. Use the +existing `GET /jobs/{job_id}` to inspect a known attempt in detail. + +Archiving removes the document from listing/retrieval and leaves its job ledger +and document-scoped history readable. This endpoint has no deletion or hiding +operation and performs no ledger mutation. + +## Pending submissions and namespace ownership + +Clients retain the job ID returned at upload and poll `GET /jobs/{job_id}`. +History is only available once the document exists. Durable reconciliation of +abandoned initial uploads, potentially using external `data_id`, remains a +separate design problem. + +No namespace registry, `GET /documents/namespaces`, namespace-filtered job +listing or job deletion is introduced. No schema migration or environment +configuration is required. `all_namespaces`, history association precedence, +summary fields and demo behavior are local Draft decisions for review. + +## Verification + +`apps/api/tests/contract/test_document_history_contract.py` exercises actual +HTTP routes with synthetic PostgreSQL data on v1 and v2: old/current revisions, +failed and running attempts, canonical-only and legacy links, conflicting +links, stable pagination, archive retention, job inspection, unknown/foreign +and unmaterialized documents, empty history, authentication, pagination bounds, +and cross-namespace listing with unchanged default/blank behavior. From b2f843451aefc39ed3fb8df86cd6660e45968f11 Mon Sep 17 00:00:00 2001 From: Frankie-Xu <92643488+Frankie-Xu@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:46:04 +0800 Subject: [PATCH 3/3] test(documents): exercise historical revision reads after archive --- .../test_document_history_contract.py | 31 +++++++++++++++++++ docs/design/document-centric-list-history.md | 8 +++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/apps/api/tests/contract/test_document_history_contract.py b/apps/api/tests/contract/test_document_history_contract.py index 123dc721c..e83c267e8 100644 --- a/apps/api/tests/contract/test_document_history_contract.py +++ b/apps/api/tests/contract/test_document_history_contract.py @@ -101,6 +101,18 @@ async def test_document_history_keeps_attempts_and_revisions_after_archive( job_id=job_id, document_id=document_id, ) + for revision in ("old", "current"): + await ContractDatabase.insert_document_chunk( + chunk_id=f"chunk_{revision}", + user_id="local-dev-user", + namespace="history", + document_id="doc_history", + job_result_id=f"result_{revision}", + section_id=None, + chunk_type="text", + content=f"{revision} revision content", + section_path=f"{revision}.json", + ) await ContractDatabase.execute( "UPDATE documents SET current_job_result_id = 'result_current' WHERE document_id = 'doc_history'" ) @@ -125,6 +137,20 @@ async def test_document_history_keeps_attempts_and_revisions_after_archive( assert body["jobs"][1]["job_result_id"] == "result_legacy" assert body["jobs"][1]["error_code"] == "INVALID_ARGUMENT" assert all("job_metadata" not in job for job in body["jobs"]) + older_result_id = next( + job["job_result_id"] for job in body["jobs"] if job["job_id"] == "job_old" + ) + chunks_url = f"/api/{version}/documents/doc_history/chunks" + older_revision = await client.get( + chunks_url, params={"job_result_id": older_result_id} + ) + assert older_revision.status_code == 200 + older_payload = older_revision.json() + assert older_payload["job_result_id"] == "result_old" + assert older_payload["job_id"] == "job_old" + assert [chunk["content"] for chunk in older_payload["chunks"]] == [ + "old revision content" + ] pages = [ await client.get(url, params={"page": page, "page_size": 2}) for page in (1, 2, 3, 4) @@ -144,6 +170,11 @@ async def test_document_history_keeps_attempts_and_revisions_after_archive( after = await client.get(url) assert after.status_code == 200 assert after.json() == body + archived_revision = await client.get( + chunks_url, params={"job_result_id": older_result_id} + ) + assert archived_revision.status_code == 200 + assert archived_revision.json() == older_payload listing = await client.get( f"/api/{version}/documents", params={"namespace": "history"} ) diff --git a/docs/design/document-centric-list-history.md b/docs/design/document-centric-list-history.md index 4686cef49..addadb65e 100644 --- a/docs/design/document-centric-list-history.md +++ b/docs/design/document-centric-list-history.md @@ -103,7 +103,10 @@ Every associated result is represented once. `job_result_id` identifies the attempt's result when present; `is_current_revision` compares it with the current document pointer. Full job metadata, source URLs, result payloads, asset URLs and raw error messages are excluded from the summary. Use the -existing `GET /jobs/{job_id}` to inspect a known attempt in detail. +existing `GET /jobs/{job_id}` to inspect a known attempt in detail. A returned +published `job_result_id` can select an older revision through the existing +`GET /documents/{document_id}/chunks?job_result_id=...`, including after private +archive. Private document detail continues to describe the current document. Archiving removes the document from listing/retrieval and leaves its job ledger and document-scoped history readable. This endpoint has no deletion or hiding @@ -128,4 +131,5 @@ HTTP routes with synthetic PostgreSQL data on v1 and v2: old/current revisions, failed and running attempts, canonical-only and legacy links, conflicting links, stable pagination, archive retention, job inspection, unknown/foreign and unmaterialized documents, empty history, authentication, pagination bounds, -and cross-namespace listing with unchanged default/blank behavior. +cross-namespace listing with unchanged default/blank behavior, and reading an +older revision's chunks from a history result ID before and after archive.