Skip to content

feat(operators): self-hosted OpenAI-compatible model provider, off by default - #1436

Merged
jfrench9 merged 2 commits into
mainfrom
feature/openai-compat-provider
Sep 18, 2026
Merged

jfrench9 merged 2 commits into
mainfrom
feature/openai-compat-provider

Conversation

@jfrench9

Copy link
Copy Markdown
Member

Summary

Adds an optional second model provider, so the open-source stack can run the AI operators on open weights through any OpenAI-compatible Chat Completions server (Ollama, LM Studio, vLLM, NVIDIA NIM), with no proprietary inference service. Bedrock stays the platform path. The new provider is off by default: hosted deployments leave it off, and a deployment turns it on only when it wants its own model.

Changes

Registry and gating (config/operators.py, config/env.py, config/billing/ai.py)

  • OPENAI_COMPAT_ENABLED turns the provider on. It is read from the env var first, then the features/ SSM flag, and defaults to off.
  • When off, the openai-compat registry row and its openai_compat rate-card key are never created. Nothing can resolve to the model, and the meter refuses it.
  • When on, OPENAI_COMPAT_BASE_URL, OPENAI_COMPAT_MODEL and OPENAI_COMPAT_API_KEY are read through get_secret_value: the env var first, then the deployment secret in staging/prod. When off, none of the three is read at all.
  • The config fails at import in three cases:
    • the base URL or model is missing while the provider is on;
    • OPENAI_COMPAT_MODEL collides with a registered short name, a profile name or a Bedrock wire id, because resolution would then run and bill it as a different model;
    • an OPERATOR_PROFILE_* value is unknown, or names the self-hosted model while the provider is off.
  • ModelSpec gains a provider field (bedrock | openai_compat).
  • OPERATOR_PROFILE_ECONOMY / _BALANCED / _QUALITY point a tier at any registered model for one deployment. This is deployment-scoped and never set by customers. With all three unset, the platform mapping is unchanged.
  • Self-hosted rates come from OPENAI_COMPAT_CREDITS_PER_1K_INPUT / _OUTPUT and default to 0: the rate card is a cost passthrough, and a self-hosted GPU has no per-token vendor cost. Cache reads and writes bill at the input rate, since no caching discount is assumed. Malformed, infinite or negative rates raise.
  • BedrockModel is renamed OperatorModel, because the registry is no longer Bedrock-only. The enum is internal and not part of the API or the SDKs.

Client (operations/operators/ai_client.py, new operations/operators/openai_compat.py)

  • AIClient.create_message dispatches on spec.provider. The Bedrock path is unchanged.
  • Converse stays the canonical transcript, and openai_compat.py translates at the edge:
    • outbound, toolUse blocks become tool_calls, and each toolResult becomes a tool message, with error status written as an Error: prefix because Chat Completions has no error flag;
    • reasoning blocks and cache points are dropped;
    • inbound, the reply is turned back into Converse blocks;
    • any turn that carries tool calls is reported as tool_use, even when the server says stop;
    • unparseable tool arguments are kept under a marker key, so the tool rejects the call and the model can correct it;
    • inline <think> blocks are stripped;
    • cached_tokens is reported as cache reads.
  • The tool loop, the operators and the meter are unchanged.
  • The transport is httpx, which is already a dependency, with one client per call so no connection pool outlives its event loop.
  • Every non-2xx response and every transport failure raises AIProviderError, the same operator-surfaced failure Bedrock refusals already use.

Docs

  • .env.example has a commented self-hosted block.
  • config/README.md has a new paragraph describing the provider.

Worth a close look: the import-time validation in build_model_registry / build_profile_models, and the user-turn ordering in _user_messages. Tool messages must come directly after the assistant turn that called them; the loop's wrap-up nudge follows as trailing text.

Breaking Changes

None. There is no change to the REST, GraphQL or operations surface, so neither SDK tier is touched.

Testing

  • New tests:
    • tests/operations/test_openai_compat.py (29 tests): message translation, request building, response parsing, HTTP errors against httpx.MockTransport, provider dispatch, and a full run_tool_loop run on the self-hosted path in which a failed query is fed back as an error, the model corrects it, and the answer lands.
    • tests/config/test_operators.py: off-by-default gating, registry validation and tier overrides.
    • tests/config/billing/test_ai.py: rate validation.
  • Suites run:
    • tests/config, tests/operations/test_ai_client.py and tests/operations/operators pass.
    • just test-code passes (ruff, format, basedpyright, cf-lint).
    • The full unit suite, run locally with -n 2, finished with failures only in eight database-backed modules this change does not touch. Their errors were connection exhaustion (too many clients already, out of shared memory) on the local Postgres, which chore(dev): pin local Postgres connection and memory settings #1435 addresses. All eight pass when rerun serially.
    • just test-all at default parallelism hits the same local Postgres limit and was not green locally. CI is the gate.
  • Manual check: a dev process with no AWS credentials and every tier set to the self-hosted model sent the call to the configured endpoint and never reached Bedrock. The endpoint was unreachable, and the error surfaced as AIProviderError.
  • Not done yet: a live run against a real open-weight model server, and the twelve-question operator eval on that model.

Certification

  • I have the right to submit this work under the Apache 2.0 license, and do so. Where any part of it is owned by my employer, I have their permission.

… default

Adds an optional second model provider so the open-source stack can run
the AI operators on open weights through any OpenAI-compatible Chat
Completions server (Ollama, LM Studio, vLLM, NVIDIA NIM), with no
proprietary inference service. Bedrock remains the platform path.

