feat(llm): add a system one client that prefers the ai-gateway - #106735
Conversation
🤖 CI report
|
|
[Medium risk] Refactors System One API types into a shared module. The PR appears safe to merge; no outstanding findings or new actionable issues remain. Reviews (2) · Last reviewed commit: "fix(llm): accept typeless system one ans..." |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe change adds shared System One request and response types, body construction, and response validation. It adds client selection between the configured AI gateway and an explicitly configured TypeSafe fallback. The TypeSafe client now uses the shared contract, and its package exports, documentation, and test imports are updated. New tests cover client selection, gateway requests and failures, and request limits. Priority: ➖ Normal Merge Risk: 🔵 Low · up to This adds a System One client that prefers the PostHog-hosted gateway. Nothing calls it yet, so users see no change. The README does not fully state when requests go to TypeSafe instead of the gateway. Operators could misjudge where data is sent, so correct the wording as a small follow-up. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The new client has safeguards against accidental third-party use, but a future call can send request data and a service credential to whichever secure gateway URL is configured. The destination and caller-data policies need to remain explicit when the client is adopted. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
860868b to
71a423b
Compare
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
posthog/egress/typesafe/README.md-8-9 (1)
8-9: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winState the full gateway selection rule.
Line 8 says the client uses the ai-gateway when
AI_GATEWAY_URLis set. That is incomplete._usable_gatewayalso requiresAI_GATEWAY_API_KEY. It also requires anhttpsURL unless the host is loopback. If the URL uses plain HTTP, a caller that passed aTypeSafeFallbackgoes to TypeSafe with no error. Only a warning is logged. Operators need this rule to know which third party gets the request.📝 Proposed wording
-It reaches the ai-gateway where `AI_GATEWAY_URL` is set. -It falls back to this domain only when the caller passes a `TypeSafeFallback`, and every caller that does so meets the usage policy below. +It reaches the ai-gateway where `AI_GATEWAY_URL` (https, or plain http on the local machine) and `AI_GATEWAY_API_KEY` are both set. +Otherwise it falls back to this domain, but only when the caller passes a `TypeSafeFallback`. Every caller that does so meets the usage policy below.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 29fd52e3-9104-4d3a-8327-4ac2f68c3600
📒 Files selected for processing (7)
posthog/egress/test/test_typesafe.pyposthog/egress/typesafe/README.mdposthog/egress/typesafe/__init__.pyposthog/egress/typesafe/client.pyposthog/llm/system_one.pyposthog/llm/system_one_client.pyposthog/llm/test_system_one_client.py
Included review availability: Your plan provides up to 12 included reviews per hour; 4 remain after this review.
build_system_one_client returns a client for the Go ai-gateway's /v1/systemone route where AI_GATEWAY_URL is set, and for TypeSafe elsewhere. Callers name one model per server, because the two serve different models. The System One request and answer types move out of the TypeSafe egress domain into posthog/llm/system_one.py, so both servers share them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… up front Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
71a423b to
b65b97c
Compare
There was a problem hiding this comment.
Approved.
This adds a new, currently-uncalled client builder plus a mechanical type move with matching test updates — no production behavior changes yet since nothing invokes the new builder until a follow-on PR. It touches secret/API-key handling for an internal AI gateway, but two independent agent reviews (Greptile, CodeRabbit) examined the diff and raised only a minor doc-completeness nit, with a corroborating 👍 reaction, giving adequate independent assurance for that sensitive surface.
- 👍 on the PR from greptile-apps[bot].
- CodeRabbit flagged that the README doesn't state the full gateway-selection rule (also requires AI_GATEWAY_API_KEY and https) — cosmetic/doc-only, not blocking.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 565L, 4F substantive, 752L/7F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (752L, 7F, single-area, feat) |
| stamphog 2.1.0 | .stamphog/policy.yml @ b65b97c · reviewed head b65b97c |
|
😎 Stack merged successfully - details. |
Problem
posthog/hogference/jevk5-fp8-0.2, a Jev build PostHog hosts, onPOST /v1/systemone.build_openai_clientreaches the gateway for chat models.Changes
build_system_one_clientreturns a client for the ai-gateway whereAI_GATEWAY_URLandAI_GATEWAY_API_KEYare set.TypeSafeFallbackgets TypeSafe where no gateway is configured, through the existing egress budget.SystemOneNotConfigured, so customer data cannot reach a third party by accident.X-PostHog-*attribution headers, and bills the key owner's wallet.posthog/egress/typesafe/toposthog/llm/system_one.py, so both servers share them. The TypeSafe errors now subclass the shared ones.products/ml_inferencekeeps its own gateway client for now. It could move to this builder once its owners agree.How did you test this code?
posthog/llm/test_system_one_client.pycatches these regressions:/v1/systemone, the bearer, or the attribution headers;Release status
Automatic notifications
Docs update
posthog/egress/typesafe/README.mdnow points callers at the shared types and the builder.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5
🤖 Generated with Claude Code