diff --git a/Dockerfile b/Dockerfile index 6d3d75f3..c918d99f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -31,6 +31,10 @@ RUN groupadd --system --gid 10001 episodic \ && chown -R episodic:episodic /app COPY --from=builder --chown=episodic:episodic /app/.venv /app/.venv +# Pricing snapshots are read at boot; the packaged default path resolves +# inside site-packages, so ship the repository snapshots and point +# PRICING_SNAPSHOT_DIRECTORY at this location in deployment configuration. +COPY --chown=episodic:episodic config/pricing-snapshots /app/config/pricing-snapshots USER 10001:10001 diff --git a/Makefile b/Makefile index 76089938..6300edc8 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,11 @@ DF12_FUTURE_ANNOTATIONS = $(DF12_PYLINT_BASE) --enable=C9112 \ AMBRLEAKS = $(UV_ENV) $(UV) tool run --python $(DF12_PYTHON) \ --from '$(DF12_PYTHON_LINTS)' ambrleaks SKYLOS_VERSION = 4.33.2 -SKYLOS = $(UV_ENV) $(UV) tool run --from 'skylos==$(SKYLOS_VERSION)' skylos \ +# Pin the tool interpreter: Skylos parses sources with its own runtime `ast`, +# so an older default Python misreads the project's 3.14 syntax. +SKYLOS_CLI = $(UV_ENV) $(UV) tool run --python 3.14 \ + --from 'skylos==$(SKYLOS_VERSION)' skylos +SKYLOS = $(SKYLOS_CLI) \ --config-file pyproject.toml SKYLOS_PRODUCTION_TARGETS ?= alembic episodic openai_test_types.py @@ -111,12 +115,18 @@ lint: check-architecture ## Run linters $(AMBRLEAKS) tests $(SKYLOS) $(SKYLOS_PRODUCTION_TARGETS) --category dead_code --gate --format concise --no-upload --no-provenance --no-grep-verify -skylos-allow: export SKYLOS_NAME = $(value NAME) -skylos-allow: export SKYLOS_REASON = $(value REASON) +# Accept NAME/REASON only from the make command line so ambient environment +# variables (for example, a host NAME export) cannot leak into the whitelist. +cli_value = $(if $(filter command line,$(origin $(1))),$(value $(1))) + +skylos-allow: export SKYLOS_NAME = $(call cli_value,NAME) +skylos-allow: export SKYLOS_REASON = $(call cli_value,REASON) skylos-allow: ## Document one named Skylos exception, not an entry point @test -n "$${SKYLOS_NAME}" || { printf "Error: NAME is required for a named whitelist exception\\n" >&2; exit 2; } @test -n "$${SKYLOS_REASON}" || { printf "Error: REASON is required for a named whitelist exception\\n" >&2; exit 2; } - $(SKYLOS) whitelist "$${SKYLOS_NAME}" --reason "$${SKYLOS_REASON}" + # The whitelist subcommand must be skylos's first argument; global + # options such as --config-file make the main parser treat it as a path. + $(SKYLOS_CLI) whitelist "$${SKYLOS_NAME}" --reason "$${SKYLOS_REASON}" check-architecture: build ## Check hexagonal architecture import boundaries $(UV_ENV) $(UV) run hecate check diff --git a/charts/episodic/templates/deployment.yaml b/charts/episodic/templates/deployment.yaml index b4c34b84..50c91635 100644 --- a/charts/episodic/templates/deployment.yaml +++ b/charts/episodic/templates/deployment.yaml @@ -70,6 +70,14 @@ spec: resources: {{- toYaml . | nindent 12 }} {{- end }} + {{- with .Values.volumeMounts }} + volumeMounts: + {{- toYaml . | nindent 12 }} + {{- end }} + {{- with .Values.volumes }} + volumes: + {{- toYaml . | nindent 8 }} + {{- end }} {{- with .Values.nodeSelector }} nodeSelector: {{- toYaml . | nindent 8 }} diff --git a/charts/episodic/values.local.yaml b/charts/episodic/values.local.yaml index d4f739b2..a72146e8 100644 --- a/charts/episodic/values.local.yaml +++ b/charts/episodic/values.local.yaml @@ -5,6 +5,28 @@ image: config: EPISODIC_ENV: local + # The HTTP runtime refuses to boot without these settings, so the local + # preview must supply them. The object store writes beneath /tmp, which + # the emptyDir volume below keeps writable under the chart's + # readOnlyRootFilesystem default. + SOURCE_INTAKE_OBJECT_STORE_ROOT: /tmp/episodic-object-store + PRICING_SNAPSHOT_DIRECTORY: /app/config/pricing-snapshots + API_AUTHORIZATION_PRINCIPAL_ID: local-preview + DRAFT_MODEL: gpt-5.6-sol + OPENAI_REASONING_EFFORT: low + OPENAI_TOKEN_LIMIT_PARAM: max_completion_tokens + OPENAI_TIMEOUT_SECONDS: "600" + GENERATION_MAX_OUTPUT_TOKENS: "32768" + +# Keep the chart's readOnlyRootFilesystem default and make /tmp writable +# through an emptyDir so the source-intake object store has somewhere to +# write. The volume is pod-scoped: blobs vanish when the pod is replaced. +volumes: + - name: tmp + emptyDir: {} +volumeMounts: + - name: tmp + mountPath: /tmp existingSecretName: episodic-local allowMissingSecret: false @@ -12,6 +34,18 @@ secretEnvFromKeys: DATABASE_URL: key: database-url optional: false + API_AUTHORIZATION_BEARER_TOKEN: + key: api-bearer-token + optional: false + # The OpenAI pair is optional so the preview still boots without a key; + # the runtime requires both or neither, and the preview secret writes + # both together. + OPENAI_BASE_URL: + key: openai-base-url + optional: true + OPENAI_API_KEY: + key: openai-api-key + optional: true ingress: enabled: true diff --git a/charts/episodic/values.yaml b/charts/episodic/values.yaml index 1dac4b6f..7fe35503 100644 --- a/charts/episodic/values.yaml +++ b/charts/episodic/values.yaml @@ -107,6 +107,12 @@ podDisruptionBudget: minAvailable: 1 # maxUnavailable: 1 # Use this OR minAvailable, not both. +# Extra pod volumes and container volume mounts, passed through verbatim. +# The container runs with readOnlyRootFilesystem, so any writable path +# (for example, the source-intake object store root) needs a volume here. +volumes: [] +volumeMounts: [] + nodeSelector: {} tolerations: [] affinity: {} diff --git a/config/pricing-snapshots/openai-2026-08.yaml b/config/pricing-snapshots/openai-2026-08.yaml new file mode 100644 index 00000000..ce510a38 --- /dev/null +++ b/config/pricing-snapshots/openai-2026-08.yaml @@ -0,0 +1,15 @@ +pricing_snapshot_id: 018f15f8-8c12-7c3a-9e9f-9f8f8f8f8f92 +provider_name: openai +model: gpt-5.6-sol +operation: chat_completions +source_kind: provider_rate_card +currency: USD +billing_period_key: "2026-08" +rates_minor_per_metric: + input_tokens: 200 + output_tokens: 1000 + cached_input_tokens: 20 +source_metadata: + source_url: https://developers.openai.com/api/docs/pricing +retrieved_at: "2026-08-22T00:00:00Z" +effective_from: "2026-08-01T00:00:00Z" diff --git a/docs/alpha-test-4-3-2-setup-notes.md b/docs/alpha-test-4-3-2-setup-notes.md new file mode 100644 index 00000000..5c730660 --- /dev/null +++ b/docs/alpha-test-4-3-2-setup-notes.md @@ -0,0 +1,257 @@ +# Alpha test notes: no-QA generation on the local podman/kind preview + +Date: 2026-08-22. Branch: +`4-3-2-no-qa-generation-runs-and-tei-p5-retrieval-alpha-feedback`. + +Goal: generate a TEI P5 episode script from a source document (an Aldus +PageMaker history) and a show specification (*Worlds Apart*) using the roadmap +`4.3` source-to-script vertical slice, on the local podman/kind preview +cluster, and record what it took. + +## Environment + +- Fedora-family Linux under WSL2, rootless Podman 5.8.0. +- `docker` on the PATH is the Podman shim (`podman-docker`), not real Docker. +- `helm`, `uv`, and `jq` were already present. + +## Tools installed during the test + +| Tool | Version | How installed | +| ----------- | ------- | -------------------------------------------------------- | +| `kind` | v0.32.0 | Binary download from GitHub releases into `~/.local/bin` | +| `kubectl` | v1.36.4 | Binary download from `dl.k8s.io` into `~/.local/bin` | +| `e2fsprogs` | 1.47.3 | `sudo dnf install e2fsprogs` (superblock checks) | + +*Table 1: Tools installed to run the local podman/kind preview.* + +Neither `kind` nor `kubectl` was present, although the users' guide preview +workflow requires both. `k3d` was also absent, so the k3d default provider was +never an option on this host; the documented rootless-Podman guidance (use +`LOCAL_K8S_ENGINE=podman LOCAL_K8S_PROVIDER=kind`) is the path taken. + +## Log + +(Chronological; updated as the test progresses.) + +- Confirmed input files exist: `~/docs/the-crown-and-the-pasetboard.md` + (source document — note the filename typo "pasetboard" is in the file system, + not this document), `~/docs/worlds-apart.md` (show spec), and + `~/gpt-image-2-key.txt` (OpenAI key). +- The users' guide (`docs/users-guide.md`) documents the intended workflow + clearly: `POST /v1/uploads` → `POST /v1/ingestion-jobs` → attach source → + poll job → `POST .../generation-runs` with `quality_mode=draft_without_qa` → + poll run → `GET /v1/episodes/{id}/tei` with `Accept: application/tei+xml`. +- Installed `kind` and `kubectl` (see table above). The first `kind` + download used the `latest` channel, which served an alpha build + (0.33.0-alpha); replaced it with the stable v0.32.0 release. +- **First `make local-k8s-up LOCAL_K8S_ENGINE=podman LOCAL_K8S_PROVIDER=kind` + failure — netavark firewall rules.** `kind create cluster` failed at + "Preparing nodes" with + `netavark: nftables error: "nft" did not return successfully`. Two + compounding causes on this WSL2 host: + - The WSL2 kernel (6.6.87.2-microsoft) builds `nf_tables` in, but leaves + `CONFIG_NFT_FIB_IPV6` unset, so netavark's default nftables driver + cannot install its ruleset ("Could not process rule: No such file or + directory"). + - Switching netavark to the iptables driver + (`~/.config/containers/containers.conf` → + `[network] firewall_driver = "iptables"`) then failed because no + `iptables` binary was installed. + + Fix: `sudo dnf install -y iptables-nft` plus the `containers.conf` snippet + above, then `podman network rm kind` so the network is recreated with the + working driver. A plain `podman run --net podman alpine true` is a useful + smoke test — this failure is a host-podman problem, not an Episodic one, but + the preview docs assume container networking already works. +- A second `local-k8s-up` failure mode worth recording: the wrapper runs + every command with `capture_output=True` and lets `CalledProcessError` + escape, so the operator sees a Python traceback with **no stderr from the + failing command**. Diagnosing the netavark failure required re-running the + `kind create cluster` command by hand. `scripts/local_k8s/commands.py` should + surface captured stderr on failure (fixed in this branch: the runner now + echoes captured output before re-raising). +- **The documented preview cannot boot the API service at all.** The local + chart values only supplied `EPISODIC_ENV` and `DATABASE_URL`, but + `episodic/api/runtime_config.py` refuses to start without + `SOURCE_INTAKE_OBJECT_STORE_ROOT`, `API_AUTHORIZATION_BEARER_TOKEN`, and + `API_AUTHORIZATION_PRINCIPAL_ID`, and it validates the pricing-snapshot + directory at boot. The users' guide describes `make local-k8s-up` as + producing a working service; in reality the pod could never have passed + boot-time configuration validation. Fixes made on this branch: + - `Dockerfile` now copies `config/pricing-snapshots` into the runtime + image. The default path in code resolves relative to the installed + package (site-packages), so the ConfigMap sets + `PRICING_SNAPSHOT_DIRECTORY=/app/config/pricing-snapshots` explicitly. + - `charts/episodic/values.local.yaml` adds the boot-required settings, + plus optional `OPENAI_BASE_URL`/`OPENAI_API_KEY` secret references so + generation can reach a real provider. + - The chart originally had **no volume support**, and the container runs + with `readOnlyRootFilesystem: true`, so the source-intake object store + had nowhere to write. The chart now accepts pass-through `volumes` and + `volumeMounts` values, and the local values mount an `emptyDir` at + `/tmp`; the container keeps its read-only root filesystem. + - `scripts/local_k8s` now writes `api-bearer-token` (default + `local-dev-token`) into the preview secret, and, when `OPENAI_API_KEY` + is present in the operator's environment, the paired + `openai-base-url`/`openai-api-key` literals. + - Helm chart contract tests pinned the old (non-booting) ConfigMap and + env exactly; they are updated to the new contract. +- First deploy raced these fixes and sat in `CreateContainerConfigError` + (the pre-fix secret lacked `api-bearer-token`); a second + `OPENAI_API_KEY=... make local-k8s-up` run rebuilt and redeployed. +- **kube-proxy crashloop → cluster DNS dead.** With the app booting, readiness + stayed red: `/health/ready` returned 503 because the pod could not resolve + `postgres`. CoreDNS was not ready because kube-proxy was in CrashLoopBackOff + with `failed complete: too many open files` — the well-known kind-on-Linux + inotify exhaustion. Fix: + `sudo sysctl -w fs.inotify.max_user_watches=524288 + fs.inotify.max_user_instances=512` + (the host already had 524288 watches but only 128 instances) and delete the + kube-proxy pod. The repo's local preview documentation does not mention this + prerequisite. +- **Migrations are a manual, undocumented-for-k8s step.** The users' guide + says "apply the latest Alembic migrations" but the preview provides no hook. + Applied from the host through a port-forward: + `kubectl -n episodic port-forward svc/postgres 15432:5432` then + `DATABASE_URL=postgresql+asyncpg://episodic:episodic@127.0.0.1:15432/episodic + uv run alembic upgrade head`. + This worked first time. +- **Driving the API worked as documented.** With a port-forward on + `svc/episodic` (8088:80), the documented 4.3 slice behaved exactly as the + users' guide describes: series profile → host-profile reference documents → + revisions → series bindings → two uploads (source document and show + specification) → ingestion job → source attachments (the job flips to + `ready_for_generation` on first attachment) → `POST .../generation-runs` with + `quality_mode=draft_without_qa` → poll → episode TEI. Payload shapes had to + be read from the resource code (`episodic/api/resources/`); the users' guide + names the endpoints but not the request bodies. +- **First generation run failed: `LLM response is not valid JSON.`** The + OpenAI adapter never requests a constrained response format, and the default + draft system prompt merely asks for JSON, so `gpt-4o-mini` wrapped its JSON + in markdown fences and the fail-fast parser (correctly) rejected it. Fixed on + this branch by adding `json_response: bool` to `LLMRequest`, emitting + `response_format={"type": "json_object"}` (chat completions) / `text.format` + (Responses API), and setting it from the draft script generator. Unit tests + added in `tests/test_llm_openai_request_payload.py`. +- **Model switch to `gpt-5.6-sol` (operator request) exposed three adapter + gaps.** Probing the model directly showed: (1) reasoning models reject + `max_tokens` and require `max_completion_tokens`; (2) the flex service tier + was capacity-constrained ("Flex does not have sufficient resources… change + service_tier=default"), so the preview runs on the default tier; (3) there + was no way to set `reasoning_effort`. Added `OpenAIPayloadOptions` (reasoning + effort, service tier, token-limit parameter name) to the adapter, wired + through new runtime settings `OPENAI_REASONING_EFFORT`, + `OPENAI_SERVICE_TIER`, and `OPENAI_TOKEN_LIMIT_PARAM`. The local preview pins + `DRAFT_MODEL=gpt-5.6-sol`, effort `low`, default tier. +- **Cost pinning requires a pricing snapshot for the current month.** The + pricing catalogue is exact-match on provider/model/operation/billing period, + and a missing snapshot fails the whole generation run. Added + `config/pricing-snapshots/openai-2026-08.yaml` for `gpt-5.6-sol` (USD 2.00/M + input, 10.00/M output, 0.20/M cached input). +- **Second failure mode: `Transient provider failure after exhausting retries.` + ** The adapter's HTTP timeout was hard-coded at 30 s; a reasoning model + drafting a full episode script comfortably exceeds that, so every attempt was + cancelled client-side and retried until the run failed. Host-side curl proved + the provider and pod egress were both healthy. Fixed by adding + `OPENAI_TIMEOUT_SECONDS` (default 30, local preview sets 600). +- **The WSL2 VM itself crashed twice during image rebuilds**, taking the + kind container with it. Recovery each time: + `systemd-run --scope --user -p Delegate=yes podman start + episodic-preview-control-plane`, + wait for the API server, then let the pods restart. The inotify sysctls were + lost on the first restart, so they are now persisted in + `/etc/sysctl.d/99-kind-inotify.conf` (confirmed to survive the second + restart). The crashes look like a WSL platform issue rather than anything + kind or podman did. +- **The object store is pod-ephemeral, but upload rows are not.** After a + pod restart, uploads recorded in Postgres point at blobs under + `/tmp/episodic-object-store` that no longer exist, and a generation run then + fails with `[Errno 2] No such file or directory`. The database (a StatefulSet + with a PVC) and the object store (container tmpfs) have different lifetimes; + the object store needs a volume with a lifetime matched to the upload + rows. This branch adds the `emptyDir` mount (which still dies with the + pod) and documents in the users' guide that uploads must be redone after + any application pod restart; durable blob storage remains follow-up work. +- **`make local-k8s-up` refuses to run while its own port-forward is + alive.** The preview tooling validates that port 8088 is free, so an operator + following the docs (which say to keep a port-forward running) cannot re-run + the up command without first killing their own kubectl. +- **The second WSL crash corrupted rootless podman's overlay storage.** + `podman build` failed with `readlink …/overlay/l: invalid argument`; the + layer link directory `~/.local/share/containers/storage/overlay/l` had been + destroyed and several layers had truncated (empty) `link` files. Repaired + without a `podman system reset` (which would have destroyed the running kind + cluster's storage) by removing stale buildah working containers + (`buildah rm --all`), recreating `overlay/l`, and regenerating one symlink + per layer from each layer's `link` file + (`ln -sfn ..//diff l/`). The subsequent build succeeded from + cache. +- **Third failure mode: pricing pins violate a foreign key on a fresh + database.** With the timeout fixed, the draft generated successfully + (`draft.generated`, `finish_reason=stop`), but the run then failed: + `run_pricing_pins.pricing_snapshot_id` references `pricing_snapshots.id`, and + nothing ever syncs the file-based pricing catalogue + (`config/pricing-snapshots/*.yaml`) into that table. The first run to reach + cost pinning on any fresh deployment can only fail. Fixed on this branch by + adding `ensure_snapshot` to `CostLedgerPort`: the recorder now persists the + resolved snapshot (idempotent `ON CONFLICT DO NOTHING` on the immutable + snapshot id) before pinning it or recording a provider call against it. +- **Fixed `make skylos-allow` and its lint noise.** Two independent + defects: Skylos parses sources with its own runtime's `ast`, so without + `uv tool run --python 3.14` an older default interpreter misreads 3.14 + syntax and reports phantom dead code (fix cherry-picked from the + `code-duplication-gate` branch); and the `whitelist` subcommand only + dispatches when it is Skylos's first argument, so the shared macro's + `--config-file` prefix made `--reason` an "unrecognized argument". The + target now uses a bare `SKYLOS_CLI` macro for the subcommand and accepts + `NAME`/`REASON` only from the make command line, so an ambient `NAME` + environment variable can no longer leak into the whitelist (the full + test suite had previously written the host's `NAME=ibara` and a + shell-injection probe into the real `pyproject.toml`). +- **Storage post-mortem after the third crash (whole-PC restart).** The + ext4 superblock on the distro disk reports `clean` with no recorded + error history, and podman's overlay layer links survived intact this + time. The overlay driver configuration itself is sound: native kernel + overlayfs (not fuse-overlayfs) on ext4, `d_type` supported. The + corruption pattern (EIO on every process spawn, journald files + corrupted, overlay link directory destroyed once) with no ext4 error + records points at lost writes in the WSL2 VHDX/virtio-blk layer when + the VM dies under container-build I/O, not at podman or kind. Worth + checking on the Windows side: Event Viewer disk/vhdmp events around the + crash times and free space on the drive holding the VHDX. Moving + podman's `graphroot` off the distro VHDX onto the separate `/data` + disk would also take the build I/O out of the blast radius. +- **Success.** With all fixes deployed (JSON response mode, + `max_completion_tokens`, 600 s timeout, snapshot persistence, + zero-metric suppression, and the `emptyDir` mount under a read-only + root filesystem), the full workflow completed end to end: series + profile → host reference documents and bindings → source document and + show specification uploads → ingestion job → source attachment → + `draft_without_qa` generation run → `run.succeeded` → TEI P5 download + via `Accept: application/tei+xml`. The `gpt-5.6-sol` draft consumed + 13,803 input and 2,476 output tokens (≈ USD 0.05 at the pinned + 2026-08 rates) and produced a 10 kB, 39-turn TEI document whose + dialogue follows the show specification's host personas and the + source document's narrative arc. +- **Moved rootless podman's `graphroot` off the distro VHDX.** Following + the storage post-mortem, container storage now lives at + `/data/leynos/containers/storage` on the separate xfs disk + (`ftype=1`, so overlayfs is supported) instead of + `~/.local/share/containers/storage` inside the WSL distro's ext4 + VHDX. Procedure: stop all containers, `rsync -aHX --numeric-ids` the + 74 GiB storage tree, point `~/.config/containers/storage.conf` at the + new `graphroot`, and verify images and containers are visible. Two + traps: the copy must run under `podman unshare` (a plain rsync + silently skips every subuid-owned file with permission errors while + masking them behind a nonzero exit code), and podman's SQLite state + database (`db.sql` at the storage root) records absolute paths, so + after the move its `DBConfig` row needs updating + (`StaticDir`/`GraphRoot`/`VolumeDir`) or podman refuses to start with + "database configuration mismatch". All containers except `qdrant` + (whose data is a bind mount to `/data/qdrant`, outside container + storage) were pruned before the move, shrinking the copy from 74 GiB. + A smoke `podman run` on the new graphroot succeeded. This takes + image-build I/O — the load implicated in all three VM crashes — off + the VHDX entirely. The old storage tree is left in place as a + fallback until the new location has proven itself; delete + `~/.local/share/containers/storage` to reclaim the space. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 81317eb9..3428815d 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -136,9 +136,13 @@ both commands. Maintainers must update both package pins together and validate the complete `make lint` pipeline. Skylos is separately provisioned by the Makefile at exact release `4.33.2` and -runs locally with concise, non-interactive output. The lint command disables -uploads and provenance collection, selects only dead-code analysis, and fails -when an unexplained finding remains. It does not invoke cloud or Large Language +runs locally with concise, non-interactive output. The `SKYLOS_CLI` and +`SKYLOS` Makefile macros invoke it through `uv tool run --python 3.14`, +because Skylos parses source files with its own interpreter's `ast` module and +must therefore run under the CPython 3.14 the project targets. The lint +command disables uploads and provenance collection, selects only dead-code +analysis, and fails when an unexplained finding remains. It does not invoke +cloud or Large Language Model (LLM) analysis and never modifies source files. Treat every new finding as dead code until its runtime caller is verified. @@ -399,6 +403,11 @@ Helm conventions: - Keep chart values aligned with the Nile Valley example chart contract: `config`, `existingSecretName`, `allowMissingSecret`, `secretEnvFromKeys`, `externalSecret`, ingress, and HTTP probes. +- `volumes` and `volumeMounts` are pass-through chart values (default `[]`) + applied verbatim to the pod template and container spec. Use them to make a + path writable under the chart's `readOnlyRootFilesystem` default, such as + the source-intake object store root; `values.local.yaml` mounts an + `emptyDir` at `/tmp` for that purpose. - Keep the Deployment pod-template `checksum/config` annotation aligned with `templates/configmap.yaml` so ConfigMap-backed environment changes roll pods. - Validate chart edits with `uv run pytest tests/test_helm_chart_contract.py`. @@ -415,6 +424,10 @@ Local preview conventions: command failures. - `local-k8s-up` must apply the local-only Postgres dependency before invoking Helm with `--wait`, because `/health/ready` depends on database connectivity. +- The preview's application Secret is built as a manifest with credentials in + `stringData` and applied with `kubectl apply -f -` on stdin, so secret + values never appear in command arguments the runner would otherwise print + in dry-run mode or echo to stderr on a failed command. - Existing clusters must be reused only when the requested ingress port matches the k3d load-balancer mapping. - Add focused tests in `tests/test_local_k8s_tooling.py` for new command @@ -563,8 +576,8 @@ When adding new worker tasks: ## Observability port abstractions -Two canonical observability ports live in `episodic/observability.py` and must -be the default when adding new operational instrumentation: +Three canonical observability ports live in `episodic/observability.py` and +must be the default when adding new operational instrumentation: - `MetricsPort` is the canonical bounded-cardinality metrics interface. Its `labels` parameters are typed as `collections.abc.Mapping[str, str]` so @@ -574,6 +587,18 @@ be the default when adding new operational instrumentation: operation time. Feature modules (for example `episodic.qa.chrono`) must reuse this port rather than declaring parallel hierarchies. The matching default adapter `PerfCounterClock` is exported from the same module. +- `TracerPort` is the canonical tracing interface. `start_span(name, + attributes=...)` returns a `SpanHandle`, a context manager that bounds an + operation with `set_attribute(name, value)` calls recorded as the span + completes. + +`StructuredLogTracer` is the production tracer adapter; it logs the span +name and completion outcome as structured log lines. It only records +attributes whose name is in a fixed allow-list (`operation`, `outcome`, +`failure_category`, `representation`, `pagination`) and silently drops any +other attribute, so spans cannot leak request payloads, identifiers, or other +sensitive operation metadata into logs. `NoopTracer` is the default no-op +adapter used when no tracing backend is wired. `episodic/metrics_ports.py` retains the narrower `BoundedMetricsPort` and `BoundedValueMetricsPort` protocols, whose `labels` parameters are typed as @@ -586,7 +611,10 @@ Adapters that satisfy `MetricsPort` also satisfy `BoundedMetricsPort` for callers that construct their label dictionaries as concrete `dict` instances. Tests should reuse `episodic.observability.NoopMetrics` and `PerfCounterClock` (or the feature-specific noops, such as the private `_NoopChronoMetrics`) as -default test doubles for the boundary. +default test doubles for the boundary. For tracing, tests should reuse +`episodic.observability.RecordingTracer`, which records each started span as +a `RecordedSpan` (with its attributes and completion state) for deterministic +assertions. ## Database migrations @@ -1565,9 +1593,19 @@ records, ordered terminal events, and status. A request-scoped unit of work must never be captured by a background task. When configured, `CostRecorder` records the provider call and final run roll-up -in the persistence unit of work. It first pins the immutable provider pricing -selected for the run, then records usage with a run-scoped idempotency key. -`PRICING_SNAPSHOT_DIRECTORY` is optional: its default is +in the persistence unit of work. It first calls `CostLedgerPort.ensure_snapshot` +to persist the resolved pricing snapshot idempotently, either before pinning +it to the run or before recording an unpinned provider call; repeated calls +with the same snapshot identifier reuse the stored row. Pricing snapshots are +immutable and content-addressed, so a content-hash collision against another +identifier raises `PricingSnapshotCollisionError` rather than silently +overwriting the stored snapshot. `ensure_snapshot` carries `effective_from` +from the resolved catalogue snapshot through to the persisted row (parsed as a +timezone-aware instant, or left unset when the catalogue entry has none), so +the stored snapshot preserves the same effective-date precedence the catalogue +used to resolve it. `CostRecorder` then records usage with a run-scoped +idempotency key. `PRICING_SNAPSHOT_DIRECTORY` is optional: its +default is `config/pricing-snapshots`; a configured relative path is resolved from the repository root, and startup rejects a path that is not an existing directory. The runtime constructs `FilePricingCatalogue` from the validated directory, so @@ -1784,7 +1822,8 @@ absent. `episodic.llm` now owns a richer outbound contract: - `LLMRequest` carries the prompt text, optional system prompt, target model, - provider operation (`chat_completions` or `responses`), and token budget. + provider operation (`chat_completions` or `responses`), token budget, and + the provider-neutral `json_response` flag. - `OpenAICompatibleLLMAdapter` implements `LLMPort` over explicit OpenAI-compatible HTTP calls, so OpenRouter-style chat completions and OpenAI Responses stay behind the same port. @@ -1799,6 +1838,19 @@ absent. target model and prompt shape. - Persisted `guardrails` belong to canonical profile/template state and are composed before the adapter call, not inside the vendor transport layer. +- `LLMRequest.json_response` asks the provider to enforce a JSON object + response so it cannot wrap the payload in markdown fences or prose. The + adapter maps the flag onto the operation-specific provider shape: + `response_format={"type": "json_object"}` for chat completions, and + `text.format={"type": "json_object"}` for the Responses API. +- `OpenAIPayloadOptions` carries provider-specific request options applied to + outbound payloads: `reasoning_effort` (read from + `OPENAI_REASONING_EFFORT`), `service_tier` (read from + `OPENAI_SERVICE_TIER`), and `token_limit_param` (read from + `OPENAI_TOKEN_LIMIT_PARAM`, one of `max_tokens` or + `max_completion_tokens`, defaulting to `max_tokens`). `OPENAI_TIMEOUT_SECONDS` + sets the adapter's HTTP timeout and defaults to `30.0`. All four are wired + from runtime settings in `episodic.api.runtime_config`. ### OpenAI-compatible adapter package layout @@ -1823,6 +1875,17 @@ module remains responsible for HTTP lifecycle and retry orchestration. the facade or the `LLMPort` contract rather than importing helper functions directly. +`episodic.llm.openai_validation` normalizes raw provider usage into the +canonical `ProviderCallUsage.usage_metrics` used for cost accounting. Cached +and audio token counts are subsets of the prompt and completion totals, so +they are made mutually exclusive with the parent metric: subset counts are +subtracted from `input_tokens`/`output_tokens` and priced under their own +rates (`cached_input_tokens`, `audio_input_tokens`, `audio_output_tokens`). +Reasoning tokens remain inside `output_tokens` and are never priced as a +separate metric. A zero-valued optional metric is omitted entirely, so a +pricing snapshot never needs a rate for a modality the provider did not +report. + ## Multi-source ingestion The multi-source ingestion service normalizes heterogeneous source documents, diff --git a/docs/execplans/4-3-2-no-qa-generation-runs-and-tei-p5-retrieval.md b/docs/execplans/4-3-2-no-qa-generation-runs-and-tei-p5-retrieval.md index d2741888..db6e9a40 100644 --- a/docs/execplans/4-3-2-no-qa-generation-runs-and-tei-p5-retrieval.md +++ b/docs/execplans/4-3-2-no-qa-generation-runs-and-tei-p5-retrieval.md @@ -2126,3 +2126,28 @@ its public-boundary documentation and behaviour. Validation passed: `make check-fmt`, `make lint`, `make typecheck`, `make markdownlint`, and `make nixie`; focused extracted suites passed 42 tests; and `make test` passed 1,214 tests with one skipped test and 49 snapshots. + +Alpha test outcome, 2026-08-23: manual alpha testing on the local podman/kind +preview cluster drove the delivered slice end to end — source and show-spec +uploads, ingestion-job attachment, a `draft_without_qa` generation run, and a +TEI-P5 download over `Accept: application/tei+xml` — and reached +`run.succeeded` with a coherent generated script (see +[alpha-test-4-3-2-setup-notes.md](../alpha-test-4-3-2-setup-notes.md) for the +full session log). Reaching that result required fixes beyond the documented +core slice: the OpenAI adapter now requests constrained JSON output +(`response_format`/`text.format`) so reasoning-model responses wrapped in +markdown fences no longer fail the fail-fast JSON parser; +`OpenAIPayloadOptions` adds configurable `max_completion_tokens` selection, +reasoning effort, and service tier for reasoning models that reject +`max_tokens`; the provider HTTP timeout is now configurable via +`OPENAI_TIMEOUT_SECONDS` (the prior hard-coded 30 s timeout under-provisioned +reasoning-model drafting); `CostLedgerPort.ensure_snapshot` persists a +resolved pricing snapshot idempotently before it is pinned or referenced, so +the first run against a fresh database no longer fails a foreign-key check on +`run_pricing_pins`; usage metering omits zero-valued optional token metrics +instead of reporting spurious cached/audio counters; and the Helm chart +gained pass-through `volumes`/`volumeMounts` support (with the local +preview's Secret moved to a stdin-applied manifest) so the +source-intake object store has a writable mount under the chart's +`readOnlyRootFilesystem` default. Review hardening for these fixes continues +on PR #277. diff --git a/docs/users-guide.md b/docs/users-guide.md index 905e1c56..100acbec 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -90,7 +90,21 @@ adapter that can use pre-signed upload URLs. Before using these endpoints, apply the latest Alembic migrations to the service database with `alembic upgrade head`. The application does not apply -schema migrations during startup. +schema migrations during startup, and no deployment hook applies them for you. + +Against the local Kubernetes preview, run the migrations from the host through +a port-forward to the preview Postgres Service. The `kubectl` context depends +on the chosen local Kubernetes provider: the default k3d provider registers +`k3d-episodic-preview`, while the optional rootless-Podman kind provider +(`LOCAL_K8S_PROVIDER=kind`, described later in this guide) registers +`kind-episodic-preview` instead. + +```shell +kubectl --context k3d-episodic-preview --namespace episodic \ + port-forward svc/postgres 15432:5432 & +DATABASE_URL=postgresql+asyncpg://episodic:episodic@127.0.0.1:15432/episodic \ + uv run alembic upgrade head +``` Create a draft run for a ready ingestion job with `POST /v1/ingestion-jobs/{ingestion_job_id}/generation-runs`. Supply an @@ -171,6 +185,24 @@ draft output. Each value must be a positive integer: - `GENERATION_MAX_RESPONSE_BYTES` defaults to `1048576` and caps the UTF-8 response before generated JSON parsing. +The HTTP runtime also accepts these optional settings for the OpenAI-compatible +provider request: + +- `OPENAI_BASE_URL` and `OPENAI_API_KEY` configure a provider endpoint and + credential. They are optional, but must be set together: configuring one + without the other fails startup. Neither is set by default. +- `DRAFT_MODEL` selects the model used for no-QA draft generation. It defaults + to `gpt-4o-mini`. +- `OPENAI_REASONING_EFFORT` is forwarded to the provider as a reasoning-effort + hint. It is unset by default. +- `OPENAI_SERVICE_TIER` selects the provider service tier. It is unset by + default. +- `OPENAI_TOKEN_LIMIT_PARAM` selects the request field used to cap output + tokens: `max_tokens` (the default) or `max_completion_tokens`, which + reasoning models require instead. Any other value is rejected at boot. +- `OPENAI_TIMEOUT_SECONDS` sets the provider HTTP timeout in seconds. It must + be a positive number and defaults to `30`. + #### Resumable orchestration Generation workflows now persist an internal checkpoint before a suspendable @@ -401,6 +433,31 @@ the `localhost/episodic:local` image into the `episodic-preview` cluster, bootstraps a local-only Postgres Service and StatefulSet, and exposes ingress through `http://episodic.localhost:8088`. +The preview writes its generated Kubernetes Secret with `api-bearer-token` set +to `local-dev-token` by default, so `/v1` requests against the preview +authenticate with: + +```http +Authorization: Bearer local-dev-token +``` + +When `OPENAI_API_KEY` is set in the operator's environment, +`make local-k8s-up` also writes the paired `openai-base-url`/`openai-api-key` +keys into the same Secret so preview-generated drafts can reach a real +provider. + +`charts/episodic/values.local.yaml` also pins the effective generation +settings for the local preview: + +- `DRAFT_MODEL: gpt-5.6-sol` +- `OPENAI_REASONING_EFFORT: low` +- `OPENAI_TOKEN_LIMIT_PARAM: max_completion_tokens` +- `OPENAI_TIMEOUT_SECONDS: 600` +- `GENERATION_MAX_OUTPUT_TOKENS: 32768` + +Override these by editing `values.local.yaml` before running +`make local-k8s-up`. + On rootless Podman hosts, use the kind provider directly: ```shell @@ -412,6 +469,21 @@ kubectl --context kind-episodic-preview --namespace episodic \ Kind does not install the `traefik` ingress controller used by the local chart values, so the preview URL is reached through the printed port-forward command. +The preview stores uploaded source blobs in an `emptyDir` volume mounted at the +configured `SOURCE_INTAKE_OBJECT_STORE_ROOT`. An `emptyDir` lives and dies with +its pod: the Postgres StatefulSet keeps upload rows on a persistent volume, but +the blobs they point at vanish whenever the application pod is replaced (for +example, after `kubectl rollout restart` or a redeploy). A generation run that +references pre-restart uploads then fails with a missing-file error. After any +application pod restart, re-upload the source documents and attach the fresh +uploads before starting a generation run. + +The chart's `volumes` and `volumeMounts` values are passed through verbatim to +the pod template and container spec, so any writable path needed under the +chart's default `readOnlyRootFilesystem` (such as the source-intake object +store root) must be declared there. `charts/episodic/values.local.yaml` sets +both to mount the `emptyDir` described above at `/tmp`. + If a cluster with the configured name already exists, `local-k8s-up` reuses it only when its ingress port matches the requested port. `local-k8s-status` and `local-k8s-logs` report a missing cluster clearly when the preview has not been diff --git a/episodic/api/resources/generation_runs.py b/episodic/api/resources/generation_runs.py index 32a0f94e..98cce45c 100644 --- a/episodic/api/resources/generation_runs.py +++ b/episodic/api/resources/generation_runs.py @@ -45,6 +45,7 @@ _RETRY_AFTER = "1" _MAX_EVENT_LIMIT = 100 _DEFAULT_EVENT_LIMIT = 20 +_DEFAULT_MAX_SOURCE_COUNT = 32 type Clock = cabc.Callable[[], dt.datetime] type UuidFactory = cabc.Callable[[], uuid.UUID] @@ -64,9 +65,12 @@ class GenerationRunsResource: """Create no-QA generation runs for authenticated ingestion-job owners. The resource materialises the caller-owned ready ingestion job, persists a - durable generation-run checkpoint, and schedules detached execution. The - authenticated principal becomes the durable run actor; request payloads - cannot select another actor. + durable generation-run checkpoint, and schedules detached execution. + + Notes + ----- + The authenticated principal becomes the durable run actor; request + payloads cannot select another actor. """ def __init__( # noqa: PLR0913 # HTTP composition requires independent test seams. @@ -111,7 +115,11 @@ async def on_post( Notes ----- - ``Idempotency-Key`` is required. Replaying the same key for the same + The coroutine returns nothing; the response is populated on + ``resp``. A successful request produces HTTP 202 Accepted with the + serialized generation run as the body, a ``Location`` header + pointing at the run resource, and a ``Retry-After`` header + (``"1"``). ``Idempotency-Key`` is required. Replaying the same key for the same authenticated principal returns the original accepted response. Malformed input, unknown or inaccessible jobs, invalid source materialisation, unavailable launcher configuration, idempotency @@ -210,7 +218,7 @@ async def _create_run( clock=self._clock, uuid_factory=self._uuid_factory, max_source_count=( - 32 + _DEFAULT_MAX_SOURCE_COUNT if self._max_source_count is None else self._max_source_count ), diff --git a/episodic/api/runtime.py b/episodic/api/runtime.py index d8680150..f2dace83 100644 --- a/episodic/api/runtime.py +++ b/episodic/api/runtime.py @@ -40,7 +40,7 @@ from episodic.canonical.object_store import ObjectStorePort from episodic.canonical.unit_of_work_protocols import CanonicalUnitOfWork from episodic.llm import LLMPort - from episodic.observability import MetricsPort, ValueMetricsPort + from episodic.observability import MetricsPort, TracerPort, ValueMetricsPort from .types import UowFactory @@ -50,6 +50,7 @@ class _GenerationLauncherRuntime: """Composition inputs for the in-process generation launcher.""" metrics: ValueMetricsPort + tracer: TracerPort object_store: ObjectStorePort | None = None @@ -83,6 +84,10 @@ def _build_llm_port(config: RuntimeConfig) -> OpenAICompatibleLLMAdapter | None: base_url=config.llm_base_url, api_key=config.llm_api_key, provider_operation=LLMProviderOperation.CHAT_COMPLETIONS, + reasoning_effort=config.llm_reasoning_effort, + service_tier=config.llm_service_tier, + token_limit_param=config.llm_token_limit_param, + timeout_seconds=config.llm_timeout_seconds, ) ) @@ -131,7 +136,7 @@ def _build_generation_launcher( *, config: RuntimeConfig, ) -> InProcessGenerationRunLauncher: - """Build the no-QA generation-run launcher when an LLM port is configured.""" + """Build the no-QA generation-run launcher from configured runtime inputs.""" pricing_catalogue = FilePricingCatalogue(config.pricing_snapshot_directory) def _cost_recorder(uow: CanonicalUnitOfWork) -> CostRecorder: @@ -163,7 +168,7 @@ def _cost_recorder(uow: CanonicalUnitOfWork) -> CostRecorder: provider_name=_DEFAULT_LLM_PROVIDER_NAME, provider_operation=LLMProviderOperation.CHAT_COMPLETIONS.value, metrics=runtime.metrics, - tracer=StructuredLogTracer(), + tracer=runtime.tracer, source_limits=config.generation_source_limits, ) @@ -255,6 +260,7 @@ def create_app_from_env() -> asgi.App: llm_port, _GenerationLauncherRuntime( metrics=metrics, + tracer=tracer, object_store=object_store, ), config=config, diff --git a/episodic/api/runtime_config.py b/episodic/api/runtime_config.py index ea08620d..3749b4d2 100644 --- a/episodic/api/runtime_config.py +++ b/episodic/api/runtime_config.py @@ -8,6 +8,7 @@ """ import dataclasses as dc +import math import os import pathlib import typing as typ @@ -19,6 +20,7 @@ import collections.abc as cabc _DEFAULT_DRAFT_MODEL = "gpt-4o-mini" +_DEFAULT_LLM_TIMEOUT_SECONDS = 30.0 _DEFAULT_GENERATION_MAX_OUTPUT_TOKENS = 4_096 _DEFAULT_GENERATION_MAX_RESPONSE_BYTES = 1_048_576 _REPOSITORY_ROOT = pathlib.Path(__file__).resolve().parents[2] @@ -56,6 +58,21 @@ class RuntimeConfig: Maximum provider output-token budget. generation_max_response_bytes : int Maximum provider response size before parsing. + llm_reasoning_effort : str | None + Optional reasoning-effort request parameter forwarded to the + provider, read from ``OPENAI_REASONING_EFFORT``. Unset (``None``) + by default, in which case the parameter is omitted from requests. + llm_service_tier : str | None + Optional service-tier request parameter forwarded to the provider, + read from ``OPENAI_SERVICE_TIER``. Unset (``None``) by default, in + which case the parameter is omitted from requests. + llm_token_limit_param : str + Name of the output-token-limit request parameter, read from + ``OPENAI_TOKEN_LIMIT_PARAM`` and restricted to ``"max_tokens"`` or + ``"max_completion_tokens"``. Defaults to ``"max_tokens"``. + llm_timeout_seconds : float + Positive HTTP timeout, in seconds, applied to provider requests, + read from ``OPENAI_TIMEOUT_SECONDS``. Defaults to 30.0. Examples -------- @@ -73,6 +90,10 @@ class RuntimeConfig: generation_source_limits: GenerationSourceLimits generation_max_output_tokens: int = _DEFAULT_GENERATION_MAX_OUTPUT_TOKENS generation_max_response_bytes: int = _DEFAULT_GENERATION_MAX_RESPONSE_BYTES + llm_reasoning_effort: str | None = None + llm_service_tier: str | None = None + llm_token_limit_param: str = "max_tokens" # noqa: S105 - parameter name, not a secret. + llm_timeout_seconds: float = _DEFAULT_LLM_TIMEOUT_SECONDS class RuntimeConfigurationError(RuntimeError): @@ -226,6 +247,37 @@ def _llm_settings( return base_url, api_key +def _llm_request_options( + environment: cabc.Mapping[str, str], +) -> tuple[str | None, str | None, str]: + """Return optional provider request options for outbound generation.""" + reasoning_effort = environment.get("OPENAI_REASONING_EFFORT", "").strip() or None + service_tier = environment.get("OPENAI_SERVICE_TIER", "").strip() or None + token_limit_param = ( + environment.get("OPENAI_TOKEN_LIMIT_PARAM", "").strip() or "max_tokens" + ) + if token_limit_param not in {"max_tokens", "max_completion_tokens"}: + msg = "OPENAI_TOKEN_LIMIT_PARAM must be max_tokens or max_completion_tokens." + raise RuntimeConfigurationError(msg) + return reasoning_effort, service_tier, token_limit_param + + +def _llm_timeout_seconds(environment: cabc.Mapping[str, str]) -> float: + """Read the optional positive provider HTTP timeout in seconds.""" + raw = environment.get("OPENAI_TIMEOUT_SECONDS", "").strip() + if not raw: + return _DEFAULT_LLM_TIMEOUT_SECONDS + try: + value = float(raw) + except ValueError as exc: + msg = "OPENAI_TIMEOUT_SECONDS must be a positive number." + raise RuntimeConfigurationError(msg) from exc + if not math.isfinite(value) or value <= 0: + msg = "OPENAI_TIMEOUT_SECONDS must be a positive number." + raise RuntimeConfigurationError(msg) + return value + + def _pricing_snapshot_directory( environment: cabc.Mapping[str, str], ) -> pathlib.Path: @@ -246,14 +298,11 @@ def _load_runtime_config( """Read and validate runtime configuration from environment variables.""" environment = os.environ if environ is None else environ llm_settings = _llm_settings(environment) + llm_request_options = _llm_request_options(environment) pricing_snapshot_directory = _pricing_snapshot_directory(environment) authorization = _authorization_settings(environment) output_limits = _generation_output_limits(environment) - log_info( - logger, - "runtime_config_loaded source_intake_object_store_configured", - ) - return RuntimeConfig( + config = RuntimeConfig( database_url=_required_setting( environment, "DATABASE_URL", @@ -262,6 +311,10 @@ def _load_runtime_config( source_intake_object_store_root=_source_intake_object_store_root(environment), llm_base_url=llm_settings[0], llm_api_key=llm_settings[1], + llm_reasoning_effort=llm_request_options[0], + llm_service_tier=llm_request_options[1], + llm_token_limit_param=llm_request_options[2], + llm_timeout_seconds=_llm_timeout_seconds(environment), draft_model=_draft_model(environment), pricing_snapshot_directory=pricing_snapshot_directory, authorization_bearer_token=authorization[0], @@ -270,3 +323,8 @@ def _load_runtime_config( generation_max_output_tokens=output_limits[0], generation_max_response_bytes=output_limits[1], ) + log_info( + logger, + "runtime_config_loaded source_intake_object_store_configured", + ) + return config diff --git a/episodic/canonical/entity_protocols.py b/episodic/canonical/entity_protocols.py index 22b15b02..ace75252 100644 --- a/episodic/canonical/entity_protocols.py +++ b/episodic/canonical/entity_protocols.py @@ -133,6 +133,11 @@ async def set_target_episode( updated_at Timestamp supplied by the caller for the durable job update. + Returns + ------- + None + The operation completes without a return value. + Notes ----- The caller owns transaction boundaries. Unknown ``job_id`` values diff --git a/episodic/canonical/generation_run_ports.py b/episodic/canonical/generation_run_ports.py index 262110f9..4e72eeb5 100644 --- a/episodic/canonical/generation_run_ports.py +++ b/episodic/canonical/generation_run_ports.py @@ -164,7 +164,13 @@ async def count_events( *, after_seq: EventSeq | None = None, ) -> int: - """Count events for a run after an optional sequence cursor.""" + """Count events for a run after an optional sequence cursor. + + Raises + ------ + RunNotFound + If ``run_id`` does not identify a stored generation run. + """ raise NotImplementedError diff --git a/episodic/canonical/storage/ingestion_job_repositories.py b/episodic/canonical/storage/ingestion_job_repositories.py index e9ec83be..0ad865f7 100644 --- a/episodic/canonical/storage/ingestion_job_repositories.py +++ b/episodic/canonical/storage/ingestion_job_repositories.py @@ -68,8 +68,10 @@ async def set_target_episode( Notes ----- - The caller owns transaction boundaries. The SQL ``UPDATE`` does not - validate rowcount, so an unknown ``job_id`` affects zero rows. + The operation returns ``None`` and completes without a return + value. The caller owns transaction boundaries. The SQL ``UPDATE`` + does not validate rowcount, so an unknown ``job_id`` affects zero + rows. """ await self._session.execute( sa diff --git a/episodic/canonical/storage/integrity_helpers.py b/episodic/canonical/storage/integrity_helpers.py index a173dcc9..9646aa35 100644 --- a/episodic/canonical/storage/integrity_helpers.py +++ b/episodic/canonical/storage/integrity_helpers.py @@ -96,7 +96,26 @@ def is_revision_conflict_integrity_error( def is_source_document_duplicate_integrity_error(exc: IntegrityError) -> bool: - """Return whether ``exc`` is the deterministic source-document ID conflict.""" + """Return whether ``exc`` is the deterministic source-document ID conflict. + + Parameters + ---------- + exc : sqlalchemy.exc.IntegrityError + Integrity error to classify. + + Returns + ------- + bool + ``True`` for a duplicate ``source_documents.id`` write; otherwise + ``False``. + + Notes + ----- + PostgreSQL reports the ``source_documents_pkey`` constraint name. SQLite + does not surface a constraint name, so it is matched via the + ``"UNIQUE constraint failed: source_documents.id"`` driver-message + fallback. + """ if constraint_name(exc) == "source_documents_pkey": return True return "UNIQUE constraint failed: source_documents.id" in str(exc.orig) diff --git a/episodic/canonical/storage/uow.py b/episodic/canonical/storage/uow.py index e49821a3..da340034 100644 --- a/episodic/canonical/storage/uow.py +++ b/episodic/canonical/storage/uow.py @@ -12,13 +12,28 @@ ... await uow.commit() """ -import typing as typ +# These imports appear in method annotations that `inspect.signature` +# (for example, via `mock.create_autospec`) evaluates at runtime, so they +# must not be deferred behind `typing.TYPE_CHECKING`. +import collections.abc as cabc # noqa: TC003 - runtime-evaluated annotation. +from types import TracebackType # noqa: TC003 - runtime-evaluated annotation. + +from sqlalchemy.ext.asyncio import ( + AsyncSession, # noqa: TC002 - runtime-evaluated annotation. +) from episodic.canonical.unit_of_work_protocols import CanonicalUnitOfWork from episodic.cost.storage import SqlAlchemyCostLedgerStore from episodic.logging import get_logger +from episodic.observability import ( # noqa: TC001 - runtime-evaluated annotations. + MetricsPort, + MonotonicClockPort, +) from .episode_repository import SqlAlchemyEpisodeRepository +from .generation_run_storage_runtime import ( # noqa: TC001 - runtime-evaluated annotation. + GenerationRunStorageRuntime, +) from .generation_runs import SqlAlchemyGenerationRunStore from .ingestion_job_repositories import SqlAlchemyIngestionJobRepository from .repositories import ( @@ -41,16 +56,6 @@ ) from .workflow_checkpoints import SqlAlchemyWorkflowCheckpointStore -if typ.TYPE_CHECKING: - import collections.abc as cabc - from types import TracebackType - - from sqlalchemy.ext.asyncio import AsyncSession - - from episodic.observability import MetricsPort, MonotonicClockPort - - from .generation_run_storage_runtime import GenerationRunStorageRuntime - logger = get_logger(__name__) diff --git a/episodic/cost/__init__.py b/episodic/cost/__init__.py index 7d553db6..fc4ae8f9 100644 --- a/episodic/cost/__init__.py +++ b/episodic/cost/__init__.py @@ -31,6 +31,7 @@ PricingCataloguePort, PricingModel, PricingSnapshot, + PricingSnapshotCollisionError, PricingSnapshotId, PricingSourceKind, ProviderCallLedgerEntry, @@ -55,6 +56,7 @@ "PricingEngine", "PricingModel", "PricingSnapshot", + "PricingSnapshotCollisionError", "PricingSnapshotId", "PricingSourceKind", "ProviderCallLedgerEntry", diff --git a/episodic/cost/ports.py b/episodic/cost/ports.py index 249ab34c..0265b134 100644 --- a/episodic/cost/ports.py +++ b/episodic/cost/ports.py @@ -11,16 +11,11 @@ module are frozen dataclasses or `NewType` aliases that carry immutable snapshot, priced-call, and ledger-entry data. -Tests can use small in-memory fakes that structurally satisfy the Protocols: - -```python -class FakeLedger: - async def record_call(self, entry: ProviderCallLedgerEntry) -> CostLedgerEntryId: - return CostLedgerEntryId(str(entry.idempotency_key)) -``` +Tests can use in-memory fakes; see ``tests/test_cost_ports_protocols.py``. """ import dataclasses as dc +import datetime as dt import enum import typing as typ @@ -87,6 +82,10 @@ class BillingPeriodMismatchError(CostAccountingError): """Raised when a pricing snapshot is used for the wrong billing period.""" +class PricingSnapshotCollisionError(CostAccountingError): + """Raised when one content hash maps to two snapshot identifiers.""" + + def _validate_currency_code(currency: CurrencyCode) -> None: """Validate an ISO 4217-style currency code.""" currency_value = str(currency) @@ -109,7 +108,11 @@ def _validate_usage_metrics(usage: cabc.Mapping[str, int]) -> None: @dc.dataclass(frozen=True, slots=True) class PricingSnapshot: - """Immutable pricing input used by the deterministic pricing engine.""" + """Immutable pricing input used by the deterministic pricing engine. + + ``effective_from`` is an optional timezone-aware ``datetime``; the + catalogue keeps snapshots unset or not after now, selecting the latest. + """ pricing_snapshot_id: PricingSnapshotId provider_name: str @@ -122,11 +125,20 @@ class PricingSnapshot: source_metadata: cabc.Mapping[str, str] content_hash: str retrieved_at: str + effective_from: dt.datetime | None = None def __post_init__(self) -> None: """Validate value-object invariants.""" _validate_currency_code(self.currency) _validate_usage_metrics(self.rates_minor_per_metric) + if self.effective_from is None: + return + if not isinstance(self.effective_from, dt.datetime): + msg = "effective_from must be a datetime or None." + raise TypeError(msg) + if self.effective_from.tzinfo is None: + msg = "effective_from must be timezone-aware." + raise ValueError(msg) @dc.dataclass(frozen=True, slots=True) @@ -202,10 +214,7 @@ def __post_init__(self) -> None: @dc.dataclass(frozen=True, slots=True) class RunPricingKey: - """Composite key identifying a pricing pin. - - The key identifies one provider operation within a run. - """ + """Composite key identifying one provider operation's pricing pin.""" workflow_run_id: str provider_name: str @@ -218,6 +227,24 @@ class RunPricingKey: class CostLedgerPort(typ.Protocol): """Port for append-only cost ledger persistence.""" + async def ensure_snapshot(self, snapshot: PricingSnapshot) -> None: + """Persist an immutable pricing snapshot; reuse an existing row. + + Run pricing pins and ledger entries reference snapshots by + identifier, so the snapshot must be persisted before it is pinned. + + Parameters + ---------- + snapshot : PricingSnapshot + Immutable snapshot to persist. + + Examples + -------- + Repeated ``await ledger.ensure_snapshot(snapshot)`` calls with + one ``pricing_snapshot_id`` reuse the persisted row. + """ + raise NotImplementedError + async def pin_run_pricing( self, key: RunPricingKey, @@ -238,20 +265,12 @@ async def sum_provider_call_costs(self, workflow_run_id: str) -> int: async def record_call(self, entry: ProviderCallLedgerEntry) -> CostLedgerEntryId: """Record or return an idempotent provider-call ledger entry. - Parameters - ---------- - entry : ProviderCallLedgerEntry - Provider-call ledger entry to persist. - Returns ------- CostLedgerEntryId - Identifier of the inserted or existing ledger row. - - Notes - ----- - Repeated calls with the same idempotency key must return the same - `CostLedgerEntryId` without creating duplicate rows. + Identifier of the inserted or existing row; repeated calls + with one idempotency key return the same identifier without + creating duplicate rows. """ raise NotImplementedError diff --git a/episodic/cost/pricing_catalogue/file_loader.py b/episodic/cost/pricing_catalogue/file_loader.py index 3f7d9f94..9de36491 100644 --- a/episodic/cost/pricing_catalogue/file_loader.py +++ b/episodic/cost/pricing_catalogue/file_loader.py @@ -50,6 +50,24 @@ def _require_mapping(value: object, *, path: pathlib.Path) -> dict[str, object]: return typ.cast("dict[str, object]", value) +def _optional_string( + data: cabc.Mapping[str, object], + key: str, + path: pathlib.Path, +) -> str | None: + """Return an optional string field, rejecting present non-string values.""" + value = data.get(key) + if value is None: + return None + if not isinstance(value, str) or not value.strip(): + msg = ( + f"Pricing snapshot {path} field {key!r} must be a non-empty " + "string when present." + ) + raise ValueError(msg) + return value.strip() + + def _require_string( data: cabc.Mapping[str, object], key: str, path: pathlib.Path ) -> str: @@ -225,13 +243,13 @@ def _load_snapshot(path: pathlib.Path) -> _LoadedSnapshot: raw_bytes = path.read_bytes() raw_data = yaml.safe_load(raw_bytes) or {} data = _require_mapping(raw_data, path=path) - effective_from_value = data.get("effective_from") + effective_from_value = _optional_string(data, "effective_from", path) effective_from = ( parse_instant( effective_from_value, error_message=_timestamp_timezone_error, ) - if isinstance(effective_from_value, str) + if effective_from_value is not None else None ) snapshot = PricingSnapshot( @@ -254,5 +272,6 @@ def _load_snapshot(path: pathlib.Path) -> _LoadedSnapshot: source_metadata=_optional_string_mapping(data, "source_metadata", path), content_hash=hashlib.sha256(raw_bytes).hexdigest(), retrieved_at=_require_string(data, "retrieved_at", path), + effective_from=effective_from, ) return _LoadedSnapshot(snapshot=snapshot, effective_from=effective_from) diff --git a/episodic/cost/recorder.py b/episodic/cost/recorder.py index 6aec699c..56d692fd 100644 --- a/episodic/cost/recorder.py +++ b/episodic/cost/recorder.py @@ -171,6 +171,7 @@ async def pin_run_pricing( provider.operation, billing_period_key, ) + await self.ledger.ensure_snapshot(snapshot) await self.ledger.pin_run_pricing( key, snapshot.pricing_snapshot_id, @@ -207,8 +208,18 @@ def _build_provider_call_entry( async def _resolve_snapshot_for_record( self, record: ProviderCallRecord - ) -> PricingSnapshot: - """Resolve the pricing snapshot for a provider-call record.""" + ) -> tuple[PricingSnapshot, bool]: + """Resolve the snapshot for a record and whether it was pinned. + + This helper is read-only; the caller owns any persistence the + resolution outcome requires. + + Returns + ------- + tuple[PricingSnapshot, bool] + The resolved snapshot and whether a run pricing pin selected + it. + """ key = RunPricingKey( workflow_run_id=record.workflow_run_id, provider_name=record.provider_name, @@ -218,13 +229,14 @@ async def _resolve_snapshot_for_record( ) pinned_snapshot_id = await self.ledger.get_run_pricing_pin(key) if pinned_snapshot_id is None: - return await self._resolve_pricing_snapshot( + snapshot = await self._resolve_pricing_snapshot( record.provider_name, record.model, record.operation, record.billing_period_key, ) - return await self.pricing_catalogue.get_snapshot(pinned_snapshot_id) + return snapshot, False + return await self.pricing_catalogue.get_snapshot(pinned_snapshot_id), True async def record_provider_call( self, @@ -249,7 +261,12 @@ async def record_provider_call( CostAccountingError If pricing or ledger validation fails. """ # noqa: DOC502 # Collaborating ports propagate these domain exceptions. - snapshot = await self._resolve_snapshot_for_record(record) + snapshot, pinned = await self._resolve_snapshot_for_record(record) + if not pinned: + # Unpinned calls must persist the snapshot before the ledger row + # references it; pinned calls skip this because the pin's foreign + # key already guarantees the stored row exists. + await self.ledger.ensure_snapshot(snapshot) priced_call = self.pricing_engine.price( snapshot, PricingRequest( diff --git a/episodic/cost/storage/adapters.py b/episodic/cost/storage/adapters.py index 20760b7d..055aaac8 100644 --- a/episodic/cost/storage/adapters.py +++ b/episodic/cost/storage/adapters.py @@ -16,6 +16,7 @@ import sqlalchemy as sa from sqlalchemy.dialects.postgresql import insert +from sqlalchemy.exc import IntegrityError from episodic.cost._time import parse_instant from episodic.cost.ports import ( @@ -25,6 +26,8 @@ LedgerScope, MeteringCounterKey, PricingModel, + PricingSnapshot, + PricingSnapshotCollisionError, PricingSnapshotId, ProviderCallLedgerEntry, RunPricingKey, @@ -36,6 +39,7 @@ CostLedgerEntryRecord, MeteringCounterEventRecord, MeteringCounterRecord, + PricingSnapshotRecord, RunPricingPinRecord, ) @@ -115,6 +119,64 @@ class SqlAlchemyCostLedgerStore: def __init__(self, session: AsyncSession) -> None: self._session = session + async def ensure_snapshot(self, snapshot: PricingSnapshot) -> None: + """Persist an immutable pricing snapshot; reuse an existing row. + + Persists the snapshot identifier, provider name, model, operation, + source kind, currency, billing period, rates, source metadata, + content hash, and retrieved-at timestamp. If a row with the same + ``id`` already exists, the insert is a no-op via + ``ON CONFLICT DO NOTHING``, leaving the stored row unchanged. + + Raises + ------ + PricingSnapshotCollisionError + If ``snapshot.content_hash`` is already stored under a + different snapshot identifier. + sqlalchemy.exc.IntegrityError + If the insert violates a constraint other than the identifier + or content-hash uniqueness. + ValueError + If ``snapshot.retrieved_at`` lacks timezone information, via + ``parse_instant``'s ``"timestamp must include timezone + information."`` error. + """ # noqa: DOC502 # parse_instant raises on the adapter's behalf. + statement = ( + insert(PricingSnapshotRecord) + .values( + id=uuid.UUID(str(snapshot.pricing_snapshot_id)), + provider_name=snapshot.provider_name, + model=snapshot.model, + operation=snapshot.operation, + source_kind=str(snapshot.source_kind), + currency=str(snapshot.currency), + billing_period_key=str(snapshot.billing_period_key), + rates_minor_per_metric=dict(snapshot.rates_minor_per_metric), + source_metadata=dict(snapshot.source_metadata), + content_hash=snapshot.content_hash, + retrieved_at=parse_instant( + snapshot.retrieved_at, + error_message="timestamp must include timezone information.", + ), + effective_from=snapshot.effective_from, + ) + .on_conflict_do_nothing(index_elements=["id"]) + ) + try: + await self._session.execute(statement) + except IntegrityError as exc: + # The id conflict target does not cover the unique content + # hash; a duplicate hash under a different identifier is a + # catalogue defect, not a transient storage failure. + if "content_hash" in str(exc.orig): + msg = ( + "pricing snapshot content hash " + f"{snapshot.content_hash!r} is already stored under a " + "different snapshot identifier" + ) + raise PricingSnapshotCollisionError(msg) from exc + raise + async def pin_run_pricing( self, key: RunPricingKey, diff --git a/episodic/generation/draft_script.py b/episodic/generation/draft_script.py index c9f2ad92..7b57c007 100644 --- a/episodic/generation/draft_script.py +++ b/episodic/generation/draft_script.py @@ -287,6 +287,7 @@ async def generate(self, request: DraftScriptRequest) -> DraftScriptResult: system_prompt=self.config.system_prompt, provider_operation=self.config.provider_operation, token_budget=self.config.token_budget, + json_response=True, ) try: response = await self.llm.generate(llm_request) diff --git a/episodic/llm/openai_api/adapter.py b/episodic/llm/openai_api/adapter.py index 78b70884..d5a71efd 100644 --- a/episodic/llm/openai_api/adapter.py +++ b/episodic/llm/openai_api/adapter.py @@ -25,6 +25,7 @@ ) from episodic.llm.openai_api.request import ( + OpenAIPayloadOptions, _build_payload, _coerce_operation, _path_for_operation, @@ -96,6 +97,10 @@ class OpenAICompatibleLLMConfig: retry_delay_seconds: float = 0.5 timeout_seconds: float = 30.0 chars_per_token: float = 4.0 + # Optional provider-specific request options; see OpenAIPayloadOptions. + reasoning_effort: str | None = None + service_tier: str | None = None + token_limit_param: str = "max_tokens" # noqa: S105 - parameter name, not a secret. __post_init__ = _validate_llm_config @@ -138,6 +143,12 @@ def __init__( self._retry_delay_seconds = config.retry_delay_seconds self._timeout_seconds = config.timeout_seconds self._chars_per_token = config.chars_per_token + # OpenAIPayloadOptions validates token_limit_param at construction. + self._payload_options = OpenAIPayloadOptions( + reasoning_effort=config.reasoning_effort, + service_tier=config.service_tier, + token_limit_param=config.token_limit_param, + ) async def __aenter__(self) -> OpenAICompatibleLLMAdapter: """Return the adapter for use as an async context manager. @@ -208,7 +219,7 @@ async def generate(self, request: LLMRequest) -> LLMResponse: ) response_payload = await self._send_with_retries( path=_path_for_operation(operation), - payload=_build_payload(request, operation), + payload=_build_payload(request, operation, options=self._payload_options), ) if token_budget is not None: _require_concrete_usage_counts(response_payload, operation) diff --git a/episodic/llm/openai_api/request.py b/episodic/llm/openai_api/request.py index e4ce2b59..d71c0ecf 100644 --- a/episodic/llm/openai_api/request.py +++ b/episodic/llm/openai_api/request.py @@ -29,12 +29,48 @@ 'Draft an intro.' """ +import dataclasses as dc + from episodic.llm.ports import ( LLMProviderOperation, LLMProviderResponseError, LLMRequest, ) +_TOKEN_LIMIT_PARAMS = frozenset({"max_tokens", "max_completion_tokens"}) + + +@dc.dataclass(frozen=True, slots=True) +class OpenAIPayloadOptions: + """Provider-specific request options applied to outbound payloads. + + Attributes + ---------- + reasoning_effort : str | None + Reasoning effort hint for reasoning-capable models (for example, + ``"low"``). Omitted from the payload when ``None``. + service_tier : str | None + Provider service tier (for example, ``"flex"``). Omitted from the + payload when ``None``. + token_limit_param : str + Chat-completions parameter name that carries the output token cap. + Reasoning models require ``"max_completion_tokens"``; older + OpenAI-compatible providers expect ``"max_tokens"``. + """ + + reasoning_effort: str | None = None + service_tier: str | None = None + token_limit_param: str = "max_tokens" # noqa: S105 - parameter name, not a secret. + + def __post_init__(self) -> None: + """Reject unsupported token-limit parameter names.""" + if self.token_limit_param not in _TOKEN_LIMIT_PARAMS: + msg = ( + "token_limit_param must be one of " + f"{sorted(_TOKEN_LIMIT_PARAMS)}; got {self.token_limit_param!r}." + ) + raise ValueError(msg) + def _coerce_operation(value: LLMProviderOperation | str) -> LLMProviderOperation: """Normalize a provider operation enum value.""" @@ -59,12 +95,45 @@ def _path_for_operation(operation: LLMProviderOperation) -> str: raise LLMProviderResponseError(msg) +def _apply_chat_options( + payload: dict[str, object], + request: LLMRequest, + opts: OpenAIPayloadOptions, +) -> None: + """Apply chat-completions request options to a payload.""" + if request.token_budget is not None: + payload[opts.token_limit_param] = request.token_budget.max_output_tokens + if request.json_response: + payload["response_format"] = {"type": "json_object"} + if opts.reasoning_effort is not None: + payload["reasoning_effort"] = opts.reasoning_effort + + +def _apply_responses_options( + payload: dict[str, object], + request: LLMRequest, + opts: OpenAIPayloadOptions, +) -> None: + """Apply Responses API request options to a payload.""" + if request.token_budget is not None: + payload["max_output_tokens"] = request.token_budget.max_output_tokens + if request.json_response: + payload["text"] = {"format": {"type": "json_object"}} + if opts.reasoning_effort is not None: + payload["reasoning"] = {"effort": opts.reasoning_effort} + + def _build_payload( request: LLMRequest, operation: LLMProviderOperation, + *, + options: OpenAIPayloadOptions | None = None, ) -> dict[str, object]: """Build a provider request payload from a provider-neutral request.""" + opts = options if options is not None else OpenAIPayloadOptions() payload: dict[str, object] = {"model": request.model} + if opts.service_tier is not None: + payload["service_tier"] = opts.service_tier match operation: case LLMProviderOperation.CHAT_COMPLETIONS: messages: list[dict[str, str]] = [] @@ -72,15 +141,13 @@ def _build_payload( messages.append({"role": "system", "content": request.system_prompt}) messages.append({"role": "user", "content": request.prompt}) payload["messages"] = messages - if request.token_budget is not None: - payload["max_tokens"] = request.token_budget.max_output_tokens + _apply_chat_options(payload, request, opts) return payload case LLMProviderOperation.RESPONSES: payload["input"] = request.prompt if request.system_prompt is not None: payload["instructions"] = request.system_prompt - if request.token_budget is not None: - payload["max_output_tokens"] = request.token_budget.max_output_tokens + _apply_responses_options(payload, request, opts) return payload case _: msg = f"Unsupported provider operation: {operation!r}." diff --git a/episodic/llm/openai_validation.py b/episodic/llm/openai_validation.py index df90c7d4..bbbce42d 100644 --- a/episodic/llm/openai_validation.py +++ b/episodic/llm/openai_validation.py @@ -15,6 +15,10 @@ class OpenAIResponseValidationError(ValueError): "Invalid OpenAI chat completion payload. Expected non-empty string id/model, " "a non-empty choices list, and choices with message.content strings." ) +_INVALID_USAGE_DETAIL_MESSAGE = ( + "Invalid OpenAI usage payload. Nested token details must not exceed " + "their parent token totals." +) _INVALID_RESPONSES_PAYLOAD_MESSAGE = ( "Invalid OpenAI Responses payload. Expected non-empty string id/model, " "a non-empty output list, and output items with content text strings." @@ -196,59 +200,117 @@ def _add_metric_if_present( key: str, value: int | None, ) -> None: - """Add a canonical usage metric when the provider reported it.""" - if value is not None: + """Add a canonical usage metric when the provider reported a nonzero count. + + Zero-valued optional metrics carry no cost information, and recording + them would require every pricing snapshot to price every modality the + provider happens to mention in its usage details. + """ + if value: metrics[key] = value +def _validate_chat_usage_detail_totals( # noqa: PLR0913 # pylint: disable=too-many-arguments # Keyword-only usage fields travel together. + *, + prompt_tokens: int, + completion_tokens: int, + cached_input: int | None, + audio_input: int | None, + audio_output: int | None, +) -> None: + """Validate that nested chat usage details fit within parent totals. + + Raises + ------ + OpenAIResponseValidationError + If nested token details exceed their parent token totals. + """ + input_detail_tokens = (cached_input or 0) + (audio_input or 0) + if input_detail_tokens > prompt_tokens: + raise OpenAIResponseValidationError(_INVALID_USAGE_DETAIL_MESSAGE) + output_audio_tokens = audio_output or 0 + if output_audio_tokens > completion_tokens: + raise OpenAIResponseValidationError(_INVALID_USAGE_DETAIL_MESSAGE) + + +def _build_chat_usage_metrics( + usage_payload: cabc.Mapping[str, object], +) -> dict[str, int]: + """Build mutually exclusive canonical metrics from chat usage details. + + Returns + ------- + dict[str, int] + Mutually exclusive canonical usage metrics. + + Raises + ------ + OpenAIResponseValidationError + If nested token details exceed their parent token totals. + """ # noqa: DOC502 # _validate_chat_usage_detail_totals raises for this helper. + prompt_tokens = _extract_token_count(usage_payload, "prompt_tokens") + completion_tokens = _extract_token_count(usage_payload, "completion_tokens") + cached_input = _extract_nested_token_count( + usage_payload, + "prompt_tokens_details", + "cached_tokens", + ) + audio_input = _extract_nested_token_count( + usage_payload, + "prompt_tokens_details", + "audio_tokens", + ) + audio_output = _extract_nested_token_count( + usage_payload, + "completion_tokens_details", + "audio_tokens", + ) + _validate_chat_usage_detail_totals( + prompt_tokens=prompt_tokens, + completion_tokens=completion_tokens, + cached_input=cached_input, + audio_input=audio_input, + audio_output=audio_output, + ) + input_detail_tokens = (cached_input or 0) + (audio_input or 0) + output_audio_tokens = audio_output or 0 + metrics = { + "input_tokens": prompt_tokens - input_detail_tokens, + "output_tokens": completion_tokens - output_audio_tokens, + } + _add_metric_if_present(metrics, "cached_input_tokens", cached_input) + _add_metric_if_present(metrics, "audio_input_tokens", audio_input) + _add_metric_if_present(metrics, "audio_output_tokens", audio_output) + return metrics + + def _normalize_chat_provider_call_usage( payload_mapping: cabc.Mapping[str, object], usage_payload: cabc.Mapping[str, object] | None, finish_reason: str | None, ) -> ProviderCallUsage | None: - """Convert OpenAI chat usage details into canonical cost metrics.""" + """Convert OpenAI chat usage details into canonical cost metrics. + + The provider reports cached and audio counts as subsets of the prompt + and completion totals, so the canonical metrics are made mutually + exclusive: subset counts are subtracted from their parent totals and + priced under their own rates. Reasoning tokens are billed at the + output rate and stay inside ``output_tokens`` rather than becoming a + separately priced metric. + + Returns + ------- + ProviderCallUsage | None + Canonical usage metadata, or ``None`` without a usage payload. + + Raises + ------ + OpenAIResponseValidationError + If nested token details exceed their parent token totals. + """ # noqa: DOC502 # _build_chat_usage_metrics raises for the normalizer. if usage_payload is None: return None - metrics = { - "input_tokens": _extract_token_count(usage_payload, "prompt_tokens"), - "output_tokens": _extract_token_count(usage_payload, "completion_tokens"), - } - _add_metric_if_present( - metrics, - "cached_input_tokens", - _extract_nested_token_count( - usage_payload, - "prompt_tokens_details", - "cached_tokens", - ), - ) - _add_metric_if_present( - metrics, - "audio_input_tokens", - _extract_nested_token_count( - usage_payload, - "prompt_tokens_details", - "audio_tokens", - ), - ) - _add_metric_if_present( - metrics, - "reasoning_tokens", - _extract_nested_token_count( - usage_payload, - "completion_tokens_details", - "reasoning_tokens", - ), - ) - _add_metric_if_present( - metrics, - "audio_output_tokens", - _extract_nested_token_count( - usage_payload, - "completion_tokens_details", - "audio_tokens", - ), - ) + metrics = _build_chat_usage_metrics(usage_payload) return ProviderCallUsage( usage_metrics=metrics, usage_source=UsageSource.PROVIDER, @@ -265,39 +327,47 @@ def _normalize_responses_provider_call_usage( usage_payload: cabc.Mapping[str, object] | None, finish_reason: str | None, ) -> ProviderCallUsage | None: - """Convert OpenAI Responses usage details into canonical cost metrics.""" + """Convert OpenAI Responses usage details into canonical cost metrics. + + Cached input tokens are a subset of the input total, so they are + subtracted from ``input_tokens`` and priced under their own rate. + Reasoning tokens are billed at the output rate and stay inside + ``output_tokens`` rather than becoming a separately priced metric. + + Returns + ------- + ProviderCallUsage | None + Canonical usage metadata, or ``None`` without a usage payload. + + Raises + ------ + OpenAIResponseValidationError + If nested token details exceed their parent token totals. + """ if usage_payload is None: return None - metrics = { - "input_tokens": _extract_token_count( - usage_payload, - "input_tokens", - error_message=_INVALID_RESPONSES_PAYLOAD_MESSAGE, - ), - "output_tokens": _extract_token_count( - usage_payload, - "output_tokens", - error_message=_INVALID_RESPONSES_PAYLOAD_MESSAGE, - ), - } - _add_metric_if_present( - metrics, - "cached_input_tokens", - _extract_nested_token_count( - usage_payload, - "input_tokens_details", - "cached_tokens", - ), + input_tokens = _extract_token_count( + usage_payload, + "input_tokens", + error_message=_INVALID_RESPONSES_PAYLOAD_MESSAGE, ) - _add_metric_if_present( - metrics, - "reasoning_tokens", - _extract_nested_token_count( - usage_payload, - "output_tokens_details", - "reasoning_tokens", - ), + output_tokens = _extract_token_count( + usage_payload, + "output_tokens", + error_message=_INVALID_RESPONSES_PAYLOAD_MESSAGE, ) + cached_input = _extract_nested_token_count( + usage_payload, + "input_tokens_details", + "cached_tokens", + ) + if (cached_input or 0) > input_tokens: + raise OpenAIResponseValidationError(_INVALID_USAGE_DETAIL_MESSAGE) + metrics = { + "input_tokens": input_tokens - (cached_input or 0), + "output_tokens": output_tokens, + } + _add_metric_if_present(metrics, "cached_input_tokens", cached_input) return ProviderCallUsage( usage_metrics=metrics, usage_source=UsageSource.PROVIDER, diff --git a/episodic/llm/ports.py b/episodic/llm/ports.py index 63202364..44cad8c6 100644 --- a/episodic/llm/ports.py +++ b/episodic/llm/ports.py @@ -111,6 +111,10 @@ class LLMRequest: system_prompt: str | None = None provider_operation: LLMProviderOperation | str | None = None token_budget: LLMTokenBudget | None = None + # Request a provider-enforced JSON object response. Callers that parse + # the response as JSON should set this so providers cannot wrap the + # payload in markdown fences or prose. + json_response: bool = False class LLMError(Exception): diff --git a/pyproject.toml b/pyproject.toml index 503504de..fd3a1c8d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -607,6 +607,7 @@ full_name = [ "episodic.canonical.uploads._require_non_negative", "episodic.canonical.uploads._require_non_negative_if_present", "episodic.canonical.domain._require_positive_integer", + "episodic.canonical.domain._require_value", "episodic.canonical.domain._validate_draft_without_qa_metadata", ] reason = "Canonical value objects call these validators from dataclass post-initialization hooks." diff --git a/scripts/local_k8s/commands.py b/scripts/local_k8s/commands.py index add6d7a4..f102770e 100644 --- a/scripts/local_k8s/commands.py +++ b/scripts/local_k8s/commands.py @@ -1,7 +1,9 @@ """Command construction for the local Kubernetes preview workflow.""" import dataclasses as dc +import re import subprocess +import sys import typing as typ import urllib.parse as urlparse @@ -44,13 +46,22 @@ def run( stdout="", stderr="", ) - return subprocess.run( # noqa: S603 - commands are constructed internally. - args, - input=input_text, - check=check, - text=True, - capture_output=True, - ) + try: + return subprocess.run( # noqa: S603 - commands are constructed internally. + args, + input=input_text, + check=check, + text=True, + capture_output=True, + ) + except subprocess.CalledProcessError as exc: + # Captured output would otherwise vanish into the traceback, + # leaving the operator with no diagnostic from the failed tool. + if exc.stdout: + print(exc.stdout, end="", file=sys.stderr) + if exc.stderr: + print(exc.stderr, end="", file=sys.stderr) + raise def k3d_cluster_create_command(config: PreviewConfig) -> list[str]: @@ -251,24 +262,64 @@ def kubectl_apply_command(config: PreviewConfig) -> list[str]: return [*_kubectl_cmd(config), "apply", "-f", "-"] -def kubectl_secret_command(config: PreviewConfig) -> list[str]: - """Build the idempotent application Secret creation command.""" - return [ - *_kubectl_ns_cmd(config), - "create", - "secret", - "generic", - config.secret_name, - f"--from-literal=database-url={config.database_url}", - "--dry-run=client", - "-o", - "yaml", +def secret_manifest(config: PreviewConfig) -> str: + """Build the application Secret manifest for one stdin apply. + + The manifest carries the credentials in ``stringData`` so secret + values never appear in command arguments, which the runner prints in + dry-run mode and which failed commands may echo to stderr. + + Returns + ------- + str + Secret manifest YAML for ``kubectl apply -f -`` on stdin. + """ + lines = [ + "apiVersion: v1", + "kind: Secret", + "metadata:", + f" name: {_require_dns1123_label(config.secret_name, 'secret_name')}", + f" namespace: {_require_dns1123_label(config.namespace, 'namespace')}", + "type: Opaque", + "stringData:", + f" database-url: {_yaml_string(config.database_url)}", + f" api-bearer-token: {_yaml_string(config.api_bearer_token)}", ] + if config.openai_api_key: + # The runtime requires the base URL and key together, so the + # preview secret only ever writes the pair. + lines.extend([ + f" openai-base-url: {_yaml_string(config.openai_base_url)}", + f" openai-api-key: {_yaml_string(config.openai_api_key)}", + ]) + return "\n".join(lines) + "\n" + + +_DNS1123_LABEL = re.compile(r"^[a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?$") + + +def _require_dns1123_label(value: str, field_name: str) -> str: + """Return a Kubernetes DNS-1123 label or reject the manifest value.""" + if not _DNS1123_LABEL.match(value): + msg = ( + f"{field_name} must be a DNS-1123 label " + f"(lowercase alphanumerics and hyphens); got {value!r}" + ) + raise ValueError(msg) + return value def _yaml_string(value: str) -> str: - """Quote a simple scalar for the local manifest.""" - return '"' + value.replace("\\", "\\\\").replace('"', '\\"') + '"' + """Quote a scalar for the local manifest, escaping control characters.""" + escaped = ( + value + .replace("\\", "\\\\") + .replace('"', '\\"') + .replace("\n", "\\n") + .replace("\r", "\\r") + .replace("\t", "\\t") + ) + return '"' + escaped + '"' def local_postgres_manifest(config: PreviewConfig) -> str: diff --git a/scripts/local_k8s/config.py b/scripts/local_k8s/config.py index 509108da..881a32f6 100644 --- a/scripts/local_k8s/config.py +++ b/scripts/local_k8s/config.py @@ -1,6 +1,7 @@ """Configuration for the local Kubernetes preview workflow.""" import dataclasses as dc +import os import pathlib as pl import tempfile import typing as typ @@ -13,7 +14,85 @@ @dc.dataclass(frozen=True, slots=True) class PreviewConfig: - """User-adjustable local preview settings.""" + """User-adjustable local preview settings. + + Defaults target a local k3d cluster and Docker engine, the ``episodic`` + namespace and Helm release, and the repository's own chart and + ``values.local.yaml`` overlay. ``openai_base_url`` and ``openai_api_key`` + are read from the ``OPENAI_BASE_URL`` and ``OPENAI_API_KEY`` environment + variables at construction time (``openai_base_url`` falls back to + ``https://api.openai.com/v1`` when unset), so setting the key before + running ``make local-k8s-up`` wires generation without committing a + credential anywhere. + + The Kubernetes Secret generated for the preview always writes + ``database-url`` and ``api-bearer-token``. The ``openai-base-url``/ + ``openai-api-key`` pair is written together only when ``openai_api_key`` + is non-empty, since the runtime requires the pair or neither. Secret + values travel via stdin ``stringData``, never as command arguments. + + Attributes + ---------- + cluster_name : str + Name of the local cluster created by the configured provider. + namespace : str + Kubernetes namespace the preview release is installed into. + release_name : str + Helm release name for the preview deployment. + image_name : str + Tag applied to the locally built image before it is loaded into the + cluster. + image_archive_path : pathlib.Path + Filesystem path used to stage the built image archive before it is + imported into the cluster. + ingress_port : int + Host port the preview's ingress is exposed on. + container_engine : ContainerEngine + Container engine used to build and load the preview image + (``"docker"`` or ``"podman"``). + cluster_provider : ClusterProvider + Local cluster provider used to create the preview cluster + (``"k3d"`` or ``"kind"``). + chart_path : pathlib.Path + Path to the Helm chart deployed for the preview. + values_path : pathlib.Path + Path to the chart values overlay applied on top of the chart + defaults. + secret_name : str + Name of the Kubernetes Secret written for the preview. + database_url : str + Connection string for the local-preview-only Postgres container; + not a production credential. + api_bearer_token : str + Bearer token accepted by ``/v1`` requests against the preview; not + a production credential. + openai_base_url : str + OpenAI-compatible base URL for generation requests, read from + ``OPENAI_BASE_URL`` and falling back to + ``https://api.openai.com/v1`` when that variable is unset. + openai_api_key : str + OpenAI API key for generation requests, read from + ``OPENAI_API_KEY`` at construction time and defaulting to the + empty string when that variable is unset. + + Examples + -------- + With ``OPENAI_API_KEY`` set in the environment, the preview Secret + gains the paired ``openai-base-url``/``openai-api-key`` keys: + + >>> import os + >>> os.environ["OPENAI_API_KEY"] = "sk-example" + >>> config = PreviewConfig() + >>> bool(config.openai_api_key) + True + + With ``OPENAI_API_KEY`` absent, the pair is omitted from the Secret: + + >>> os.environ.pop("OPENAI_API_KEY", None) + >>> config = PreviewConfig() + >>> config.openai_api_key + '' + """ cluster_name: str = "episodic-preview" namespace: str = "episodic" @@ -32,6 +111,20 @@ class PreviewConfig: # Production deployments must inject real credentials through Kubernetes # Secrets or ExternalSecret resources. database_url: str = "postgresql+asyncpg://episodic:episodic@postgres:5432/episodic" + # Local-preview bearer token for /v1 requests; not a production credential. + api_bearer_token: str = "local-dev-token" # noqa: S105 - local-only token. + openai_base_url: str = dc.field( + default_factory=lambda: os.environ.get( + "OPENAI_BASE_URL", "https://api.openai.com/v1" + ) + ) + # Read at construction so `OPENAI_API_KEY=... make local-k8s-up` wires + # generation without committing a credential anywhere. repr=False keeps + # the credential out of the dataclass representation. + openai_api_key: str = dc.field( + repr=False, + default_factory=lambda: os.environ.get("OPENAI_API_KEY", ""), + ) def kube_context(self) -> str: """Return the context name the cluster provider creates.""" diff --git a/scripts/local_k8s/orchestration.py b/scripts/local_k8s/orchestration.py index 835ec2bf..da85980e 100644 --- a/scripts/local_k8s/orchestration.py +++ b/scripts/local_k8s/orchestration.py @@ -215,11 +215,10 @@ def up( input_text=namespace.stdout, ) - secret = runner.run(commands.kubectl_secret_command(config)) runner.run( commands.kubectl_apply_command(config), check=True, - input_text=secret.stdout, + input_text=commands.secret_manifest(config), ) runner.run( commands.kubectl_apply_command(config), diff --git a/tests/__snapshots__/test_llm_openai_adapter_usage_metering.ambr b/tests/__snapshots__/test_llm_openai_adapter_usage_metering.ambr index 2b67ee47..50e89f6a 100644 --- a/tests/__snapshots__/test_llm_openai_adapter_usage_metering.ambr +++ b/tests/__snapshots__/test_llm_openai_adapter_usage_metering.ambr @@ -4,16 +4,14 @@ 'audio_input_tokens': 3, 'audio_output_tokens': 5, 'cached_input_tokens': 11, - 'input_tokens': 42, - 'output_tokens': 18, - 'reasoning_tokens': 7, + 'input_tokens': 28, + 'output_tokens': 13, }) # --- # name: test_responses_provider_call_usage_includes_reasoning_tokens dict({ 'cached_input_tokens': 4, - 'input_tokens': 15, + 'input_tokens': 11, 'output_tokens': 12, - 'reasoning_tokens': 6, }) # --- diff --git a/tests/canonical_storage/test_generation_runs.py b/tests/canonical_storage/test_generation_runs.py index d28f6815..6ea0ef78 100644 --- a/tests/canonical_storage/test_generation_runs.py +++ b/tests/canonical_storage/test_generation_runs.py @@ -77,8 +77,8 @@ async def test_generation_run_store_persists_across_unit_of_work( fetched = await uow.generation_runs.get_run(run.id) events = await uow.generation_runs.list_events(run.id) - assert fetched == stored, f"expected stored run {stored!r}, got {fetched!r}" assert fetched is not None, f"expected run {run.id} to persist, got {fetched!r}" + assert fetched == stored, f"expected stored run {stored!r}, got {fetched!r}" assert fetched.quality_mode is QualityMode.DRAFT_WITHOUT_QA, ( f"expected draft-without-QA quality mode, got {fetched.quality_mode!r}" ) diff --git a/tests/canonical_storage/test_sql_generation_run_property_contract.py b/tests/canonical_storage/test_sql_generation_run_property_contract.py index 1c885e0d..f6f146d9 100644 --- a/tests/canonical_storage/test_sql_generation_run_property_contract.py +++ b/tests/canonical_storage/test_sql_generation_run_property_contract.py @@ -214,6 +214,10 @@ async def test_sql_idempotency_keys_are_scoped_to_principals( ) -> None: """The same key replays per principal while remaining independent across actors.""" factory = session_factory + # The session factory outlives the Hypothesis examples, so persisted rows + # survive between examples; scope the key uniquely per example to keep + # them independent. + scoped_key = f"{key}-{uuid.uuid7()}" first_run = make_generation_run() replay_run = make_generation_run() other_principal_run = make_generation_run() @@ -226,17 +230,17 @@ async def test_sql_idempotency_keys_are_scoped_to_principals( async with SqlAlchemyUnitOfWork(factory) as uow: first = await uow.generation_runs.create_run( first_run, - idempotency_key=key, + idempotency_key=scoped_key, idempotency_principal_id="principal-a", ) replay = await uow.generation_runs.create_run( replay_run, - idempotency_key=key, + idempotency_key=scoped_key, idempotency_principal_id="principal-a", ) other = await uow.generation_runs.create_run( other_principal_run, - idempotency_key=key, + idempotency_key=scoped_key, idempotency_principal_id="principal-b", ) await uow.commit() diff --git a/tests/fixtures/generation_run_api.py b/tests/fixtures/generation_run_api.py index 33e6b642..b1e4948f 100644 --- a/tests/fixtures/generation_run_api.py +++ b/tests/fixtures/generation_run_api.py @@ -74,7 +74,8 @@ async def create_ready_ingestion_job( headers : dict[str, str] | None, optional Authorization headers determining the persisted job owner. key_prefix : str, default="generation" - Prefix used to isolate the fixture's idempotency keys. + Prefix used to isolate the fixture's idempotency keys and the + series-profile slug. Returns ------- @@ -86,7 +87,7 @@ async def create_ready_ingestion_job( "/v1/series-profiles", headers={**request_headers, "Idempotency-Key": f"{key_prefix}-profile-key"}, json={ - "slug": "generation-api-profile", + "slug": f"{key_prefix}-api-profile", "title": "Generation API profile", "description": "Generation endpoint fixture.", "configuration": {}, diff --git a/tests/test_container_image_contract.py b/tests/test_container_image_contract.py index d050a93d..b5c0c7b0 100644 --- a/tests/test_container_image_contract.py +++ b/tests/test_container_image_contract.py @@ -185,3 +185,28 @@ def test_docker_image_serves_liveness_when_docker_smoke_enabled() -> None: runtime.GRANIAN_FACTORY_TARGET, str(runtime.HTTP_BIND_PORT), ], f"unexpected container runtime constants: {run.stdout!r}" + + +def test_dockerfile_ships_pricing_snapshots_into_the_runtime_stage() -> None: + """The runtime image must carry the immutable pricing catalogue.""" + stage = None + copies_by_stage: dict[str | None, list[str]] = {} + for raw_line in _dockerfile_text().splitlines(): + line = raw_line.strip() + if line.startswith("FROM ") and " AS " in line: + stage = line.split(" AS ")[-1].strip() + elif line.startswith("COPY") and "pricing-snapshots" in line: + copies_by_stage.setdefault(stage, []).append(line) + + assert copies_by_stage == { + "runtime": [ + ( + "COPY --chown=episodic:episodic config/pricing-snapshots " + "/app/config/pricing-snapshots" + ) + ] + }, ( + "the runtime stage, and only the runtime stage, must copy " + "config/pricing-snapshots to /app/config/pricing-snapshots owned by " + f"the episodic user; got {copies_by_stage!r}" + ) diff --git a/tests/test_cost_ports_protocols.py b/tests/test_cost_ports_protocols.py index be211b28..9c712612 100644 --- a/tests/test_cost_ports_protocols.py +++ b/tests/test_cost_ports_protocols.py @@ -1,7 +1,11 @@ """Port contract tests for cost accounting protocols.""" +import dataclasses as dc +import datetime as dt import inspect +import pytest + from episodic.cost.engine import PricingEngine, PricingRequest from episodic.cost.ports import ( BillingPeriodKey, @@ -57,6 +61,10 @@ def _make_snapshot( class _InMemoryCostLedger: + async def ensure_snapshot(self, snapshot: PricingSnapshot) -> None: + """Accept a fake snapshot persistence request.""" + _ = snapshot + async def pin_run_pricing( self, key: RunPricingKey, @@ -231,3 +239,25 @@ def test_llm_response_accepts_optional_provider_call_usage() -> None: assert response.provider_call_usage == provider_usage, "Expected values to match" assert legacy_response.provider_call_usage is None, "Expected value to be absent" + + +def test_pricing_snapshot_rejects_naive_effective_from() -> None: + """A naive effective date must fail value-object validation.""" + aware = _make_snapshot(PricingSnapshotId("snapshot:aware")) + + with pytest.raises(ValueError, match="effective_from must be timezone-aware"): + dc.replace( + aware, + effective_from=dt.datetime(2026, 6, 1), # noqa: DTZ001 - naive on purpose. + ) + + +def test_pricing_snapshot_rejects_non_datetime_effective_from() -> None: + """A non-datetime effective date must fail with a TypeError.""" + aware = _make_snapshot(PricingSnapshotId("snapshot:typed")) + + with pytest.raises(TypeError, match="effective_from must be a datetime"): + dc.replace( + aware, + effective_from="2026-06-01T00:00:00Z", # type: ignore[arg-type] # invalid on purpose. + ) diff --git a/tests/test_cost_pricing_catalogue_file_loader.py b/tests/test_cost_pricing_catalogue_file_loader.py index 9ecf1a65..b40c674c 100644 --- a/tests/test_cost_pricing_catalogue_file_loader.py +++ b/tests/test_cost_pricing_catalogue_file_loader.py @@ -1,5 +1,6 @@ """Tests for file-backed pricing catalogue resolution.""" +import datetime as dt import textwrap import typing as typ @@ -150,3 +151,85 @@ async def test_file_pricing_catalogue_rejects_missing_snapshot( "chat_completions", BillingPeriodKey("2026-06"), ) + + +@pytest.mark.asyncio +async def test_file_pricing_catalogue_propagates_effective_from( + tmp_path: pathlib.Path, +) -> None: + """The loader carries the snapshot's effective date into the domain.""" + _write_snapshot( + tmp_path, + "openai.yaml", + """ + pricing_snapshot_id: 018f15f8-8c12-7c3a-9e9f-9f8f8f8f8f95 + provider_name: openai + model: gpt-4o-mini + operation: chat_completions + source_kind: provider_rate_card + currency: USD + billing_period_key: "2026-06" + rates_minor_per_metric: + input_tokens: 100 + output_tokens: 200 + source_metadata: + source_url: https://example.test/current + retrieved_at: "2026-06-01T00:00:00Z" + effective_from: "2026-06-01T00:00:00Z" + """, + ) + + catalogue = FilePricingCatalogue(tmp_path) + snapshot = await catalogue.resolve( + "openai", + "gpt-4o-mini", + "chat_completions", + BillingPeriodKey("2026-06"), + ) + + effective_from = snapshot.effective_from + assert effective_from is not None, "expected a loaded effective date" + assert effective_from == dt.datetime(2026, 6, 1, tzinfo=dt.UTC), ( + f"expected the parsed YAML effective date, got {effective_from!r}" + ) + assert effective_from.tzinfo is not None, ( + "the loaded effective date must be timezone-aware" + ) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("invalid_value", ["false", "0", "[]", "{}"]) +async def test_file_pricing_catalogue_rejects_invalid_effective_from( + tmp_path: pathlib.Path, + invalid_value: str, +) -> None: + """Present but non-string effective dates must fail loading.""" + _write_snapshot( + tmp_path, + "openai.yaml", + f""" + pricing_snapshot_id: 018f15f8-8c12-7c3a-9e9f-9f8f8f8f8f96 + provider_name: openai + model: gpt-4o-mini + operation: chat_completions + source_kind: provider_rate_card + currency: USD + billing_period_key: "2026-06" + rates_minor_per_metric: + input_tokens: 100 + output_tokens: 200 + source_metadata: + source_url: https://example.test/invalid + retrieved_at: "2026-06-01T00:00:00Z" + effective_from: {invalid_value} + """, + ) + + catalogue = FilePricingCatalogue(tmp_path) + with pytest.raises(ValueError, match="effective_from"): + await catalogue.resolve( + "openai", + "gpt-4o-mini", + "chat_completions", + BillingPeriodKey("2026-06"), + ) diff --git a/tests/test_cost_recorder.py b/tests/test_cost_recorder.py index da242dbf..9c101204 100644 --- a/tests/test_cost_recorder.py +++ b/tests/test_cost_recorder.py @@ -19,7 +19,11 @@ TaskRollupLedgerEntry, UsageSource, ) -from episodic.cost.recorder import CostRecorder, ProviderCallRecord +from episodic.cost.recorder import ( + CostProviderOperation, + CostRecorder, + ProviderCallRecord, +) def _snapshot( @@ -60,8 +64,16 @@ def _snapshot( class _PinnedLedger: """Ledger fake that exposes one existing run pricing pin.""" - pinned_snapshot_id: PricingSnapshotId + pinned_snapshot_id: PricingSnapshotId | None recorded_call: ProviderCallLedgerEntry | None = None + ensured_snapshots: list[PricingSnapshot] = dc.field(default_factory=list) + calls: list[tuple[str, PricingSnapshotId]] = dc.field(default_factory=list) + + async def ensure_snapshot(self, snapshot: PricingSnapshot) -> None: + """Capture snapshots the recorder persists, mirroring idempotency.""" + self.calls.append(("ensure", snapshot.pricing_snapshot_id)) + if snapshot not in self.ensured_snapshots: + self.ensured_snapshots.append(snapshot) async def pin_run_pricing( self, @@ -69,8 +81,9 @@ async def pin_run_pricing( pricing_snapshot_id: PricingSnapshotId, pinned_at: str, ) -> None: - """Accept a fake run-pricing pin.""" - _ = (key, pricing_snapshot_id, pinned_at) + """Accept a fake run-pricing pin, recording the call order.""" + _ = (key, pinned_at) + self.calls.append(("pin", pricing_snapshot_id)) async def get_run_pricing_pin(self, key: RunPricingKey) -> PricingSnapshotId | None: """Return the fake pinned snapshot identifier.""" @@ -167,3 +180,78 @@ async def test_cost_recorder_prices_provider_call_with_pinned_snapshot() -> None assert ledger.recorded_call.computed_cost_minor == 3, ( "cost must be computed from the pinned 1_000_000 rate, not drifted rates" ) + assert not ledger.ensured_snapshots, ( + "pinned calls must skip snapshot persistence; the pin's foreign key " + "already guarantees the stored row exists" + ) + + +@pytest.mark.asyncio +async def test_cost_recorder_persists_snapshot_for_unpinned_provider_call() -> None: + """Unpinned calls persist the resolved snapshot before the ledger row.""" + latest_snapshot = _snapshot("snapshot:new", input_token_rate=1_000_000) + ledger = _PinnedLedger(pinned_snapshot_id=None) + recorder = CostRecorder( + ledger=ledger, + pricing_catalogue=_DriftingCatalogue( + pinned_snapshot=latest_snapshot, + latest_snapshot=latest_snapshot, + ), + pricing_engine=PricingEngine(), + ) + + await recorder.record_provider_call( + ProviderCallRecord( + idempotency_key=IdempotencyKey("run:abc:node:planner:call:2:attempt:0"), + parent_cost_entry_id=None, + provider_type="llm", + provider_name="openai", + model="gpt-4o-mini", + workflow_node="planner", + operation="chat_completions", + usage={"input_tokens": 3}, + usage_source=UsageSource.PROVIDER, + usage_complete=True, + pricing_model=PricingModel.PAYG, + retry_attempt=0, + billing_period_key=BillingPeriodKey("2026-06"), + workflow_run_id="run-abc", + recorded_at="2026-06-04T00:00:00Z", + ), + ) + + assert ledger.ensured_snapshots == [latest_snapshot], ( + "unpinned calls must persist the resolved snapshot before recording" + ) + + +@pytest.mark.asyncio +async def test_cost_recorder_persists_snapshot_before_pinning() -> None: + """The real pin method ensures the snapshot before writing the pin.""" + snapshot = _snapshot("snapshot:pin-order", input_token_rate=1_000_000) + ledger = _PinnedLedger(pinned_snapshot_id=None) + recorder = CostRecorder( + ledger=ledger, + pricing_catalogue=_DriftingCatalogue( + pinned_snapshot=snapshot, + latest_snapshot=snapshot, + ), + pricing_engine=PricingEngine(), + ) + + await recorder.pin_run_pricing( + "run-pin-order", + ( + CostProviderOperation( + provider_name="openai", + model="gpt-4o-mini", + operation="chat_completions", + ), + ), + BillingPeriodKey("2026-06"), + ) + + assert ledger.calls == [ + ("ensure", snapshot.pricing_snapshot_id), + ("pin", snapshot.pricing_snapshot_id), + ], f"the snapshot must be persisted before it is pinned; got {ledger.calls!r}" diff --git a/tests/test_cost_recorder_integration.py b/tests/test_cost_recorder_integration.py new file mode 100644 index 00000000..f7ed1995 --- /dev/null +++ b/tests/test_cost_recorder_integration.py @@ -0,0 +1,102 @@ +"""Integration tests for the cost recorder against SQL persistence.""" + +import dataclasses as dc +import typing as typ +import uuid + +import pytest +import sqlalchemy as sa + +from episodic.cost import ( + BillingPeriodKey, + PricingSnapshot, + PricingSnapshotId, +) +from episodic.cost.storage import ( + PricingSnapshotRecord, + RunPricingPinRecord, + SqlAlchemyCostLedgerStore, +) +from tests.test_cost_storage_ledger import _pricing_snapshot + +if typ.TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + + +@dc.dataclass(frozen=True, slots=True) +class _SingleSnapshotCatalogue: + """Catalogue fake resolving one fixed snapshot.""" + + snapshot: PricingSnapshot + + async def get_snapshot( + self, + pricing_snapshot_id: PricingSnapshotId, + ) -> PricingSnapshot: + """Return the fixture snapshot for its identifier.""" + assert pricing_snapshot_id == self.snapshot.pricing_snapshot_id, ( + "Expected the fixture snapshot identifier" + ) + return self.snapshot + + async def resolve( # pylint: disable=too-many-arguments,too-many-positional-arguments # The parameter-rich signature is fixed by the explicit port or fixture contract. + self, + provider_name: str, + model: str, + operation: str, + billing_period_key: BillingPeriodKey, + ) -> PricingSnapshot: + """Return the fixture snapshot for any lookup.""" + _ = (provider_name, model, operation, billing_period_key) + return self.snapshot + + +@pytest.mark.asyncio +async def test_cost_recorder_pins_on_a_fresh_database( + session_factory: async_sessionmaker[AsyncSession], +) -> None: + """The real recorder persists the snapshot before the foreign-key pin.""" + from episodic.cost.engine import PricingEngine + from episodic.cost.recorder import CostProviderOperation, CostRecorder + + snapshot = _pricing_snapshot("018f15f8-8c12-7c3a-9e9f-9f8f8f8f8f97") + async with session_factory() as session: + recorder = CostRecorder( + ledger=SqlAlchemyCostLedgerStore(session), + pricing_catalogue=_SingleSnapshotCatalogue(snapshot), + pricing_engine=PricingEngine(), + ) + await recorder.pin_run_pricing( + "workflow-run-fresh-pin", + ( + CostProviderOperation( + provider_name="openai", + model="gpt-4o-mini", + operation="chat_completions", + ), + ), + BillingPeriodKey("2026-06"), + ) + await session.commit() + + async with session_factory() as session: + stored = ( + await session.execute( + sa.select(sa.func.count(PricingSnapshotRecord.id)).where( + PricingSnapshotRecord.id + == uuid.UUID("018f15f8-8c12-7c3a-9e9f-9f8f8f8f8f97") + ) + ) + ).scalar_one() + pinned = ( + await session.execute( + sa.select(RunPricingPinRecord.pricing_snapshot_id).where( + RunPricingPinRecord.workflow_run_id == "workflow-run-fresh-pin" + ) + ) + ).scalar_one() + + assert stored == 1, "pinning on a fresh database must persist the snapshot" + assert str(pinned) == "018f15f8-8c12-7c3a-9e9f-9f8f8f8f8f97", ( + "the pin must reference the persisted snapshot" + ) diff --git a/tests/test_cost_snapshot_concurrency.py b/tests/test_cost_snapshot_concurrency.py new file mode 100644 index 00000000..be16a51f --- /dev/null +++ b/tests/test_cost_snapshot_concurrency.py @@ -0,0 +1,219 @@ +"""Concurrency and cancellation coverage for snapshot persistence.""" + +import asyncio +import contextlib +import typing as typ +import uuid + +import pytest +import sqlalchemy as sa + +from episodic.cost import ( + BillingPeriodKey, + CurrencyCode, + PricingSnapshot, + PricingSnapshotId, + PricingSourceKind, + RunPricingKey, +) +from episodic.cost.storage import ( + PricingSnapshotRecord, + RunPricingPinRecord, + SqlAlchemyCostLedgerStore, +) + +if typ.TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + + type SessionFactory = async_sessionmaker[AsyncSession] +else: # pragma: no cover - runtime alias for evaluated test annotations. + type SessionFactory = object + + +def _snapshot(snapshot_id: str) -> PricingSnapshot: + """Build a deterministic snapshot for concurrency scenarios.""" + return PricingSnapshot( + pricing_snapshot_id=PricingSnapshotId(snapshot_id), + provider_name="openai", + model="gpt-4o-mini", + operation="chat_completions", + source_kind=PricingSourceKind.PROVIDER_RATE_CARD, + currency=CurrencyCode("USD"), + billing_period_key=BillingPeriodKey("2026-06"), + rates_minor_per_metric={"input_tokens": 100, "output_tokens": 200}, + source_metadata={"source": "concurrency-test"}, + content_hash=f"hash-{snapshot_id}", + retrieved_at="2026-06-04T09:00:00Z", + ) + + +def _pin_key(run_id: str) -> RunPricingKey: + """Build the pricing pin key for one workflow run.""" + return RunPricingKey( + workflow_run_id=run_id, + provider_name="openai", + model="gpt-4o-mini", + operation="chat_completions", + billing_period_key=BillingPeriodKey("2026-06"), + ) + + +async def _snapshot_row_count( + session_factory: SessionFactory, + snapshot_id: str, +) -> int: + """Count persisted snapshot rows through a fresh session.""" + async with session_factory() as session: + return ( + await session.execute( + sa.select(sa.func.count(PricingSnapshotRecord.id)).where( + PricingSnapshotRecord.id == uuid.UUID(snapshot_id) + ) + ) + ).scalar_one() + + +@pytest.mark.asyncio +async def test_concurrent_ensure_persists_exactly_one_snapshot( + session_factory: SessionFactory, +) -> None: + """Two sessions ensuring one snapshot leave exactly one immutable row.""" + snapshot_id = str(uuid.uuid7()) + snapshot = _snapshot(snapshot_id) + start = asyncio.Event() + + async def ensure_once() -> None: + async with session_factory() as session: + await start.wait() + await SqlAlchemyCostLedgerStore(session).ensure_snapshot(snapshot) + await session.commit() + + tasks = [asyncio.create_task(ensure_once()), asyncio.create_task(ensure_once())] + start.set() + await asyncio.gather(*tasks) + + assert await _snapshot_row_count(session_factory, snapshot_id) == 1, ( + "concurrent ensures must persist exactly one snapshot row" + ) + async with session_factory() as session: + stored_hash = ( + await session.execute( + sa.select(PricingSnapshotRecord.content_hash).where( + PricingSnapshotRecord.id == uuid.UUID(snapshot_id) + ) + ) + ).scalar_one() + assert stored_hash == snapshot.content_hash, ( + "the surviving row must carry the first committed snapshot's values" + ) + + +@pytest.mark.asyncio +async def test_concurrent_ensure_and_pin_persist_one_snapshot_and_pin( + session_factory: SessionFactory, +) -> None: + """Two sessions racing ensure-then-pin leave one snapshot and one pin.""" + snapshot_id = str(uuid.uuid7()) + snapshot = _snapshot(snapshot_id) + run_id = f"run-{snapshot_id}" + key = _pin_key(run_id) + start = asyncio.Event() + + async def ensure_and_pin() -> None: + async with session_factory() as session: + await start.wait() + store = SqlAlchemyCostLedgerStore(session) + await store.ensure_snapshot(snapshot) + await store.pin_run_pricing( + key, + snapshot.pricing_snapshot_id, + "2026-06-04T10:00:00Z", + ) + await session.commit() + + tasks = [ + asyncio.create_task(ensure_and_pin()), + asyncio.create_task(ensure_and_pin()), + ] + start.set() + await asyncio.gather(*tasks) + + assert await _snapshot_row_count(session_factory, snapshot_id) == 1, ( + "concurrent ensure-and-pin must persist exactly one snapshot row" + ) + async with session_factory() as session: + pins = ( + ( + await session.execute( + sa.select(RunPricingPinRecord).where( + RunPricingPinRecord.workflow_run_id == run_id + ) + ) + ) + .scalars() + .all() + ) + assert len(pins) == 1, "concurrent pinning must persist exactly one pin row" + assert str(pins[0].pricing_snapshot_id) == snapshot_id, ( + "the surviving pin must reference the ensured snapshot" + ) + + +@pytest.mark.asyncio +async def test_cancelled_transaction_leaves_no_partial_snapshot( + session_factory: SessionFactory, +) -> None: + """Cancellation before commit rolls back; a retry then succeeds.""" + snapshot_id = str(uuid.uuid7()) + snapshot = _snapshot(snapshot_id) + run_id = f"run-{snapshot_id}" + key = _pin_key(run_id) + ensured = asyncio.Event() + release = asyncio.Event() + + async def ensure_then_stall() -> None: + async with session_factory() as session: + store = SqlAlchemyCostLedgerStore(session) + await store.ensure_snapshot(snapshot) + await store.pin_run_pricing( + key, + snapshot.pricing_snapshot_id, + "2026-06-04T10:00:00Z", + ) + ensured.set() + # Hold the transaction open, uncommitted, until cancelled. + await release.wait() + await session.commit() # pragma: no cover - cancelled before commit. + + task = asyncio.create_task(ensure_then_stall()) + await ensured.wait() + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + + assert await _snapshot_row_count(session_factory, snapshot_id) == 0, ( + "a cancelled, uncommitted transaction must leave no snapshot row" + ) + async with session_factory() as session: + pins = ( + await session.execute( + sa.select(sa.func.count(RunPricingPinRecord.workflow_run_id)).where( + RunPricingPinRecord.workflow_run_id == run_id + ) + ) + ).scalar_one() + assert pins == 0, "a cancelled transaction must roll back its uncommitted pin row" + + async with session_factory() as session: + store = SqlAlchemyCostLedgerStore(session) + await store.ensure_snapshot(snapshot) + await store.pin_run_pricing( + key, + snapshot.pricing_snapshot_id, + "2026-06-04T10:00:00Z", + ) + await session.commit() + + assert await _snapshot_row_count(session_factory, snapshot_id) == 1, ( + "a retry after cancellation must persist the snapshot" + ) diff --git a/tests/test_cost_snapshot_persistence_properties.py b/tests/test_cost_snapshot_persistence_properties.py new file mode 100644 index 00000000..c3eeb748 --- /dev/null +++ b/tests/test_cost_snapshot_persistence_properties.py @@ -0,0 +1,126 @@ +"""Property tests for idempotent, immutable pricing-snapshot persistence.""" + +import typing as typ +import uuid + +import hypothesis.strategies as st +import pytest +import sqlalchemy as sa +from hypothesis import HealthCheck, given, settings + +from episodic.cost import ( + BillingPeriodKey, + CurrencyCode, + PricingSnapshot, + PricingSnapshotCollisionError, + PricingSnapshotId, + PricingSourceKind, +) +from episodic.cost.storage import PricingSnapshotRecord, SqlAlchemyCostLedgerStore + +if typ.TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + + type SessionFactory = async_sessionmaker[AsyncSession] +else: # pragma: no cover - runtime alias for evaluated test annotations. + type SessionFactory = object + +_RATE_VARIANTS = ( + {"input_tokens": 100, "output_tokens": 200}, + {"input_tokens": 150, "output_tokens": 250}, +) + +_OPERATIONS = st.lists( + st.tuples( + st.integers(min_value=0, max_value=1), + st.integers(min_value=0, max_value=1), + st.integers(min_value=0, max_value=1), + ), + min_size=1, + max_size=6, +) + + +def _snapshot( + snapshot_id: str, + content_hash: str, + rates: dict[str, int], +) -> PricingSnapshot: + """Build a pricing snapshot from the example's finite domains.""" + return PricingSnapshot( + pricing_snapshot_id=PricingSnapshotId(snapshot_id), + provider_name="openai", + model="gpt-4o-mini", + operation="chat_completions", + source_kind=PricingSourceKind.PROVIDER_RATE_CARD, + currency=CurrencyCode("USD"), + billing_period_key=BillingPeriodKey("2026-06"), + rates_minor_per_metric=rates, + source_metadata={"source": "property-test"}, + content_hash=content_hash, + retrieved_at="2026-06-04T09:00:00Z", + ) + + +@given(scope=st.uuids(version=4), operations=_OPERATIONS) +@settings( + max_examples=6, + deadline=None, + suppress_health_check=[HealthCheck.function_scoped_fixture], +) +@pytest.mark.asyncio +async def test_ensure_snapshot_keeps_persisted_identifiers_immutable( + session_factory: SessionFactory, + scope: uuid.UUID, + operations: list[tuple[int, int, int]], +) -> None: + """No operation ordering can mutate a persisted snapshot identifier.""" + # Identifier and hash domains are finite within one example but scoped + # uniquely per example, because persisted rows outlive Hypothesis + # examples in the shared database fixture. + identifiers = (str(uuid.uuid5(scope, "id-0")), str(uuid.uuid5(scope, "id-1"))) + hashes = (f"hash-{scope}-0", f"hash-{scope}-1") + recorded: dict[str, tuple[str, dict[str, int]]] = {} + claimed_hashes: dict[str, str] = {} + + for id_index, hash_index, rates_index in operations: + snapshot = _snapshot( + identifiers[id_index], + hashes[hash_index], + _RATE_VARIANTS[rates_index], + ) + snapshot_id = identifiers[id_index] + content_hash = hashes[hash_index] + async with session_factory() as session: + store = SqlAlchemyCostLedgerStore(session) + if ( + snapshot_id not in recorded + and claimed_hashes.get(content_hash, snapshot_id) != snapshot_id + ): + with pytest.raises(PricingSnapshotCollisionError): + await store.ensure_snapshot(snapshot) + continue + await store.ensure_snapshot(snapshot) + await session.commit() + if snapshot_id not in recorded: + recorded[snapshot_id] = (content_hash, _RATE_VARIANTS[rates_index]) + claimed_hashes[content_hash] = snapshot_id + + async with session_factory() as session: + for snapshot_id, (content_hash, rates) in recorded.items(): + stored_hash, stored_rates = ( + await session.execute( + sa.select( + PricingSnapshotRecord.content_hash, + PricingSnapshotRecord.rates_minor_per_metric, + ).where(PricingSnapshotRecord.id == uuid.UUID(snapshot_id)) + ) + ).one() + assert stored_hash == content_hash, ( + f"snapshot {snapshot_id} changed its content hash; " + f"expected {content_hash!r}, got {stored_hash!r}" + ) + assert stored_rates == rates, ( + f"snapshot {snapshot_id} changed its rates; " + f"expected {rates!r}, got {stored_rates!r}" + ) diff --git a/tests/test_cost_storage_ledger.py b/tests/test_cost_storage_ledger.py index bff83f76..76574bf7 100644 --- a/tests/test_cost_storage_ledger.py +++ b/tests/test_cost_storage_ledger.py @@ -1,5 +1,6 @@ """Integration tests for the SQLAlchemy cost ledger adapter.""" +import dataclasses as dc import datetime as dt import typing as typ import uuid @@ -13,7 +14,10 @@ IdempotencyKey, LedgerScope, PricingModel, + PricingSnapshot, + PricingSnapshotCollisionError, PricingSnapshotId, + PricingSourceKind, ProviderCallLedgerEntry, RunPricingKey, TaskRollupLedgerEntry, @@ -230,3 +234,108 @@ def test_recorded_at_fixture_is_timezone_aware() -> None: parsed = dt.datetime.fromisoformat("2026-06-04T10:00:00+00:00") assert parsed.tzinfo is not None, "expected parsed datetime to be timezone-aware" + + +def _pricing_snapshot(snapshot_id: str) -> PricingSnapshot: + """Build a domain pricing snapshot for persistence tests.""" + return PricingSnapshot( + pricing_snapshot_id=PricingSnapshotId(snapshot_id), + provider_name="openai", + model="gpt-4o-mini", + operation="chat_completions", + source_kind=PricingSourceKind.PROVIDER_RATE_CARD, + currency=CurrencyCode("USD"), + billing_period_key=BillingPeriodKey("2026-06"), + rates_minor_per_metric={"input_tokens": 100, "output_tokens": 200}, + source_metadata={"source_url": "https://example.test/pricing"}, + content_hash="ensure-hash", + retrieved_at="2026-06-04T09:00:00Z", + effective_from=dt.datetime(2026, 6, 1, tzinfo=dt.UTC), + ) + + +@pytest.mark.asyncio +async def test_ensure_snapshot_persists_once_and_satisfies_pins( + session_factory: async_sessionmaker[AsyncSession], +) -> None: + """Ensuring a snapshot inserts one row that run pricing pins can reference.""" + snapshot_id = "018f15f8-8c12-7c3a-9e9f-9f8f8f8f8f90" + snapshot = _pricing_snapshot(snapshot_id) + conflicting = dc.replace( + snapshot, + content_hash="ensure-hash-conflicting", + rates_minor_per_metric={"input_tokens": 999, "output_tokens": 999}, + ) + async with session_factory() as session: + store = SqlAlchemyCostLedgerStore(session) + await store.ensure_snapshot(snapshot) + await store.ensure_snapshot(snapshot) + await store.ensure_snapshot(conflicting) + await store.pin_run_pricing( + RunPricingKey( + workflow_run_id="workflow-run-ensure", + provider_name="openai", + model="gpt-4o-mini", + operation="chat_completions", + billing_period_key=BillingPeriodKey("2026-06"), + ), + PricingSnapshotId(snapshot_id), + "2026-06-04T10:00:00Z", + ) + await session.commit() + + async with session_factory() as session: + stored = ( + await session.execute( + sa.select(sa.func.count(PricingSnapshotRecord.id)).where( + PricingSnapshotRecord.id == uuid.UUID(snapshot_id) + ) + ) + ).scalar_one() + stored_hash, stored_effective_from, stored_rates = ( + await session.execute( + sa.select( + PricingSnapshotRecord.content_hash, + PricingSnapshotRecord.effective_from, + PricingSnapshotRecord.rates_minor_per_metric, + ).where(PricingSnapshotRecord.id == uuid.UUID(snapshot_id)) + ) + ).one() + pins = ( + await session.execute( + sa.select(sa.func.count(RunPricingPinRecord.workflow_run_id)) + ) + ).scalar_one() + + assert stored == 1, "ensure_snapshot must persist exactly one snapshot row" + assert stored_hash == snapshot.content_hash, ( + "a later snapshot sharing the identifier must not overwrite the row" + ) + assert stored_effective_from == dt.datetime(2026, 6, 1, tzinfo=dt.UTC), ( + "ensure_snapshot must persist the snapshot's effective date" + ) + assert stored_rates == dict(snapshot.rates_minor_per_metric), ( + "a conflicting snapshot must not replace the stored rate map" + ) + assert pins == 1, "the pinned snapshot must satisfy the foreign key" + + +@pytest.mark.asyncio +async def test_ensure_snapshot_rejects_content_hash_collisions( + session_factory: async_sessionmaker[AsyncSession], +) -> None: + """A duplicate content hash under a new identifier raises a domain error.""" + snapshot = _pricing_snapshot("018f15f8-8c12-7c3a-9e9f-9f8f8f8f8f93") + colliding = dc.replace( + snapshot, + pricing_snapshot_id=PricingSnapshotId("018f15f8-8c12-7c3a-9e9f-9f8f8f8f8f94"), + ) + async with session_factory() as session: + store = SqlAlchemyCostLedgerStore(session) + await store.ensure_snapshot(snapshot) + + with pytest.raises( + PricingSnapshotCollisionError, + match="already stored under a different snapshot identifier", + ): + await store.ensure_snapshot(colliding) diff --git a/tests/test_draft_script_generation.py b/tests/test_draft_script_generation.py index 7089ef28..dc24f7de 100644 --- a/tests/test_draft_script_generation.py +++ b/tests/test_draft_script_generation.py @@ -172,6 +172,9 @@ async def test_draft_script_generator_emits_valid_stable_tei( assert fake_llm.requests[0].system_prompt is not None, ( "expected a system prompt, got None" ) + assert fake_llm.requests[0].json_response is True, ( + "draft generation must request a provider-enforced JSON response" + ) assert result.tei_xml == snapshot, "generated TEI must match the approved snapshot" diff --git a/tests/test_env_runtime_provider_options.py b/tests/test_env_runtime_provider_options.py new file mode 100644 index 00000000..5bb1cf2d --- /dev/null +++ b/tests/test_env_runtime_provider_options.py @@ -0,0 +1,194 @@ +"""Runtime composition tests for the OPENAI_* provider request options.""" + +import asyncio +import json +import typing as typ + +import httpx +import pytest + +if typ.TYPE_CHECKING: + from pathlib import Path + + from episodic.api.dependencies import ApiDependencies + + +def _provider_option_environment( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Set the runtime environment carrying the provider request options.""" + monkeypatch.setenv("DATABASE_URL", "postgresql://example.test/episodic") + monkeypatch.setenv("SOURCE_INTAKE_OBJECT_STORE_ROOT", str(tmp_path)) + monkeypatch.setenv("API_AUTHORIZATION_BEARER_TOKEN", "runtime-test-token") + monkeypatch.setenv("API_AUTHORIZATION_PRINCIPAL_ID", "runtime-test-principal") + monkeypatch.setenv("OPENAI_BASE_URL", "https://llm.example.test/v1") + monkeypatch.setenv("OPENAI_API_KEY", "test-key") + monkeypatch.setenv("OPENAI_REASONING_EFFORT", "low") + monkeypatch.setenv("OPENAI_SERVICE_TIER", "flex") + monkeypatch.setenv("OPENAI_TOKEN_LIMIT_PARAM", "max_completion_tokens") + monkeypatch.setenv("OPENAI_TIMEOUT_SECONDS", "600") + + +def _compose_runtime_dependencies( + monkeypatch: pytest.MonkeyPatch, +) -> ApiDependencies: + """Run create_app_from_env with a stubbed database, capturing dependencies.""" + from unittest import mock + + from episodic.api import runtime as runtime_module + from episodic.api.dependencies import ApiDependencies + + captured: dict[str, ApiDependencies] = {} + + async def check_database() -> bool: + await asyncio.sleep(0) + return True + + async def shutdown_database() -> None: + await asyncio.sleep(0) + + probe = runtime_module.ReadinessProbe(name="database", check=check_database) + + def capture_dependencies(dependencies: ApiDependencies) -> object: + captured["dependencies"] = dependencies + return object() + + with ( + mock.patch.object( + runtime_module, + "_build_database_probe", + return_value=(probe, object, shutdown_database), + ), + mock.patch.object( + runtime_module, + "create_app", + side_effect=capture_dependencies, + ), + ): + runtime_module.create_app_from_env() + dependencies = captured["dependencies"] + assert isinstance(dependencies, ApiDependencies), ( + f"expected captured ApiDependencies, got {type(dependencies).__name__}" + ) + return dependencies + + +def _assert_configured_provider_request(request: httpx.Request) -> None: + """Assert that the request carries the configured provider options.""" + body = json.loads(request.content.decode()) + assert body["reasoning_effort"] == "low", ( + f"expected reasoning_effort 'low' in the body, got {body!r}" + ) + assert body["service_tier"] == "flex", ( + f"expected service_tier 'flex' in the body, got {body!r}" + ) + assert body["max_completion_tokens"] == 2000, ( + f"expected the requested output budget, got {body!r}" + ) + assert "max_tokens" not in body, ( + "the default token parameter must be replaced, not duplicated" + ) + timeout = request.extensions["timeout"] + expected_timeout = dict.fromkeys(("connect", "read", "write", "pool"), 600.0) + assert timeout == expected_timeout, ( + f"expected a 600 second request timeout, got {timeout!r}" + ) + + +def test_create_app_from_env_propagates_provider_request_options( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Runtime composition carries the OPENAI_* options into the LLM config.""" + from unittest import mock + + from episodic.api import runtime as runtime_module + from episodic.llm.openai_adapter import OpenAICompatibleLLMConfig + + _provider_option_environment(monkeypatch, tmp_path) + with mock.patch.object( + runtime_module, + "OpenAICompatibleLLMConfig", + wraps=OpenAICompatibleLLMConfig, + ) as config_factory: + _compose_runtime_dependencies(monkeypatch) + + assert config_factory.call_count == 1, ( + f"expected one constructed LLM config, got {config_factory.call_count}" + ) + kwargs = config_factory.call_args.kwargs + assert kwargs["reasoning_effort"] == "low", ( + f"expected reasoning effort 'low', got {kwargs['reasoning_effort']!r}" + ) + assert kwargs["service_tier"] == "flex", ( + f"expected service tier 'flex', got {kwargs['service_tier']!r}" + ) + assert kwargs["token_limit_param"] == "max_completion_tokens", ( # noqa: S105 - parameter name, not a secret. + f"expected max_completion_tokens, got {kwargs['token_limit_param']!r}" + ) + assert kwargs["timeout_seconds"] == 600.0, ( + f"expected a 600 second timeout, got {kwargs['timeout_seconds']!r}" + ) + + +@pytest.mark.asyncio +async def test_runtime_composed_adapter_sends_configured_provider_request( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """The runtime-created adapter emits the configured provider HTTP request.""" + from episodic.llm import LLMRequest, LLMTokenBudget + + _provider_option_environment(monkeypatch, tmp_path) + from episodic.llm.openai_adapter import OpenAICompatibleLLMAdapter + + dependencies = _compose_runtime_dependencies(monkeypatch) + adapter = dependencies.llm_port + assert isinstance(adapter, OpenAICompatibleLLMAdapter), ( + f"expected a runtime-composed OpenAI adapter, got {type(adapter).__name__}" + ) + + captured_requests: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + captured_requests.append(request) + return httpx.Response( + 200, + json={ + "id": "chatcmpl-runtime-wiring", + "model": "gpt-5.6-sol", + "choices": [ + { + "message": {"content": "Draft intro copy."}, + "finish_reason": "stop", + } + ], + "usage": { + "prompt_tokens": 12, + "completion_tokens": 7, + "total_tokens": 19, + }, + }, + ) + + adapter._client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + try: + await adapter.generate( + LLMRequest( + model="gpt-5.6-sol", + prompt="Draft an intro.", + token_budget=LLMTokenBudget( + max_input_tokens=1000, + max_output_tokens=2000, + max_total_tokens=3000, + ), + ) + ) + finally: + await dependencies.shutdown_hooks[0]() + + assert len(captured_requests) == 1, ( + f"expected one provider request, got {len(captured_requests)}" + ) + _assert_configured_provider_request(captured_requests[0]) diff --git a/tests/test_env_runtime_wiring.py b/tests/test_env_runtime_wiring.py index 01465127..a72a75cc 100644 --- a/tests/test_env_runtime_wiring.py +++ b/tests/test_env_runtime_wiring.py @@ -107,12 +107,15 @@ async def test_build_generation_launcher_wires_cost_recorder( GenerationSourceLimits, InProcessGenerationRunLauncher, ) - from episodic.observability import StructuredLogMetrics + from episodic.observability import StructuredLogMetrics, StructuredLogTracer launcher = _build_generation_launcher( lambda: SqlAlchemyUnitOfWork(session_factory), _UnusedLLMPort(), - _GenerationLauncherRuntime(metrics=StructuredLogMetrics()), + _GenerationLauncherRuntime( + metrics=StructuredLogMetrics(), + tracer=StructuredLogTracer(), + ), config=RuntimeConfig( database_url="postgresql+psycopg://unused", source_intake_object_store_root=tmp_path, diff --git a/tests/test_episode_tei_api.py b/tests/test_episode_tei_api.py index 9c85f013..58ad7ae7 100644 --- a/tests/test_episode_tei_api.py +++ b/tests/test_episode_tei_api.py @@ -373,4 +373,5 @@ async def _create_generation_run( def _tei_hash(tei_xml: str) -> str: + """Return the prefixed SHA-256 content hash for one TEI payload.""" return f"sha256:{hashlib.sha256(tei_xml.encode()).hexdigest()}" diff --git a/tests/test_helm_chart_contract.py b/tests/test_helm_chart_contract.py index c109b413..98deed67 100644 --- a/tests/test_helm_chart_contract.py +++ b/tests/test_helm_chart_contract.py @@ -13,6 +13,19 @@ CHART_PATH = REPOSITORY_ROOT / "charts" / "episodic" LOCAL_VALUES_PATH = CHART_PATH / "values.local.yaml" +# The boot-required runtime settings the local preview ConfigMap must carry. +_EXPECTED_LOCAL_CONFIGMAP_DATA = { + "EPISODIC_ENV": "local", + "SOURCE_INTAKE_OBJECT_STORE_ROOT": "/tmp/episodic-object-store", # noqa: S108 - pod-local preview path. + "PRICING_SNAPSHOT_DIRECTORY": "/app/config/pricing-snapshots", + "API_AUTHORIZATION_PRINCIPAL_ID": "local-preview", + "DRAFT_MODEL": "gpt-5.6-sol", + "OPENAI_REASONING_EFFORT": "low", + "OPENAI_TOKEN_LIMIT_PARAM": "max_completion_tokens", + "OPENAI_TIMEOUT_SECONDS": "600", + "GENERATION_MAX_OUTPUT_TOKENS": "32768", +} + class _Metadata(typ.TypedDict): name: str @@ -268,8 +281,8 @@ def test_helm_local_configmap_carries_the_preview_environment( assert config_map["metadata"]["name"] == "episodic", ( f"the local ConfigMap must be named episodic; got {config_map['metadata']}" ) - assert config_map["data"] == {"EPISODIC_ENV": "local"}, ( - f"the local ConfigMap must expose only EPISODIC_ENV=local; " + assert config_map["data"] == _EXPECTED_LOCAL_CONFIGMAP_DATA, ( + f"the local ConfigMap must expose the boot-required runtime settings; " f"got {config_map['data']}" ) @@ -293,19 +306,30 @@ def test_helm_local_deployment_wires_the_preview_image_and_secret( assert container["envFrom"] == [{"configMapRef": {"name": "episodic"}}], ( f"the container must source configuration from the ConfigMap; got {container}" ) - assert [variable["name"] for variable in container["env"]] == ["DATABASE_URL"], ( - f"the container must declare only DATABASE_URL; got {container['env']}" - ) - secret_ref = container["env"][0]["valueFrom"]["secretKeyRef"] - assert secret_ref["name"] == "episodic-local", ( - f"DATABASE_URL must come from the local secret; got {secret_ref}" - ) - assert secret_ref["key"] == "database-url", ( - f"DATABASE_URL must read the database-url secret key; got {secret_ref}" - ) - assert secret_ref["optional"] is False, ( - f"DATABASE_URL must be a required secret key; got {secret_ref}" - ) + env_by_name = {variable["name"]: variable for variable in container["env"]} + assert set(env_by_name) == { + "DATABASE_URL", + "API_AUTHORIZATION_BEARER_TOKEN", + "OPENAI_BASE_URL", + "OPENAI_API_KEY", + }, f"the container must declare the local secret env keys; got {container['env']}" + expected_secret_keys = { + "DATABASE_URL": ("database-url", False), + "API_AUTHORIZATION_BEARER_TOKEN": ("api-bearer-token", False), + "OPENAI_BASE_URL": ("openai-base-url", True), + "OPENAI_API_KEY": ("openai-api-key", True), + } + for env_name, (secret_key, optional) in expected_secret_keys.items(): + secret_ref = env_by_name[env_name]["valueFrom"]["secretKeyRef"] + assert secret_ref["name"] == "episodic-local", ( + f"{env_name} must come from the local secret; got {secret_ref}" + ) + assert secret_ref["key"] == secret_key, ( + f"{env_name} must read the {secret_key} secret key; got {secret_ref}" + ) + assert secret_ref["optional"] is optional, ( + f"{env_name} optionality must be {optional}; got {secret_ref}" + ) def test_helm_local_deployment_hardens_the_container( @@ -320,11 +344,27 @@ def test_helm_local_deployment_hardens_the_container( f"the pod must refuse to run as root; got {pod_security}" ) assert container_security["readOnlyRootFilesystem"] is True, ( - f"the container root filesystem must be read only; got {container_security}" + f"the local container must keep the chart's read-only root filesystem; " + f"got {container_security}" ) assert container_security["allowPrivilegeEscalation"] is False, ( f"the container must not allow privilege escalation; got {container_security}" ) + pod_spec = _string_key_mapping( + typ.cast("object", deployment["spec"]["template"]["spec"]), + "Deployment pod spec", + ) + assert pod_spec.get("volumes") == [{"name": "tmp", "emptyDir": {}}], ( + f"the local pod must carry the tmp emptyDir volume; got {pod_spec}" + ) + container_mounts = _string_key_mapping( + typ.cast("object", _container(deployment)), + "container", + ).get("volumeMounts") + assert container_mounts == [{"name": "tmp", "mountPath": "/tmp"}], ( # noqa: S108 - pod-local emptyDir mount path. + f"the container must mount the tmp emptyDir at /tmp for the " + f"source-intake object store; got {container_mounts}" + ) def test_helm_local_ingress_publishes_the_preview_host( diff --git a/tests/test_llm_openai_adapter_usage_metering.py b/tests/test_llm_openai_adapter_usage_metering.py index 76178488..701bf5bf 100644 --- a/tests/test_llm_openai_adapter_usage_metering.py +++ b/tests/test_llm_openai_adapter_usage_metering.py @@ -6,6 +6,7 @@ import pytest from episodic.cost import UsageSource +from episodic.llm.ports import LLMProviderResponseError if typ.TYPE_CHECKING: from syrupy.assertion import SnapshotAssertion @@ -122,3 +123,99 @@ def handler(request: httpx.Request) -> httpx.Response: assert response.provider_call_usage.usage_metrics == snapshot, ( "Responses usage metrics must match the recorded snapshot" ) + + +@pytest.mark.asyncio +async def test_chat_completion_usage_omits_zero_valued_optional_metrics( + openai_adapter_factory: _OpenAIAdapterFactory, + openai_json_response: _OpenAIJsonResponseBuilder, + openai_request_builder: _OpenAIRequestBuilder, +) -> None: + """Zero-valued optional usage details must not become priced metrics.""" + + def handler(request: httpx.Request) -> httpx.Response: + del request + return openai_json_response({ + "id": "chatcmpl-zero-usage", + "model": "gpt-4o-mini", + "choices": [ + { + "message": {"content": "Draft intro copy."}, + "finish_reason": "stop", + } + ], + "usage": { + "prompt_tokens": 42, + "completion_tokens": 18, + "total_tokens": 60, + "prompt_tokens_details": {"cached_tokens": 0, "audio_tokens": 0}, + "completion_tokens_details": { + "reasoning_tokens": 0, + "audio_tokens": 0, + }, + }, + }) + + async with openai_adapter_factory( + transport=httpx.MockTransport(handler) + ) as adapter: + response = await adapter.generate(openai_request_builder()) + + assert response.provider_call_usage is not None, ( + "provider_call_usage should not be None" + ) + assert response.provider_call_usage.usage_metrics == { + "input_tokens": 42, + "output_tokens": 18, + }, "zero-valued optional metrics must be omitted from usage metrics" + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "usage_details", + [ + pytest.param( + {"prompt_tokens_details": {"cached_tokens": 6, "audio_tokens": 5}}, + id="prompt_details_exceed_prompt_tokens", + ), + pytest.param( + {"completion_tokens_details": {"audio_tokens": 6}}, + id="completion_audio_exceeds_completion_tokens", + ), + ], +) +async def test_chat_completion_usage_rejects_oversubscribed_details( + openai_adapter_factory: _OpenAIAdapterFactory, + openai_json_response: _OpenAIJsonResponseBuilder, + openai_request_builder: _OpenAIRequestBuilder, + usage_details: dict[str, dict[str, int]], +) -> None: + """Nested counts exceeding the parent totals must fail validation.""" + + def handler(request: httpx.Request) -> httpx.Response: + del request + return openai_json_response({ + "id": "chatcmpl-oversubscribed", + "model": "gpt-4o-mini", + "choices": [ + { + "message": {"content": "Draft intro copy."}, + "finish_reason": "stop", + } + ], + "usage": { + "prompt_tokens": 10, + "completion_tokens": 5, + "total_tokens": 15, + **usage_details, + }, + }) + + async with openai_adapter_factory( + transport=httpx.MockTransport(handler) + ) as adapter: + with pytest.raises( + LLMProviderResponseError, + match="invalid OpenAI-compatible response payload", + ): + await adapter.generate(openai_request_builder()) diff --git a/tests/test_llm_openai_request_payload.py b/tests/test_llm_openai_request_payload.py new file mode 100644 index 00000000..78b5fa4c --- /dev/null +++ b/tests/test_llm_openai_request_payload.py @@ -0,0 +1,92 @@ +"""Unit tests for OpenAI-compatible request payload construction.""" + +import pytest + +from episodic.llm.openai_api.request import OpenAIPayloadOptions, _build_payload +from episodic.llm.ports import ( + LLMProviderOperation, + LLMRequest, + LLMTokenBudget, +) + + +def test_chat_payload_omits_response_format_by_default() -> None: + """Chat payloads leave the provider response format unconstrained.""" + request = LLMRequest(model="gpt-4o-mini", prompt="Draft an intro.") + + payload = _build_payload(request, LLMProviderOperation.CHAT_COMPLETIONS) + + assert "response_format" not in payload, ( + "chat payloads must not constrain the response format by default" + ) + + +def test_chat_payload_requests_json_object_response() -> None: + """JSON-parsing callers get a provider-enforced JSON object response.""" + request = LLMRequest( + model="gpt-4o-mini", + prompt="Draft an intro.", + json_response=True, + ) + + payload = _build_payload(request, LLMProviderOperation.CHAT_COMPLETIONS) + + assert payload["response_format"] == {"type": "json_object"}, ( + "chat payloads must request a JSON object response when asked" + ) + + +def test_chat_payload_applies_provider_request_options() -> None: + """Chat payloads carry effort, tier, and the configured token parameter.""" + request = LLMRequest( + model="gpt-5.6-sol", + prompt="Draft an intro.", + token_budget=LLMTokenBudget( + max_input_tokens=1000, + max_output_tokens=2000, + max_total_tokens=3000, + ), + ) + options = OpenAIPayloadOptions( + reasoning_effort="low", + service_tier="flex", + token_limit_param="max_completion_tokens", # noqa: S106 - parameter name, not a secret. + ) + + payload = _build_payload( + request, LLMProviderOperation.CHAT_COMPLETIONS, options=options + ) + + assert payload["reasoning_effort"] == "low", ( + "chat payloads must carry the configured reasoning effort" + ) + assert payload["service_tier"] == "flex", ( + "chat payloads must carry the configured service tier" + ) + assert payload["max_completion_tokens"] == 2000, ( + "the output cap must use the configured token parameter name" + ) + assert "max_tokens" not in payload, ( + "the default token parameter must be replaced, not duplicated" + ) + + +def test_payload_options_reject_unknown_token_parameter() -> None: + """Unknown token-limit parameter names fail fast at construction.""" + with pytest.raises(ValueError, match="token_limit_param"): + OpenAIPayloadOptions(token_limit_param="max_words") # noqa: S106 - parameter name, not a secret. + + +def test_responses_payload_requests_json_object_response() -> None: + """The Responses API shape carries the JSON format under text.format.""" + request = LLMRequest( + model="gpt-4o-mini", + prompt="Draft an intro.", + json_response=True, + ) + + payload = _build_payload(request, LLMProviderOperation.RESPONSES) + + assert payload["text"] == {"format": {"type": "json_object"}}, ( + "responses payloads must request a JSON object response when asked" + ) diff --git a/tests/test_llm_usage_normalization_properties.py b/tests/test_llm_usage_normalization_properties.py new file mode 100644 index 00000000..d27501be --- /dev/null +++ b/tests/test_llm_usage_normalization_properties.py @@ -0,0 +1,107 @@ +"""Property tests for mutually exclusive chat usage normalization.""" + +import typing as typ + +import hypothesis.strategies as st +import pytest +from hypothesis import given, settings + +from episodic.llm.openai_validation import ( + OpenAIResponseValidationError, + _normalize_chat_provider_call_usage, +) + +if typ.TYPE_CHECKING: + import collections.abc as cabc + +_TOKEN_COUNTS = st.integers(min_value=0, max_value=200) +_OPTIONAL_COUNTS = st.none() | st.integers(min_value=0, max_value=250) + +type _UsageCase = tuple[int, int, int | None, int | None, int | None] + +_USAGE_CASES = st.tuples( + _TOKEN_COUNTS, + _TOKEN_COUNTS, + _OPTIONAL_COUNTS, + _OPTIONAL_COUNTS, + _OPTIONAL_COUNTS, +) + + +def _usage_payload(case: _UsageCase) -> dict[str, object]: + """Build a chat usage payload with optional nested detail counts.""" + ( + prompt_tokens, + completion_tokens, + cached_input, + audio_input, + audio_output, + ) = case + payload: dict[str, object] = { + "prompt_tokens": prompt_tokens, + "completion_tokens": completion_tokens, + "total_tokens": prompt_tokens + completion_tokens, + } + prompt_details: dict[str, int] = {} + if cached_input is not None: + prompt_details["cached_tokens"] = cached_input + if audio_input is not None: + prompt_details["audio_tokens"] = audio_input + if prompt_details: + payload["prompt_tokens_details"] = prompt_details + if audio_output is not None: + payload["completion_tokens_details"] = {"audio_tokens": audio_output} + return payload + + +def _normalized_metrics( + payload: dict[str, object], +) -> cabc.Mapping[str, int]: + """Normalize one usage payload and return its canonical metrics.""" + usage = _normalize_chat_provider_call_usage( + {"id": "chatcmpl-property"}, + payload, + "stop", + ) + assert usage is not None, "expected usage metadata for a present payload" + return usage.usage_metrics + + +@given(case=_USAGE_CASES) +@settings(max_examples=100) +def test_chat_usage_metrics_partition_the_parent_totals(case: _UsageCase) -> None: + """Valid nested details partition the parent totals exactly once.""" + ( + prompt_tokens, + completion_tokens, + cached_input, + audio_input, + audio_output, + ) = case + payload = _usage_payload(case) + oversubscribed = (cached_input or 0) + (audio_input or 0) > prompt_tokens or ( + audio_output or 0 + ) > completion_tokens + + if oversubscribed: + with pytest.raises(OpenAIResponseValidationError): + _normalized_metrics(payload) + return + + metrics = _normalized_metrics(payload) + + input_total = ( + metrics["input_tokens"] + + metrics.get("cached_input_tokens", 0) + + metrics.get("audio_input_tokens", 0) + ) + output_total = metrics["output_tokens"] + metrics.get("audio_output_tokens", 0) + assert input_total == prompt_tokens, ( + f"input metrics must partition prompt_tokens; got {metrics!r}" + ) + assert output_total == completion_tokens, ( + f"output metrics must partition completion_tokens; got {metrics!r}" + ) + assert all(value >= 0 for value in metrics.values()), ( + f"normalized metrics must be non-negative; got {metrics!r}" + ) diff --git a/tests/test_local_k8s_config.py b/tests/test_local_k8s_config.py new file mode 100644 index 00000000..638fb16e --- /dev/null +++ b/tests/test_local_k8s_config.py @@ -0,0 +1,93 @@ +"""Tests for preview configuration credentials and Secret manifests.""" + +import pytest + +from scripts.local_k8s import commands +from scripts.local_k8s.config import PreviewConfig + + +def test_preview_config_reads_openai_key_from_environment( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A configured OPENAI_API_KEY reaches the preview configuration.""" + monkeypatch.setenv("OPENAI_API_KEY", "sk-env-test") + + config = PreviewConfig() + + assert config.openai_api_key == "sk-env-test", ( + "the preview config must read the OpenAI key from the environment" + ) + + +def test_preview_config_defaults_to_empty_openai_key( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unset OPENAI_API_KEY defaults to an empty string.""" + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + + config = PreviewConfig() + + assert not config.openai_api_key, "the preview config must default to no OpenAI key" + + +def test_preview_config_reads_openai_base_url_from_environment( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A configured OPENAI_BASE_URL overrides the provider default.""" + monkeypatch.setenv("OPENAI_BASE_URL", "https://llm.example.test/v1") + + config = PreviewConfig() + + assert config.openai_base_url == "https://llm.example.test/v1", ( + "the preview config must read the provider base URL from the environment" + ) + + +def test_preview_config_defaults_openai_base_url( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unset OPENAI_BASE_URL falls back to the public endpoint.""" + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + + config = PreviewConfig() + + assert config.openai_base_url == "https://api.openai.com/v1", ( + "the preview config must default to the public OpenAI endpoint" + ) + + +def test_preview_config_repr_hides_the_openai_key( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The dataclass representation must not expose the API key.""" + monkeypatch.setenv("OPENAI_API_KEY", "sk-secret-value") + + config = PreviewConfig() + + assert "sk-secret-value" not in repr(config), ( + "repr must not expose the OpenAI API key" + ) + + +def test_secret_manifest_rejects_invalid_namespace() -> None: + """A namespace outside DNS-1123 rules must fail manifest generation.""" + config = PreviewConfig(namespace="bad\nnamespace: injected") + + with pytest.raises(ValueError, match="namespace must be a DNS-1123 label"): + commands.secret_manifest(config) + + +def test_secret_manifest_escapes_newlines_in_values() -> None: + """Control characters in secret values cannot break out of the scalar.""" + config = PreviewConfig( + api_bearer_token="line-one\nline-two", # noqa: S106 - local-only test token. + ) + + manifest = commands.secret_manifest(config) + + assert '"line-one\\nline-two"' in manifest, ( + "newlines must be escaped inside the quoted YAML scalar" + ) + assert "\nline-two" not in manifest.replace("\\nline-two", ""), ( + "no raw newline from a value may reach the manifest structure" + ) diff --git a/tests/test_local_k8s_tooling.py b/tests/test_local_k8s_tooling.py index 0275db54..8659edec 100644 --- a/tests/test_local_k8s_tooling.py +++ b/tests/test_local_k8s_tooling.py @@ -2,6 +2,7 @@ import socket import subprocess +import sys import typing as typ from collections.abc import Callable # noqa: ICN003, TC003 - requested test shape. @@ -79,15 +80,68 @@ def test_helm_upgrade_command_uses_local_chart_values() -> None: assert str(config.values_path) in command, "local values path must be rendered." -def test_kubectl_secret_command_renders_database_url_literal() -> None: +def test_secret_manifest_renders_database_url() -> None: """Create the app Secret from the configured local database URL.""" config = PreviewConfig(database_url="postgresql+asyncpg://user:pass@postgres/db") - command = commands.kubectl_secret_command(config) + manifest = commands.secret_manifest(config) - assert "--from-literal=database-url=postgresql+asyncpg://user:pass@postgres/db" in ( - command - ), "Expected collection to contain the value" + assert ' database-url: "postgresql+asyncpg://user:pass@postgres/db"' in manifest, ( + "the preview secret must include the local database URL" + ) + + +def test_secret_manifest_renders_bearer_token() -> None: + """Create the app Secret with the local authorization bearer token.""" + config = PreviewConfig(api_bearer_token="alpha-token") # noqa: S106 - local-only test token. + + manifest = commands.secret_manifest(config) + + assert ' api-bearer-token: "alpha-token"' in manifest, ( + "the preview secret must include the local bearer token" + ) + + +def test_secret_manifest_omits_openai_pair_without_key() -> None: + """Omit the OpenAI entries when no key is configured.""" + config = PreviewConfig(openai_api_key="") + + manifest = commands.secret_manifest(config) + + assert "openai" not in manifest, ( + "the preview secret must omit OpenAI entries without a key" + ) + + +def test_secret_manifest_renders_openai_pair_with_key() -> None: + """Write the OpenAI base URL and key together when a key is configured.""" + config = PreviewConfig(openai_api_key="sk-local-test") + + manifest = commands.secret_manifest(config) + + assert ' openai-api-key: "sk-local-test"' in manifest, ( + "the preview secret must include the configured OpenAI key" + ) + assert f' openai-base-url: "{config.openai_base_url}"' in manifest, ( + "the preview secret must pair the base URL with the key" + ) + + +def test_secret_values_stay_out_of_command_arguments() -> None: + """Secret values must never appear in printable command arguments.""" + config = PreviewConfig( + api_bearer_token="alpha-token", # noqa: S106 - local-only test token. + openai_api_key="sk-local-test", + ) + + apply_command = commands.kubectl_apply_command(config) + + assert not any("alpha-token" in part for part in apply_command), ( + "the bearer token must travel via stdin, not command arguments" + ) + assert not any("sk-local-test" in part for part in apply_command), ( + "the OpenAI key must travel via stdin, not command arguments" + ) def test_loopback_port_validation_reports_occupied_port() -> None: @@ -247,3 +301,25 @@ def test_command_reports_missing_cluster_without_kubectl( assert "does not exist" in capsys.readouterr().out, ( "Expected collection to contain the value" ) + + +def test_command_runner_surfaces_captured_output_on_failure( + capsys: pytest.CaptureFixture[str], +) -> None: + """A failed command's captured stdout and stderr reach the operator.""" + runner = commands.CommandRunner() + script = ( + "import sys; sys.stdout.write('out-diag'); " + "sys.stderr.write('err-diag'); sys.exit(3)" + ) + + with pytest.raises(subprocess.CalledProcessError): + runner.run([sys.executable, "-c", script]) + + captured = capsys.readouterr() + assert "out-diag" in captured.err, ( + "captured stdout of a failed command must be surfaced on stderr" + ) + assert "err-diag" in captured.err, ( + "captured stderr of a failed command must be surfaced on stderr" + ) diff --git a/tests/test_runtime_configuration.py b/tests/test_runtime_configuration.py index 14b043ff..75940616 100644 --- a/tests/test_runtime_configuration.py +++ b/tests/test_runtime_configuration.py @@ -4,24 +4,32 @@ import pytest +from episodic.api.runtime import RuntimeConfigurationError, _load_runtime_config +from episodic.generation import GenerationSourceLimits + if typ.TYPE_CHECKING: from pathlib import Path -def test_load_runtime_config_uses_configured_pricing_directory( - tmp_path: "Path", # noqa: UP037 # Imported only during type checking. -) -> None: - """Pricing snapshots should be loaded from a validated configured directory.""" - from episodic.api.runtime import _load_runtime_config - +def _base_environment(tmp_path: Path) -> dict[str, str]: + """Return the minimal environment that boots the runtime configuration.""" pricing_directory = tmp_path / "pricing" - pricing_directory.mkdir() - config = _load_runtime_config({ + pricing_directory.mkdir(exist_ok=True) + return { "DATABASE_URL": "postgresql://example.test/episodic", "SOURCE_INTAKE_OBJECT_STORE_ROOT": str(tmp_path / "objects"), "PRICING_SNAPSHOT_DIRECTORY": str(pricing_directory), "API_AUTHORIZATION_BEARER_TOKEN": "test-token", "API_AUTHORIZATION_PRINCIPAL_ID": "test-principal", + } + + +def test_load_runtime_config_uses_configured_pricing_directory( + tmp_path: Path, +) -> None: + """Pricing snapshots should be loaded from a validated configured directory.""" + config = _load_runtime_config({ + **_base_environment(tmp_path), "GENERATION_MAX_SOURCE_COUNT": "3", "GENERATION_MAX_SOURCE_BYTES": "400", "GENERATION_MAX_AGGREGATE_SOURCE_BYTES": "800", @@ -30,7 +38,7 @@ def test_load_runtime_config_uses_configured_pricing_directory( "GENERATION_MAX_RESPONSE_BYTES": "4096", }) - assert config.pricing_snapshot_directory == pricing_directory.resolve(), ( + assert config.pricing_snapshot_directory == (tmp_path / "pricing").resolve(), ( f"expected configured pricing path, got {config.pricing_snapshot_directory}" ) assert config.generation_source_limits.max_source_count == 3, ( @@ -59,66 +67,159 @@ def test_load_runtime_config_uses_configured_pricing_directory( ) -@pytest.mark.parametrize("value", ["0", "-1", "not-an-integer"]) -def test_load_runtime_config_rejects_invalid_generation_source_limit( - tmp_path: "Path", # noqa: UP037 # Imported only during type checking. - value: str, -) -> None: - """Generation source limits must be positive integer runtime settings.""" - from episodic.api.runtime import RuntimeConfigurationError, _load_runtime_config +def test_load_runtime_config_applies_declared_defaults(tmp_path: Path) -> None: + """Optional provider and limit settings fall back to declared defaults.""" + config = _load_runtime_config(_base_environment(tmp_path)) - pricing_directory = tmp_path / "pricing" - pricing_directory.mkdir() + assert config.draft_model == "gpt-4o-mini", ( + f"expected the default draft model, got {config.draft_model!r}" + ) + assert config.llm_base_url is None, ( + f"expected no provider base URL by default, got {config.llm_base_url!r}" + ) + assert config.llm_api_key is None, "expected no provider API key by default" + defaults = GenerationSourceLimits() + assert config.generation_source_limits == defaults, ( + f"expected declared source-limit defaults {defaults!r}, got " + f"{config.generation_source_limits!r}" + ) + assert config.generation_max_output_tokens == 4_096, ( + "expected the declared default output-token limit, got " + f"{config.generation_max_output_tokens}" + ) + assert config.generation_max_response_bytes == 1_048_576, ( + "expected the declared default response-byte limit, got " + f"{config.generation_max_response_bytes}" + ) + + +def test_load_runtime_config_rejects_unpaired_openai_base_url( + tmp_path: Path, +) -> None: + """A provider base URL without an API key must fail configuration.""" with pytest.raises( RuntimeConfigurationError, - match="GENERATION_MAX_SOURCE_COUNT must be a positive integer", + match="OPENAI_BASE_URL and OPENAI_API_KEY must be configured together", ): _load_runtime_config({ - "DATABASE_URL": "postgresql://example.test/episodic", - "SOURCE_INTAKE_OBJECT_STORE_ROOT": str(tmp_path / "objects"), - "PRICING_SNAPSHOT_DIRECTORY": str(pricing_directory), - "API_AUTHORIZATION_BEARER_TOKEN": "test-token", - "API_AUTHORIZATION_PRINCIPAL_ID": "test-principal", - "GENERATION_MAX_SOURCE_COUNT": value, + **_base_environment(tmp_path), + "OPENAI_BASE_URL": "https://api.openai.example/v1", }) @pytest.mark.parametrize( - "setting", - ["GENERATION_MAX_OUTPUT_TOKENS", "GENERATION_MAX_RESPONSE_BYTES"], + ("setting", "value"), + [ + ("GENERATION_MAX_SOURCE_COUNT", "0"), + ("GENERATION_MAX_SOURCE_COUNT", "-1"), + ("GENERATION_MAX_SOURCE_COUNT", "not-an-integer"), + ("GENERATION_MAX_OUTPUT_TOKENS", "0"), + ("GENERATION_MAX_RESPONSE_BYTES", "0"), + ], ) -def test_load_runtime_config_rejects_invalid_generation_output_limit( - tmp_path: "Path", # noqa: UP037 # Imported only during type checking. +def test_load_runtime_config_rejects_invalid_generation_limit( + tmp_path: Path, setting: str, + value: str, ) -> None: - """Generation output limits must be positive integer runtime settings.""" - from episodic.api.runtime import RuntimeConfigurationError, _load_runtime_config - - pricing_directory = tmp_path / "pricing" - pricing_directory.mkdir() + """Generation limits must be positive integer runtime settings.""" with pytest.raises( RuntimeConfigurationError, match=f"{setting} must be a positive integer", ): _load_runtime_config({ - "DATABASE_URL": "postgresql://example.test/episodic", - "SOURCE_INTAKE_OBJECT_STORE_ROOT": str(tmp_path / "objects"), - "PRICING_SNAPSHOT_DIRECTORY": str(pricing_directory), - "API_AUTHORIZATION_BEARER_TOKEN": "test-token", - "API_AUTHORIZATION_PRINCIPAL_ID": "test-principal", - setting: "0", + **_base_environment(tmp_path), + setting: value, }) def test_load_runtime_config_rejects_missing_pricing_directory( - tmp_path: "Path", # noqa: UP037 # Imported only during type checking. + tmp_path: Path, ) -> None: """Pricing configuration should fail before launcher construction.""" - from episodic.api.runtime import RuntimeConfigurationError, _load_runtime_config - with pytest.raises(RuntimeConfigurationError, match="PRICING_SNAPSHOT_DIRECTORY"): _load_runtime_config({ "DATABASE_URL": "postgresql://example.test/episodic", "SOURCE_INTAKE_OBJECT_STORE_ROOT": str(tmp_path / "objects"), "PRICING_SNAPSHOT_DIRECTORY": str(tmp_path / "missing"), }) + + +@pytest.mark.parametrize( + "missing_setting", + ["DATABASE_URL", "SOURCE_INTAKE_OBJECT_STORE_ROOT"], +) +def test_load_runtime_config_failure_does_not_log_success( + tmp_path: Path, + missing_setting: str, + caplog: pytest.LogCaptureFixture, +) -> None: + """A failed load must not emit the runtime_config_loaded success line.""" + environment = _base_environment(tmp_path) + del environment[missing_setting] + + with ( + caplog.at_level("INFO"), + pytest.raises(RuntimeConfigurationError, match=missing_setting), + ): + _load_runtime_config(environment) + + assert "runtime_config_loaded" not in caplog.text, ( + "the success log line must not be emitted for invalid configuration" + ) + + +def test_load_runtime_config_loads_provider_request_options( + tmp_path: Path, +) -> None: + """Configured OPENAI_* request options reach the runtime configuration.""" + config = _load_runtime_config({ + **_base_environment(tmp_path), + "OPENAI_REASONING_EFFORT": "low", + "OPENAI_SERVICE_TIER": "flex", + "OPENAI_TOKEN_LIMIT_PARAM": "max_completion_tokens", + "OPENAI_TIMEOUT_SECONDS": "600", + }) + + assert config.llm_reasoning_effort == "low", ( + f"expected the configured reasoning effort, got {config.llm_reasoning_effort!r}" + ) + assert config.llm_service_tier == "flex", ( + f"expected the configured service tier, got {config.llm_service_tier!r}" + ) + assert config.llm_token_limit_param == "max_completion_tokens", ( # noqa: S105 - parameter name, not a secret. + f"expected the configured token parameter, got {config.llm_token_limit_param!r}" + ) + assert config.llm_timeout_seconds == 600.0, ( + f"expected the configured timeout, got {config.llm_timeout_seconds!r}" + ) + + +def test_load_runtime_config_rejects_unknown_token_limit_param( + tmp_path: Path, +) -> None: + """Unknown token-limit parameter names fail configuration.""" + with pytest.raises( + RuntimeConfigurationError, + match="OPENAI_TOKEN_LIMIT_PARAM must be max_tokens or", + ): + _load_runtime_config({ + **_base_environment(tmp_path), + "OPENAI_TOKEN_LIMIT_PARAM": "max_words", + }) + + +@pytest.mark.parametrize("value", ["0", "-3", "abc", "inf", "nan"]) +def test_load_runtime_config_rejects_invalid_timeout( + tmp_path: Path, + value: str, +) -> None: + """The provider timeout must be a positive, finite number of seconds.""" + with pytest.raises( + RuntimeConfigurationError, + match="OPENAI_TIMEOUT_SECONDS must be a positive number", + ): + _load_runtime_config({ + **_base_environment(tmp_path), + "OPENAI_TIMEOUT_SECONDS": value, + }) diff --git a/tests/test_runtime_metrics_wiring.py b/tests/test_runtime_metrics_wiring.py index 0866280d..93e2d713 100644 --- a/tests/test_runtime_metrics_wiring.py +++ b/tests/test_runtime_metrics_wiring.py @@ -143,3 +143,49 @@ async def test_generation_route_metrics_use_injected_monotonic_clock() -> None: assert metrics.latencies == [ ("generation_api_request_latency_ms", 250.0, expected_labels) ], metrics.latencies + + +@pytest.mark.asyncio +async def test_create_app_from_env_shares_composition_root_tracer( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """The launcher must receive the same tracer instance as ApiDependencies.""" + from episodic.api import runtime as runtime_module + from episodic.generation import InProcessGenerationRunLauncher + from episodic.observability import StructuredLogTracer + + monkeypatch.setenv("DATABASE_URL", "postgresql://example.test/episodic") + monkeypatch.setenv("SOURCE_INTAKE_OBJECT_STORE_ROOT", str(tmp_path)) + monkeypatch.setenv("API_AUTHORIZATION_BEARER_TOKEN", "runtime-test-token") + monkeypatch.setenv("API_AUTHORIZATION_PRINCIPAL_ID", "runtime-test-principal") + monkeypatch.setenv("OPENAI_BASE_URL", "https://llm.example.test/v1") + monkeypatch.setenv("OPENAI_API_KEY", "test-key") + captured_dependencies: ApiDependencies | None = None + + def capture_dependencies(dependencies: ApiDependencies) -> object: + nonlocal captured_dependencies + captured_dependencies = dependencies + return object() + + with mock.patch.object( + runtime_module, + "create_app", + side_effect=capture_dependencies, + ): + runtime_module.create_app_from_env() + + assert captured_dependencies is not None, "expected captured dependencies, got None" + assert isinstance(captured_dependencies.launcher, InProcessGenerationRunLauncher), ( + "expected an in-process launcher, got " + f"{type(captured_dependencies.launcher).__name__}" + ) + assert isinstance(captured_dependencies.tracer, StructuredLogTracer), ( + f"expected a structured-log tracer, got " + f"{type(captured_dependencies.tracer).__name__}" + ) + assert captured_dependencies.launcher.tracer is captured_dependencies.tracer, ( + "the launcher must share the composition root's tracer instance" + ) + + await captured_dependencies.shutdown_hooks[0]() diff --git a/tests/test_skylos_lint_contract.py b/tests/test_skylos_lint_contract.py index 9d3a9513..741e11c4 100644 --- a/tests/test_skylos_lint_contract.py +++ b/tests/test_skylos_lint_contract.py @@ -131,8 +131,8 @@ def test_skylos_allow_requires_name_and_reason() -> None: required_fragments = ( "skylos-allow: ## Document one named Skylos exception, not an entry point", - "skylos-allow: export SKYLOS_NAME = $(value NAME)", - "skylos-allow: export SKYLOS_REASON = $(value REASON)", + "skylos-allow: export SKYLOS_NAME = $(call cli_value,NAME)", + "skylos-allow: export SKYLOS_REASON = $(call cli_value,REASON)", 'test -n "$${SKYLOS_NAME}"', 'test -n "$${SKYLOS_REASON}"', "NAME is required for a named whitelist exception", @@ -144,7 +144,9 @@ def test_skylos_allow_requires_name_and_reason() -> None: assert not missing_fragments, ( f"Expected skylos-allow target requirements; missing {missing_fragments!r}." ) - command = '$(SKYLOS) whitelist "$${SKYLOS_NAME}" --reason "$${SKYLOS_REASON}"' + # The whitelist subcommand must run without the --config-file prefix that + # $(SKYLOS) carries; global options stop Skylos dispatching the subcommand. + command = '$(SKYLOS_CLI) whitelist "$${SKYLOS_NAME}" --reason "$${SKYLOS_REASON}"' assert makefile.count(command) == 1, ( "Expected exactly one safely quoted Skylos whitelist command." ) @@ -172,7 +174,7 @@ def test_skylos_allow_preserves_metacharacters_as_arguments(tmp_path: Path) -> N "skylos-allow", f"NAME={name}", f"REASON={reason}", - f"SKYLOS={recorder}", + f"SKYLOS_CLI={recorder}", ], cwd=REPOSITORY_ROOT, env={**os.environ, "SKYLOS_CAPTURE": str(capture)}, @@ -226,7 +228,7 @@ def test_skylos_allow_rejects_missing_required_value( "--no-print-directory", "skylos-allow", provided_assignment, - f"SKYLOS={recorder}", + f"SKYLOS_CLI={recorder}", ], cwd=REPOSITORY_ROOT, env={**os.environ, "SKYLOS_CAPTURE": str(capture)}, diff --git a/tests/test_source_idempotency_encoding.py b/tests/test_source_idempotency_encoding.py index fefc8205..06d5ac05 100644 --- a/tests/test_source_idempotency_encoding.py +++ b/tests/test_source_idempotency_encoding.py @@ -14,3 +14,16 @@ def test_idempotency_outcome_encoding_rejects_large_payloads() -> None: with pytest.raises(ValueError, match="64 KiB"): _encode_outcome(response) + + +def test_idempotency_outcome_encoding_accepts_small_payloads() -> None: + """Replay envelopes below the 64 KiB cap encode to non-empty bytes.""" + response = IdempotentResponse( + "201 Created", + {"content": "small payload"}, + ) + + payload = _encode_outcome(response) + + assert isinstance(payload, bytes), f"expected bytes, got {type(payload)!r}" + assert payload, "expected a non-empty serialized replay envelope" diff --git a/tests/test_uow_autospec_regression.py b/tests/test_uow_autospec_regression.py new file mode 100644 index 00000000..47b0ae22 --- /dev/null +++ b/tests/test_uow_autospec_regression.py @@ -0,0 +1,25 @@ +"""Regression coverage for runtime-evaluated unit-of-work annotations.""" + +from unittest import mock + +from episodic.canonical.storage import SqlAlchemyUnitOfWork + + +def test_unit_of_work_supports_autospec_creation() -> None: + """Autospeccing the unit of work must evaluate its annotations. + + ``mock.create_autospec`` calls ``inspect.signature``, which evaluates + the ``__init__`` and ``__aexit__`` annotations at runtime. Those + annotations reference ``collections.abc`` and ``types.TracebackType``, + so this test fails with ``NameError`` if the imports move back behind + ``typing.TYPE_CHECKING``. + """ + specced = mock.create_autospec(SqlAlchemyUnitOfWork, instance=True) + + assert specced is not None, "expected an autospecced unit of work" + assert isinstance(specced.__aenter__, mock.AsyncMock), ( + "the autospecced unit of work must expose an awaitable __aenter__" + ) + assert isinstance(specced.__aexit__, mock.AsyncMock), ( + "the autospecced unit of work must expose an awaitable __aexit__" + )