- OPENAI_COMPAT_ENABLED gates it (env, then the SSM feature flag) and is
  off by default. When off, the `openai-compat` registry row and its
  `openai_compat` rate-card key do not exist, so nothing can resolve to
  the model and the meter refuses it.
- OPERATOR_PROFILE_ECONOMY/_BALANCED/_QUALITY point a tier at any
  registered model for one deployment. An unknown name, or a tier on the
  self-hosted model while it is off, fails the boot.
- Converse stays the canonical transcript. openai_compat.py translates
  at the edge (tool calls, tool results with error status, reasoning
  dropped, cached-token usage), so the tool loop, the operators and the
  meter are unchanged. Transport is httpx; no new dependency.
- Self-hosted rates are configured per deployment and default to 0,
  since the rate card is a cost passthrough.
- BedrockModel is renamed OperatorModel now that the registry is not
  Bedrock-only.
@github-actions github-actions Bot added the change:normal Normal change: automated review documented on the pull request before merge label Sep 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Change classification: normal — platform or application code changed.

A normal change carries a documented automated review on this pull request before merge, in place of a second human approver. The change-classification job requests that review and passes once it has been posted. Recorded by the change-classification job.

@jfrench9

Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown

Claude finished @jfrench9's task in 3m 7s —— View job


Review: self-hosted OpenAI-compatible provider

  • Gather context (PR description, diff, changed files)
  • Read changed files in detail
  • Review registry/gating logic (config/operators.py, config/env.py, config/billing/ai.py)
  • Review client/translation logic (ai_client.py, openai_compat.py)
  • Review tests
  • Post review feedback

Solid piece of work — the off-by-default gating is airtight (env var read only when OPENAI_COMPAT_ENABLED, registry row and rate-card entry both absent when off, collision checks on the registered id, Converse kept as the one canonical transcript with translation isolated to the edge module). Test coverage for the translation logic and gating is thorough, including the full tool-loop round trip.

One finding worth addressing before this ships as a real Bedrock-free path:

AIClient.__init__ still unconditionally builds — and, outside ENVIRONMENT=dev, live-validates — a Bedrock client, even when OPENAI_COMPAT_ENABLED=true and every profile is mapped to the self-hosted model.
robosystems/operations/operators/ai_client.py:137-141 always calls self._initialize_bedrock_client() before self._initialize_self_hosted_client(). _initialize_bedrock_client (lines 143-182) calls boto3.client("sts", ...).get_caller_identity() whenever env.ENVIRONMENT != "dev" — a real network call that raises ValueError on missing/invalid AWS credentials. So a deployment running as staging/prod (the natural target for a real self-hosted install, not just local dev) with no AWS credentials at all will fail at get_ai_client() before it ever reaches the self-hosted client, regardless of whether any profile still points at Bedrock. This directly cuts against the PR's stated goal ("run the AI operators on open weights ... with no proprietary inference service") for anything other than ENVIRONMENT=dev — which matches the PR's own "Manual check" note (only verified in a dev process). Worth either skipping Bedrock client construction/STS validation when no profile resolves to a Bedrock row, or explicitly scoping the claim to dev deployments in the docs.

Minor, non-blocking:

  • OPENAI_COMPAT_MAX_OUTPUT_TOKENS (config/env.py:1107) is read via plain get_int_env with no validation. A negative value flows into self_hosted_model_spec's max_output_tokens and then min(max_tokens, spec.max_output_tokens) in build_chat_request, silently sending a negative max_tokens to the endpoint. Same asymmetry as the credits fields, but those are validated (self_hosted_rates rejects negative/malformed) and this one isn't — probably fine given it's a deployment-scoped config value, but worth a one-line guard for consistency with the "config fails at import" philosophy stated in the PR description.
  • AIResponse.cache_creation_input_tokens is always 0 for the self-hosted path (openai_compat.py never sets it), so TOKEN_PRICING["openai_compat"]["cache_write"] is dead — not wrong, just unused given no OpenAI-compatible server reports a separate cache-write count today.

Not verified in this review: I was unable to run uv run pytest / uv run ruff / uv run basedpyright in this sandbox (tool approval for non-git Bash commands wasn't available), so I read the diff and existing tests but did not execute the suite. The PR description already reports tests/config, tests/operations/test_ai_client.py, tests/operations/operators, and just test-code passing locally, and CI will confirm.

@jfrench9

Copy link
Copy Markdown
Member Author

Review addressed in 088a359:

  • Negative output cap: taken. OPENAI_COMPAT_MAX_OUTPUT_TOKENS < 0 now fails at import, like the other self-hosted settings, and has a test.
  • Bedrock client built outside dev: scoped rather than changed. Every non-dev deployment runs on AWS anyway, because graph allocation needs S3 and the DynamoDB registries. Building the client eagerly and checking its credentials at startup is deliberate: it fails at construction rather than on the first billable call. config/README.md now says that only a dev stack runs with no AWS credentials.
  • Unused cache_write rate: left as is. Every rate-card key carries all four dimensions, and a server that someday reports cache writes will be priced without a code change.

@jfrench9
jfrench9 merged commit c20b3da into main Sep 18, 2026
8 checks passed
@jfrench9
jfrench9 deleted the feature/openai-compat-provider branch September 18, 2026 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:normal Normal change: automated review documented on the pull request before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant