feat(evaluations): add LD judge event support - #63
Draft
donei003 wants to merge 13 commits into
Draft
Conversation
donei003
force-pushed
the
feature/ld-judges-phase3
branch
from
September 2, 2026 20:38
e7fffd7 to
614adca
Compare
Contributor
Author
|
Addressed the feedback in the draft:
Validation after changes:
|
donei003
force-pushed
the
feature/ld-judges-phase3
branch
from
September 2, 2026 21:14
614adca to
6637d48
Compare
Contributor
Author
|
Updated for the usage feedback:
Validation:
|
donei003
force-pushed
the
feature/ld-judges-phase3
branch
from
September 2, 2026 21:56
6637d48 to
6104ae0
Compare
Contributor
Author
|
Updated Validation:
|
donei003
force-pushed
the
feature/ld-judges-phase3
branch
from
September 2, 2026 21:56
6104ae0 to
7ed3d05
Compare
Contributor
Author
|
Updated
Validation:
|
donei003
force-pushed
the
feature/ld-judges-phase3
branch
from
September 2, 2026 22:15
7ed3d05 to
ccfdc0a
Compare
Contributor
Author
|
Updated based on the latest feedback:
Validation:
|
Rename the public run() parameter judges= to criteria= (with JudgeReference -> Criterion and evaluations/judges.py -> criteria.py): the wire format already calls these criteria, and a Scorer is not a judge. Scorer callbacks now receive the public DatasetRow instead of the internal result dict, so internal key renames cannot break customer scorers; Criterion and DatasetRow are exported. Extract the judge response contract (formatting instructions, JSON score parsing, finite-number guard) into judge_scoring.py shared by the online judge path and the offline evaluations runner. The two copies had already drifted textually. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dering
- Resolve LD judges with a valid {kind: evaluation, key: project} context.
The previous empty context is invalid to the real LD SDK, so every
resolution returned the None default and failed as 'not found'. The
resolution error now also carries the underlying cause instead of
always claiming the judge does not exist.
- Validate judge scores when the judge responds: non-JSON output,
non-numeric, non-finite, and out-of-range scores become per-criterion
ERROR events with cause codes (invalid_judge_output, invalid_score,
handler_raised, generation_incomplete, scorer_raised) instead of
crashing the run at event-build time after all LLM spend.
- Pass judge configs to the handler unrendered. The handler owns the
single template pass, so {{...}} sequences inside generated output or
dataset values can no longer be expanded into the judge prompt.
Absent judge variables render as empty strings rather than leaving
literal mustache in the prompt.
- Reject duplicate criterion identities (judge keys / scorer names)
before any records are created; they would share event identity.
- Flush queued generation events in a finally so they reach LaunchDarkly
even when the criteria phase fails.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrite the evaluation event payloads as frozen dataclasses with an explicit to_track_payload(), matching the Usage/RunSummary wire pattern used everywhere else in the SDK, and remove the pydantic>=2 dependency. The models validated the SDK's own dicts, and a validation failure surfaced as a run-aborting crash at emission time; payload construction is now also wrapped per result so one bad criterion result is logged and skipped instead of dropping the whole batch. ERROR events carry a top-level errorMessage for parity with generation events. Run (row x criterion) pairs through the same ConcurrencyController and concurrency parameter the generation phase uses, instead of one criterion at a time: a 200-row dataset with 3 judges was 600 serial LLM calls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Emit per-event telemetry lines through the module logger instead of printing to the host application's stdout, refresh the run() docstring (no longer generation-only), and cover the shared judge response parser with unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The SDK has no way to learn whether a judge is lower-is-better or
upper-is-better today. Gonfalon is adding isInverted as a top-level
key in the flag-variation payload (same delivery path already used
for variationKey/version); this wires the client side up to consume
it.
- ResolvedJudge gains is_inverted, read from the resolved variation's
raw config dict (already passed through untouched by
extract_variation/parse_ai_config, so no change needed there).
- _resolve_judges populates it via config.get("isInverted"), tolerant
of the key being absent (older Gonfalon) or non-bool.
- LDJudgeEvaluationEventPayload gains success_direction, emitted as
successDirection only when non-None, matching how usage/version are
conditionally included.
- _emit_evaluation_events maps is_inverted -> "lower_is_better" /
"upper_is_better" / omitted (unresolved), never trusting client-side
direction for verdict computation - only for what gets reported.
Direction never affects the score itself; it's forwarded for
ClickHouse ingestion in ai-evaluator, which is deferred/out of scope
here.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Course change: rather than reporting a raw score/threshold/direction for ai-evaluator to compute a verdict from server-side, the SDK now computes pass/fail itself using the judge's resolved isInverted, and sends that verdict directly. A missing threshold still means "no verdict" (score-only reporting), and an unresolved direction defaults to upper-is-better, matching Gonfalon's own default for a judge with no stored isInverted. verdict lives on the shared EvaluationEventPayload base class rather than just the judge payload, since deterministic scorers will need the same field once they compute their own verdicts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
## Summary Companion to launchdarkly/gonfalon#71281, which serves a judge's `isInverted` in the flag-evaluation payload the SDK already fetches. Course change from the initial approach: rather than reporting a raw score/threshold and a `successDirection` hint for ai-evaluator to compute a verdict server-side, the SDK now computes `verdict` (`pass`/`fail`) itself and sends that directly: - `evaluations/types.py`: `ResolvedJudge` gains `is_inverted: bool | None`. - `evaluations/runner.py::_resolve_judges`: captures `isInverted` off the resolved config (absent/non-bool resolves to `None`, not an error). - `evaluations/runner.py::_run_ld_judge_for_result`: when the `Judge` has a `threshold`, computes `verdict` by comparing `score` against `threshold` using `is_inverted` (unresolved direction defaults to upper-is-better, matching Gonfalon's own default for a judge with no stored `isInverted`). No threshold set means no verdict — score-only reporting. - `evaluations/events.py`: `verdict` lives on the shared `EvaluationEventPayload` base class (not just the judge payload), since deterministic scorers will need the same field once they compute their own verdicts. Emitted as `"verdict"` on the wire, omitted when `None`. This also resolves the "how would deepeval avoid a hardcoded metric→direction table" question from review: deepeval's `metric.success` is already a computed boolean from the library itself — if/when deepeval execution moves into the SDK, it reports that boolean directly, the same pattern this PR establishes for judges. ## Test plan - [x] `uv run pytest -k judge` — 51 passed, including 5 new parametrized verdict cases (upper/lower-is-better × pass/fail, plus unresolved-direction default) - [x] Full suite: 1156 passed, 11 skipped, no regressions - [x] `mypy`, `ruff check`, `ruff format --check` clean via LD Research 🤖
Reverses the client-side verdict from #78 and finishes the wire contract around it. Why the course change back: verdict policy will keep moving -- run-level pass rates are already specified and unbuilt, and warn bands and per-criterion policy are the obvious next asks. Anything the SDK computes is frozen at each customer's installed version and cannot be re-derived, because only the answer was stored. Server-side it is one implementation that applies to every SDK version and to runs already recorded. The reason #78 gave for moving it into the SDK -- that ai-evaluator has no way to learn a judge's direction -- is answered by hydrating successDirection onto the criterion through the Gonfalon proxy instead, the same way tool versions are already pinned there. - runner/events/types: judge results carry score and reason, never a verdict; ResolvedJudge drops is_inverted and _resolve_judges no longer reads isInverted off the served config. - Event key is now $ld:ai:offline-evals:criterion, matching the worker that polls for it; payload classes renamed to match. The previous ":evaluation" name was never read by anything, so nothing was ingested. - criteria: the create body carries kind and judgeKey, without which ai-evaluator defaults the criterion to deepeval and either rejects a judge key outright or silently registers "toxicity" as a server-judged metric and dispatches it to the judging worker. - Scorer declares success_direction (default higher_is_better). A scorer has no AI Config for the proxy to read a direction from, so the caller is the only source, and ai-evaluator needs one to rule at all. - Judge.threshold defaults to 0.5. A criterion with no threshold leaves nothing to compare a score against, so it would be stored and never ruled on. - The gate now counts failed_rows. It was omissible while runs were generation-only -- a row either generated or errored, and nothing produced a fail -- and with criteria it is the normal way a run fails, so a run where every row failed its judge was exiting 0. This reverses test_generation_failed_rows_do_not_fail_the_result, which asserted the old behavior without stating a rationale. - New test pins that judges resolve once per run, not per row: extract_variation reads flag delivery, so per-row resolution would let a mid-run edit change rubric, model, and provider between rows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5 tasks
## Summary Reverses the client-side verdict from #78 and finishes the wire contract around it. Targets `feature/ld-judges-phase3` (so it stacks under #63). **Why the course change back.** Verdict policy will keep moving — run-level per-judge pass rates are already specified in the Phase 3 tech spec and unbuilt, and warn bands and per-criterion policy are the obvious next asks. Anything the SDK computes is frozen at each customer's installed SDK version and **cannot be re-derived**, because only the answer was stored. Server-side it's one implementation that applies to every SDK version *and* to runs already recorded. The reason #78 gave for moving the comparison into the SDK — that ai-evaluator has no way to learn a judge's success direction — is answered instead by hydrating `successDirection` onto the criterion through the **Gonfalon proxy**, the same mechanism that already version-pins tool refs (`NewToolsValidationMiddleware`). That also keeps the one input a verdict is ruled on server-attested, even though the score beside it is client-reported. Companions: launchdarkly/ai-evaluator#693 (criterion carries `successDirection`), launchdarkly/ai-evaluator#699 (a non-error result must carry a verdict), a Gonfalon PR for the proxy middleware. launchdarkly/gonfalon#71281 is no longer needed and can close — the SDK doesn't read `isInverted` any more. ## Contract fixes this also carries Three of these were silent breakages between the SDK and the backend, not refinements: | Fix | Without it | |---|---| | Event key → `$ld:ai:offline-evals:criterion` | The worker in ai-evaluator#692 polls for `:criterion`; the SDK emitted `:evaluation`. **Nothing was ever ingested.** | | Criteria body carries `kind` + `judgeKey` | ai-evaluator defaults an absent `kind` to `deepeval`, whose enum constrains `criterionType` — so a judge key is **rejected at create**, and `criterionType="toxicity"` is silently registered as a server-judged metric and dispatched to the judging worker. | | Gate counts `failed_rows` | `passed` was `error_rows == 0 and pending_rows == 0`. A run where **every row failed its judge exited 0.** | ## Also - **`Scorer.success_direction`** (default `higher_is_better`). A scorer has no AI Config for the proxy to read a direction off, so the caller is the only source — and ai-evaluator needs one to rule at all. Set `lower_is_better` for a scorer counting something unwanted (regex hits, edit distance). - **`Judge.threshold` defaults to 0.5.** A criterion with no threshold leaves nothing to compare a score against, so it would be stored and never ruled on. (#78's "no threshold means no verdict" is not survivable now that a NULL verdict is how ai-evaluator marks an error.) - **New test: judges resolve once per run, not per row.** `extract_variation` reads flag delivery, an in-memory store that updates within seconds of a UI edit, so per-row resolution would let a mid-run edit change rubric text, judge model, and provider *between rows of one run*. The online path (`judges.build_judge_tasks`) does resolve per invocation, so reusing it for convenience is a live way to reintroduce this. ##⚠️ One deliberate behavior reversal `test_generation_failed_rows_do_not_fail_the_result` asserted that failed rows keep `passed is True`. It's now `test_failed_rows_fail_the_result` and asserts the opposite. That assertion arrived with an unrelated polling fix and stated no rationale; it was unobservable when runs were generation-only (a row either generated or errored, and nothing produced a "failed"). Worth a second opinion if it was load-bearing for a reason not written down. ## Test plan - [x] `uv run pytest packages/client/tests -q` — 402 passed - [x] Replaced the verdict-computation test with its inverse: parametrized over `isInverted` (including the served-payload value) to pin that the SDK does not compare **even when it could** - [x] New: scorer `lower_is_better` reaches the wire verbatim; judge threshold defaults onto the criteria body; judges resolve once across a 3-row run - [x] `mypy` clean over `evaluations/`, `judge_scoring.py`, `judges.py`, `__init__.py` - [x] `ruff check` / `ruff format` clean via LD Research 🤖
The API's own duplicate-criterion check lowercases criterionType before comparing (the worker's retry gate does the same), so two criteria differing only by case still collide server-side. The SDK's local fail-fast check compared case-sensitively, so a case-variant duplicate would pass the SDK check, pay for judge resolution and a dataset fetch, and only then be rejected by the API. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Dylan has a gonfalon-judge-proxy PR adding the successDirection injection middleware but its enabling flag is off, so ai-evaluator's create-time validation 400s on a judge criterion with no successDirection. Hardcoding higher_is_better on the wire unblocks testing that proxy PR end to end. DO NOT MERGE THIS COMMIT: Judge.to_criteria_wire deliberately omits successDirection so the proxy can inject a server-attested value from the judge's AI Config; revert this alongside its two updated test assertions once the proxy lands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…_INSTRUCTIONS
_judge_variables built message_history from just input+output, then
exposed FORMATTING_INSTRUCTIONS as a separate formatting_instructions
variable. But no judge -- including every one of the AI Library's
default templates (accuracy, relevance, toxicity) and anything cloned
from them -- references {{formatting_instructions}}; they all
reference {{message_history}}, because judges.run_judges (the online
path) already bakes FORMATTING_INSTRUCTIONS into that value:
message_history = "\n\n".join(filter(None, [user_input, llm_response,
FORMATTING_INSTRUCTIONS]))
Offline diverged from that and never asked the model for the
{score, reasoning} JSON shape, so every judge response came back as
free-text prose and failed to parse (invalid_judge_output) -- for any
judge built the standard way, not just a misconfigured one. Match the
online construction so an unmodified default-template judge scores
correctly through both paths, per judge_scoring.py's own "the two
paths cannot drift" contract.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
JudgeandScorercriterion references for SDK-run evaluations, passed viarun(criteria=[...]){kind: "evaluation"}context) before mutating evaluation/run state; resolution failures fail fast and carry the underlying causeconcurrencyparameter as generation — and emit one$ld:ai:offline-evals:evaluationevent per(row, criterion)resultDesign notes
criteria=(union typeCriterion = Judge | Scorer) to match the wire format (criteria/criterionType); a scorer is not a judge.Scorer.fnreceives the publicDatasetRowdataclass plus the generated output, so internal result-dict keys are not part of the customer contract.parse_templatepass, so{{...}}sequences inside generated output or dataset values are never expanded into judge prompts.invalid_judge_output,invalid_score,handler_raised,scorer_raised,generation_incomplete) and a top-levelerrorMessage, mirroring generation events — it never aborts the run after LLM spend. Queued generation events are flushed in afinally.{score, reasoning}parsing) is shared with the online judge path viajudge_scoring.py.Notes
(row, criterionType), not via batch ingest.error.code(string cause codes here vs. numeric5001on generation events).Validation
uv run ruff format ./uv run ruff check .uv run pytest -q— 1151 passed, 11 skippeduv run mypy packages/client/src/launchdarkly_ai_server/evaluations packages/client/src/launchdarkly_ai_server/judge_scoring.py packages/client/src/launchdarkly_ai_server/judges.py packages/client/src/launchdarkly_ai_server/__init__.pyldclientthat the judge-resolution context is valid (Context.from_dict({"kind": "evaluation", "key": ...}).valid is True)Full
uv run mypy .currently fails before checking due to duplicate testconftestmodule names across packages, which appears unrelated to this change.