Commit c051f2b
authored
refactor: use common LDValueConverter and LDContextEncoder in AI SDK (#188)
## Summary
Wires configured judges into completion and agent configs so **online
evaluation actually runs**. Previously `completionConfig`/`agentConfig`
always attached `Evaluator.noop()` regardless of a config's
`judgeConfiguration`, so judges were dormant (intentionally descoped in
v1.0 — see #180). This completes that wiring.
Applications supply a `Runner` per judge via a new `AIRunnerProvider`;
the SDK builds a real `Evaluator` from the `judgeConfiguration`.
Behavior is unchanged when no provider is configured — everything falls
back to `Evaluator.noop()`.
### New type
```java
@FunctionalInterface
public interface AIRunnerProvider {
// Returns a Runner for the given judge AI Config, or null to skip this judge.
Runner create(AIJudgeConfig judgeConfig);
}
```
The application implements this to wrap any model provider. The client
calls it once per judge key when building an `Evaluator` for a
completion or agent config that carries a `judgeConfiguration`.
### New constructor
```java
// Existing one- and two-arg constructors delegate with a null provider (online eval disabled).
public LDAIClientImpl(LDClientInterface client, LDLogger logger, AIRunnerProvider runnerProvider);
```
### `LDAIClientImpl` changes
The hard-coded `Evaluator.noop()` in the completion and agent paths
(both the resolved-variation and default paths) is replaced with
`buildEvaluator(judgeConfiguration, context, variables)`. Judge configs
themselves still wire `Evaluator.noop()` internally — judges do not
evaluate themselves.
```java
private Evaluator buildEvaluator(JudgeConfiguration judgeConfig, LDContext context, Map<String, Object> variables) {
if (runnerProvider == null || judgeConfig == null || judgeConfig.getJudges().isEmpty()) {
return Evaluator.noop();
}
// For each judge key: fetch its AI Config (Mode.JUDGE), get a Runner from the
// provider, build a Judge. Skip (and log) disabled judges, null runners, or any
// construction failure so one bad judge never blocks the parent config.
// ...
return judges.isEmpty() ? Evaluator.noop() : new Evaluator(judges, judgeConfig, logger);
}
```
Per-judge sampling rates continue to flow from the `JudgeConfiguration`
through `Evaluator.evaluate` — the `JudgeConfiguration` remains the
source of truth. Judge configs are fetched through the internal
evaluation path, so building an evaluator does not emit judge
usage-metric events.
### Fallback / isolation behavior
- Returns `Evaluator.noop()` when no provider is configured, the
`judgeConfiguration` is absent/empty, or every judge fails to construct.
- A disabled judge config, a `null` runner from the provider, or an
exception during construction skips **only that judge** (logged); the
parent config is still built with the surviving judges.
### Migration
**None required.** Additive only — a new interface plus a new
constructor overload. Existing callers behave identically (noop
evaluator). Online evaluation is opt-in by passing an
`AIRunnerProvider`.
## Test plan
- [ ] `./gradlew :lib:sdk:server-ai:test` passes
- [ ] `completionConfigWithJudgeConfigAndProviderBuildsRealEvaluator` —
real (non-noop) evaluator built
- [ ] `agentConfigWithJudgeConfigAndProviderBuildsRealEvaluator` — same
for agent configs
- [ ] `completionConfigWithNoJudgesYieldsNoopEvaluator` — no
`judgeConfiguration` → noop
- [ ] `completionConfigWithNullRunnerProviderYieldsNoopEvaluator` —
two-arg constructor → noop even with a `judgeConfiguration`
- [ ] `disabledJudgeConfigIsFilteredAndEvaluatorIsNoop` — disabled judge
skipped and logged
- [ ] `nullRunnerFromProviderSkipsThatJudge` — provider returns null →
judge skipped, logged
- [ ] `throwingRunnerProviderSkipsThatJudgeButKeepsOthers` — one bad
judge skipped, others survive
## Additional context
- Completes the online-evaluation wiring deferred from v1.0 in #180.
- When a provider is configured, each judge in a config's
`judgeConfiguration` is fetched and constructed eagerly at
config-retrieval time (one flag evaluation per judge key), even if the
evaluator is never invoked.
- Known follow-up (out of scope): unlike js-core, Java does not yet
reserve `message_history` / `response_to_evaluate` variables or strip
legacy judge template messages before evaluation. Not a regression.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Internal refactor behind existing parsers/interpolation; risk is
mainly behavioral parity if shared encoder/converter semantics differ
slightly from the removed local code.
>
> **Overview**
> **Replaces duplicated server-ai internals with
`launchdarkly-java-sdk-common` (2.5.0).**
>
> `server-ai` now depends on **`launchdarkly-java-sdk-common`** and
drops the local **`LDValueConverter`** implementation (and its unit
tests). **`AIConfigParser`** still maps model/tool JSON via
**`LDValueConverter.toMap`**, but that type now comes from
**`com.launchdarkly.sdk`**.
>
> **`Interpolator`** no longer builds the **`ldctx`** Mustache variable
with private **`contextToMap`** / **`singleContextToMap`** helpers; it
uses **`LDContextEncoder.encode(context)`** instead, while keeping the
same rule that caller-supplied **`ldctx`** is overridden.
>
> No public API changes; this is internal parsing/interpolation
alignment with other Java SDKs.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
3ca7b7f. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->1 parent 26d01e8 commit c051f2b
5 files changed
Lines changed: 4 additions & 237 deletions
File tree
- lib/sdk/server-ai
- src
- main/java/com/launchdarkly/sdk/server/ai/internal
- test/java/com/launchdarkly/sdk/server/ai/internal
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
| 55 | + | |
55 | 56 | | |
56 | 57 | | |
57 | 58 | | |
| |||
Lines changed: 1 addition & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
| |||
Lines changed: 2 additions & 48 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
| |||
61 | 62 | | |
62 | 63 | | |
63 | 64 | | |
64 | | - | |
| 65 | + | |
65 | 66 | | |
66 | 67 | | |
67 | 68 | | |
| |||
83 | 84 | | |
84 | 85 | | |
85 | 86 | | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
90 | | - | |
91 | | - | |
92 | | - | |
93 | | - | |
94 | | - | |
95 | | - | |
96 | | - | |
97 | | - | |
98 | | - | |
99 | | - | |
100 | | - | |
101 | | - | |
102 | | - | |
103 | | - | |
104 | | - | |
105 | | - | |
106 | | - | |
107 | | - | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
114 | | - | |
115 | | - | |
116 | | - | |
117 | | - | |
118 | | - | |
119 | | - | |
120 | | - | |
121 | | - | |
122 | | - | |
123 | | - | |
124 | | - | |
125 | | - | |
126 | | - | |
127 | | - | |
128 | | - | |
129 | | - | |
130 | | - | |
131 | | - | |
132 | | - | |
133 | 87 | | |
Lines changed: 0 additions & 116 deletions
This file was deleted.
Lines changed: 0 additions & 73 deletions
This file was deleted.
0 commit comments