feat(points): add the /v1/points read paths
Why:
- Ingestion writes points in bulk but nothing could read one back. Plan 002
Phase 2 opens the read surface an admin frontend needs.
Changes:
- GET /v1/points/{point_id}, /v1/points?file_id=..., /v1/points/count,
/v1/points/search, and /v1/files/{file_id}/points, all under points:read --
the scope follows the data, so an upload key does not become a way to read
every chunk of every file.
- The keyword query is Persian-normalized before matching, because ingestion
letter-folds content at ingest and an unfolded query would return an empty
result set silently rather than an error.
- file_id is required on the listing: the cursor is an order_id value and
order_id is only unique within one file.
- PointNotFoundError maps to 404, never 403, so a cross-tenant point id is
indistinguishable from a nonexistent one.
- Route order is load-bearing: /count and /search precede /{point_id}, or
"count" is parsed as a UUID and fails 422.
Impact:
- Requires the content/is_active/chunk_index payload indexes, so a deployed
environment needs qdrant_bootstrap re-run before search works.
- Keyword search returns no relevance score and no ranked order; callers must
not read array position as relevance.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -27,6 +27,7 @@ from src.application.ingestion.errors import (
|
|||||||
PointIndexingError,
|
PointIndexingError,
|
||||||
UnsupportedSourceTypeError,
|
UnsupportedSourceTypeError,
|
||||||
)
|
)
|
||||||
|
from src.application.points.point import PointNotFoundError
|
||||||
|
|
||||||
logger = structlog.get_logger(__name__)
|
logger = structlog.get_logger(__name__)
|
||||||
|
|
||||||
@@ -41,6 +42,9 @@ _MAPPING: tuple[tuple[type[Exception], int, str], ...] = (
|
|||||||
(TenantInactiveError, status.HTTP_401_UNAUTHORIZED, "tenant_not_found"),
|
(TenantInactiveError, status.HTTP_401_UNAUTHORIZED, "tenant_not_found"),
|
||||||
(MissingScopeError, status.HTTP_403_FORBIDDEN, "missing_scope"),
|
(MissingScopeError, status.HTTP_403_FORBIDDEN, "missing_scope"),
|
||||||
(InvalidUploadError, status.HTTP_400_BAD_REQUEST, "validation_error"),
|
(InvalidUploadError, status.HTTP_400_BAD_REQUEST, "validation_error"),
|
||||||
|
# 404, never 403: a cross-tenant point id must be indistinguishable from a
|
||||||
|
# nonexistent one, or the API becomes an existence oracle (ADR-0016).
|
||||||
|
(PointNotFoundError, status.HTTP_404_NOT_FOUND, "not_found"),
|
||||||
(UnknownDomainError, status.HTTP_400_BAD_REQUEST, "unknown_domain"),
|
(UnknownDomainError, status.HTTP_400_BAD_REQUEST, "unknown_domain"),
|
||||||
(DomainAlreadyExistsError, status.HTTP_409_CONFLICT, "conflict"),
|
(DomainAlreadyExistsError, status.HTTP_409_CONFLICT, "conflict"),
|
||||||
(DocumentParseError, status.HTTP_400_BAD_REQUEST, "validation_error"),
|
(DocumentParseError, status.HTTP_400_BAD_REQUEST, "validation_error"),
|
||||||
|
|||||||
@@ -2,7 +2,9 @@ from fastapi import APIRouter
|
|||||||
|
|
||||||
from src.api.routers.domains import router as domains_router
|
from src.api.routers.domains import router as domains_router
|
||||||
from src.api.routers.files import router as files_router
|
from src.api.routers.files import router as files_router
|
||||||
|
from src.api.routers.points import router as points_router
|
||||||
|
|
||||||
router = APIRouter()
|
router = APIRouter()
|
||||||
router.include_router(domains_router)
|
router.include_router(domains_router)
|
||||||
router.include_router(files_router)
|
router.include_router(files_router)
|
||||||
|
router.include_router(points_router)
|
||||||
|
|||||||
@@ -14,17 +14,21 @@ from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
|||||||
|
|
||||||
from src.api.dependencies.auth import require_scope
|
from src.api.dependencies.auth import require_scope
|
||||||
from src.api.schemas.files import FileStatusResponse, FileUploadResponse
|
from src.api.schemas.files import FileStatusResponse, FileUploadResponse
|
||||||
|
from src.api.schemas.points import DEFAULT_PAGE_SIZE, LimitQuery, PointListResponse
|
||||||
from src.application.auth.context import AuthContext
|
from src.application.auth.context import AuthContext
|
||||||
from src.application.files.status import get_file_status
|
from src.application.files.status import get_file_status
|
||||||
from src.application.files.upload import upload_source_file
|
from src.application.files.upload import upload_source_file
|
||||||
|
from src.application.points.queries import list_file_points
|
||||||
from src.application.ports.embedding import DenseEmbedder, SparseEmbedder
|
from src.application.ports.embedding import DenseEmbedder, SparseEmbedder
|
||||||
from src.application.ports.object_storage import ObjectStorage
|
from src.application.ports.object_storage import ObjectStorage
|
||||||
|
from src.application.ports.point_repository import PointRepository
|
||||||
from src.application.ports.point_storage import PointStorage
|
from src.application.ports.point_storage import PointStorage
|
||||||
from src.bootstrap.dependencies import (
|
from src.bootstrap.dependencies import (
|
||||||
get_dense_embedders,
|
get_dense_embedders,
|
||||||
get_ingestion_concurrency_limiter,
|
get_ingestion_concurrency_limiter,
|
||||||
get_ingestion_limiter,
|
get_ingestion_limiter,
|
||||||
get_object_storage,
|
get_object_storage,
|
||||||
|
get_point_repository,
|
||||||
get_point_storage,
|
get_point_storage,
|
||||||
get_sessionmaker,
|
get_sessionmaker,
|
||||||
get_settings,
|
get_settings,
|
||||||
@@ -35,9 +39,11 @@ from src.config import Settings
|
|||||||
router = APIRouter(prefix="/files", tags=["files"])
|
router = APIRouter(prefix="/files", tags=["files"])
|
||||||
|
|
||||||
_RequireFilesWrite = Annotated[AuthContext, Depends(require_scope("files:write"))]
|
_RequireFilesWrite = Annotated[AuthContext, Depends(require_scope("files:write"))]
|
||||||
|
_RequirePointsRead = Annotated[AuthContext, Depends(require_scope("points:read"))]
|
||||||
_SessionmakerDep = Annotated[async_sessionmaker[AsyncSession], Depends(get_sessionmaker)]
|
_SessionmakerDep = Annotated[async_sessionmaker[AsyncSession], Depends(get_sessionmaker)]
|
||||||
_ObjectStorageDep = Annotated[ObjectStorage, Depends(get_object_storage)]
|
_ObjectStorageDep = Annotated[ObjectStorage, Depends(get_object_storage)]
|
||||||
_PointStorageDep = Annotated[PointStorage, Depends(get_point_storage)]
|
_PointStorageDep = Annotated[PointStorage, Depends(get_point_storage)]
|
||||||
|
_PointRepositoryDep = Annotated[PointRepository, Depends(get_point_repository)]
|
||||||
_SettingsDep = Annotated[Settings, Depends(get_settings)]
|
_SettingsDep = Annotated[Settings, Depends(get_settings)]
|
||||||
_IngestionLimiterDep = Annotated[CapacityLimiter, Depends(get_ingestion_limiter)]
|
_IngestionLimiterDep = Annotated[CapacityLimiter, Depends(get_ingestion_limiter)]
|
||||||
_ConcurrencyLimiterDep = Annotated[Semaphore, Depends(get_ingestion_concurrency_limiter)]
|
_ConcurrencyLimiterDep = Annotated[Semaphore, Depends(get_ingestion_concurrency_limiter)]
|
||||||
@@ -92,3 +98,29 @@ async def get_file(
|
|||||||
if result is None:
|
if result is None:
|
||||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="file not found")
|
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="file not found")
|
||||||
return FileStatusResponse.from_result(result)
|
return FileStatusResponse.from_result(result)
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/{file_id}/points")
|
||||||
|
async def list_points_for_file(
|
||||||
|
file_id: uuid.UUID,
|
||||||
|
auth: _RequirePointsRead,
|
||||||
|
repository: _PointRepositoryDep,
|
||||||
|
limit: LimitQuery = DEFAULT_PAGE_SIZE,
|
||||||
|
cursor: str | None = None,
|
||||||
|
include_inactive: bool = False,
|
||||||
|
) -> PointListResponse:
|
||||||
|
"""The same listing as `GET /v1/points?file_id=...`, addressed by file.
|
||||||
|
|
||||||
|
Gated on `points:read`, not `files:write`: the resource being read is the
|
||||||
|
file's chunks, so the scope follows the data rather than the URL prefix. An
|
||||||
|
upload-only key must not become a way to read every chunk of every file.
|
||||||
|
"""
|
||||||
|
page = await list_file_points(
|
||||||
|
repository,
|
||||||
|
tenant_id=auth.tenant_id,
|
||||||
|
file_id=file_id,
|
||||||
|
limit=limit,
|
||||||
|
cursor=cursor,
|
||||||
|
include_inactive=include_inactive,
|
||||||
|
)
|
||||||
|
return PointListResponse.from_page(page)
|
||||||
|
|||||||
134
src/api/routers/points.py
Normal file
134
src/api/routers/points.py
Normal file
@@ -0,0 +1,134 @@
|
|||||||
|
"""`/v1/points` read paths (ADR-0002, ADR-0008).
|
||||||
|
|
||||||
|
Routes adapt HTTP to `application/points` calls. They build no Qdrant filters
|
||||||
|
and hold no CRUD semantics (ADR-0015), and they never read a tenant from the
|
||||||
|
request — `auth.tenant_id` is the only source, which is what makes ADR-0002's
|
||||||
|
isolation rule structural rather than a habit.
|
||||||
|
|
||||||
|
**Route order is load-bearing.** `/count` and `/search` are declared before
|
||||||
|
`/{point_id}`. FastAPI matches in declaration order, so with `/{point_id}` first
|
||||||
|
a request for `/v1/points/count` would try to parse `"count"` as a UUID and
|
||||||
|
fail with `422` instead of counting anything. The failure is loud but confusing,
|
||||||
|
and it comes back the moment someone reorders these for tidiness.
|
||||||
|
|
||||||
|
Gated on `points:read`, separately from `files:write`: a key that can upload
|
||||||
|
documents should not thereby be able to read every chunk of every file, and
|
||||||
|
plan 002's mutating paths will want `points:write` distinct again.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import uuid
|
||||||
|
from typing import Annotated
|
||||||
|
|
||||||
|
from fastapi import APIRouter, Depends, Query
|
||||||
|
|
||||||
|
from src.api.dependencies.auth import require_scope
|
||||||
|
from src.api.schemas.points import (
|
||||||
|
DEFAULT_PAGE_SIZE,
|
||||||
|
LimitQuery,
|
||||||
|
PointCountResponse,
|
||||||
|
PointListResponse,
|
||||||
|
PointResponse,
|
||||||
|
PointSearchResponse,
|
||||||
|
)
|
||||||
|
from src.application.auth.context import AuthContext
|
||||||
|
from src.application.points.queries import (
|
||||||
|
count_points,
|
||||||
|
get_point,
|
||||||
|
list_file_points,
|
||||||
|
search_points,
|
||||||
|
)
|
||||||
|
from src.application.ports.point_repository import PointRepository
|
||||||
|
from src.bootstrap.dependencies import get_point_repository
|
||||||
|
|
||||||
|
router = APIRouter(prefix="/points", tags=["points"])
|
||||||
|
|
||||||
|
_RequirePointsRead = Annotated[AuthContext, Depends(require_scope("points:read"))]
|
||||||
|
_PointRepositoryDep = Annotated[PointRepository, Depends(get_point_repository)]
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/count")
|
||||||
|
async def count_tenant_points(
|
||||||
|
auth: _RequirePointsRead,
|
||||||
|
repository: _PointRepositoryDep,
|
||||||
|
domain: str | None = None,
|
||||||
|
file_id: uuid.UUID | None = None,
|
||||||
|
include_inactive: bool = False,
|
||||||
|
) -> PointCountResponse:
|
||||||
|
count = await count_points(
|
||||||
|
repository,
|
||||||
|
tenant_id=auth.tenant_id,
|
||||||
|
domain=domain,
|
||||||
|
file_id=file_id,
|
||||||
|
include_inactive=include_inactive,
|
||||||
|
)
|
||||||
|
return PointCountResponse(count=count)
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/search")
|
||||||
|
async def search_tenant_points(
|
||||||
|
auth: _RequirePointsRead,
|
||||||
|
repository: _PointRepositoryDep,
|
||||||
|
q: Annotated[str, Query(min_length=1)],
|
||||||
|
limit: LimitQuery = DEFAULT_PAGE_SIZE,
|
||||||
|
cursor: str | None = None,
|
||||||
|
domain: str | None = None,
|
||||||
|
file_id: uuid.UUID | None = None,
|
||||||
|
include_inactive: bool = False,
|
||||||
|
) -> PointSearchResponse:
|
||||||
|
"""Keyword search over point content — **not** semantic retrieval.
|
||||||
|
|
||||||
|
Matches Qdrant's full-text payload index on `content`, combined with the
|
||||||
|
structured filters below. Results are unranked: the index filters rather
|
||||||
|
than scores, so there is no relevance order and no score to return. Callers
|
||||||
|
wanting ranked answers want the agent retrieval path (plan 003), not this.
|
||||||
|
"""
|
||||||
|
page = await search_points(
|
||||||
|
repository,
|
||||||
|
tenant_id=auth.tenant_id,
|
||||||
|
query=q,
|
||||||
|
limit=limit,
|
||||||
|
cursor=cursor,
|
||||||
|
domain=domain,
|
||||||
|
file_id=file_id,
|
||||||
|
include_inactive=include_inactive,
|
||||||
|
)
|
||||||
|
return PointSearchResponse.from_search(page, query=q)
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("")
|
||||||
|
async def list_tenant_points(
|
||||||
|
auth: _RequirePointsRead,
|
||||||
|
repository: _PointRepositoryDep,
|
||||||
|
file_id: uuid.UUID,
|
||||||
|
limit: LimitQuery = DEFAULT_PAGE_SIZE,
|
||||||
|
cursor: str | None = None,
|
||||||
|
include_inactive: bool = False,
|
||||||
|
) -> PointListResponse:
|
||||||
|
"""A file's points in `order_id` order.
|
||||||
|
|
||||||
|
`file_id` is required rather than optional: the pagination cursor is an
|
||||||
|
`order_id` value, and `order_id` is only unique within one file. Listing
|
||||||
|
across files would silently drop or repeat rows at every page boundary.
|
||||||
|
"""
|
||||||
|
page = await list_file_points(
|
||||||
|
repository,
|
||||||
|
tenant_id=auth.tenant_id,
|
||||||
|
file_id=file_id,
|
||||||
|
limit=limit,
|
||||||
|
cursor=cursor,
|
||||||
|
include_inactive=include_inactive,
|
||||||
|
)
|
||||||
|
return PointListResponse.from_page(page)
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/{point_id}")
|
||||||
|
async def get_tenant_point(
|
||||||
|
point_id: uuid.UUID,
|
||||||
|
auth: _RequirePointsRead,
|
||||||
|
repository: _PointRepositoryDep,
|
||||||
|
with_vectors: bool = False,
|
||||||
|
) -> PointResponse:
|
||||||
|
point = await get_point(
|
||||||
|
repository, tenant_id=auth.tenant_id, point_id=point_id, with_vectors=with_vectors
|
||||||
|
)
|
||||||
|
return PointResponse.from_point(point)
|
||||||
@@ -13,12 +13,24 @@ The shape callers see, kept separate from `application/points`' domain models
|
|||||||
|
|
||||||
import uuid
|
import uuid
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
|
from typing import Annotated
|
||||||
|
|
||||||
|
from fastapi import Query
|
||||||
from pydantic import BaseModel, ConfigDict, Field
|
from pydantic import BaseModel, ConfigDict, Field
|
||||||
|
|
||||||
from src.application.points.point import Point
|
from src.application.points.point import Point
|
||||||
from src.application.ports.point_repository import PointPage
|
from src.application.ports.point_repository import PointPage
|
||||||
|
|
||||||
|
# A page ceiling the caller cannot raise. Scroll pages are materialized in
|
||||||
|
# memory both here and in Qdrant, so an unbounded `limit` is a cheap way for one
|
||||||
|
# request to hurt every other tenant sharing the process. Declared once because
|
||||||
|
# two routers paginate points -- `/v1/points` and `/v1/files/{file_id}/points` --
|
||||||
|
# and a ceiling that differs between them is a ceiling in only one of them.
|
||||||
|
DEFAULT_PAGE_SIZE = 50
|
||||||
|
MAX_PAGE_SIZE = 200
|
||||||
|
|
||||||
|
LimitQuery = Annotated[int, Query(ge=1, le=MAX_PAGE_SIZE)]
|
||||||
|
|
||||||
|
|
||||||
class PointResponse(BaseModel):
|
class PointResponse(BaseModel):
|
||||||
point_id: uuid.UUID
|
point_id: uuid.UUID
|
||||||
@@ -91,6 +103,14 @@ class PointSearchResponse(PointListResponse):
|
|||||||
|
|
||||||
query: str
|
query: str
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def from_search(cls, page: PointPage, *, query: str) -> "PointSearchResponse":
|
||||||
|
return cls(
|
||||||
|
query=query,
|
||||||
|
points=[PointResponse.from_point(point) for point in page.points],
|
||||||
|
next_cursor=page.next_cursor,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class PointCreateRequest(BaseModel):
|
class PointCreateRequest(BaseModel):
|
||||||
"""Create one point. The server assigns identity, ordering, and provenance.
|
"""Create one point. The server assigns identity, ordering, and provenance.
|
||||||
|
|||||||
115
src/application/points/queries.py
Normal file
115
src/application/points/queries.py
Normal file
@@ -0,0 +1,115 @@
|
|||||||
|
"""Read paths for `/v1/points` (ADR-0002, ADR-0008).
|
||||||
|
|
||||||
|
The caller-facing entry points for point reads. Routers call these; they never
|
||||||
|
touch the `PointRepository` directly, and never build a filter.
|
||||||
|
|
||||||
|
This module is thin on purpose but not empty, and the two things it does own are
|
||||||
|
exactly the ones a route would otherwise get wrong:
|
||||||
|
|
||||||
|
- **`tenant_id` always comes from the caller's `AuthContext`.** Every function
|
||||||
|
takes it as a required keyword and hands it to the repository. Nothing here
|
||||||
|
reads a tenant from a query string or body.
|
||||||
|
- **A keyword query is Persian-normalized before it reaches the index.**
|
||||||
|
Ingestion letter-folds chunk content (`normalize_persian_text`, ADR-0018), so
|
||||||
|
stored text contains Persian yeh/keheh. A query typed on an Arabic keyboard
|
||||||
|
carries U+064A/U+0643 and would match nothing at all — a silent empty result,
|
||||||
|
not an error. Folding the query the same way is what makes the two comparable.
|
||||||
|
|
||||||
|
Reads emit no log events. The request middleware already records every call, and
|
||||||
|
ADR-0011 reserves `INFO` for lifecycle events rather than per-read volume;
|
||||||
|
mutations get their own events when those paths land.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import uuid
|
||||||
|
|
||||||
|
from src.application.ingestion.normalization import normalize_persian_text
|
||||||
|
from src.application.points.point import Point, PointNotFoundError
|
||||||
|
from src.application.ports.point_repository import PointPage, PointRepository
|
||||||
|
|
||||||
|
|
||||||
|
async def get_point(
|
||||||
|
repository: PointRepository,
|
||||||
|
*,
|
||||||
|
tenant_id: uuid.UUID,
|
||||||
|
point_id: uuid.UUID,
|
||||||
|
with_vectors: bool = False,
|
||||||
|
) -> Point:
|
||||||
|
"""One point, or `PointNotFoundError` if it is not this tenant's.
|
||||||
|
|
||||||
|
Raises rather than returning `None` so a route cannot forget the check and
|
||||||
|
serve `200 null`. "Absent" and "another tenant's" are the same outcome by
|
||||||
|
design (ADR-0016: cross-tenant access is `404`, never `403`).
|
||||||
|
"""
|
||||||
|
point = await repository.get(tenant_id=tenant_id, point_id=point_id, with_vectors=with_vectors)
|
||||||
|
if point is None:
|
||||||
|
raise PointNotFoundError(f"point {point_id} not found")
|
||||||
|
return point
|
||||||
|
|
||||||
|
|
||||||
|
async def list_file_points(
|
||||||
|
repository: PointRepository,
|
||||||
|
*,
|
||||||
|
tenant_id: uuid.UUID,
|
||||||
|
file_id: uuid.UUID,
|
||||||
|
limit: int,
|
||||||
|
cursor: str | None = None,
|
||||||
|
include_inactive: bool = False,
|
||||||
|
) -> PointPage:
|
||||||
|
"""One file's points in display (`order_id`) order.
|
||||||
|
|
||||||
|
An unknown or foreign `file_id` yields an empty page rather than an error:
|
||||||
|
the two are indistinguishable to the caller, which is the same
|
||||||
|
non-disclosure property `get_point` gets from raising.
|
||||||
|
"""
|
||||||
|
return await repository.list_by_file(
|
||||||
|
tenant_id=tenant_id,
|
||||||
|
file_id=file_id,
|
||||||
|
limit=limit,
|
||||||
|
cursor=cursor,
|
||||||
|
include_inactive=include_inactive,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def count_points(
|
||||||
|
repository: PointRepository,
|
||||||
|
*,
|
||||||
|
tenant_id: uuid.UUID,
|
||||||
|
domain: str | None = None,
|
||||||
|
file_id: uuid.UUID | None = None,
|
||||||
|
include_inactive: bool = False,
|
||||||
|
) -> int:
|
||||||
|
return await repository.count(
|
||||||
|
tenant_id=tenant_id,
|
||||||
|
domain=domain,
|
||||||
|
file_id=file_id,
|
||||||
|
include_inactive=include_inactive,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def search_points(
|
||||||
|
repository: PointRepository,
|
||||||
|
*,
|
||||||
|
tenant_id: uuid.UUID,
|
||||||
|
query: str,
|
||||||
|
limit: int,
|
||||||
|
cursor: str | None = None,
|
||||||
|
domain: str | None = None,
|
||||||
|
file_id: uuid.UUID | None = None,
|
||||||
|
include_inactive: bool = False,
|
||||||
|
) -> PointPage:
|
||||||
|
"""Keyword match on `content`, within this tenant.
|
||||||
|
|
||||||
|
**Not semantic retrieval.** Qdrant's full-text index filters rather than
|
||||||
|
scores, so results carry no relevance ranking and their order is
|
||||||
|
unspecified. Ranked retrieval is ADR-0003's hybrid path in plan 003; this
|
||||||
|
function must not grow a semantic mode (ADR-0002).
|
||||||
|
"""
|
||||||
|
return await repository.keyword_search(
|
||||||
|
tenant_id=tenant_id,
|
||||||
|
query=normalize_persian_text(query),
|
||||||
|
limit=limit,
|
||||||
|
cursor=cursor,
|
||||||
|
domain=domain,
|
||||||
|
file_id=file_id,
|
||||||
|
include_inactive=include_inactive,
|
||||||
|
)
|
||||||
Reference in New Issue
Block a user