feat: add evaluations module scaffold, credentials, LD API client, result types - #39
feat: add evaluations module scaffold, credentials, LD API client, result types#39donei003 wants to merge 1 commit into
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| api_client = LDApiClient( | ||
| api_token=token, | ||
| base_uri=base_uri or _env("LD_BASE_URI") or DEFAULT_BASE_URI, | ||
| transport=transport, | ||
| ) |
There was a problem hiding this comment.
🟡 Custom LaunchDarkly host setting for flag delivery is reused for management API calls, sending requests to the wrong server
The evaluations client picks up the same host setting already used for flag delivery (_env("LD_BASE_URI") at packages/client/src/launchdarkly_ai_server/evaluations/module.py:74) even though the two point at different LaunchDarkly services, so anyone who configured a streaming/relay host will have their evaluation requests sent to a server that cannot answer them.
Impact: Users with a relay proxy or staging streaming endpoint configured get failing or misdirected evaluation API calls instead of reaching the LaunchDarkly management API.
Env var collision between SDK polling base URI and /api/v2 base URI
packages/client/src/launchdarkly_ai_server/lifecycle.py:175 already consumes LD_BASE_URI as the SDK polling/streaming base URI (documented in packages/client/README.md:36 as "Override the LaunchDarkly polling base URI"). The evaluations module reuses the same variable but appends /api/v2/... (packages/client/src/launchdarkly_ai_server/evaluations/api.py:93), which targets the public management API (default https://app.launchdarkly.com). A user pointing LD_BASE_URI at e.g. a relay proxy or https://sdk.launchdarkly.com would silently redirect all evaluations API traffic there. A distinct variable (e.g. LD_API_BASE_URI) avoids the conflict.
| api_client = LDApiClient( | |
| api_token=token, | |
| base_uri=base_uri or _env("LD_BASE_URI") or DEFAULT_BASE_URI, | |
| transport=transport, | |
| ) | |
| api_client = LDApiClient( | |
| api_token=token, | |
| base_uri=base_uri or _env("LD_API_BASE_URI") or DEFAULT_BASE_URI, | |
| transport=transport, | |
| ) |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
AIC-3077 — the scaffold for the Phase 2 SDK-run evaluations harness, in Tier 0
launchdarkly_ai_serversince the harness is provider-independent (it only speaks LD HTTP; provider work stays behind theProviderHandlercontract).Three pieces, no harness logic yet (
run()lands with a later milestone):init_evaluations(api_token=..., sdk_key=..., base_uri=...), explicit args first, thenLD_API_TOKEN/LD_SDK_KEY/LD_BASE_URI. A missing API token raisesEvaluationsErrorat init, before any socket is opened, rather than surfacing as an opaque 401 mid-run; a missing SDK key just logs and disables traces.LDApiClient— every request carriesAuthorization: <token>, base URI is overridable for non-default instances, non-2xx raisesLDApiErrorcarrying status/method/path/body. HTTP goes through aTransportprotocol defaulting tourllib(no new dependency on the core package), which is also the seam tests swap out.EvalRunResult { passed, url, run_id, summary },RunSummary { total_rows, passed_rows, failed_rows, error_rows }, andUsage.Usagedeliberately duplicates rather than reusesUsageDict: the eval ingest wire shape isinput_tokens/output_tokens, so handler results can be forwarded verbatim.Tests are fully mocked (
RecordingTransport), and the missing-token case uses a transport that raises on any call to prove no network I/O happens.Link to Devin session: https://app.devin.ai/sessions/02393d0331464004ba78a6d1fcfcc43f
Requested by: @donei003