Compare commits
10 Commits
9a4b173b95
...
012b44d5f2
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
012b44d5f2 | ||
|
|
ac779dec7e | ||
|
|
9e8987968c | ||
|
|
e9e83b3a26 | ||
|
|
fa933b08ff | ||
|
|
cc915f0f1a | ||
|
|
d00d436e5c | ||
|
|
58ca6109d1 | ||
|
|
5e0addcc55 | ||
|
|
e8fb41af87 |
11
.env.example
11
.env.example
@@ -10,10 +10,18 @@
|
||||
# Application
|
||||
APP_ENV=local
|
||||
APP_READINESS_CHECK_TIMEOUT_SECONDS=2.0
|
||||
# Set by CI/CD at build/deploy time; never computed at runtime.
|
||||
APP_SERVICE_VERSION=dev
|
||||
|
||||
# Logging
|
||||
LOG_LEVEL=INFO
|
||||
LOG_JSON_FORMAT=false
|
||||
# Optional second sink, always JSON regardless of LOG_JSON_FORMAT. Local dev
|
||||
# only -- leave unset in production, where stdout/stderr collection is
|
||||
# preferred over an in-container log file.
|
||||
# LOG_FILE_PATH=logs/app.log
|
||||
LOG_FILE_MAX_BYTES=10485760
|
||||
LOG_FILE_BACKUP_COUNT=5
|
||||
|
||||
# Postgres (application database, separate from Langfuse's Postgres)
|
||||
# Use 127.0.0.1 rather than localhost: some environments resolve localhost to
|
||||
@@ -44,6 +52,9 @@ INGESTION_EMBED_CONCURRENCY=4
|
||||
# Qdrant
|
||||
QDRANT_URL=http://127.0.0.1:6343
|
||||
QDRANT_API_KEY=
|
||||
QDRANT_COLLECTION=chunks
|
||||
QDRANT_UPSERT_BATCH_SIZE=128
|
||||
QDRANT_UPSERT_CONCURRENCY=4
|
||||
|
||||
# Dense embedders (ADR-0001). Both speak an OpenAI-compatible /embeddings
|
||||
# endpoint, so one adapter serves both. Models and endpoints are the ones the
|
||||
|
||||
50
CLAUDE.md
50
CLAUDE.md
@@ -18,10 +18,18 @@ the chunk-count ceiling (`413`). The embedding configuration is **ported from
|
||||
the `emet` evaluation lab** (`~/code/talie/emet`), which benchmarked these
|
||||
models and analyzers on the real Farsi corpus — the analyzer and BM25 weights
|
||||
are verified token-for-token against it, so treat them as a measured artifact
|
||||
and re-benchmark rather than tune them in place (ADR-0005). Not built yet: Qdrant collection bootstrap,
|
||||
Qdrant point upserts (so uploaded chunks are parsed/embedded but not yet
|
||||
searchable), and `src/agent/`. That maps to plan 001 Phases 1-4 done, Phase 5
|
||||
not started.
|
||||
and re-benchmark rather than tune them in place (ADR-0005). Also working: the
|
||||
`chunks` collection bootstrap (`src/infrastructure/qdrant/collection.py`, run as
|
||||
a deployment step via `uv run python -m src.cli.qdrant_bootstrap` — never at
|
||||
startup) and tenant-scoped point upserts (`src/application/points/` behind the
|
||||
`PointStorage` port), so an upload is searchable by the time `201` returns.
|
||||
Also working: `tenant_domains` plus `/v1/domains` (`src/application/domains/`),
|
||||
a strict per-tenant allowlist — `POST /v1/files` rejects an unregistered or
|
||||
disabled `domain` with `400` before anything is written, and domain management
|
||||
sits behind its own `domains:read`/`domains:write` scopes, never `files:write`.
|
||||
Not built yet: `/v1/points` CRUD and keyword search (plan 002), and
|
||||
`src/agent/`. That maps to plan 001 Phases 1-5 done, Phase 6 (runbook, e2e
|
||||
tests, Compose smoke test) not started.
|
||||
|
||||
Architecture decisions live in `docs/adr/` (18 ADRs plus the 0000 template;
|
||||
0001–0004 are `Accepted` — 0004 amended by 0018; 0014 is `Superseded by 0017`;
|
||||
@@ -140,9 +148,13 @@ as its only caller-facing entry point. It dispatches on source type and owns
|
||||
the `anyio.to_thread.run_sync` + `CapacityLimiter` offload ADR-0017 requires;
|
||||
`parse_docx`/`parse_csv`/`parse_xlsx`/`chunk_document` stay in the package,
|
||||
exported mainly for their own tests, not for outside callers to reach for
|
||||
directly. Follow this pattern in `application/` as new packages are added
|
||||
there — `points/`, `retrieval/`, `threads/` — rather than exposing their
|
||||
internals as the primary surface.
|
||||
directly. `src/application/points/` follows the same shape: `index_chunks` is
|
||||
the only caller-facing entry point, owning payload construction, batching,
|
||||
the `upsert_concurrency` semaphore, and the ordering rule that the soft-delete
|
||||
sweep runs only after every upsert succeeds; `build_chunk_payload` stays
|
||||
internal. Follow this pattern in `application/` as new packages are added
|
||||
there — `retrieval/`, `threads/` — rather than exposing their internals as the
|
||||
primary surface.
|
||||
|
||||
### Resource lifetime rules (ADR-0012)
|
||||
|
||||
@@ -221,6 +233,11 @@ content_sha256)` idempotency, no terminal job returning to `running`.
|
||||
|
||||
### Postgres conventions (ADR-0009)
|
||||
|
||||
`domain` is never free-form: it must match an `active` `tenant_domains` row for
|
||||
the authenticated tenant (ADR-0009). Domain sets are per-tenant and vary in
|
||||
size. The key itself is immutable — it is denormalized into every Qdrant point
|
||||
payload and into `source_files`, so renaming it is a migration, not an edit.
|
||||
|
||||
UUID primary keys (app-generated), `timestamptz` for all timestamps,
|
||||
`Numeric(18, 8)` for money (never floats), `JSONB` for flexible metadata but
|
||||
typed/indexed columns for query-critical fields, string status columns with
|
||||
@@ -237,7 +254,24 @@ Postgres remains system of record for tenants, API keys, audit, jobs,
|
||||
`graph_runs`, `llm_calls`/`llm_pricing`. Correlate the two via `request_id`,
|
||||
`tenant_id`, `thread_id`, `run_id`. Use `structlog` with stable event names
|
||||
and structured fields (`logger.info("graph.run.completed", ...)`), not
|
||||
interpolated prose; JSON logs by default in production.
|
||||
interpolated prose; JSON logs by default in production, plus an optional
|
||||
local-only JSON file sink independent of the console renderer (`LOG_FILE_PATH`).
|
||||
|
||||
**Add logging in the same change that adds the code, not as a follow-up.**
|
||||
When you add a new service-level entry point (an `application/` function a
|
||||
route calls directly, an ingestion phase, a mutation) or a new failure branch
|
||||
inside one, add its `logger.*` event in that same diff, using ADR-0011's
|
||||
level/event-naming table. Deferring it means re-deriving the failure modes and
|
||||
field names later from code that no longer has them in working memory — as
|
||||
happened with `src/application/files/upload.py`, where four failure branches
|
||||
(`parse_failed`, `chunk_limit_exceeded`, `embedding_failed`, `index_failed`)
|
||||
shipped with no log event and had to be retrofitted.
|
||||
|
||||
This does not mean logging every function. Pure functions, models, schemas,
|
||||
and repositories (`infrastructure/postgres/repositories/`) stay silent by
|
||||
convention — the caller that turns their result into a business-meaningful
|
||||
outcome (job succeeded, upload rejected, domain disabled) is where the event
|
||||
belongs, not the row-level function underneath it.
|
||||
|
||||
## Testing (ADR-0016)
|
||||
|
||||
|
||||
28
README.md
28
README.md
@@ -3,6 +3,34 @@
|
||||
Architecture decisions live in [`docs/adr`](docs/adr). The first implementation
|
||||
milestone is documented in the [ingestion vertical-slice plan](docs/plans/001-ingestion-vertical-slice.md).
|
||||
|
||||
## Provisioning the datastores
|
||||
|
||||
Both schema steps run as explicit deployment steps. The application performs no
|
||||
DDL at startup — not for Postgres (ADR-0009) and not for Qdrant (ADR-0001,
|
||||
"Collection provisioning").
|
||||
|
||||
```bash
|
||||
docker compose up -d # Postgres, MinIO, Qdrant
|
||||
uv run alembic upgrade head # Postgres schema
|
||||
uv run python -m src.cli.qdrant_bootstrap # the `chunks` collection
|
||||
uv run fastapi dev src/main.py
|
||||
```
|
||||
|
||||
Before a tenant can upload, its domains must be registered — `POST /v1/files`
|
||||
rejects an unregistered or disabled `domain` with `400`. The calling backend
|
||||
manages them over `/v1/domains` using a key with the `domains:write` scope:
|
||||
|
||||
```bash
|
||||
curl -X POST http://localhost:8000/v1/domains \
|
||||
-H "Authorization: Bearer $API_KEY" \
|
||||
-H 'Content-Type: application/json' \
|
||||
-d '{"domain": "fire", "display_name": "Fire insurance"}'
|
||||
```
|
||||
|
||||
Both bootstrap commands are idempotent and safe to re-run. `qdrant_bootstrap` verifies an
|
||||
existing collection against the pinned schema and exits non-zero on a mismatch,
|
||||
rather than leaving a silently degraded sparse index in place.
|
||||
|
||||
## Local Langfuse
|
||||
|
||||
This repo includes a root-level development Compose file for Langfuse:
|
||||
|
||||
48
alembic/versions/41335d162de8_create_tenant_domains.py
Normal file
48
alembic/versions/41335d162de8_create_tenant_domains.py
Normal file
@@ -0,0 +1,48 @@
|
||||
"""create tenant_domains
|
||||
|
||||
Revision ID: 41335d162de8
|
||||
Revises: bfc6c81c2542
|
||||
Create Date: 2026-08-20 17:48:29.443293
|
||||
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
from sqlalchemy.dialects import postgresql
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision: str = '41335d162de8'
|
||||
down_revision: Union[str, Sequence[str], None] = 'bfc6c81c2542'
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
"""Upgrade schema."""
|
||||
# ### commands auto generated by Alembic - please adjust! ###
|
||||
op.create_table('tenant_domains',
|
||||
sa.Column('id', sa.Uuid(), nullable=False),
|
||||
sa.Column('tenant_id', sa.Uuid(), nullable=False),
|
||||
sa.Column('domain', sa.String(length=80), nullable=False),
|
||||
sa.Column('display_name', sa.String(length=200), nullable=False),
|
||||
sa.Column('status', sa.String(length=20), server_default='active', nullable=False),
|
||||
sa.Column('metadata', postgresql.JSONB(astext_type=sa.Text()), server_default='{}', nullable=False),
|
||||
sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
|
||||
sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
|
||||
sa.Column('disabled_at', sa.DateTime(timezone=True), nullable=True),
|
||||
sa.CheckConstraint("status IN ('active', 'disabled')", name='ck_tenant_domains_status'),
|
||||
sa.ForeignKeyConstraint(['tenant_id'], ['tenants.id'], ondelete='CASCADE'),
|
||||
sa.PrimaryKeyConstraint('id'),
|
||||
sa.UniqueConstraint('tenant_id', 'domain', name='uq_tenant_domains_tenant_id_domain')
|
||||
)
|
||||
op.create_index(op.f('ix_tenant_domains_tenant_id'), 'tenant_domains', ['tenant_id'], unique=False)
|
||||
# ### end Alembic commands ###
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
"""Downgrade schema."""
|
||||
# ### commands auto generated by Alembic - please adjust! ###
|
||||
op.drop_index(op.f('ix_tenant_domains_tenant_id'), table_name='tenant_domains')
|
||||
op.drop_table('tenant_domains')
|
||||
# ### end Alembic commands ###
|
||||
@@ -108,6 +108,38 @@ measurement rather than assumption:
|
||||
- Payload index on `previous_chunk_id` / `next_chunk_id`: keyword index,
|
||||
used for O(1) adjacency retrieval (see below).
|
||||
|
||||
### Collection provisioning
|
||||
|
||||
The collection is created by an explicit **deployment step**, not by application
|
||||
startup and not lazily on first write:
|
||||
|
||||
uv run python -m src.cli.qdrant_bootstrap
|
||||
|
||||
Creating a collection is DDL, and this project already keeps DDL out of the boot
|
||||
and request paths: [0009](0009-postgres-sqlalchemy-alembic-schema.md) requires
|
||||
Alembic for Postgres schema and forbids `create_all()` at startup, and
|
||||
[0012](0012-application-resource-lifetime-and-dependency-ownership.md) makes
|
||||
LangGraph's `.setup()` a deployment step for the same reason. Neither ADR named
|
||||
Qdrant explicitly; this section closes that gap rather than letting the placement
|
||||
be decided by whichever code happened to need it first.
|
||||
|
||||
Doing it in the FastAPI lifespan was rejected: it couples process boot to Qdrant
|
||||
being reachable (which is `/readyz`'s job, not boot's), races across replicas,
|
||||
and turns a misconfigured collection into a silent skip. Doing it lazily on first
|
||||
upsert was rejected for putting DDL on a user request and hiding the
|
||||
misconfiguration until traffic arrives.
|
||||
|
||||
`ensure_chunks_collection` is idempotent and **verifying**: against an existing
|
||||
collection it compares the dense dimensions and the sparse `modifier` to the
|
||||
pinned values and fails loudly on divergence. That check is the point of making
|
||||
the step explicit — both properties degrade silently in production if wrong (a
|
||||
missing `modifier="idf"` produces no error, just unweighted lexical retrieval).
|
||||
|
||||
Payload indexes are (re)created on every run, since unlike vector configuration
|
||||
they can be added to a live collection. The full-text index on `content` is
|
||||
therefore deferred to the keyword-search work in
|
||||
[0002](0002-chunk-crud-and-search-api.md), not created here.
|
||||
|
||||
### Payload schema
|
||||
|
||||
This schema is now decided for the fields below. Additional document-context
|
||||
@@ -124,7 +156,7 @@ involves format-specific tradeoffs not yet made.
|
||||
| `chunk_id` | keyword | stable identifier for a single chunk |
|
||||
| `content_type` | keyword | classification of the chunk's content; exact value set (e.g. `paragraph`, `table_row`, `heading`) to be finalized alongside the chunking-strategy ADR |
|
||||
| `source_filename` | keyword | original uploaded filename |
|
||||
| `source_type` | keyword (`docx` \| `csv`) | which parser produced this chunk |
|
||||
| `source_type` | keyword (`docx` \| `xlsx` \| `csv`) | which parser produced this chunk — `xlsx` added by [0018](0018-docx-and-spreadsheet-parsing-with-fixed-size-chunking.md) |
|
||||
| `order_id` | float (see below) | chunk's *display* position within the file; mutable so the backend can reorder/insert chunks |
|
||||
| `chunk_index` | integer | chunk's *original ingestion* ordinal — immutable, used to derive the deterministic point ID below (kept separate from `order_id` precisely because `order_id` can change) |
|
||||
| `previous_chunk_id` | keyword, nullable | `chunk_id` of the preceding chunk in display order (`null` for the first chunk in a file) — O(1) adjacency pointer for context-window expansion in ADR-0003 |
|
||||
@@ -136,7 +168,7 @@ involves format-specific tradeoffs not yet made.
|
||||
| `updated_at` | datetime | last modification timestamp |
|
||||
| `created_by` | keyword | user/service that created the chunk |
|
||||
| `updated_by` | keyword | user/service that last modified the chunk |
|
||||
| `version` | integer | optimistic-concurrency counter, used in ADR-0002 |
|
||||
| `version` | integer | optimistic-concurrency counter, used in ADR-0002. Ingestion currently writes `1` unconditionally: the read-check-write that makes the guard meaningful costs one read per point and belongs with the `/v1/points` write paths, so plan 002 owns it. Safe while ingestion is the only writer of a file's points; it would clobber a concurrent manual edit's counter once `/v1/points` ships. |
|
||||
| `content_hash` | keyword | hash of the chunk's raw text; lets re-ingestion detect unchanged content and skip re-embedding it |
|
||||
| `embedding_model_version` | keyword | identifies which embedding model(s) produced this chunk's vectors; needed to know which chunks require re-embedding after a future model swap |
|
||||
|
||||
|
||||
@@ -98,13 +98,16 @@ One row per customer/tenant.
|
||||
| `slug` | Stable short name, unique, human-readable. |
|
||||
| `name` | Display name. |
|
||||
| `status` | `active` \| `suspended` \| `deleted`. Suspended tenants authenticate to a clear error but cannot run work. |
|
||||
| `settings` | JSONB for tenant-level feature flags/limits (max upload size, enabled file types, allowed domains, etc.). |
|
||||
| `settings` | JSONB for tenant-level feature flags/limits (max upload size, enabled file types, etc.). Allowed domains were previously listed here as well; they live in `tenant_domains` instead, per this ADR's own rule that query-critical fields get typed columns — `domain` is validated on every upload and filtered on every query. |
|
||||
| `created_at`, `updated_at`, `deleted_at` | Audit/soft-delete timestamps. |
|
||||
|
||||
#### `tenant_domains`
|
||||
|
||||
Optional but recommended. Validates the `domain` values used throughout Qdrant
|
||||
payloads (`car`, `fire`, etc.) per tenant.
|
||||
**Required.** (Previously "optional but recommended"; implemented and made
|
||||
mandatory alongside `/v1/domains`.) Validates the `domain` values used
|
||||
throughout Qdrant payloads (`car`, `fire`, etc.) per tenant. Domain sets are
|
||||
per-tenant and differ in size — one tenant may run 14 insurance lines and
|
||||
another 6 — so this is data, not an enum.
|
||||
|
||||
| Column | Notes |
|
||||
|---|---|
|
||||
@@ -116,7 +119,37 @@ payloads (`car`, `fire`, etc.) per tenant.
|
||||
| `metadata` | JSONB for domain-specific ingestion/retrieval settings. |
|
||||
|
||||
This prevents arbitrary caller-supplied domains from silently creating new
|
||||
partitions in Qdrant.
|
||||
partitions in Qdrant. The failure it guards against is quiet: a typo such as
|
||||
`fier` for `fire` produces no error anywhere — the file is stored, parsed,
|
||||
embedded, and indexed into a partition retrieval never queries, so it is
|
||||
invisible rather than failed.
|
||||
|
||||
##### Enforcement and management
|
||||
|
||||
- **Strict allowlist.** `POST /v1/files` rejects a domain with no `active` row
|
||||
for the tenant (`400`, error code `unknown_domain`). There is no auto-create
|
||||
on first use: that would record the typo rather than prevent it. The check
|
||||
runs inside the upload's first transaction, before any MinIO object, job row,
|
||||
or Qdrant point is written.
|
||||
- **Managed over the API, not by an operator.** `/v1/domains` (list, create,
|
||||
update, disable, enable) is the surface the calling backend uses. Domains are
|
||||
created by an explicit, scoped call rather than as a side effect of an upload
|
||||
— that distinction, not who makes the call, is what "strict" means here.
|
||||
- **Its own scope.** `domains:read`/`domains:write`, deliberately separate from
|
||||
`files:write`. Folding domain creation into the upload scope would let an
|
||||
upload key create partitions again, which is the exact hole this closes.
|
||||
`api_keys.scopes` is already a free JSONB list, so this needs no schema change.
|
||||
- **`tenant_id` stays derived from the API key.** One key per tenant; nothing
|
||||
request-suppliable. A platform key acting across tenants would need a real
|
||||
actor model and is not adopted.
|
||||
- **`domain` is immutable; `display_name` is not.** The key is denormalized into
|
||||
every Qdrant point payload and into `source_files`, so renaming it means
|
||||
rewriting all of them — a migration, not a `PATCH`. The update schema
|
||||
therefore has no `domain` field.
|
||||
- **Disable is not delete.** `status='disabled'` blocks new uploads and hides
|
||||
the domain from listings, leaving already-indexed points intact and
|
||||
retrievable. Actual removal needs the retention/erasure workflow this ADR and
|
||||
plan 001 defer.
|
||||
|
||||
#### `api_keys`
|
||||
|
||||
|
||||
@@ -70,17 +70,30 @@ logger.info(
|
||||
Do not build log messages by interpolating operational metadata into prose.
|
||||
Prefer fields over long strings because fields are queryable.
|
||||
|
||||
### Emit JSON logs by default in production
|
||||
### Emit JSON logs by default in production; console and file are independent sinks locally
|
||||
|
||||
Production logs are JSON on stdout so process managers, container runtimes, and
|
||||
log collectors can ingest them directly. Local development may use a colored
|
||||
console renderer controlled by configuration.
|
||||
log collectors can ingest them directly. This does not change.
|
||||
|
||||
File logging is optional and mainly for local development. If enabled, it must
|
||||
use explicit rotation settings such as `maxBytes` and `backupCount`. Do not rely
|
||||
on a default `RotatingFileHandler` with no rotation parameters. In containerized
|
||||
production, stdout/stderr collection is preferred over writing `logs/app.log`
|
||||
inside the application container.
|
||||
Locally, stdout and an optional file are two **independent, simultaneous**
|
||||
handlers on the same logger, not a single renderer chosen by a flag — the same
|
||||
structlog event fans out to both:
|
||||
|
||||
- **Console handler**: always on, `structlog.dev.ConsoleRenderer(colors=True)`.
|
||||
This is what a developer reads while the process runs, so it stays
|
||||
human-readable regardless of whether file logging is also enabled.
|
||||
- **File handler**: off by default, enabled by setting `LOG_FILE_PATH`. Always
|
||||
renders JSON (`structlog.processors.JSONRenderer()`), independent of the
|
||||
console handler's renderer, so a saved log is machine-parseable even though
|
||||
the terminal output next to it is not. Must use explicit rotation
|
||||
(`RotatingFileHandler` with `maxBytes`/`backupCount` — never an unrotated
|
||||
handler).
|
||||
|
||||
In containerized production, stdout/stderr collection remains preferred over
|
||||
writing `logs/app.log` inside the application container, so `LOG_FILE_PATH` is
|
||||
expected to be unset there; the file handler exists for local development,
|
||||
where reading a colored terminal *and* keeping a JSON trail to grep/parse later
|
||||
are both useful at once.
|
||||
|
||||
### Configure stdlib and structlog together
|
||||
|
||||
@@ -188,6 +201,36 @@ Notes:
|
||||
- `structlog.contextvars.merge_contextvars` ensures request-bound fields appear
|
||||
on both structlog and stdlib logs processed through the formatter.
|
||||
|
||||
### Bind process-level environment context once at startup
|
||||
|
||||
Deployment identity — which build is running, in which environment, on which
|
||||
instance — answers a different question than request correlation: "is this
|
||||
issue specific to one deployment / one region / one instance?" rather than "is
|
||||
this issue specific to one request?" It does not vary per request, so it must
|
||||
not go through `structlog.contextvars`, which `RequestIdMiddleware` clears on
|
||||
every request; a value bound there before the first request would be wiped the
|
||||
moment that middleware runs.
|
||||
|
||||
Instead, add a static structlog **processor** — a plain closure over values read
|
||||
once at `configure_logging()` time — so it runs on every event regardless of
|
||||
request context:
|
||||
|
||||
```python
|
||||
def _bind_environment(settings: AppLimitSettings):
|
||||
def processor(logger, method_name, event_dict):
|
||||
event_dict["env"] = settings.env
|
||||
event_dict["service_version"] = settings.service_version
|
||||
return event_dict
|
||||
|
||||
return processor
|
||||
```
|
||||
|
||||
`service_version` should be the deployed commit SHA or release tag (e.g. from a
|
||||
`GIT_SHA`/`APP_VERSION` build-time env var — not computed at runtime by
|
||||
shelling out to `git`). This makes "is this only happening on the new
|
||||
deployment?" answerable directly from logs, without cross-referencing a
|
||||
separate deployment record.
|
||||
|
||||
### Bind request context with contextvars
|
||||
|
||||
At FastAPI ingress, clear stale context, bind request identifiers, and return the
|
||||
|
||||
@@ -159,8 +159,25 @@ retry, and phase 2 has no transaction protecting it:
|
||||
return the existing file/job rather than re-ingesting (plan 001).
|
||||
- `tenant_id` comes from `AuthContext`, never from the request body.
|
||||
- A terminal job is never transitioned back to `running`.
|
||||
- Qdrant points from a failed attempt do not replace the previous successful
|
||||
index; replacement happens only after a successful attempt.
|
||||
- A failed attempt never *removes* content from a working index. The
|
||||
soft-delete sweep that retires a shortened file's leftover points runs only
|
||||
after every upsert in the attempt has succeeded.
|
||||
|
||||
This is deliberately weaker than "replacement happens only after a successful
|
||||
attempt", which an earlier revision of this ADR claimed. That guarantee is not
|
||||
achievable alongside ADR-0001's deterministic point ids: those ids are exactly
|
||||
what makes a retry idempotent, and they also mean a re-ingestion overwrites
|
||||
points **in place**, so a crash partway through leaves a prefix updated and the
|
||||
remainder still on the old content. Buying literal atomicity would mean
|
||||
generation-suffixed ids and an activation flip, which contradicts ADR-0001 and
|
||||
ADR-0002's stable point ids. Staging the new points as `is_active=false` and
|
||||
flipping them on success is strictly worse — the in-place overwrite would
|
||||
deactivate the previously live points, silently emptying a working index if the
|
||||
attempt were interrupted.
|
||||
|
||||
What holds instead: the index is never emptied, never partially deleted, and a
|
||||
retry converges — deterministic ids rewrite every point and the sweep re-runs,
|
||||
reaching the exact correct state.
|
||||
|
||||
### Failures are HTTP failures
|
||||
|
||||
|
||||
@@ -125,9 +125,12 @@ them:
|
||||
- Use `(tenant_id, domain, content_sha256)` to recognize identical uploads.
|
||||
- An identical active upload should return the existing source-file/job reference
|
||||
rather than create a duplicate ingestion.
|
||||
- A changed upload creates a new ingestion job. Existing active Qdrant points are
|
||||
replaced only after the new job completes successfully, so a failed re-ingestion
|
||||
does not remove a working index.
|
||||
- A changed upload creates a new ingestion job. A failed re-ingestion never
|
||||
removes a working index: the soft-delete sweep for a shortened file runs only
|
||||
after every upsert has succeeded. Because ADR-0001's point ids are
|
||||
deterministic, upserts overwrite in place, so an interrupted attempt can leave
|
||||
a prefix updated — it cannot empty or partially delete the index, and a retry
|
||||
converges. See ADR-0017, "Re-running an ingestion stays safe".
|
||||
- Preserve the original filename in Postgres metadata. MinIO object keys remain
|
||||
internal ID-based paths.
|
||||
|
||||
@@ -272,6 +275,11 @@ code and a terminal job row.
|
||||
tenant-filtered upserts, terminal state persistence, retrying an upload, and
|
||||
parser/Qdrant failure handling.
|
||||
|
||||
The `chunks` collection itself is provisioned by a deployment step —
|
||||
`uv run python -m src.cli.qdrant_bootstrap` — not by FastAPI startup, for the
|
||||
same reason ADR-0009 keeps Alembic out of startup and ADR-0012 makes LangGraph's
|
||||
`.setup()` a deployment step. See ADR-0001, "Collection provisioning".
|
||||
|
||||
**Exit criteria:** a successful upload returns `201` with a terminal status, and
|
||||
its points are retrievable only under the owning tenant's Qdrant filter. A forced
|
||||
failure mid-ingestion produces a `failed` job and the right HTTP status, and
|
||||
|
||||
@@ -37,6 +37,11 @@ dev = [
|
||||
testpaths = ["tests"]
|
||||
asyncio_mode = "strict"
|
||||
timeout = 10
|
||||
# Bound the test function only, not fixture setup. Testcontainers' container
|
||||
# startup is charged to whichever test first pulls a session-scoped container
|
||||
# fixture; on a cold Docker cache that is ~25s and would trip the 10s budget
|
||||
# for every integration test, regardless of how fast the test itself is.
|
||||
timeout_func_only = true
|
||||
markers = [
|
||||
"unit: fast tests with no external services",
|
||||
"integration: tests against a real disposable service",
|
||||
|
||||
@@ -16,6 +16,7 @@ from src.application.auth.errors import (
|
||||
MissingScopeError,
|
||||
TenantInactiveError,
|
||||
)
|
||||
from src.application.domains.errors import DomainAlreadyExistsError, UnknownDomainError
|
||||
from src.application.files.errors import FileTooLargeError, InvalidUploadError
|
||||
from src.application.ingestion.errors import (
|
||||
ChunkLimitExceededError,
|
||||
@@ -23,6 +24,7 @@ from src.application.ingestion.errors import (
|
||||
EmbedderError,
|
||||
IngestionAtCapacityError,
|
||||
IngestionTimeoutError,
|
||||
PointIndexingError,
|
||||
UnsupportedSourceTypeError,
|
||||
)
|
||||
|
||||
@@ -39,11 +41,14 @@ _MAPPING: tuple[tuple[type[Exception], int, str], ...] = (
|
||||
(TenantInactiveError, status.HTTP_401_UNAUTHORIZED, "tenant_not_found"),
|
||||
(MissingScopeError, status.HTTP_403_FORBIDDEN, "missing_scope"),
|
||||
(InvalidUploadError, status.HTTP_400_BAD_REQUEST, "validation_error"),
|
||||
(UnknownDomainError, status.HTTP_400_BAD_REQUEST, "unknown_domain"),
|
||||
(DomainAlreadyExistsError, status.HTTP_409_CONFLICT, "conflict"),
|
||||
(DocumentParseError, status.HTTP_400_BAD_REQUEST, "validation_error"),
|
||||
(UnsupportedSourceTypeError, status.HTTP_415_UNSUPPORTED_MEDIA_TYPE, "unsupported_media_type"),
|
||||
(FileTooLargeError, status.HTTP_413_CONTENT_TOO_LARGE, "payload_too_large"),
|
||||
(ChunkLimitExceededError, status.HTTP_413_CONTENT_TOO_LARGE, "payload_too_large"),
|
||||
(EmbedderError, status.HTTP_502_BAD_GATEWAY, "embedder_error"),
|
||||
(PointIndexingError, status.HTTP_502_BAD_GATEWAY, "index_error"),
|
||||
(IngestionTimeoutError, status.HTTP_504_GATEWAY_TIMEOUT, "ingestion_timeout"),
|
||||
)
|
||||
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
from fastapi import APIRouter
|
||||
|
||||
from src.api.routers.domains import router as domains_router
|
||||
from src.api.routers.files import router as files_router
|
||||
|
||||
router = APIRouter()
|
||||
router.include_router(domains_router)
|
||||
router.include_router(files_router)
|
||||
|
||||
110
src/api/routers/domains.py
Normal file
110
src/api/routers/domains.py
Normal file
@@ -0,0 +1,110 @@
|
||||
"""`/v1/domains` (ADR-0008, ADR-0009).
|
||||
|
||||
The management surface for a tenant's domain allowlist, used by the calling
|
||||
backend rather than by an operator with a psql prompt.
|
||||
|
||||
Gated on `domains:read`/`domains:write`, deliberately **not** on `files:write`:
|
||||
if an upload key could create domains, the allowlist would no longer prevent a
|
||||
typo'd `domain` from creating a Qdrant partition, which is its only purpose.
|
||||
"""
|
||||
|
||||
from typing import Annotated
|
||||
|
||||
from fastapi import APIRouter, Depends, status
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.api.dependencies.auth import require_scope
|
||||
from src.api.schemas.domains import (
|
||||
CreateDomainRequest,
|
||||
DomainListResponse,
|
||||
DomainResponse,
|
||||
UpdateDomainRequest,
|
||||
)
|
||||
from src.application.auth.context import AuthContext
|
||||
from src.application.domains import (
|
||||
create_domain,
|
||||
list_domains,
|
||||
set_domain_status,
|
||||
update_domain,
|
||||
)
|
||||
from src.bootstrap.dependencies import get_sessionmaker
|
||||
|
||||
router = APIRouter(prefix="/domains", tags=["domains"])
|
||||
|
||||
_RequireDomainsRead = Annotated[AuthContext, Depends(require_scope("domains:read"))]
|
||||
_RequireDomainsWrite = Annotated[AuthContext, Depends(require_scope("domains:write"))]
|
||||
_SessionmakerDep = Annotated[async_sessionmaker[AsyncSession], Depends(get_sessionmaker)]
|
||||
|
||||
|
||||
@router.get("")
|
||||
async def list_tenant_domains(
|
||||
auth: _RequireDomainsRead,
|
||||
sessionmaker: _SessionmakerDep,
|
||||
include_disabled: bool = False,
|
||||
) -> DomainListResponse:
|
||||
results = await list_domains(
|
||||
sessionmaker, tenant_id=auth.tenant_id, include_disabled=include_disabled
|
||||
)
|
||||
return DomainListResponse(domains=[DomainResponse.from_result(item) for item in results])
|
||||
|
||||
|
||||
@router.post("", status_code=status.HTTP_201_CREATED)
|
||||
async def create_tenant_domain(
|
||||
request: CreateDomainRequest,
|
||||
auth: _RequireDomainsWrite,
|
||||
sessionmaker: _SessionmakerDep,
|
||||
) -> DomainResponse:
|
||||
result = await create_domain(
|
||||
sessionmaker,
|
||||
tenant_id=auth.tenant_id,
|
||||
domain=request.domain,
|
||||
display_name=request.display_name,
|
||||
metadata=request.metadata,
|
||||
)
|
||||
return DomainResponse.from_result(result)
|
||||
|
||||
|
||||
@router.patch("/{domain}")
|
||||
async def update_tenant_domain(
|
||||
domain: str,
|
||||
request: UpdateDomainRequest,
|
||||
auth: _RequireDomainsWrite,
|
||||
sessionmaker: _SessionmakerDep,
|
||||
) -> DomainResponse:
|
||||
result = await update_domain(
|
||||
sessionmaker,
|
||||
tenant_id=auth.tenant_id,
|
||||
domain=domain,
|
||||
display_name=request.display_name,
|
||||
)
|
||||
return DomainResponse.from_result(result)
|
||||
|
||||
|
||||
@router.delete("/{domain}")
|
||||
async def disable_tenant_domain(
|
||||
domain: str,
|
||||
auth: _RequireDomainsWrite,
|
||||
sessionmaker: _SessionmakerDep,
|
||||
) -> DomainResponse:
|
||||
"""Disable, not delete.
|
||||
|
||||
Blocks new uploads and drops the domain from pickers while leaving the
|
||||
points already indexed under it intact and retrievable. Actually removing
|
||||
them needs the tenant-erasure workflow plan 001 defers.
|
||||
"""
|
||||
result = await set_domain_status(
|
||||
sessionmaker, tenant_id=auth.tenant_id, domain=domain, status="disabled"
|
||||
)
|
||||
return DomainResponse.from_result(result)
|
||||
|
||||
|
||||
@router.post("/{domain}/enable")
|
||||
async def enable_tenant_domain(
|
||||
domain: str,
|
||||
auth: _RequireDomainsWrite,
|
||||
sessionmaker: _SessionmakerDep,
|
||||
) -> DomainResponse:
|
||||
result = await set_domain_status(
|
||||
sessionmaker, tenant_id=auth.tenant_id, domain=domain, status="active"
|
||||
)
|
||||
return DomainResponse.from_result(result)
|
||||
@@ -19,11 +19,13 @@ from src.application.files.status import get_file_status
|
||||
from src.application.files.upload import upload_source_file
|
||||
from src.application.ports.embedding import DenseEmbedder, SparseEmbedder
|
||||
from src.application.ports.object_storage import ObjectStorage
|
||||
from src.application.ports.point_storage import PointStorage
|
||||
from src.bootstrap.dependencies import (
|
||||
get_dense_embedders,
|
||||
get_ingestion_concurrency_limiter,
|
||||
get_ingestion_limiter,
|
||||
get_object_storage,
|
||||
get_point_storage,
|
||||
get_sessionmaker,
|
||||
get_settings,
|
||||
get_sparse_embedder,
|
||||
@@ -35,6 +37,7 @@ router = APIRouter(prefix="/files", tags=["files"])
|
||||
_RequireFilesWrite = Annotated[AuthContext, Depends(require_scope("files:write"))]
|
||||
_SessionmakerDep = Annotated[async_sessionmaker[AsyncSession], Depends(get_sessionmaker)]
|
||||
_ObjectStorageDep = Annotated[ObjectStorage, Depends(get_object_storage)]
|
||||
_PointStorageDep = Annotated[PointStorage, Depends(get_point_storage)]
|
||||
_SettingsDep = Annotated[Settings, Depends(get_settings)]
|
||||
_IngestionLimiterDep = Annotated[CapacityLimiter, Depends(get_ingestion_limiter)]
|
||||
_ConcurrencyLimiterDep = Annotated[Semaphore, Depends(get_ingestion_concurrency_limiter)]
|
||||
@@ -50,6 +53,7 @@ async def upload_file(
|
||||
auth: _RequireFilesWrite,
|
||||
sessionmaker: _SessionmakerDep,
|
||||
storage: _ObjectStorageDep,
|
||||
point_storage: _PointStorageDep,
|
||||
settings: _SettingsDep,
|
||||
limiter: _IngestionLimiterDep,
|
||||
concurrency_limiter: _ConcurrencyLimiterDep,
|
||||
@@ -60,12 +64,14 @@ async def upload_file(
|
||||
result = await upload_source_file(
|
||||
sessionmaker=sessionmaker,
|
||||
storage=storage,
|
||||
point_storage=point_storage,
|
||||
auth=auth,
|
||||
domain=domain,
|
||||
filename=file.filename or "",
|
||||
data=data,
|
||||
ingestion_settings=settings.ingestion,
|
||||
chunking_settings=settings.chunking,
|
||||
qdrant_settings=settings.qdrant,
|
||||
thread_limiter=limiter,
|
||||
concurrency_limiter=concurrency_limiter,
|
||||
dense_embedders=dense_embedders,
|
||||
|
||||
@@ -23,7 +23,9 @@ async def readyz(request: Request, response: Response) -> dict[str, bool]:
|
||||
postgres_ready, minio_ready, qdrant_ready = await asyncio.gather(
|
||||
ping_postgres(resources.db_engine, timeout),
|
||||
ping_minio(resources.minio_client, timeout),
|
||||
ping_qdrant(resources.qdrant_client, timeout),
|
||||
ping_qdrant(
|
||||
resources.qdrant_client, timeout, collection=resources.settings.qdrant.collection
|
||||
),
|
||||
)
|
||||
|
||||
result = {
|
||||
|
||||
63
src/api/schemas/domains.py
Normal file
63
src/api/schemas/domains.py
Normal file
@@ -0,0 +1,63 @@
|
||||
"""Public request/response models for `/v1/domains` (ADR-0008, ADR-0009).
|
||||
|
||||
`tenant_id` appears in none of these: it comes from the authenticated key, and
|
||||
accepting it from a body would break the isolation boundary (ADR-0002).
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from datetime import datetime
|
||||
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
|
||||
from src.application.domains.models import DomainResult
|
||||
|
||||
# Lowercase alphanumerics plus - and _; the key is embedded in every Qdrant
|
||||
# payload and filtered on as a keyword, so it stays boring on purpose.
|
||||
_DOMAIN_PATTERN = r"^[a-z0-9][a-z0-9_-]*$"
|
||||
|
||||
|
||||
class DomainResponse(BaseModel):
|
||||
id: uuid.UUID
|
||||
domain: str
|
||||
display_name: str
|
||||
status: str
|
||||
metadata: dict[str, object]
|
||||
created_at: datetime
|
||||
updated_at: datetime
|
||||
|
||||
@classmethod
|
||||
def from_result(cls, result: DomainResult) -> "DomainResponse":
|
||||
return cls(
|
||||
id=result.id,
|
||||
domain=result.domain,
|
||||
display_name=result.display_name,
|
||||
status=result.status,
|
||||
metadata=result.metadata,
|
||||
created_at=result.created_at,
|
||||
updated_at=result.updated_at,
|
||||
)
|
||||
|
||||
|
||||
class DomainListResponse(BaseModel):
|
||||
domains: list[DomainResponse]
|
||||
|
||||
|
||||
class CreateDomainRequest(BaseModel):
|
||||
domain: str = Field(min_length=1, max_length=80, pattern=_DOMAIN_PATTERN)
|
||||
display_name: str = Field(min_length=1, max_length=200)
|
||||
metadata: dict[str, object] = Field(default_factory=dict)
|
||||
|
||||
@field_validator("domain")
|
||||
@classmethod
|
||||
def _normalize(cls, value: str) -> str:
|
||||
return value.strip()
|
||||
|
||||
|
||||
class UpdateDomainRequest(BaseModel):
|
||||
"""`domain` is absent by design — the key is immutable.
|
||||
|
||||
It is denormalized into every point payload and into `source_files`, so
|
||||
renaming it is a migration rather than an edit (ADR-0009).
|
||||
"""
|
||||
|
||||
display_name: str = Field(min_length=1, max_length=200)
|
||||
@@ -8,6 +8,7 @@ must run with no Postgres session held open at all.
|
||||
|
||||
from datetime import UTC, datetime
|
||||
|
||||
import structlog
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.auth.context import AuthContext
|
||||
@@ -16,28 +17,66 @@ from src.application.auth.keys import parse_api_key, verify_secret
|
||||
from src.infrastructure.postgres.repositories import api_keys as api_keys_repo
|
||||
from src.infrastructure.postgres.repositories import tenants as tenants_repo
|
||||
|
||||
logger = structlog.get_logger(__name__)
|
||||
|
||||
|
||||
async def resolve_auth_context(
|
||||
sessionmaker: async_sessionmaker[AsyncSession], bearer_token: str
|
||||
) -> AuthContext:
|
||||
"""Resolve a bearer token, logging the outcome either way (ADR-0011).
|
||||
|
||||
This runs on every authenticated request, so `auth.failed` is the one
|
||||
event most likely to matter first when diagnosing a client integration
|
||||
issue -- and the reason string alone (never logged; it can echo back
|
||||
attacker-supplied key material) is not enough to tell a malformed token
|
||||
apart from a revoked one without this.
|
||||
"""
|
||||
parsed = parse_api_key(bearer_token)
|
||||
if parsed is None:
|
||||
logger.warning("auth.failed", reason="malformed_key")
|
||||
raise InvalidApiKeyError("malformed API key")
|
||||
key_prefix, secret = parsed
|
||||
|
||||
async with sessionmaker() as session:
|
||||
api_key = await api_keys_repo.get_by_prefix(session, key_prefix)
|
||||
if api_key is None or not verify_secret(secret, api_key.key_hash):
|
||||
logger.warning("auth.failed", reason="unknown_key", key_prefix=key_prefix)
|
||||
raise InvalidApiKeyError("unknown API key")
|
||||
if api_key.status != "active":
|
||||
logger.warning(
|
||||
"auth.failed",
|
||||
reason="key_inactive",
|
||||
key_prefix=key_prefix,
|
||||
api_key_id=str(api_key.id),
|
||||
key_status=api_key.status,
|
||||
)
|
||||
raise InvalidApiKeyError(f"API key is {api_key.status}")
|
||||
if api_key.expires_at is not None and api_key.expires_at <= datetime.now(UTC):
|
||||
logger.warning(
|
||||
"auth.failed",
|
||||
reason="key_expired",
|
||||
key_prefix=key_prefix,
|
||||
api_key_id=str(api_key.id),
|
||||
)
|
||||
raise InvalidApiKeyError("API key has expired")
|
||||
|
||||
tenant = await tenants_repo.get_by_id(session, api_key.tenant_id)
|
||||
if tenant is None or tenant.status != "active":
|
||||
logger.warning(
|
||||
"auth.failed",
|
||||
reason="tenant_inactive",
|
||||
key_prefix=key_prefix,
|
||||
api_key_id=str(api_key.id),
|
||||
tenant_id=str(api_key.tenant_id),
|
||||
)
|
||||
raise TenantInactiveError("tenant is not active")
|
||||
|
||||
logger.info(
|
||||
"auth.succeeded",
|
||||
tenant_id=str(tenant.id),
|
||||
api_key_id=str(api_key.id),
|
||||
actor_type=api_key.actor_type,
|
||||
)
|
||||
return AuthContext(
|
||||
tenant_id=tenant.id,
|
||||
tenant_slug=tenant.slug,
|
||||
|
||||
27
src/application/domains/__init__.py
Normal file
27
src/application/domains/__init__.py
Normal file
@@ -0,0 +1,27 @@
|
||||
"""Tenant-domain management and the upload-time allowlist check (ADR-0009)."""
|
||||
|
||||
from src.application.domains.errors import (
|
||||
DomainAlreadyExistsError,
|
||||
DomainsError,
|
||||
UnknownDomainError,
|
||||
)
|
||||
from src.application.domains.models import DomainResult
|
||||
from src.application.domains.service import (
|
||||
create_domain,
|
||||
ensure_domain_allowed,
|
||||
list_domains,
|
||||
set_domain_status,
|
||||
update_domain,
|
||||
)
|
||||
|
||||
__all__ = [
|
||||
"DomainAlreadyExistsError",
|
||||
"DomainResult",
|
||||
"DomainsError",
|
||||
"UnknownDomainError",
|
||||
"create_domain",
|
||||
"ensure_domain_allowed",
|
||||
"list_domains",
|
||||
"set_domain_status",
|
||||
"update_domain",
|
||||
]
|
||||
21
src/application/domains/errors.py
Normal file
21
src/application/domains/errors.py
Normal file
@@ -0,0 +1,21 @@
|
||||
"""Domain-management failures (ADR-0009). No HTTP knowledge here —
|
||||
`src/api/errors.py` maps these to status codes.
|
||||
"""
|
||||
|
||||
|
||||
class DomainsError(Exception):
|
||||
"""Base class for tenant-domain failures."""
|
||||
|
||||
|
||||
class UnknownDomainError(DomainsError):
|
||||
"""The upload named a domain the tenant has not registered, or one that is
|
||||
disabled. Maps to `400`.
|
||||
|
||||
Rejecting is the whole point: an unrecognized `domain` would otherwise
|
||||
create a new Qdrant partition silently, and a file in a partition nothing
|
||||
queries is invisible rather than failed (ADR-0009).
|
||||
"""
|
||||
|
||||
|
||||
class DomainAlreadyExistsError(DomainsError):
|
||||
"""The tenant already has a domain with this key. Maps to `409`."""
|
||||
16
src/application/domains/models.py
Normal file
16
src/application/domains/models.py
Normal file
@@ -0,0 +1,16 @@
|
||||
"""Transport-agnostic results for the domain-management service."""
|
||||
|
||||
import uuid
|
||||
from dataclasses import dataclass
|
||||
from datetime import datetime
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class DomainResult:
|
||||
id: uuid.UUID
|
||||
domain: str
|
||||
display_name: str
|
||||
status: str
|
||||
metadata: dict[str, object]
|
||||
created_at: datetime
|
||||
updated_at: datetime
|
||||
164
src/application/domains/service.py
Normal file
164
src/application/domains/service.py
Normal file
@@ -0,0 +1,164 @@
|
||||
"""Tenant-domain management (ADR-0009).
|
||||
|
||||
A tenant's domain set is per-tenant and varies in size — one may run 14
|
||||
insurance lines, another 6 — so it is data, not an enum.
|
||||
|
||||
`ensure_domain_allowed` is the reason this package exists: it is the strict
|
||||
allowlist check the upload path runs before anything is written. Everything
|
||||
else here is the management surface the calling backend uses to populate that
|
||||
allowlist, under its own `domains:write` scope so an upload key cannot create
|
||||
partitions.
|
||||
|
||||
`tenant_id` is always a required parameter taken from `AuthContext`, never from
|
||||
a request body (ADR-0002).
|
||||
"""
|
||||
|
||||
import uuid
|
||||
|
||||
import structlog
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.domains.errors import DomainAlreadyExistsError, UnknownDomainError
|
||||
from src.application.domains.models import DomainResult
|
||||
from src.infrastructure.postgres.models.tenant_domain import TenantDomain
|
||||
from src.infrastructure.postgres.repositories import tenant_domains as repo
|
||||
|
||||
logger = structlog.get_logger(__name__)
|
||||
|
||||
|
||||
async def _flush_and_refresh(session: AsyncSession, tenant_domain: TenantDomain) -> None:
|
||||
"""Materialize server-generated columns before the row leaves the session.
|
||||
|
||||
`updated_at` is `onupdate=func.now()`, so after an UPDATE its value lives in
|
||||
the database, not in the instance. Reading it later would trigger a lazy
|
||||
load outside any greenlet context (`MissingGreenlet`), so it is fetched here
|
||||
while the session is still open.
|
||||
"""
|
||||
await session.flush()
|
||||
await session.refresh(tenant_domain)
|
||||
|
||||
|
||||
def _to_result(tenant_domain: TenantDomain) -> DomainResult:
|
||||
return DomainResult(
|
||||
id=tenant_domain.id,
|
||||
domain=tenant_domain.domain,
|
||||
display_name=tenant_domain.display_name,
|
||||
status=tenant_domain.status,
|
||||
metadata=tenant_domain.metadata_,
|
||||
created_at=tenant_domain.created_at,
|
||||
updated_at=tenant_domain.updated_at,
|
||||
)
|
||||
|
||||
|
||||
async def ensure_domain_allowed(
|
||||
session: AsyncSession, *, tenant_id: uuid.UUID, domain: str
|
||||
) -> None:
|
||||
"""Raise `UnknownDomainError` unless the tenant has this domain active.
|
||||
|
||||
Takes a session rather than a sessionmaker: the upload path calls this
|
||||
inside its existing txn A, so the check costs no extra connection and
|
||||
cannot pass and then go stale before the row is written.
|
||||
|
||||
Logs the rejection here rather than at the call site: this runs before any
|
||||
`ingestion_jobs` row exists, so `upload_source_file`'s job-level
|
||||
`ingestion.job.failed` event (ADR-0011) never fires for it -- without a log
|
||||
here, a rejected upload would leave no operational trace at all.
|
||||
"""
|
||||
tenant_domain = await repo.get(session, tenant_id=tenant_id, domain=domain)
|
||||
if tenant_domain is None:
|
||||
logger.warning(
|
||||
"domain.rejected", tenant_id=str(tenant_id), domain=domain, reason="unregistered"
|
||||
)
|
||||
raise UnknownDomainError(
|
||||
f"domain '{domain}' is not registered for this tenant; "
|
||||
f"create it via POST /v1/domains before uploading to it"
|
||||
)
|
||||
if tenant_domain.status != "active":
|
||||
logger.warning(
|
||||
"domain.rejected", tenant_id=str(tenant_id), domain=domain, reason="disabled"
|
||||
)
|
||||
raise UnknownDomainError(f"domain '{domain}' is disabled for this tenant")
|
||||
|
||||
|
||||
async def list_domains(
|
||||
sessionmaker: async_sessionmaker[AsyncSession],
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
include_disabled: bool = False,
|
||||
) -> list[DomainResult]:
|
||||
async with sessionmaker() as session:
|
||||
found = await repo.list_for_tenant(
|
||||
session, tenant_id=tenant_id, include_disabled=include_disabled
|
||||
)
|
||||
return [_to_result(item) for item in found]
|
||||
|
||||
|
||||
async def create_domain(
|
||||
sessionmaker: async_sessionmaker[AsyncSession],
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
domain: str,
|
||||
display_name: str,
|
||||
metadata: dict[str, object] | None = None,
|
||||
) -> DomainResult:
|
||||
async with sessionmaker() as session:
|
||||
if await repo.get(session, tenant_id=tenant_id, domain=domain) is not None:
|
||||
raise DomainAlreadyExistsError(f"domain '{domain}' already exists for this tenant")
|
||||
created = repo.create(
|
||||
session,
|
||||
tenant_id=tenant_id,
|
||||
domain=domain,
|
||||
display_name=display_name,
|
||||
metadata=metadata,
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
logger.info("domain.created", tenant_id=str(tenant_id), domain=domain)
|
||||
return _to_result(created)
|
||||
|
||||
|
||||
async def update_domain(
|
||||
sessionmaker: async_sessionmaker[AsyncSession],
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
domain: str,
|
||||
display_name: str,
|
||||
) -> DomainResult:
|
||||
"""Only the label is mutable — see `repo.update_display_name`."""
|
||||
async with sessionmaker() as session:
|
||||
found = await repo.get(session, tenant_id=tenant_id, domain=domain)
|
||||
if found is None:
|
||||
raise UnknownDomainError(f"domain '{domain}' is not registered for this tenant")
|
||||
repo.update_display_name(found, display_name=display_name)
|
||||
await _flush_and_refresh(session, found)
|
||||
await session.commit()
|
||||
result = _to_result(found)
|
||||
|
||||
logger.info("domain.updated", tenant_id=str(tenant_id), domain=domain)
|
||||
return result
|
||||
|
||||
|
||||
async def set_domain_status(
|
||||
sessionmaker: async_sessionmaker[AsyncSession],
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
domain: str,
|
||||
status: str,
|
||||
) -> DomainResult:
|
||||
"""Disable or re-enable a domain.
|
||||
|
||||
Disabling blocks new uploads and hides the domain from pickers. It does not
|
||||
touch the points already indexed under it — removing those needs the
|
||||
tenant-erasure workflow plan 001 defers.
|
||||
"""
|
||||
async with sessionmaker() as session:
|
||||
found = await repo.get(session, tenant_id=tenant_id, domain=domain)
|
||||
if found is None:
|
||||
raise UnknownDomainError(f"domain '{domain}' is not registered for this tenant")
|
||||
repo.set_status(found, status=status)
|
||||
await _flush_and_refresh(session, found)
|
||||
await session.commit()
|
||||
result = _to_result(found)
|
||||
|
||||
logger.info("domain.status_changed", tenant_id=str(tenant_id), domain=domain, status=status)
|
||||
return result
|
||||
@@ -16,9 +16,10 @@ job stuck in `running`. The whole request additionally holds one of
|
||||
phase 2 is bounded by `INGESTION_TIMEOUT_SECONDS` (`504`) (ADR-0017, plan 001
|
||||
Phase 4).
|
||||
|
||||
Qdrant point upserts are Phase 5 work, not implemented here: this phase
|
||||
parses, chunks, and embeds, so a successful job still reports
|
||||
`chunks_indexed=0` — nothing is searchable yet.
|
||||
Phase 2 ends by upserting the embedded chunks as tenant-scoped Qdrant points
|
||||
(`src/application/points/`), so a successful upload is searchable by the time
|
||||
the `201` returns. The collection those points land in is provisioned by a
|
||||
deployment step, not by this path — see `src/cli/qdrant_bootstrap.py`.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
@@ -29,6 +30,7 @@ from anyio import CapacityLimiter, Semaphore, fail_after, to_thread
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.auth.context import AuthContext
|
||||
from src.application.domains import ensure_domain_allowed
|
||||
from src.application.files.errors import InvalidUploadError
|
||||
from src.application.files.models import UploadResult
|
||||
from src.application.files.storage_keys import source_file_object_key
|
||||
@@ -45,10 +47,13 @@ from src.application.ingestion.errors import (
|
||||
ChunkLimitExceededError,
|
||||
EmbedderError,
|
||||
IngestionTimeoutError,
|
||||
PointIndexingError,
|
||||
)
|
||||
from src.application.points import index_chunks
|
||||
from src.application.ports.embedding import DenseEmbedder, SparseEmbedder
|
||||
from src.application.ports.object_storage import ObjectStorage
|
||||
from src.config import ChunkingSettings, IngestionSettings
|
||||
from src.application.ports.point_storage import PointStorage
|
||||
from src.config import ChunkingSettings, IngestionSettings, QdrantSettings
|
||||
from src.infrastructure.postgres.repositories import ingestion_jobs as jobs_repo
|
||||
from src.infrastructure.postgres.repositories import source_files as source_files_repo
|
||||
|
||||
@@ -63,6 +68,16 @@ async def _mark_job_failed(
|
||||
error_code: str,
|
||||
error_message: str,
|
||||
) -> None:
|
||||
"""Write the terminal `failed` job row and emit its log event together.
|
||||
|
||||
Every failure branch below calls this, so logging here once closes every
|
||||
branch at once rather than duplicating a `logger.warning` at each call
|
||||
site (CLAUDE.md, "prefer deep modules") -- previously only
|
||||
`storage_upload_failed` and `timeout` did that ad hoc, and
|
||||
`parse_failed`/`chunk_limit_exceeded`/`embedding_failed`/`index_failed`
|
||||
logged nothing at all: visible in `ingestion_job_events` but invisible to
|
||||
log-based alerting (ADR-0011).
|
||||
"""
|
||||
async with sessionmaker() as session:
|
||||
job = await jobs_repo.mark_terminal(
|
||||
session,
|
||||
@@ -83,17 +98,27 @@ async def _mark_job_failed(
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
logger.warning(
|
||||
"ingestion.job.failed",
|
||||
tenant_id=str(tenant_id),
|
||||
ingestion_job_id=str(ingestion_job_id),
|
||||
error_code=error_code,
|
||||
error_message=error_message,
|
||||
)
|
||||
|
||||
|
||||
async def upload_source_file(
|
||||
*,
|
||||
sessionmaker: async_sessionmaker[AsyncSession],
|
||||
storage: ObjectStorage,
|
||||
point_storage: PointStorage,
|
||||
auth: AuthContext,
|
||||
domain: str,
|
||||
filename: str,
|
||||
data: bytes,
|
||||
ingestion_settings: IngestionSettings,
|
||||
chunking_settings: ChunkingSettings,
|
||||
qdrant_settings: QdrantSettings,
|
||||
thread_limiter: CapacityLimiter,
|
||||
concurrency_limiter: Semaphore,
|
||||
dense_embedders: Sequence[DenseEmbedder],
|
||||
@@ -112,6 +137,12 @@ async def upload_source_file(
|
||||
|
||||
async with acquire_ingestion_slot(concurrency_limiter):
|
||||
async with sessionmaker() as session:
|
||||
# Strict allowlist, checked inside txn A before anything is written
|
||||
# (ADR-0009). An unregistered domain would otherwise create a new
|
||||
# Qdrant partition silently, leaving the file invisible to
|
||||
# retrieval rather than failing.
|
||||
await ensure_domain_allowed(session, tenant_id=auth.tenant_id, domain=domain)
|
||||
|
||||
existing = await source_files_repo.find_active_by_content_hash(
|
||||
session,
|
||||
tenant_id=auth.tenant_id,
|
||||
@@ -178,6 +209,15 @@ async def upload_source_file(
|
||||
await session.commit()
|
||||
ingestion_job_id = job.id
|
||||
|
||||
logger.info(
|
||||
"ingestion.job.started",
|
||||
tenant_id=str(auth.tenant_id),
|
||||
ingestion_job_id=str(ingestion_job_id),
|
||||
file_id=str(source_file_id),
|
||||
domain=domain,
|
||||
source_type=validated.source_type,
|
||||
)
|
||||
|
||||
# Phase 2: no Postgres session open across this work (ADR-0017),
|
||||
# bounded end-to-end by INGESTION_TIMEOUT_SECONDS.
|
||||
try:
|
||||
@@ -187,12 +227,6 @@ async def upload_source_file(
|
||||
key=object_key, data=data, content_type=validated.content_type
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"files.upload.storage_failed",
|
||||
tenant_id=str(auth.tenant_id),
|
||||
file_id=str(source_file_id),
|
||||
ingestion_job_id=str(ingestion_job_id),
|
||||
)
|
||||
await _mark_job_failed(
|
||||
sessionmaker,
|
||||
tenant_id=auth.tenant_id,
|
||||
@@ -249,13 +283,32 @@ async def upload_source_file(
|
||||
error_message=str(exc),
|
||||
)
|
||||
raise
|
||||
except TimeoutError:
|
||||
logger.warning(
|
||||
"files.upload.timeout",
|
||||
tenant_id=str(auth.tenant_id),
|
||||
file_id=str(source_file_id),
|
||||
ingestion_job_id=str(ingestion_job_id),
|
||||
|
||||
try:
|
||||
indexed = await index_chunks(
|
||||
embedded,
|
||||
storage=point_storage,
|
||||
tenant_id=auth.tenant_id,
|
||||
domain=domain,
|
||||
file_id=source_file_id,
|
||||
source_filename=filename,
|
||||
source_type=validated.source_type,
|
||||
actor=f"api_key:{auth.api_key_id}",
|
||||
dense_embedders=dense_embedders,
|
||||
sparse_embedder=sparse_embedder,
|
||||
settings=qdrant_settings,
|
||||
thread_limiter=thread_limiter,
|
||||
)
|
||||
except PointIndexingError as exc:
|
||||
await _mark_job_failed(
|
||||
sessionmaker,
|
||||
tenant_id=auth.tenant_id,
|
||||
ingestion_job_id=ingestion_job_id,
|
||||
error_code="index_failed",
|
||||
error_message=str(exc),
|
||||
)
|
||||
raise
|
||||
except TimeoutError:
|
||||
await _mark_job_failed(
|
||||
sessionmaker,
|
||||
tenant_id=auth.tenant_id,
|
||||
@@ -273,7 +326,11 @@ async def upload_source_file(
|
||||
tenant_id=auth.tenant_id,
|
||||
ingestion_job_id=ingestion_job_id,
|
||||
status="succeeded",
|
||||
points_created=0,
|
||||
# An upsert with deterministic ids cannot tell an insert from
|
||||
# an overwrite, so every written point is reported here and
|
||||
# `points_updated` stays 0 rather than being guessed at.
|
||||
points_created=indexed.points_upserted,
|
||||
points_soft_deleted=indexed.points_soft_deleted,
|
||||
)
|
||||
jobs_repo.append_event(
|
||||
session,
|
||||
@@ -281,21 +338,29 @@ async def upload_source_file(
|
||||
ingestion_job_id=ingestion_job_id,
|
||||
level="info",
|
||||
stage="completed",
|
||||
message="chunks parsed and embedded; Qdrant indexing not yet implemented",
|
||||
details={"chunks_parsed": len(chunks), "chunks_embedded": len(embedded)},
|
||||
message="chunks parsed, embedded, and indexed",
|
||||
details={
|
||||
"chunks_parsed": len(chunks),
|
||||
"chunks_embedded": len(embedded),
|
||||
"points_upserted": indexed.points_upserted,
|
||||
"points_soft_deleted": indexed.points_soft_deleted,
|
||||
},
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
logger.info(
|
||||
"files.upload.succeeded",
|
||||
"ingestion.job.completed",
|
||||
tenant_id=str(auth.tenant_id),
|
||||
file_id=str(source_file_id),
|
||||
ingestion_job_id=str(ingestion_job_id),
|
||||
file_id=str(source_file_id),
|
||||
chunks_parsed=len(chunks),
|
||||
points_upserted=indexed.points_upserted,
|
||||
points_soft_deleted=indexed.points_soft_deleted,
|
||||
)
|
||||
return UploadResult(
|
||||
file_id=source_file_id,
|
||||
ingestion_job_id=ingestion_job_id,
|
||||
status="succeeded",
|
||||
chunks_indexed=0,
|
||||
chunks_indexed=indexed.points_upserted,
|
||||
is_new_attempt=True,
|
||||
)
|
||||
|
||||
@@ -49,6 +49,15 @@ class EmbedderError(IngestionError):
|
||||
"""
|
||||
|
||||
|
||||
class PointIndexingError(IngestionError):
|
||||
"""Upserting or soft-deleting Qdrant points failed.
|
||||
|
||||
Maps to `502` — like `EmbedderError`, this is an upstream dependency
|
||||
failing, not a malformed request. Kept distinct from `EmbedderError` so the
|
||||
job's `error_code` says which dependency broke.
|
||||
"""
|
||||
|
||||
|
||||
class IngestionAtCapacityError(IngestionError):
|
||||
"""`INGESTION_MAX_CONCURRENCY` in-process ingestions are already running.
|
||||
|
||||
|
||||
16
src/application/points/__init__.py
Normal file
16
src/application/points/__init__.py
Normal file
@@ -0,0 +1,16 @@
|
||||
"""Ingestion-generated Qdrant point CRUD (ADR-0001, ADR-0002).
|
||||
|
||||
`index_chunks` is the entry point callers outside this package should use: it
|
||||
dispatches payload construction, batching, bounded-concurrency upserts, and the
|
||||
post-success soft-delete sweep. `build_chunk_payload` and the batching helpers
|
||||
stay internal, exported mainly for their own unit tests.
|
||||
|
||||
Direct `/v1/points` CRUD (single-point edits, reordering, keyword search) is
|
||||
plan 002's surface, not this package's — plan 001 scopes it to "the reusable
|
||||
service layer required by ingestion".
|
||||
"""
|
||||
|
||||
from src.application.points.indexing import IndexingResult, index_chunks
|
||||
from src.application.points.models import ChunkPoint
|
||||
|
||||
__all__ = ["ChunkPoint", "IndexingResult", "index_chunks"]
|
||||
203
src/application/points/indexing.py
Normal file
203
src/application/points/indexing.py
Normal file
@@ -0,0 +1,203 @@
|
||||
"""The one caller-facing entry point for indexing embedded chunks (ADR-0001, ADR-0017).
|
||||
|
||||
`index_chunks` is the only version of this step callers should reach for. It
|
||||
owns the whole composition a correct upsert needs:
|
||||
|
||||
- building ADR-0001's payload for every chunk, with `tenant_id`/`domain` taken
|
||||
from server-derived context;
|
||||
- offloading that (and the per-chunk content hashing) to a thread, since it is
|
||||
blocking CPU work (ADR-0017);
|
||||
- batching at `QDRANT_UPSERT_BATCH_SIZE` inside ADR-0001's 64-256 band;
|
||||
- bounding in-flight batches with an `asyncio.Semaphore` rather than an
|
||||
unbounded `gather` (ADR-0017);
|
||||
- running the soft-delete sweep for a shortened file **only after every batch
|
||||
has succeeded**.
|
||||
|
||||
That last ordering is the point, not an implementation detail — see
|
||||
`_deactivate_stale` below. `build_chunk_payload` and `_batches` stay internal;
|
||||
pushing that composition onto every call site is exactly the obligation a deep
|
||||
module absorbs once (CLAUDE.md, "prefer deep modules").
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import uuid
|
||||
from collections.abc import Sequence
|
||||
from dataclasses import dataclass
|
||||
from datetime import UTC, datetime
|
||||
|
||||
from anyio import CapacityLimiter, to_thread
|
||||
|
||||
from src.application.ingestion.errors import PointIndexingError
|
||||
from src.application.ingestion.models import EmbeddedChunk
|
||||
from src.application.points.models import ChunkPoint
|
||||
from src.application.points.payload import build_chunk_payload
|
||||
from src.application.ports.embedding import DenseEmbedder, SparseEmbedder
|
||||
from src.application.ports.point_storage import PointStorage
|
||||
from src.config import QdrantSettings
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class IndexingResult:
|
||||
"""What one indexing pass wrote.
|
||||
|
||||
`points_upserted` counts points written, not points *created* — a
|
||||
deterministic-id upsert cannot distinguish an insert from an overwrite, so
|
||||
the ingestion job reports this as `points_created` and leaves
|
||||
`points_updated` at zero rather than guessing.
|
||||
"""
|
||||
|
||||
points_upserted: int
|
||||
points_soft_deleted: int
|
||||
|
||||
|
||||
def _embedding_model_version(
|
||||
dense_embedders: Sequence[DenseEmbedder], sparse_embedder: SparseEmbedder
|
||||
) -> str:
|
||||
"""Compose the `embedding_model_version` payload value (ADR-0001).
|
||||
|
||||
Sorted so the string is stable regardless of the order the embedders were
|
||||
wired in — an unstable value would make "which chunks need re-embedding?"
|
||||
unanswerable, which is the field's only reason to exist.
|
||||
"""
|
||||
versions = sorted(
|
||||
[embedder.model_version for embedder in dense_embedders] + [sparse_embedder.model_version]
|
||||
)
|
||||
return "+".join(versions)
|
||||
|
||||
|
||||
def _batches(points: Sequence[ChunkPoint], size: int) -> list[Sequence[ChunkPoint]]:
|
||||
return [points[i : i + size] for i in range(0, len(points), size)]
|
||||
|
||||
|
||||
def _build_points(
|
||||
embedded: Sequence[EmbeddedChunk],
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
domain: str,
|
||||
file_id: uuid.UUID,
|
||||
source_filename: str,
|
||||
source_type: str,
|
||||
actor: str,
|
||||
embedding_model_version: str,
|
||||
indexed_at: datetime,
|
||||
) -> list[ChunkPoint]:
|
||||
"""Blocking: hashes every chunk's content. Always called through a thread."""
|
||||
return [
|
||||
ChunkPoint(
|
||||
point_id=item.chunk.chunk_id,
|
||||
dense=item.dense,
|
||||
sparse=item.sparse,
|
||||
payload=build_chunk_payload(
|
||||
item.chunk,
|
||||
tenant_id=tenant_id,
|
||||
domain=domain,
|
||||
file_id=file_id,
|
||||
source_filename=source_filename,
|
||||
source_type=source_type,
|
||||
actor=actor,
|
||||
embedding_model_version=embedding_model_version,
|
||||
indexed_at=indexed_at,
|
||||
),
|
||||
)
|
||||
for item in embedded
|
||||
]
|
||||
|
||||
|
||||
async def _upsert_bounded(
|
||||
storage: PointStorage, batch: Sequence[ChunkPoint], *, semaphore: asyncio.Semaphore
|
||||
) -> None:
|
||||
async with semaphore:
|
||||
try:
|
||||
await storage.upsert_points(batch)
|
||||
except Exception as exc:
|
||||
raise PointIndexingError(f"upserting {len(batch)} points failed: {exc}") from exc
|
||||
|
||||
|
||||
async def _deactivate_stale(
|
||||
storage: PointStorage,
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
file_id: uuid.UUID,
|
||||
from_chunk_index: int,
|
||||
actor: str,
|
||||
deleted_at: datetime,
|
||||
) -> int:
|
||||
"""Soft-delete points left over from a longer previous version of this file.
|
||||
|
||||
Chunk indices are contiguous from 0, so "index >= the new chunk count" is
|
||||
exactly the set of points the new version no longer produces.
|
||||
|
||||
This runs **only after every upsert has succeeded**, and that ordering is
|
||||
what keeps a failed attempt from damaging a working index. ADR-0001's
|
||||
deterministic point ids mean a re-ingestion overwrites in place, so literal
|
||||
atomic replacement is not available; what *is* guaranteed is that a failed
|
||||
attempt never removes content (it can only leave a prefix updated), and that
|
||||
a retry converges to the correct state. See ADR-0017.
|
||||
"""
|
||||
try:
|
||||
return await storage.deactivate_points_from_index(
|
||||
tenant_id=tenant_id,
|
||||
file_id=file_id,
|
||||
from_chunk_index=from_chunk_index,
|
||||
deleted_at=deleted_at,
|
||||
updated_by=actor,
|
||||
)
|
||||
except Exception as exc:
|
||||
raise PointIndexingError(f"soft-deleting stale points failed: {exc}") from exc
|
||||
|
||||
|
||||
async def index_chunks(
|
||||
embedded: Sequence[EmbeddedChunk],
|
||||
*,
|
||||
storage: PointStorage,
|
||||
tenant_id: uuid.UUID,
|
||||
domain: str,
|
||||
file_id: uuid.UUID,
|
||||
source_filename: str,
|
||||
source_type: str,
|
||||
actor: str,
|
||||
dense_embedders: Sequence[DenseEmbedder],
|
||||
sparse_embedder: SparseEmbedder,
|
||||
settings: QdrantSettings,
|
||||
thread_limiter: CapacityLimiter,
|
||||
) -> IndexingResult:
|
||||
"""Upsert every embedded chunk as a tenant-scoped point, then sweep leftovers.
|
||||
|
||||
Raises `PointIndexingError` (502) if any batch or the sweep fails.
|
||||
"""
|
||||
if not embedded:
|
||||
return IndexingResult(points_upserted=0, points_soft_deleted=0)
|
||||
|
||||
indexed_at = datetime.now(UTC)
|
||||
points = await to_thread.run_sync(
|
||||
lambda: _build_points(
|
||||
embedded,
|
||||
tenant_id=tenant_id,
|
||||
domain=domain,
|
||||
file_id=file_id,
|
||||
source_filename=source_filename,
|
||||
source_type=source_type,
|
||||
actor=actor,
|
||||
embedding_model_version=_embedding_model_version(dense_embedders, sparse_embedder),
|
||||
indexed_at=indexed_at,
|
||||
),
|
||||
limiter=thread_limiter,
|
||||
)
|
||||
|
||||
semaphore = asyncio.Semaphore(settings.upsert_concurrency)
|
||||
await asyncio.gather(
|
||||
*(
|
||||
_upsert_bounded(storage, batch, semaphore=semaphore)
|
||||
for batch in _batches(points, settings.upsert_batch_size)
|
||||
)
|
||||
)
|
||||
|
||||
soft_deleted = await _deactivate_stale(
|
||||
storage,
|
||||
tenant_id=tenant_id,
|
||||
file_id=file_id,
|
||||
from_chunk_index=len(points),
|
||||
actor=actor,
|
||||
deleted_at=indexed_at,
|
||||
)
|
||||
return IndexingResult(points_upserted=len(points), points_soft_deleted=soft_deleted)
|
||||
29
src/application/points/models.py
Normal file
29
src/application/points/models.py
Normal file
@@ -0,0 +1,29 @@
|
||||
"""Domain models for Qdrant points (ADR-0001).
|
||||
|
||||
Deliberately free of the `qdrant_client` SDK: `src/infrastructure/qdrant/`
|
||||
converts these to `PointStruct`/`models.SparseVector` at upsert time
|
||||
(ADR-0015 — application code and ports carry no infrastructure imports).
|
||||
"""
|
||||
|
||||
import uuid
|
||||
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from src.application.ingestion.models import SparseVector
|
||||
|
||||
|
||||
class ChunkPoint(BaseModel):
|
||||
"""One chunk, ready to upsert: its id, its named vectors, and its payload.
|
||||
|
||||
`point_id` is the chunk's deterministic UUIDv5 (`chunk_id_for`), so
|
||||
re-ingesting a file overwrites its points rather than duplicating them
|
||||
(ADR-0001).
|
||||
|
||||
`dense` is keyed by named-vector name (`dense_nomic`, `dense_openai`).
|
||||
`late_interaction` is absent — ADR-0017 does not compute it at ingest.
|
||||
"""
|
||||
|
||||
point_id: uuid.UUID
|
||||
dense: dict[str, list[float]]
|
||||
sparse: SparseVector
|
||||
payload: dict[str, object] = Field(default_factory=dict)
|
||||
70
src/application/points/payload.py
Normal file
70
src/application/points/payload.py
Normal file
@@ -0,0 +1,70 @@
|
||||
"""Builds ADR-0001's point payload from a chunk plus its ingestion context.
|
||||
|
||||
Internal to `src/application/points/` — callers use `index_chunks`, which owns
|
||||
composing this with batching and the deactivation sweep. Exported for its own
|
||||
unit tests, not as a surface to build payloads by hand.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from datetime import datetime
|
||||
from hashlib import sha256
|
||||
|
||||
from src.application.ingestion.models import Chunk
|
||||
|
||||
|
||||
def _optional_id(value: uuid.UUID | None) -> str | None:
|
||||
return str(value) if value is not None else None
|
||||
|
||||
|
||||
def build_chunk_payload(
|
||||
chunk: Chunk,
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
domain: str,
|
||||
file_id: uuid.UUID,
|
||||
source_filename: str,
|
||||
source_type: str,
|
||||
actor: str,
|
||||
embedding_model_version: str,
|
||||
indexed_at: datetime,
|
||||
) -> dict[str, object]:
|
||||
"""Return ADR-0001's payload for one chunk.
|
||||
|
||||
`tenant_id` and `domain` are passed in from the server-derived `AuthContext`
|
||||
and the validated request — never from anything the client could assert as
|
||||
authority (ADR-0002's non-negotiable isolation rule).
|
||||
|
||||
UUIDs are serialized as strings because the `tenant_id`/`domain`/`file_id`/
|
||||
`previous_chunk_id`/`next_chunk_id` payload indexes are *keyword* indexes;
|
||||
a native UUID would not match a keyword filter.
|
||||
|
||||
**Known gap — `version` is always written as `1`.** ADR-0002 uses this field
|
||||
for optimistic concurrency between ingestion and manual `/v1/points` edits,
|
||||
which needs a read-check-write (one read per point). Ingestion is
|
||||
authoritative for its own file today, so writing `1` is safe until
|
||||
`/v1/points` exists; plan 002 owns closing this.
|
||||
"""
|
||||
timestamp = indexed_at.isoformat()
|
||||
return {
|
||||
"tenant_id": str(tenant_id),
|
||||
"domain": domain,
|
||||
"file_id": str(file_id),
|
||||
"chunk_id": str(chunk.chunk_id),
|
||||
"content": chunk.content,
|
||||
"content_type": chunk.content_type.value,
|
||||
"source_filename": source_filename,
|
||||
"source_type": source_type,
|
||||
"order_id": chunk.order_id,
|
||||
"chunk_index": chunk.chunk_index,
|
||||
"previous_chunk_id": _optional_id(chunk.previous_chunk_id),
|
||||
"next_chunk_id": _optional_id(chunk.next_chunk_id),
|
||||
"is_active": True,
|
||||
"deleted_at": None,
|
||||
"created_at": timestamp,
|
||||
"updated_at": timestamp,
|
||||
"created_by": actor,
|
||||
"updated_by": actor,
|
||||
"version": 1,
|
||||
"content_hash": sha256(chunk.content.encode("utf-8")).hexdigest(),
|
||||
"embedding_model_version": embedding_model_version,
|
||||
}
|
||||
@@ -19,6 +19,14 @@ class DenseEmbedder(Protocol):
|
||||
"""
|
||||
|
||||
name: str
|
||||
model_version: str
|
||||
"""Identifies the model that produced these vectors (ADR-0001).
|
||||
|
||||
Written into every point's `embedding_model_version` payload field, which
|
||||
exists so a future model swap can tell which chunks need re-embedding. The
|
||||
embedder is what knows this, so it is reported here rather than
|
||||
reconstructed from configuration at the call site.
|
||||
"""
|
||||
|
||||
async def embed_batch(self, texts: Sequence[str]) -> list[list[float]]:
|
||||
"""Return one vector per input text, same order. Raises `EmbedderError`
|
||||
@@ -37,6 +45,12 @@ class SparseEmbedder(Protocol):
|
||||
"""
|
||||
|
||||
name: str
|
||||
model_version: str
|
||||
"""Identifies the analyzer/parameters that produced these vectors.
|
||||
|
||||
Same purpose as `DenseEmbedder.model_version`; for BM25 the "model" is the
|
||||
analyzer choice (ADR-0005), which is equally a re-embedding trigger.
|
||||
"""
|
||||
|
||||
def embed_batch(self, texts: Sequence[str], *, query: bool = False) -> list[SparseVector]:
|
||||
"""Return one sparse vector per input text, same order.
|
||||
|
||||
45
src/application/ports/point_storage.py
Normal file
45
src/application/ports/point_storage.py
Normal file
@@ -0,0 +1,45 @@
|
||||
"""The point-storage port (ADR-0001, ADR-0015).
|
||||
|
||||
`src/infrastructure/qdrant/points.py` is the production adapter; tests use a
|
||||
hand-written fake (ADR-0016). Application code depends on this Protocol, not on
|
||||
the `qdrant_client` SDK.
|
||||
|
||||
Deliberately narrow: exactly the two operations ingestion performs. Reads,
|
||||
single-point edits, reordering, and keyword search are plan 002's `/v1/points`
|
||||
surface and belong on a port of their own rather than accreting here.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from collections.abc import Sequence
|
||||
from datetime import datetime
|
||||
from typing import Protocol
|
||||
|
||||
from src.application.points.models import ChunkPoint
|
||||
|
||||
|
||||
class PointStorage(Protocol):
|
||||
async def upsert_points(self, points: Sequence[ChunkPoint]) -> None:
|
||||
"""Upsert one batch of points.
|
||||
|
||||
Callers own batching and concurrency bounding (ADR-0017's
|
||||
`upsert_concurrency` semaphore), not this Protocol — the same division
|
||||
`DenseEmbedder.embed_batch` uses.
|
||||
"""
|
||||
...
|
||||
|
||||
async def deactivate_points_from_index(
|
||||
self,
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
file_id: uuid.UUID,
|
||||
from_chunk_index: int,
|
||||
deleted_at: datetime,
|
||||
updated_by: str,
|
||||
) -> int:
|
||||
"""Soft-delete this file's points at or past `from_chunk_index`.
|
||||
|
||||
Sets `is_active=false`/`deleted_at` rather than removing the points
|
||||
(ADR-0002: delete is soft by default). Tenant-filtered — a `file_id`
|
||||
alone is never sufficient authority. Returns how many points matched.
|
||||
"""
|
||||
...
|
||||
@@ -9,6 +9,7 @@ from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.ports.embedding import DenseEmbedder, SparseEmbedder
|
||||
from src.application.ports.object_storage import ObjectStorage
|
||||
from src.application.ports.point_storage import PointStorage
|
||||
from src.config import Settings
|
||||
|
||||
|
||||
@@ -20,6 +21,7 @@ class AppResources:
|
||||
minio_client: Minio
|
||||
qdrant_client: AsyncQdrantClient
|
||||
object_storage: ObjectStorage
|
||||
point_storage: PointStorage
|
||||
ingestion_limiter: CapacityLimiter
|
||||
dense_embedders: Sequence[DenseEmbedder]
|
||||
sparse_embedder: SparseEmbedder
|
||||
@@ -46,6 +48,10 @@ def get_object_storage(request: Request) -> ObjectStorage:
|
||||
return _resources(request).object_storage
|
||||
|
||||
|
||||
def get_point_storage(request: Request) -> PointStorage:
|
||||
return _resources(request).point_storage
|
||||
|
||||
|
||||
def get_ingestion_limiter(request: Request) -> CapacityLimiter:
|
||||
return _resources(request).ingestion_limiter
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ from src.infrastructure.minio.storage import MinioObjectStorage
|
||||
from src.infrastructure.observability.logging import configure_logging
|
||||
from src.infrastructure.postgres.database import create_engine, create_sessionmaker
|
||||
from src.infrastructure.qdrant.client import create_client as create_qdrant_client
|
||||
from src.infrastructure.qdrant.points import QdrantPointStorage
|
||||
|
||||
logger = structlog.get_logger(__name__)
|
||||
|
||||
@@ -58,7 +59,7 @@ def create_lifespan(
|
||||
@asynccontextmanager
|
||||
async def lifespan(app: FastAPI) -> AsyncIterator[None]:
|
||||
resolved_settings = settings or Settings()
|
||||
configure_logging(resolved_settings.logging)
|
||||
configure_logging(resolved_settings.logging, resolved_settings.app)
|
||||
|
||||
# tiktoken fetches its vocabulary over the network on first use, so warm
|
||||
# it here: a missing vocabulary should fail the process at boot, not the
|
||||
@@ -77,6 +78,13 @@ def create_lifespan(
|
||||
logger.info("lifespan.minio.client.created")
|
||||
|
||||
qdrant_client = create_qdrant_client(resolved_settings.qdrant)
|
||||
# No collection DDL here: `ensure_chunks_collection` is a deployment
|
||||
# step (`python -m src.cli.qdrant_bootstrap`), for the same reason
|
||||
# ADR-0009 keeps Alembic out of startup and ADR-0012 makes LangGraph's
|
||||
# `.setup()` a deployment step.
|
||||
point_storage = QdrantPointStorage(
|
||||
qdrant_client, collection=resolved_settings.qdrant.collection
|
||||
)
|
||||
logger.info("lifespan.qdrant.client.created")
|
||||
|
||||
nomic_settings = resolved_settings.embedding.nomic
|
||||
@@ -136,6 +144,7 @@ def create_lifespan(
|
||||
minio_client=minio_client,
|
||||
qdrant_client=qdrant_client,
|
||||
object_storage=object_storage,
|
||||
point_storage=point_storage,
|
||||
ingestion_limiter=ingestion_limiter,
|
||||
dense_embedders=dense_embedders,
|
||||
sparse_embedder=sparse_embedder,
|
||||
|
||||
1
src/cli/__init__.py
Normal file
1
src/cli/__init__.py
Normal file
@@ -0,0 +1 @@
|
||||
"""Operator entry points that run as deployment steps, not at app startup."""
|
||||
54
src/cli/qdrant_bootstrap.py
Normal file
54
src/cli/qdrant_bootstrap.py
Normal file
@@ -0,0 +1,54 @@
|
||||
"""Create the `chunks` collection — the Qdrant analogue of `alembic upgrade head`.
|
||||
|
||||
uv run python -m src.cli.qdrant_bootstrap
|
||||
|
||||
A deployment step, deliberately not part of the FastAPI lifespan: collection
|
||||
creation is DDL, which ADR-0009 keeps out of application startup for Postgres
|
||||
and ADR-0012 keeps out of it for LangGraph's `.setup()`. See
|
||||
`src/infrastructure/qdrant/collection.py` for the full reasoning.
|
||||
|
||||
Idempotent and safe to re-run. Exits non-zero if an existing collection
|
||||
diverges from the pinned schema, rather than leaving a silently degraded
|
||||
sparse index behind.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import sys
|
||||
|
||||
import structlog
|
||||
|
||||
from src.config import Settings
|
||||
from src.infrastructure.observability.logging import configure_logging
|
||||
from src.infrastructure.qdrant.client import create_client
|
||||
from src.infrastructure.qdrant.collection import (
|
||||
CollectionSchemaMismatchError,
|
||||
ensure_chunks_collection,
|
||||
)
|
||||
|
||||
logger = structlog.get_logger(__name__)
|
||||
|
||||
|
||||
async def bootstrap(settings: Settings | None = None) -> int:
|
||||
resolved = settings or Settings()
|
||||
configure_logging(resolved.logging, resolved.app)
|
||||
client = create_client(resolved.qdrant)
|
||||
try:
|
||||
created = await ensure_chunks_collection(client, collection=resolved.qdrant.collection)
|
||||
except CollectionSchemaMismatchError as exc:
|
||||
logger.error("qdrant.bootstrap.schema_mismatch", error=str(exc))
|
||||
return 1
|
||||
finally:
|
||||
await client.close()
|
||||
|
||||
logger.info(
|
||||
"qdrant.bootstrap.completed", collection=resolved.qdrant.collection, created=created
|
||||
)
|
||||
return 0
|
||||
|
||||
|
||||
def main() -> None:
|
||||
sys.exit(asyncio.run(bootstrap()))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
@@ -109,12 +109,28 @@ class ChunkingSettings(BaseSettings):
|
||||
|
||||
|
||||
class QdrantSettings(BaseSettings):
|
||||
"""Qdrant connection and bulk-upsert bounds (ADR-0001, ADR-0017).
|
||||
|
||||
`collection` names the single shared collection all tenants live in
|
||||
(ADR-0001); it is deliberately configurable so tests can point at a
|
||||
disposable one. The vector *dimensions* are not settings -- they are model
|
||||
facts pinned in `src/infrastructure/qdrant/collection.py`, and changing one
|
||||
is a re-embedding migration.
|
||||
|
||||
`upsert_batch_size` sits inside ADR-0001's 64-256 bulk-upload band, and
|
||||
`upsert_concurrency` bounds in-flight batches so ingestion issues parallel
|
||||
streams rather than an unbounded `gather` (ADR-0017).
|
||||
"""
|
||||
|
||||
model_config = SettingsConfigDict(
|
||||
env_prefix="QDRANT_", extra="ignore", env_file=".env", env_ignore_empty=True
|
||||
)
|
||||
|
||||
url: str = "http://127.0.0.1:6343"
|
||||
api_key: str | None = None
|
||||
collection: str = "chunks"
|
||||
upsert_batch_size: int = 128
|
||||
upsert_concurrency: int = 4
|
||||
|
||||
|
||||
class NomicEmbeddingSettings(BaseSettings):
|
||||
@@ -210,15 +226,34 @@ class AppLimitSettings(BaseSettings):
|
||||
|
||||
env: str = "local"
|
||||
readiness_check_timeout_seconds: float = 2.0
|
||||
# The deployed commit SHA or release tag (ADR-0011, "Bind process-level
|
||||
# environment context"). Set by CI/CD at build/deploy time -- never
|
||||
# computed at runtime by shelling out to git, which would fail in a
|
||||
# container image with no .git directory.
|
||||
service_version: str = "dev"
|
||||
|
||||
|
||||
class LoggingSettings(BaseSettings):
|
||||
"""Logging sinks (ADR-0011).
|
||||
|
||||
`json_format` controls stdout's renderer only. Production sets it `true`
|
||||
so stdout is JSON for the container log collector; local development
|
||||
leaves it `false` for a colored console renderer. `file_path`, when set,
|
||||
is a second, independent handler that always renders JSON regardless of
|
||||
`json_format` -- a developer can read a human console while still keeping
|
||||
a machine-parseable file. Unset in production: stdout/stderr collection is
|
||||
preferred there over a log file inside the container.
|
||||
"""
|
||||
|
||||
model_config = SettingsConfigDict(
|
||||
env_prefix="LOG_", extra="ignore", env_file=".env", env_ignore_empty=True
|
||||
)
|
||||
|
||||
level: str = "INFO"
|
||||
json_format: bool = False
|
||||
file_path: str | None = None
|
||||
file_max_bytes: int = 10 * 1024 * 1024
|
||||
file_backup_count: int = 5
|
||||
|
||||
|
||||
class Settings(BaseSettings):
|
||||
|
||||
@@ -86,6 +86,7 @@ class Bm25SparseEmbedder:
|
||||
name = "sparse"
|
||||
|
||||
def __init__(self, settings: SparseEmbeddingSettings) -> None:
|
||||
self.model_version = f"bm25-{settings.analyzer}"
|
||||
self._settings = settings
|
||||
|
||||
def embed_batch(self, texts: Sequence[str], *, query: bool = False) -> list[SparseVector]:
|
||||
|
||||
@@ -52,6 +52,7 @@ class OpenAICompatibleEmbedder:
|
||||
keep_alive: str | None = None,
|
||||
) -> None:
|
||||
self.name = name
|
||||
self.model_version = model
|
||||
self._client = client
|
||||
self._model = model
|
||||
self._dimensions = dimensions
|
||||
|
||||
@@ -1,14 +1,50 @@
|
||||
"""Logging configuration: structlog + stdlib, dual local sinks (ADR-0011).
|
||||
|
||||
Console and an optional file are independent, simultaneous handlers on the
|
||||
same logger, not a single renderer chosen by a flag -- the same structlog
|
||||
event fans out to both. The console handler is always human-readable
|
||||
(`ConsoleRenderer`); the file handler, when enabled via `LOG_FILE_PATH`,
|
||||
always renders JSON regardless of `LOG_JSON_FORMAT`, so a saved log stays
|
||||
machine-parseable even when the terminal next to it is not.
|
||||
|
||||
`LOG_JSON_FORMAT` controls *stdout's* renderer only: production sets it `true`
|
||||
so the container log collector gets JSON; local development leaves it `false`
|
||||
for the colored console. `LOG_FILE_PATH` is expected to be unset in
|
||||
production -- stdout/stderr collection is preferred there over a log file
|
||||
inside the container.
|
||||
"""
|
||||
|
||||
import logging
|
||||
import logging.config
|
||||
import sys
|
||||
from collections.abc import Callable
|
||||
|
||||
import structlog
|
||||
|
||||
from src.config import LoggingSettings
|
||||
from src.config import AppLimitSettings, LoggingSettings
|
||||
|
||||
|
||||
def configure_logging(settings: LoggingSettings) -> None:
|
||||
def _bind_environment(settings: AppLimitSettings) -> Callable[..., dict[str, object]]:
|
||||
"""A static processor, not a contextvar: `env`/`service_version` don't
|
||||
vary per request, and a contextvar bound before the first request would
|
||||
be wiped by `RequestIdMiddleware`'s `clear_contextvars()` on that request.
|
||||
Closing over `settings` at configure time makes every event carry them
|
||||
instead, regardless of request context (ADR-0011).
|
||||
"""
|
||||
|
||||
def processor(
|
||||
logger: object, method_name: str, event_dict: dict[str, object]
|
||||
) -> dict[str, object]:
|
||||
event_dict["env"] = settings.env
|
||||
event_dict["service_version"] = settings.service_version
|
||||
return event_dict
|
||||
|
||||
return processor
|
||||
|
||||
|
||||
def configure_logging(settings: LoggingSettings, app_settings: AppLimitSettings) -> None:
|
||||
shared_processors = [
|
||||
_bind_environment(app_settings),
|
||||
structlog.contextvars.merge_contextvars,
|
||||
structlog.stdlib.add_log_level,
|
||||
structlog.stdlib.add_logger_name,
|
||||
@@ -27,55 +63,84 @@ def configure_logging(settings: LoggingSettings) -> None:
|
||||
cache_logger_on_first_use=True,
|
||||
)
|
||||
|
||||
renderer = (
|
||||
console_renderer = (
|
||||
structlog.processors.JSONRenderer()
|
||||
if settings.json_format
|
||||
else structlog.dev.ConsoleRenderer(colors=True)
|
||||
)
|
||||
|
||||
logging.config.dictConfig(
|
||||
{
|
||||
"version": 1,
|
||||
"disable_existing_loggers": False,
|
||||
"formatters": {
|
||||
"default": {
|
||||
formatters = {
|
||||
"console": {
|
||||
"()": structlog.stdlib.ProcessorFormatter,
|
||||
"processors": [
|
||||
structlog.stdlib.ProcessorFormatter.remove_processors_meta,
|
||||
renderer,
|
||||
console_renderer,
|
||||
],
|
||||
"foreign_pre_chain": [
|
||||
structlog.stdlib.ExtraAdder(),
|
||||
*shared_processors,
|
||||
],
|
||||
},
|
||||
},
|
||||
"handlers": {
|
||||
}
|
||||
handlers: dict[str, dict[str, object]] = {
|
||||
"console": {
|
||||
"class": "logging.StreamHandler",
|
||||
"level": settings.level,
|
||||
"formatter": "default",
|
||||
"formatter": "console",
|
||||
"stream": sys.stdout,
|
||||
},
|
||||
},
|
||||
}
|
||||
root_handlers = ["console"]
|
||||
|
||||
if settings.file_path is not None:
|
||||
# File handler always renders JSON, independent of the console
|
||||
# renderer chosen above -- a saved log stays machine-parseable even
|
||||
# when stdout is the colored, human-readable renderer.
|
||||
formatters["file"] = {
|
||||
"()": structlog.stdlib.ProcessorFormatter,
|
||||
"processors": [
|
||||
structlog.stdlib.ProcessorFormatter.remove_processors_meta,
|
||||
structlog.processors.JSONRenderer(),
|
||||
],
|
||||
"foreign_pre_chain": [
|
||||
structlog.stdlib.ExtraAdder(),
|
||||
*shared_processors,
|
||||
],
|
||||
}
|
||||
handlers["file"] = {
|
||||
"class": "logging.handlers.RotatingFileHandler",
|
||||
"level": settings.level,
|
||||
"formatter": "file",
|
||||
"filename": settings.file_path,
|
||||
"maxBytes": settings.file_max_bytes,
|
||||
"backupCount": settings.file_backup_count,
|
||||
}
|
||||
root_handlers.append("file")
|
||||
|
||||
logging.config.dictConfig(
|
||||
{
|
||||
"version": 1,
|
||||
"disable_existing_loggers": False,
|
||||
"formatters": formatters,
|
||||
"handlers": handlers,
|
||||
"loggers": {
|
||||
"": {
|
||||
"handlers": ["console"],
|
||||
"handlers": root_handlers,
|
||||
"level": settings.level,
|
||||
"propagate": False,
|
||||
},
|
||||
"uvicorn": {
|
||||
"handlers": ["console"],
|
||||
"handlers": root_handlers,
|
||||
"level": settings.level,
|
||||
"propagate": False,
|
||||
},
|
||||
"uvicorn.access": {
|
||||
"handlers": ["console"],
|
||||
"handlers": root_handlers,
|
||||
"level": settings.level,
|
||||
"propagate": False,
|
||||
},
|
||||
"sqlalchemy.engine": {
|
||||
"handlers": ["console"],
|
||||
"handlers": root_handlers,
|
||||
"level": "WARNING",
|
||||
"propagate": False,
|
||||
},
|
||||
|
||||
@@ -4,6 +4,7 @@ from src.infrastructure.postgres.models.ingestion_job import IngestionJob
|
||||
from src.infrastructure.postgres.models.ingestion_job_event import IngestionJobEvent
|
||||
from src.infrastructure.postgres.models.source_file import SourceFile
|
||||
from src.infrastructure.postgres.models.tenant import Tenant
|
||||
from src.infrastructure.postgres.models.tenant_domain import TenantDomain
|
||||
|
||||
__all__ = [
|
||||
"ApiKey",
|
||||
@@ -12,4 +13,5 @@ __all__ = [
|
||||
"IngestionJobEvent",
|
||||
"SourceFile",
|
||||
"Tenant",
|
||||
"TenantDomain",
|
||||
]
|
||||
|
||||
52
src/infrastructure/postgres/models/tenant_domain.py
Normal file
52
src/infrastructure/postgres/models/tenant_domain.py
Normal file
@@ -0,0 +1,52 @@
|
||||
import uuid
|
||||
from datetime import datetime
|
||||
|
||||
from sqlalchemy import CheckConstraint, DateTime, ForeignKey, String, UniqueConstraint, func
|
||||
from sqlalchemy.dialects.postgresql import JSONB
|
||||
from sqlalchemy.orm import Mapped, mapped_column
|
||||
|
||||
from src.infrastructure.postgres.models.base import Base
|
||||
|
||||
TENANT_DOMAIN_STATUSES = ("active", "disabled")
|
||||
|
||||
|
||||
class TenantDomain(Base):
|
||||
"""A domain a tenant is allowed to ingest into (ADR-0009).
|
||||
|
||||
Tenants do not share a domain list — one may run 14 insurance lines and
|
||||
another 6 — so this is a per-tenant table rather than an enum or a global
|
||||
lookup.
|
||||
|
||||
Its purpose is to stop an arbitrary caller-supplied `domain` from silently
|
||||
creating a new Qdrant partition. `domain` is denormalized into every point's
|
||||
payload and into `source_files`, and a typo like `fier` for `fire` produces
|
||||
no error anywhere: the file indexes into a partition retrieval never queries,
|
||||
so it is invisible rather than failed.
|
||||
|
||||
`domain` is the immutable key. Renaming it would mean rewriting every point
|
||||
payload that carries it, which is a migration, not an edit — `display_name`
|
||||
is the mutable human-facing label instead.
|
||||
"""
|
||||
|
||||
__tablename__ = "tenant_domains"
|
||||
__table_args__ = (
|
||||
UniqueConstraint("tenant_id", "domain", name="uq_tenant_domains_tenant_id_domain"),
|
||||
CheckConstraint(f"status IN {TENANT_DOMAIN_STATUSES}", name="ck_tenant_domains_status"),
|
||||
)
|
||||
|
||||
id: Mapped[uuid.UUID] = mapped_column(primary_key=True)
|
||||
tenant_id: Mapped[uuid.UUID] = mapped_column(
|
||||
ForeignKey("tenants.id", ondelete="CASCADE"), index=True
|
||||
)
|
||||
domain: Mapped[str] = mapped_column(String(80))
|
||||
display_name: Mapped[str] = mapped_column(String(200))
|
||||
status: Mapped[str] = mapped_column(String(20), default="active", server_default="active")
|
||||
metadata_: Mapped[dict[str, object]] = mapped_column(
|
||||
"metadata", JSONB, default=dict, server_default="{}"
|
||||
)
|
||||
|
||||
created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now())
|
||||
updated_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True), server_default=func.now(), onupdate=func.now()
|
||||
)
|
||||
disabled_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), default=None)
|
||||
73
src/infrastructure/postgres/repositories/tenant_domains.py
Normal file
73
src/infrastructure/postgres/repositories/tenant_domains.py
Normal file
@@ -0,0 +1,73 @@
|
||||
"""`tenant_domains` persistence (ADR-0009).
|
||||
|
||||
Plain functions over an `AsyncSession` the caller owns. No function here
|
||||
commits, rolls back, or closes the session (ADR-0012). Every read and write is
|
||||
tenant-scoped by a required `tenant_id` argument, so a missing filter is a
|
||||
signature error rather than a cross-tenant leak.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from datetime import UTC, datetime
|
||||
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from src.infrastructure.postgres.models.tenant_domain import TenantDomain
|
||||
|
||||
|
||||
async def get(session: AsyncSession, *, tenant_id: uuid.UUID, domain: str) -> TenantDomain | None:
|
||||
result = await session.execute(
|
||||
select(TenantDomain).where(
|
||||
TenantDomain.tenant_id == tenant_id, TenantDomain.domain == domain
|
||||
)
|
||||
)
|
||||
return result.scalar_one_or_none()
|
||||
|
||||
|
||||
async def list_for_tenant(
|
||||
session: AsyncSession, *, tenant_id: uuid.UUID, include_disabled: bool = False
|
||||
) -> list[TenantDomain]:
|
||||
statement = select(TenantDomain).where(TenantDomain.tenant_id == tenant_id)
|
||||
if not include_disabled:
|
||||
statement = statement.where(TenantDomain.status == "active")
|
||||
result = await session.execute(statement.order_by(TenantDomain.domain))
|
||||
return list(result.scalars().all())
|
||||
|
||||
|
||||
def create(
|
||||
session: AsyncSession,
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
domain: str,
|
||||
display_name: str,
|
||||
metadata: dict[str, object] | None = None,
|
||||
) -> TenantDomain:
|
||||
tenant_domain = TenantDomain(
|
||||
id=uuid.uuid4(),
|
||||
tenant_id=tenant_id,
|
||||
domain=domain,
|
||||
display_name=display_name,
|
||||
metadata_=metadata or {},
|
||||
)
|
||||
session.add(tenant_domain)
|
||||
return tenant_domain
|
||||
|
||||
|
||||
def update_display_name(tenant_domain: TenantDomain, *, display_name: str) -> TenantDomain:
|
||||
"""`domain` itself is deliberately not updatable.
|
||||
|
||||
It is denormalized into every Qdrant point payload and into `source_files`,
|
||||
so changing the key would mean rewriting all of them — a migration, not an
|
||||
edit. The label is what callers actually want to change.
|
||||
"""
|
||||
tenant_domain.display_name = display_name
|
||||
return tenant_domain
|
||||
|
||||
|
||||
def set_status(tenant_domain: TenantDomain, *, status: str) -> TenantDomain:
|
||||
"""Disable/re-enable a domain. Existing points are untouched either way —
|
||||
disabling blocks new uploads, it is not a delete (ADR-0002).
|
||||
"""
|
||||
tenant_domain.status = status
|
||||
tenant_domain.disabled_at = datetime.now(UTC) if status == "disabled" else None
|
||||
return tenant_domain
|
||||
@@ -9,10 +9,22 @@ def create_client(settings: QdrantSettings) -> AsyncQdrantClient:
|
||||
return AsyncQdrantClient(url=settings.url, api_key=settings.api_key)
|
||||
|
||||
|
||||
async def ping(client: AsyncQdrantClient, timeout: float) -> bool:
|
||||
async def ping(client: AsyncQdrantClient, timeout: float, *, collection: str) -> bool:
|
||||
"""Whether Qdrant is reachable **and** the `chunks` collection exists.
|
||||
|
||||
Reachability alone is not readiness here. The collection is created by a
|
||||
deployment step (`python -m src.cli.qdrant_bootstrap`, see ADR-0001
|
||||
"Collection provisioning"), so a process can boot against a healthy Qdrant
|
||||
that has no collection at all. Without this check that misconfiguration
|
||||
stays invisible until the first upload fails with a `502` — after the
|
||||
request has already paid for the MinIO write and the embedding round trips.
|
||||
|
||||
This is the Qdrant analogue of an unapplied Alembic migration, and it
|
||||
belongs in `/readyz` for the same reason: it is a dependency-readiness
|
||||
condition, not a process-health one.
|
||||
"""
|
||||
try:
|
||||
async with asyncio.timeout(timeout):
|
||||
await client.get_collections()
|
||||
return await client.collection_exists(collection)
|
||||
except Exception:
|
||||
return False
|
||||
return True
|
||||
|
||||
182
src/infrastructure/qdrant/collection.py
Normal file
182
src/infrastructure/qdrant/collection.py
Normal file
@@ -0,0 +1,182 @@
|
||||
"""The `chunks` collection schema and its provisioning (ADR-0001).
|
||||
|
||||
**This runs as a deployment step, never at FastAPI startup.** Creating a
|
||||
collection is DDL, and the project already rules DDL out of the request/boot
|
||||
path: ADR-0009 requires Alembic for Postgres schema, and ADR-0012 makes
|
||||
LangGraph's `.setup()` a deployment step. Doing it in the lifespan would also
|
||||
couple boot to Qdrant being reachable (that is `/readyz`'s job), race across
|
||||
replicas, and hide a misconfigured collection until traffic arrives.
|
||||
|
||||
Entry point for operators: `uv run python -m src.cli.qdrant_bootstrap`.
|
||||
|
||||
Two properties of this schema are load-bearing and fail *silently* if wrong,
|
||||
which is why `ensure_chunks_collection` verifies rather than skips:
|
||||
|
||||
- **`sparse` must carry `modifier=IDF`.** `src/infrastructure/embedding/bm25.py`
|
||||
computes only BM25's term-frequency saturation; Qdrant supplies IDF from
|
||||
collection-wide statistics. Without the modifier there is no error and no
|
||||
warning — lexical retrieval just quietly loses its IDF term (ADR-0005).
|
||||
- **The dense dimensions are pinned**: `dense_nomic` 768,
|
||||
`dense_openai` 3072 (native, `dimensions` deliberately unset). They are
|
||||
constants here rather than settings because they are model facts; changing
|
||||
one is a re-embedding migration, not a config tweak (ADR-0001).
|
||||
|
||||
All four named vectors are defined at creation even though `late_interaction`
|
||||
stays unpopulated until ADR-0003's rerank work (ADR-0017 does not compute it at
|
||||
ingest). Sparse and multivector fields cannot be added to an existing
|
||||
collection without recreating it, so deferring them is the one thing this
|
||||
schema cannot afford.
|
||||
"""
|
||||
|
||||
import structlog
|
||||
from qdrant_client import AsyncQdrantClient, models
|
||||
|
||||
logger = structlog.get_logger(__name__)
|
||||
|
||||
DENSE_NOMIC_VECTOR = "dense_nomic"
|
||||
DENSE_OPENAI_VECTOR = "dense_openai"
|
||||
SPARSE_VECTOR = "sparse"
|
||||
LATE_INTERACTION_VECTOR = "late_interaction"
|
||||
|
||||
DENSE_NOMIC_DIMENSIONS = 768
|
||||
DENSE_OPENAI_DIMENSIONS = 3072
|
||||
# jina-colbert-v2's per-token output dimension (ADR-0005).
|
||||
LATE_INTERACTION_DIMENSIONS = 128
|
||||
|
||||
|
||||
class CollectionSchemaMismatchError(RuntimeError):
|
||||
"""An existing collection does not match the schema this code expects.
|
||||
|
||||
Raised loudly instead of returning: silently accepting a collection whose
|
||||
dense size or sparse modifier differs is precisely the failure this
|
||||
explicit bootstrap step exists to prevent.
|
||||
"""
|
||||
|
||||
|
||||
def _vectors_config() -> dict[str, models.VectorParams]:
|
||||
return {
|
||||
DENSE_NOMIC_VECTOR: models.VectorParams(
|
||||
size=DENSE_NOMIC_DIMENSIONS, distance=models.Distance.COSINE
|
||||
),
|
||||
DENSE_OPENAI_VECTOR: models.VectorParams(
|
||||
size=DENSE_OPENAI_DIMENSIONS, distance=models.Distance.COSINE
|
||||
),
|
||||
# Rerank-only: never independently ANN-searched, so its HNSW graph is
|
||||
# disabled (m=0), and stored on disk so its larger footprint does not
|
||||
# degrade dense/sparse query latency (ADR-0001).
|
||||
LATE_INTERACTION_VECTOR: models.VectorParams(
|
||||
size=LATE_INTERACTION_DIMENSIONS,
|
||||
distance=models.Distance.COSINE,
|
||||
multivector_config=models.MultiVectorConfig(
|
||||
comparator=models.MultiVectorComparator.MAX_SIM
|
||||
),
|
||||
hnsw_config=models.HnswConfigDiff(m=0),
|
||||
on_disk=True,
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
def _sparse_vectors_config() -> dict[str, models.SparseVectorParams]:
|
||||
return {SPARSE_VECTOR: models.SparseVectorParams(modifier=models.Modifier.IDF)}
|
||||
|
||||
|
||||
# (field name, schema). `order_id` is float because Qdrant's `Range` conditions
|
||||
# and `order_by` only support numeric/datetime payloads -- a keyword key could
|
||||
# only be sorted client-side after fetching every chunk (ADR-0001).
|
||||
#
|
||||
# The full-text index on `content` is deliberately absent: it belongs to plan
|
||||
# 002's keyword search, and payload indexes -- unlike vector config -- can be
|
||||
# added to a live collection later.
|
||||
_PAYLOAD_INDEXES: tuple[tuple[str, models.PayloadSchemaType | models.KeywordIndexParams], ...] = (
|
||||
# `is_tenant` co-locates a tenant's vectors on disk for sequential reads,
|
||||
# which is the whole point of payload-partitioned multitenancy.
|
||||
(
|
||||
"tenant_id",
|
||||
models.KeywordIndexParams(type=models.KeywordIndexType.KEYWORD, is_tenant=True),
|
||||
),
|
||||
("domain", models.PayloadSchemaType.KEYWORD),
|
||||
("file_id", models.PayloadSchemaType.KEYWORD),
|
||||
("order_id", models.PayloadSchemaType.FLOAT),
|
||||
("previous_chunk_id", models.PayloadSchemaType.KEYWORD),
|
||||
("next_chunk_id", models.PayloadSchemaType.KEYWORD),
|
||||
)
|
||||
|
||||
|
||||
def _verify_existing(collection: str, info: models.CollectionInfo) -> None:
|
||||
params = info.config.params
|
||||
vectors = params.vectors
|
||||
if not isinstance(vectors, dict):
|
||||
raise CollectionSchemaMismatchError(
|
||||
f"collection {collection!r} has an unnamed dense vector; ADR-0001 requires "
|
||||
f"named vectors and this cannot be fixed without recreating the collection"
|
||||
)
|
||||
|
||||
for name, expected_size in (
|
||||
(DENSE_NOMIC_VECTOR, DENSE_NOMIC_DIMENSIONS),
|
||||
(DENSE_OPENAI_VECTOR, DENSE_OPENAI_DIMENSIONS),
|
||||
(LATE_INTERACTION_VECTOR, LATE_INTERACTION_DIMENSIONS),
|
||||
):
|
||||
existing = vectors.get(name)
|
||||
if existing is None:
|
||||
raise CollectionSchemaMismatchError(
|
||||
f"collection {collection!r} is missing the {name!r} vector"
|
||||
)
|
||||
if existing.size != expected_size:
|
||||
raise CollectionSchemaMismatchError(
|
||||
f"collection {collection!r} has {name!r} at {existing.size} dimensions, "
|
||||
f"expected {expected_size}; re-dimensioning is a re-embedding migration"
|
||||
)
|
||||
|
||||
sparse = (params.sparse_vectors or {}).get(SPARSE_VECTOR)
|
||||
if sparse is None:
|
||||
raise CollectionSchemaMismatchError(
|
||||
f"collection {collection!r} is missing the {SPARSE_VECTOR!r} vector; sparse "
|
||||
f"vectors cannot be added without recreating the collection"
|
||||
)
|
||||
if sparse.modifier != models.Modifier.IDF:
|
||||
raise CollectionSchemaMismatchError(
|
||||
f"collection {collection!r} has {SPARSE_VECTOR!r} with modifier "
|
||||
f"{sparse.modifier!r}, expected 'idf'; without it Qdrant applies no IDF "
|
||||
f"and lexical retrieval silently degrades (ADR-0005)"
|
||||
)
|
||||
|
||||
|
||||
async def ensure_chunks_collection(client: AsyncQdrantClient, *, collection: str) -> bool:
|
||||
"""Create the `chunks` collection and its payload indexes if absent.
|
||||
|
||||
Idempotent: an existing collection is verified against the pinned schema
|
||||
and left alone. Returns whether it created the collection.
|
||||
|
||||
Raises `CollectionSchemaMismatchError` if an existing collection diverges.
|
||||
"""
|
||||
if await client.collection_exists(collection):
|
||||
_verify_existing(collection, await client.get_collection(collection))
|
||||
logger.info("qdrant.collection.verified", collection=collection)
|
||||
# Payload indexes are additive and idempotent, so (re)creating them
|
||||
# here is what lets an index be added to an already-live collection.
|
||||
await _create_payload_indexes(client, collection=collection)
|
||||
return False
|
||||
|
||||
await client.create_collection(
|
||||
collection_name=collection,
|
||||
vectors_config=_vectors_config(),
|
||||
sparse_vectors_config=_sparse_vectors_config(),
|
||||
# m=0 disables the global index; payload_m builds per-tenant graphs
|
||||
# instead, per Qdrant's multitenant guidance (ADR-0001).
|
||||
hnsw_config=models.HnswConfigDiff(m=0, payload_m=16),
|
||||
)
|
||||
logger.info("qdrant.collection.created", collection=collection)
|
||||
await _create_payload_indexes(client, collection=collection)
|
||||
return True
|
||||
|
||||
|
||||
async def _create_payload_indexes(client: AsyncQdrantClient, *, collection: str) -> None:
|
||||
for field_name, field_schema in _PAYLOAD_INDEXES:
|
||||
await client.create_payload_index(
|
||||
collection_name=collection, field_name=field_name, field_schema=field_schema
|
||||
)
|
||||
logger.info(
|
||||
"qdrant.collection.payload_indexes.ensured",
|
||||
collection=collection,
|
||||
fields=[name for name, _ in _PAYLOAD_INDEXES],
|
||||
)
|
||||
97
src/infrastructure/qdrant/points.py
Normal file
97
src/infrastructure/qdrant/points.py
Normal file
@@ -0,0 +1,97 @@
|
||||
"""Qdrant adapter for the `PointStorage` port (ADR-0001, ADR-0002, ADR-0015).
|
||||
|
||||
The `qdrant_client` SDK appears here and nowhere in `application/`. This module
|
||||
translates the SDK-free `ChunkPoint` into `PointStruct`s and builds every
|
||||
filter — routers and application services never construct Qdrant filters.
|
||||
|
||||
`AsyncQdrantClient` is genuinely async, so unlike the `minio` adapter nothing
|
||||
here needs a thread offload.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from collections.abc import Sequence
|
||||
from datetime import datetime
|
||||
|
||||
from qdrant_client import AsyncQdrantClient, models
|
||||
|
||||
from src.application.points.models import ChunkPoint
|
||||
from src.infrastructure.qdrant.collection import SPARSE_VECTOR
|
||||
|
||||
|
||||
def _tenant_file_filter(
|
||||
tenant_id: uuid.UUID, file_id: uuid.UUID, *, from_chunk_index: int
|
||||
) -> models.Filter:
|
||||
"""Points of one file, at or past `from_chunk_index`, within one tenant.
|
||||
|
||||
`tenant_id` is always a condition, never optional: a `file_id` alone is not
|
||||
authority to mutate anything (ADR-0002's isolation rule applies to every
|
||||
code path, not just reads).
|
||||
"""
|
||||
return models.Filter(
|
||||
must=[
|
||||
models.FieldCondition(key="tenant_id", match=models.MatchValue(value=str(tenant_id))),
|
||||
models.FieldCondition(key="file_id", match=models.MatchValue(value=str(file_id))),
|
||||
models.FieldCondition(key="chunk_index", range=models.Range(gte=from_chunk_index)),
|
||||
]
|
||||
)
|
||||
|
||||
|
||||
class QdrantPointStorage:
|
||||
"""A `PointStorage` (see `src/application/ports/point_storage.py`)."""
|
||||
|
||||
def __init__(self, client: AsyncQdrantClient, *, collection: str) -> None:
|
||||
self._client = client
|
||||
self._collection = collection
|
||||
|
||||
async def upsert_points(self, points: Sequence[ChunkPoint]) -> None:
|
||||
if not points:
|
||||
return
|
||||
await self._client.upsert(
|
||||
collection_name=self._collection,
|
||||
points=[
|
||||
models.PointStruct(
|
||||
id=str(point.point_id),
|
||||
vector={
|
||||
**point.dense,
|
||||
SPARSE_VECTOR: models.SparseVector(
|
||||
indices=point.sparse.indices, values=point.sparse.values
|
||||
),
|
||||
},
|
||||
payload=point.payload,
|
||||
)
|
||||
for point in points
|
||||
],
|
||||
)
|
||||
|
||||
async def deactivate_points_from_index(
|
||||
self,
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
file_id: uuid.UUID,
|
||||
from_chunk_index: int,
|
||||
deleted_at: datetime,
|
||||
updated_by: str,
|
||||
) -> int:
|
||||
"""Soft-delete via `set_payload` — the points stay for audit (ADR-0002).
|
||||
|
||||
Counts first so the caller can report how many points the sweep
|
||||
touched; `set_payload` itself reports only an operation status.
|
||||
"""
|
||||
point_filter = _tenant_file_filter(tenant_id, file_id, from_chunk_index=from_chunk_index)
|
||||
stale = await self._client.count(
|
||||
collection_name=self._collection, count_filter=point_filter, exact=True
|
||||
)
|
||||
if stale.count == 0:
|
||||
return 0
|
||||
|
||||
await self._client.set_payload(
|
||||
collection_name=self._collection,
|
||||
payload={
|
||||
"is_active": False,
|
||||
"deleted_at": deleted_at.isoformat(),
|
||||
"updated_at": deleted_at.isoformat(),
|
||||
"updated_by": updated_by,
|
||||
},
|
||||
points=models.FilterSelector(filter=point_filter),
|
||||
)
|
||||
return stale.count
|
||||
@@ -1,7 +1,8 @@
|
||||
from collections.abc import AsyncIterator
|
||||
from collections.abc import AsyncIterator, Iterator
|
||||
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
import structlog
|
||||
from asgi_lifespan import LifespanManager
|
||||
from fastapi import FastAPI
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
@@ -10,6 +11,25 @@ from src.config import Settings
|
||||
from src.main import create_app
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_structlog_after_test() -> Iterator[None]:
|
||||
"""Undo any real `configure_logging()` call before the next test runs.
|
||||
|
||||
Any test that exercises the app's lifespan (directly, or via the `client`/
|
||||
`api_client` fixtures below and in `test_domains_api.py`) calls the real
|
||||
`configure_logging()`, which mutates *global* structlog/stdlib state --
|
||||
including `cache_logger_on_first_use=True`. Left in place, that setting
|
||||
silently breaks `structlog.testing.capture_logs()` in unrelated tests
|
||||
later in the same pytest process: a module-level
|
||||
`logger = structlog.get_logger(__name__)` cached under the real config no
|
||||
longer routes through `capture_logs()`'s temporary processor swap, so
|
||||
assertions on captured events see nothing (ADR-0016: isolate per test --
|
||||
this generalizes to global config mutations, not just data).
|
||||
"""
|
||||
yield
|
||||
structlog.reset_defaults()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def settings() -> Settings:
|
||||
# Every external dependency points at a closed port so unit tests never
|
||||
@@ -36,6 +56,24 @@ def app(settings: Settings) -> FastAPI:
|
||||
return create_app(settings)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_real_logging_configuration(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Prevent the app lifespan from calling the real `configure_logging()`.
|
||||
|
||||
It sets `cache_logger_on_first_use=True` (ADR-0011), which permanently
|
||||
monkeypatches the `.bind` method on whichever module-level
|
||||
`logger = structlog.get_logger(__name__)` instance is used first --
|
||||
`structlog.reset_defaults()` only resets *global* config, not that
|
||||
per-instance mutation, so real configuration leaking into one test would
|
||||
silently break `structlog.testing.capture_logs()` in every test that runs
|
||||
afterward in the same process (ADR-0016: isolate per test). Tests that
|
||||
spin up the full app via `LifespanManager` (`client`, `api_client`) are
|
||||
testing HTTP behavior, not logging output, so they don't need it for
|
||||
real.
|
||||
"""
|
||||
monkeypatch.setattr("src.bootstrap.lifespan.configure_logging", lambda *a, **k: None)
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def client(app: FastAPI) -> AsyncIterator[AsyncClient]:
|
||||
async with (
|
||||
|
||||
@@ -1,10 +1,13 @@
|
||||
"""Hand-written fakes for narrow application-owned ports (ADR-0016)."""
|
||||
|
||||
import asyncio
|
||||
import uuid
|
||||
from collections.abc import Sequence
|
||||
from dataclasses import dataclass, field
|
||||
from datetime import datetime
|
||||
|
||||
from src.application.ingestion.models import SparseVector
|
||||
from src.application.points.models import ChunkPoint
|
||||
|
||||
|
||||
@dataclass
|
||||
@@ -29,6 +32,7 @@ class FakeDenseEmbedder:
|
||||
|
||||
name: str
|
||||
dimensions: int = 4
|
||||
model_version: str = "fake-dense-v1"
|
||||
calls: list[list[str]] = field(default_factory=list)
|
||||
fail_next: bool = False
|
||||
delay_seconds: float = 0.0
|
||||
@@ -49,6 +53,7 @@ class FakeSparseEmbedder:
|
||||
"""A scripted `SparseEmbedder`. Returns an empty sparse vector per text."""
|
||||
|
||||
name: str = "sparse"
|
||||
model_version: str = "fake-sparse-v1"
|
||||
calls: list[list[str]] = field(default_factory=list)
|
||||
fail_next: bool = False
|
||||
|
||||
@@ -58,3 +63,76 @@ class FakeSparseEmbedder:
|
||||
self.fail_next = False
|
||||
raise RuntimeError("simulated embedder failure")
|
||||
return [SparseVector(indices=[], values=[]) for _ in texts]
|
||||
|
||||
|
||||
@dataclass
|
||||
class FakePointStorage:
|
||||
"""An in-memory `PointStorage`.
|
||||
|
||||
`points` is keyed by point id, so a re-upsert of the same deterministic id
|
||||
overwrites rather than accumulating — the property a test asserting "a
|
||||
retry produces no duplicate points" needs the fake to actually model.
|
||||
|
||||
`fail_on_batch` fails the Nth (0-based) upsert batch, which is how a test
|
||||
checks that the soft-delete sweep never runs after a partial failure.
|
||||
"""
|
||||
|
||||
points: dict[str, ChunkPoint] = field(default_factory=dict)
|
||||
upsert_batches: list[int] = field(default_factory=list)
|
||||
deactivate_calls: list[dict[str, object]] = field(default_factory=list)
|
||||
fail_on_batch: int | None = None
|
||||
fail_deactivate: bool = False
|
||||
max_in_flight: int = 0
|
||||
_in_flight: int = 0
|
||||
|
||||
async def upsert_points(self, points: Sequence[ChunkPoint]) -> None:
|
||||
self._in_flight += 1
|
||||
self.max_in_flight = max(self.max_in_flight, self._in_flight)
|
||||
try:
|
||||
# Yield so concurrent batches actually overlap; without this the
|
||||
# in-flight ceiling is trivially 1 and the bound goes untested.
|
||||
await asyncio.sleep(0)
|
||||
index = len(self.upsert_batches)
|
||||
self.upsert_batches.append(len(points))
|
||||
if self.fail_on_batch is not None and index == self.fail_on_batch:
|
||||
raise RuntimeError("simulated point storage failure")
|
||||
for point in points:
|
||||
self.points[str(point.point_id)] = point
|
||||
finally:
|
||||
self._in_flight -= 1
|
||||
|
||||
async def deactivate_points_from_index(
|
||||
self,
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
file_id: uuid.UUID,
|
||||
from_chunk_index: int,
|
||||
deleted_at: datetime,
|
||||
updated_by: str,
|
||||
) -> int:
|
||||
self.deactivate_calls.append(
|
||||
{
|
||||
"tenant_id": tenant_id,
|
||||
"file_id": file_id,
|
||||
"from_chunk_index": from_chunk_index,
|
||||
"updated_by": updated_by,
|
||||
}
|
||||
)
|
||||
if self.fail_deactivate:
|
||||
raise RuntimeError("simulated deactivate failure")
|
||||
|
||||
def is_stale(point: ChunkPoint) -> bool:
|
||||
chunk_index = point.payload.get("chunk_index")
|
||||
return (
|
||||
point.payload.get("file_id") == str(file_id)
|
||||
and point.payload.get("tenant_id") == str(tenant_id)
|
||||
and point.payload.get("is_active") is True
|
||||
and isinstance(chunk_index, int)
|
||||
and chunk_index >= from_chunk_index
|
||||
)
|
||||
|
||||
stale = [point for point in self.points.values() if is_stale(point)]
|
||||
for point in stale:
|
||||
point.payload["is_active"] = False
|
||||
point.payload["deleted_at"] = deleted_at.isoformat()
|
||||
return len(stale)
|
||||
|
||||
134
tests/integration/postgres/test_auth_service_logging.py
Normal file
134
tests/integration/postgres/test_auth_service_logging.py
Normal file
@@ -0,0 +1,134 @@
|
||||
"""`resolve_auth_context` emits `auth.succeeded`/`auth.failed` (ADR-0011).
|
||||
|
||||
This runs on every authenticated request, so every rejection reason needs a
|
||||
distinguishable log event -- previously none of them logged anything.
|
||||
"""
|
||||
|
||||
from collections.abc import MutableMapping
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
import structlog
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.auth.errors import InvalidApiKeyError, TenantInactiveError
|
||||
from src.application.auth.service import resolve_auth_context
|
||||
from tests.support.factories import create_api_key, create_tenant
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.postgres,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
|
||||
def _events_by_name(
|
||||
logs: list[MutableMapping[str, Any]], name: str
|
||||
) -> list[MutableMapping[str, Any]]:
|
||||
return [entry for entry in logs if entry.get("event") == name]
|
||||
|
||||
|
||||
async def test_valid_key_emits_auth_succeeded(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
_, full_key = await create_api_key(db_session, tenant=tenant, scopes=["files:write"])
|
||||
await db_session.commit()
|
||||
|
||||
with structlog.testing.capture_logs() as logs:
|
||||
auth = await resolve_auth_context(db_sessionmaker, full_key)
|
||||
|
||||
succeeded = _events_by_name(logs, "auth.succeeded")
|
||||
assert len(succeeded) == 1
|
||||
assert succeeded[0]["tenant_id"] == str(auth.tenant_id)
|
||||
assert succeeded[0]["api_key_id"] == str(auth.api_key_id)
|
||||
assert _events_by_name(logs, "auth.failed") == []
|
||||
|
||||
|
||||
async def test_malformed_token_emits_auth_failed(
|
||||
db_sessionmaker: async_sessionmaker[AsyncSession],
|
||||
) -> None:
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(InvalidApiKeyError):
|
||||
await resolve_auth_context(db_sessionmaker, "not-a-bearer-token-at-all")
|
||||
|
||||
failed = _events_by_name(logs, "auth.failed")
|
||||
assert len(failed) == 1
|
||||
assert failed[0]["reason"] == "malformed_key"
|
||||
|
||||
|
||||
async def test_wrong_secret_emits_auth_failed(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
api_key, _ = await create_api_key(db_session, tenant=tenant)
|
||||
await db_session.commit()
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(InvalidApiKeyError):
|
||||
await resolve_auth_context(db_sessionmaker, f"sk_{api_key.key_prefix}_wrong-secret")
|
||||
|
||||
failed = _events_by_name(logs, "auth.failed")
|
||||
assert len(failed) == 1
|
||||
assert failed[0]["reason"] == "unknown_key"
|
||||
|
||||
|
||||
async def test_unknown_prefix_emits_auth_failed(
|
||||
db_sessionmaker: async_sessionmaker[AsyncSession],
|
||||
) -> None:
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(InvalidApiKeyError):
|
||||
await resolve_auth_context(db_sessionmaker, "sk_doesnotexist_secret")
|
||||
|
||||
failed = _events_by_name(logs, "auth.failed")
|
||||
assert len(failed) == 1
|
||||
assert failed[0]["reason"] == "unknown_key"
|
||||
|
||||
|
||||
async def test_revoked_key_emits_auth_failed_with_key_status(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
_, full_key = await create_api_key(db_session, tenant=tenant, status="revoked")
|
||||
await db_session.commit()
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(InvalidApiKeyError):
|
||||
await resolve_auth_context(db_sessionmaker, full_key)
|
||||
|
||||
failed = _events_by_name(logs, "auth.failed")
|
||||
assert len(failed) == 1
|
||||
assert failed[0]["reason"] == "key_inactive"
|
||||
assert failed[0]["key_status"] == "revoked"
|
||||
|
||||
|
||||
async def test_suspended_tenant_emits_auth_failed(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session, status="suspended")
|
||||
_, full_key = await create_api_key(db_session, tenant=tenant)
|
||||
await db_session.commit()
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(TenantInactiveError):
|
||||
await resolve_auth_context(db_sessionmaker, full_key)
|
||||
|
||||
failed = _events_by_name(logs, "auth.failed")
|
||||
assert len(failed) == 1
|
||||
assert failed[0]["reason"] == "tenant_inactive"
|
||||
assert failed[0]["tenant_id"] == str(tenant.id)
|
||||
|
||||
|
||||
async def test_auth_failed_never_logs_the_secret(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
"""ADR-0011's redaction rule: never log plaintext API keys. `key_prefix`
|
||||
is the non-secret lookup portion (same distinction `ApiKey.key_prefix`
|
||||
makes); the secret itself must not appear in any field's value.
|
||||
"""
|
||||
tenant = await create_tenant(db_session)
|
||||
api_key, _ = await create_api_key(db_session, tenant=tenant)
|
||||
await db_session.commit()
|
||||
wrong_secret = "definitely-not-the-real-secret"
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(InvalidApiKeyError):
|
||||
await resolve_auth_context(db_sessionmaker, f"sk_{api_key.key_prefix}_{wrong_secret}")
|
||||
|
||||
failed = _events_by_name(logs, "auth.failed")
|
||||
assert len(failed) == 1
|
||||
assert wrong_secret not in str(failed[0])
|
||||
193
tests/integration/postgres/test_domains_api.py
Normal file
193
tests/integration/postgres/test_domains_api.py
Normal file
@@ -0,0 +1,193 @@
|
||||
"""`/v1/domains` over HTTP, against real Postgres (ADR-0008, ADR-0009).
|
||||
|
||||
The property worth testing at this layer is the scope boundary: an upload key
|
||||
must not be able to create domains, or the allowlist stops preventing anything.
|
||||
"""
|
||||
|
||||
from collections.abc import AsyncIterator
|
||||
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
from asgi_lifespan import LifespanManager
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.bootstrap.dependencies import get_sessionmaker
|
||||
from src.config import Settings
|
||||
from src.main import create_app
|
||||
from tests.support.factories import create_api_key, create_tenant, create_tenant_domain
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.postgres,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
|
||||
@pytest_asyncio.fixture(loop_scope="session")
|
||||
async def api_client(
|
||||
settings: Settings, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> AsyncIterator[AsyncClient]:
|
||||
"""The real app, with only the database swapped for the test's session
|
||||
factory -- so routing, auth, scopes, and the error envelope are exercised.
|
||||
"""
|
||||
app = create_app(settings)
|
||||
app.dependency_overrides[get_sessionmaker] = lambda: db_sessionmaker
|
||||
async with (
|
||||
LifespanManager(app) as manager,
|
||||
AsyncClient(transport=ASGITransport(app=manager.app), base_url="http://test") as client,
|
||||
):
|
||||
yield client
|
||||
|
||||
|
||||
def _auth(token: str) -> dict[str, str]:
|
||||
return {"Authorization": f"Bearer {token}"}
|
||||
|
||||
|
||||
async def test_create_domain_returns_201_and_lists_it(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
_, token = await create_api_key(
|
||||
db_session, tenant=tenant, scopes=["domains:read", "domains:write"]
|
||||
)
|
||||
await db_session.commit()
|
||||
|
||||
created = await api_client.post(
|
||||
"/v1/domains",
|
||||
json={"domain": "fire", "display_name": "Fire insurance"},
|
||||
headers=_auth(token),
|
||||
)
|
||||
listed = await api_client.get("/v1/domains", headers=_auth(token))
|
||||
|
||||
assert created.status_code == 201
|
||||
assert created.json()["domain"] == "fire"
|
||||
assert [item["domain"] for item in listed.json()["domains"]] == ["fire"]
|
||||
|
||||
|
||||
async def test_create_domain_requires_the_domains_write_scope(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
"""An upload key creating domains would defeat the allowlist entirely."""
|
||||
tenant = await create_tenant(db_session)
|
||||
_, token = await create_api_key(db_session, tenant=tenant, scopes=["files:write"])
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.post(
|
||||
"/v1/domains",
|
||||
json={"domain": "fire", "display_name": "Fire"},
|
||||
headers=_auth(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 403
|
||||
assert response.json()["error"]["code"] == "missing_scope"
|
||||
|
||||
|
||||
async def test_list_domains_never_shows_another_tenants_domains(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
owner = await create_tenant(db_session)
|
||||
other = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=owner, domain="fire")
|
||||
_, other_token = await create_api_key(db_session, tenant=other, scopes=["domains:read"])
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.get("/v1/domains", headers=_auth(other_token))
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["domains"] == []
|
||||
|
||||
|
||||
async def test_create_domain_rejects_a_duplicate_with_409(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
_, token = await create_api_key(db_session, tenant=tenant, scopes=["domains:write"])
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.post(
|
||||
"/v1/domains",
|
||||
json={"domain": "fire", "display_name": "Fire"},
|
||||
headers=_auth(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 409
|
||||
assert response.json()["error"]["code"] == "conflict"
|
||||
|
||||
|
||||
async def test_create_domain_rejects_a_malformed_key(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
_, token = await create_api_key(db_session, tenant=tenant, scopes=["domains:write"])
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.post(
|
||||
"/v1/domains",
|
||||
json={"domain": "Fire Insurance!", "display_name": "Fire"},
|
||||
headers=_auth(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 422
|
||||
|
||||
|
||||
async def test_patch_domain_cannot_rename_the_key(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
"""`domain` is not part of the update schema -- it is denormalized into
|
||||
every point payload, so renaming it is a migration, not an edit.
|
||||
"""
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
_, token = await create_api_key(
|
||||
db_session, tenant=tenant, scopes=["domains:read", "domains:write"]
|
||||
)
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.patch(
|
||||
"/v1/domains/fire",
|
||||
json={"display_name": "Fire & perils", "domain": "renamed"},
|
||||
headers=_auth(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["domain"] == "fire"
|
||||
assert response.json()["display_name"] == "Fire & perils"
|
||||
|
||||
|
||||
async def test_delete_domain_disables_it_without_removing_it(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
_, token = await create_api_key(
|
||||
db_session, tenant=tenant, scopes=["domains:read", "domains:write"]
|
||||
)
|
||||
await db_session.commit()
|
||||
|
||||
deleted = await api_client.delete("/v1/domains/fire", headers=_auth(token))
|
||||
default_list = await api_client.get("/v1/domains", headers=_auth(token))
|
||||
full_list = await api_client.get(
|
||||
"/v1/domains", params={"include_disabled": True}, headers=_auth(token)
|
||||
)
|
||||
|
||||
assert deleted.status_code == 200
|
||||
assert deleted.json()["status"] == "disabled"
|
||||
assert default_list.json()["domains"] == []
|
||||
assert [item["domain"] for item in full_list.json()["domains"]] == ["fire"]
|
||||
|
||||
|
||||
async def test_patch_unknown_domain_returns_400(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
_, token = await create_api_key(db_session, tenant=tenant, scopes=["domains:write"])
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.patch(
|
||||
"/v1/domains/absent", json={"display_name": "x"}, headers=_auth(token)
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert response.json()["error"]["code"] == "unknown_domain"
|
||||
184
tests/integration/postgres/test_domains_service.py
Normal file
184
tests/integration/postgres/test_domains_service.py
Normal file
@@ -0,0 +1,184 @@
|
||||
"""Tenant-domain management and the upload-time allowlist (ADR-0009)."""
|
||||
|
||||
import pytest
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.domains import (
|
||||
DomainAlreadyExistsError,
|
||||
UnknownDomainError,
|
||||
create_domain,
|
||||
ensure_domain_allowed,
|
||||
list_domains,
|
||||
set_domain_status,
|
||||
update_domain,
|
||||
)
|
||||
from tests.support.factories import create_tenant, create_tenant_domain
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.postgres,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_passes_for_a_registered_active_domain(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
|
||||
await ensure_domain_allowed(db_session, tenant_id=tenant.id, domain="fire")
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_rejects_an_unregistered_domain(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
"""The typo case: `fier` must not silently become a new Qdrant partition."""
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
|
||||
with pytest.raises(UnknownDomainError, match="fier"):
|
||||
await ensure_domain_allowed(db_session, tenant_id=tenant.id, domain="fier")
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_rejects_a_disabled_domain(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire", status="disabled")
|
||||
|
||||
with pytest.raises(UnknownDomainError, match="disabled"):
|
||||
await ensure_domain_allowed(db_session, tenant_id=tenant.id, domain="fire")
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_rejects_another_tenants_domain(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
"""Domain lists are per-tenant; one tenant's `fire` is not another's."""
|
||||
owner = await create_tenant(db_session)
|
||||
other = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=owner, domain="fire")
|
||||
|
||||
with pytest.raises(UnknownDomainError):
|
||||
await ensure_domain_allowed(db_session, tenant_id=other.id, domain="fire")
|
||||
|
||||
|
||||
async def test_tenants_hold_independent_domain_sets_of_different_sizes(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
big = await create_tenant(db_session)
|
||||
small = await create_tenant(db_session)
|
||||
for index in range(14):
|
||||
await create_tenant_domain(db_session, tenant=big, domain=f"line-{index:02d}")
|
||||
for index in range(6):
|
||||
await create_tenant_domain(db_session, tenant=small, domain=f"line-{index:02d}")
|
||||
await db_session.commit()
|
||||
|
||||
assert len(await list_domains(db_sessionmaker, tenant_id=big.id)) == 14
|
||||
assert len(await list_domains(db_sessionmaker, tenant_id=small.id)) == 6
|
||||
|
||||
|
||||
async def test_create_domain_then_upload_is_allowed(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await db_session.commit()
|
||||
|
||||
created = await create_domain(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="car", display_name="Car insurance"
|
||||
)
|
||||
|
||||
assert created.domain == "car"
|
||||
assert created.status == "active"
|
||||
async with db_sessionmaker() as session:
|
||||
await ensure_domain_allowed(session, tenant_id=tenant.id, domain="car")
|
||||
|
||||
|
||||
async def test_create_domain_rejects_a_duplicate_key_for_the_same_tenant(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
with pytest.raises(DomainAlreadyExistsError):
|
||||
await create_domain(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="fire", display_name="Fire again"
|
||||
)
|
||||
|
||||
|
||||
async def test_create_domain_allows_the_same_key_for_different_tenants(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
first = await create_tenant(db_session)
|
||||
second = await create_tenant(db_session)
|
||||
await db_session.commit()
|
||||
|
||||
await create_domain(db_sessionmaker, tenant_id=first.id, domain="fire", display_name="Fire")
|
||||
await create_domain(db_sessionmaker, tenant_id=second.id, domain="fire", display_name="Fire")
|
||||
|
||||
assert len(await list_domains(db_sessionmaker, tenant_id=first.id)) == 1
|
||||
assert len(await list_domains(db_sessionmaker, tenant_id=second.id)) == 1
|
||||
|
||||
|
||||
async def test_update_domain_changes_only_the_display_name(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
updated = await update_domain(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="fire", display_name="Fire & perils"
|
||||
)
|
||||
|
||||
assert updated.display_name == "Fire & perils"
|
||||
# The key is immutable: it is denormalized into every point payload.
|
||||
assert updated.domain == "fire"
|
||||
|
||||
|
||||
async def test_disabling_a_domain_blocks_new_uploads_without_deleting_it(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
disabled = await set_domain_status(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="fire", status="disabled"
|
||||
)
|
||||
|
||||
assert disabled.status == "disabled"
|
||||
async with db_sessionmaker() as session:
|
||||
with pytest.raises(UnknownDomainError):
|
||||
await ensure_domain_allowed(session, tenant_id=tenant.id, domain="fire")
|
||||
# Still there, just hidden from the default listing.
|
||||
assert await list_domains(db_sessionmaker, tenant_id=tenant.id) == []
|
||||
assert len(await list_domains(db_sessionmaker, tenant_id=tenant.id, include_disabled=True)) == 1
|
||||
|
||||
|
||||
async def test_re_enabling_a_domain_restores_uploads(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire", status="disabled")
|
||||
await db_session.commit()
|
||||
|
||||
await set_domain_status(db_sessionmaker, tenant_id=tenant.id, domain="fire", status="active")
|
||||
|
||||
async with db_sessionmaker() as session:
|
||||
await ensure_domain_allowed(session, tenant_id=tenant.id, domain="fire")
|
||||
|
||||
|
||||
async def test_update_domain_rejects_another_tenants_domain(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
owner = await create_tenant(db_session)
|
||||
other = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=owner, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
with pytest.raises(UnknownDomainError):
|
||||
await update_domain(
|
||||
db_sessionmaker, tenant_id=other.id, domain="fire", display_name="hijacked"
|
||||
)
|
||||
142
tests/integration/postgres/test_domains_service_logging.py
Normal file
142
tests/integration/postgres/test_domains_service_logging.py
Normal file
@@ -0,0 +1,142 @@
|
||||
"""`domains/service.py` emits log events for the allowlist rejection and every
|
||||
mutation (ADR-0011). `ensure_domain_allowed` is the one that matters most: it
|
||||
runs before any `ingestion_jobs` row exists, so without its own log a rejected
|
||||
upload leaves no operational trace at all.
|
||||
"""
|
||||
|
||||
from collections.abc import MutableMapping
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
import structlog
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.domains import (
|
||||
DomainAlreadyExistsError,
|
||||
UnknownDomainError,
|
||||
create_domain,
|
||||
ensure_domain_allowed,
|
||||
set_domain_status,
|
||||
update_domain,
|
||||
)
|
||||
from tests.support.factories import create_tenant, create_tenant_domain
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.postgres,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
|
||||
def _events_by_name(
|
||||
logs: list[MutableMapping[str, Any]], name: str
|
||||
) -> list[MutableMapping[str, Any]]:
|
||||
return [entry for entry in logs if entry.get("event") == name]
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_logs_nothing_when_the_domain_is_active(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
|
||||
with structlog.testing.capture_logs() as logs:
|
||||
await ensure_domain_allowed(db_session, tenant_id=tenant.id, domain="fire")
|
||||
|
||||
assert _events_by_name(logs, "domain.rejected") == []
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_logs_rejection_for_an_unregistered_domain(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(UnknownDomainError):
|
||||
await ensure_domain_allowed(db_session, tenant_id=tenant.id, domain="fier")
|
||||
|
||||
rejected = _events_by_name(logs, "domain.rejected")
|
||||
assert len(rejected) == 1
|
||||
assert rejected[0]["reason"] == "unregistered"
|
||||
assert rejected[0]["domain"] == "fier"
|
||||
assert rejected[0]["tenant_id"] == str(tenant.id)
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_logs_rejection_for_a_disabled_domain(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire", status="disabled")
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(UnknownDomainError):
|
||||
await ensure_domain_allowed(db_session, tenant_id=tenant.id, domain="fire")
|
||||
|
||||
rejected = _events_by_name(logs, "domain.rejected")
|
||||
assert len(rejected) == 1
|
||||
assert rejected[0]["reason"] == "disabled"
|
||||
|
||||
|
||||
async def test_create_domain_emits_domain_created(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await db_session.commit()
|
||||
|
||||
with structlog.testing.capture_logs() as logs:
|
||||
await create_domain(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="car", display_name="Car insurance"
|
||||
)
|
||||
|
||||
created = _events_by_name(logs, "domain.created")
|
||||
assert len(created) == 1
|
||||
assert created[0]["domain"] == "car"
|
||||
|
||||
|
||||
async def test_create_domain_duplicate_does_not_emit_domain_created(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(DomainAlreadyExistsError):
|
||||
await create_domain(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="fire", display_name="Fire again"
|
||||
)
|
||||
|
||||
assert _events_by_name(logs, "domain.created") == []
|
||||
|
||||
|
||||
async def test_update_domain_emits_domain_updated(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
with structlog.testing.capture_logs() as logs:
|
||||
await update_domain(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="fire", display_name="Fire & perils"
|
||||
)
|
||||
|
||||
updated = _events_by_name(logs, "domain.updated")
|
||||
assert len(updated) == 1
|
||||
assert updated[0]["domain"] == "fire"
|
||||
|
||||
|
||||
async def test_set_domain_status_emits_domain_status_changed(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
with structlog.testing.capture_logs() as logs:
|
||||
await set_domain_status(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="fire", status="disabled"
|
||||
)
|
||||
|
||||
changed = _events_by_name(logs, "domain.status_changed")
|
||||
assert len(changed) == 1
|
||||
assert changed[0]["domain"] == "fire"
|
||||
assert changed[0]["status"] == "disabled"
|
||||
@@ -10,6 +10,7 @@ pytestmark = [
|
||||
|
||||
EXPECTED_TABLES = {
|
||||
"tenants",
|
||||
"tenant_domains",
|
||||
"api_keys",
|
||||
"source_files",
|
||||
"ingestion_jobs",
|
||||
@@ -25,3 +26,17 @@ async def test_migrations_create_schema_from_empty_database(postgres_engine: Asy
|
||||
)
|
||||
|
||||
assert EXPECTED_TABLES.issubset(set(table_names))
|
||||
|
||||
|
||||
async def test_tenant_domains_enforces_one_row_per_tenant_and_key(
|
||||
postgres_engine: AsyncEngine,
|
||||
) -> None:
|
||||
"""The unique constraint is what stops the same domain being registered
|
||||
twice for a tenant while still letting two tenants share a key.
|
||||
"""
|
||||
async with postgres_engine.connect() as connection:
|
||||
constraints = await connection.run_sync(
|
||||
lambda sync_conn: inspect(sync_conn).get_unique_constraints("tenant_domains")
|
||||
)
|
||||
|
||||
assert any(constraint["column_names"] == ["tenant_id", "domain"] for constraint in constraints)
|
||||
|
||||
@@ -11,14 +11,25 @@ from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.auth.context import AuthContext
|
||||
from src.application.domains import UnknownDomainError
|
||||
from src.application.files.models import UploadResult
|
||||
from src.application.files.upload import upload_source_file
|
||||
from src.application.ingestion.errors import IngestionAtCapacityError, IngestionTimeoutError
|
||||
from src.application.ingestion.errors import (
|
||||
IngestionAtCapacityError,
|
||||
IngestionTimeoutError,
|
||||
PointIndexingError,
|
||||
)
|
||||
from src.application.ports.object_storage import ObjectStorage
|
||||
from src.config import ChunkingSettings, IngestionSettings
|
||||
from src.application.ports.point_storage import PointStorage
|
||||
from src.config import ChunkingSettings, IngestionSettings, QdrantSettings
|
||||
from src.infrastructure.postgres.models.ingestion_job import IngestionJob
|
||||
from tests.fakes import FakeDenseEmbedder, FakeObjectStorage, FakeSparseEmbedder
|
||||
from tests.support.factories import create_api_key, create_tenant
|
||||
from tests.fakes import (
|
||||
FakeDenseEmbedder,
|
||||
FakeObjectStorage,
|
||||
FakePointStorage,
|
||||
FakeSparseEmbedder,
|
||||
)
|
||||
from tests.support.factories import create_api_key, create_tenant, create_tenant_domain
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
@@ -34,6 +45,7 @@ async def _upload(
|
||||
sessionmaker: async_sessionmaker[AsyncSession],
|
||||
storage: ObjectStorage,
|
||||
auth: AuthContext,
|
||||
point_storage: PointStorage | None = None,
|
||||
domain: str = "general",
|
||||
filename: str = "report.csv",
|
||||
data: bytes = _CSV_BYTES,
|
||||
@@ -41,12 +53,14 @@ async def _upload(
|
||||
return await upload_source_file(
|
||||
sessionmaker=sessionmaker,
|
||||
storage=storage,
|
||||
point_storage=point_storage if point_storage is not None else FakePointStorage(),
|
||||
auth=auth,
|
||||
domain=domain,
|
||||
filename=filename,
|
||||
data=data,
|
||||
ingestion_settings=IngestionSettings(),
|
||||
chunking_settings=ChunkingSettings(),
|
||||
qdrant_settings=QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
concurrency_limiter=Semaphore(2),
|
||||
dense_embedders=[
|
||||
@@ -57,9 +71,12 @@ async def _upload(
|
||||
)
|
||||
|
||||
|
||||
async def _auth_for(db_session: AsyncSession) -> AuthContext:
|
||||
async def _auth_for(db_session: AsyncSession, *, domain: str = "general") -> AuthContext:
|
||||
tenant = await create_tenant(db_session)
|
||||
api_key, _ = await create_api_key(db_session, tenant=tenant)
|
||||
# Uploads reject an unregistered domain (ADR-0009), so register the one the
|
||||
# helper below uploads to.
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain=domain)
|
||||
await db_session.commit()
|
||||
return AuthContext(
|
||||
tenant_id=tenant.id,
|
||||
@@ -83,7 +100,7 @@ async def test_upload_source_file_commits_running_job_before_storage_write(
|
||||
result = await _upload(sessionmaker=db_sessionmaker, storage=storage, auth=auth)
|
||||
|
||||
assert result.status == "succeeded"
|
||||
assert result.chunks_indexed == 0
|
||||
assert result.chunks_indexed == 1
|
||||
assert result.is_new_attempt
|
||||
|
||||
async with db_sessionmaker() as verify_session:
|
||||
@@ -170,12 +187,14 @@ async def test_upload_source_file_timeout_writes_failed_job_and_raises(
|
||||
await upload_source_file(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
point_storage=FakePointStorage(),
|
||||
auth=auth,
|
||||
domain="general",
|
||||
filename="report.csv",
|
||||
data=_CSV_BYTES,
|
||||
ingestion_settings=IngestionSettings(timeout_seconds=0.05),
|
||||
chunking_settings=ChunkingSettings(),
|
||||
qdrant_settings=QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
concurrency_limiter=Semaphore(2),
|
||||
dense_embedders=[slow_embedder, FakeDenseEmbedder(name="dense_openai")],
|
||||
@@ -212,12 +231,14 @@ async def test_upload_source_file_at_capacity_rejects_before_any_job_row(
|
||||
await upload_source_file(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
point_storage=FakePointStorage(),
|
||||
auth=auth,
|
||||
domain="general",
|
||||
filename="report.csv",
|
||||
data=_CSV_BYTES,
|
||||
ingestion_settings=IngestionSettings(),
|
||||
chunking_settings=ChunkingSettings(),
|
||||
qdrant_settings=QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
concurrency_limiter=concurrency_limiter,
|
||||
dense_embedders=[
|
||||
@@ -238,3 +259,193 @@ async def test_upload_source_file_at_capacity_rejects_before_any_job_row(
|
||||
.all()
|
||||
)
|
||||
assert jobs == []
|
||||
|
||||
|
||||
async def test_upload_source_file_indexes_points_and_records_real_counters(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
"""The Phase 5 payoff: a successful upload is searchable, and the job row
|
||||
says how many points it wrote rather than a hardcoded zero.
|
||||
"""
|
||||
auth = await _auth_for(db_session)
|
||||
point_storage = FakePointStorage()
|
||||
|
||||
result = await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
auth=auth,
|
||||
point_storage=point_storage,
|
||||
data=b"name,value\nfirst,1\nsecond,2\nthird,3\n",
|
||||
)
|
||||
|
||||
assert result.status == "succeeded"
|
||||
assert result.chunks_indexed == len(point_storage.points)
|
||||
assert result.chunks_indexed > 0
|
||||
|
||||
async with db_sessionmaker() as verify_session:
|
||||
job = await verify_session.get(IngestionJob, result.ingestion_job_id)
|
||||
assert job is not None
|
||||
assert job.points_created == result.chunks_indexed
|
||||
# An upsert cannot tell an insert from an overwrite, so this stays 0.
|
||||
assert job.points_updated == 0
|
||||
|
||||
|
||||
async def test_upload_source_file_indexes_points_under_the_authenticated_tenant(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
auth = await _auth_for(db_session, domain="fire")
|
||||
point_storage = FakePointStorage()
|
||||
|
||||
result = await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
auth=auth,
|
||||
point_storage=point_storage,
|
||||
domain="fire",
|
||||
)
|
||||
|
||||
payloads = [point.payload for point in point_storage.points.values()]
|
||||
assert payloads
|
||||
for payload in payloads:
|
||||
assert payload["tenant_id"] == str(auth.tenant_id)
|
||||
assert payload["domain"] == "fire"
|
||||
assert payload["file_id"] == str(result.file_id)
|
||||
assert payload["created_by"] == f"api_key:{auth.api_key_id}"
|
||||
|
||||
|
||||
async def test_upload_source_file_index_failure_writes_terminal_failed_job(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
auth = await _auth_for(db_session)
|
||||
point_storage = FakePointStorage(fail_on_batch=0)
|
||||
|
||||
with pytest.raises(PointIndexingError):
|
||||
await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
auth=auth,
|
||||
point_storage=point_storage,
|
||||
)
|
||||
|
||||
async with db_sessionmaker() as verify_session:
|
||||
jobs = (await verify_session.execute(select(IngestionJob))).scalars().all()
|
||||
job = next(job for job in jobs if job.tenant_id == auth.tenant_id)
|
||||
assert job.status == "failed"
|
||||
assert job.error_code == "index_failed"
|
||||
assert job.completed_at is not None
|
||||
|
||||
|
||||
async def test_upload_source_file_failed_index_does_not_soft_delete_existing_points(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
"""A failed attempt must never remove content from a working index."""
|
||||
auth = await _auth_for(db_session)
|
||||
point_storage = FakePointStorage()
|
||||
await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
auth=auth,
|
||||
point_storage=point_storage,
|
||||
data=b"name,value\nfirst,1\nsecond,2\n",
|
||||
)
|
||||
# fail_on_batch indexes into upsert_batches, which accumulates across
|
||||
# uploads -- reset it so "batch 0" means the retry's first batch.
|
||||
point_storage.deactivate_calls.clear()
|
||||
point_storage.upsert_batches.clear()
|
||||
point_storage.fail_on_batch = 0
|
||||
|
||||
with pytest.raises(PointIndexingError):
|
||||
await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
auth=auth,
|
||||
point_storage=point_storage,
|
||||
data=b"name,value\nonly,1\n",
|
||||
)
|
||||
|
||||
assert point_storage.deactivate_calls == []
|
||||
assert all(point.payload["is_active"] is True for point in point_storage.points.values())
|
||||
|
||||
|
||||
async def test_upload_source_file_retry_after_index_failure_produces_no_duplicate_points(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
auth = await _auth_for(db_session)
|
||||
point_storage = FakePointStorage(fail_on_batch=0)
|
||||
|
||||
with pytest.raises(PointIndexingError):
|
||||
await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
auth=auth,
|
||||
point_storage=point_storage,
|
||||
)
|
||||
|
||||
point_storage.fail_on_batch = None
|
||||
retry = await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
auth=auth,
|
||||
point_storage=point_storage,
|
||||
)
|
||||
|
||||
assert retry.status == "succeeded"
|
||||
assert len(point_storage.points) == retry.chunks_indexed
|
||||
|
||||
async with db_sessionmaker() as verify_session:
|
||||
jobs = (await verify_session.execute(select(IngestionJob))).scalars().all()
|
||||
tenant_jobs = [job for job in jobs if job.tenant_id == auth.tenant_id]
|
||||
# A terminal job never returns to `running` (ADR-0017); the retry is a new row.
|
||||
assert len(tenant_jobs) == 2
|
||||
assert {job.status for job in tenant_jobs} == {"failed", "succeeded"}
|
||||
|
||||
|
||||
async def test_upload_source_file_rejects_an_unregistered_domain_before_any_write(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
"""A typo'd domain must fail loudly, not create a new Qdrant partition
|
||||
whose contents retrieval never queries (ADR-0009).
|
||||
"""
|
||||
auth = await _auth_for(db_session, domain="fire")
|
||||
storage = FakeObjectStorage()
|
||||
point_storage = FakePointStorage()
|
||||
|
||||
with pytest.raises(UnknownDomainError, match="fier"):
|
||||
await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=storage,
|
||||
auth=auth,
|
||||
point_storage=point_storage,
|
||||
domain="fier",
|
||||
)
|
||||
|
||||
# Nothing was written anywhere: no object, no points, and no job row.
|
||||
assert storage.objects == {}
|
||||
assert point_storage.points == {}
|
||||
async with db_sessionmaker() as verify_session:
|
||||
jobs = (await verify_session.execute(select(IngestionJob))).scalars().all()
|
||||
assert [job for job in jobs if job.tenant_id == auth.tenant_id] == []
|
||||
|
||||
|
||||
async def test_upload_source_file_rejects_a_disabled_domain(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
api_key, _ = await create_api_key(db_session, tenant=tenant)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire", status="disabled")
|
||||
await db_session.commit()
|
||||
auth = AuthContext(
|
||||
tenant_id=tenant.id,
|
||||
tenant_slug=tenant.slug,
|
||||
api_key_id=api_key.id,
|
||||
scopes=frozenset({"files:write"}),
|
||||
actor_type="backend",
|
||||
)
|
||||
|
||||
with pytest.raises(UnknownDomainError, match="disabled"):
|
||||
await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
auth=auth,
|
||||
domain="fire",
|
||||
)
|
||||
|
||||
223
tests/integration/postgres/test_upload_service_logging.py
Normal file
223
tests/integration/postgres/test_upload_service_logging.py
Normal file
@@ -0,0 +1,223 @@
|
||||
"""`upload_source_file` emits `ingestion.job.*` log events (ADR-0011).
|
||||
|
||||
Every failure branch funnels through `_mark_job_failed`, so this asserts the
|
||||
log event once per branch rather than re-testing the Postgres job-row
|
||||
behavior already covered in `test_upload_service.py`. Uses
|
||||
`structlog.testing.capture_logs()`, which captures events independent of
|
||||
whichever handlers/renderers happen to be configured in this process.
|
||||
"""
|
||||
|
||||
from collections.abc import MutableMapping
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
import structlog
|
||||
from anyio import CapacityLimiter, Semaphore
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.auth.context import AuthContext
|
||||
from src.application.files.upload import upload_source_file
|
||||
from src.application.ingestion.errors import (
|
||||
EmbedderError,
|
||||
IngestionTimeoutError,
|
||||
PointIndexingError,
|
||||
)
|
||||
from src.config import ChunkingSettings, IngestionSettings, QdrantSettings
|
||||
from tests.fakes import (
|
||||
FakeDenseEmbedder,
|
||||
FakeObjectStorage,
|
||||
FakePointStorage,
|
||||
FakeSparseEmbedder,
|
||||
)
|
||||
from tests.support.factories import create_api_key, create_tenant, create_tenant_domain
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.postgres,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
_CSV_BYTES = b"name,value\nfirst,1\n"
|
||||
|
||||
|
||||
async def _auth_for(db_session: AsyncSession, *, domain: str = "general") -> AuthContext:
|
||||
tenant = await create_tenant(db_session)
|
||||
api_key, _ = await create_api_key(db_session, tenant=tenant)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain=domain)
|
||||
await db_session.commit()
|
||||
return AuthContext(
|
||||
tenant_id=tenant.id,
|
||||
tenant_slug=tenant.slug,
|
||||
api_key_id=api_key.id,
|
||||
scopes=frozenset({"files:write"}),
|
||||
actor_type="backend",
|
||||
)
|
||||
|
||||
|
||||
def _events_by_name(
|
||||
logs: list[MutableMapping[str, Any]], name: str
|
||||
) -> list[MutableMapping[str, Any]]:
|
||||
return [entry for entry in logs if entry.get("event") == name]
|
||||
|
||||
|
||||
async def test_successful_upload_emits_started_and_completed_events(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
auth = await _auth_for(db_session)
|
||||
|
||||
with structlog.testing.capture_logs() as logs:
|
||||
result = await upload_source_file(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
point_storage=FakePointStorage(),
|
||||
auth=auth,
|
||||
domain="general",
|
||||
filename="report.csv",
|
||||
data=_CSV_BYTES,
|
||||
ingestion_settings=IngestionSettings(),
|
||||
chunking_settings=ChunkingSettings(),
|
||||
qdrant_settings=QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
concurrency_limiter=Semaphore(2),
|
||||
dense_embedders=[
|
||||
FakeDenseEmbedder(name="dense_nomic"),
|
||||
FakeDenseEmbedder(name="dense_openai"),
|
||||
],
|
||||
sparse_embedder=FakeSparseEmbedder(),
|
||||
)
|
||||
|
||||
started = _events_by_name(logs, "ingestion.job.started")
|
||||
completed = _events_by_name(logs, "ingestion.job.completed")
|
||||
assert len(started) == 1
|
||||
assert started[0]["tenant_id"] == str(auth.tenant_id)
|
||||
assert started[0]["ingestion_job_id"] == str(result.ingestion_job_id)
|
||||
assert len(completed) == 1
|
||||
assert completed[0]["points_upserted"] == result.chunks_indexed
|
||||
assert _events_by_name(logs, "ingestion.job.failed") == []
|
||||
|
||||
|
||||
async def test_storage_failure_emits_ingestion_job_failed(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
auth = await _auth_for(db_session)
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(OSError):
|
||||
await upload_source_file(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(fail_next=True),
|
||||
point_storage=FakePointStorage(),
|
||||
auth=auth,
|
||||
domain="general",
|
||||
filename="report.csv",
|
||||
data=_CSV_BYTES,
|
||||
ingestion_settings=IngestionSettings(),
|
||||
chunking_settings=ChunkingSettings(),
|
||||
qdrant_settings=QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
concurrency_limiter=Semaphore(2),
|
||||
dense_embedders=[
|
||||
FakeDenseEmbedder(name="dense_nomic"),
|
||||
FakeDenseEmbedder(name="dense_openai"),
|
||||
],
|
||||
sparse_embedder=FakeSparseEmbedder(),
|
||||
)
|
||||
|
||||
failed = _events_by_name(logs, "ingestion.job.failed")
|
||||
assert len(failed) == 1
|
||||
assert failed[0]["error_code"] == "storage_upload_failed"
|
||||
assert failed[0]["tenant_id"] == str(auth.tenant_id)
|
||||
|
||||
|
||||
async def test_embedding_failure_emits_ingestion_job_failed(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
"""Previously silent: embedding_failed reached Postgres but never logged."""
|
||||
auth = await _auth_for(db_session)
|
||||
failing_embedder = FakeDenseEmbedder(name="dense_nomic", fail_next=True)
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(EmbedderError):
|
||||
await upload_source_file(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
point_storage=FakePointStorage(),
|
||||
auth=auth,
|
||||
domain="general",
|
||||
filename="report.csv",
|
||||
data=_CSV_BYTES,
|
||||
ingestion_settings=IngestionSettings(),
|
||||
chunking_settings=ChunkingSettings(),
|
||||
qdrant_settings=QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
concurrency_limiter=Semaphore(2),
|
||||
dense_embedders=[failing_embedder, FakeDenseEmbedder(name="dense_openai")],
|
||||
sparse_embedder=FakeSparseEmbedder(),
|
||||
)
|
||||
|
||||
failed = _events_by_name(logs, "ingestion.job.failed")
|
||||
assert len(failed) == 1
|
||||
assert failed[0]["error_code"] == "embedding_failed"
|
||||
|
||||
|
||||
async def test_index_failure_emits_ingestion_job_failed(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
"""Previously silent: index_failed reached Postgres but never logged."""
|
||||
auth = await _auth_for(db_session)
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(PointIndexingError):
|
||||
await upload_source_file(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
point_storage=FakePointStorage(fail_on_batch=0),
|
||||
auth=auth,
|
||||
domain="general",
|
||||
filename="report.csv",
|
||||
data=_CSV_BYTES,
|
||||
ingestion_settings=IngestionSettings(),
|
||||
chunking_settings=ChunkingSettings(),
|
||||
qdrant_settings=QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
concurrency_limiter=Semaphore(2),
|
||||
dense_embedders=[
|
||||
FakeDenseEmbedder(name="dense_nomic"),
|
||||
FakeDenseEmbedder(name="dense_openai"),
|
||||
],
|
||||
sparse_embedder=FakeSparseEmbedder(),
|
||||
)
|
||||
|
||||
failed = _events_by_name(logs, "ingestion.job.failed")
|
||||
assert len(failed) == 1
|
||||
assert failed[0]["error_code"] == "index_failed"
|
||||
|
||||
|
||||
async def test_timeout_emits_ingestion_job_failed_not_a_duplicate_event(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
"""The old ad-hoc `files.upload.timeout` log is gone -- `_mark_job_failed`
|
||||
is now the single place a failure is logged, so there is exactly one
|
||||
`ingestion.job.failed` event, not two events for one failure.
|
||||
"""
|
||||
auth = await _auth_for(db_session)
|
||||
slow_embedder = FakeDenseEmbedder(name="dense_nomic", delay_seconds=10)
|
||||
|
||||
with structlog.testing.capture_logs() as logs, pytest.raises(IngestionTimeoutError):
|
||||
await upload_source_file(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
point_storage=FakePointStorage(),
|
||||
auth=auth,
|
||||
domain="general",
|
||||
filename="report.csv",
|
||||
data=_CSV_BYTES,
|
||||
ingestion_settings=IngestionSettings(timeout_seconds=0.05),
|
||||
chunking_settings=ChunkingSettings(),
|
||||
qdrant_settings=QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
concurrency_limiter=Semaphore(2),
|
||||
dense_embedders=[slow_embedder, FakeDenseEmbedder(name="dense_openai")],
|
||||
sparse_embedder=FakeSparseEmbedder(),
|
||||
)
|
||||
|
||||
failed = _events_by_name(logs, "ingestion.job.failed")
|
||||
assert len(failed) == 1
|
||||
assert failed[0]["error_code"] == "timeout"
|
||||
52
tests/integration/qdrant/conftest.py
Normal file
52
tests/integration/qdrant/conftest.py
Normal file
@@ -0,0 +1,52 @@
|
||||
import uuid
|
||||
from collections.abc import AsyncIterator, Iterator
|
||||
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
from qdrant_client import AsyncQdrantClient
|
||||
from testcontainers.community.qdrant import QdrantContainer
|
||||
|
||||
from src.config import QdrantSettings
|
||||
from src.infrastructure.qdrant.client import create_client
|
||||
|
||||
# Pinned to match the `qdrant-client` major/minor in pyproject.toml. The
|
||||
# testcontainers default image trails it far enough that the client emits an
|
||||
# incompatibility warning, and testing against a version we do not deploy is
|
||||
# the wrong signal anyway.
|
||||
_QDRANT_IMAGE = "qdrant/qdrant:v1.19.0"
|
||||
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
def qdrant_container() -> Iterator[QdrantContainer]:
|
||||
with QdrantContainer(image=_QDRANT_IMAGE) as container:
|
||||
yield container
|
||||
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
def qdrant_url(qdrant_container: QdrantContainer) -> str:
|
||||
"""The container's REST URL, pinned to IPv4.
|
||||
|
||||
Same gotcha as postgres_url/minio_settings (see their conftests):
|
||||
testcontainers reports the host as `localhost`, which resolves to `::1`
|
||||
first, but Docker publishes the mapped port on IPv4 only. The IPv6 SYN is
|
||||
dropped rather than refused, so the client hangs until its timeout instead
|
||||
of falling back to the second address -- the connection does not fail, it
|
||||
hangs.
|
||||
"""
|
||||
host = qdrant_container.get_container_host_ip().replace("localhost", "127.0.0.1")
|
||||
return f"http://{host}:{qdrant_container.get_exposed_port(6333)}"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def qdrant_settings(qdrant_url: str) -> QdrantSettings:
|
||||
"""Settings naming a collection unique to this test (ADR-0016 isolation)."""
|
||||
return QdrantSettings(url=qdrant_url, collection=f"chunks_{uuid.uuid4().hex}")
|
||||
|
||||
|
||||
@pytest_asyncio.fixture(loop_scope="session")
|
||||
async def qdrant_client(qdrant_settings: QdrantSettings) -> AsyncIterator[AsyncQdrantClient]:
|
||||
client = create_client(qdrant_settings)
|
||||
try:
|
||||
yield client
|
||||
finally:
|
||||
await client.close()
|
||||
114
tests/integration/qdrant/test_collection.py
Normal file
114
tests/integration/qdrant/test_collection.py
Normal file
@@ -0,0 +1,114 @@
|
||||
"""`ensure_chunks_collection` against a real Qdrant (ADR-0001).
|
||||
|
||||
The assertions that matter most here are the ones for schema properties that
|
||||
fail *silently* in production: the sparse `modifier=idf` and the pinned dense
|
||||
dimensions.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from qdrant_client import AsyncQdrantClient, models
|
||||
|
||||
from src.config import QdrantSettings
|
||||
from src.infrastructure.qdrant.collection import (
|
||||
CollectionSchemaMismatchError,
|
||||
ensure_chunks_collection,
|
||||
)
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.qdrant,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
|
||||
async def test_ensure_chunks_collection_creates_all_four_named_vectors(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
created = await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
|
||||
assert created is True
|
||||
info = await qdrant_client.get_collection(qdrant_settings.collection)
|
||||
vectors = info.config.params.vectors
|
||||
assert isinstance(vectors, dict)
|
||||
assert vectors["dense_nomic"].size == 768
|
||||
assert vectors["dense_openai"].size == 3072
|
||||
assert vectors["late_interaction"].size == 128
|
||||
assert vectors["late_interaction"].multivector_config is not None
|
||||
assert vectors["late_interaction"].on_disk is True
|
||||
|
||||
|
||||
async def test_ensure_chunks_collection_sets_the_sparse_idf_modifier(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
"""Without this, Qdrant applies no IDF and lexical retrieval silently
|
||||
degrades -- no error, no warning (ADR-0005).
|
||||
"""
|
||||
await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
|
||||
info = await qdrant_client.get_collection(qdrant_settings.collection)
|
||||
sparse = info.config.params.sparse_vectors
|
||||
assert sparse is not None
|
||||
assert sparse["sparse"].modifier == models.Modifier.IDF
|
||||
|
||||
|
||||
async def test_ensure_chunks_collection_creates_the_payload_indexes(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
|
||||
info = await qdrant_client.get_collection(qdrant_settings.collection)
|
||||
schema = info.payload_schema
|
||||
assert set(schema) >= {
|
||||
"tenant_id",
|
||||
"domain",
|
||||
"file_id",
|
||||
"order_id",
|
||||
"previous_chunk_id",
|
||||
"next_chunk_id",
|
||||
}
|
||||
# order_id must be numeric: Qdrant's Range/order_by reject keyword payloads.
|
||||
assert schema["order_id"].data_type == models.PayloadSchemaType.FLOAT
|
||||
assert schema["tenant_id"].data_type == models.PayloadSchemaType.KEYWORD
|
||||
|
||||
|
||||
async def test_ensure_chunks_collection_is_idempotent(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
assert await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
|
||||
assert not await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
|
||||
|
||||
async def test_ensure_chunks_collection_rejects_a_mismatched_existing_collection(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
"""A wrong-dimension collection must fail loudly, not be silently accepted."""
|
||||
await qdrant_client.create_collection(
|
||||
collection_name=qdrant_settings.collection,
|
||||
vectors_config={
|
||||
"dense_nomic": models.VectorParams(size=384, distance=models.Distance.COSINE),
|
||||
"dense_openai": models.VectorParams(size=3072, distance=models.Distance.COSINE),
|
||||
"late_interaction": models.VectorParams(size=128, distance=models.Distance.COSINE),
|
||||
},
|
||||
sparse_vectors_config={"sparse": models.SparseVectorParams(modifier=models.Modifier.IDF)},
|
||||
)
|
||||
|
||||
with pytest.raises(CollectionSchemaMismatchError, match="768"):
|
||||
await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
|
||||
|
||||
async def test_ensure_chunks_collection_rejects_a_collection_without_the_idf_modifier(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
await qdrant_client.create_collection(
|
||||
collection_name=qdrant_settings.collection,
|
||||
vectors_config={
|
||||
"dense_nomic": models.VectorParams(size=768, distance=models.Distance.COSINE),
|
||||
"dense_openai": models.VectorParams(size=3072, distance=models.Distance.COSINE),
|
||||
"late_interaction": models.VectorParams(size=128, distance=models.Distance.COSINE),
|
||||
},
|
||||
sparse_vectors_config={"sparse": models.SparseVectorParams()},
|
||||
)
|
||||
|
||||
with pytest.raises(CollectionSchemaMismatchError, match="idf"):
|
||||
await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
182
tests/integration/qdrant/test_points.py
Normal file
182
tests/integration/qdrant/test_points.py
Normal file
@@ -0,0 +1,182 @@
|
||||
"""`QdrantPointStorage` against a real Qdrant (ADR-0001, ADR-0002).
|
||||
|
||||
Reads here go through the raw client rather than the port: `PointStorage` is
|
||||
deliberately write-only, because point reads are plan 002's `/v1/points`
|
||||
surface. The reads below are the test's own verification, not a preview of an
|
||||
API this slice ships.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from datetime import UTC, datetime
|
||||
|
||||
import pytest
|
||||
from qdrant_client import AsyncQdrantClient, models
|
||||
|
||||
from src.application.ingestion.chunking import chunk_id_for
|
||||
from src.application.ingestion.models import SparseVector
|
||||
from src.application.points.models import ChunkPoint
|
||||
from src.config import QdrantSettings
|
||||
from src.infrastructure.qdrant.collection import (
|
||||
DENSE_NOMIC_DIMENSIONS,
|
||||
DENSE_OPENAI_DIMENSIONS,
|
||||
ensure_chunks_collection,
|
||||
)
|
||||
from src.infrastructure.qdrant.points import QdrantPointStorage
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.qdrant,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
|
||||
def _point(tenant_id: uuid.UUID, file_id: uuid.UUID, chunk_index: int) -> ChunkPoint:
|
||||
return ChunkPoint(
|
||||
point_id=chunk_id_for(file_id, chunk_index),
|
||||
dense={
|
||||
"dense_nomic": [0.1] * DENSE_NOMIC_DIMENSIONS,
|
||||
"dense_openai": [0.2] * DENSE_OPENAI_DIMENSIONS,
|
||||
},
|
||||
sparse=SparseVector(indices=[1, 2], values=[0.5, 0.25]),
|
||||
payload={
|
||||
"tenant_id": str(tenant_id),
|
||||
"domain": "fire",
|
||||
"file_id": str(file_id),
|
||||
"chunk_id": str(chunk_id_for(file_id, chunk_index)),
|
||||
"chunk_index": chunk_index,
|
||||
"order_id": float(chunk_index + 1),
|
||||
"content": f"chunk {chunk_index}",
|
||||
"is_active": True,
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
async def _storage(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> QdrantPointStorage:
|
||||
await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
return QdrantPointStorage(qdrant_client, collection=qdrant_settings.collection)
|
||||
|
||||
|
||||
async def _count_for_tenant(
|
||||
client: AsyncQdrantClient, collection: str, tenant_id: uuid.UUID
|
||||
) -> int:
|
||||
result = await client.count(
|
||||
collection_name=collection,
|
||||
count_filter=models.Filter(
|
||||
must=[
|
||||
models.FieldCondition(
|
||||
key="tenant_id", match=models.MatchValue(value=str(tenant_id))
|
||||
)
|
||||
]
|
||||
),
|
||||
exact=True,
|
||||
)
|
||||
return result.count
|
||||
|
||||
|
||||
async def test_upsert_points_stores_points_readable_under_the_owning_tenant_filter(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
tenant_id, file_id = uuid.uuid4(), uuid.uuid4()
|
||||
|
||||
await storage.upsert_points([_point(tenant_id, file_id, i) for i in range(3)])
|
||||
|
||||
assert await _count_for_tenant(qdrant_client, qdrant_settings.collection, tenant_id) == 3
|
||||
|
||||
|
||||
async def test_upsert_points_are_invisible_to_another_tenants_filter(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
"""The Qdrant-level form of "cross-tenant access finds nothing" (ADR-0002)."""
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
owner, other, file_id = uuid.uuid4(), uuid.uuid4(), uuid.uuid4()
|
||||
|
||||
await storage.upsert_points([_point(owner, file_id, i) for i in range(3)])
|
||||
|
||||
assert await _count_for_tenant(qdrant_client, qdrant_settings.collection, other) == 0
|
||||
|
||||
|
||||
async def test_upsert_points_is_idempotent_for_deterministic_ids(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
tenant_id, file_id = uuid.uuid4(), uuid.uuid4()
|
||||
points = [_point(tenant_id, file_id, i) for i in range(4)]
|
||||
|
||||
await storage.upsert_points(points)
|
||||
await storage.upsert_points(points)
|
||||
|
||||
assert await _count_for_tenant(qdrant_client, qdrant_settings.collection, tenant_id) == 4
|
||||
|
||||
|
||||
async def test_deactivate_points_from_index_soft_deletes_only_the_tail(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
tenant_id, file_id = uuid.uuid4(), uuid.uuid4()
|
||||
await storage.upsert_points([_point(tenant_id, file_id, i) for i in range(5)])
|
||||
|
||||
deactivated = await storage.deactivate_points_from_index(
|
||||
tenant_id=tenant_id,
|
||||
file_id=file_id,
|
||||
from_chunk_index=2,
|
||||
deleted_at=datetime.now(UTC),
|
||||
updated_by="api_key:test",
|
||||
)
|
||||
|
||||
assert deactivated == 3
|
||||
records, _ = await qdrant_client.scroll(
|
||||
collection_name=qdrant_settings.collection,
|
||||
scroll_filter=models.Filter(
|
||||
must=[models.FieldCondition(key="file_id", match=models.MatchValue(value=str(file_id)))]
|
||||
),
|
||||
limit=10,
|
||||
with_payload=True,
|
||||
)
|
||||
by_index = {
|
||||
record.payload["chunk_index"]: record.payload["is_active"]
|
||||
for record in records
|
||||
if record.payload is not None
|
||||
}
|
||||
assert by_index == {0: True, 1: True, 2: False, 3: False, 4: False}
|
||||
# Soft delete, not removal -- the points stay for audit (ADR-0002).
|
||||
assert await _count_for_tenant(qdrant_client, qdrant_settings.collection, tenant_id) == 5
|
||||
|
||||
|
||||
async def test_deactivate_points_from_index_does_not_touch_another_tenants_points(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
"""A file_id alone is never authority to mutate (ADR-0002)."""
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
owner, other, file_id = uuid.uuid4(), uuid.uuid4(), uuid.uuid4()
|
||||
await storage.upsert_points([_point(owner, file_id, i) for i in range(3)])
|
||||
|
||||
deactivated = await storage.deactivate_points_from_index(
|
||||
tenant_id=other,
|
||||
file_id=file_id,
|
||||
from_chunk_index=0,
|
||||
deleted_at=datetime.now(UTC),
|
||||
updated_by="api_key:intruder",
|
||||
)
|
||||
|
||||
assert deactivated == 0
|
||||
|
||||
|
||||
async def test_deactivate_points_from_index_returns_zero_when_nothing_is_stale(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
tenant_id, file_id = uuid.uuid4(), uuid.uuid4()
|
||||
await storage.upsert_points([_point(tenant_id, file_id, i) for i in range(3)])
|
||||
|
||||
deactivated = await storage.deactivate_points_from_index(
|
||||
tenant_id=tenant_id,
|
||||
file_id=file_id,
|
||||
from_chunk_index=3,
|
||||
deleted_at=datetime.now(UTC),
|
||||
updated_by="api_key:test",
|
||||
)
|
||||
|
||||
assert deactivated == 0
|
||||
45
tests/integration/qdrant/test_readiness.py
Normal file
45
tests/integration/qdrant/test_readiness.py
Normal file
@@ -0,0 +1,45 @@
|
||||
"""`/readyz`'s Qdrant probe against a real Qdrant.
|
||||
|
||||
The case worth an integration test is the one a fake cannot produce
|
||||
convincingly: Qdrant is up and answering, but the collection the deployment
|
||||
step was supposed to create is not there.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
|
||||
import pytest
|
||||
from qdrant_client import AsyncQdrantClient
|
||||
|
||||
from src.config import QdrantSettings
|
||||
from src.infrastructure.qdrant.client import ping
|
||||
from src.infrastructure.qdrant.collection import ensure_chunks_collection
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.qdrant,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
|
||||
async def test_ping_is_not_ready_when_the_collection_was_never_bootstrapped(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
"""A healthy Qdrant with no collection is *not* ready: uploads would 502."""
|
||||
assert await ping(qdrant_client, 5.0, collection=qdrant_settings.collection) is False
|
||||
|
||||
|
||||
async def test_ping_is_ready_after_bootstrap(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
|
||||
assert await ping(qdrant_client, 5.0, collection=qdrant_settings.collection) is True
|
||||
|
||||
|
||||
async def test_ping_is_not_ready_for_a_different_collection_name(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
"""A misconfigured QDRANT_COLLECTION is as unready as a missing one."""
|
||||
await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
|
||||
assert await ping(qdrant_client, 5.0, collection=f"absent_{uuid.uuid4().hex}") is False
|
||||
@@ -12,6 +12,7 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from src.application.auth.keys import generate_api_key, hash_secret
|
||||
from src.infrastructure.postgres.models.api_key import ApiKey
|
||||
from src.infrastructure.postgres.models.tenant import Tenant
|
||||
from src.infrastructure.postgres.models.tenant_domain import TenantDomain
|
||||
|
||||
|
||||
async def create_tenant(
|
||||
@@ -47,3 +48,27 @@ async def create_api_key(
|
||||
session.add(api_key)
|
||||
await session.flush()
|
||||
return api_key, full_key
|
||||
|
||||
|
||||
async def create_tenant_domain(
|
||||
session: AsyncSession,
|
||||
*,
|
||||
tenant: Tenant,
|
||||
domain: str = "general",
|
||||
status: str = "active",
|
||||
) -> TenantDomain:
|
||||
"""Register a domain so an upload to it passes the allowlist check.
|
||||
|
||||
Uploads reject an unregistered domain (ADR-0009), so any test that uploads
|
||||
needs one of these.
|
||||
"""
|
||||
tenant_domain = TenantDomain(
|
||||
id=uuid.uuid4(),
|
||||
tenant_id=tenant.id,
|
||||
domain=domain,
|
||||
display_name=domain,
|
||||
status=status,
|
||||
)
|
||||
session.add(tenant_domain)
|
||||
await session.flush()
|
||||
return tenant_domain
|
||||
|
||||
0
tests/unit/application/points/__init__.py
Normal file
0
tests/unit/application/points/__init__.py
Normal file
227
tests/unit/application/points/test_indexing.py
Normal file
227
tests/unit/application/points/test_indexing.py
Normal file
@@ -0,0 +1,227 @@
|
||||
"""`index_chunks`: payload correctness, bounded batching, and the ordering
|
||||
that keeps a failed attempt from damaging a working index (ADR-0001, ADR-0017).
|
||||
"""
|
||||
|
||||
import uuid
|
||||
|
||||
import pytest
|
||||
from anyio import CapacityLimiter
|
||||
|
||||
from src.application.ingestion.chunking import chunk_id_for
|
||||
from src.application.ingestion.errors import PointIndexingError
|
||||
from src.application.ingestion.models import Chunk, ContentType, EmbeddedChunk, SparseVector
|
||||
from src.application.points import index_chunks
|
||||
from src.config import QdrantSettings
|
||||
from tests.fakes import FakeDenseEmbedder, FakePointStorage, FakeSparseEmbedder
|
||||
|
||||
pytestmark = [pytest.mark.unit, pytest.mark.asyncio]
|
||||
|
||||
_TENANT_ID = uuid.UUID("11111111-1111-1111-1111-111111111111")
|
||||
_FILE_ID = uuid.UUID("22222222-2222-2222-2222-222222222222")
|
||||
_API_KEY_ID = uuid.UUID("33333333-3333-3333-3333-333333333333")
|
||||
_ACTOR = f"api_key:{_API_KEY_ID}"
|
||||
|
||||
|
||||
def _embedded(count: int) -> list[EmbeddedChunk]:
|
||||
chunks = [
|
||||
Chunk(
|
||||
chunk_id=chunk_id_for(_FILE_ID, index),
|
||||
chunk_index=index,
|
||||
order_id=float(index + 1),
|
||||
content=f"chunk {index}",
|
||||
content_type=ContentType.PARAGRAPH,
|
||||
token_count=2,
|
||||
character_count=7,
|
||||
)
|
||||
for index in range(count)
|
||||
]
|
||||
for position, chunk in enumerate(chunks):
|
||||
if position > 0:
|
||||
chunk.previous_chunk_id = chunks[position - 1].chunk_id
|
||||
if position < len(chunks) - 1:
|
||||
chunk.next_chunk_id = chunks[position + 1].chunk_id
|
||||
return [
|
||||
EmbeddedChunk(
|
||||
chunk=chunk,
|
||||
dense={"dense_nomic": [0.0] * 4, "dense_openai": [1.0] * 4},
|
||||
sparse=SparseVector(indices=[7], values=[0.5]),
|
||||
)
|
||||
for chunk in chunks
|
||||
]
|
||||
|
||||
|
||||
async def _index(
|
||||
storage: FakePointStorage,
|
||||
*,
|
||||
count: int = 3,
|
||||
settings: QdrantSettings | None = None,
|
||||
domain: str = "fire",
|
||||
):
|
||||
return await index_chunks(
|
||||
_embedded(count),
|
||||
storage=storage,
|
||||
tenant_id=_TENANT_ID,
|
||||
domain=domain,
|
||||
file_id=_FILE_ID,
|
||||
source_filename="policy.docx",
|
||||
source_type="docx",
|
||||
actor=_ACTOR,
|
||||
dense_embedders=[
|
||||
FakeDenseEmbedder(name="dense_nomic", model_version="nomic-embed-text-v2-moe"),
|
||||
FakeDenseEmbedder(name="dense_openai", model_version="text-embedding-3-large"),
|
||||
],
|
||||
sparse_embedder=FakeSparseEmbedder(model_version="bm25-fa_norm_stop"),
|
||||
settings=settings or QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
)
|
||||
|
||||
|
||||
async def test_index_chunks_writes_every_adr_0001_payload_field() -> None:
|
||||
storage = FakePointStorage()
|
||||
|
||||
await _index(storage, count=3)
|
||||
|
||||
payload = storage.points[str(chunk_id_for(_FILE_ID, 1))].payload
|
||||
assert payload["tenant_id"] == str(_TENANT_ID)
|
||||
assert payload["domain"] == "fire"
|
||||
assert payload["file_id"] == str(_FILE_ID)
|
||||
assert payload["chunk_id"] == str(chunk_id_for(_FILE_ID, 1))
|
||||
assert payload["content"] == "chunk 1"
|
||||
assert payload["content_type"] == "paragraph"
|
||||
assert payload["source_filename"] == "policy.docx"
|
||||
assert payload["source_type"] == "docx"
|
||||
assert payload["order_id"] == 2.0
|
||||
assert payload["chunk_index"] == 1
|
||||
assert payload["previous_chunk_id"] == str(chunk_id_for(_FILE_ID, 0))
|
||||
assert payload["next_chunk_id"] == str(chunk_id_for(_FILE_ID, 2))
|
||||
assert payload["is_active"] is True
|
||||
assert payload["deleted_at"] is None
|
||||
assert payload["created_by"] == _ACTOR
|
||||
assert payload["updated_by"] == _ACTOR
|
||||
assert payload["version"] == 1
|
||||
assert payload["created_at"] == payload["updated_at"]
|
||||
assert isinstance(payload["content_hash"], str)
|
||||
# Sorted, so wiring order cannot change the value (ADR-0001).
|
||||
assert payload["embedding_model_version"] == (
|
||||
"bm25-fa_norm_stop+nomic-embed-text-v2-moe+text-embedding-3-large"
|
||||
)
|
||||
|
||||
|
||||
async def test_index_chunks_uses_null_neighbours_at_the_file_ends() -> None:
|
||||
storage = FakePointStorage()
|
||||
|
||||
await _index(storage, count=3)
|
||||
|
||||
assert storage.points[str(chunk_id_for(_FILE_ID, 0))].payload["previous_chunk_id"] is None
|
||||
assert storage.points[str(chunk_id_for(_FILE_ID, 2))].payload["next_chunk_id"] is None
|
||||
|
||||
|
||||
async def test_index_chunks_derives_tenant_and_domain_from_the_caller_not_the_chunk() -> None:
|
||||
"""Tenant identity is server-derived; nothing in the chunk can assert it."""
|
||||
storage = FakePointStorage()
|
||||
|
||||
await _index(storage, count=1, domain="car")
|
||||
|
||||
payload = storage.points[str(chunk_id_for(_FILE_ID, 0))].payload
|
||||
assert payload["tenant_id"] == str(_TENANT_ID)
|
||||
assert payload["domain"] == "car"
|
||||
|
||||
|
||||
async def test_index_chunks_uses_deterministic_point_ids() -> None:
|
||||
storage = FakePointStorage()
|
||||
|
||||
result = await _index(storage, count=4)
|
||||
|
||||
assert result.points_upserted == 4
|
||||
assert set(storage.points) == {str(chunk_id_for(_FILE_ID, i)) for i in range(4)}
|
||||
|
||||
|
||||
async def test_index_chunks_repeated_run_produces_no_duplicate_points() -> None:
|
||||
storage = FakePointStorage()
|
||||
|
||||
await _index(storage, count=4)
|
||||
await _index(storage, count=4)
|
||||
|
||||
assert len(storage.points) == 4
|
||||
|
||||
|
||||
async def test_index_chunks_batches_at_the_configured_size() -> None:
|
||||
storage = FakePointStorage()
|
||||
settings = QdrantSettings(upsert_batch_size=2, upsert_concurrency=4)
|
||||
|
||||
await _index(storage, count=5, settings=settings)
|
||||
|
||||
assert storage.upsert_batches == [2, 2, 1]
|
||||
|
||||
|
||||
async def test_index_chunks_bounds_in_flight_batches() -> None:
|
||||
storage = FakePointStorage()
|
||||
settings = QdrantSettings(upsert_batch_size=1, upsert_concurrency=2)
|
||||
|
||||
await _index(storage, count=8, settings=settings)
|
||||
|
||||
assert len(storage.upsert_batches) == 8
|
||||
assert storage.max_in_flight <= 2
|
||||
|
||||
|
||||
async def test_index_chunks_soft_deletes_only_points_past_the_new_chunk_count() -> None:
|
||||
storage = FakePointStorage()
|
||||
await _index(storage, count=5)
|
||||
|
||||
result = await _index(storage, count=2)
|
||||
|
||||
assert result.points_soft_deleted == 3
|
||||
assert storage.points[str(chunk_id_for(_FILE_ID, 1))].payload["is_active"] is True
|
||||
assert storage.points[str(chunk_id_for(_FILE_ID, 2))].payload["is_active"] is False
|
||||
assert storage.points[str(chunk_id_for(_FILE_ID, 4))].payload["is_active"] is False
|
||||
|
||||
|
||||
async def test_index_chunks_does_not_soft_delete_when_an_upsert_batch_fails() -> None:
|
||||
"""A failed attempt must never remove content from a working index."""
|
||||
storage = FakePointStorage()
|
||||
await _index(storage, count=5)
|
||||
storage.deactivate_calls.clear()
|
||||
storage.fail_on_batch = 1
|
||||
|
||||
with pytest.raises(PointIndexingError):
|
||||
await _index(storage, count=2, settings=QdrantSettings(upsert_batch_size=1))
|
||||
|
||||
assert storage.deactivate_calls == []
|
||||
assert all(point.payload["is_active"] is True for point in storage.points.values())
|
||||
|
||||
|
||||
async def test_index_chunks_raises_point_indexing_error_when_a_batch_fails() -> None:
|
||||
storage = FakePointStorage(fail_on_batch=0)
|
||||
|
||||
with pytest.raises(PointIndexingError, match="upserting"):
|
||||
await _index(storage, count=2)
|
||||
|
||||
|
||||
async def test_index_chunks_raises_point_indexing_error_when_the_sweep_fails() -> None:
|
||||
storage = FakePointStorage(fail_deactivate=True)
|
||||
|
||||
with pytest.raises(PointIndexingError, match="soft-deleting"):
|
||||
await _index(storage, count=2)
|
||||
|
||||
|
||||
async def test_index_chunks_on_empty_input_touches_no_storage() -> None:
|
||||
storage = FakePointStorage()
|
||||
|
||||
result = await index_chunks(
|
||||
[],
|
||||
storage=storage,
|
||||
tenant_id=_TENANT_ID,
|
||||
domain="fire",
|
||||
file_id=_FILE_ID,
|
||||
source_filename="empty.csv",
|
||||
source_type="csv",
|
||||
actor=_ACTOR,
|
||||
dense_embedders=[FakeDenseEmbedder(name="dense_nomic")],
|
||||
sparse_embedder=FakeSparseEmbedder(),
|
||||
settings=QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
)
|
||||
|
||||
assert result.points_upserted == 0
|
||||
assert storage.upsert_batches == []
|
||||
assert storage.deactivate_calls == []
|
||||
@@ -37,6 +37,7 @@ class _TrackingDenseEmbedder:
|
||||
"""
|
||||
|
||||
name: str
|
||||
model_version: str = "stub-v1"
|
||||
dimensions: int = 3
|
||||
batches: list[list[str]] = field(default_factory=list)
|
||||
in_flight: int = 0
|
||||
@@ -54,6 +55,7 @@ class _TrackingDenseEmbedder:
|
||||
@dataclass
|
||||
class _FailingDenseEmbedder:
|
||||
name: str
|
||||
model_version: str = "stub-v1"
|
||||
|
||||
async def embed_batch(self, texts: Sequence[str]) -> list[list[float]]:
|
||||
raise RuntimeError("boom")
|
||||
@@ -62,6 +64,7 @@ class _FailingDenseEmbedder:
|
||||
@dataclass
|
||||
class _StubSparseEmbedder:
|
||||
name: str = "sparse"
|
||||
model_version: str = "stub-sparse-v1"
|
||||
calls: list[list[str]] = field(default_factory=list)
|
||||
|
||||
def embed_batch(self, texts: Sequence[str], *, query: bool = False) -> list[SparseVector]:
|
||||
@@ -72,6 +75,7 @@ class _StubSparseEmbedder:
|
||||
@dataclass
|
||||
class _FailingSparseEmbedder:
|
||||
name: str = "sparse"
|
||||
model_version: str = "stub-sparse-v1"
|
||||
|
||||
def embed_batch(self, texts: Sequence[str], *, query: bool = False) -> list[SparseVector]:
|
||||
raise RuntimeError("boom")
|
||||
|
||||
0
tests/unit/infrastructure/observability/__init__.py
Normal file
0
tests/unit/infrastructure/observability/__init__.py
Normal file
106
tests/unit/infrastructure/observability/test_logging.py
Normal file
106
tests/unit/infrastructure/observability/test_logging.py
Normal file
@@ -0,0 +1,106 @@
|
||||
"""Dual-sink logging config and the static environment processor (ADR-0011).
|
||||
|
||||
`configure_logging` mutates global logging state (`logging.config.dictConfig`,
|
||||
`structlog.configure`), so these tests assert on the *handler configuration it
|
||||
builds*, plus one end-to-end capture per sink, rather than trying to isolate
|
||||
process-global state across tests.
|
||||
"""
|
||||
|
||||
import json
|
||||
import logging
|
||||
|
||||
import pytest
|
||||
import structlog
|
||||
|
||||
from src.config import AppLimitSettings, LoggingSettings
|
||||
from src.infrastructure.observability.logging import configure_logging
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
# Cross-test pollution from configure_logging() (cache_logger_on_first_use=True
|
||||
# etc.) is reset by the autouse fixture in tests/conftest.py after every test,
|
||||
# not just this module's -- these tests call the real configure_logging()
|
||||
# directly and need the same cleanup any other test does.
|
||||
|
||||
|
||||
def test_configure_logging_without_file_path_registers_only_console(tmp_path) -> None:
|
||||
configure_logging(LoggingSettings(file_path=None), AppLimitSettings())
|
||||
|
||||
root = logging.getLogger()
|
||||
handler_names = {type(h).__name__ for h in root.handlers}
|
||||
assert handler_names == {"StreamHandler"}
|
||||
|
||||
|
||||
def test_configure_logging_with_file_path_adds_a_rotating_file_handler(tmp_path) -> None:
|
||||
log_file = tmp_path / "app.log"
|
||||
|
||||
configure_logging(
|
||||
LoggingSettings(file_path=str(log_file), file_max_bytes=1024, file_backup_count=2),
|
||||
AppLimitSettings(),
|
||||
)
|
||||
|
||||
root = logging.getLogger()
|
||||
handler_names = {type(h).__name__ for h in root.handlers}
|
||||
assert handler_names == {"StreamHandler", "RotatingFileHandler"}
|
||||
|
||||
|
||||
def test_file_sink_is_json_even_when_console_is_not(tmp_path, capsys) -> None:
|
||||
"""The two sinks render independently: console stays human-readable while
|
||||
the file stays JSON, regardless of LOG_JSON_FORMAT.
|
||||
"""
|
||||
log_file = tmp_path / "app.log"
|
||||
|
||||
configure_logging(
|
||||
LoggingSettings(json_format=False, file_path=str(log_file)),
|
||||
AppLimitSettings(),
|
||||
)
|
||||
structlog.get_logger("test").info("logging.dual_sink.test", widget_id="abc123")
|
||||
|
||||
console_output = capsys.readouterr().out
|
||||
file_output = log_file.read_text().strip()
|
||||
|
||||
# Console: human-readable, not parseable JSON.
|
||||
with pytest.raises(json.JSONDecodeError):
|
||||
json.loads(console_output)
|
||||
assert "logging.dual_sink.test" in console_output
|
||||
|
||||
# File: valid JSON with the same event.
|
||||
file_event = json.loads(file_output)
|
||||
assert file_event["event"] == "logging.dual_sink.test"
|
||||
assert file_event["widget_id"] == "abc123"
|
||||
|
||||
|
||||
def test_every_event_carries_env_and_service_version(tmp_path, capsys) -> None:
|
||||
"""Static environment context, not a per-request contextvar -- it must
|
||||
show up on an event with no request in flight.
|
||||
"""
|
||||
configure_logging(
|
||||
LoggingSettings(json_format=True, file_path=None),
|
||||
AppLimitSettings(env="staging", service_version="abc1234"),
|
||||
)
|
||||
|
||||
structlog.get_logger("test").info("logging.env_context.test")
|
||||
|
||||
event = json.loads(capsys.readouterr().out.strip())
|
||||
assert event["env"] == "staging"
|
||||
assert event["service_version"] == "abc1234"
|
||||
|
||||
|
||||
def test_env_context_survives_request_contextvar_clearing(tmp_path, capsys) -> None:
|
||||
"""The bug this design avoids: if env/service_version were bound via
|
||||
contextvars before a request, RequestIdMiddleware's clear_contextvars()
|
||||
would wipe them. They must still appear after a clear.
|
||||
"""
|
||||
configure_logging(
|
||||
LoggingSettings(json_format=True, file_path=None),
|
||||
AppLimitSettings(env="prod", service_version="v42"),
|
||||
)
|
||||
|
||||
structlog.contextvars.clear_contextvars()
|
||||
structlog.contextvars.bind_contextvars(request_id="req-1")
|
||||
structlog.get_logger("test").info("logging.post_clear.test")
|
||||
|
||||
event = json.loads(capsys.readouterr().out.strip())
|
||||
assert event["env"] == "prod"
|
||||
assert event["service_version"] == "v42"
|
||||
assert event["request_id"] == "req-1"
|
||||
Reference in New Issue
Block a user