From 83d79a8b4775453b73d88b0c5ca2c9a95956c28c Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sat, 22 Aug 2026 23:40:57 +0100 Subject: [PATCH 01/26] Import runtime-evaluated annotation names unconditionally in the UoW `mock.create_autospec` (used by the runtime metrics wiring tests) calls `inspect.signature` on `SqlAlchemyUnitOfWork`, which evaluates the method annotations at runtime. The names those annotations reference were imported under `typing.TYPE_CHECKING`, so autospeccing the class raised `NameError: name 'cabc' is not defined`. Import them unconditionally, with `TC00x` suppressions documenting why they cannot live in a type-checking block. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- episodic/canonical/storage/uow.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) 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__) From 5292ac8dda0f7c430aa8ca3a352e22970f7a2df1 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sat, 22 Aug 2026 23:40:57 +0100 Subject: [PATCH 02/26] Add provider request options, JSON mode, and a configurable timeout Driving the no-QA generation slice against a real reasoning model (`gpt-5.6-sol`) exposed three gaps in the OpenAI-compatible adapter: - The draft prompt demands JSON but nothing constrained the provider response, so models wrapped JSON in markdown fences and the fail-fast parser rejected the run. `LLMRequest` gains `json_response`, emitted as `response_format={"type": "json_object"}` (chat completions) or `text.format` (Responses API). - Reasoning models reject `max_tokens` (requiring `max_completion_tokens`) and accept `reasoning_effort` and `service_tier`. `OpenAIPayloadOptions` carries these, configured via `OPENAI_REASONING_EFFORT`, `OPENAI_SERVICE_TIER`, and `OPENAI_TOKEN_LIMIT_PARAM`. - The per-request HTTP timeout was hard-coded at 30 s, which a reasoning model drafting a full episode script comfortably exceeds; every attempt was cancelled client-side and the run failed with "Transient provider failure after exhausting retries". `OPENAI_TIMEOUT_SECONDS` now configures it. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- episodic/api/runtime.py | 4 ++ episodic/api/runtime_config.py | 41 +++++++++++ episodic/generation/draft_script.py | 1 + episodic/llm/openai_api/adapter.py | 13 +++- episodic/llm/openai_api/request.py | 75 +++++++++++++++++-- episodic/llm/ports.py | 4 ++ tests/test_llm_openai_request_payload.py | 92 ++++++++++++++++++++++++ 7 files changed, 225 insertions(+), 5 deletions(-) create mode 100644 tests/test_llm_openai_request_payload.py diff --git a/episodic/api/runtime.py b/episodic/api/runtime.py index d8680150..ea45b39f 100644 --- a/episodic/api/runtime.py +++ b/episodic/api/runtime.py @@ -83,6 +83,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, ) ) diff --git a/episodic/api/runtime_config.py b/episodic/api/runtime_config.py index ea08620d..088ecb5d 100644 --- a/episodic/api/runtime_config.py +++ b/episodic/api/runtime_config.py @@ -19,6 +19,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] @@ -73,6 +74,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 +231,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 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,6 +282,7 @@ 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) @@ -262,6 +299,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], 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/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/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" + ) From 7d154f2843f8f98c9702cc3291f46cadf0914bec Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sat, 22 Aug 2026 23:40:57 +0100 Subject: [PATCH 03/26] Omit zero-valued optional usage metrics from provider-call usage `gpt-5.6-sol` reports `audio_tokens: 0` in both usage detail blocks. Recording those zero-valued metrics made cost pricing fail with "usage contains unpriced metrics: audio_input_tokens, audio_output_tokens", because the pricing engine requires a rate for every recorded metric. A zero-valued optional metric carries no cost information, and pricing it would force every snapshot to price every modality the provider mentions, so drop zeros at normalization. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- episodic/llm/openai_validation.py | 9 +++- .../test_llm_openai_adapter_usage_metering.py | 45 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/episodic/llm/openai_validation.py b/episodic/llm/openai_validation.py index df90c7d4..588f2573 100644 --- a/episodic/llm/openai_validation.py +++ b/episodic/llm/openai_validation.py @@ -196,8 +196,13 @@ 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 diff --git a/tests/test_llm_openai_adapter_usage_metering.py b/tests/test_llm_openai_adapter_usage_metering.py index 76178488..00ace4cb 100644 --- a/tests/test_llm_openai_adapter_usage_metering.py +++ b/tests/test_llm_openai_adapter_usage_metering.py @@ -122,3 +122,48 @@ 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" From d3a7a340f7ac490069cf0686bf42446bf71b4524 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sat, 22 Aug 2026 23:41:13 +0100 Subject: [PATCH 04/26] Persist pricing snapshots before pinning or recording against them `run_pricing_pins.pricing_snapshot_id` and `cost_ledger_entries.pricing_snapshot_id` are foreign keys into `pricing_snapshots`, but nothing ever synchronized the file-based pricing catalogue into that table, so the first generation run to reach cost pinning on any fresh database failed with a `ForeignKeyViolationError`. Add `ensure_snapshot` to `CostLedgerPort`: the recorder now persists the resolved snapshot (an idempotent `ON CONFLICT DO NOTHING` insert keyed on the immutable snapshot identifier) before pinning it for a run and before recording a provider call against it. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- episodic/cost/ports.py | 10 +++++ episodic/cost/recorder.py | 2 + episodic/cost/storage/adapters.py | 26 +++++++++++++ tests/test_cost_ports_protocols.py | 4 ++ tests/test_cost_recorder.py | 6 +++ tests/test_cost_storage_ledger.py | 61 ++++++++++++++++++++++++++++++ 6 files changed, 109 insertions(+) diff --git a/episodic/cost/ports.py b/episodic/cost/ports.py index 249ab34c..9b8a4c5d 100644 --- a/episodic/cost/ports.py +++ b/episodic/cost/ports.py @@ -218,6 +218,16 @@ 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. + Snapshots are immutable: repeated calls with the same identifier + must leave the stored row unchanged. + """ + raise NotImplementedError + async def pin_run_pricing( self, key: RunPricingKey, diff --git a/episodic/cost/recorder.py b/episodic/cost/recorder.py index 6aec699c..fa13c316 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, @@ -250,6 +251,7 @@ async def record_provider_call( If pricing or ledger validation fails. """ # noqa: DOC502 # Collaborating ports propagate these domain exceptions. snapshot = await self._resolve_snapshot_for_record(record) + 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..e625759e 100644 --- a/episodic/cost/storage/adapters.py +++ b/episodic/cost/storage/adapters.py @@ -25,6 +25,7 @@ LedgerScope, MeteringCounterKey, PricingModel, + PricingSnapshot, PricingSnapshotId, ProviderCallLedgerEntry, RunPricingKey, @@ -36,6 +37,7 @@ CostLedgerEntryRecord, MeteringCounterEventRecord, MeteringCounterRecord, + PricingSnapshotRecord, RunPricingPinRecord, ) @@ -115,6 +117,30 @@ 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.""" + 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.", + ), + ) + .on_conflict_do_nothing(index_elements=["id"]) + ) + await self._session.execute(statement) + async def pin_run_pricing( self, key: RunPricingKey, diff --git a/tests/test_cost_ports_protocols.py b/tests/test_cost_ports_protocols.py index be211b28..8ffa481b 100644 --- a/tests/test_cost_ports_protocols.py +++ b/tests/test_cost_ports_protocols.py @@ -57,6 +57,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, diff --git a/tests/test_cost_recorder.py b/tests/test_cost_recorder.py index da242dbf..d6ecd750 100644 --- a/tests/test_cost_recorder.py +++ b/tests/test_cost_recorder.py @@ -62,6 +62,12 @@ class _PinnedLedger: pinned_snapshot_id: PricingSnapshotId recorded_call: ProviderCallLedgerEntry | None = None + ensured_snapshots: list[PricingSnapshot] = dc.field(default_factory=list) + + async def ensure_snapshot(self, snapshot: PricingSnapshot) -> None: + """Capture snapshots the recorder persists, mirroring idempotency.""" + if snapshot not in self.ensured_snapshots: + self.ensured_snapshots.append(snapshot) async def pin_run_pricing( self, diff --git a/tests/test_cost_storage_ledger.py b/tests/test_cost_storage_ledger.py index bff83f76..d534cd40 100644 --- a/tests/test_cost_storage_ledger.py +++ b/tests/test_cost_storage_ledger.py @@ -13,7 +13,9 @@ IdempotencyKey, LedgerScope, PricingModel, + PricingSnapshot, PricingSnapshotId, + PricingSourceKind, ProviderCallLedgerEntry, RunPricingKey, TaskRollupLedgerEntry, @@ -230,3 +232,62 @@ 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", + ) + + +@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) + async with session_factory() as session: + store = SqlAlchemyCostLedgerStore(session) + await store.ensure_snapshot(snapshot) + await store.ensure_snapshot(snapshot) + 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() + 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 pins == 1, "the pinned snapshot must satisfy the foreign key" From 0eed5c704847ddfc7ec33ee5ad15fb2be8e24ef4 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sat, 22 Aug 2026 23:41:13 +0100 Subject: [PATCH 05/26] Ship pricing snapshots in the image and add the 2026-08 snapshot The pricing catalogue resolves snapshots by exact provider/model/operation/billing-period match, and a missing snapshot fails the whole generation run. Add the August 2026 snapshot for `gpt-5.6-sol` chat completions (USD 2.00/M input, 10.00/M output, 0.20/M cached input, standard tier) and copy `config/pricing-snapshots` into the runtime image so `PRICING_SNAPSHOT_DIRECTORY` can point at a path that exists in the container. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- Dockerfile | 4 ++++ config/pricing-snapshots/openai-2026-08.yaml | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 config/pricing-snapshots/openai-2026-08.yaml 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/config/pricing-snapshots/openai-2026-08.yaml b/config/pricing-snapshots/openai-2026-08.yaml new file mode 100644 index 00000000..0569b90b --- /dev/null +++ b/config/pricing-snapshots/openai-2026-08.yaml @@ -0,0 +1,16 @@ +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 + reasoning_tokens: 1000 +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" From 3bd7d8b0c97b116ad1e6f6a2b0f25353d761fc1e Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sat, 22 Aug 2026 23:41:13 +0100 Subject: [PATCH 06/26] Give the chart volume support and boot-required local preview values The rendered local preview could never boot: the runtime refuses to start without `SOURCE_INTAKE_OBJECT_STORE_ROOT`, `API_AUTHORIZATION_BEARER_TOKEN`/`_PRINCIPAL_ID`, and a valid pricing snapshot directory, none of which the chart supplied, and the container's `readOnlyRootFilesystem` left the source-intake object store nowhere to write. - Add pass-through `volumes`/`volumeMounts` values to the Deployment template; the chart previously had no volume support at all. - Mount an `emptyDir` at `/tmp` in the local values so the object store can write while the root filesystem stays read-only. - Supply the boot-required settings and the `gpt-5.6-sol` draft-model configuration (low reasoning effort, `max_completion_tokens`, 600 s provider timeout) in `values.local.yaml`, with optional `OPENAI_BASE_URL`/`OPENAI_API_KEY` secret references. - Update the chart contract tests to pin the new rendering. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- charts/episodic/templates/deployment.yaml | 8 +++ charts/episodic/values.local.yaml | 34 +++++++++++ charts/episodic/values.yaml | 6 ++ tests/test_helm_chart_contract.py | 72 ++++++++++++++++++----- 4 files changed, 104 insertions(+), 16 deletions(-) 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/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( From 49ca84080853a342354d686af1b3f4f35a86b007 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sat, 22 Aug 2026 23:41:31 +0100 Subject: [PATCH 07/26] Provision auth and OpenAI credentials in the local preview secret The preview secret only carried the database URL, so the app pod sat in `CreateContainerConfigError` once the chart began referencing the bearer-token key. The tooling now writes `api-bearer-token` (default `local-dev-token`) and, when `OPENAI_API_KEY` is present in the operator's environment, the paired `openai-base-url`/`openai-api-key` literals. `CommandRunner.run` also surfaces captured stdout/stderr on failure; previously `capture_output=True` swallowed the diagnostics of any failing provisioning command (a failed `kind create cluster` printed nothing but a Python traceback). Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- scripts/local_k8s/commands.py | 39 +++++++++++++++++++++++---------- scripts/local_k8s/config.py | 9 ++++++++ tests/test_local_k8s_tooling.py | 36 ++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 11 deletions(-) diff --git a/scripts/local_k8s/commands.py b/scripts/local_k8s/commands.py index add6d7a4..1d223956 100644 --- a/scripts/local_k8s/commands.py +++ b/scripts/local_k8s/commands.py @@ -2,6 +2,7 @@ import dataclasses as dc import subprocess +import sys import typing as typ import urllib.parse as urlparse @@ -44,13 +45,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]: @@ -253,17 +263,24 @@ def kubectl_apply_command(config: PreviewConfig) -> list[str]: def kubectl_secret_command(config: PreviewConfig) -> list[str]: """Build the idempotent application Secret creation command.""" - return [ + command = [ *_kubectl_ns_cmd(config), "create", "secret", "generic", config.secret_name, f"--from-literal=database-url={config.database_url}", - "--dry-run=client", - "-o", - "yaml", + f"--from-literal=api-bearer-token={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. + command.extend([ + f"--from-literal=openai-base-url={config.openai_base_url}", + f"--from-literal=openai-api-key={config.openai_api_key}", + ]) + command.extend(["--dry-run=client", "-o", "yaml"]) + return command def _yaml_string(value: str) -> str: diff --git a/scripts/local_k8s/config.py b/scripts/local_k8s/config.py index 509108da..67ca51e3 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 @@ -32,6 +33,14 @@ 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 = "https://api.openai.com/v1" + # Read at construction so `OPENAI_API_KEY=... make local-k8s-up` wires + # generation without committing a credential anywhere. + openai_api_key: str = dc.field( + 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/tests/test_local_k8s_tooling.py b/tests/test_local_k8s_tooling.py index 0275db54..f4dd3a8d 100644 --- a/tests/test_local_k8s_tooling.py +++ b/tests/test_local_k8s_tooling.py @@ -90,6 +90,42 @@ def test_kubectl_secret_command_renders_database_url_literal() -> None: ), "Expected collection to contain the value" +def test_kubectl_secret_command_renders_bearer_token_literal() -> None: + """Create the app Secret with the local authorization bearer token.""" + config = PreviewConfig(api_bearer_token="alpha-token") # noqa: S106 - local-only test token. + + command = commands.kubectl_secret_command(config) + + assert "--from-literal=api-bearer-token=alpha-token" in command, ( + "the preview secret must include the local bearer token" + ) + + +def test_kubectl_secret_command_omits_openai_pair_without_key() -> None: + """Omit the OpenAI literals when no key is configured.""" + config = PreviewConfig(openai_api_key="") + + command = commands.kubectl_secret_command(config) + + assert not any("openai" in part for part in command), ( + "the preview secret must omit OpenAI literals without a key" + ) + + +def test_kubectl_secret_command_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") + + command = commands.kubectl_secret_command(config) + + assert "--from-literal=openai-api-key=sk-local-test" in command, ( + "the preview secret must include the configured OpenAI key" + ) + assert f"--from-literal=openai-base-url={config.openai_base_url}" in command, ( + "the preview secret must pair the base URL with the key" + ) + + def test_loopback_port_validation_reports_occupied_port() -> None: """Reject a local ingress port that is already bound.""" with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as listener: From 44ac35f19148c33c0a3629c9b3bc48ae163d1551 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sat, 22 Aug 2026 23:41:31 +0100 Subject: [PATCH 08/26] Fix the Skylos interpreter pin and the skylos-allow target Two independent tooling defects: - Skylos parses sources with its own runtime's `ast`, so resolving an older default Python misreads 3.14 syntax and reports phantom dead code (`SKY-U003`/`SKY-U004`). Pin the tool interpreter with `uv tool run --python 3.14` (as first done on the `code-duplication-gate` branch). - `make skylos-allow` always failed with "unrecognized arguments: --reason": the `whitelist` subcommand only dispatches when it is Skylos's first argument, and the shared `$(SKYLOS)` macro inserted `--config-file` before it. Split out a bare `$(SKYLOS_CLI)` macro for the subcommand, and accept `NAME`/`REASON` only from the make command line so an ambient `NAME` environment variable cannot leak into the whitelist (the contract tests had been writing the host's `NAME` value and a shell-injection probe into the real `pyproject.toml` once the target started working). Update the contract tests to pin the fixed behaviour. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- Makefile | 18 ++++++++++++++---- tests/test_skylos_lint_contract.py | 12 +++++++----- 2 files changed, 21 insertions(+), 9 deletions(-) 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/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)}, From 0e61b79633f8fcad7dc5c091a79a637c452c3253 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sat, 22 Aug 2026 23:41:31 +0100 Subject: [PATCH 09/26] Document preview migrations, object-store lifetime, and alpha findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The users' guide said "apply the latest Alembic migrations" without saying how against the Kubernetes preview; document the port-forward-and-run procedure that works. Also document that the preview object store is an `emptyDir` whose blobs die with the pod while upload rows persist in Postgres, so uploads must be redone after any application pod restart. Add `docs/alpha-test-4-3-2-setup-notes.md`: a chronological record of the 4.3.2 alpha test on WSL2 with rootless podman and kind — tools installed, where the repository documentation was wrong, every failure mode hit (netavark firewall driver, inotify exhaustion, provider JSON/parameter/timeout gaps, pricing-pin foreign key, zero-valued usage metrics, WSL VM crashes and the overlay-storage repair), and what fixed each. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- docs/alpha-test-4-3-2-setup-notes.md | 221 +++++++++++++++++++++++++++ docs/users-guide.md | 21 ++- 2 files changed, 241 insertions(+), 1 deletion(-) create mode 100644 docs/alpha-test-4-3-2-setup-notes.md 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..86ae482d --- /dev/null +++ b/docs/alpha-test-4-3-2-setup-notes.md @@ -0,0 +1,221 @@ +# 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) | + +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. diff --git a/docs/users-guide.md b/docs/users-guide.md index 905e1c56..1f873656 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -90,7 +90,17 @@ 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: + +```shell +kubectl --context kind-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 @@ -412,6 +422,15 @@ 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. + 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 From 989f5f3a2f57ba5847529bf69fc4590d75228cf4 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sat, 22 Aug 2026 23:48:08 +0100 Subject: [PATCH 10/26] Record the successful end-to-end no-QA generation run With every fix on this branch deployed, the full 4.3.2 workflow completed against the local podman/kind preview: uploads through `draft_without_qa` generation to TEI P5 download. The `gpt-5.6-sol` draft consumed 13,803 input and 2,476 output tokens and produced a 39-turn script following the show specification. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- docs/alpha-test-4-3-2-setup-notes.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/alpha-test-4-3-2-setup-notes.md b/docs/alpha-test-4-3-2-setup-notes.md index 86ae482d..48289d53 100644 --- a/docs/alpha-test-4-3-2-setup-notes.md +++ b/docs/alpha-test-4-3-2-setup-notes.md @@ -219,3 +219,15 @@ never an option on this host; the documented rootless-Podman guidance (use 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. From c71c3dd43f2dfb2db1946a458138130b264f961d Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 23 Aug 2026 00:35:19 +0100 Subject: [PATCH 11/26] Harden runtime composition from review feedback - Emit the `runtime_config_loaded` success log only after every `RuntimeConfig` argument has validated; previously a missing `DATABASE_URL` or `SOURCE_INTAKE_OBJECT_STORE_ROOT` logged success immediately before raising. - Reject non-finite `OPENAI_TIMEOUT_SECONDS` values; `inf` and `nan` parsed as floats and slipped past the positive-value check. - Share the composition root's tracer with the generation launcher via a `tracer` field on `_GenerationLauncherRuntime` instead of constructing a second `StructuredLogTracer` inside the builder. - Name the draft materialisation fallback `_DEFAULT_MAX_SOURCE_COUNT` rather than passing a bare literal. - Rework `tests/test_runtime_configuration.py`: module-scope imports, unquoted `Path` annotations, a shared base-environment helper, and new coverage for the unpaired `OPENAI_BASE_URL` error, declared defaults, and the no-success-log-on-failure contract. Add a wiring test asserting the launcher shares `ApiDependencies`' tracer. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- episodic/api/resources/generation_runs.py | 18 +++- episodic/api/runtime.py | 8 +- episodic/api/runtime_config.py | 29 ++++-- tests/test_env_runtime_wiring.py | 7 +- tests/test_runtime_configuration.py | 118 ++++++++++++++++------ tests/test_runtime_metrics_wiring.py | 46 +++++++++ 6 files changed, 178 insertions(+), 48 deletions(-) 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 ea45b39f..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 @@ -135,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: @@ -167,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, ) @@ -259,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 088ecb5d..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 @@ -57,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 -------- @@ -256,7 +272,7 @@ def _llm_timeout_seconds(environment: cabc.Mapping[str, str]) -> float: except ValueError as exc: msg = "OPENAI_TIMEOUT_SECONDS must be a positive number." raise RuntimeConfigurationError(msg) from exc - if value <= 0: + if not math.isfinite(value) or value <= 0: msg = "OPENAI_TIMEOUT_SECONDS must be a positive number." raise RuntimeConfigurationError(msg) return value @@ -286,11 +302,7 @@ def _load_runtime_config( 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", @@ -311,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/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_runtime_configuration.py b/tests/test_runtime_configuration.py index 14b043ff..743994fe 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,26 +67,58 @@ def test_load_runtime_config_uses_configured_pricing_directory( ) +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)) + + 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="OPENAI_BASE_URL and OPENAI_API_KEY must be configured together", + ): + _load_runtime_config({ + **_base_environment(tmp_path), + "OPENAI_BASE_URL": "https://api.openai.example/v1", + }) + + @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. + tmp_path: Path, value: str, ) -> None: """Generation source limits must be positive integer runtime settings.""" - from episodic.api.runtime import RuntimeConfigurationError, _load_runtime_config - - pricing_directory = tmp_path / "pricing" - pricing_directory.mkdir() with pytest.raises( RuntimeConfigurationError, match="GENERATION_MAX_SOURCE_COUNT 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", + **_base_environment(tmp_path), "GENERATION_MAX_SOURCE_COUNT": value, }) @@ -88,37 +128,51 @@ def test_load_runtime_config_rejects_invalid_generation_source_limit( ["GENERATION_MAX_OUTPUT_TOKENS", "GENERATION_MAX_RESPONSE_BYTES"], ) def test_load_runtime_config_rejects_invalid_generation_output_limit( - tmp_path: "Path", # noqa: UP037 # Imported only during type checking. + tmp_path: Path, setting: 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() 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", + **_base_environment(tmp_path), setting: "0", }) 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" + ) 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]() From 970b67f5e616a6cc3e569e844cb5c30aa7ff8e11 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 23 Aug 2026 00:35:19 +0100 Subject: [PATCH 12/26] Price provider usage once and harden snapshot persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenAI reports cached and audio token counts as subsets of the prompt and completion totals, but normalization recorded both the parent totals and the nested counts while the pricing engine sums every metric independently — charging the overlap twice. Usage metrics are now mutually exclusive: cached and audio counts are subtracted from their parent totals and priced under their own rates, and reasoning tokens stay inside `output_tokens` (they bill at the output rate) instead of becoming a separately priced metric. The `gpt-5.6-sol` snapshot drops its now-unused `reasoning_tokens` rate. Snapshot persistence gains three hardenings: - `ensure_snapshot` now persists `effective_from`, carried through the domain `PricingSnapshot` from the catalogue YAML; previously the stored row lost the effective-dating metadata included in its own content hash. - A duplicate content hash under a different snapshot identifier now raises the new `PricingSnapshotCollisionError` instead of surfacing a raw `IntegrityError`. - `record_provider_call` persists the snapshot only on the unpinned fallback path; a pinned call's foreign key already guarantees the stored row exists, so the per-call insert was redundant. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- config/pricing-snapshots/openai-2026-08.yaml | 1 - episodic/cost/__init__.py | 2 + episodic/cost/ports.py | 32 +++-- .../cost/pricing_catalogue/file_loader.py | 3 + episodic/cost/recorder.py | 8 +- episodic/cost/storage/adapters.py | 47 ++++++- episodic/llm/openai_validation.py | 132 +++++++++--------- ...est_llm_openai_adapter_usage_metering.ambr | 8 +- tests/test_cost_recorder.py | 45 +++++- tests/test_cost_storage_ledger.py | 44 ++++++ 10 files changed, 231 insertions(+), 91 deletions(-) diff --git a/config/pricing-snapshots/openai-2026-08.yaml b/config/pricing-snapshots/openai-2026-08.yaml index 0569b90b..ce510a38 100644 --- a/config/pricing-snapshots/openai-2026-08.yaml +++ b/config/pricing-snapshots/openai-2026-08.yaml @@ -9,7 +9,6 @@ rates_minor_per_metric: input_tokens: 200 output_tokens: 1000 cached_input_tokens: 20 - reasoning_tokens: 1000 source_metadata: source_url: https://developers.openai.com/api/docs/pricing retrieved_at: "2026-08-22T00:00:00Z" 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 9b8a4c5d..4f6050d6 100644 --- a/episodic/cost/ports.py +++ b/episodic/cost/ports.py @@ -11,13 +11,8 @@ 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 small in-memory fakes that structurally satisfy the +Protocols; see ``tests/test_cost_ports_protocols.py`` for an example. """ import dataclasses as dc @@ -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) @@ -122,6 +121,7 @@ class PricingSnapshot: source_metadata: cabc.Mapping[str, str] content_hash: str retrieved_at: str + effective_from: str | None = None def __post_init__(self) -> None: """Validate value-object invariants.""" @@ -202,10 +202,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 @@ -223,8 +220,17 @@ async def ensure_snapshot(self, snapshot: PricingSnapshot) -> None: Run pricing pins and ledger entries reference snapshots by identifier, so the snapshot must be persisted before it is pinned. - Snapshots are immutable: repeated calls with the same identifier - must leave the stored row unchanged. + + Parameters + ---------- + snapshot : PricingSnapshot + Immutable snapshot to persist. + + Examples + -------- + ``await ledger.ensure_snapshot(snapshot)`` twice with one + ``pricing_snapshot_id`` is idempotent: the repeat call reuses + the persisted row. """ raise NotImplementedError diff --git a/episodic/cost/pricing_catalogue/file_loader.py b/episodic/cost/pricing_catalogue/file_loader.py index 3f7d9f94..7850e091 100644 --- a/episodic/cost/pricing_catalogue/file_loader.py +++ b/episodic/cost/pricing_catalogue/file_loader.py @@ -254,5 +254,8 @@ 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_value if isinstance(effective_from_value, str) else None + ), ) return _LoadedSnapshot(snapshot=snapshot, effective_from=effective_from) diff --git a/episodic/cost/recorder.py b/episodic/cost/recorder.py index fa13c316..568d0f0c 100644 --- a/episodic/cost/recorder.py +++ b/episodic/cost/recorder.py @@ -219,12 +219,17 @@ 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, ) + # 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) + return snapshot return await self.pricing_catalogue.get_snapshot(pinned_snapshot_id) async def record_provider_call( @@ -251,7 +256,6 @@ async def record_provider_call( If pricing or ledger validation fails. """ # noqa: DOC502 # Collaborating ports propagate these domain exceptions. snapshot = await self._resolve_snapshot_for_record(record) - 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 e625759e..2a25c352 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 ( @@ -26,6 +27,7 @@ MeteringCounterKey, PricingModel, PricingSnapshot, + PricingSnapshotCollisionError, PricingSnapshotId, ProviderCallLedgerEntry, RunPricingKey, @@ -118,7 +120,27 @@ 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.""" + """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( @@ -136,10 +158,31 @@ async def ensure_snapshot(self, snapshot: PricingSnapshot) -> None: snapshot.retrieved_at, error_message="timestamp must include timezone information.", ), + effective_from=( + None + if snapshot.effective_from is None + else parse_instant( + snapshot.effective_from, + error_message=("timestamp must include timezone information."), + ) + ), ) .on_conflict_do_nothing(index_elements=["id"]) ) - await self._session.execute(statement) + 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, diff --git a/episodic/llm/openai_validation.py b/episodic/llm/openai_validation.py index 588f2573..9a512042 100644 --- a/episodic/llm/openai_validation.py +++ b/episodic/llm/openai_validation.py @@ -211,49 +211,46 @@ def _normalize_chat_provider_call_usage( 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. + """ 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", - ), + 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", ) - _add_metric_if_present( - metrics, - "reasoning_tokens", - _extract_nested_token_count( - usage_payload, - "completion_tokens_details", - "reasoning_tokens", - ), + audio_input = _extract_nested_token_count( + usage_payload, + "prompt_tokens_details", + "audio_tokens", ) - _add_metric_if_present( - metrics, - "audio_output_tokens", - _extract_nested_token_count( - usage_payload, - "completion_tokens_details", - "audio_tokens", - ), + audio_output = _extract_nested_token_count( + usage_payload, + "completion_tokens_details", + "audio_tokens", ) + metrics = { + "input_tokens": prompt_tokens - (cached_input or 0) - (audio_input or 0), + "output_tokens": completion_tokens - (audio_output or 0), + } + _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 ProviderCallUsage( usage_metrics=metrics, usage_source=UsageSource.PROVIDER, @@ -270,39 +267,40 @@ 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. + """ 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", ) + 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/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/test_cost_recorder.py b/tests/test_cost_recorder.py index d6ecd750..df2578fd 100644 --- a/tests/test_cost_recorder.py +++ b/tests/test_cost_recorder.py @@ -60,7 +60,7 @@ 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) @@ -173,3 +173,46 @@ 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" + ) diff --git a/tests/test_cost_storage_ledger.py b/tests/test_cost_storage_ledger.py index d534cd40..d1310271 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 @@ -14,6 +15,7 @@ LedgerScope, PricingModel, PricingSnapshot, + PricingSnapshotCollisionError, PricingSnapshotId, PricingSourceKind, ProviderCallLedgerEntry, @@ -248,6 +250,7 @@ def _pricing_snapshot(snapshot_id: str) -> PricingSnapshot: source_metadata={"source_url": "https://example.test/pricing"}, content_hash="ensure-hash", retrieved_at="2026-06-04T09:00:00Z", + effective_from="2026-06-01T00:00:00Z", ) @@ -258,10 +261,16 @@ async def test_ensure_snapshot_persists_once_and_satisfies_pins( """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", @@ -283,6 +292,14 @@ async def test_ensure_snapshot_persists_once_and_satisfies_pins( ) ) ).scalar_one() + stored_hash, stored_effective_from = ( + await session.execute( + sa.select( + PricingSnapshotRecord.content_hash, + PricingSnapshotRecord.effective_from, + ).where(PricingSnapshotRecord.id == uuid.UUID(snapshot_id)) + ) + ).one() pins = ( await session.execute( sa.select(sa.func.count(RunPricingPinRecord.workflow_run_id)) @@ -290,4 +307,31 @@ async def test_ensure_snapshot_persists_once_and_satisfies_pins( ).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 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) From ba6bbc349e28e4a9c707a68cbd92d37cb004083e Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 23 Aug 2026 00:35:19 +0100 Subject: [PATCH 13/26] Keep preview secrets out of command arguments The preview tooling created the application Secret with `kubectl create secret --from-literal=...` arguments, so dry-run mode printed the bearer token and OpenAI API key to stdout and a failed command could echo them to stderr. Build the Secret manifest in Python with `stringData` and feed it to `kubectl apply -f -` on stdin instead; secret values no longer appear in any printable command line. Add a regression test pinning that property and a test covering `CommandRunner`'s failure diagnostics. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- scripts/local_k8s/commands.py | 41 +++++++++++------ scripts/local_k8s/config.py | 15 +++++- scripts/local_k8s/orchestration.py | 3 +- tests/test_local_k8s_tooling.py | 74 +++++++++++++++++++++++------- 4 files changed, 98 insertions(+), 35 deletions(-) diff --git a/scripts/local_k8s/commands.py b/scripts/local_k8s/commands.py index 1d223956..aa43e7dc 100644 --- a/scripts/local_k8s/commands.py +++ b/scripts/local_k8s/commands.py @@ -261,26 +261,37 @@ 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.""" - command = [ - *_kubectl_ns_cmd(config), - "create", - "secret", - "generic", - config.secret_name, - f"--from-literal=database-url={config.database_url}", - f"--from-literal=api-bearer-token={config.api_bearer_token}", +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: {config.secret_name}", + f" namespace: {config.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. - command.extend([ - f"--from-literal=openai-base-url={config.openai_base_url}", - f"--from-literal=openai-api-key={config.openai_api_key}", + lines.extend([ + f" openai-base-url: {_yaml_string(config.openai_base_url)}", + f" openai-api-key: {_yaml_string(config.openai_api_key)}", ]) - command.extend(["--dry-run=client", "-o", "yaml"]) - return command + return "\n".join(lines) + "\n" def _yaml_string(value: str) -> str: diff --git a/scripts/local_k8s/config.py b/scripts/local_k8s/config.py index 67ca51e3..aa860dc4 100644 --- a/scripts/local_k8s/config.py +++ b/scripts/local_k8s/config.py @@ -14,7 +14,20 @@ @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_api_key`` is read from the + ``OPENAI_API_KEY`` environment variable at construction time, so setting + it before running ``make local-k8s-up`` wires generation without + committing a credential anywhere. + + The Kubernetes Secret generated for the preview always writes + ``api-bearer-token``. The ``openai-base-url``/``openai-api-key`` pair is + written only when ``openai_api_key`` is non-empty (that is, when + ``OPENAI_API_KEY`` was set). + """ cluster_name: str = "episodic-preview" namespace: str = "episodic" 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/test_local_k8s_tooling.py b/tests/test_local_k8s_tooling.py index f4dd3a8d..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,53 +80,70 @@ 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_kubectl_secret_command_renders_bearer_token_literal() -> None: +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. - command = commands.kubectl_secret_command(config) + manifest = commands.secret_manifest(config) - assert "--from-literal=api-bearer-token=alpha-token" in command, ( + assert ' api-bearer-token: "alpha-token"' in manifest, ( "the preview secret must include the local bearer token" ) -def test_kubectl_secret_command_omits_openai_pair_without_key() -> None: - """Omit the OpenAI literals when no key is configured.""" +def test_secret_manifest_omits_openai_pair_without_key() -> None: + """Omit the OpenAI entries when no key is configured.""" config = PreviewConfig(openai_api_key="") - command = commands.kubectl_secret_command(config) + manifest = commands.secret_manifest(config) - assert not any("openai" in part for part in command), ( - "the preview secret must omit OpenAI literals without a key" + assert "openai" not in manifest, ( + "the preview secret must omit OpenAI entries without a key" ) -def test_kubectl_secret_command_renders_openai_pair_with_key() -> None: +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") - command = commands.kubectl_secret_command(config) + manifest = commands.secret_manifest(config) - assert "--from-literal=openai-api-key=sk-local-test" in command, ( + assert ' openai-api-key: "sk-local-test"' in manifest, ( "the preview secret must include the configured OpenAI key" ) - assert f"--from-literal=openai-base-url={config.openai_base_url}" in command, ( + 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: """Reject a local ingress port that is already bound.""" with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as listener: @@ -283,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" + ) From a93d9ea5f5dd10d34a5caf585c530682b354dc0a Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 23 Aug 2026 00:35:19 +0100 Subject: [PATCH 14/26] Document canonical port and storage contracts from review feedback Docstring-only changes: `GenerationEventLog.count_events` documents `RunNotFound`; `is_source_document_duplicate_integrity_error` gains Parameters/Returns/Notes matching its neighbours; `set_target_episode` documents its completion semantics in both the protocol and the SQLAlchemy adapter; the generation-runs resource documents the 202 response contract; `CostLedgerPort.ensure_snapshot`, `PreviewConfig`, and the `RuntimeConfig` provider options gain full NumPy-style documentation. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- episodic/canonical/entity_protocols.py | 5 +++++ episodic/canonical/generation_run_ports.py | 8 ++++++- .../storage/ingestion_job_repositories.py | 6 ++++-- .../canonical/storage/integrity_helpers.py | 21 ++++++++++++++++++- 4 files changed, 36 insertions(+), 4 deletions(-) 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) From 242fc93e56b18bd669991e2cea50503e24deb58e Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 23 Aug 2026 00:35:19 +0100 Subject: [PATCH 15/26] Strengthen test coverage and hygiene from review feedback - Scope the SQL idempotency property test's key per Hypothesis example so persisted rows from earlier examples cannot collide. - Assert `fetched is not None` before the equality comparison in the generation-run persistence test. - Derive the fixture series-profile slug from `key_prefix` so callers get isolated profiles, and document it. - Add a happy-path idempotency-encoding test, a draft-generation assertion that `json_response` is requested, and a docstring for the `_tei_hash` helper. - Whitelist `episodic.canonical.domain._require_value` in the Skylos entrypoints; it is called from `EpisodeTeiUpdate.__post_init__` like its already-listed siblings. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- pyproject.toml | 1 + tests/canonical_storage/test_generation_runs.py | 2 +- .../test_sql_generation_run_property_contract.py | 10 +++++++--- tests/fixtures/generation_run_api.py | 5 +++-- tests/test_draft_script_generation.py | 3 +++ tests/test_episode_tei_api.py | 1 + tests/test_source_idempotency_encoding.py | 13 +++++++++++++ 7 files changed, 29 insertions(+), 6 deletions(-) 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/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_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_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_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" From 2a3bd29b912a50c0e0fdceb8245926239318fad8 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 23 Aug 2026 00:35:19 +0100 Subject: [PATCH 16/26] Document tracing, provider options, and preview credentials in guides - The developers' guide observability section now covers `TracerPort`, the `SpanHandle` lifecycle, the safe-attribute allow-list, `StructuredLogTracer`, `NoopTracer`, and `RecordingTracer`, plus the provider request contract (`json_response`, `OpenAIPayloadOptions`), mutually exclusive usage normalization, `ensure_snapshot` persistence, and the Skylos Python 3.14 pin. - The users' guide documents the `OPENAI_*` request settings and notes that the migration port-forward context depends on the chosen preview provider (k3d default versus the optional kind provider). - The alpha-test notes table gains a caption, and the notes record the storage-migration and container-cleanup work. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- docs/alpha-test-4-3-2-setup-notes.md | 24 ++++++++++ docs/developers-guide.md | 70 ++++++++++++++++++++++++---- docs/users-guide.md | 21 ++++++++- 3 files changed, 103 insertions(+), 12 deletions(-) diff --git a/docs/alpha-test-4-3-2-setup-notes.md b/docs/alpha-test-4-3-2-setup-notes.md index 48289d53..5c730660 100644 --- a/docs/alpha-test-4-3-2-setup-notes.md +++ b/docs/alpha-test-4-3-2-setup-notes.md @@ -22,6 +22,8 @@ cluster, and record what it took. | `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 @@ -231,3 +233,25 @@ never an option on this host; the documented rootless-Podman guidance (use 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..8437d47e 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. @@ -563,8 +567,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 +578,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 +602,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 +1584,15 @@ 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. `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 +1809,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 +1825,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 +1862,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/users-guide.md b/docs/users-guide.md index 1f873656..085c96e8 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -93,10 +93,14 @@ service database with `alembic upgrade head`. The application does not apply 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: +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 kind-episodic-preview --namespace episodic \ +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 @@ -181,6 +185,19 @@ 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_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 From 5294608fae153a785688c3931925d75456d26674 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 23 Aug 2026 17:59:52 +0100 Subject: [PATCH 17/26] Reject malformed provider usage and pricing snapshot inputs Two validation gaps from review: - The pricing-snapshot loader silently converted a present but non-string `effective_from` (for example `false` or `0`) to `None`. A new `_optional_string` helper rejects such values with the module's established `ValueError` shape while preserving absent and valid string values. - The mutually exclusive usage normalization could produce a negative `input_tokens` metric when a provider reported nested cached/audio counts exceeding their parent totals; the failure would otherwise surface late as an unexpected cost-recording error. Both normalizers now reject oversubscribed details with `OpenAIResponseValidationError` at the adapter boundary. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- .../cost/pricing_catalogue/file_loader.py | 26 +++++-- episodic/llm/openai_validation.py | 20 +++++ ...test_cost_pricing_catalogue_file_loader.py | 77 +++++++++++++++++++ .../test_llm_openai_adapter_usage_metering.py | 38 +++++++++ 4 files changed, 156 insertions(+), 5 deletions(-) diff --git a/episodic/cost/pricing_catalogue/file_loader.py b/episodic/cost/pricing_catalogue/file_loader.py index 7850e091..2d806608 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,8 +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_value if isinstance(effective_from_value, str) else None - ), + effective_from=effective_from_value, ) return _LoadedSnapshot(snapshot=snapshot, effective_from=effective_from) diff --git a/episodic/llm/openai_validation.py b/episodic/llm/openai_validation.py index 9a512042..9240fc7a 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." @@ -224,6 +228,11 @@ def _normalize_chat_provider_call_usage( ------- 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 @@ -244,6 +253,10 @@ def _normalize_chat_provider_call_usage( "completion_tokens_details", "audio_tokens", ) + if (cached_input or 0) + (audio_input or 0) > prompt_tokens or ( + audio_output or 0 + ) > completion_tokens: + raise OpenAIResponseValidationError(_INVALID_USAGE_DETAIL_MESSAGE) metrics = { "input_tokens": prompt_tokens - (cached_input or 0) - (audio_input or 0), "output_tokens": completion_tokens - (audio_output or 0), @@ -278,6 +291,11 @@ def _normalize_responses_provider_call_usage( ------- 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 @@ -296,6 +314,8 @@ def _normalize_responses_provider_call_usage( "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, diff --git a/tests/test_cost_pricing_catalogue_file_loader.py b/tests/test_cost_pricing_catalogue_file_loader.py index 9ecf1a65..771eb1ad 100644 --- a/tests/test_cost_pricing_catalogue_file_loader.py +++ b/tests/test_cost_pricing_catalogue_file_loader.py @@ -150,3 +150,80 @@ 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"), + ) + + assert snapshot.effective_from == "2026-06-01T00:00:00Z", ( + f"expected the YAML effective date, got {snapshot.effective_from!r}" + ) + + +@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_llm_openai_adapter_usage_metering.py b/tests/test_llm_openai_adapter_usage_metering.py index 00ace4cb..3e895c83 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 @@ -167,3 +168,40 @@ def handler(request: httpx.Request) -> httpx.Response: "input_tokens": 42, "output_tokens": 18, }, "zero-valued optional metrics must be omitted from usage metrics" + + +@pytest.mark.asyncio +async def test_chat_completion_usage_rejects_oversubscribed_details( + openai_adapter_factory: _OpenAIAdapterFactory, + openai_json_response: _OpenAIJsonResponseBuilder, + openai_request_builder: _OpenAIRequestBuilder, +) -> 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, + "prompt_tokens_details": {"cached_tokens": 11}, + }, + }) + + 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()) From 264ce207d5d91b2c0c2cfeff3f9477f4de808294 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 23 Aug 2026 17:59:52 +0100 Subject: [PATCH 18/26] Extend runtime, snapshot, and preview configuration test coverage - Consolidate the generation-limit rejection tests into one parametrized test covering source-count and output-limit settings. - Cover `_load_runtime_config` reading the `OPENAI_*` request options, rejecting unknown token-limit parameter names, and rejecting non-positive or non-finite timeouts. - Assert the stored rate map survives a conflicting `ensure_snapshot` call alongside the existing hash and effective-date assertions. - Cover `PreviewConfig` sourcing `openai_api_key` from the `OPENAI_API_KEY` environment variable and defaulting to empty. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- tests/test_cost_storage_ledger.py | 6 +- tests/test_local_k8s_tooling.py | 24 ++++++++ tests/test_runtime_configuration.py | 89 ++++++++++++++++++++++------- 3 files changed, 97 insertions(+), 22 deletions(-) diff --git a/tests/test_cost_storage_ledger.py b/tests/test_cost_storage_ledger.py index d1310271..8d190c72 100644 --- a/tests/test_cost_storage_ledger.py +++ b/tests/test_cost_storage_ledger.py @@ -292,11 +292,12 @@ async def test_ensure_snapshot_persists_once_and_satisfies_pins( ) ) ).scalar_one() - stored_hash, stored_effective_from = ( + 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() @@ -313,6 +314,9 @@ async def test_ensure_snapshot_persists_once_and_satisfies_pins( 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" diff --git a/tests/test_local_k8s_tooling.py b/tests/test_local_k8s_tooling.py index 8659edec..d5c98653 100644 --- a/tests/test_local_k8s_tooling.py +++ b/tests/test_local_k8s_tooling.py @@ -323,3 +323,27 @@ def test_command_runner_surfaces_captured_output_on_failure( assert "err-diag" in captured.err, ( "captured stderr of a failed command must be surfaced on stderr" ) + + +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" diff --git a/tests/test_runtime_configuration.py b/tests/test_runtime_configuration.py index 743994fe..75940616 100644 --- a/tests/test_runtime_configuration.py +++ b/tests/test_runtime_configuration.py @@ -107,38 +107,29 @@ def test_load_runtime_config_rejects_unpaired_openai_base_url( }) -@pytest.mark.parametrize("value", ["0", "-1", "not-an-integer"]) -def test_load_runtime_config_rejects_invalid_generation_source_limit( - tmp_path: Path, - value: str, -) -> None: - """Generation source limits must be positive integer runtime settings.""" - with pytest.raises( - RuntimeConfigurationError, - match="GENERATION_MAX_SOURCE_COUNT must be a positive integer", - ): - _load_runtime_config({ - **_base_environment(tmp_path), - "GENERATION_MAX_SOURCE_COUNT": value, - }) - - @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( +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.""" + """Generation limits must be positive integer runtime settings.""" with pytest.raises( RuntimeConfigurationError, match=f"{setting} must be a positive integer", ): _load_runtime_config({ **_base_environment(tmp_path), - setting: "0", + setting: value, }) @@ -176,3 +167,59 @@ def test_load_runtime_config_failure_does_not_log_success( 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, + }) From 1f36c9f0f36440d65c4d1422f3e5147a950f9743 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 23 Aug 2026 17:59:52 +0100 Subject: [PATCH 19/26] Document effective dates, preview credentials, and alpha progress - `PricingSnapshot` documents `effective_from` and the latest-applicable selection rule; `PreviewConfig` documents the Secret-manifest credential contract. - The users' guide covers the paired provider credentials, `DRAFT_MODEL`, the preview bearer token, and the chart's pass-through volume values; the developers' guide notes `effective_from` persistence and the stdin Secret flow. - The 4.3.2 ExecPlan gains a dated alpha-test outcome entry recording the end-to-end run and the fixes it required. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- docs/developers-guide.md | 17 +++++++++++-- ...qa-generation-runs-and-tei-p5-retrieval.md | 24 +++++++++++++++++++ docs/users-guide.md | 24 +++++++++++++++++++ episodic/cost/ports.py | 15 +++++++----- scripts/local_k8s/config.py | 8 ++++--- 5 files changed, 77 insertions(+), 11 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 8437d47e..3428815d 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -403,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`. @@ -419,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 @@ -1590,8 +1599,12 @@ 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. `CostRecorder` then records usage with a -run-scoped idempotency key. `PRICING_SNAPSHOT_DIRECTORY` is optional: its +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. 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..ccb1c42b 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,27 @@ 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 +`docs/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 085c96e8..851c9606 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -188,6 +188,11 @@ draft output. Each value must be a positive integer: 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 @@ -428,6 +433,19 @@ 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. + On rootless Podman hosts, use the kind provider directly: ```shell @@ -448,6 +466,12 @@ 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/cost/ports.py b/episodic/cost/ports.py index 4f6050d6..a1a65414 100644 --- a/episodic/cost/ports.py +++ b/episodic/cost/ports.py @@ -11,8 +11,7 @@ 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; see ``tests/test_cost_ports_protocols.py`` for an example. +Tests can use in-memory fakes; see ``tests/test_cost_ports_protocols.py``. """ import dataclasses as dc @@ -108,7 +107,12 @@ 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 ISO-8601 timestamp + (for example ``"2026-08-01T00:00:00Z"``); catalogue resolution keeps + snapshots unset or not after now and selects the latest. + """ pricing_snapshot_id: PricingSnapshotId provider_name: str @@ -228,9 +232,8 @@ async def ensure_snapshot(self, snapshot: PricingSnapshot) -> None: Examples -------- - ``await ledger.ensure_snapshot(snapshot)`` twice with one - ``pricing_snapshot_id`` is idempotent: the repeat call reuses - the persisted row. + Repeated ``await ledger.ensure_snapshot(snapshot)`` calls with + one ``pricing_snapshot_id`` reuse the persisted row. """ raise NotImplementedError diff --git a/scripts/local_k8s/config.py b/scripts/local_k8s/config.py index aa860dc4..80856af2 100644 --- a/scripts/local_k8s/config.py +++ b/scripts/local_k8s/config.py @@ -24,9 +24,11 @@ class PreviewConfig: committing a credential anywhere. The Kubernetes Secret generated for the preview always writes - ``api-bearer-token``. The ``openai-base-url``/``openai-api-key`` pair is - written only when ``openai_api_key`` is non-empty (that is, when - ``OPENAI_API_KEY`` was set). + ``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 (that is, when ``OPENAI_API_KEY`` was set), since the + runtime requires the pair or neither. Secret values travel via stdin + ``stringData``, never as command arguments. """ cluster_name: str = "episodic-preview" From 2ded8eec60b89423ceddb00482598ec26731354c Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Mon, 24 Aug 2026 21:18:07 +0100 Subject: [PATCH 20/26] Prove runtime option propagation, autospec safety, and image packaging - Add a composition test asserting `create_app_from_env` carries `OPENAI_REASONING_EFFORT`, `OPENAI_SERVICE_TIER`, `OPENAI_TOKEN_LIMIT_PARAM`, and `OPENAI_TIMEOUT_SECONDS` into the constructed `OpenAICompatibleLLMConfig`. - Add an adapter-boundary behavioural test: the runtime-composed adapter, driven through a capturing `httpx.MockTransport`, emits `reasoning_effort`, `service_tier`, and `max_completion_tokens` (and no `max_tokens`) in the provider request body with the configured 600-second request timeout. This proves the full path from environment to HTTP request rather than re-testing `_build_payload`. - Add a regression test autospeccing `SqlAlchemyUnitOfWork`; it fails with `NameError` if the runtime-evaluated annotation imports move back behind `typing.TYPE_CHECKING`. - Add a static Dockerfile contract test pinning the `COPY --chown=episodic:episodic config/pricing-snapshots /app/config/pricing-snapshots` runtime-stage instruction. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- tests/test_container_image_contract.py | 20 +++ tests/test_env_runtime_provider_options.py | 191 +++++++++++++++++++++ tests/test_uow_autospec_regression.py | 25 +++ 3 files changed, 236 insertions(+) create mode 100644 tests/test_env_runtime_provider_options.py create mode 100644 tests/test_uow_autospec_regression.py diff --git a/tests/test_container_image_contract.py b/tests/test_container_image_contract.py index d050a93d..2b053f77 100644 --- a/tests/test_container_image_contract.py +++ b/tests/test_container_image_contract.py @@ -185,3 +185,23 @@ 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.""" + copy_lines = [ + line.strip() + for line in _dockerfile_text().splitlines() + if line.strip().startswith("COPY") and "pricing-snapshots" in line + ] + + assert copy_lines == [ + ( + "COPY --chown=episodic:episodic config/pricing-snapshots " + "/app/config/pricing-snapshots" + ) + ], ( + "the runtime stage must copy config/pricing-snapshots to " + "/app/config/pricing-snapshots owned by the episodic user; got " + f"{copy_lines!r}" + ) diff --git a/tests/test_env_runtime_provider_options.py b/tests/test_env_runtime_provider_options.py new file mode 100644 index 00000000..4758c464 --- /dev/null +++ b/tests/test_env_runtime_provider_options.py @@ -0,0 +1,191 @@ +"""Runtime composition tests for the OPENAI_* provider request options.""" + +import asyncio +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 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)}" + ) + request = captured_requests[0] + import json as jsonlib + + body = jsonlib.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}" + ) diff --git a/tests/test_uow_autospec_regression.py b/tests/test_uow_autospec_regression.py new file mode 100644 index 00000000..af10041e --- /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) + + assert specced is not None, "expected an autospecced unit of work" + assert hasattr(specced, "__aenter__"), ( + "the autospecced unit of work must keep its context-manager surface" + ) + assert hasattr(specced, "__aexit__"), ( + "the autospecced unit of work must keep its context-manager surface" + ) From 419bc51cbb53e0d2b05f7cc29ad39952948e20a7 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Mon, 24 Aug 2026 21:18:07 +0100 Subject: [PATCH 21/26] Carry effective_from as a timezone-aware datetime in the domain `PricingSnapshot.effective_from` becomes `datetime | None`, validated timezone-aware at construction, instead of a raw ISO-8601 string parsed per adapter. The catalogue loader now parses the YAML value exactly once and threads the same parsed instant into both its internal resolution ordering and the domain snapshot; the SQL adapter persists the typed value directly and loses its per-write parsing. Database `NULL` is preserved for absent values. Tests assert the loaded domain value is a timezone-aware `datetime`, that persistence round-trips the same instant, and that a naive datetime is rejected at construction. Partially addresses #282 for this field; the sibling string timestamps remain for the issue's uniform conversion. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- episodic/cost/ports.py | 26 ++++++++----------- .../cost/pricing_catalogue/file_loader.py | 2 +- episodic/cost/storage/adapters.py | 9 +------ tests/test_cost_ports_protocols.py | 15 +++++++++++ ...test_cost_pricing_catalogue_file_loader.py | 10 +++++-- tests/test_cost_storage_ledger.py | 2 +- 6 files changed, 37 insertions(+), 27 deletions(-) diff --git a/episodic/cost/ports.py b/episodic/cost/ports.py index a1a65414..705f3002 100644 --- a/episodic/cost/ports.py +++ b/episodic/cost/ports.py @@ -15,6 +15,7 @@ """ import dataclasses as dc +import datetime as dt # noqa: TC003 - dataclass field annotation evaluated at runtime. import enum import typing as typ @@ -109,9 +110,9 @@ def _validate_usage_metrics(usage: cabc.Mapping[str, int]) -> None: class PricingSnapshot: """Immutable pricing input used by the deterministic pricing engine. - ``effective_from`` is an optional timezone-aware ISO-8601 timestamp - (for example ``"2026-08-01T00:00:00Z"``); catalogue resolution keeps - snapshots unset or not after now and selects the latest. + ``effective_from`` is an optional timezone-aware ``datetime`` (for + example 2026-08-01T00:00:00Z); catalogue resolution keeps snapshots + unset or not after now and selects the latest. """ pricing_snapshot_id: PricingSnapshotId @@ -125,12 +126,15 @@ class PricingSnapshot: source_metadata: cabc.Mapping[str, str] content_hash: str retrieved_at: str - effective_from: str | None = None + 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 not None and self.effective_from.tzinfo is None: + msg = "effective_from must be timezone-aware." + raise ValueError(msg) @dc.dataclass(frozen=True, slots=True) @@ -257,20 +261,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 2d806608..9de36491 100644 --- a/episodic/cost/pricing_catalogue/file_loader.py +++ b/episodic/cost/pricing_catalogue/file_loader.py @@ -272,6 +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_value, + effective_from=effective_from, ) return _LoadedSnapshot(snapshot=snapshot, effective_from=effective_from) diff --git a/episodic/cost/storage/adapters.py b/episodic/cost/storage/adapters.py index 2a25c352..055aaac8 100644 --- a/episodic/cost/storage/adapters.py +++ b/episodic/cost/storage/adapters.py @@ -158,14 +158,7 @@ async def ensure_snapshot(self, snapshot: PricingSnapshot) -> None: snapshot.retrieved_at, error_message="timestamp must include timezone information.", ), - effective_from=( - None - if snapshot.effective_from is None - else parse_instant( - snapshot.effective_from, - error_message=("timestamp must include timezone information."), - ) - ), + effective_from=snapshot.effective_from, ) .on_conflict_do_nothing(index_elements=["id"]) ) diff --git a/tests/test_cost_ports_protocols.py b/tests/test_cost_ports_protocols.py index 8ffa481b..596cce0b 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, @@ -235,3 +239,14 @@ 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. + ) diff --git a/tests/test_cost_pricing_catalogue_file_loader.py b/tests/test_cost_pricing_catalogue_file_loader.py index 771eb1ad..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 @@ -186,8 +187,13 @@ async def test_file_pricing_catalogue_propagates_effective_from( BillingPeriodKey("2026-06"), ) - assert snapshot.effective_from == "2026-06-01T00:00:00Z", ( - f"expected the YAML effective date, got {snapshot.effective_from!r}" + 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" ) diff --git a/tests/test_cost_storage_ledger.py b/tests/test_cost_storage_ledger.py index 8d190c72..76574bf7 100644 --- a/tests/test_cost_storage_ledger.py +++ b/tests/test_cost_storage_ledger.py @@ -250,7 +250,7 @@ def _pricing_snapshot(snapshot_id: str) -> PricingSnapshot: source_metadata={"source_url": "https://example.test/pricing"}, content_hash="ensure-hash", retrieved_at="2026-06-04T09:00:00Z", - effective_from="2026-06-01T00:00:00Z", + effective_from=dt.datetime(2026, 6, 1, tzinfo=dt.UTC), ) From a12c858ac4c37e30066aa6ee56d22d5466a6b3e8 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Mon, 24 Aug 2026 21:18:07 +0100 Subject: [PATCH 22/26] Split chat usage normalization into validation and metric helpers `_normalize_chat_provider_call_usage` had accreted extraction, oversubscription validation, and metric construction. Extract `_validate_chat_usage_detail_totals` (two independent guard clauses over `input_detail_tokens` and `output_audio_tokens`) and `_build_chat_usage_metrics` (extraction, validation, and the mutually exclusive metric map), leaving the normalizer with the `None` guard and `ProviderCallUsage` construction. Behaviour, metric names, error types, and messages are unchanged; the Responses normalizer is untouched. Parameterize the adapter-boundary rejection test to cover both oversubscription directions: prompt details exceeding `prompt_tokens`, and completion audio exceeding `completion_tokens`. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- episodic/llm/openai_validation.py | 93 ++++++++++++++----- .../test_llm_openai_adapter_usage_metering.py | 16 +++- 2 files changed, 85 insertions(+), 24 deletions(-) diff --git a/episodic/llm/openai_validation.py b/episodic/llm/openai_validation.py index 9240fc7a..bbbce42d 100644 --- a/episodic/llm/openai_validation.py +++ b/episodic/llm/openai_validation.py @@ -210,32 +210,44 @@ def _add_metric_if_present( metrics[key] = value -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. +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. - 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. + 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 ------- - ProviderCallUsage | None - Canonical usage metadata, or ``None`` without a usage payload. + dict[str, int] + Mutually exclusive canonical usage metrics. Raises ------ OpenAIResponseValidationError If nested token details exceed their parent token totals. - """ - if usage_payload is None: - return None + """ # 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( @@ -253,17 +265,52 @@ def _normalize_chat_provider_call_usage( "completion_tokens_details", "audio_tokens", ) - if (cached_input or 0) + (audio_input or 0) > prompt_tokens or ( - audio_output or 0 - ) > completion_tokens: - raise OpenAIResponseValidationError(_INVALID_USAGE_DETAIL_MESSAGE) + _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 - (cached_input or 0) - (audio_input or 0), - "output_tokens": completion_tokens - (audio_output or 0), + "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. + + 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 = _build_chat_usage_metrics(usage_payload) return ProviderCallUsage( usage_metrics=metrics, usage_source=UsageSource.PROVIDER, diff --git a/tests/test_llm_openai_adapter_usage_metering.py b/tests/test_llm_openai_adapter_usage_metering.py index 3e895c83..701bf5bf 100644 --- a/tests/test_llm_openai_adapter_usage_metering.py +++ b/tests/test_llm_openai_adapter_usage_metering.py @@ -171,10 +171,24 @@ def handler(request: httpx.Request) -> httpx.Response: @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.""" @@ -193,7 +207,7 @@ def handler(request: httpx.Request) -> httpx.Response: "prompt_tokens": 10, "completion_tokens": 5, "total_tokens": 15, - "prompt_tokens_details": {"cached_tokens": 11}, + **usage_details, }, }) From 7597d3a0dacdcfb1fbd06e0c089c93efa022a046 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Mon, 24 Aug 2026 21:18:07 +0100 Subject: [PATCH 23/26] Add property and concurrency coverage for usage and snapshot invariants - A Hypothesis suite over non-negative parent totals and optional nested counts proves normalized chat metrics partition the parent totals exactly (`input_tokens + cached + audio_in == prompt_tokens`, `output_tokens + audio_out == completion_tokens`) and that oversubscribed details raise the validation error. - A bounded Hypothesis suite drives `ensure_snapshot` through per-example finite identifier/content-hash domains across operation orderings, asserting persisted identifiers never mutate their immutable fields and same-content/different-identifier writes raise `PricingSnapshotCollisionError`. - Deterministic concurrency tests use two independent sessions with event-based synchronization (no sleeps): concurrent ensures leave one row; concurrent ensure-then-pin races leave one snapshot and one pin referencing it; a cancelled uncommitted transaction leaves no partial rows and a retry then persists and pins successfully. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- tests/test_cost_snapshot_concurrency.py | 213 ++++++++++++++++++ ...st_cost_snapshot_persistence_properties.py | 126 +++++++++++ ...test_llm_usage_normalization_properties.py | 108 +++++++++ 3 files changed, 447 insertions(+) create mode 100644 tests/test_cost_snapshot_concurrency.py create mode 100644 tests/test_cost_snapshot_persistence_properties.py create mode 100644 tests/test_llm_usage_normalization_properties.py diff --git a/tests/test_cost_snapshot_concurrency.py b/tests/test_cost_snapshot_concurrency.py new file mode 100644 index 00000000..f8d51c13 --- /dev/null +++ b/tests/test_cost_snapshot_concurrency.py @@ -0,0 +1,213 @@ +"""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: + await SqlAlchemyCostLedgerStore(session).ensure_snapshot(snapshot) + 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 leave no 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_llm_usage_normalization_properties.py b/tests/test_llm_usage_normalization_properties.py new file mode 100644 index 00000000..5cebd7eb --- /dev/null +++ b/tests/test_llm_usage_normalization_properties.py @@ -0,0 +1,108 @@ +"""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) + + +def _usage_payload( # pylint: disable=too-many-arguments # Usage fields travel together. + *, + prompt_tokens: int, + completion_tokens: int, + cached_input: int | None, + audio_input: int | None, + audio_output: int | None, +) -> dict[str, object]: + """Build a chat usage payload with optional nested detail counts.""" + 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( + prompt_tokens=_TOKEN_COUNTS, + completion_tokens=_TOKEN_COUNTS, + cached_input=_OPTIONAL_COUNTS, + audio_input=_OPTIONAL_COUNTS, + audio_output=_OPTIONAL_COUNTS, +) +@settings(max_examples=100) +def test_chat_usage_metrics_partition_the_parent_totals( # pylint: disable=too-many-arguments,too-many-positional-arguments # Hypothesis injects one argument per strategy. + prompt_tokens: int, + completion_tokens: int, + cached_input: int | None, + audio_input: int | None, + audio_output: int | None, +) -> None: + """Valid nested details partition the parent totals exactly once.""" + payload = _usage_payload( + prompt_tokens=prompt_tokens, + completion_tokens=completion_tokens, + cached_input=cached_input, + audio_input=audio_input, + audio_output=audio_output, + ) + 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}" + ) From 603985b88607623d32109aec17d15b51f3e78750 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 27 Aug 2026 01:15:38 +0100 Subject: [PATCH 24/26] Harden preview manifests and credential handling from review feedback - `secret_manifest` validates `secret_name` and `namespace` against Kubernetes DNS-1123 label rules before interpolating them, and `_yaml_string` now escapes newlines, carriage returns, and tabs, so no configuration value can alter the applied manifest's structure. Regression tests cover namespace injection and newline escaping. - `PreviewConfig.openai_api_key` is excluded from the dataclass representation, and `openai_base_url` now reads `OPENAI_BASE_URL` from the environment with the public endpoint as the fallback, with tests for both behaviours. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- scripts/local_k8s/commands.py | 31 +++++++++-- scripts/local_k8s/config.py | 89 +++++++++++++++++++++++++++---- tests/test_local_k8s_config.py | 93 +++++++++++++++++++++++++++++++++ tests/test_local_k8s_tooling.py | 24 --------- 4 files changed, 199 insertions(+), 38 deletions(-) create mode 100644 tests/test_local_k8s_config.py diff --git a/scripts/local_k8s/commands.py b/scripts/local_k8s/commands.py index aa43e7dc..f102770e 100644 --- a/scripts/local_k8s/commands.py +++ b/scripts/local_k8s/commands.py @@ -1,6 +1,7 @@ """Command construction for the local Kubernetes preview workflow.""" import dataclasses as dc +import re import subprocess import sys import typing as typ @@ -277,8 +278,8 @@ def secret_manifest(config: PreviewConfig) -> str: "apiVersion: v1", "kind: Secret", "metadata:", - f" name: {config.secret_name}", - f" namespace: {config.namespace}", + 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)}", @@ -294,9 +295,31 @@ def secret_manifest(config: PreviewConfig) -> str: 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 80856af2..881a32f6 100644 --- a/scripts/local_k8s/config.py +++ b/scripts/local_k8s/config.py @@ -18,17 +18,80 @@ class PreviewConfig: 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_api_key`` is read from the - ``OPENAI_API_KEY`` environment variable at construction time, so setting - it before running ``make local-k8s-up`` wires generation without - committing a credential anywhere. + ``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 (that is, when ``OPENAI_API_KEY`` was set), since the - runtime requires the pair or neither. Secret values travel via stdin - ``stringData``, never as command arguments. + 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" @@ -50,11 +113,17 @@ class PreviewConfig: 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 = "https://api.openai.com/v1" + 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. + # generation without committing a credential anywhere. repr=False keeps + # the credential out of the dataclass representation. openai_api_key: str = dc.field( - default_factory=lambda: os.environ.get("OPENAI_API_KEY", "") + repr=False, + default_factory=lambda: os.environ.get("OPENAI_API_KEY", ""), ) def kube_context(self) -> str: 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 d5c98653..8659edec 100644 --- a/tests/test_local_k8s_tooling.py +++ b/tests/test_local_k8s_tooling.py @@ -323,27 +323,3 @@ def test_command_runner_surfaces_captured_output_on_failure( assert "err-diag" in captured.err, ( "captured stderr of a failed command must be surfaced on stderr" ) - - -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" From 0e4f1a9e52f4c8ef2bfe724153e90a09b29038c1 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 27 Aug 2026 01:15:38 +0100 Subject: [PATCH 25/26] Keep snapshot resolution read-only and reject invalid effective dates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `_resolve_snapshot_for_record` no longer persists as a side effect: it returns the snapshot and whether a pin selected it, and `record_provider_call` — the command — performs `ensure_snapshot` on the unpinned path. - `PricingSnapshot.__post_init__` raises `TypeError` for a non-datetime `effective_from` before touching `tzinfo`, with a regression test; previously a string produced `AttributeError`. - Add the first tests through the real `CostRecorder.pin_run_pricing`: a call-order test proving the snapshot is persisted before the pin, and a fresh-database integration test proving the pin's foreign key is satisfied. The cancellation test now pins inside the stalled transaction so pin rollback is genuinely exercised. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- episodic/cost/ports.py | 14 ++-- episodic/cost/recorder.py | 29 ++++--- tests/test_cost_ports_protocols.py | 11 +++ tests/test_cost_recorder.py | 45 ++++++++++- tests/test_cost_recorder_integration.py | 102 ++++++++++++++++++++++++ tests/test_cost_snapshot_concurrency.py | 10 ++- 6 files changed, 192 insertions(+), 19 deletions(-) create mode 100644 tests/test_cost_recorder_integration.py diff --git a/episodic/cost/ports.py b/episodic/cost/ports.py index 705f3002..0265b134 100644 --- a/episodic/cost/ports.py +++ b/episodic/cost/ports.py @@ -15,7 +15,7 @@ """ import dataclasses as dc -import datetime as dt # noqa: TC003 - dataclass field annotation evaluated at runtime. +import datetime as dt import enum import typing as typ @@ -110,9 +110,8 @@ def _validate_usage_metrics(usage: cabc.Mapping[str, int]) -> None: class PricingSnapshot: """Immutable pricing input used by the deterministic pricing engine. - ``effective_from`` is an optional timezone-aware ``datetime`` (for - example 2026-08-01T00:00:00Z); catalogue resolution keeps snapshots - unset or not after now and selects the latest. + ``effective_from`` is an optional timezone-aware ``datetime``; the + catalogue keeps snapshots unset or not after now, selecting the latest. """ pricing_snapshot_id: PricingSnapshotId @@ -132,7 +131,12 @@ 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 not None and self.effective_from.tzinfo is None: + 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) diff --git a/episodic/cost/recorder.py b/episodic/cost/recorder.py index 568d0f0c..56d692fd 100644 --- a/episodic/cost/recorder.py +++ b/episodic/cost/recorder.py @@ -208,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, @@ -225,12 +235,8 @@ async def _resolve_snapshot_for_record( record.operation, record.billing_period_key, ) - # 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) - return snapshot - 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, @@ -255,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/tests/test_cost_ports_protocols.py b/tests/test_cost_ports_protocols.py index 596cce0b..9c712612 100644 --- a/tests/test_cost_ports_protocols.py +++ b/tests/test_cost_ports_protocols.py @@ -250,3 +250,14 @@ def test_pricing_snapshot_rejects_naive_effective_from() -> None: 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_recorder.py b/tests/test_cost_recorder.py index df2578fd..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( @@ -63,9 +67,11 @@ class _PinnedLedger: 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) @@ -75,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.""" @@ -216,3 +223,35 @@ async def test_cost_recorder_persists_snapshot_for_unpinned_provider_call() -> N 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 index f8d51c13..be16a51f 100644 --- a/tests/test_cost_snapshot_concurrency.py +++ b/tests/test_cost_snapshot_concurrency.py @@ -173,7 +173,13 @@ async def test_cancelled_transaction_leaves_no_partial_snapshot( async def ensure_then_stall() -> None: async with session_factory() as session: - await SqlAlchemyCostLedgerStore(session).ensure_snapshot(snapshot) + 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() @@ -196,7 +202,7 @@ async def ensure_then_stall() -> None: ) ) ).scalar_one() - assert pins == 0, "a cancelled transaction must leave no pin row" + assert pins == 0, "a cancelled transaction must roll back its uncommitted pin row" async with session_factory() as session: store = SqlAlchemyCostLedgerStore(session) From a6267a665577251c5769355ef9b86363fd11db9b Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 27 Aug 2026 01:15:38 +0100 Subject: [PATCH 26/26] Tighten review-driven test contracts and documentation - The Dockerfile pricing-snapshots contract is stage-aware: the copy must sit in the runtime stage and nowhere else. - The unit-of-work autospec regression asserts `__aenter__`/`__aexit__` are `AsyncMock` instances on an instance autospec. - The usage-normalization property test consolidates its five Hypothesis arguments into one `_UsageCase` tuple, and the provider request assertions move into a shared helper. - `PreviewConfig` gains full NumPy-style documentation stating the non-empty-key Secret condition precisely; the users' guide documents the effective local-preview provider settings; the 4.3.2 ExecPlan links the alpha-test notes with reflowed prose. Claude-Session: https://claude.ai/code/session_015huci8kKgRWx3ULAWN5gh8 --- ...qa-generation-runs-and-tei-p5-retrieval.md | 31 +++++----- docs/users-guide.md | 12 ++++ tests/test_container_image_contract.py | 35 +++++++----- tests/test_env_runtime_provider_options.py | 45 ++++++++------- ...test_llm_usage_normalization_properties.py | 57 +++++++++---------- tests/test_uow_autospec_regression.py | 10 ++-- 6 files changed, 105 insertions(+), 85 deletions(-) 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 ccb1c42b..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 @@ -2132,21 +2132,22 @@ 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 -`docs/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 +[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 851c9606..100acbec 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -446,6 +446,18 @@ When `OPENAI_API_KEY` is set in the operator's environment, 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 diff --git a/tests/test_container_image_contract.py b/tests/test_container_image_contract.py index 2b053f77..b5c0c7b0 100644 --- a/tests/test_container_image_contract.py +++ b/tests/test_container_image_contract.py @@ -189,19 +189,24 @@ def test_docker_image_serves_liveness_when_docker_smoke_enabled() -> None: def test_dockerfile_ships_pricing_snapshots_into_the_runtime_stage() -> None: """The runtime image must carry the immutable pricing catalogue.""" - copy_lines = [ - line.strip() - for line in _dockerfile_text().splitlines() - if line.strip().startswith("COPY") and "pricing-snapshots" in line - ] - - assert copy_lines == [ - ( - "COPY --chown=episodic:episodic config/pricing-snapshots " - "/app/config/pricing-snapshots" - ) - ], ( - "the runtime stage must copy config/pricing-snapshots to " - "/app/config/pricing-snapshots owned by the episodic user; got " - f"{copy_lines!r}" + 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_env_runtime_provider_options.py b/tests/test_env_runtime_provider_options.py index 4758c464..5bb1cf2d 100644 --- a/tests/test_env_runtime_provider_options.py +++ b/tests/test_env_runtime_provider_options.py @@ -1,6 +1,7 @@ """Runtime composition tests for the OPENAI_* provider request options.""" import asyncio +import json import typing as typ import httpx @@ -73,6 +74,28 @@ def capture_dependencies(dependencies: ApiDependencies) -> object: 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, @@ -168,24 +191,4 @@ def handler(request: httpx.Request) -> httpx.Response: assert len(captured_requests) == 1, ( f"expected one provider request, got {len(captured_requests)}" ) - request = captured_requests[0] - import json as jsonlib - - body = jsonlib.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}" - ) + _assert_configured_provider_request(captured_requests[0]) diff --git a/tests/test_llm_usage_normalization_properties.py b/tests/test_llm_usage_normalization_properties.py index 5cebd7eb..d27501be 100644 --- a/tests/test_llm_usage_normalization_properties.py +++ b/tests/test_llm_usage_normalization_properties.py @@ -17,16 +17,26 @@ _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] -def _usage_payload( # pylint: disable=too-many-arguments # Usage fields travel together. - *, - prompt_tokens: int, - completion_tokens: int, - cached_input: int | None, - audio_input: int | None, - audio_output: int | None, -) -> dict[str, object]: +_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, @@ -57,29 +67,18 @@ def _normalized_metrics( return usage.usage_metrics -@given( - prompt_tokens=_TOKEN_COUNTS, - completion_tokens=_TOKEN_COUNTS, - cached_input=_OPTIONAL_COUNTS, - audio_input=_OPTIONAL_COUNTS, - audio_output=_OPTIONAL_COUNTS, -) +@given(case=_USAGE_CASES) @settings(max_examples=100) -def test_chat_usage_metrics_partition_the_parent_totals( # pylint: disable=too-many-arguments,too-many-positional-arguments # Hypothesis injects one argument per strategy. - prompt_tokens: int, - completion_tokens: int, - cached_input: int | None, - audio_input: int | None, - audio_output: int | None, -) -> None: +def test_chat_usage_metrics_partition_the_parent_totals(case: _UsageCase) -> None: """Valid nested details partition the parent totals exactly once.""" - payload = _usage_payload( - prompt_tokens=prompt_tokens, - completion_tokens=completion_tokens, - cached_input=cached_input, - audio_input=audio_input, - audio_output=audio_output, - ) + ( + 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 diff --git a/tests/test_uow_autospec_regression.py b/tests/test_uow_autospec_regression.py index af10041e..47b0ae22 100644 --- a/tests/test_uow_autospec_regression.py +++ b/tests/test_uow_autospec_regression.py @@ -14,12 +14,12 @@ def test_unit_of_work_supports_autospec_creation() -> None: so this test fails with ``NameError`` if the imports move back behind ``typing.TYPE_CHECKING``. """ - specced = mock.create_autospec(SqlAlchemyUnitOfWork) + specced = mock.create_autospec(SqlAlchemyUnitOfWork, instance=True) assert specced is not None, "expected an autospecced unit of work" - assert hasattr(specced, "__aenter__"), ( - "the autospecced unit of work must keep its context-manager surface" + assert isinstance(specced.__aenter__, mock.AsyncMock), ( + "the autospecced unit of work must expose an awaitable __aenter__" ) - assert hasattr(specced, "__aexit__"), ( - "the autospecced unit of work must keep its context-manager surface" + assert isinstance(specced.__aexit__, mock.AsyncMock), ( + "the autospecced unit of work must expose an awaitable __aexit__" )