From 3fd56d60d3961b346b98c6db67067f18d16d03a8 Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Wed, 5 Aug 2026 11:56:44 +0200 Subject: [PATCH 1/8] feat(inference): allow disabling chat completions persistence via null store Allow operators to disable chat completion persistence by setting storage.stores.inference to null in a run config. When the reference is absent of value None, the auto-router factory skips constructing and initializing the InferenceStore entirely: no inference_store table is created and no background write workers are started, so no chat completion payload is ever persisted. Chat completions continue to work for both streaming and non-streaming requests. The history endpoints (list, retrieve, messages) raise NotImplementedError, which the exception mapping translates to HTTP 501, rather than returning an empty list or a 404. Other stores stay enabled, so disabling inference persistence is independent of the rest of the storage layer. This follows the same optional-store pattern the Responses store already uses. Signed-off-by: Matt, Matthias --- scripts/README.md | 4 + ...gen_inference_store_disabled_recordings.py | 254 ++++++++++++++++++ src/ogx/core/routers/__init__.py | 27 +- src/ogx/core/storage/README.md | 19 ++ src/ogx/distributions/README.md | 24 ++ ...361486c6de38019206f3281ab9df1227cecd6.json | 61 +++++ ...17deeb6e07aa051ef058f909a676dfb5d67ec.json | 61 +++++ ...d452008645fa0abee31912ff10c86a92340db.json | 61 +++++ ...ff738dba13173691df5dc910a54e642837e20.json | 110 ++++++++ .../test_inference_store_disabled.py | 159 +++++++++++ .../core/routers/test_inference_router.py | 205 +++++++++++++- tests/unit/core/test_storage_references.py | 84 +++++- tests/unit/server/test_resolver.py | 98 ++++++- 13 files changed, 1131 insertions(+), 36 deletions(-) create mode 100644 scripts/gen_inference_store_disabled_recordings.py create mode 100644 tests/integration/inference/recordings/28467c87aa751176cfbae2e4dfe361486c6de38019206f3281ab9df1227cecd6.json create mode 100644 tests/integration/inference/recordings/3765a840ae86565c72696d32a5617deeb6e07aa051ef058f909a676dfb5d67ec.json create mode 100644 tests/integration/inference/recordings/7c573c978200175222e201e0139d452008645fa0abee31912ff10c86a92340db.json create mode 100644 tests/integration/inference/recordings/f783fd17cc659dcfb85c84676b6ff738dba13173691df5dc910a54e642837e20.json create mode 100644 tests/integration/inference/test_inference_store_disabled.py diff --git a/scripts/README.md b/scripts/README.md index 1662e519e63..98255a68ee9 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -22,6 +22,7 @@ scripts/ cleanup_recordings.py # Remove orphaned test recordings diagnose_recordings.py # Debug test recording issues normalize_recordings.py # Normalize test recordings for consistency + gen_inference_store_disabled_recordings.py # Regenerate recordings for the inference-store-disabled integration test docker.sh # Docker build helper install.sh # Installation helper integration-tests.sh # Run integration test suite @@ -66,6 +67,9 @@ uv run python scripts/cleanup_recordings.py uv run python scripts/diagnose_recordings.py # Normalize recordings uv run python scripts/normalize_recordings.py +# Regenerate recordings for the inference-store-disabled integration test +# (records against a local mock OpenAI server; isolates the shared recordings dir) +uv run python scripts/gen_inference_store_disabled_recordings.py ``` ### Remote test recording (via GitHub Actions) diff --git a/scripts/gen_inference_store_disabled_recordings.py b/scripts/gen_inference_store_disabled_recordings.py new file mode 100644 index 00000000000..91112dd60ec --- /dev/null +++ b/scripts/gen_inference_store_disabled_recordings.py @@ -0,0 +1,254 @@ +#!/usr/bin/env python3 +"""Generate recordings for test_inference_store_disabled.py against a local mock OpenAI server. + +Run from repo root: + uv run python scripts/gen_inference_store_disabled_recordings.py + +This spins up a tiny OpenAI-compatible HTTP mock, boots the ci-tests stack with +the inference store disabled and openai pointed at the mock, and exercises the +same chat-completion calls the test makes -- in RECORD mode -- so the recording +harness captures them into tests/integration/inference/recordings/. + +A mock is used instead of a live provider because the test only needs the +id/model populated and asserts nothing about real completion content; this +keeps the recordings self-contained and regenerable offline. +""" + +import asyncio +import json +import os +import re +import shutil +import sys +import tempfile +import threading +from http.server import BaseHTTPRequestHandler, HTTPServer + +import yaml + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, os.path.join(REPO_ROOT, "src")) + +from ogx.core.library_client import OGXAsLibraryClient # noqa: E402 +from ogx.core.stack import get_stack_run_config_from_distro # noqa: E402 +from ogx.core.testing_context import set_test_context # noqa: E402 + +RECORDINGS_DIR = os.path.join( + REPO_ROOT, "tests", "integration", "inference", "recordings" +) + +# The exact prompts the test uses -- the recording hash depends on the body. +TEXT_MODEL = "openai/gpt-4o" +NON_STREAMING_PROMPT = "Say hello." +STREAMING_PROMPT = "Say hello in one sentence." + +# pytest node ids the test will use -- the recording hash includes the test id. +TEST_NODE_IDS = [ + "tests/integration/inference/test_inference_store_disabled.py::test_non_streaming_chat_completion_without_store", + "tests/integration/inference/test_inference_store_disabled.py::test_streaming_chat_completion_without_store", + "tests/integration/inference/test_inference_store_disabled.py::test_retrieve_chat_completion_reports_not_configured", + "tests/integration/inference/test_inference_store_disabled.py::test_list_chat_completion_messages_reports_not_configured", +] + +MOCK_HOST = "127.0.0.1" +MOCK_PORT = 0 # ephemeral + + +def _completion_body(model: str, prompt: str, completion_id: str) -> dict: + return { + "id": completion_id, + "object": "chat.completion", + "created": 0, + "model": model, + "choices": [ + { + "index": 0, + "message": {"role": "assistant", "content": f"Response to: {prompt}"}, + "finish_reason": "stop", + } + ], + "usage": {"prompt_tokens": 5, "completion_tokens": 5, "total_tokens": 10}, + } + + +def _stream_chunks(model: str, completion_id: str): + return [ + { + "id": completion_id, + "object": "chat.completion.chunk", + "created": 0, + "model": model, + "choices": [ + { + "index": 0, + "delta": {"role": "assistant", "content": "Hello"}, + "finish_reason": None, + } + ], + }, + { + "id": completion_id, + "object": "chat.completion.chunk", + "created": 0, + "model": model, + "choices": [ + {"index": 0, "delta": {"content": " world."}, "finish_reason": None} + ], + }, + { + "id": completion_id, + "object": "chat.completion.chunk", + "created": 0, + "model": model, + "choices": [{"index": 0, "delta": {}, "finish_reason": "stop"}], + }, + ] + + +class MockHandler(BaseHTTPRequestHandler): + def do_GET(self): + if self.path.endswith("/models"): + body = json.dumps( + {"object": "list", "data": [{"id": "gpt-4o", "object": "model"}]} + ).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + else: + self.send_response(404) + self.end_headers() + + def do_POST(self): + length = int(self.headers.get("Content-Length", "0")) + raw = self.rfile.read(length) if length else b"" + try: + payload = json.loads(raw) if raw else {} + except Exception: + payload = {} + stream = payload.get("stream", False) + model = payload.get("model", "gpt-4o") + completion_id = "chatcmpl-mock-recording" + if stream: + chunks = _stream_chunks(model, completion_id) + self.send_response(200) + self.send_header("Content-Type", "text/event-stream") + self.end_headers() + for ch in chunks: + self.wfile.write(f"data: {json.dumps(ch)}\n\n".encode()) + self.wfile.flush() + self.wfile.write(b"data: [DONE]\n\n") + self.wfile.flush() + else: + body = _completion_body(model, "Say hello.", completion_id) + data = json.dumps(body).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(data))) + self.end_headers() + self.wfile.write(data) + + def log_message(self, *args): # silence + pass + + +def start_mock_server() -> HTTPServer: + server = HTTPServer((MOCK_HOST, MOCK_PORT), MockHandler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + return server + + +async def run(test_id: str, config_path: str) -> None: + set_test_context(test_id) + client = OGXAsLibraryClient(config_path, skip_logger_removal=True) + try: + if test_id.endswith("::test_streaming_chat_completion_without_store"): + stream = client.chat.completions.create( + model=TEXT_MODEL, + messages=[{"role": "user", "content": STREAMING_PROMPT}], + stream=True, + ) + list(stream) + else: + client.chat.completions.create( + model=TEXT_MODEL, + messages=[{"role": "user", "content": NON_STREAMING_PROMPT}], + ) + finally: + client.shutdown() + + +def main() -> None: + os.environ["OGX_TEST_INFERENCE_MODE"] = "record" + os.environ["OGX_LOGGING"] = "all=warning" + os.environ["OPENAI_API_KEY"] = "fake-key-for-replay" + sqlite_dir = tempfile.mkdtemp(prefix="ogx-record-") + os.environ["SQLITE_STORE_DIR"] = sqlite_dir + + server = start_mock_server() + port = server.server_address[1] + mock_base = f"http://{MOCK_HOST}:{port}/v1" + os.environ["OPENAI_BASE_URL"] = mock_base + + run_config = get_stack_run_config_from_distro("ci-tests") + run_config.storage.stores.inference = None + run_config.vector_stores = None + + config_file = os.path.join(tempfile.mkdtemp(), "run.yaml") + with open(config_file, "w") as f: + yaml.dump(run_config.model_dump(mode="json"), f) + + # Isolate recording so the shared inference recordings directory is untouched. + # The recorder always writes into the test file's ``recordings/`` dir (relative + # to CWD) when a test context is set, so move the real directory aside while + # recording and merge only the chat-completion recordings back afterwards. + backup = None + if os.path.isdir(RECORDINGS_DIR): + backup = RECORDINGS_DIR + ".bak" + shutil.move(RECORDINGS_DIR, backup) + + try: + for test_id in TEST_NODE_IDS: + print(f"recording for {test_id} ...") + asyncio.run(run(test_id, config_file)) + finally: + server.shutdown() + + # Collect the freshly recorded chat-completion recordings (skip models-list + # recordings -- the test does not need them in replay mode) and rewrite their + # mock-host URLs to the canonical provider URL. The recording hash ignores + # the host, so replay works against the real provider URL. + staged = tempfile.mkdtemp(prefix="ogx-staged-") + for name in os.listdir(RECORDINGS_DIR): + if not name.endswith(".json") or name.startswith("models-"): + continue + src = os.path.join(RECORDINGS_DIR, name) + with open(src) as f: + data = json.load(f) + url = data.get("request", {}).get("url", "") + if re.search(r"\d+\.\d+\.\d+\.\d+:\d+", url): + data["request"]["url"] = ( + "https://api.openai.com/v1" + url.split("/v1", 1)[1] + ) + with open(os.path.join(staged, name), "w") as f: + json.dump(data, f, indent=2) + f.write("\n") + + # Restore the original recordings directory and drop in the new recordings. + shutil.rmtree(RECORDINGS_DIR, ignore_errors=True) + if backup is not None: + shutil.move(backup, RECORDINGS_DIR) + else: + os.makedirs(RECORDINGS_DIR, exist_ok=True) + n_written = len(os.listdir(staged)) + for name in os.listdir(staged): + shutil.copy2(os.path.join(staged, name), os.path.join(RECORDINGS_DIR, name)) + shutil.rmtree(staged, ignore_errors=True) + print(f"wrote {n_written} recordings") + print("done") + + +if __name__ == "__main__": + main() diff --git a/src/ogx/core/routers/__init__.py b/src/ogx/core/routers/__init__.py index ebb955b3cc7..35896324369 100644 --- a/src/ogx/core/routers/__init__.py +++ b/src/ogx/core/routers/__init__.py @@ -43,7 +43,11 @@ async def get_routing_table_impl( async def get_auto_router_impl( - api: Api, routing_table: RoutingTable, deps: dict[str, Any], run_config: StackConfig, policy: list[AccessRule] + api: Api, + routing_table: RoutingTable, + deps: dict[str, Any], + run_config: StackConfig, + policy: list[AccessRule], ) -> Any: from .inference import InferenceRouter from .tool_runtime import ToolRuntimeRouter @@ -60,16 +64,19 @@ async def get_auto_router_impl( api_to_dep_impl = {} # TODO: move pass configs to routers instead if api == Api.inference: + # An absent inference store reference disables chat completion + # persistence: no store is constructed, no table is created, and no + # background write workers are started. The inference router handles a + # missing store (it guards every write and raises NotImplementedError + # on the history endpoints), mirroring the optional Responses store. inference_ref = run_config.storage.stores.inference - if not inference_ref: - raise ValueError("storage.stores.inference must be configured in run config") - - inference_store = InferenceStore( - reference=inference_ref, - policy=policy, - ) - await inference_store.initialize() - api_to_dep_impl["store"] = inference_store + if inference_ref: + inference_store = InferenceStore( + reference=inference_ref, + policy=policy, + ) + await inference_store.initialize() + api_to_dep_impl["store"] = inference_store elif api == Api.vector_io: api_to_dep_impl["vector_stores_config"] = run_config.vector_stores api_to_dep_impl["inference_api"] = deps.get(Api.inference) diff --git a/src/ogx/core/storage/README.md b/src/ogx/core/storage/README.md index e4526f8b18e..41c8e5b7d1c 100644 --- a/src/ogx/core/storage/README.md +++ b/src/ogx/core/storage/README.md @@ -54,3 +54,22 @@ The tenancy mode is set process-wide during startup via `set_default_tenancy_mod Storage is configured in `StackConfig.storage` via `StorageConfig`. The `stores` field contains typed references (`KVStoreReference`, `SqlStoreReference`, `InferenceStoreReference`) that point to specific backend configurations. See `datatypes.py` for all config types and `StorageBackendType` for the enum of supported backends. + +### Optional Stores (null to disable) + +Some store references are optional: setting the reference to `null` (not omitting +it) means OGX does not construct the store at all, and the API that depends on +it degrades gracefully. This is an explicit operator choice made in a run config; +the default for every optional reference remains an enabled reference, so +existing deployments are unaffected unless they opt in. + +- **`inference`** -- when `null`, no `InferenceStore` is constructed: no + `inference_store` table is created and no background write workers run. Chat + completions still work (streaming and non-streaming); the chat completion + history endpoints (`list`, `retrieve`, `messages`) report that persistence is + not configured (HTTP 501) rather than returning an empty list or a 404. +- **`responses`** -- the Responses store already follows the same pattern. + +Other stores (`datasets`, `eval`, `files`, `prompts`, `vector_io`) are not +affected by disabling the inference store, so persistence can be turned off for +one API independently of the rest of the storage layer. diff --git a/src/ogx/distributions/README.md b/src/ogx/distributions/README.md index a474983b4b2..b7e63f3d529 100644 --- a/src/ogx/distributions/README.md +++ b/src/ogx/distributions/README.md @@ -52,3 +52,27 @@ ogx run starter # or ogx stack run --config path/to/config.yaml ``` + +## Disabling Chat Completions Persistence + +By default the `inference` store reference is enabled, so chat completion +request/response payloads are persisted and the history endpoints (`list`, +`retrieve`, `messages`) are served from it. An operator can disable that +persistence for the inference API by setting the reference to `null` in a run +config: + +```yaml +storage: + stores: + inference: null +``` + +When `inference` is `null`, OGX does not construct an `InferenceStore` at all: +no `inference_store` table is created and no background write workers run. Chat +completions still work for both streaming and non-streaming requests; the +history endpoints report that persistence is not configured (HTTP 501) rather +than returning an empty list or a 404. Other stores (`responses`, `datasets`, +`eval`, `files`, `prompts`, `vector_io`) stay enabled, so disabling inference +persistence is independent of the rest of the storage layer. This follows the +same optional-store pattern the Responses store already uses; see the storage +module README for details. diff --git a/tests/integration/inference/recordings/28467c87aa751176cfbae2e4dfe361486c6de38019206f3281ab9df1227cecd6.json b/tests/integration/inference/recordings/28467c87aa751176cfbae2e4dfe361486c6de38019206f3281ab9df1227cecd6.json new file mode 100644 index 00000000000..6ec0341385c --- /dev/null +++ b/tests/integration/inference/recordings/28467c87aa751176cfbae2e4dfe361486c6de38019206f3281ab9df1227cecd6.json @@ -0,0 +1,61 @@ +{ + "test_id": "tests/integration/inference/test_inference_store_disabled.py::test_non_streaming_chat_completion_without_store", + "request": { + "method": "POST", + "url": "https://api.openai.com/v1/v1/chat/completions", + "headers": {}, + "body": { + "model": "gpt-4o", + "messages": [ + { + "role": "user", + "content": "Say hello." + } + ] + }, + "endpoint": "/v1/chat/completions", + "model": "gpt-4o", + "provider_metadata": { + "openai_sdk_version": "2.43.0" + } + }, + "response": { + "body": { + "__type__": "openai.types.chat.chat_completion.ChatCompletion", + "__data__": { + "id": "rec-28467c87aa75", + "choices": [ + { + "finish_reason": "stop", + "index": 0, + "logprobs": null, + "message": { + "content": "Response to: Say hello.", + "refusal": null, + "role": "assistant", + "annotations": null, + "audio": null, + "function_call": null, + "tool_calls": null + } + } + ], + "created": 0, + "model": "gpt-4o", + "object": "chat.completion", + "moderation": null, + "service_tier": null, + "system_fingerprint": null, + "usage": { + "completion_tokens": 5, + "prompt_tokens": 5, + "total_tokens": 10, + "completion_tokens_details": null, + "prompt_tokens_details": null + } + } + }, + "is_streaming": false + }, + "id_normalization_mapping": {} +} diff --git a/tests/integration/inference/recordings/3765a840ae86565c72696d32a5617deeb6e07aa051ef058f909a676dfb5d67ec.json b/tests/integration/inference/recordings/3765a840ae86565c72696d32a5617deeb6e07aa051ef058f909a676dfb5d67ec.json new file mode 100644 index 00000000000..e95ba48ddcc --- /dev/null +++ b/tests/integration/inference/recordings/3765a840ae86565c72696d32a5617deeb6e07aa051ef058f909a676dfb5d67ec.json @@ -0,0 +1,61 @@ +{ + "test_id": "tests/integration/inference/test_inference_store_disabled.py::test_list_chat_completion_messages_reports_not_configured", + "request": { + "method": "POST", + "url": "https://api.openai.com/v1/v1/chat/completions", + "headers": {}, + "body": { + "model": "gpt-4o", + "messages": [ + { + "role": "user", + "content": "Say hello." + } + ] + }, + "endpoint": "/v1/chat/completions", + "model": "gpt-4o", + "provider_metadata": { + "openai_sdk_version": "2.43.0" + } + }, + "response": { + "body": { + "__type__": "openai.types.chat.chat_completion.ChatCompletion", + "__data__": { + "id": "rec-3765a840ae86", + "choices": [ + { + "finish_reason": "stop", + "index": 0, + "logprobs": null, + "message": { + "content": "Response to: Say hello.", + "refusal": null, + "role": "assistant", + "annotations": null, + "audio": null, + "function_call": null, + "tool_calls": null + } + } + ], + "created": 0, + "model": "gpt-4o", + "object": "chat.completion", + "moderation": null, + "service_tier": null, + "system_fingerprint": null, + "usage": { + "completion_tokens": 5, + "prompt_tokens": 5, + "total_tokens": 10, + "completion_tokens_details": null, + "prompt_tokens_details": null + } + } + }, + "is_streaming": false + }, + "id_normalization_mapping": {} +} diff --git a/tests/integration/inference/recordings/7c573c978200175222e201e0139d452008645fa0abee31912ff10c86a92340db.json b/tests/integration/inference/recordings/7c573c978200175222e201e0139d452008645fa0abee31912ff10c86a92340db.json new file mode 100644 index 00000000000..7033caf90ac --- /dev/null +++ b/tests/integration/inference/recordings/7c573c978200175222e201e0139d452008645fa0abee31912ff10c86a92340db.json @@ -0,0 +1,61 @@ +{ + "test_id": "tests/integration/inference/test_inference_store_disabled.py::test_retrieve_chat_completion_reports_not_configured", + "request": { + "method": "POST", + "url": "https://api.openai.com/v1/v1/chat/completions", + "headers": {}, + "body": { + "model": "gpt-4o", + "messages": [ + { + "role": "user", + "content": "Say hello." + } + ] + }, + "endpoint": "/v1/chat/completions", + "model": "gpt-4o", + "provider_metadata": { + "openai_sdk_version": "2.43.0" + } + }, + "response": { + "body": { + "__type__": "openai.types.chat.chat_completion.ChatCompletion", + "__data__": { + "id": "rec-7c573c978200", + "choices": [ + { + "finish_reason": "stop", + "index": 0, + "logprobs": null, + "message": { + "content": "Response to: Say hello.", + "refusal": null, + "role": "assistant", + "annotations": null, + "audio": null, + "function_call": null, + "tool_calls": null + } + } + ], + "created": 0, + "model": "gpt-4o", + "object": "chat.completion", + "moderation": null, + "service_tier": null, + "system_fingerprint": null, + "usage": { + "completion_tokens": 5, + "prompt_tokens": 5, + "total_tokens": 10, + "completion_tokens_details": null, + "prompt_tokens_details": null + } + } + }, + "is_streaming": false + }, + "id_normalization_mapping": {} +} diff --git a/tests/integration/inference/recordings/f783fd17cc659dcfb85c84676b6ff738dba13173691df5dc910a54e642837e20.json b/tests/integration/inference/recordings/f783fd17cc659dcfb85c84676b6ff738dba13173691df5dc910a54e642837e20.json new file mode 100644 index 00000000000..112c6008cf6 --- /dev/null +++ b/tests/integration/inference/recordings/f783fd17cc659dcfb85c84676b6ff738dba13173691df5dc910a54e642837e20.json @@ -0,0 +1,110 @@ +{ + "test_id": "tests/integration/inference/test_inference_store_disabled.py::test_streaming_chat_completion_without_store", + "request": { + "method": "POST", + "url": "https://api.openai.com/v1/v1/chat/completions", + "headers": {}, + "body": { + "model": "gpt-4o", + "messages": [ + { + "role": "user", + "content": "Say hello in one sentence." + } + ], + "stream": true + }, + "endpoint": "/v1/chat/completions", + "model": "gpt-4o", + "provider_metadata": { + "openai_sdk_version": "2.43.0" + } + }, + "response": { + "body": [ + { + "__type__": "openai.types.chat.chat_completion_chunk.ChatCompletionChunk", + "__data__": { + "id": "rec-f783fd17cc65", + "choices": [ + { + "delta": { + "content": "Hello", + "function_call": null, + "refusal": null, + "role": "assistant", + "tool_calls": null + }, + "finish_reason": null, + "index": 0, + "logprobs": null + } + ], + "created": 0, + "model": "gpt-4o", + "object": "chat.completion.chunk", + "moderation": null, + "service_tier": null, + "system_fingerprint": null, + "usage": null + } + }, + { + "__type__": "openai.types.chat.chat_completion_chunk.ChatCompletionChunk", + "__data__": { + "id": "rec-f783fd17cc65", + "choices": [ + { + "delta": { + "content": " world.", + "function_call": null, + "refusal": null, + "role": null, + "tool_calls": null + }, + "finish_reason": null, + "index": 0, + "logprobs": null + } + ], + "created": 0, + "model": "gpt-4o", + "object": "chat.completion.chunk", + "moderation": null, + "service_tier": null, + "system_fingerprint": null, + "usage": null + } + }, + { + "__type__": "openai.types.chat.chat_completion_chunk.ChatCompletionChunk", + "__data__": { + "id": "rec-f783fd17cc65", + "choices": [ + { + "delta": { + "content": null, + "function_call": null, + "refusal": null, + "role": null, + "tool_calls": null + }, + "finish_reason": "stop", + "index": 0, + "logprobs": null + } + ], + "created": 0, + "model": "gpt-4o", + "object": "chat.completion.chunk", + "moderation": null, + "service_tier": null, + "system_fingerprint": null, + "usage": null + } + } + ], + "is_streaming": true + }, + "id_normalization_mapping": {} +} diff --git a/tests/integration/inference/test_inference_store_disabled.py b/tests/integration/inference/test_inference_store_disabled.py new file mode 100644 index 00000000000..4b7a9e32f4b --- /dev/null +++ b/tests/integration/inference/test_inference_store_disabled.py @@ -0,0 +1,159 @@ +# Copyright (c) The OGX Contributors. +# All rights reserved. +# +# This source code is licensed under the terms described in the LICENSE file in +# the root directory of this source tree. + +"""Chat Completions behavior when persistence is disabled. + +When an operator configures ``storage.stores.inference`` as ``null``, OGX must +not construct an ``InferenceStore`` at all: no ``inference_store`` table is +created, no background write workers run, and no chat completion request or +response payload is ever persisted. Chat completions still work for both +streaming and non-streaming requests, and the history endpoints report that +persistence is not configured (501) rather than returning an empty list or 404. + +This test builds a ``StackConfig`` from the ``ci-tests`` distribution with the +inference store reference removed, boots an in-process library client from it, +and exercises the full HTTP path through the OpenAI-compatible client. +""" + +import os +import tempfile +from pathlib import Path + +import pytest +import yaml + +from ogx.core.datatypes import StackConfig +from ogx.core.library_client import OGXAsLibraryClient +from ogx.core.stack import get_stack_run_config_from_distro + +# A model reachable through the ci-tests routing via the provider fallback path +# (``provider_id/resource_id``). The completion request against the provider is +# replayed from a recording, so no live API key is needed. +TEXT_MODEL = "openai/gpt-4o" + +NON_STREAMING_PROMPT = "Say hello." +STREAMING_PROMPT = "Say hello in one sentence." + + +def _build_non_persisting_config(sqlite_dir: str) -> tuple[StackConfig, Path]: + run_config = get_stack_run_config_from_distro("ci-tests") + # Disable chat completion persistence: no store is constructed, no table is + # created, and no background write workers are started. + run_config.storage.stores.inference = None + # Vector-store config pulls in embedding/reranker model validation that is + # unrelated to chat completion persistence and makes every in-process boot + # load the sentence-transformers stack; drop it so booting is deterministic. + run_config.vector_stores = None + return run_config, Path(sqlite_dir) / "sql_store.db" + + +@pytest.fixture(scope="session") +def non_persisting_client(): + """Boot an in-process library client with chat completion persistence disabled. + + Booted once for the whole session because constructing the ci-tests stack is + expensive. These tests do not mutate shared client state, so a single shared + client is safe. + """ + # The OpenAI provider validates that an API key is present before the + # recording harness short-circuits the request in replay mode. A fake key + # is sufficient because the request is never sent to the provider. + os.environ["OPENAI_API_KEY"] = "fake-key-for-replay" + sqlite_dir = tempfile.mkdtemp(prefix="ogx-no-store-") + os.environ["SQLITE_STORE_DIR"] = sqlite_dir + run_config, sql_db = _build_non_persisting_config(sqlite_dir) + config_file = tempfile.NamedTemporaryFile(delete=False, suffix="-run.yaml").name + with open(config_file, "w") as f: + yaml.dump(run_config.model_dump(mode="json"), f) + client = OGXAsLibraryClient(config_file, skip_logger_removal=True) + try: + yield client, sql_db + finally: + client.shutdown() + os.unlink(config_file) + + +def test_non_streaming_chat_completion_without_store(non_persisting_client): + """A non-streaming completion is returned normally with id/model populated.""" + client, _ = non_persisting_client + response = client.chat.completions.create( + model=TEXT_MODEL, + messages=[{"role": "user", "content": NON_STREAMING_PROMPT}], + ) + assert response.id + assert response.model == TEXT_MODEL + assert response.choices + assert response.choices[0].message.content + + +def test_streaming_chat_completion_without_store(non_persisting_client): + """A streaming completion streams normally with the requested model id.""" + client, _ = non_persisting_client + stream = client.chat.completions.create( + model=TEXT_MODEL, + messages=[{"role": "user", "content": STREAMING_PROMPT}], + stream=True, + ) + chunks = list(stream) + assert chunks + response_id = None + for chunk in chunks: + assert chunk.model == TEXT_MODEL + if chunk.id: + response_id = chunk.id + assert response_id + + +def test_list_chat_completions_reports_not_configured(non_persisting_client): + """list raises a not-configured error rather than returning an empty list. + + In library-client mode the router's ``NotImplementedError`` propagates + directly; the server's exception mapping translates it to HTTP 501. + """ + client, _ = non_persisting_client + with pytest.raises(NotImplementedError): + client.chat.completions.list(limit=10) + + +def test_retrieve_chat_completion_reports_not_configured(non_persisting_client): + """retrieve for a just-completed id raises a not-configured error rather than a 404.""" + client, _ = non_persisting_client + response = client.chat.completions.create( + model=TEXT_MODEL, + messages=[{"role": "user", "content": NON_STREAMING_PROMPT}], + ) + with pytest.raises(NotImplementedError): + client.chat.completions.retrieve(response.id) + + +def test_list_chat_completion_messages_reports_not_configured(non_persisting_client): + """messages raises a not-configured error, consistent with list/retrieve.""" + client, _ = non_persisting_client + response = client.chat.completions.create( + model=TEXT_MODEL, + messages=[{"role": "user", "content": NON_STREAMING_PROMPT}], + ) + with pytest.raises(NotImplementedError): + client.chat.completions.messages.list(completion_id=response.id) + + +def test_no_inference_store_table_when_persistence_disabled(non_persisting_client): + """No ``inference_store`` table exists in the SQL backend, proving payloads were never written.""" + _client, sql_db = non_persisting_client + if not Path(sql_db).exists(): + pytest.skip(f"SQL backend db not found at {sql_db}") + import sqlite3 + + conn = sqlite3.connect(str(sql_db)) + try: + rows = conn.execute( + "SELECT name FROM sqlite_master WHERE type='table' AND name='inference_store'" + ).fetchall() + finally: + conn.close() + assert ( + rows == [] + ), "inference_store table must not exist when persistence is disabled" diff --git a/tests/unit/core/routers/test_inference_router.py b/tests/unit/core/routers/test_inference_router.py index 80e951e7098..624eca3fdbc 100644 --- a/tests/unit/core/routers/test_inference_router.py +++ b/tests/unit/core/routers/test_inference_router.py @@ -22,6 +22,9 @@ from ogx.core.routers.inference import InferenceRouter from ogx_api import ( + GetChatCompletionRequest, + ListChatCompletionMessagesRequest, + ListChatCompletionsRequest, ModelType, OpenAICompletion, OpenAICompletionRequestWithExtraBody, @@ -30,7 +33,16 @@ RoutingTable, ) from ogx_api.inference import RerankRequest -from ogx_api.inference.models import OpenAICompletionChoice +from ogx_api.inference.models import ( + OpenAIChatCompletion, + OpenAIChatCompletionChunk, + OpenAIChatCompletionRequestWithExtraBody, + OpenAIChatCompletionResponseMessage, + OpenAIChoice, + OpenAIChoiceDelta, + OpenAIChunkChoice, + OpenAICompletionChoice, +) @pytest.fixture @@ -110,10 +122,14 @@ async def provider_stream(): chunks = [chunk async for chunk in stream] assert len(chunks) == 2 - assert [chunk.model for chunk in chunks] == ["test_provider/test-llm-model", "test_provider/test-llm-model"], ( - "Streamed completion chunks should carry the requested model id, not the provider resource id" - ) - assert [choice.text for chunk in chunks for choice in chunk.choices] == ["Hello", " world"] + assert [chunk.model for chunk in chunks] == [ + "test_provider/test-llm-model", + "test_provider/test-llm-model", + ], "Streamed completion chunks should carry the requested model id, not the provider resource id" + assert [choice.text for chunk in chunks for choice in chunk.choices] == [ + "Hello", + " world", + ] # The provider itself should still be called with its own resource id called_params = mock_provider.openai_completion.call_args.args[0] @@ -143,7 +159,9 @@ async def provider_stream(): assert chunks == [] -async def test_openai_completion_streaming_model_id_already_correct(mock_llm_routing_table): +async def test_openai_completion_streaming_model_id_already_correct( + mock_llm_routing_table, +): """Chunks that already carry the fully qualified model id are passed through unchanged.""" routing_table, mock_provider = mock_llm_routing_table router = InferenceRouter(routing_table=routing_table) @@ -188,11 +206,19 @@ async def provider_stream(): stream = await router.openai_completion(params) chunks = [chunk async for chunk in stream] - assert [chunk.model for chunk in chunks] == ["test_provider/test-llm-model", "test_provider/test-llm-model"] - assert [choice.text for chunk in chunks for choice in chunk.choices] == ["Hello", " world"] + assert [chunk.model for chunk in chunks] == [ + "test_provider/test-llm-model", + "test_provider/test-llm-model", + ] + assert [choice.text for chunk in chunks for choice in chunk.choices] == [ + "Hello", + " world", + ] -async def test_openai_completion_streaming_propagates_provider_errors(mock_llm_routing_table): +async def test_openai_completion_streaming_propagates_provider_errors( + mock_llm_routing_table, +): """Errors raised by the provider mid-stream propagate to the caller after earlier chunks are delivered.""" routing_table, mock_provider = mock_llm_routing_table router = InferenceRouter(routing_table=routing_table) @@ -219,7 +245,9 @@ async def provider_stream(): assert chunks[0].model == "test_provider/test-llm-model" -async def test_openai_completion_non_streaming_rewrites_model_id(mock_llm_routing_table): +async def test_openai_completion_non_streaming_rewrites_model_id( + mock_llm_routing_table, +): """Non-streaming /v1/completions responses report the requested model id (regression guard).""" routing_table, mock_provider = mock_llm_routing_table router = InferenceRouter(routing_table=routing_table) @@ -269,14 +297,165 @@ async def test_rerank_calls_provider_correctly(mock_routing_table): mock_provider.rerank.assert_called_once() call_args = mock_provider.rerank.call_args - assert len(call_args.args) == 1, "Provider.rerank should be called with exactly one argument" - assert isinstance(call_args.args[0], RerankRequest), "Provider.rerank should receive a RerankRequest object" + assert ( + len(call_args.args) == 1 + ), "Provider.rerank should be called with exactly one argument" + assert isinstance( + call_args.args[0], RerankRequest + ), "Provider.rerank should receive a RerankRequest object" called_request = call_args.args[0] - assert called_request.model == "provider-rerank-model-123", "Model should be substituted with provider_resource_id" + assert ( + called_request.model == "provider-rerank-model-123" + ), "Model should be substituted with provider_resource_id" assert called_request.query == "test query" assert called_request.items == ["item1", "item2"] assert called_request.max_num_results == 1 assert result == expected_response + + +def _make_chat_completion( + model: str = "test_provider/test-llm-model", +) -> OpenAIChatCompletion: + """Build a minimal non-streaming chat completion response.""" + return OpenAIChatCompletion( + id="chatcmpl-test", + choices=[ + OpenAIChoice( + message=OpenAIChatCompletionResponseMessage( + role="assistant", content="Hello world" + ), + finish_reason="stop", + index=0, + ) + ], + created=0, + model=model, + ) + + +def _make_chat_completion_chunk( + text: str, model: str = "test-llm-model" +) -> OpenAIChatCompletionChunk: + """Build a minimal streaming chat completion chunk.""" + return OpenAIChatCompletionChunk( + id="chatcmpl-test", + choices=[ + OpenAIChunkChoice( + delta=OpenAIChoiceDelta(content=text, role="assistant"), + finish_reason=None, + index=0, + ) + ], + created=0, + model=model, + ) + + +async def test_openai_chat_completion_non_streaming_without_store( + mock_llm_routing_table, +): + """A non-streaming chat completion succeeds when persistence is disabled (no store). + + The completion is returned to the caller with the requested model id and the + router never attempts to store it (there is no store). + """ + routing_table, mock_provider = mock_llm_routing_table + router = InferenceRouter(routing_table=routing_table) + assert router.store is None + + mock_provider.openai_chat_completion = AsyncMock( + return_value=_make_chat_completion() + ) + + params = OpenAIChatCompletionRequestWithExtraBody( + model="test_provider/test-llm-model", + messages=[{"role": "user", "content": "Say hello"}], + ) + + response = await router.openai_chat_completion(params) + + assert response.id == "chatcmpl-test" + assert response.model == "test_provider/test-llm-model" + assert response.choices[0].message.content == "Hello world" + # Provider was called with its resource id, not the fully qualified id. + assert ( + mock_provider.openai_chat_completion.call_args.args[0].model == "test-llm-model" + ) + + +async def test_openai_chat_completion_streaming_without_store(mock_llm_routing_table): + """A streaming chat completion streams normally when persistence is disabled. + + Chunks are rewritten to carry the requested model id and the router never + attempts to assemble/store a final completion (there is no store). + """ + routing_table, mock_provider = mock_llm_routing_table + router = InferenceRouter(routing_table=routing_table) + assert router.store is None + + async def provider_stream(): + yield _make_chat_completion_chunk("Hello") + yield _make_chat_completion_chunk(" world") + + mock_provider.openai_chat_completion = AsyncMock(return_value=provider_stream()) + + params = OpenAIChatCompletionRequestWithExtraBody( + model="test_provider/test-llm-model", + messages=[{"role": "user", "content": "Say hello"}], + stream=True, + ) + + stream = await router.openai_chat_completion(params) + chunks = [chunk async for chunk in stream] + + assert len(chunks) == 2 + assert [chunk.model for chunk in chunks] == [ + "test_provider/test-llm-model", + "test_provider/test-llm-model", + ] + assert [ + "".join(c.delta.content or "" for c in chunk.choices) for chunk in chunks + ] == ["Hello", " world"] + + +async def test_list_chat_completions_without_store_raises_not_implemented( + mock_llm_routing_table, +): + """The list history endpoint reports an error (not an empty list) when persistence is off.""" + routing_table, _ = mock_llm_routing_table + router = InferenceRouter(routing_table=routing_table) + assert router.store is None + + with pytest.raises(NotImplementedError): + await router.list_chat_completions(ListChatCompletionsRequest()) + + +async def test_get_chat_completion_without_store_raises_not_implemented( + mock_llm_routing_table, +): + """The retrieve history endpoint reports an error (not a 404) when persistence is off.""" + routing_table, _ = mock_llm_routing_table + router = InferenceRouter(routing_table=routing_table) + assert router.store is None + + with pytest.raises(NotImplementedError): + await router.get_chat_completion( + GetChatCompletionRequest(completion_id="chatcmpl-test") + ) + + +async def test_list_chat_completion_messages_without_store_raises_not_implemented( + mock_llm_routing_table, +): + """The messages history endpoint reports an error when persistence is off, consistent with list/retrieve.""" + routing_table, _ = mock_llm_routing_table + router = InferenceRouter(routing_table=routing_table) + assert router.store is None + + with pytest.raises(NotImplementedError): + await router.list_chat_completion_messages( + ListChatCompletionMessagesRequest(completion_id="chatcmpl-test") + ) diff --git a/tests/unit/core/test_storage_references.py b/tests/unit/core/test_storage_references.py index 413458f1d2e..1b07e895977 100644 --- a/tests/unit/core/test_storage_references.py +++ b/tests/unit/core/test_storage_references.py @@ -7,6 +7,7 @@ """Unit tests for storage backend/reference validation.""" import os +from typing import Any import pytest from pydantic import ValidationError @@ -65,16 +66,24 @@ def _base_run_config(**overrides): def test_references_require_known_backend(): with pytest.raises(ValidationError, match="unknown backend 'missing'"): - _base_run_config(metadata_reference=KVStoreReference(backend="missing", namespace="registry")) + _base_run_config( + metadata_reference=KVStoreReference(backend="missing", namespace="registry") + ) def test_references_must_match_backend_family(): with pytest.raises(ValidationError, match="kv_.* is required"): - _base_run_config(metadata_reference=KVStoreReference(backend="sql_default", namespace="registry")) + _base_run_config( + metadata_reference=KVStoreReference( + backend="sql_default", namespace="registry" + ) + ) with pytest.raises(ValidationError, match="sql_.* is required"): _base_run_config( - inference_reference=InferenceStoreReference(backend="kv_default", table_name="inference"), + inference_reference=InferenceStoreReference( + backend="kv_default", table_name="inference" + ), ) @@ -83,7 +92,62 @@ def test_valid_configuration_passes_validation(): stores = config.storage.stores assert stores.metadata is not None and stores.metadata.backend == "kv_default" assert stores.inference is not None and stores.inference.backend == "sql_default" - assert stores.conversations is not None and stores.conversations.backend == "sql_default" + assert ( + stores.conversations is not None + and stores.conversations.backend == "sql_default" + ) + + +def test_inference_store_defaults_enabled_when_omitted(): + """Omitting the `inference` key keeps persistence enabled (backward compatible default). + + The optional typing on `ServerStoresConfig.inference` means `None` is a valid, + explicit, opt-in way to disable persistence. The default must remain an enabled + reference so that existing deployments that omit the key keep their behavior, + and so that a valid non-persisting config is expressed as `inference: null` + rather than by deleting the key. + """ + stores = ServerStoresConfig() + assert stores.inference is not None + assert stores.inference.backend == "sql_default" + assert stores.inference.table_name == "inference_store" + + +def test_inference_store_none_disables_persistence(): + """Setting `inference` to null explicitly opts out of chat completion persistence.""" + stores = ServerStoresConfig(inference=None) + assert stores.inference is None + + +def test_inference_store_config_omit_vs_null_parsing(): + """Lock the omit-vs-null semantics at the StackConfig parsing level. + + - `inference` key absent -> enabled default (persistence on) + - `inference: null` -> disabled (persistence off) + """ + base = _default_stores_dict() + + # Omit the inference key entirely: the default applies (persistence on). + omitted = dict(base) + omitted.pop("inference") + stores_omitted = ServerStoresConfig.model_validate(omitted) + assert stores_omitted.inference is not None + + # Explicitly set inference to null: persistence disabled. + explicit_null = dict(base) + explicit_null["inference"] = None + stores_null = ServerStoresConfig.model_validate(explicit_null) + assert stores_null.inference is None + + +def _default_stores_dict() -> dict[str, Any]: + """A valid `ServerStoresConfig` serialized with only the non-inference keys.""" + return ServerStoresConfig( + metadata=KVStoreReference(backend="kv_default", namespace="registry"), + conversations=SqlStoreReference( + backend="sql_default", table_name="conversations" + ), + ).model_dump(mode="python") @pytest.mark.parametrize("backend_key", ["kv_default", "sql_default"]) @@ -98,7 +162,9 @@ def test_default_backends_resolve_env_vars(backend_key, monkeypatch): monkeypatch.delenv("SQLITE_STORE_DIR", raising=False) config = StorageConfig() db_path = config.backends[backend_key].db_path - assert "${env." not in db_path, f"Unresolved env var syntax in default {backend_key}: {db_path}" + assert ( + "${env." not in db_path + ), f"Unresolved env var syntax in default {backend_key}: {db_path}" def test_default_backends_respect_sqlite_store_dir(monkeypatch): @@ -115,8 +181,12 @@ def test_default_backends_fallback_to_distribs_base_dir(monkeypatch): monkeypatch.delenv("SQLITE_STORE_DIR", raising=False) config = StorageConfig() - assert config.backends["kv_default"].db_path == os.path.join(str(DISTRIBS_BASE_DIR), "kvstore.db") - assert config.backends["sql_default"].db_path == os.path.join(str(DISTRIBS_BASE_DIR), "sql_store.db") + assert config.backends["kv_default"].db_path == os.path.join( + str(DISTRIBS_BASE_DIR), "kvstore.db" + ) + assert config.backends["sql_default"].db_path == os.path.join( + str(DISTRIBS_BASE_DIR), "sql_store.db" + ) def test_default_backends_expand_user_home(monkeypatch): diff --git a/tests/unit/server/test_resolver.py b/tests/unit/server/test_resolver.py index 5a4be8c2001..fe408648157 100644 --- a/tests/unit/server/test_resolver.py +++ b/tests/unit/server/test_resolver.py @@ -7,7 +7,7 @@ import inspect import sys from typing import Any, Protocol -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import AsyncMock, MagicMock, patch from pydantic import BaseModel, Field @@ -60,7 +60,12 @@ def sample_run_config(cls, **kwargs: Any) -> dict[str, Any]: class SampleImpl: - def __init__(self, config: SampleConfig, deps: dict[Api, Any], provider_spec: ProviderSpec = None): + def __init__( + self, + config: SampleConfig, + deps: dict[Api, Any], + provider_spec: ProviderSpec = None, + ): self.__provider_id__ = "test_provider" self.__provider_spec__ = provider_spec self.__provider_config__ = config @@ -81,14 +86,28 @@ def make_run_config(**overrides) -> StackConfig: }, stores=ServerStoresConfig( metadata=KVStoreReference(backend="kv_default", namespace="registry"), - inference=InferenceStoreReference(backend="sql_default", table_name="inference_store"), - conversations=SqlStoreReference(backend="sql_default", table_name="conversations"), + inference=InferenceStoreReference( + backend="sql_default", table_name="inference_store" + ), + conversations=SqlStoreReference( + backend="sql_default", table_name="conversations" + ), ), ), ) - register_kvstore_backends({name: cfg for name, cfg in storage.backends.items() if cfg.type.value.startswith("kv_")}) + register_kvstore_backends( + { + name: cfg + for name, cfg in storage.backends.items() + if cfg.type.value.startswith("kv_") + } + ) register_sqlstore_backends( - {name: cfg for name, cfg in storage.backends.items() if cfg.type.value.startswith("sql_")} + { + name: cfg + for name, cfg in storage.backends.items() + if cfg.type.value.startswith("sql_") + } ) defaults = dict( distro_name="test_image", @@ -149,3 +168,70 @@ async def test_resolve_impls_basic(): assert impl.foo == "baz" assert impl.__provider_id__ == "sample_provider" assert impl.__provider_spec__ == provider_spec + + +async def test_resolve_impls_inference_without_store_skips_persistence(): + """An absent inference store reference must not construct an InferenceStore. + + When `storage.stores.inference` is None, the auto-router factory skips + constructing (and initializing) the InferenceStore entirely and builds the + InferenceRouter with no store dependency. No table is created and no + background write workers are started, so no chat completion payload is ever + persisted. + """ + provider_spec = InlineProviderSpec( + api=Api.inference, + provider_type="sample", + module="test_module", + config_class="test_resolver.SampleConfig", + api_dependencies=[], + ) + + provider_registry = {Api.inference: {provider_spec.provider_type: provider_spec}} + + run_config = make_run_config( + distro_name="test_image", + providers={ + "inference": [ + Provider( + provider_id="sample_provider", + provider_type="sample", + config=SampleConfig.sample_run_config(), + ) + ] + }, + storage=StorageConfig( + backends={ + "kv_default": SqliteKVStoreConfig(db_path=":memory:"), + "sql_default": SqliteSqlStoreConfig(db_path=":memory:"), + }, + stores=ServerStoresConfig( + metadata=KVStoreReference(backend="kv_default", namespace="registry"), + inference=None, + conversations=SqlStoreReference( + backend="sql_default", table_name="conversations" + ), + ), + ), + ) + + dist_registry = MagicMock() + + mock_module = MagicMock() + impl = SampleImpl(SampleConfig(foo="baz"), {}, provider_spec) + add_protocol_methods(SampleImpl, Inference) + + mock_module.get_provider_impl = AsyncMock(return_value=impl) + mock_module.get_provider_impl.__text_signature__ = "()" + sys.modules["test_module"] = mock_module + + with patch("ogx.core.routers.InferenceStore") as mock_inference_store: + impls = await resolve_impls( + run_config, provider_registry, dist_registry, policy={} + ) + + mock_inference_store.assert_not_called() + + router = impls[Api.inference] + assert isinstance(router, InferenceRouter) + assert router.store is None From a449f10b4aeb28e08379f4c87a12d59cf8e0fd6b Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Wed, 5 Aug 2026 15:11:37 +0200 Subject: [PATCH 2/8] chore: revert incidental reformatting from inference store commit Drop the formatting-only changes that were bundled into 'feat(inference): allow disabling chat completions persistence via null store' so the feature diff is limited to the actual behavior change: - Restore the pre-existing formatting of the get_auto_router_impl signature in src/ogx/core/routers/__init__.py and of untouched lines in the touched unit tests. - Normalize lines added by the feature (new tests and the recording generator script) with the pinned ruff 0.12.2 used by pre-commit. - Add the project license header to the recording generator script and make it executable like the other scripts/ helpers, and add noqa: N802 markers to its BaseHTTPRequestHandler.do_GET/do_POST, matching precedent in tests/integration/telemetry/collectors/otlp.py. No functional changes; all affected unit tests still pass. Signed-off-by: Matt, Matthias --- ...gen_inference_store_disabled_recordings.py | 26 +++---- src/ogx/core/routers/__init__.py | 6 +- .../core/routers/test_inference_router.py | 74 +++++-------------- tests/unit/core/test_storage_references.py | 35 ++------- tests/unit/server/test_resolver.py | 37 ++-------- 5 files changed, 47 insertions(+), 131 deletions(-) mode change 100644 => 100755 scripts/gen_inference_store_disabled_recordings.py diff --git a/scripts/gen_inference_store_disabled_recordings.py b/scripts/gen_inference_store_disabled_recordings.py old mode 100644 new mode 100755 index 91112dd60ec..74af980be81 --- a/scripts/gen_inference_store_disabled_recordings.py +++ b/scripts/gen_inference_store_disabled_recordings.py @@ -1,4 +1,10 @@ #!/usr/bin/env python3 +# Copyright (c) The OGX Contributors. +# All rights reserved. +# +# This source code is licensed under the terms described in the LICENSE file in +# the root directory of this source tree. + """Generate recordings for test_inference_store_disabled.py against a local mock OpenAI server. Run from repo root: @@ -33,9 +39,7 @@ from ogx.core.stack import get_stack_run_config_from_distro # noqa: E402 from ogx.core.testing_context import set_test_context # noqa: E402 -RECORDINGS_DIR = os.path.join( - REPO_ROOT, "tests", "integration", "inference", "recordings" -) +RECORDINGS_DIR = os.path.join(REPO_ROOT, "tests", "integration", "inference", "recordings") # The exact prompts the test uses -- the recording hash depends on the body. TEXT_MODEL = "openai/gpt-4o" @@ -91,9 +95,7 @@ def _stream_chunks(model: str, completion_id: str): "object": "chat.completion.chunk", "created": 0, "model": model, - "choices": [ - {"index": 0, "delta": {"content": " world."}, "finish_reason": None} - ], + "choices": [{"index": 0, "delta": {"content": " world."}, "finish_reason": None}], }, { "id": completion_id, @@ -106,11 +108,9 @@ def _stream_chunks(model: str, completion_id: str): class MockHandler(BaseHTTPRequestHandler): - def do_GET(self): + def do_GET(self): # noqa: N802 Function name `do_GET` should be lowercase if self.path.endswith("/models"): - body = json.dumps( - {"object": "list", "data": [{"id": "gpt-4o", "object": "model"}]} - ).encode() + body = json.dumps({"object": "list", "data": [{"id": "gpt-4o", "object": "model"}]}).encode() self.send_response(200) self.send_header("Content-Type", "application/json") self.send_header("Content-Length", str(len(body))) @@ -120,7 +120,7 @@ def do_GET(self): self.send_response(404) self.end_headers() - def do_POST(self): + def do_POST(self): # noqa: N802 Function name `do_POST` should be lowercase length = int(self.headers.get("Content-Length", "0")) raw = self.rfile.read(length) if length else b"" try: @@ -229,9 +229,7 @@ def main() -> None: data = json.load(f) url = data.get("request", {}).get("url", "") if re.search(r"\d+\.\d+\.\d+\.\d+:\d+", url): - data["request"]["url"] = ( - "https://api.openai.com/v1" + url.split("/v1", 1)[1] - ) + data["request"]["url"] = "https://api.openai.com/v1" + url.split("/v1", 1)[1] with open(os.path.join(staged, name), "w") as f: json.dump(data, f, indent=2) f.write("\n") diff --git a/src/ogx/core/routers/__init__.py b/src/ogx/core/routers/__init__.py index 35896324369..6b48b52956e 100644 --- a/src/ogx/core/routers/__init__.py +++ b/src/ogx/core/routers/__init__.py @@ -43,11 +43,7 @@ async def get_routing_table_impl( async def get_auto_router_impl( - api: Api, - routing_table: RoutingTable, - deps: dict[str, Any], - run_config: StackConfig, - policy: list[AccessRule], + api: Api, routing_table: RoutingTable, deps: dict[str, Any], run_config: StackConfig, policy: list[AccessRule] ) -> Any: from .inference import InferenceRouter from .tool_runtime import ToolRuntimeRouter diff --git a/tests/unit/core/routers/test_inference_router.py b/tests/unit/core/routers/test_inference_router.py index 624eca3fdbc..921b1fd0295 100644 --- a/tests/unit/core/routers/test_inference_router.py +++ b/tests/unit/core/routers/test_inference_router.py @@ -122,14 +122,10 @@ async def provider_stream(): chunks = [chunk async for chunk in stream] assert len(chunks) == 2 - assert [chunk.model for chunk in chunks] == [ - "test_provider/test-llm-model", - "test_provider/test-llm-model", - ], "Streamed completion chunks should carry the requested model id, not the provider resource id" - assert [choice.text for chunk in chunks for choice in chunk.choices] == [ - "Hello", - " world", - ] + assert [chunk.model for chunk in chunks] == ["test_provider/test-llm-model", "test_provider/test-llm-model"], ( + "Streamed completion chunks should carry the requested model id, not the provider resource id" + ) + assert [choice.text for chunk in chunks for choice in chunk.choices] == ["Hello", " world"] # The provider itself should still be called with its own resource id called_params = mock_provider.openai_completion.call_args.args[0] @@ -159,9 +155,7 @@ async def provider_stream(): assert chunks == [] -async def test_openai_completion_streaming_model_id_already_correct( - mock_llm_routing_table, -): +async def test_openai_completion_streaming_model_id_already_correct(mock_llm_routing_table): """Chunks that already carry the fully qualified model id are passed through unchanged.""" routing_table, mock_provider = mock_llm_routing_table router = InferenceRouter(routing_table=routing_table) @@ -206,19 +200,11 @@ async def provider_stream(): stream = await router.openai_completion(params) chunks = [chunk async for chunk in stream] - assert [chunk.model for chunk in chunks] == [ - "test_provider/test-llm-model", - "test_provider/test-llm-model", - ] - assert [choice.text for chunk in chunks for choice in chunk.choices] == [ - "Hello", - " world", - ] + assert [chunk.model for chunk in chunks] == ["test_provider/test-llm-model", "test_provider/test-llm-model"] + assert [choice.text for chunk in chunks for choice in chunk.choices] == ["Hello", " world"] -async def test_openai_completion_streaming_propagates_provider_errors( - mock_llm_routing_table, -): +async def test_openai_completion_streaming_propagates_provider_errors(mock_llm_routing_table): """Errors raised by the provider mid-stream propagate to the caller after earlier chunks are delivered.""" routing_table, mock_provider = mock_llm_routing_table router = InferenceRouter(routing_table=routing_table) @@ -245,9 +231,7 @@ async def provider_stream(): assert chunks[0].model == "test_provider/test-llm-model" -async def test_openai_completion_non_streaming_rewrites_model_id( - mock_llm_routing_table, -): +async def test_openai_completion_non_streaming_rewrites_model_id(mock_llm_routing_table): """Non-streaming /v1/completions responses report the requested model id (regression guard).""" routing_table, mock_provider = mock_llm_routing_table router = InferenceRouter(routing_table=routing_table) @@ -297,17 +281,11 @@ async def test_rerank_calls_provider_correctly(mock_routing_table): mock_provider.rerank.assert_called_once() call_args = mock_provider.rerank.call_args - assert ( - len(call_args.args) == 1 - ), "Provider.rerank should be called with exactly one argument" - assert isinstance( - call_args.args[0], RerankRequest - ), "Provider.rerank should receive a RerankRequest object" + assert len(call_args.args) == 1, "Provider.rerank should be called with exactly one argument" + assert isinstance(call_args.args[0], RerankRequest), "Provider.rerank should receive a RerankRequest object" called_request = call_args.args[0] - assert ( - called_request.model == "provider-rerank-model-123" - ), "Model should be substituted with provider_resource_id" + assert called_request.model == "provider-rerank-model-123", "Model should be substituted with provider_resource_id" assert called_request.query == "test query" assert called_request.items == ["item1", "item2"] @@ -324,9 +302,7 @@ def _make_chat_completion( id="chatcmpl-test", choices=[ OpenAIChoice( - message=OpenAIChatCompletionResponseMessage( - role="assistant", content="Hello world" - ), + message=OpenAIChatCompletionResponseMessage(role="assistant", content="Hello world"), finish_reason="stop", index=0, ) @@ -336,9 +312,7 @@ def _make_chat_completion( ) -def _make_chat_completion_chunk( - text: str, model: str = "test-llm-model" -) -> OpenAIChatCompletionChunk: +def _make_chat_completion_chunk(text: str, model: str = "test-llm-model") -> OpenAIChatCompletionChunk: """Build a minimal streaming chat completion chunk.""" return OpenAIChatCompletionChunk( id="chatcmpl-test", @@ -366,9 +340,7 @@ async def test_openai_chat_completion_non_streaming_without_store( router = InferenceRouter(routing_table=routing_table) assert router.store is None - mock_provider.openai_chat_completion = AsyncMock( - return_value=_make_chat_completion() - ) + mock_provider.openai_chat_completion = AsyncMock(return_value=_make_chat_completion()) params = OpenAIChatCompletionRequestWithExtraBody( model="test_provider/test-llm-model", @@ -381,9 +353,7 @@ async def test_openai_chat_completion_non_streaming_without_store( assert response.model == "test_provider/test-llm-model" assert response.choices[0].message.content == "Hello world" # Provider was called with its resource id, not the fully qualified id. - assert ( - mock_provider.openai_chat_completion.call_args.args[0].model == "test-llm-model" - ) + assert mock_provider.openai_chat_completion.call_args.args[0].model == "test-llm-model" async def test_openai_chat_completion_streaming_without_store(mock_llm_routing_table): @@ -416,9 +386,7 @@ async def provider_stream(): "test_provider/test-llm-model", "test_provider/test-llm-model", ] - assert [ - "".join(c.delta.content or "" for c in chunk.choices) for chunk in chunks - ] == ["Hello", " world"] + assert ["".join(c.delta.content or "" for c in chunk.choices) for chunk in chunks] == ["Hello", " world"] async def test_list_chat_completions_without_store_raises_not_implemented( @@ -442,9 +410,7 @@ async def test_get_chat_completion_without_store_raises_not_implemented( assert router.store is None with pytest.raises(NotImplementedError): - await router.get_chat_completion( - GetChatCompletionRequest(completion_id="chatcmpl-test") - ) + await router.get_chat_completion(GetChatCompletionRequest(completion_id="chatcmpl-test")) async def test_list_chat_completion_messages_without_store_raises_not_implemented( @@ -456,6 +422,4 @@ async def test_list_chat_completion_messages_without_store_raises_not_implemente assert router.store is None with pytest.raises(NotImplementedError): - await router.list_chat_completion_messages( - ListChatCompletionMessagesRequest(completion_id="chatcmpl-test") - ) + await router.list_chat_completion_messages(ListChatCompletionMessagesRequest(completion_id="chatcmpl-test")) diff --git a/tests/unit/core/test_storage_references.py b/tests/unit/core/test_storage_references.py index 1b07e895977..06cd80fc92a 100644 --- a/tests/unit/core/test_storage_references.py +++ b/tests/unit/core/test_storage_references.py @@ -66,24 +66,16 @@ def _base_run_config(**overrides): def test_references_require_known_backend(): with pytest.raises(ValidationError, match="unknown backend 'missing'"): - _base_run_config( - metadata_reference=KVStoreReference(backend="missing", namespace="registry") - ) + _base_run_config(metadata_reference=KVStoreReference(backend="missing", namespace="registry")) def test_references_must_match_backend_family(): with pytest.raises(ValidationError, match="kv_.* is required"): - _base_run_config( - metadata_reference=KVStoreReference( - backend="sql_default", namespace="registry" - ) - ) + _base_run_config(metadata_reference=KVStoreReference(backend="sql_default", namespace="registry")) with pytest.raises(ValidationError, match="sql_.* is required"): _base_run_config( - inference_reference=InferenceStoreReference( - backend="kv_default", table_name="inference" - ), + inference_reference=InferenceStoreReference(backend="kv_default", table_name="inference"), ) @@ -92,10 +84,7 @@ def test_valid_configuration_passes_validation(): stores = config.storage.stores assert stores.metadata is not None and stores.metadata.backend == "kv_default" assert stores.inference is not None and stores.inference.backend == "sql_default" - assert ( - stores.conversations is not None - and stores.conversations.backend == "sql_default" - ) + assert stores.conversations is not None and stores.conversations.backend == "sql_default" def test_inference_store_defaults_enabled_when_omitted(): @@ -144,9 +133,7 @@ def _default_stores_dict() -> dict[str, Any]: """A valid `ServerStoresConfig` serialized with only the non-inference keys.""" return ServerStoresConfig( metadata=KVStoreReference(backend="kv_default", namespace="registry"), - conversations=SqlStoreReference( - backend="sql_default", table_name="conversations" - ), + conversations=SqlStoreReference(backend="sql_default", table_name="conversations"), ).model_dump(mode="python") @@ -162,9 +149,7 @@ def test_default_backends_resolve_env_vars(backend_key, monkeypatch): monkeypatch.delenv("SQLITE_STORE_DIR", raising=False) config = StorageConfig() db_path = config.backends[backend_key].db_path - assert ( - "${env." not in db_path - ), f"Unresolved env var syntax in default {backend_key}: {db_path}" + assert "${env." not in db_path, f"Unresolved env var syntax in default {backend_key}: {db_path}" def test_default_backends_respect_sqlite_store_dir(monkeypatch): @@ -181,12 +166,8 @@ def test_default_backends_fallback_to_distribs_base_dir(monkeypatch): monkeypatch.delenv("SQLITE_STORE_DIR", raising=False) config = StorageConfig() - assert config.backends["kv_default"].db_path == os.path.join( - str(DISTRIBS_BASE_DIR), "kvstore.db" - ) - assert config.backends["sql_default"].db_path == os.path.join( - str(DISTRIBS_BASE_DIR), "sql_store.db" - ) + assert config.backends["kv_default"].db_path == os.path.join(str(DISTRIBS_BASE_DIR), "kvstore.db") + assert config.backends["sql_default"].db_path == os.path.join(str(DISTRIBS_BASE_DIR), "sql_store.db") def test_default_backends_expand_user_home(monkeypatch): diff --git a/tests/unit/server/test_resolver.py b/tests/unit/server/test_resolver.py index fe408648157..3f0fe5a8986 100644 --- a/tests/unit/server/test_resolver.py +++ b/tests/unit/server/test_resolver.py @@ -60,12 +60,7 @@ def sample_run_config(cls, **kwargs: Any) -> dict[str, Any]: class SampleImpl: - def __init__( - self, - config: SampleConfig, - deps: dict[Api, Any], - provider_spec: ProviderSpec = None, - ): + def __init__(self, config: SampleConfig, deps: dict[Api, Any], provider_spec: ProviderSpec = None): self.__provider_id__ = "test_provider" self.__provider_spec__ = provider_spec self.__provider_config__ = config @@ -86,28 +81,14 @@ def make_run_config(**overrides) -> StackConfig: }, stores=ServerStoresConfig( metadata=KVStoreReference(backend="kv_default", namespace="registry"), - inference=InferenceStoreReference( - backend="sql_default", table_name="inference_store" - ), - conversations=SqlStoreReference( - backend="sql_default", table_name="conversations" - ), + inference=InferenceStoreReference(backend="sql_default", table_name="inference_store"), + conversations=SqlStoreReference(backend="sql_default", table_name="conversations"), ), ), ) - register_kvstore_backends( - { - name: cfg - for name, cfg in storage.backends.items() - if cfg.type.value.startswith("kv_") - } - ) + register_kvstore_backends({name: cfg for name, cfg in storage.backends.items() if cfg.type.value.startswith("kv_")}) register_sqlstore_backends( - { - name: cfg - for name, cfg in storage.backends.items() - if cfg.type.value.startswith("sql_") - } + {name: cfg for name, cfg in storage.backends.items() if cfg.type.value.startswith("sql_")} ) defaults = dict( distro_name="test_image", @@ -208,9 +189,7 @@ async def test_resolve_impls_inference_without_store_skips_persistence(): stores=ServerStoresConfig( metadata=KVStoreReference(backend="kv_default", namespace="registry"), inference=None, - conversations=SqlStoreReference( - backend="sql_default", table_name="conversations" - ), + conversations=SqlStoreReference(backend="sql_default", table_name="conversations"), ), ), ) @@ -226,9 +205,7 @@ async def test_resolve_impls_inference_without_store_skips_persistence(): sys.modules["test_module"] = mock_module with patch("ogx.core.routers.InferenceStore") as mock_inference_store: - impls = await resolve_impls( - run_config, provider_registry, dist_registry, policy={} - ) + impls = await resolve_impls(run_config, provider_registry, dist_registry, policy={}) mock_inference_store.assert_not_called() From 9f779a491ba94cf90ca4f52ef2cdaba5fa954862 Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Mon, 10 Aug 2026 15:36:51 +0200 Subject: [PATCH 3/8] update readme --- src/ogx/core/storage/README.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/ogx/core/storage/README.md b/src/ogx/core/storage/README.md index 41c8e5b7d1c..00cdbd5fbbf 100644 --- a/src/ogx/core/storage/README.md +++ b/src/ogx/core/storage/README.md @@ -60,15 +60,22 @@ See `datatypes.py` for all config types and `StorageBackendType` for the enum of Some store references are optional: setting the reference to `null` (not omitting it) means OGX does not construct the store at all, and the API that depends on it degrades gracefully. This is an explicit operator choice made in a run config; -the default for every optional reference remains an enabled reference, so -existing deployments are unaffected unless they opt in. +the default for every optional reference remains an enabled reference (except +`responses`, which defaults to `None`), so existing deployments are unaffected +unless they opt in. - **`inference`** -- when `null`, no `InferenceStore` is constructed: no `inference_store` table is created and no background write workers run. Chat completions still work (streaming and non-streaming); the chat completion history endpoints (`list`, `retrieve`, `messages`) report that persistence is not configured (HTTP 501) rather than returning an empty list or a 404. -- **`responses`** -- the Responses store already follows the same pattern. +- **`responses`** -- the `responses` reference is nullable in the config schema + (`default=None`) and `null` passes validation, but unlike `inference` it is not + a runtime toggle: the built-in responses provider always constructs and + initializes its store from its own `persistence.responses` (a required, + non-nullable reference), and the shared `storage.stores.responses` reference + is only validated, never consumed at startup. Setting it to `null` is accepted + but does not currently disable Responses persistence. Other stores (`datasets`, `eval`, `files`, `prompts`, `vector_io`) are not affected by disabling the inference store, so persistence can be turned off for From 63fe27dd7a713d78970a702556fec99284901358 Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Tue, 11 Aug 2026 09:27:20 +0200 Subject: [PATCH 4/8] fix inference-store-disabled integration test to run in server mode ci --- ...gen_inference_store_disabled_recordings.py | 21 ++++---- .../inference/store_disabled_constants.py | 31 ++++++++++++ .../test_inference_store_disabled.py | 50 +++++++++++++------ 3 files changed, 75 insertions(+), 27 deletions(-) create mode 100644 tests/integration/inference/store_disabled_constants.py diff --git a/scripts/gen_inference_store_disabled_recordings.py b/scripts/gen_inference_store_disabled_recordings.py index 74af980be81..92e1502017b 100755 --- a/scripts/gen_inference_store_disabled_recordings.py +++ b/scripts/gen_inference_store_disabled_recordings.py @@ -34,25 +34,22 @@ REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, os.path.join(REPO_ROOT, "src")) +sys.path.insert(0, REPO_ROOT) from ogx.core.library_client import OGXAsLibraryClient # noqa: E402 from ogx.core.stack import get_stack_run_config_from_distro # noqa: E402 from ogx.core.testing_context import set_test_context # noqa: E402 +from tests.integration.inference.store_disabled_constants import ( # noqa: E402 + NON_STREAMING_PROMPT, + RECORDING_TEST_IDS, + STREAMING_PROMPT, + TEXT_MODEL, +) RECORDINGS_DIR = os.path.join(REPO_ROOT, "tests", "integration", "inference", "recordings") -# The exact prompts the test uses -- the recording hash depends on the body. -TEXT_MODEL = "openai/gpt-4o" -NON_STREAMING_PROMPT = "Say hello." -STREAMING_PROMPT = "Say hello in one sentence." - # pytest node ids the test will use -- the recording hash includes the test id. -TEST_NODE_IDS = [ - "tests/integration/inference/test_inference_store_disabled.py::test_non_streaming_chat_completion_without_store", - "tests/integration/inference/test_inference_store_disabled.py::test_streaming_chat_completion_without_store", - "tests/integration/inference/test_inference_store_disabled.py::test_retrieve_chat_completion_reports_not_configured", - "tests/integration/inference/test_inference_store_disabled.py::test_list_chat_completion_messages_reports_not_configured", -] +TEST_NODE_IDS = RECORDING_TEST_IDS MOCK_HOST = "127.0.0.1" MOCK_PORT = 0 # ephemeral @@ -141,7 +138,7 @@ def do_POST(self): # noqa: N802 Function name `do_POST` should be lowercase self.wfile.write(b"data: [DONE]\n\n") self.wfile.flush() else: - body = _completion_body(model, "Say hello.", completion_id) + body = _completion_body(model, NON_STREAMING_PROMPT, completion_id) data = json.dumps(body).encode() self.send_response(200) self.send_header("Content-Type", "application/json") diff --git a/tests/integration/inference/store_disabled_constants.py b/tests/integration/inference/store_disabled_constants.py new file mode 100644 index 00000000000..a39fb3cd24f --- /dev/null +++ b/tests/integration/inference/store_disabled_constants.py @@ -0,0 +1,31 @@ +# Copyright (c) The OGX Contributors. +# All rights reserved. +# +# This source code is licensed under the terms described in the LICENSE file in +# the root directory of this source tree. + +"""Shared constants for the inference-store-disabled tests and their recording generator. + +The recording harness keys recordings by a SHA256 hash of the request body and the +pytest node id, so the model id, prompts, and node ids MUST match between +``tests/integration/inference/test_inference_store_disabled.py`` and +``scripts/gen_inference_store_disabled_recordings.py``. Keeping them in one module +prevents silent drift between the test and its regenerable recordings. +""" + +TEXT_MODEL = "openai/gpt-4o" + +NON_STREAMING_PROMPT = "Say hello." +STREAMING_PROMPT = "Say hello in one sentence." + +# The full pytest node ids the generator records completions for. The test's +# list/retrieve/messages tests raise before any provider call, so only the four +# ids below perform chat-completion requests worth recording; the remaining tests +# need no recording. +TEST_MODULE = "tests/integration/inference/test_inference_store_disabled.py" +NON_STREAMING_TEST = f"{TEST_MODULE}::test_non_streaming_chat_completion_without_store" +STREAMING_TEST = f"{TEST_MODULE}::test_streaming_chat_completion_without_store" +RETRIEVE_TEST = f"{TEST_MODULE}::test_retrieve_chat_completion_reports_not_configured" +MESSAGES_TEST = f"{TEST_MODULE}::test_list_chat_completion_messages_reports_not_configured" + +RECORDING_TEST_IDS = [NON_STREAMING_TEST, STREAMING_TEST, RETRIEVE_TEST, MESSAGES_TEST] diff --git a/tests/integration/inference/test_inference_store_disabled.py b/tests/integration/inference/test_inference_store_disabled.py index 4b7a9e32f4b..869d8e50ef6 100644 --- a/tests/integration/inference/test_inference_store_disabled.py +++ b/tests/integration/inference/test_inference_store_disabled.py @@ -28,14 +28,11 @@ from ogx.core.datatypes import StackConfig from ogx.core.library_client import OGXAsLibraryClient from ogx.core.stack import get_stack_run_config_from_distro - -# A model reachable through the ci-tests routing via the provider fallback path -# (``provider_id/resource_id``). The completion request against the provider is -# replayed from a recording, so no live API key is needed. -TEXT_MODEL = "openai/gpt-4o" - -NON_STREAMING_PROMPT = "Say hello." -STREAMING_PROMPT = "Say hello in one sentence." +from tests.integration.inference.store_disabled_constants import ( + NON_STREAMING_PROMPT, + STREAMING_PROMPT, + TEXT_MODEL, +) def _build_non_persisting_config(sqlite_dir: str) -> tuple[StackConfig, Path]: @@ -57,6 +54,12 @@ def non_persisting_client(): Booted once for the whole session because constructing the ci-tests stack is expensive. These tests do not mutate shared client state, so a single shared client is safe. + + This deliberately does not use the shared ``ogx_client`` fixture: unlike that + fixture it must boot its own stack with a custom store config + (``inference: null``), which cannot be expressed through the standard + server-mode run config. See ``_boot_library_client`` for how the in-process + boot avoids colliding with the outer server-mode OGX server. """ # The OpenAI provider validates that an API key is present before the # recording harness short-circuits the request in replay mode. A fake key @@ -68,7 +71,7 @@ def non_persisting_client(): config_file = tempfile.NamedTemporaryFile(delete=False, suffix="-run.yaml").name with open(config_file, "w") as f: yaml.dump(run_config.model_dump(mode="json"), f) - client = OGXAsLibraryClient(config_file, skip_logger_removal=True) + client = _boot_library_client(config_file) try: yield client, sql_db finally: @@ -76,6 +79,27 @@ def non_persisting_client(): os.unlink(config_file) +def _boot_library_client(config_file: str) -> OGXAsLibraryClient: + """Boot an in-process library client with the standalone metrics endpoint off. + + ``integration-tests.sh`` exports ``OGX_METRICS_ENDPOINT_ENABLED=1`` so the outer + server-mode OGX server exposes a metrics scrape endpoint on port 9464. Booting a + second stack in-process would try to bind the same port; nothing scrapes the + in-process stack, so the endpoint is turned off for the boot and restored right + after so sibling tests in the same pytest process (e.g. the metrics endpoint + integration test) still observe the script's flag. + """ + metrics_env = os.environ.get("OGX_METRICS_ENDPOINT_ENABLED") + os.environ["OGX_METRICS_ENDPOINT_ENABLED"] = "0" + try: + return OGXAsLibraryClient(config_file, skip_logger_removal=True) + finally: + if metrics_env is not None: + os.environ["OGX_METRICS_ENDPOINT_ENABLED"] = metrics_env + else: + os.environ.pop("OGX_METRICS_ENDPOINT_ENABLED", None) + + def test_non_streaming_chat_completion_without_store(non_persisting_client): """A non-streaming completion is returned normally with id/model populated.""" client, _ = non_persisting_client @@ -149,11 +173,7 @@ def test_no_inference_store_table_when_persistence_disabled(non_persisting_clien conn = sqlite3.connect(str(sql_db)) try: - rows = conn.execute( - "SELECT name FROM sqlite_master WHERE type='table' AND name='inference_store'" - ).fetchall() + rows = conn.execute("SELECT name FROM sqlite_master WHERE type='table' AND name='inference_store'").fetchall() finally: conn.close() - assert ( - rows == [] - ), "inference_store table must not exist when persistence is disabled" + assert rows == [], "inference_store table must not exist when persistence is disabled" From 775dd4779b932e0fa1b5e063134e260e94292d3c Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Tue, 11 Aug 2026 09:56:43 +0200 Subject: [PATCH 5/8] improve disabled-store test reliability. Signed-off-by: Matt, Matthias --- ...gen_inference_store_disabled_recordings.py | 160 ++++++++++-------- src/ogx/core/routers/__init__.py | 2 +- ...constants.py => store_disabled_support.py} | 15 +- .../test_inference_store_disabled.py | 77 +++++---- .../core/routers/test_inference_router.py | 23 +-- tests/unit/core/test_storage_references.py | 6 +- tests/unit/server/test_resolver.py | 2 +- 7 files changed, 156 insertions(+), 129 deletions(-) rename tests/integration/inference/{store_disabled_constants.py => store_disabled_support.py} (69%) diff --git a/scripts/gen_inference_store_disabled_recordings.py b/scripts/gen_inference_store_disabled_recordings.py index 92e1502017b..7a834c6c0fb 100755 --- a/scripts/gen_inference_store_disabled_recordings.py +++ b/scripts/gen_inference_store_disabled_recordings.py @@ -28,7 +28,10 @@ import sys import tempfile import threading +from collections.abc import Iterator +from contextlib import contextmanager from http.server import BaseHTTPRequestHandler, HTTPServer +from typing import Any import yaml @@ -36,14 +39,14 @@ sys.path.insert(0, os.path.join(REPO_ROOT, "src")) sys.path.insert(0, REPO_ROOT) -from ogx.core.library_client import OGXAsLibraryClient # noqa: E402 -from ogx.core.stack import get_stack_run_config_from_distro # noqa: E402 -from ogx.core.testing_context import set_test_context # noqa: E402 -from tests.integration.inference.store_disabled_constants import ( # noqa: E402 +from ogx.core.library_client import OGXAsLibraryClient # noqa: E402 # Requires local checkout path setup above. +from ogx.core.testing_context import set_test_context # noqa: E402 # Requires local checkout path setup above. +from tests.integration.inference.store_disabled_support import ( # noqa: E402 # Requires path setup above. NON_STREAMING_PROMPT, RECORDING_TEST_IDS, STREAMING_PROMPT, TEXT_MODEL, + build_inference_store_disabled_run_config, ) RECORDINGS_DIR = os.path.join(REPO_ROOT, "tests", "integration", "inference", "recordings") @@ -55,7 +58,7 @@ MOCK_PORT = 0 # ephemeral -def _completion_body(model: str, prompt: str, completion_id: str) -> dict: +def _completion_body(model: str, prompt: str, completion_id: str) -> dict[str, Any]: return { "id": completion_id, "object": "chat.completion", @@ -72,7 +75,7 @@ def _completion_body(model: str, prompt: str, completion_id: str) -> dict: } -def _stream_chunks(model: str, completion_id: str): +def _stream_chunks(model: str, completion_id: str) -> list[dict[str, Any]]: return [ { "id": completion_id, @@ -105,7 +108,7 @@ def _stream_chunks(model: str, completion_id: str): class MockHandler(BaseHTTPRequestHandler): - def do_GET(self): # noqa: N802 Function name `do_GET` should be lowercase + def do_GET(self) -> None: # noqa: N802 Function name `do_GET` should be lowercase if self.path.endswith("/models"): body = json.dumps({"object": "list", "data": [{"id": "gpt-4o", "object": "model"}]}).encode() self.send_response(200) @@ -117,13 +120,14 @@ def do_GET(self): # noqa: N802 Function name `do_GET` should be lowercase self.send_response(404) self.end_headers() - def do_POST(self): # noqa: N802 Function name `do_POST` should be lowercase + def do_POST(self) -> None: # noqa: N802 Function name `do_POST` should be lowercase length = int(self.headers.get("Content-Length", "0")) raw = self.rfile.read(length) if length else b"" try: - payload = json.loads(raw) if raw else {} - except Exception: - payload = {} + decoded_payload = json.loads(raw) if raw else {} + except (json.JSONDecodeError, UnicodeDecodeError): + decoded_payload = {} + payload = decoded_payload if isinstance(decoded_payload, dict) else {} stream = payload.get("stream", False) model = payload.get("model", "gpt-4o") completion_id = "chatcmpl-mock-recording" @@ -132,8 +136,8 @@ def do_POST(self): # noqa: N802 Function name `do_POST` should be lowercase self.send_response(200) self.send_header("Content-Type", "text/event-stream") self.end_headers() - for ch in chunks: - self.wfile.write(f"data: {json.dumps(ch)}\n\n".encode()) + for chunk in chunks: + self.wfile.write(f"data: {json.dumps(chunk)}\n\n".encode()) self.wfile.flush() self.wfile.write(b"data: [DONE]\n\n") self.wfile.flush() @@ -146,18 +150,24 @@ def do_POST(self): # noqa: N802 Function name `do_POST` should be lowercase self.end_headers() self.wfile.write(data) - def log_message(self, *args): # silence + def log_message(self, format: str, *args: Any) -> None: # silence pass -def start_mock_server() -> HTTPServer: +@contextmanager +def _mock_server() -> Iterator[HTTPServer]: server = HTTPServer((MOCK_HOST, MOCK_PORT), MockHandler) thread = threading.Thread(target=server.serve_forever, daemon=True) thread.start() - return server + try: + yield server + finally: + server.shutdown() + server.server_close() + thread.join() -async def run(test_id: str, config_path: str) -> None: +async def _run(test_id: str, config_path: str) -> None: set_test_context(test_id) client = OGXAsLibraryClient(config_path, skip_logger_removal=True) try: @@ -177,70 +187,72 @@ async def run(test_id: str, config_path: str) -> None: client.shutdown() -def main() -> None: - os.environ["OGX_TEST_INFERENCE_MODE"] = "record" - os.environ["OGX_LOGGING"] = "all=warning" - os.environ["OPENAI_API_KEY"] = "fake-key-for-replay" - sqlite_dir = tempfile.mkdtemp(prefix="ogx-record-") - os.environ["SQLITE_STORE_DIR"] = sqlite_dir - - server = start_mock_server() - port = server.server_address[1] - mock_base = f"http://{MOCK_HOST}:{port}/v1" - os.environ["OPENAI_BASE_URL"] = mock_base - - run_config = get_stack_run_config_from_distro("ci-tests") - run_config.storage.stores.inference = None - run_config.vector_stores = None - - config_file = os.path.join(tempfile.mkdtemp(), "run.yaml") - with open(config_file, "w") as f: - yaml.dump(run_config.model_dump(mode="json"), f) - - # Isolate recording so the shared inference recordings directory is untouched. - # The recorder always writes into the test file's ``recordings/`` dir (relative - # to CWD) when a test context is set, so move the real directory aside while - # recording and merge only the chat-completion recordings back afterwards. - backup = None - if os.path.isdir(RECORDINGS_DIR): - backup = RECORDINGS_DIR + ".bak" - shutil.move(RECORDINGS_DIR, backup) - - try: - for test_id in TEST_NODE_IDS: - print(f"recording for {test_id} ...") - asyncio.run(run(test_id, config_file)) - finally: - server.shutdown() - - # Collect the freshly recorded chat-completion recordings (skip models-list - # recordings -- the test does not need them in replay mode) and rewrite their - # mock-host URLs to the canonical provider URL. The recording hash ignores - # the host, so replay works against the real provider URL. - staged = tempfile.mkdtemp(prefix="ogx-staged-") +def _stage_recordings(staged_dir: str) -> int: + """Copy generated chat recordings to staging and normalize their provider URLs.""" for name in os.listdir(RECORDINGS_DIR): if not name.endswith(".json") or name.startswith("models-"): continue - src = os.path.join(RECORDINGS_DIR, name) - with open(src) as f: - data = json.load(f) + source = os.path.join(RECORDINGS_DIR, name) + with open(source, encoding="utf-8") as file: + data = json.load(file) url = data.get("request", {}).get("url", "") if re.search(r"\d+\.\d+\.\d+\.\d+:\d+", url): data["request"]["url"] = "https://api.openai.com/v1" + url.split("/v1", 1)[1] - with open(os.path.join(staged, name), "w") as f: - json.dump(data, f, indent=2) - f.write("\n") - - # Restore the original recordings directory and drop in the new recordings. - shutil.rmtree(RECORDINGS_DIR, ignore_errors=True) - if backup is not None: - shutil.move(backup, RECORDINGS_DIR) - else: + with open(os.path.join(staged_dir, name), "w", encoding="utf-8") as file: + json.dump(data, file, indent=2) + file.write("\n") + return len(os.listdir(staged_dir)) + + +def _generate_recordings(config_file: str) -> int: + """Generate recordings while preserving the repository's existing fixtures.""" + recordings_parent = os.path.dirname(RECORDINGS_DIR) + with ( + tempfile.TemporaryDirectory(prefix=".recordings-backup-", dir=recordings_parent) as backup_dir, + tempfile.TemporaryDirectory(prefix="ogx-staged-") as staged_dir, + ): + original_recordings = os.path.join(backup_dir, "recordings") + had_original_recordings = os.path.isdir(RECORDINGS_DIR) + if had_original_recordings: + shutil.move(RECORDINGS_DIR, original_recordings) + + try: + for test_id in TEST_NODE_IDS: + print(f"recording for {test_id} ...") + asyncio.run(_run(test_id, config_file)) + n_written = _stage_recordings(staged_dir) + finally: + shutil.rmtree(RECORDINGS_DIR, ignore_errors=True) + if had_original_recordings: + shutil.move(original_recordings, RECORDINGS_DIR) + os.makedirs(RECORDINGS_DIR, exist_ok=True) - n_written = len(os.listdir(staged)) - for name in os.listdir(staged): - shutil.copy2(os.path.join(staged, name), os.path.join(RECORDINGS_DIR, name)) - shutil.rmtree(staged, ignore_errors=True) + for name in os.listdir(staged_dir): + shutil.copy2(os.path.join(staged_dir, name), os.path.join(RECORDINGS_DIR, name)) + return n_written + + +def main() -> None: + os.environ["OGX_TEST_INFERENCE_MODE"] = "record" + os.environ["OGX_LOGGING"] = "all=warning" + os.environ["OPENAI_API_KEY"] = "fake-key-for-replay" + + with ( + tempfile.TemporaryDirectory(prefix="ogx-record-") as sqlite_dir, + tempfile.TemporaryDirectory(prefix="ogx-config-") as config_dir, + _mock_server() as server, + ): + os.environ["SQLITE_STORE_DIR"] = sqlite_dir + port = server.server_address[1] + os.environ["OPENAI_BASE_URL"] = f"http://{MOCK_HOST}:{port}/v1" + + run_config = build_inference_store_disabled_run_config() + config_file = os.path.join(config_dir, "run.yaml") + with open(config_file, "w", encoding="utf-8") as file: + yaml.safe_dump(run_config.model_dump(mode="json"), file) + + n_written = _generate_recordings(config_file) + print(f"wrote {n_written} recordings") print("done") diff --git a/src/ogx/core/routers/__init__.py b/src/ogx/core/routers/__init__.py index 6b48b52956e..0e13604b899 100644 --- a/src/ogx/core/routers/__init__.py +++ b/src/ogx/core/routers/__init__.py @@ -66,7 +66,7 @@ async def get_auto_router_impl( # missing store (it guards every write and raises NotImplementedError # on the history endpoints), mirroring the optional Responses store. inference_ref = run_config.storage.stores.inference - if inference_ref: + if inference_ref is not None: inference_store = InferenceStore( reference=inference_ref, policy=policy, diff --git a/tests/integration/inference/store_disabled_constants.py b/tests/integration/inference/store_disabled_support.py similarity index 69% rename from tests/integration/inference/store_disabled_constants.py rename to tests/integration/inference/store_disabled_support.py index a39fb3cd24f..cd269007d78 100644 --- a/tests/integration/inference/store_disabled_constants.py +++ b/tests/integration/inference/store_disabled_support.py @@ -4,7 +4,7 @@ # This source code is licensed under the terms described in the LICENSE file in # the root directory of this source tree. -"""Shared constants for the inference-store-disabled tests and their recording generator. +"""Shared support for the inference-store-disabled tests and recording generator. The recording harness keys recordings by a SHA256 hash of the request body and the pytest node id, so the model id, prompts, and node ids MUST match between @@ -13,6 +13,9 @@ prevents silent drift between the test and its regenerable recordings. """ +from ogx.core.datatypes import StackConfig +from ogx.core.stack import get_stack_run_config_from_distro + TEXT_MODEL = "openai/gpt-4o" NON_STREAMING_PROMPT = "Say hello." @@ -29,3 +32,13 @@ MESSAGES_TEST = f"{TEST_MODULE}::test_list_chat_completion_messages_reports_not_configured" RECORDING_TEST_IDS = [NON_STREAMING_TEST, STREAMING_TEST, RETRIEVE_TEST, MESSAGES_TEST] + + +def build_inference_store_disabled_run_config() -> StackConfig: + """Build the minimal ci-tests configuration used by this test scenario.""" + run_config = get_stack_run_config_from_distro("ci-tests") + run_config.storage.stores.inference = None + # Vector-store model validation is unrelated to chat completion persistence + # and loads the sentence-transformers stack during an in-process boot. + run_config.vector_stores = None + return run_config diff --git a/tests/integration/inference/test_inference_store_disabled.py b/tests/integration/inference/test_inference_store_disabled.py index 869d8e50ef6..004694c0753 100644 --- a/tests/integration/inference/test_inference_store_disabled.py +++ b/tests/integration/inference/test_inference_store_disabled.py @@ -19,36 +19,27 @@ """ import os +import sqlite3 import tempfile +from collections.abc import Generator from pathlib import Path import pytest import yaml -from ogx.core.datatypes import StackConfig from ogx.core.library_client import OGXAsLibraryClient -from ogx.core.stack import get_stack_run_config_from_distro -from tests.integration.inference.store_disabled_constants import ( +from tests.integration.inference.store_disabled_support import ( NON_STREAMING_PROMPT, STREAMING_PROMPT, TEXT_MODEL, + build_inference_store_disabled_run_config, ) - -def _build_non_persisting_config(sqlite_dir: str) -> tuple[StackConfig, Path]: - run_config = get_stack_run_config_from_distro("ci-tests") - # Disable chat completion persistence: no store is constructed, no table is - # created, and no background write workers are started. - run_config.storage.stores.inference = None - # Vector-store config pulls in embedding/reranker model validation that is - # unrelated to chat completion persistence and makes every in-process boot - # load the sentence-transformers stack; drop it so booting is deterministic. - run_config.vector_stores = None - return run_config, Path(sqlite_dir) / "sql_store.db" +NonPersistingClient = tuple[OGXAsLibraryClient, Path] @pytest.fixture(scope="session") -def non_persisting_client(): +def non_persisting_client() -> Generator[NonPersistingClient, None, None]: """Boot an in-process library client with chat completion persistence disabled. Booted once for the whole session because constructing the ci-tests stack is @@ -61,22 +52,29 @@ def non_persisting_client(): server-mode run config. See ``_boot_library_client`` for how the in-process boot avoids colliding with the outer server-mode OGX server. """ - # The OpenAI provider validates that an API key is present before the - # recording harness short-circuits the request in replay mode. A fake key - # is sufficient because the request is never sent to the provider. - os.environ["OPENAI_API_KEY"] = "fake-key-for-replay" - sqlite_dir = tempfile.mkdtemp(prefix="ogx-no-store-") - os.environ["SQLITE_STORE_DIR"] = sqlite_dir - run_config, sql_db = _build_non_persisting_config(sqlite_dir) - config_file = tempfile.NamedTemporaryFile(delete=False, suffix="-run.yaml").name - with open(config_file, "w") as f: - yaml.dump(run_config.model_dump(mode="json"), f) - client = _boot_library_client(config_file) + environment = pytest.MonkeyPatch() try: - yield client, sql_db + with tempfile.TemporaryDirectory(prefix="ogx-no-store-") as temp_dir: + sqlite_dir = Path(temp_dir) / "sqlite" + sqlite_dir.mkdir() + # Provider validation runs before replay intercepts the request, so + # the in-process stack needs a placeholder key. + environment.setenv("OPENAI_API_KEY", "fake-key-for-replay") + environment.setenv("SQLITE_STORE_DIR", str(sqlite_dir)) + + run_config = build_inference_store_disabled_run_config() + sql_db = sqlite_dir / "sql_store.db" + config_file = Path(temp_dir) / "run.yaml" + with config_file.open("w", encoding="utf-8") as file: + yaml.safe_dump(run_config.model_dump(mode="json"), file) + + client = _boot_library_client(str(config_file)) + try: + yield client, sql_db + finally: + client.shutdown() finally: - client.shutdown() - os.unlink(config_file) + environment.undo() def _boot_library_client(config_file: str) -> OGXAsLibraryClient: @@ -100,7 +98,7 @@ def _boot_library_client(config_file: str) -> OGXAsLibraryClient: os.environ.pop("OGX_METRICS_ENDPOINT_ENABLED", None) -def test_non_streaming_chat_completion_without_store(non_persisting_client): +def test_non_streaming_chat_completion_without_store(non_persisting_client: NonPersistingClient) -> None: """A non-streaming completion is returned normally with id/model populated.""" client, _ = non_persisting_client response = client.chat.completions.create( @@ -113,7 +111,7 @@ def test_non_streaming_chat_completion_without_store(non_persisting_client): assert response.choices[0].message.content -def test_streaming_chat_completion_without_store(non_persisting_client): +def test_streaming_chat_completion_without_store(non_persisting_client: NonPersistingClient) -> None: """A streaming completion streams normally with the requested model id.""" client, _ = non_persisting_client stream = client.chat.completions.create( @@ -131,7 +129,7 @@ def test_streaming_chat_completion_without_store(non_persisting_client): assert response_id -def test_list_chat_completions_reports_not_configured(non_persisting_client): +def test_list_chat_completions_reports_not_configured(non_persisting_client: NonPersistingClient) -> None: """list raises a not-configured error rather than returning an empty list. In library-client mode the router's ``NotImplementedError`` propagates @@ -142,7 +140,7 @@ def test_list_chat_completions_reports_not_configured(non_persisting_client): client.chat.completions.list(limit=10) -def test_retrieve_chat_completion_reports_not_configured(non_persisting_client): +def test_retrieve_chat_completion_reports_not_configured(non_persisting_client: NonPersistingClient) -> None: """retrieve for a just-completed id raises a not-configured error rather than a 404.""" client, _ = non_persisting_client response = client.chat.completions.create( @@ -153,7 +151,9 @@ def test_retrieve_chat_completion_reports_not_configured(non_persisting_client): client.chat.completions.retrieve(response.id) -def test_list_chat_completion_messages_reports_not_configured(non_persisting_client): +def test_list_chat_completion_messages_reports_not_configured( + non_persisting_client: NonPersistingClient, +) -> None: """messages raises a not-configured error, consistent with list/retrieve.""" client, _ = non_persisting_client response = client.chat.completions.create( @@ -164,13 +164,12 @@ def test_list_chat_completion_messages_reports_not_configured(non_persisting_cli client.chat.completions.messages.list(completion_id=response.id) -def test_no_inference_store_table_when_persistence_disabled(non_persisting_client): +def test_no_inference_store_table_when_persistence_disabled( + non_persisting_client: NonPersistingClient, +) -> None: """No ``inference_store`` table exists in the SQL backend, proving payloads were never written.""" _client, sql_db = non_persisting_client - if not Path(sql_db).exists(): - pytest.skip(f"SQL backend db not found at {sql_db}") - import sqlite3 - + assert sql_db.exists(), "SQL backend DB must exist because the other stores remain enabled" conn = sqlite3.connect(str(sql_db)) try: rows = conn.execute("SELECT name FROM sqlite_master WHERE type='table' AND name='inference_store'").fetchall() diff --git a/tests/unit/core/routers/test_inference_router.py b/tests/unit/core/routers/test_inference_router.py index 921b1fd0295..a8ac6e0883b 100644 --- a/tests/unit/core/routers/test_inference_router.py +++ b/tests/unit/core/routers/test_inference_router.py @@ -16,6 +16,7 @@ - test_rerank_calls_provider_correctly: Validates the router calls provider.rerank() with correct RerankRequest """ +from collections.abc import AsyncIterator from unittest.mock import AsyncMock, MagicMock import pytest @@ -329,8 +330,8 @@ def _make_chat_completion_chunk(text: str, model: str = "test-llm-model") -> Ope async def test_openai_chat_completion_non_streaming_without_store( - mock_llm_routing_table, -): + mock_llm_routing_table: tuple[MagicMock, MagicMock], +) -> None: """A non-streaming chat completion succeeds when persistence is disabled (no store). The completion is returned to the caller with the requested model id and the @@ -356,7 +357,9 @@ async def test_openai_chat_completion_non_streaming_without_store( assert mock_provider.openai_chat_completion.call_args.args[0].model == "test-llm-model" -async def test_openai_chat_completion_streaming_without_store(mock_llm_routing_table): +async def test_openai_chat_completion_streaming_without_store( + mock_llm_routing_table: tuple[MagicMock, MagicMock], +) -> None: """A streaming chat completion streams normally when persistence is disabled. Chunks are rewritten to carry the requested model id and the router never @@ -366,7 +369,7 @@ async def test_openai_chat_completion_streaming_without_store(mock_llm_routing_t router = InferenceRouter(routing_table=routing_table) assert router.store is None - async def provider_stream(): + async def provider_stream() -> AsyncIterator[OpenAIChatCompletionChunk]: yield _make_chat_completion_chunk("Hello") yield _make_chat_completion_chunk(" world") @@ -390,8 +393,8 @@ async def provider_stream(): async def test_list_chat_completions_without_store_raises_not_implemented( - mock_llm_routing_table, -): + mock_llm_routing_table: tuple[MagicMock, MagicMock], +) -> None: """The list history endpoint reports an error (not an empty list) when persistence is off.""" routing_table, _ = mock_llm_routing_table router = InferenceRouter(routing_table=routing_table) @@ -402,8 +405,8 @@ async def test_list_chat_completions_without_store_raises_not_implemented( async def test_get_chat_completion_without_store_raises_not_implemented( - mock_llm_routing_table, -): + mock_llm_routing_table: tuple[MagicMock, MagicMock], +) -> None: """The retrieve history endpoint reports an error (not a 404) when persistence is off.""" routing_table, _ = mock_llm_routing_table router = InferenceRouter(routing_table=routing_table) @@ -414,8 +417,8 @@ async def test_get_chat_completion_without_store_raises_not_implemented( async def test_list_chat_completion_messages_without_store_raises_not_implemented( - mock_llm_routing_table, -): + mock_llm_routing_table: tuple[MagicMock, MagicMock], +) -> None: """The messages history endpoint reports an error when persistence is off, consistent with list/retrieve.""" routing_table, _ = mock_llm_routing_table router = InferenceRouter(routing_table=routing_table) diff --git a/tests/unit/core/test_storage_references.py b/tests/unit/core/test_storage_references.py index 06cd80fc92a..910a1916e88 100644 --- a/tests/unit/core/test_storage_references.py +++ b/tests/unit/core/test_storage_references.py @@ -87,7 +87,7 @@ def test_valid_configuration_passes_validation(): assert stores.conversations is not None and stores.conversations.backend == "sql_default" -def test_inference_store_defaults_enabled_when_omitted(): +def test_inference_store_defaults_enabled_when_omitted() -> None: """Omitting the `inference` key keeps persistence enabled (backward compatible default). The optional typing on `ServerStoresConfig.inference` means `None` is a valid, @@ -102,13 +102,13 @@ def test_inference_store_defaults_enabled_when_omitted(): assert stores.inference.table_name == "inference_store" -def test_inference_store_none_disables_persistence(): +def test_inference_store_none_disables_persistence() -> None: """Setting `inference` to null explicitly opts out of chat completion persistence.""" stores = ServerStoresConfig(inference=None) assert stores.inference is None -def test_inference_store_config_omit_vs_null_parsing(): +def test_inference_store_config_omit_vs_null_parsing() -> None: """Lock the omit-vs-null semantics at the StackConfig parsing level. - `inference` key absent -> enabled default (persistence on) diff --git a/tests/unit/server/test_resolver.py b/tests/unit/server/test_resolver.py index 3f0fe5a8986..5a3abd6aaa8 100644 --- a/tests/unit/server/test_resolver.py +++ b/tests/unit/server/test_resolver.py @@ -151,7 +151,7 @@ async def test_resolve_impls_basic(): assert impl.__provider_spec__ == provider_spec -async def test_resolve_impls_inference_without_store_skips_persistence(): +async def test_resolve_impls_inference_without_store_skips_persistence() -> None: """An absent inference store reference must not construct an InferenceStore. When `storage.stores.inference` is None, the auto-router factory skips From 9bfcdfb949bfdd23aac7d796413e8b6f1a606f81 Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Tue, 11 Aug 2026 15:29:29 +0200 Subject: [PATCH 6/8] review changes --- scripts/README.md | 4 - ...gen_inference_store_disabled_recordings.py | 261 ------------------ src/ogx/core/routers/__init__.py | 5 - src/ogx/core/storage/README.md | 38 +-- src/ogx/distributions/README.md | 3 +- ...17deeb6e07aa051ef058f909a676dfb5d67ec.json | 61 ---- ...d452008645fa0abee31912ff10c86a92340db.json | 61 ---- .../inference/store_disabled_support.py | 21 +- .../test_inference_store_disabled.py | 18 +- tests/unit/core/test_storage_references.py | 13 +- 10 files changed, 22 insertions(+), 463 deletions(-) delete mode 100755 scripts/gen_inference_store_disabled_recordings.py delete mode 100644 tests/integration/inference/recordings/3765a840ae86565c72696d32a5617deeb6e07aa051ef058f909a676dfb5d67ec.json delete mode 100644 tests/integration/inference/recordings/7c573c978200175222e201e0139d452008645fa0abee31912ff10c86a92340db.json diff --git a/scripts/README.md b/scripts/README.md index 98255a68ee9..1662e519e63 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -22,7 +22,6 @@ scripts/ cleanup_recordings.py # Remove orphaned test recordings diagnose_recordings.py # Debug test recording issues normalize_recordings.py # Normalize test recordings for consistency - gen_inference_store_disabled_recordings.py # Regenerate recordings for the inference-store-disabled integration test docker.sh # Docker build helper install.sh # Installation helper integration-tests.sh # Run integration test suite @@ -67,9 +66,6 @@ uv run python scripts/cleanup_recordings.py uv run python scripts/diagnose_recordings.py # Normalize recordings uv run python scripts/normalize_recordings.py -# Regenerate recordings for the inference-store-disabled integration test -# (records against a local mock OpenAI server; isolates the shared recordings dir) -uv run python scripts/gen_inference_store_disabled_recordings.py ``` ### Remote test recording (via GitHub Actions) diff --git a/scripts/gen_inference_store_disabled_recordings.py b/scripts/gen_inference_store_disabled_recordings.py deleted file mode 100755 index 7a834c6c0fb..00000000000 --- a/scripts/gen_inference_store_disabled_recordings.py +++ /dev/null @@ -1,261 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) The OGX Contributors. -# All rights reserved. -# -# This source code is licensed under the terms described in the LICENSE file in -# the root directory of this source tree. - -"""Generate recordings for test_inference_store_disabled.py against a local mock OpenAI server. - -Run from repo root: - uv run python scripts/gen_inference_store_disabled_recordings.py - -This spins up a tiny OpenAI-compatible HTTP mock, boots the ci-tests stack with -the inference store disabled and openai pointed at the mock, and exercises the -same chat-completion calls the test makes -- in RECORD mode -- so the recording -harness captures them into tests/integration/inference/recordings/. - -A mock is used instead of a live provider because the test only needs the -id/model populated and asserts nothing about real completion content; this -keeps the recordings self-contained and regenerable offline. -""" - -import asyncio -import json -import os -import re -import shutil -import sys -import tempfile -import threading -from collections.abc import Iterator -from contextlib import contextmanager -from http.server import BaseHTTPRequestHandler, HTTPServer -from typing import Any - -import yaml - -REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) -sys.path.insert(0, os.path.join(REPO_ROOT, "src")) -sys.path.insert(0, REPO_ROOT) - -from ogx.core.library_client import OGXAsLibraryClient # noqa: E402 # Requires local checkout path setup above. -from ogx.core.testing_context import set_test_context # noqa: E402 # Requires local checkout path setup above. -from tests.integration.inference.store_disabled_support import ( # noqa: E402 # Requires path setup above. - NON_STREAMING_PROMPT, - RECORDING_TEST_IDS, - STREAMING_PROMPT, - TEXT_MODEL, - build_inference_store_disabled_run_config, -) - -RECORDINGS_DIR = os.path.join(REPO_ROOT, "tests", "integration", "inference", "recordings") - -# pytest node ids the test will use -- the recording hash includes the test id. -TEST_NODE_IDS = RECORDING_TEST_IDS - -MOCK_HOST = "127.0.0.1" -MOCK_PORT = 0 # ephemeral - - -def _completion_body(model: str, prompt: str, completion_id: str) -> dict[str, Any]: - return { - "id": completion_id, - "object": "chat.completion", - "created": 0, - "model": model, - "choices": [ - { - "index": 0, - "message": {"role": "assistant", "content": f"Response to: {prompt}"}, - "finish_reason": "stop", - } - ], - "usage": {"prompt_tokens": 5, "completion_tokens": 5, "total_tokens": 10}, - } - - -def _stream_chunks(model: str, completion_id: str) -> list[dict[str, Any]]: - return [ - { - "id": completion_id, - "object": "chat.completion.chunk", - "created": 0, - "model": model, - "choices": [ - { - "index": 0, - "delta": {"role": "assistant", "content": "Hello"}, - "finish_reason": None, - } - ], - }, - { - "id": completion_id, - "object": "chat.completion.chunk", - "created": 0, - "model": model, - "choices": [{"index": 0, "delta": {"content": " world."}, "finish_reason": None}], - }, - { - "id": completion_id, - "object": "chat.completion.chunk", - "created": 0, - "model": model, - "choices": [{"index": 0, "delta": {}, "finish_reason": "stop"}], - }, - ] - - -class MockHandler(BaseHTTPRequestHandler): - def do_GET(self) -> None: # noqa: N802 Function name `do_GET` should be lowercase - if self.path.endswith("/models"): - body = json.dumps({"object": "list", "data": [{"id": "gpt-4o", "object": "model"}]}).encode() - self.send_response(200) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(body))) - self.end_headers() - self.wfile.write(body) - else: - self.send_response(404) - self.end_headers() - - def do_POST(self) -> None: # noqa: N802 Function name `do_POST` should be lowercase - length = int(self.headers.get("Content-Length", "0")) - raw = self.rfile.read(length) if length else b"" - try: - decoded_payload = json.loads(raw) if raw else {} - except (json.JSONDecodeError, UnicodeDecodeError): - decoded_payload = {} - payload = decoded_payload if isinstance(decoded_payload, dict) else {} - stream = payload.get("stream", False) - model = payload.get("model", "gpt-4o") - completion_id = "chatcmpl-mock-recording" - if stream: - chunks = _stream_chunks(model, completion_id) - self.send_response(200) - self.send_header("Content-Type", "text/event-stream") - self.end_headers() - for chunk in chunks: - self.wfile.write(f"data: {json.dumps(chunk)}\n\n".encode()) - self.wfile.flush() - self.wfile.write(b"data: [DONE]\n\n") - self.wfile.flush() - else: - body = _completion_body(model, NON_STREAMING_PROMPT, completion_id) - data = json.dumps(body).encode() - self.send_response(200) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(data))) - self.end_headers() - self.wfile.write(data) - - def log_message(self, format: str, *args: Any) -> None: # silence - pass - - -@contextmanager -def _mock_server() -> Iterator[HTTPServer]: - server = HTTPServer((MOCK_HOST, MOCK_PORT), MockHandler) - thread = threading.Thread(target=server.serve_forever, daemon=True) - thread.start() - try: - yield server - finally: - server.shutdown() - server.server_close() - thread.join() - - -async def _run(test_id: str, config_path: str) -> None: - set_test_context(test_id) - client = OGXAsLibraryClient(config_path, skip_logger_removal=True) - try: - if test_id.endswith("::test_streaming_chat_completion_without_store"): - stream = client.chat.completions.create( - model=TEXT_MODEL, - messages=[{"role": "user", "content": STREAMING_PROMPT}], - stream=True, - ) - list(stream) - else: - client.chat.completions.create( - model=TEXT_MODEL, - messages=[{"role": "user", "content": NON_STREAMING_PROMPT}], - ) - finally: - client.shutdown() - - -def _stage_recordings(staged_dir: str) -> int: - """Copy generated chat recordings to staging and normalize their provider URLs.""" - for name in os.listdir(RECORDINGS_DIR): - if not name.endswith(".json") or name.startswith("models-"): - continue - source = os.path.join(RECORDINGS_DIR, name) - with open(source, encoding="utf-8") as file: - data = json.load(file) - url = data.get("request", {}).get("url", "") - if re.search(r"\d+\.\d+\.\d+\.\d+:\d+", url): - data["request"]["url"] = "https://api.openai.com/v1" + url.split("/v1", 1)[1] - with open(os.path.join(staged_dir, name), "w", encoding="utf-8") as file: - json.dump(data, file, indent=2) - file.write("\n") - return len(os.listdir(staged_dir)) - - -def _generate_recordings(config_file: str) -> int: - """Generate recordings while preserving the repository's existing fixtures.""" - recordings_parent = os.path.dirname(RECORDINGS_DIR) - with ( - tempfile.TemporaryDirectory(prefix=".recordings-backup-", dir=recordings_parent) as backup_dir, - tempfile.TemporaryDirectory(prefix="ogx-staged-") as staged_dir, - ): - original_recordings = os.path.join(backup_dir, "recordings") - had_original_recordings = os.path.isdir(RECORDINGS_DIR) - if had_original_recordings: - shutil.move(RECORDINGS_DIR, original_recordings) - - try: - for test_id in TEST_NODE_IDS: - print(f"recording for {test_id} ...") - asyncio.run(_run(test_id, config_file)) - n_written = _stage_recordings(staged_dir) - finally: - shutil.rmtree(RECORDINGS_DIR, ignore_errors=True) - if had_original_recordings: - shutil.move(original_recordings, RECORDINGS_DIR) - - os.makedirs(RECORDINGS_DIR, exist_ok=True) - for name in os.listdir(staged_dir): - shutil.copy2(os.path.join(staged_dir, name), os.path.join(RECORDINGS_DIR, name)) - return n_written - - -def main() -> None: - os.environ["OGX_TEST_INFERENCE_MODE"] = "record" - os.environ["OGX_LOGGING"] = "all=warning" - os.environ["OPENAI_API_KEY"] = "fake-key-for-replay" - - with ( - tempfile.TemporaryDirectory(prefix="ogx-record-") as sqlite_dir, - tempfile.TemporaryDirectory(prefix="ogx-config-") as config_dir, - _mock_server() as server, - ): - os.environ["SQLITE_STORE_DIR"] = sqlite_dir - port = server.server_address[1] - os.environ["OPENAI_BASE_URL"] = f"http://{MOCK_HOST}:{port}/v1" - - run_config = build_inference_store_disabled_run_config() - config_file = os.path.join(config_dir, "run.yaml") - with open(config_file, "w", encoding="utf-8") as file: - yaml.safe_dump(run_config.model_dump(mode="json"), file) - - n_written = _generate_recordings(config_file) - - print(f"wrote {n_written} recordings") - print("done") - - -if __name__ == "__main__": - main() diff --git a/src/ogx/core/routers/__init__.py b/src/ogx/core/routers/__init__.py index 0e13604b899..4d9bfff6469 100644 --- a/src/ogx/core/routers/__init__.py +++ b/src/ogx/core/routers/__init__.py @@ -60,11 +60,6 @@ async def get_auto_router_impl( api_to_dep_impl = {} # TODO: move pass configs to routers instead if api == Api.inference: - # An absent inference store reference disables chat completion - # persistence: no store is constructed, no table is created, and no - # background write workers are started. The inference router handles a - # missing store (it guards every write and raises NotImplementedError - # on the history endpoints), mirroring the optional Responses store. inference_ref = run_config.storage.stores.inference if inference_ref is not None: inference_store = InferenceStore( diff --git a/src/ogx/core/storage/README.md b/src/ogx/core/storage/README.md index 00cdbd5fbbf..07311e12add 100644 --- a/src/ogx/core/storage/README.md +++ b/src/ogx/core/storage/README.md @@ -55,28 +55,16 @@ Storage is configured in `StackConfig.storage` via `StorageConfig`. The `stores` See `datatypes.py` for all config types and `StorageBackendType` for the enum of supported backends. -### Optional Stores (null to disable) - -Some store references are optional: setting the reference to `null` (not omitting -it) means OGX does not construct the store at all, and the API that depends on -it degrades gracefully. This is an explicit operator choice made in a run config; -the default for every optional reference remains an enabled reference (except -`responses`, which defaults to `None`), so existing deployments are unaffected -unless they opt in. - -- **`inference`** -- when `null`, no `InferenceStore` is constructed: no - `inference_store` table is created and no background write workers run. Chat - completions still work (streaming and non-streaming); the chat completion - history endpoints (`list`, `retrieve`, `messages`) report that persistence is - not configured (HTTP 501) rather than returning an empty list or a 404. -- **`responses`** -- the `responses` reference is nullable in the config schema - (`default=None`) and `null` passes validation, but unlike `inference` it is not - a runtime toggle: the built-in responses provider always constructs and - initializes its store from its own `persistence.responses` (a required, - non-nullable reference), and the shared `storage.stores.responses` reference - is only validated, never consumed at startup. Setting it to `null` is accepted - but does not currently disable Responses persistence. - -Other stores (`datasets`, `eval`, `files`, `prompts`, `vector_io`) are not -affected by disabling the inference store, so persistence can be turned off for -one API independently of the rest of the storage layer. +### Inference Store (null to disable) + +Setting the `inference` store reference to `null` explicitly disables Chat +Completions persistence. Omitting the reference keeps the store enabled for +backward compatibility. + +When disabled, no `InferenceStore` is constructed, no `inference_store` table is +created, and no background write workers run. Streaming and non-streaming Chat +Completions continue to work. The history endpoints (`list`, `retrieve`, and +`messages`) report that persistence is not configured (HTTP 501). + +Other stores (`responses`, `datasets`, `eval`, `files`, `prompts`, `vector_io`) +are unaffected by disabling the inference store. diff --git a/src/ogx/distributions/README.md b/src/ogx/distributions/README.md index b7e63f3d529..1f5353aaa4f 100644 --- a/src/ogx/distributions/README.md +++ b/src/ogx/distributions/README.md @@ -73,6 +73,5 @@ completions still work for both streaming and non-streaming requests; the history endpoints report that persistence is not configured (HTTP 501) rather than returning an empty list or a 404. Other stores (`responses`, `datasets`, `eval`, `files`, `prompts`, `vector_io`) stay enabled, so disabling inference -persistence is independent of the rest of the storage layer. This follows the -same optional-store pattern the Responses store already uses; see the storage +persistence is independent of the rest of the storage layer. See the storage module README for details. diff --git a/tests/integration/inference/recordings/3765a840ae86565c72696d32a5617deeb6e07aa051ef058f909a676dfb5d67ec.json b/tests/integration/inference/recordings/3765a840ae86565c72696d32a5617deeb6e07aa051ef058f909a676dfb5d67ec.json deleted file mode 100644 index e95ba48ddcc..00000000000 --- a/tests/integration/inference/recordings/3765a840ae86565c72696d32a5617deeb6e07aa051ef058f909a676dfb5d67ec.json +++ /dev/null @@ -1,61 +0,0 @@ -{ - "test_id": "tests/integration/inference/test_inference_store_disabled.py::test_list_chat_completion_messages_reports_not_configured", - "request": { - "method": "POST", - "url": "https://api.openai.com/v1/v1/chat/completions", - "headers": {}, - "body": { - "model": "gpt-4o", - "messages": [ - { - "role": "user", - "content": "Say hello." - } - ] - }, - "endpoint": "/v1/chat/completions", - "model": "gpt-4o", - "provider_metadata": { - "openai_sdk_version": "2.43.0" - } - }, - "response": { - "body": { - "__type__": "openai.types.chat.chat_completion.ChatCompletion", - "__data__": { - "id": "rec-3765a840ae86", - "choices": [ - { - "finish_reason": "stop", - "index": 0, - "logprobs": null, - "message": { - "content": "Response to: Say hello.", - "refusal": null, - "role": "assistant", - "annotations": null, - "audio": null, - "function_call": null, - "tool_calls": null - } - } - ], - "created": 0, - "model": "gpt-4o", - "object": "chat.completion", - "moderation": null, - "service_tier": null, - "system_fingerprint": null, - "usage": { - "completion_tokens": 5, - "prompt_tokens": 5, - "total_tokens": 10, - "completion_tokens_details": null, - "prompt_tokens_details": null - } - } - }, - "is_streaming": false - }, - "id_normalization_mapping": {} -} diff --git a/tests/integration/inference/recordings/7c573c978200175222e201e0139d452008645fa0abee31912ff10c86a92340db.json b/tests/integration/inference/recordings/7c573c978200175222e201e0139d452008645fa0abee31912ff10c86a92340db.json deleted file mode 100644 index 7033caf90ac..00000000000 --- a/tests/integration/inference/recordings/7c573c978200175222e201e0139d452008645fa0abee31912ff10c86a92340db.json +++ /dev/null @@ -1,61 +0,0 @@ -{ - "test_id": "tests/integration/inference/test_inference_store_disabled.py::test_retrieve_chat_completion_reports_not_configured", - "request": { - "method": "POST", - "url": "https://api.openai.com/v1/v1/chat/completions", - "headers": {}, - "body": { - "model": "gpt-4o", - "messages": [ - { - "role": "user", - "content": "Say hello." - } - ] - }, - "endpoint": "/v1/chat/completions", - "model": "gpt-4o", - "provider_metadata": { - "openai_sdk_version": "2.43.0" - } - }, - "response": { - "body": { - "__type__": "openai.types.chat.chat_completion.ChatCompletion", - "__data__": { - "id": "rec-7c573c978200", - "choices": [ - { - "finish_reason": "stop", - "index": 0, - "logprobs": null, - "message": { - "content": "Response to: Say hello.", - "refusal": null, - "role": "assistant", - "annotations": null, - "audio": null, - "function_call": null, - "tool_calls": null - } - } - ], - "created": 0, - "model": "gpt-4o", - "object": "chat.completion", - "moderation": null, - "service_tier": null, - "system_fingerprint": null, - "usage": { - "completion_tokens": 5, - "prompt_tokens": 5, - "total_tokens": 10, - "completion_tokens_details": null, - "prompt_tokens_details": null - } - } - }, - "is_streaming": false - }, - "id_normalization_mapping": {} -} diff --git a/tests/integration/inference/store_disabled_support.py b/tests/integration/inference/store_disabled_support.py index cd269007d78..292b4163b81 100644 --- a/tests/integration/inference/store_disabled_support.py +++ b/tests/integration/inference/store_disabled_support.py @@ -4,14 +4,7 @@ # This source code is licensed under the terms described in the LICENSE file in # the root directory of this source tree. -"""Shared support for the inference-store-disabled tests and recording generator. - -The recording harness keys recordings by a SHA256 hash of the request body and the -pytest node id, so the model id, prompts, and node ids MUST match between -``tests/integration/inference/test_inference_store_disabled.py`` and -``scripts/gen_inference_store_disabled_recordings.py``. Keeping them in one module -prevents silent drift between the test and its regenerable recordings. -""" +"""Shared support for inference-store-disabled integration tests.""" from ogx.core.datatypes import StackConfig from ogx.core.stack import get_stack_run_config_from_distro @@ -21,18 +14,6 @@ NON_STREAMING_PROMPT = "Say hello." STREAMING_PROMPT = "Say hello in one sentence." -# The full pytest node ids the generator records completions for. The test's -# list/retrieve/messages tests raise before any provider call, so only the four -# ids below perform chat-completion requests worth recording; the remaining tests -# need no recording. -TEST_MODULE = "tests/integration/inference/test_inference_store_disabled.py" -NON_STREAMING_TEST = f"{TEST_MODULE}::test_non_streaming_chat_completion_without_store" -STREAMING_TEST = f"{TEST_MODULE}::test_streaming_chat_completion_without_store" -RETRIEVE_TEST = f"{TEST_MODULE}::test_retrieve_chat_completion_reports_not_configured" -MESSAGES_TEST = f"{TEST_MODULE}::test_list_chat_completion_messages_reports_not_configured" - -RECORDING_TEST_IDS = [NON_STREAMING_TEST, STREAMING_TEST, RETRIEVE_TEST, MESSAGES_TEST] - def build_inference_store_disabled_run_config() -> StackConfig: """Build the minimal ci-tests configuration used by this test scenario.""" diff --git a/tests/integration/inference/test_inference_store_disabled.py b/tests/integration/inference/test_inference_store_disabled.py index 004694c0753..4c1850ad362 100644 --- a/tests/integration/inference/test_inference_store_disabled.py +++ b/tests/integration/inference/test_inference_store_disabled.py @@ -14,8 +14,8 @@ persistence is not configured (501) rather than returning an empty list or 404. This test builds a ``StackConfig`` from the ``ci-tests`` distribution with the -inference store reference removed, boots an in-process library client from it, -and exercises the full HTTP path through the OpenAI-compatible client. +inference store reference set to ``null``, boots an in-process library client +from it, and exercises the full HTTP path through the OpenAI-compatible client. """ import os @@ -141,14 +141,10 @@ def test_list_chat_completions_reports_not_configured(non_persisting_client: Non def test_retrieve_chat_completion_reports_not_configured(non_persisting_client: NonPersistingClient) -> None: - """retrieve for a just-completed id raises a not-configured error rather than a 404.""" + """retrieve raises a not-configured error rather than a 404.""" client, _ = non_persisting_client - response = client.chat.completions.create( - model=TEXT_MODEL, - messages=[{"role": "user", "content": NON_STREAMING_PROMPT}], - ) with pytest.raises(NotImplementedError): - client.chat.completions.retrieve(response.id) + client.chat.completions.retrieve("chatcmpl-not-persisted") def test_list_chat_completion_messages_reports_not_configured( @@ -156,12 +152,8 @@ def test_list_chat_completion_messages_reports_not_configured( ) -> None: """messages raises a not-configured error, consistent with list/retrieve.""" client, _ = non_persisting_client - response = client.chat.completions.create( - model=TEXT_MODEL, - messages=[{"role": "user", "content": NON_STREAMING_PROMPT}], - ) with pytest.raises(NotImplementedError): - client.chat.completions.messages.list(completion_id=response.id) + client.chat.completions.messages.list(completion_id="chatcmpl-not-persisted") def test_no_inference_store_table_when_persistence_disabled( diff --git a/tests/unit/core/test_storage_references.py b/tests/unit/core/test_storage_references.py index 910a1916e88..eb95a2d17bc 100644 --- a/tests/unit/core/test_storage_references.py +++ b/tests/unit/core/test_storage_references.py @@ -7,7 +7,6 @@ """Unit tests for storage backend/reference validation.""" import os -from typing import Any import pytest from pydantic import ValidationError @@ -109,12 +108,12 @@ def test_inference_store_none_disables_persistence() -> None: def test_inference_store_config_omit_vs_null_parsing() -> None: - """Lock the omit-vs-null semantics at the StackConfig parsing level. + """Lock the omit-vs-null semantics when parsing `ServerStoresConfig`. - `inference` key absent -> enabled default (persistence on) - `inference: null` -> disabled (persistence off) """ - base = _default_stores_dict() + base = ServerStoresConfig().model_dump(mode="python") # Omit the inference key entirely: the default applies (persistence on). omitted = dict(base) @@ -129,14 +128,6 @@ def test_inference_store_config_omit_vs_null_parsing() -> None: assert stores_null.inference is None -def _default_stores_dict() -> dict[str, Any]: - """A valid `ServerStoresConfig` serialized with only the non-inference keys.""" - return ServerStoresConfig( - metadata=KVStoreReference(backend="kv_default", namespace="registry"), - conversations=SqlStoreReference(backend="sql_default", table_name="conversations"), - ).model_dump(mode="python") - - @pytest.mark.parametrize("backend_key", ["kv_default", "sql_default"]) def test_default_backends_resolve_env_vars(backend_key, monkeypatch): """Default StorageConfig backends must contain real paths, not literal From 12d4af718c7dd09bb8a1e0dc063ddfd5f504cef0 Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Wed, 19 Aug 2026 10:46:21 +0200 Subject: [PATCH 7/8] add store.enabled field to inference store config Signed-off-by: Matt, Matthias --- src/ogx/core/routers/__init__.py | 4 +- src/ogx/core/storage/README.md | 9 +-- src/ogx/core/storage/datatypes.py | 17 ++++- src/ogx/distributions/README.md | 7 +- src/ogx/distributions/ci-tests/config.yaml | 1 + .../ci-tests/run-with-postgres-store.yaml | 1 + src/ogx/distributions/nvidia/config.yaml | 1 + src/ogx/distributions/oci/config.yaml | 1 + .../distributions/open-benchmark/config.yaml | 1 + src/ogx/distributions/starter/config.yaml | 1 + .../starter/run-with-postgres-store.yaml | 1 + src/ogx/distributions/watsonx/config.yaml | 1 + .../inference/store_disabled_support.py | 4 +- .../test_inference_store_disabled.py | 17 ++--- tests/unit/core/test_storage_references.py | 42 ++++++------ tests/unit/server/test_resolver.py | 68 +++++++++++++++++-- 16 files changed, 131 insertions(+), 45 deletions(-) diff --git a/src/ogx/core/routers/__init__.py b/src/ogx/core/routers/__init__.py index 4d9bfff6469..960d9ebdc0e 100644 --- a/src/ogx/core/routers/__init__.py +++ b/src/ogx/core/routers/__init__.py @@ -61,7 +61,9 @@ async def get_auto_router_impl( # TODO: move pass configs to routers instead if api == Api.inference: inference_ref = run_config.storage.stores.inference - if inference_ref is not None: + if inference_ref is None: + raise ValueError("storage.stores.inference must be configured in run config") + if inference_ref.enabled: inference_store = InferenceStore( reference=inference_ref, policy=policy, diff --git a/src/ogx/core/storage/README.md b/src/ogx/core/storage/README.md index 07311e12add..cc9df551211 100644 --- a/src/ogx/core/storage/README.md +++ b/src/ogx/core/storage/README.md @@ -55,11 +55,12 @@ Storage is configured in `StackConfig.storage` via `StorageConfig`. The `stores` See `datatypes.py` for all config types and `StorageBackendType` for the enum of supported backends. -### Inference Store (null to disable) +### Inference Store (enabled flag) -Setting the `inference` store reference to `null` explicitly disables Chat -Completions persistence. Omitting the reference keeps the store enabled for -backward compatibility. +Setting `inference.enabled: false` on the inference store reference explicitly +disables Chat Completions persistence. Omitting the flag (or the whole +reference) keeps the store enabled for backward compatibility, and setting the +reference itself to `null` remains a configuration error. When disabled, no `InferenceStore` is constructed, no `inference_store` table is created, and no background write workers run. Streaming and non-streaming Chat diff --git a/src/ogx/core/storage/datatypes.py b/src/ogx/core/storage/datatypes.py index 403942c8eb2..841116da377 100644 --- a/src/ogx/core/storage/datatypes.py +++ b/src/ogx/core/storage/datatypes.py @@ -278,8 +278,8 @@ class KVStoreReference(BaseModel): ] -class InferenceStoreReference(SqlStoreReference): - """Inference store configuration with queue tuning.""" +class _QueuedSqlStoreReference(SqlStoreReference): + """Base for SQL store references with background write-queue tuning.""" max_write_queue_size: int = Field( default=10000, @@ -291,7 +291,18 @@ class InferenceStoreReference(SqlStoreReference): ) -class ResponsesStoreReference(InferenceStoreReference): +class InferenceStoreReference(_QueuedSqlStoreReference): + """Inference store configuration with queue tuning.""" + + enabled: bool = Field( + default=True, + description=( + "Whether the store is enabled; when false, the store is not constructed and payloads are not persisted" + ), + ) + + +class ResponsesStoreReference(_QueuedSqlStoreReference): """Responses store configuration with queue tuning.""" table_name: str = Field( diff --git a/src/ogx/distributions/README.md b/src/ogx/distributions/README.md index 1f5353aaa4f..534364e7bdb 100644 --- a/src/ogx/distributions/README.md +++ b/src/ogx/distributions/README.md @@ -58,16 +58,17 @@ ogx stack run --config path/to/config.yaml By default the `inference` store reference is enabled, so chat completion request/response payloads are persisted and the history endpoints (`list`, `retrieve`, `messages`) are served from it. An operator can disable that -persistence for the inference API by setting the reference to `null` in a run +persistence for the inference API by setting `inference.enabled: false` in a run config: ```yaml storage: stores: - inference: null + inference: + enabled: false ``` -When `inference` is `null`, OGX does not construct an `InferenceStore` at all: +When the store is disabled, OGX does not construct an `InferenceStore` at all: no `inference_store` table is created and no background write workers run. Chat completions still work for both streaming and non-streaming requests; the history endpoints report that persistence is not configured (HTTP 501) rather diff --git a/src/ogx/distributions/ci-tests/config.yaml b/src/ogx/distributions/ci-tests/config.yaml index 68dfa3be886..2822db21329 100644 --- a/src/ogx/distributions/ci-tests/config.yaml +++ b/src/ogx/distributions/ci-tests/config.yaml @@ -274,6 +274,7 @@ storage: backend: sql_default max_write_queue_size: 10000 num_writers: 4 + enabled: true conversations: table_name: openai_conversations backend: sql_default diff --git a/src/ogx/distributions/ci-tests/run-with-postgres-store.yaml b/src/ogx/distributions/ci-tests/run-with-postgres-store.yaml index 30d28672d45..6bdee90714e 100644 --- a/src/ogx/distributions/ci-tests/run-with-postgres-store.yaml +++ b/src/ogx/distributions/ci-tests/run-with-postgres-store.yaml @@ -287,6 +287,7 @@ storage: backend: sql_default max_write_queue_size: 10000 num_writers: 4 + enabled: true conversations: table_name: openai_conversations backend: sql_default diff --git a/src/ogx/distributions/nvidia/config.yaml b/src/ogx/distributions/nvidia/config.yaml index 86202c1fc00..6ba2818ce8c 100644 --- a/src/ogx/distributions/nvidia/config.yaml +++ b/src/ogx/distributions/nvidia/config.yaml @@ -58,6 +58,7 @@ storage: backend: sql_default max_write_queue_size: 10000 num_writers: 4 + enabled: true conversations: table_name: openai_conversations backend: sql_default diff --git a/src/ogx/distributions/oci/config.yaml b/src/ogx/distributions/oci/config.yaml index 2d2776c5d39..b771a508b6e 100644 --- a/src/ogx/distributions/oci/config.yaml +++ b/src/ogx/distributions/oci/config.yaml @@ -73,6 +73,7 @@ storage: backend: sql_default max_write_queue_size: 10000 num_writers: 4 + enabled: true conversations: table_name: openai_conversations backend: sql_default diff --git a/src/ogx/distributions/open-benchmark/config.yaml b/src/ogx/distributions/open-benchmark/config.yaml index 1e0fb054b63..47df967de7d 100644 --- a/src/ogx/distributions/open-benchmark/config.yaml +++ b/src/ogx/distributions/open-benchmark/config.yaml @@ -106,6 +106,7 @@ storage: backend: sql_default max_write_queue_size: 10000 num_writers: 4 + enabled: true conversations: table_name: openai_conversations backend: sql_default diff --git a/src/ogx/distributions/starter/config.yaml b/src/ogx/distributions/starter/config.yaml index 7349f176d69..5c9bae61f54 100644 --- a/src/ogx/distributions/starter/config.yaml +++ b/src/ogx/distributions/starter/config.yaml @@ -268,6 +268,7 @@ storage: backend: sql_default max_write_queue_size: 10000 num_writers: 4 + enabled: true conversations: table_name: openai_conversations backend: sql_default diff --git a/src/ogx/distributions/starter/run-with-postgres-store.yaml b/src/ogx/distributions/starter/run-with-postgres-store.yaml index 4f424234b24..16bec8887a8 100644 --- a/src/ogx/distributions/starter/run-with-postgres-store.yaml +++ b/src/ogx/distributions/starter/run-with-postgres-store.yaml @@ -281,6 +281,7 @@ storage: backend: sql_default max_write_queue_size: 10000 num_writers: 4 + enabled: true conversations: table_name: openai_conversations backend: sql_default diff --git a/src/ogx/distributions/watsonx/config.yaml b/src/ogx/distributions/watsonx/config.yaml index 7a5e7978bda..bd649f8cdb9 100644 --- a/src/ogx/distributions/watsonx/config.yaml +++ b/src/ogx/distributions/watsonx/config.yaml @@ -71,6 +71,7 @@ storage: backend: sql_default max_write_queue_size: 10000 num_writers: 4 + enabled: true conversations: table_name: openai_conversations backend: sql_default diff --git a/tests/integration/inference/store_disabled_support.py b/tests/integration/inference/store_disabled_support.py index 292b4163b81..f25d878638d 100644 --- a/tests/integration/inference/store_disabled_support.py +++ b/tests/integration/inference/store_disabled_support.py @@ -18,7 +18,9 @@ def build_inference_store_disabled_run_config() -> StackConfig: """Build the minimal ci-tests configuration used by this test scenario.""" run_config = get_stack_run_config_from_distro("ci-tests") - run_config.storage.stores.inference = None + # The ci-tests distribution always configures the inference store; disable + # persistence through the explicit flag while keeping the reference valid. + run_config.storage.stores.inference.enabled = False # Vector-store model validation is unrelated to chat completion persistence # and loads the sentence-transformers stack during an in-process boot. run_config.vector_stores = None diff --git a/tests/integration/inference/test_inference_store_disabled.py b/tests/integration/inference/test_inference_store_disabled.py index 4c1850ad362..2627d2c7bfb 100644 --- a/tests/integration/inference/test_inference_store_disabled.py +++ b/tests/integration/inference/test_inference_store_disabled.py @@ -6,15 +6,16 @@ """Chat Completions behavior when persistence is disabled. -When an operator configures ``storage.stores.inference`` as ``null``, OGX must -not construct an ``InferenceStore`` at all: no ``inference_store`` table is -created, no background write workers run, and no chat completion request or -response payload is ever persisted. Chat completions still work for both -streaming and non-streaming requests, and the history endpoints report that -persistence is not configured (501) rather than returning an empty list or 404. +When an operator disables the inference store +(``storage.stores.inference.enabled: false``), OGX must not construct an +``InferenceStore`` at all: no ``inference_store`` table is created, no background +write workers run, and no chat completion request or response payload is ever +persisted. Chat completions still work for both streaming and non-streaming +requests, and the history endpoints report that persistence is not configured +(501) rather than returning an empty list or 404. This test builds a ``StackConfig`` from the ``ci-tests`` distribution with the -inference store reference set to ``null``, boots an in-process library client +inference store disabled (``enabled: false``), boots an in-process library client from it, and exercises the full HTTP path through the OpenAI-compatible client. """ @@ -48,7 +49,7 @@ def non_persisting_client() -> Generator[NonPersistingClient, None, None]: This deliberately does not use the shared ``ogx_client`` fixture: unlike that fixture it must boot its own stack with a custom store config - (``inference: null``), which cannot be expressed through the standard + (``inference.enabled: false``), which cannot be expressed through the standard server-mode run config. See ``_boot_library_client`` for how the in-process boot avoids colliding with the outer server-mode OGX server. """ diff --git a/tests/unit/core/test_storage_references.py b/tests/unit/core/test_storage_references.py index eb95a2d17bc..69324523237 100644 --- a/tests/unit/core/test_storage_references.py +++ b/tests/unit/core/test_storage_references.py @@ -89,29 +89,31 @@ def test_valid_configuration_passes_validation(): def test_inference_store_defaults_enabled_when_omitted() -> None: """Omitting the `inference` key keeps persistence enabled (backward compatible default). - The optional typing on `ServerStoresConfig.inference` means `None` is a valid, - explicit, opt-in way to disable persistence. The default must remain an enabled - reference so that existing deployments that omit the key keep their behavior, - and so that a valid non-persisting config is expressed as `inference: null` - rather than by deleting the key. + Existing deployments that omit the key keep their behavior; a non-persisting + config is expressed with `inference.enabled: false` rather than by deleting + the key or nulling the reference. """ stores = ServerStoresConfig() assert stores.inference is not None + assert stores.inference.enabled is True assert stores.inference.backend == "sql_default" assert stores.inference.table_name == "inference_store" -def test_inference_store_none_disables_persistence() -> None: - """Setting `inference` to null explicitly opts out of chat completion persistence.""" - stores = ServerStoresConfig(inference=None) - assert stores.inference is None +def test_inference_store_enabled_false_disables_persistence() -> None: + """Setting `inference.enabled` to false opts out of chat completion persistence.""" + stores = ServerStoresConfig( + inference=InferenceStoreReference(backend="sql_default", table_name="inference_store", enabled=False) + ) + assert stores.inference is not None + assert stores.inference.enabled is False -def test_inference_store_config_omit_vs_null_parsing() -> None: - """Lock the omit-vs-null semantics when parsing `ServerStoresConfig`. +def test_inference_store_enabled_flag_parsing() -> None: + """Lock the enabled-flag semantics when parsing `ServerStoresConfig`. - - `inference` key absent -> enabled default (persistence on) - - `inference: null` -> disabled (persistence off) + - `inference` key absent -> enabled default (persistence on) + - `inference.enabled: false` -> disabled (persistence off) """ base = ServerStoresConfig().model_dump(mode="python") @@ -120,12 +122,14 @@ def test_inference_store_config_omit_vs_null_parsing() -> None: omitted.pop("inference") stores_omitted = ServerStoresConfig.model_validate(omitted) assert stores_omitted.inference is not None - - # Explicitly set inference to null: persistence disabled. - explicit_null = dict(base) - explicit_null["inference"] = None - stores_null = ServerStoresConfig.model_validate(explicit_null) - assert stores_null.inference is None + assert stores_omitted.inference.enabled is True + + # Explicitly disable persistence via the flag. + disabled = dict(base) + disabled["inference"] = {**base["inference"], "enabled": False} + stores_disabled = ServerStoresConfig.model_validate(disabled) + assert stores_disabled.inference is not None + assert stores_disabled.inference.enabled is False @pytest.mark.parametrize("backend_key", ["kv_default", "sql_default"]) diff --git a/tests/unit/server/test_resolver.py b/tests/unit/server/test_resolver.py index 5a3abd6aaa8..ce174eae098 100644 --- a/tests/unit/server/test_resolver.py +++ b/tests/unit/server/test_resolver.py @@ -9,6 +9,7 @@ from typing import Any, Protocol from unittest.mock import AsyncMock, MagicMock, patch +import pytest from pydantic import BaseModel, Field from ogx.core.datatypes import Api, Provider, StackConfig @@ -151,12 +152,12 @@ async def test_resolve_impls_basic(): assert impl.__provider_spec__ == provider_spec -async def test_resolve_impls_inference_without_store_skips_persistence() -> None: - """An absent inference store reference must not construct an InferenceStore. +async def test_resolve_impls_inference_store_disabled_skips_persistence() -> None: + """An inference store reference with `enabled: false` must not construct an InferenceStore. - When `storage.stores.inference` is None, the auto-router factory skips - constructing (and initializing) the InferenceStore entirely and builds the - InferenceRouter with no store dependency. No table is created and no + When `storage.stores.inference.enabled` is false, the auto-router factory + skips constructing (and initializing) the InferenceStore entirely and builds + the InferenceRouter with no store dependency. No table is created and no background write workers are started, so no chat completion payload is ever persisted. """ @@ -188,7 +189,7 @@ async def test_resolve_impls_inference_without_store_skips_persistence() -> None }, stores=ServerStoresConfig( metadata=KVStoreReference(backend="kv_default", namespace="registry"), - inference=None, + inference=InferenceStoreReference(backend="sql_default", table_name="inference_store", enabled=False), conversations=SqlStoreReference(backend="sql_default", table_name="conversations"), ), ), @@ -212,3 +213,58 @@ async def test_resolve_impls_inference_without_store_skips_persistence() -> None router = impls[Api.inference] assert isinstance(router, InferenceRouter) assert router.store is None + + +async def test_resolve_impls_inference_store_reference_null_raises() -> None: + """A null inference store reference is a configuration error, not a disable switch. + + Disabling persistence is expressed with `inference.enabled: false`; a null + reference keeps failing stack resolution with the pre-existing configuration + error instead of silently disabling the store. + """ + provider_spec = InlineProviderSpec( + api=Api.inference, + provider_type="sample", + module="test_module", + config_class="test_resolver.SampleConfig", + api_dependencies=[], + ) + + provider_registry = {Api.inference: {provider_spec.provider_type: provider_spec}} + + run_config = make_run_config( + distro_name="test_image", + providers={ + "inference": [ + Provider( + provider_id="sample_provider", + provider_type="sample", + config=SampleConfig.sample_run_config(), + ) + ] + }, + storage=StorageConfig( + backends={ + "kv_default": SqliteKVStoreConfig(db_path=":memory:"), + "sql_default": SqliteSqlStoreConfig(db_path=":memory:"), + }, + stores=ServerStoresConfig( + metadata=KVStoreReference(backend="kv_default", namespace="registry"), + inference=None, + conversations=SqlStoreReference(backend="sql_default", table_name="conversations"), + ), + ), + ) + + dist_registry = MagicMock() + + mock_module = MagicMock() + impl = SampleImpl(SampleConfig(foo="baz"), {}, provider_spec) + add_protocol_methods(SampleImpl, Inference) + + mock_module.get_provider_impl = AsyncMock(return_value=impl) + mock_module.get_provider_impl.__text_signature__ = "()" + sys.modules["test_module"] = mock_module + + with pytest.raises(ValueError, match="storage.stores.inference must be configured in run config"): + await resolve_impls(run_config, provider_registry, dist_registry, policy={}) From f8b32385a5959b2b1c34a9880814a071a3c213d9 Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Wed, 19 Aug 2026 20:21:23 +0200 Subject: [PATCH 8/8] skip inference-store-disabled test in server-mode sessions Signed-off-by: Matt, Matthias --- .../inference/test_inference_store_disabled.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/integration/inference/test_inference_store_disabled.py b/tests/integration/inference/test_inference_store_disabled.py index 2627d2c7bfb..5239002a8e0 100644 --- a/tests/integration/inference/test_inference_store_disabled.py +++ b/tests/integration/inference/test_inference_store_disabled.py @@ -17,6 +17,8 @@ This test builds a ``StackConfig`` from the ``ci-tests`` distribution with the inference store disabled (``enabled: false``), boots an in-process library client from it, and exercises the full HTTP path through the OpenAI-compatible client. +It is gated to library-client sessions (see ``pytestmark`` below): booting an +in-process stack inside a server-mode session is unsupported. """ import os @@ -36,6 +38,17 @@ build_inference_store_disabled_run_config, ) +# This test boots its own in-process stack, which must not be mixed into a +# server-mode session (where the shared ogx_client fixture runs a separate HTTP +# server and the recorder installs a server-only test-ID patch). Like +# tests/integration/inspect/test_metrics_endpoint.py, gate on the session's +# stack-config type so this only runs in library-client sessions. +pytestmark = pytest.mark.skipif( + os.environ.get("OGX_TEST_STACK_CONFIG_TYPE") == "server", + reason="Boots an in-process library client; cannot run inside a server-mode session", +) + + NonPersistingClient = tuple[OGXAsLibraryClient, Path]